Skip to content

Conversation

nadir-akhtar
Copy link
Collaborator

Motivation:

In response to a recent audit report, we are closing out Lows and Infos related to the Merkle library.

Modifications:

Result:

Cleaner, safer code

<!-- 
    🚨 ATTENTION! 🚨 
    
This PR template is REQUIRED. PRs not following this format will be
closed without review.
    
    Requirements:
- PR title must follow commit conventions:
https://www.conventionalcommits.org/en/v1.0.0/
- Label your PR with the correct type (e.g., 🐛 Bug, ✨ Enhancement, 🧪
Test, etc.)
    - Provide clear and specific details in each section
-->

**Motivation:**

As part of an audit for the Merkle library, several low and
informational findings were found. This PR consolidates all
informational findings to fix, as well as additional commits for
findings discovered along the way, with a specific commit for each
finding.

All Lows are separated into individual PRs into the
[release-dev/merkle-audit-fixes](https://github.com/Layr-Labs/eigenlayer-contracts/tree/release-dev/merkle-audit-fixes)
branch

**Modifications:**

* Completely uplifted natspec, matching current EigenLayer quality and
standards
* Four new error codes:
  * `InvalidIndex` for indices outside of the Merkle tree's max index
* `LeavesNotPowerOfTwo` for leaves (particularly for SHA256 functions)
that don't match the expected length condition
  * `NoLeaves` for an empty `leaves` array
* `NotEnoughLeaves` for leaves (particularly for SHA256 functions) that
are not 2 or greater
* Explicit return in `processInclusionProofKeccak` for 0 length proofs
to return the leaf
* Requirement in `processInclusionProof(Keccak|Sha256)` that the index
is 0, i.e. the index was for a leaf within the tree
* Refactoring in `merkleize(Sha256|Keccak)` and `getProofKeccak` of
logic for readability and performance
* Newly added `getProofSha256` to mirror `getProofKeccak` to reduce
burden of producing roots offchain
* Newly added `isPowerOfTwo` helper function
* Newly added documentation file for the Merkle library

**Result:**

* Improved documentation/natspec
* Four new error codes
* Refactored and simplified code
* Newly added `getProofSha256` and `isPowerOfTwo` functions
* Updated and added unit tests
<!-- 
    🚨 ATTENTION! 🚨 
    
This PR template is REQUIRED. PRs not following this format will be
closed without review.
    
    Requirements:
- PR title must follow commit conventions:
https://www.conventionalcommits.org/en/v1.0.0/
- Label your PR with the correct type (e.g., 🐛 Bug, ✨ Enhancement, 🧪
Test, etc.)
    - Provide clear and specific details in each section
-->

**Motivation:**

Uninitialized roots can lead to issues like the [Nomad bridge
hack](https://medium.com/nomad-xyz-blog/nomad-bridge-hack-root-cause-analysis-875ad2e5aacd).

**Modifications:**

* New error code and documentation
* Added require statements for `verifyInclusion(Keccak|SHA256)`
* Updated documentation for existing error code

**Result:**

Guard against unintialized roots being used in proofs
@nadir-akhtar nadir-akhtar force-pushed the release-dev/merkle-audit-fixes branch from 30ec964 to 2e9066c Compare August 12, 2025 21:28
@nadir-akhtar nadir-akhtar changed the title Release dev/merkle audit fixes fix(audit): merkle library audit fixes Aug 12, 2025
@nadir-akhtar nadir-akhtar requested a review from ypatil12 August 12, 2025 21:29
@nadir-akhtar nadir-akhtar merged commit dba4a18 into main Aug 13, 2025
14 of 15 checks passed
@nadir-akhtar nadir-akhtar deleted the release-dev/merkle-audit-fixes branch August 13, 2025 22:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants