feat(batcher): add queue limit #1875
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Changes to gas cost
🧾 Summary (10% most significant diffs)
Full diff report 👇
|
the function was making lots of assumptions in its usage
MarcosNicolau
approved these changes
Apr 28, 2025
Member
MarcosNicolau
left a comment
MarcosNicolau
left a comment
There was a problem hiding this comment.
Tested and it works fine. I've introduced some changes to make it more readable and remove many assumptions that were not documented with hidden behavior. Also, the pr was missing a how to test section, so I've update it.
MarcosNicolau
changed the title
MauroToscano
reviewed
May 9, 2025
MauroToscano
reviewed
May 9, 2025
JuArce
reviewed
May 12, 2025
Collaborator
JuArce
left a comment
JuArce
left a comment
There was a problem hiding this comment.
Use cases:
- Queue is full (1 element). Same user
- Send proof with less fee from same user ✅ new proof is rejected
- Send proof with same fee from same user ✅ new proof is rejected
- Send proof with greater fee from same user ✅ new proof is rejected
- Queue is full (1 element). Diferent user
- Send proof with less fee from diferent user ✅ new proof is rejected
- Send proof with same fee from diferent user ✅ new proof is rejected
- Send proof with greater fee from diferent user ✅ new proof is accepted
- Queue is full (1 element). Replacement
- Send proof with less fee, same nonce from same user ✅ new proof is rejected
- Send proof with same fee, same nonce from same user ✅ new proof is accepted
- Send proof with greater fee, same nonce from same user ✅ new proof is accepted
- Queue is full (3 elements) Same user sends 3 proofs with the same fee
- Send proof with less fee from same user ✅ new proof is rejected
- Send proof with same fee from same user ✅ new proof is rejected
- Send proof with greater fee from diferent user 🚧 new proof is accepted, but the old user connection is closed, despite it has 2 proof in the queue. Only one proof is removed
JuArce
approved these changes
May 12, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Added queue limit in the batcher. When a new proof is added and the queue limit is exceded, the least priority element is removed and the WS is closed with an UnderpricedProof to the sender.
How to test
max_queue_sizein the batcher to a lower number:config-files/config-batcher.yamcd batcher/aligned cargo run --release -- deposit-to-batcher --amount 0.1ether --private_key 0x47e179ec197488593b187f80a00eb0da91f1b9d0b13f8733639f19c30a34926a cargo run --release -- deposit-to-batcher --amount 0.1ether --private_key 0x7c852118294e51e653712a81e05800f419141751be58f605c371e15141b007a610as the max_queue_limit:cd batcher/aligned cargo run --release -- submit --proving_system SP1 --proof ../../scripts/test_files/sp1/sp1_fibonacci_4_1_3.proof --vm_program ../../scripts/test_files/sp1/sp1_fibonacci_4_1_3.elf --repetitions 10 --private_key 0x7c852118294e51e653712a81e05800f419141751be58f605c371e15141b007a6 --custom_fee_estimate 15max_fee, you should see the other ones are replaced:cd batcher/aligned cargo run --release -- submit --proving_system SP1 --proof ../../scripts/test_files/sp1/sp1_fibonacci_4_1_3.proof --vm_program ../../scripts/test_files/sp1/sp1_fibonacci_4_1_3.elf --repetitions 10 --private_key 0x7c852118294e51e653712a81e05800f419141751be58f605c371e15141b007a6 --custom_fee_estimate 14max_feeis less than the lowest priority proof, they should be rejected in that case.Type of change
Checklist
testnet, everything else tostaging