From 471b5ef76be63e694184442b5a0621b5944e2296 Mon Sep 17 00:00:00 2001 From: undercover-cactus Date: Mon, 28 Jul 2025 18:34:28 +0200 Subject: [PATCH] refactor: remove duplicate code for calculating merkle root and proof (#124) We remove `calculateMerkleRootUnsorted` and `buildMerkleProofUnsorted` and instead introduce a new parameter `sorting`. This parameter `sorting` will indicate if we need to sort the pair before hashing the new node. --- contracts/script/utils/ValidatorsUtils.sol | 4 +- contracts/src/libraries/MerkleUtils.sol | 183 +++------------------ contracts/test/SnowbridgeIntegration.t.sol | 10 +- 3 files changed, 27 insertions(+), 170 deletions(-) diff --git a/contracts/script/utils/ValidatorsUtils.sol b/contracts/script/utils/ValidatorsUtils.sol index 9a8de438..83fb696a 100644 --- a/contracts/script/utils/ValidatorsUtils.sol +++ b/contracts/script/utils/ValidatorsUtils.sol @@ -9,8 +9,8 @@ library ValidatorsUtils { uint128 id, bytes32[] memory validators ) internal pure returns (BeefyClient.ValidatorSet memory) { - // Calculate the merkle root from the validators array using the shared library - bytes32 merkleRoot = MerkleUtils.calculateMerkleRootUnsorted(validators); + // Calculate the merkle root from the validators array. We specify to not sort the pair before hashing to be compatible with Beefy Merkle Tree implementation. + bytes32 merkleRoot = MerkleUtils.calculateMerkleRoot(validators, false); // Create and return the validator set with the calculated merkle root return diff --git a/contracts/src/libraries/MerkleUtils.sol b/contracts/src/libraries/MerkleUtils.sol index 6015433f..88d236ff 100644 --- a/contracts/src/libraries/MerkleUtils.sol +++ b/contracts/src/libraries/MerkleUtils.sol @@ -9,10 +9,12 @@ library MerkleUtils { /** * @notice Calculates the Merkle root from an array of leaf nodes * @param leaves The array of leaf node hashes + * @param sorting Indicate if we should sort or no the pair before hashing. It is important when using Open Zeppelin's Merkle Tree verification function * @return The Merkle root hash */ function calculateMerkleRoot( - bytes32[] memory leaves + bytes32[] memory leaves, + bool sorting ) internal pure returns (bytes32) { // If there are no validators, return empty hash if (leaves.length == 0) { @@ -49,65 +51,13 @@ library MerkleUtils { nextLayer[nextIndex] = currentLayer[i]; nextIndex++; } else { - // Hash the pair and add to next layer - nextLayer[nextIndex] = hashPair(currentLayer[i], currentLayer[i + 1]); - nextIndex++; - } - } - - currentLayer = nextLayer; - } - - // Return the root (the only element left in currentLayer) - return currentLayer[0]; - } - - /** - * @notice Calculates the Merkle root from an array of leaf nodes, without sorting the leaves - * @param leaves The array of leaf node hashes - * @return The Merkle root hash - * @dev This is used to generate the initial merkle root for BEEFY, since it's how both the BEEFY pallet and the BEEFY relayer expect the hashing to be done - */ - function calculateMerkleRootUnsorted( - bytes32[] memory leaves - ) internal pure returns (bytes32) { - // If there are no validators, return empty hash - if (leaves.length == 0) { - return bytes32(0); - } - - // If there's only one validator, its hash is the root - if (leaves.length == 1) { - return leaves[0]; - } - - // Create a new array to hold the current layer's hashes - bytes32[] memory currentLayer = new bytes32[](leaves.length); - for (uint256 i = 0; i < leaves.length; i++) { - currentLayer[i] = leaves[i]; - } - - // Iterate until we reach the root - while (currentLayer.length > 1) { - // Calculate size of the next layer - uint256 nextLayerSize = currentLayer.length / 2; - // If there's an odd number of elements, add one more slot for the unpaired element - if (currentLayer.length % 2 == 1) { - nextLayerSize += 1; - } - - bytes32[] memory nextLayer = new bytes32[](nextLayerSize); - - // Process pairs and build the next layer - uint256 nextIndex = 0; - for (uint256 i = 0; i < currentLayer.length; i += 2) { - // If this is the last element and we have an odd number, propagate it to the next layer - if (i + 1 >= currentLayer.length) { - nextLayer[nextIndex] = currentLayer[i]; - nextIndex++; - } else { - // Hash the pair and add to next layer - nextLayer[nextIndex] = hashPairUnsorted(currentLayer[i], currentLayer[i + 1]); + // We check if the pair need to be sorted or not before hashing. Open Zeppelin Merkle Tree requires pairs to be sorted before hashing to optimize proof calculation. + // Eigen Layer use Open Zeppelin Merkle Tree lib so we need to support it too. For the rest we use `efficientHash` to generate the correct merkle root (e.g to be compatible with substrate). + if (sorting) { + nextLayer[nextIndex] = hashPair(currentLayer[i], currentLayer[i + 1]); + } else { + nextLayer[nextIndex] = efficientHash(currentLayer[i], currentLayer[i + 1]); + } nextIndex++; } } @@ -123,11 +73,13 @@ library MerkleUtils { * @notice Builds a Merkle proof for a specific leaf * @param leaves The array of leaf hashes * @param leafIndex The index of the leaf to generate a proof for + * @param sorting Indicate if we should sort or no the pair before hashing. It is important when using Open Zeppelin's Merkle Tree verification function * @return The Merkle proof as an array of hashes */ function buildMerkleProof( bytes32[] memory leaves, - uint256 leafIndex + uint256 leafIndex, + bool sorting ) internal pure returns (bytes32[] memory) { require(leaves.length > 0, "Empty leaves"); require(leafIndex < leaves.length, "Leaf index out of bounds"); @@ -182,100 +134,13 @@ library MerkleUtils { currentPosition = nextIndex; } } else { - // Normal case: pair of elements - nextLayer[nextIndex] = hashPair(currentLayer[i], currentLayer[i + 1]); - - // If our target is in this pair, add the sibling to the proof - if (currentPosition == i) { - proof[proofIndex] = currentLayer[i + 1]; - proofIndex++; - currentPosition = nextIndex; - } else if (currentPosition == i + 1) { - proof[proofIndex] = currentLayer[i]; - proofIndex++; - currentPosition = nextIndex; + // We check if the pair need to be sorted or not before hashing. Open Zeppelin Merkle Tree requires pairs to be sorted before hashing to optimize proof calculation. + // Eigen Layer use Open Zeppelin Merkle Tree lib so we need to support it too. For the rest we use `efficientHash` to generate the correct merkle root (e.g to be compatible with substrate). + if (sorting) { + nextLayer[nextIndex] = hashPair(currentLayer[i], currentLayer[i + 1]); + } else { + nextLayer[nextIndex] = efficientHash(currentLayer[i], currentLayer[i + 1]); } - } - nextIndex++; - } - - currentLayer = nextLayer; - } - - // Resize the proof array to the actual number of elements - bytes32[] memory finalProof = new bytes32[](proofIndex); - for (uint256 i = 0; i < proofIndex; i++) { - finalProof[i] = proof[i]; - } - - return finalProof; - } - - /** - * @notice Builds a Merkle proof for a specific leaf, without sorting the leaves - * @param leaves The array of leaf hashes - * @param leafIndex The index of the leaf to generate a proof for - * @return The Merkle proof as an array of hashes - */ - function buildMerkleProofUnsorted( - bytes32[] memory leaves, - uint256 leafIndex - ) internal pure returns (bytes32[] memory) { - require(leaves.length > 0, "Empty leaves"); - require(leafIndex < leaves.length, "Leaf index out of bounds"); - - // For a single leaf, there's no proof needed - if (leaves.length == 1) { - return new bytes32[](0); - } - - // Initialize proof array with maximum possible length - // The maximum depth of a binary tree with n leaves is log2(n) rounded up - uint256 maxDepth = 0; - uint256 layerSize = leaves.length; - while (layerSize > 1) { - layerSize = (layerSize + 1) / 2; - maxDepth++; - } - - bytes32[] memory proof = new bytes32[](maxDepth); - uint256 proofIndex = 0; - - // Create a copy of the leaves array - bytes32[] memory currentLayer = new bytes32[](leaves.length); - for (uint256 i = 0; i < leaves.length; i++) { - currentLayer[i] = leaves[i]; - } - - // Track the current position of our target leaf - uint256 currentPosition = leafIndex; - - // Traverse from leaves to root - while (currentLayer.length > 1) { - // Calculate size of the next layer - uint256 nextLayerSize = currentLayer.length / 2; - if (currentLayer.length % 2 == 1) { - nextLayerSize += 1; - } - - bytes32[] memory nextLayer = new bytes32[](nextLayerSize); - - // Collect the sibling for our proof and build the next layer - uint256 nextIndex = 0; - for (uint256 i = 0; i < currentLayer.length; i += 2) { - if (i + 1 >= currentLayer.length) { - // Handle the case of an odd number of elements - nextLayer[nextIndex] = currentLayer[i]; - - // If our target is the last unpaired element - if (currentPosition == i) { - // For odd element at the end with no pair, we don't add anything to the proof here - // But we update the position for the next layer - currentPosition = nextIndex; - } - } else { - // Normal case: pair of elements - nextLayer[nextIndex] = hashPairUnsorted(currentLayer[i], currentLayer[i + 1]); // If our target is in this pair, add the sibling to the proof if (currentPosition == i) { @@ -313,16 +178,6 @@ library MerkleUtils { return a < b ? efficientHash(a, b) : efficientHash(b, a); } - /** - * @notice Hashes a pair of bytes32 values not in sorted order - * @param a First bytes32 value - * @param b Second bytes32 value - * @return The hash of the concatenated pair - */ - function hashPairUnsorted(bytes32 a, bytes32 b) internal pure returns (bytes32) { - return efficientHash(a, b); - } - /** * @notice Efficiently hashes two bytes32 values using assembly * @param a First value diff --git a/contracts/test/SnowbridgeIntegration.t.sol b/contracts/test/SnowbridgeIntegration.t.sol index 72bfbbdb..476540e9 100644 --- a/contracts/test/SnowbridgeIntegration.t.sol +++ b/contracts/test/SnowbridgeIntegration.t.sol @@ -356,7 +356,8 @@ contract SnowbridgeIntegrationTest is SnowbridgeAndAVSDeployer { leaves[i] = keccak256(abi.encode(validators[i], points[i])); } - return MerkleUtils.calculateMerkleRoot(leaves); + // We calculate the merkle root by sorting the pair before hashing (See Open Zeppelin Merkle Tree lib). + return MerkleUtils.calculateMerkleRoot(leaves, true); } function _buildValidatorPointsProof( @@ -374,7 +375,7 @@ contract SnowbridgeIntegrationTest is SnowbridgeAndAVSDeployer { leaves[i] = keccak256(abi.encode(validators[i], points[i])); } - return MerkleUtils.buildMerkleProof(leaves, leafIndex); + return MerkleUtils.buildMerkleProof(leaves, leafIndex, true); } function _buildMessagesMerkleTree( @@ -385,7 +386,8 @@ contract SnowbridgeIntegrationTest is SnowbridgeAndAVSDeployer { leaves[i] = keccak256(abi.encode(messages[i])); } - return MerkleUtils.calculateMerkleRoot(leaves); + // We calculate the merkle root by sorting the pair before hashing (See Open Zeppelin Merkle Tree lib). + return MerkleUtils.calculateMerkleRoot(leaves, true); } function _buildMessagesProof( @@ -397,6 +399,6 @@ contract SnowbridgeIntegrationTest is SnowbridgeAndAVSDeployer { leaves[i] = keccak256(abi.encode(messages[i])); } - return MerkleUtils.buildMerkleProof(leaves, leafIndex); + return MerkleUtils.buildMerkleProof(leaves, leafIndex, true); } }