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

Fix concurrent transfer better #61

Merged
merged 1 commit into from
Jun 11, 2024

Conversation

mitschabaude
Copy link
Collaborator

@mitschabaude mitschabaude commented Jun 11, 2024

on top of #60, this is a more fundamental way of fixing the interaction with our token contracts

closes #56

the issue
with state.get() or state.getAndRequireEquals(), we were using o1js' lazy fetch mechanism which was a workaround for the fact that circuits could not be async. in the present case, the lazy fetch mechanism was broken, because the account to be fetched is only known after another account was fetched. in tests, this showed up as trying to fetch a dummy public key instead of the actual account, and not having the actual account later on.

solution
now that o1js supports async circuits, we no longer have to rely on the workaround. instead, we fetch the current admin public key directly inside a Provable.witnessAsync() block, and add a precondition for it using .requireEquals().

Copy link
Collaborator

@kantp kantp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mitschabaude!

@kantp kantp merged commit ef2047e into fix/concurrent-transfer Jun 11, 2024
3 checks passed
@kantp kantp deleted the fix-concurrent-transfer-2 branch June 11, 2024 14:52
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

Successfully merging this pull request may close these issues.

2 participants