fix(batcher): Update Lambdaworks Merkle Trees + Update merkle leaves length check in batcher #1067
+56
−29
Conversation
This file contains 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
uri-99
requested changes
Sep 24, 2024
| @@ -804,7 +804,7 @@ impl Batcher { | |||
|
|
|||
| // FIXME(marian): This condition should be changed to current_batch_size == 0 | |||
| // once the bug in Lambdaworks merkle tree is fixed. | |||
| if current_batch_len < 2 { | |||
| if current_batch_len == 0 { | |||
| info!("Current batch is empty or length 1. Waiting for more proofs..."); | |||
There was a problem hiding this comment.
Suggested change
| info!("Current batch is empty or length 1. Waiting for more proofs..."); | |
| info!("Current batch is empty. Waiting for more proofs..."); |
| @@ -1083,7 +1090,6 @@ impl Batcher { | |||
| signatures: Vec<SignatureData>, | |||
| fee_params: CreateNewTaskFeeParams, | |||
| ) -> Result<TransactionReceipt, BatcherError> { | |||
| // pad leaves to next power of 2 | |||
| let padded_leaves = Self::pad_leaves(leaves); | |||
There was a problem hiding this comment.
Removed comment but still padding the leaves.
The idea is to remove the padding? If the padding is removed, the contract should have a change as well
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.
closes: #970
Testing:
Test local testnet as expected and confirm no errors are emitted.
Note:
I removed the padding of the merkle root leaves as specified in #979 . However, this caused the Tx to revert in the BatcherPaymentService contract with InvalidMerkleRoot error (after removing the leaves power of two check). Since the merkle tree verification was sourced from eigen https://github.com/code-423n4/2023-04-eigenlayer/blob/main/src/contracts/libraries/Merkle.sol#L129 my intuition is its failing due an invariant the leaves are a power of 2 therefore we need to preserve padding the leaves to a power of two. Removing the padding will be done in a separate pr.