Assigned: BNT-5 - ICRC-7 NFT Implementation (Rust or Motoko)

Developers Bounties
22

hello @benji @domwoe
will there be calls again regarding icrc7?

23

Hello @ttt, did you have time to address my feedback? If you’re unable to respond within 1 week we would re-assign this bounty.

24

hello @benji , if that’s the case, I also have written my own implementation. Can I take over the bounty?

25

@pramitgaha If @ttt doesn’t respond in the next 2 days then you can take it. BTW the next NFT WG meeting is Tue 18 July, 5pm CEST. You can find a calendar here Announcing Technical Working Groups - #9 by bobbylingus

1 Like
26

what’s the update about changing image property?
dom mentioned that the team is working for the update.
changing it to url: will give flexibility of uploading any type of image. that was my thinking…

27

Yes it’s changed to url preliminarily.

2 Likes
28

Hey @benji. I am sorry for delay. I’ve had some personal issues. I’ll finish addressing changes you requested by Sunday (Jul 16).

1 Like
29

here is my implementation: GitHub - pramitgaha/icrc7: ICRC7 implementation in Rust
would like to get some review, improvement suggestion…

1 Like
30

one point I couldn’t understood:

type TransferArgs = record {
    from : opt Account;     /* if supplied and is not caller then is permit transfer, if not supplied defaults to subaccount 0 of the caller principal */
    to : Account;
    token_ids : vec nat;
    // type: leave open for now
    memo : opt blob;
    created_at_time : opt nat64;
    is_atomic : opt bool;
};

if the caller and the from are same the transaction should fail?
also, the approval transfer is only activated when from is specified and allowed to transfer approved token of user specified in from field?

31

Thanks for raising those points. Some of the discussions we had prior to ICRC-2 was finalized, and we decided just to follow ICRC-2 in the WG. Those points are reflected in my latest commit.

caller: A
spender_subaccount: S1
from: principal: A subaccount: S1
→ it’s a normal transfer

caller: A
spender_subaccount: any (defaults to 0)
from: principal: B subaccount: any (defaults to 0)
→ it’s a permit transfer, accept only if (caller, spender_subaccount) has been approved enough allowance to spend from from.

caller: A
spender_subaccount: S1
from: principal: A subaccount: S2
→ it’s a permit transfer, accept only if (caller, S1) has been approved enough allowance to spend from (A, S2).

3 Likes
32

I have some questions

A) regarding Transfer operation authorization, do i have this right?

  1. if from and to is same, throw SelfTransfer
  2. go through each token in token_ids
  3. if from is the token owner and caller is the spender, then the new owner becomes to and remove the spender.
  4. if the caller or from fails the ownership test, then put the token id in unauthorized array.
  5. check if is_atomic is true (default), rollback all of the ownership changes and throw the unauthorized array.
  6. if is_atomic is false, if ownership changes even once while the rest failed, then the transfer will be considered as success.

B) For transaction deduplication, so whenever we’re throwing the TransferError we have to rollback any changes, yes?

C) The same applies to approve operation? if Fail then Rollback?

D) One of the Approve arguments is the token_ids: ?[Nat]. Am I correct to assume that if this is null, this means that the caller want to approve the spender for ALL of his tokens?

1 Like
33

A:
1: yes. that’s what I’m also doing
2: yes, you need to go through each id, there might be some token_ids that isn’t available in the collection yet. you need to handle the error.
3: I think you need to remove all the spender assigned by previous owner. this will leave backdoor for the previous owner to gain the ownership again.
4: yes…
5: another yes.
6: this is kinda open, I guess I’m collecting unauthorized and returning it as an error. but the changes made when the ownership/approval check succeed remains unchanged.

B: you’re just checking for the transaction that matches up. there is no need for state changes to no rollback needed.

C: if you do the ownership check earlier, I think there is no place for error according to the reference document.

D: yes.

1 Like
34

  1. for the Transfer deduplication process, how should we check created_at_time potential duplicate? by using == for exactly similar time? or should we compare if the A.created_at_time window and future, overlaps with B.created_at_time window and future?

  2. When can I expect the standard spec to be completed along with other update operations such as mint and burn? Do we want to also include token_ids array argument to apply many mints per update call and many burns per update call too? along with deduplication?

  3. Why is the Transfer’s token_id arg is definite, but the Approve’s token_id is optional? Can’t we make Approve’s token_id also definite so if the token_id array being empty, it can be regarded as it being null. Also, the ApprovalError is missing the CreatedInFuture { ledger_time: Nat64 } variant, but is present in ICRC-2.

  4. When should I throw the TemporarilyUnavailable error?

  5. Based on ICRC-2, I propose that we also include Expired error if the user set very low expiry time such as

    image
    image1143×223 21.9 KB

    I add PERMITTED_DRIFT since i think it’s a good idea to also give a minimum time for approval. If the user set it to exactly NOW, then the approval will be pointless because by the time the transfer gets called, it’s already expired.

i apologize for asking too much.

1 Like
35

1:
hmm, i didn’t understood your question quite well.
so I’ll try to explain you the deduplication process.
current_time - TX_window + permitted_drift <—> current_time + pemitted_drift.
the time specified in created_at_time must be between these two time or else return error.

3: that’s open for proposal.

37

following up the changes: icrc7 update fix · pramitgaha/icrc7@0f9cb16 · GitHub

@benji what’s the update for the bounty. It will be remain assigned to ttt?

38

Hey @benji, I’ve pushed an update to my repo. The code is still a little rough, I will need some time to clean it up and write some tests to make sure I’m still within the spec. I will update you in a couple of days. Again, sorry for the delay.

1 Like
39

Hello. Unfortunately, I won’t be able to finish this assignment. Good luck to the next assignee

@benji @domwoe

40

Thanks for the heads up @ttt.

We assign @pramitgaha to this bounty.

1 Like
41

Hi @domwoe, I have a question, does the Dfinity Foundation has determined to accept the icrc7 repo written by @pramitgaha as the formal icrc7 standard on IC network?

42

hello, dom is on 2 weeks vacation!