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

refactor(batcher): add user state & other code quality refactors #1106

Merged
merged 49 commits into from
Oct 3, 2024

Conversation

entropidelic
Copy link
Contributor

@entropidelic entropidelic commented Sep 27, 2024

This PR refactors the batcher codebase, hopefully making it more readable.

A UserState struct has been created to handle the cached state of each user who sent some proof to the batch, with the information about its nonce, the number of proofs in the batch, and the minimum fee of all the proofs that it has submitted and are currently in the batch.

The handle_message function has been refactored to be more clear in its steps:

  1. Verify integrity of the message (checking correct serialization, verify signature, verify proof length and that it is valid, etc)
  2. Perform validations related to the user state (check the user has enough balance for paying all proofs in the batch, check nonce is correct, etc)
  3. Add the new proof to the batch, updating the user state too.

Since the batch queue and the user state are updated at the same time, this should get rid of the vulnerability highlited by Least Authority in issue #1081.

Testing

Test that everything works as expected, try sending proofs with both paying and nonpaying addresses. Everything should work as expected.

NOTE: Since there were a lot of changes, it could be useful for reviewing to check the whole handle_message function and try to understand each step, which should be clearer now, and validate that it makes sense. Give it special attention since is where the most changes have been done

@entropidelic entropidelic marked this pull request as ready for review September 30, 2024 16:58
@entropidelic entropidelic self-assigned this Sep 30, 2024
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
batcher/aligned-batcher/src/lib.rs Outdated Show resolved Hide resolved
batcher/aligned-batcher/src/lib.rs Outdated Show resolved Hide resolved
batcher/aligned-batcher/src/lib.rs Outdated Show resolved Hide resolved
@uri-99 uri-99 merged commit 7d04367 into staging Oct 3, 2024
2 checks passed
@uri-99 uri-99 deleted the refactor-user-states branch October 3, 2024 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants