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 single node tree verification bug #219

Merged
merged 3 commits into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions verify/merkle.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,21 @@ import (
"github.com/ethereum/go-ethereum/crypto"
)

// ProcessInclusionProof processes the Merkle root proof
// ProcessInclusionProof computes the merkle root hash based on the provided leaf and proof, returning the result.
Copy link
Collaborator

Choose a reason for hiding this comment

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

knit - should we link the on-chain method we're referencing?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done 6357d45

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice! Agree everytime we are mimicking onchain logic (which we should minimize and make eth_calls instead when applicable / not latency sensitive), we def need to add this as comment, because its a VERY important invariant that needs to be maintained.

// An error is returned if the proof param is malformed.
//
// index is the index of the leaf in the tree, starting from the bottom left of the tree at 0.
//
// If the proof length is 0, then the leaf hash is returned.
//
// NOTE: this method returning a nil error does NOT indicate that the proof is valid. Rather, it merely indicates that
// the proof was well-formed. The hash returned by this method must be compared to the claimed root hash, to
// determine if the proof is valid.
//
// This method is a reimplementation of the on-chain verification method [processInclusionProofKeccak]
// (https://github.com/Layr-Labs/eigenlayer-contracts/blob/dev/src/contracts/libraries/Merkle.sol#L49-L76)
func ProcessInclusionProof(proof []byte, leaf common.Hash, index uint64) (common.Hash, error) {
samlaf marked this conversation as resolved.
Show resolved Hide resolved
if len(proof) == 0 || len(proof)%32 != 0 {
if len(proof)%32 != 0 {
return common.Hash{}, errors.New("proof length should be a multiple of 32 bytes or 256 bits")
}

Expand Down
44 changes: 44 additions & 0 deletions verify/merkle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ package verify
import (
"testing"

"github.com/wealdtech/go-merkletree/v2"
"github.com/wealdtech/go-merkletree/v2/keccak256"

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/common/hexutil"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -39,3 +42,44 @@ func TestProcessInclusionProofFail(t *testing.T) {

require.NotEqual(t, expectedRoot, actualRoot.Bytes())
}

// TestProcessInclusionProofSingleNode confirms that a merkle tree containing a single node is successfully confirmed
func TestProcessInclusionProofSingleNode(t *testing.T) {
leaf, err := hexutil.Decode("0x616C6C206861696C20746865206772656174207361746F736869")
require.NotNil(t, leaf)
require.NoError(t, err)

tree, err := merkletree.NewTree(merkletree.WithData([][]byte{leaf}), merkletree.WithHashType(keccak256.New()))
require.NotNil(t, tree)
require.NoError(t, err)

merkleProof, err := tree.GenerateProofWithIndex(0, 0)
require.NotNil(t, merkleProof)
require.NoError(t, err)

// sanity check: there shouldn't be any sibling hashes for this tree
require.Equal(t, 0, len(merkleProof.Hashes))

emptyProof := make([]byte, 0)

computedRoot, err := ProcessInclusionProof(
emptyProof,
common.BytesToHash(keccak256.New().Hash(leaf)),
0)
require.NotNil(t, computedRoot)
require.NoError(t, err)
require.Equal(t, computedRoot.Bytes(), tree.Root())

// create an alternate leaf, and make sure that the inclusion proof fails the comparison check
badLeaf, err := hexutil.Decode("0xab")
require.NotNil(t, badLeaf)
require.NoError(t, err)

computedRoot, err = ProcessInclusionProof(
emptyProof,
common.BytesToHash(keccak256.New().Hash(badLeaf)),
0)
require.NotNil(t, computedRoot)
require.NoError(t, err)
require.NotEqual(t, computedRoot.Bytes(), tree.Root())
}
Loading