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.

Sign up for GitHub

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

Jump to bottom

Add get-nonce endpoint #146

Merged
merged 11 commits into from
Jul 16, 2024
Merged

Add get-nonce endpoint #146

merged 11 commits into from
Jul 16, 2024

Conversation

marijanp
Copy link
Contributor

@marijanp marijanp commented Jul 9, 2024

See comment in the e2e why we cannot reliably test this yet

@marijanp marijanp self-assigned this Jul 9, 2024
@koxu1996
Copy link
Contributor

koxu1996 commented Jul 12, 2024 •
edited
Loading

I am not sure about that edge case. nonce is used to count number of interactions between user and L2, and deposit is not using it, since it is initiated on L1. Should not first transfer just use nonce=0?

@marijanp
Copy link
Contributor Author

@koxu1996 after successful batch verification of a deposit the account trie will be updated by inserting an account. And that accounts nonce defaults to 0. Is that what you are referring to?

@Avi-D-coder
Copy link
Contributor

I think synchronization here is just running get nonce in a loop.
The account is added to the trie as soon as the l1 even hits the l2 server.
It does not wait until batch verification. The proof of the next batch contains the output of a matching deposit and then the pending deposit is complete.

@koxu1996
Copy link
Contributor

after successful batch verification of a deposit the account trie will be updated by inserting an account. And that accounts nonce defaults to 0. Is that what you are referring to?

Not really. Every public key you could imagine like 0000..0000 is valid account, and nonce is just number of interactions with L2. Therefore I think that nonce returned for account that does not exist in trie should be 0.

koxu1996
koxu1996 reviewed Jul 16, 2024
View reviewed changes
nixos/tests/end-to-end.nix Outdated
Comment on lines 142 to 145
# TODO we currently have to pass the nonce explicitly as we are lacking a way to synchronize
# the successful creation of an account in the account trie after a deposit.
# After adding the DA and awaiting that the server picked up the new deposit and thus
# an according account was added to the trie, we can discard the nonce and get it from the server
Copy link
Contributor

Choose a reason for hiding this comment

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

Wait mechanism for deposit is necessary indeed, but it does not have to be done with DA.

@marijanp
Copy link
Contributor Author

Don't merge yet

@marijanp
Copy link
Contributor Author

ready to merge

@Avi-D-coder Avi-D-coder merged commit abda197 into main Jul 16, 2024
4 checks passed
@marijanp marijanp deleted the add-get-nonce branch July 16, 2024 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Avi-D-coder Avi-D-coder Avi-D-coder approved these changes

@koxu1996 koxu1996 koxu1996 approved these changes

@jonas089 jonas089 Awaiting requested review from jonas089

@Rom3dius Rom3dius Awaiting requested review from Rom3dius

Assignees

@marijanp marijanp

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@marijanp @koxu1996 @Avi-D-coder