Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"icrc7_tokens_of" shows token belonging to twos principals after "icrc37_transfer_from" #3

Closed
sardariuss opened this issue Aug 8, 2024 · 2 comments

Comments

@sardariuss
Copy link

sardariuss commented Aug 8, 2024

Hello,

When transferring a token with the icrc37_transfer_from method, the method icrc7_tokens_of shows that the token belongs to both the original owner and the new owner.

Here is a scenario to test it yourself. Alice allows Bob to spend her token. Bob transfer it to Carlos. At the end, you'll see that the method icrc7_balance_of shows that both Alice and Carlos own the token:

icrc7_bug.sh.txt

@skilesare
Copy link

skilesare commented Aug 9, 2024

Great Catch!

I'll point you to the error and if you have time and want to fix it I'd welcome the pull request.

Most of this will happen in https://github.com/PanIndustrial-Org/icrc7.mo and https://github.com/PanIndustrial-Org/icrc37.mo

I have indexes for state.indexes.nft_to_owner and owners_to_nfts.

I have functions for unindexOwner and indexOwner but it looks like I miss updating state.indexes.nft_to_owner.

It also looks like I don't index on set_nft.

Now this doesn't explain the error exactly because it should be unindexing....but I think the wrong from is being referenced.

If you look here: https://github.com/PanIndustrial-Org/icrc37.mo/blob/6b05b99a4f9ff3941baeedc3948fee9fb3418b96/src/lib.mo#L1668C54-L1668C77

I pass caller in to the transfer args and I think that needs to be transferFromArgs.from.owner so that when finalize transfer gets the previous owner it uses the right principal. Fortunately the transfer is working properly, it is just the indexing that is off.

Likely we'll also need a quick utility function to reindex everything that can be run from post_upgrade when necessary.

I'll fix all of this when I get a chance, but if you want to try to tackle it first.

@sardariuss
Copy link
Author

Hey,

Thanks for the quick answer. I'll try to have a look at it soon!

panindustrial-dev added a commit to PanIndustrial-Org/icrc37.mo that referenced this issue Aug 15, 2024
panindustrial-dev added a commit that referenced this issue Aug 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants