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

hello @benji @Gwojda
here is the implementation of icrc7
Been working on the icrc30’s transaction. Will update within few days.

3 Likes

Very nice, I’ve been playing with it a bit today, and it’s very useful.

2 Likes

Hello, I’m facing an issue while implementing the transaction log for icrc30_revoke_token_approvals, in the documentation it’s mentioned:

icrc30_revoke_token_approvals Block Schema

the tx.op field MUST be "30revoke"
it MUST contain a field tx.tid: Nat
it MUST contain a field tx.from: Account
it MUST contain a field tx.spender: Account

here is how the args looked like:

pub struct RevokeTokensArgs {
    pub token_ids: Vec<u128>,
    pub from_subaccount: Option<Subaccount>,
    pub spender: Option<Account>,
    pub memo: Option<Vec<u8>>,
    pub created_at_time: Option<u64>,
}

It’s mentioned in the schema that the block should store Spender as Account,
when the Spender is None, how should I handle it?
should I iterate over every approved spender and store it as a single txn per Spender?

Can we change in the Spender in the schema to Option<Account> or Vec<Account>?

2 Likes

@cryptoschindler @benji @domwoe

1 Like

Quoting from the standard:

A null value for spender means to revoke approvals with any value for the spender.

To your question:

should I iterate over every approved spender and store it as a single txn per Spender ?

The current thinking is that for each spender you have approved a token, an ICRC-3 block is generated in the log. When you revoke with a null for a spender, you revoke all the approvals for this spender and token, so, yes, you iterate over all approved spenders.

Can we change in the Spender in the schema to Option<Account> or Vec<Account> ?

You are referring to the method signature or the ICRC-3 block schema? If you mean the further, is the reason for this that you don’t want to “find” the pending approvals in order to delete them, but already receive them as input? Like an optimization? Is it difficult / time consuming to retrieve the spenders with an approval?

Either way, we can discuss this in the next Working Group meeting.

@dieter.sommer
I was talking about when recording the transaction.

icrc30_revoke_token_approvals Block Schema

the tx.op field MUST be "30revoke"
it MUST contain a field tx.tid: Nat
it MUST contain a field tx.from: Account
it MUST contain a field tx.spender: Account

for example: A token has 10 approvals, now while recording the txn, schema says it mush contain a field of spender: Account
now should I iterate over all 10 approvals and then record it as a single transaction per approved account?
this will just fill the txn logs, is this what the standard wants?
and what my suggestion was:
how about changing tx.spender to Option<Account>
Some(Account) means a single account was revoked from approval and Null/None means all approvals were revoked!

OK, thanks for clarifying. Your suggestion does make sense, we will discuss in the WG meeting next Tuesday. Do you have any other suggestions you have found while implementing it?

Also note that we likely will update the batch API to be more powerful, see discussions in the other topic.

Also, can you join the WG meeting next Tue, 17:00 UTC+1 time?

yep, I had one more thing but it’s regarding the ICRC7
here in the icrc7_transfer function, it’s argument looks like this:

type TransferArgs = record {
    spender_subaccount: opt blob; // the subaccount of the caller (used to identify the spender)
    from : Account;
    to : Account;
    token_ids : vec nat;
    // type: leave open for now
    memo : opt blob;
    created_at_time : opt nat64;
    is_atomic : opt bool;
};

here, can we chance the from field to Option<Account>?
when it’s null, smart contract will transfer tokens from the caller(who will also be a spender) in this case.
When it’s Some(Account) then, we will check if the spender is approved to make the transfer or not.
So this thing clarifies that the list of token ids can be passed when they belong to a single owner, either from the caller or from the Some(account).
otherwise the Transaction log will have false data.
imagine one transaction, where token_ids provided are: vec {1; 2; 3; 4; 5}, all tokens are owned by different Account, but caller was approved for every tokens and in case of from field, owner of token 3’s account was provided, while the logging the transaction it will record the same from account.
although spender was able to make the transaction, but technically account specified in the from doesn’t own all the tokens.

sure, I’ll join the call

1 Like

@pramitgaha

We discussed this in our call yesterday with the outcome that we decided that it is a good idea to allow to omit the spender in a transaction log when revoking token-level approvals. This has the semantics that all matching approvals with all spenders are revoked. Only one block is written in this case.

This should be what you were asking for.

Please also note that there has been quite a big simplification in the API: All APIs are now true batch APIs and the response is flat instead of having a batch-level error possibility and then individual errors. This should be much easier overall in terms of implementation, e.g., to track txs for deduplication. This is the current proposal, see the most recent ICRC-7 and ICRC-37 proposals, links are here.

Hope this helps! Please come back to one of us in case you have further questions.

2 Likes

It seems that you are referring to a rather old draft. The method for transferring has been split into an icrc7_transfer and icrc37_tranfer_from method. Does this already answer your question?

1 Like

yep,
updated the repo again following the correct documentation.
Can you have a look over it?

1 Like

Hi all, here is the implementation using derive macroses GitHub - UncensoredGreats/NFT , I would like to get some review and improvement suggestions

1 Like