Fix audittens h05 (#19)

l1-contracts/contracts/common/L1ContractErrors.sol

@@ -418,6 +418,8 @@ error InvalidAddress(address expected, address actual);
 error NotAllowed(address addr);
 // 0x1929b7de
 error IncorrectTokenAddressFromNTV(bytes32 assetId, address tokenAddress);
+// 0x48c5fa28
+error InvalidProofLengthForFinalNode();
 
 enum SharedBridgeKey {
     PostUpgradeFirstBatch,

l1-contracts/contracts/common/libraries/Merkle.sol

@@ -118,9 +118,6 @@ library Merkle {
     }
 
     function _validatePathLengthForSingleProof(uint256 _index, uint256 _pathLength) private pure {
-        if (_pathLength == 0) {
-            revert MerklePathEmpty();
-        }
         if (_pathLength >= 256) {
             revert MerklePathOutOfBounds();
         }

l1-contracts/contracts/state-transition/chain-deps/facets/Mailbox.sol

@@ -29,7 +29,7 @@ import {IL1AssetRouter} from "../../../bridge/asset-router/IL1AssetRouter.sol";
 import {IBridgehub} from "../../../bridgehub/IBridgehub.sol";
 
 import {IChainTypeManager} from "../../IChainTypeManager.sol";
-import {MerklePathEmpty, OnlyEraSupported, BatchNotExecuted, HashedLogIsDefault, BaseTokenGasPriceDenominatorNotSet, TransactionNotAllowed, GasPerPubdataMismatch, TooManyFactoryDeps, MsgValueTooLow} from "../../../common/L1ContractErrors.sol";
+import {InvalidProofLengthForFinalNode, MerklePathEmpty, OnlyEraSupported, BatchNotExecuted, HashedLogIsDefault, BaseTokenGasPriceDenominatorNotSet, TransactionNotAllowed, GasPerPubdataMismatch, TooManyFactoryDeps, MsgValueTooLow} from "../../../common/L1ContractErrors.sol";
 
 // While formally the following import is not used, it is needed to inherit documentation from it
 import {IZKChainBase} from "../../chain-interfaces/IZKChainBase.sol";
@@ -131,7 +131,11 @@ contract MailboxFacet is ZKChainBase, IMailbox {
 
     function _parseProofMetadata(
         bytes32[] calldata _proof
-    ) internal pure returns (uint256 proofStartIndex, uint256 logLeafProofLen, uint256 batchLeafProofLen) {
+    )
+        internal
+        pure
+        returns (uint256 proofStartIndex, uint256 logLeafProofLen, uint256 batchLeafProofLen, bool finalProofNode)
+    {
         bytes32 proofMetadata = _proof[0];
 
         // We support two formats of the proofs:
@@ -140,14 +144,15 @@ contract MailboxFacet is ZKChainBase, IMailbox {
         // - first byte: metadata version (0x01).
         // - second byte: length of the log leaf proof (the proof that the log belongs to a batch).
         // - third byte: length of the batch leaf proof (the proof that the batch belongs to another settlement layer, if any).
+        // - fourth byte: whether the current proof is the last in the links of recursive proofs for settlement layers.
         // - the rest of the bytes are zeroes.
         //
         // In the future the old version will be disabled, and only the new version will be supported.
-        // For now, we need to support both for backwards compatibility. We distinguish between those based on whether the last 29 bytes are zeroes.
-        // It is safe, since the elements of the proof are hashes and are unlikely to have 29 zero bytes in them.
+        // For now, we need to support both for backwards compatibility. We distinguish between those based on whether the last 28 bytes are zeroes.
+        // It is safe, since the elements of the proof are hashes and are unlikely to have 28 zero bytes in them.
 
-        // We shift left by 3 bytes = 24 bits to remove the top 24 bits of the metadata.
-        uint256 metadataAsUint256 = (uint256(proofMetadata) << 24);
+        // We shift left by 4 bytes = 32 bits to remove the top 32 bits of the metadata.
+        uint256 metadataAsUint256 = (uint256(proofMetadata) << 32);
 
         if (metadataAsUint256 == 0) {
             // It is the new version
@@ -160,13 +165,19 @@ contract MailboxFacet is ZKChainBase, IMailbox {
             proofStartIndex = 1;
             logLeafProofLen = uint256(uint8(proofMetadata[1]));
             batchLeafProofLen = uint256(uint8(proofMetadata[2]));
+            finalProofNode = uint256(uint8(proofMetadata[3])) != 0;
         } else {
             // It is the old version
 
             // The entire proof is a merkle path
             proofStartIndex = 0;
             logLeafProofLen = _proof.length;
             batchLeafProofLen = 0;
+            finalProofNode = true;
+        }
+
+        if (finalProofNode && batchLeafProofLen != 0) {
+            revert InvalidProofLengthForFinalNode();
         }
     }
 
@@ -213,7 +224,12 @@ contract MailboxFacet is ZKChainBase, IMailbox {
         uint256 ptr = 0;
         bytes32 chainIdLeaf;
         {
-            (uint256 proofStartIndex, uint256 logLeafProofLen, uint256 batchLeafProofLen) = _parseProofMetadata(_proof);
+            (
+                uint256 proofStartIndex,
+                uint256 logLeafProofLen,
+                uint256 batchLeafProofLen,
+                bool finalProofNode
+            ) = _parseProofMetadata(_proof);
             ptr = proofStartIndex;
 
             bytes32 batchSettlementRoot = Merkle.calculateRootMemory(
@@ -223,9 +239,9 @@ contract MailboxFacet is ZKChainBase, IMailbox {
             );
             ptr += logLeafProofLen;
 
-            // If the `batchLeafProofLen` is 0, then we assume that this is L1 contract of the top-level
+            // If the `finalProofNode` is true, then we assume that this is L1 contract of the top-level
             // in the aggregation, i.e. the batch root is stored here on L1.
-            if (batchLeafProofLen == 0) {
+            if (finalProofNode) {
                 // Double checking that the batch has been executed.
                 if (_batchNumber > s.totalBatchesExecuted) {
                     revert BatchNotExecuted(_batchNumber);

l1-contracts/test/foundry/l1/unit/concrete/Utils/Utils.sol

@@ -297,14 +297,15 @@ library Utils {
     }
 
     function getMailboxSelectors() public pure returns (bytes4[] memory) {
-        bytes4[] memory selectors = new bytes4[](7);
+        bytes4[] memory selectors = new bytes4[](8);
         selectors[0] = MailboxFacet.proveL2MessageInclusion.selector;
         selectors[1] = MailboxFacet.proveL2LogInclusion.selector;
         selectors[2] = MailboxFacet.proveL1ToL2TransactionStatus.selector;
         selectors[3] = MailboxFacet.finalizeEthWithdrawal.selector;
         selectors[4] = MailboxFacet.requestL2Transaction.selector;
         selectors[5] = MailboxFacet.bridgehubRequestL2Transaction.selector;
         selectors[6] = MailboxFacet.l2TransactionBaseCost.selector;
+        selectors[7] = MailboxFacet.proveL2LeafInclusion.selector;
         return selectors;
     }
 

l1-contracts/test/foundry/l1/unit/concrete/common/libraries/Merkle/Merkle.t.sol

@@ -57,12 +57,12 @@ contract MerkleTestTest is Test {
         testElements(elements.length - 1);
     }
 
-    function testEmptyProof_shouldRevert() public {
+    function testEmptyProof_shouldSucceed() public {
         bytes32 leaf = elements[0];
         bytes32[] memory proof;
 
-        vm.expectRevert(MerklePathEmpty.selector);
-        merkleTest.calculateRoot(proof, 0, leaf);
+        bytes32 root = merkleTest.calculateRoot(proof, 0, leaf);
+        assertEq(root, leaf);
     }
 
     function testLeafIndexTooBig_shouldRevert() public {

l1-contracts/test/foundry/l1/unit/concrete/state-transition/chain-deps/facets/Mailbox/ProvingL2LogsInclusion.t.sol

@@ -13,7 +13,12 @@ import {MurkyBase} from "murky/common/MurkyBase.sol";
 import {MerkleTest} from "contracts/dev-contracts/test/MerkleTest.sol";
 import {TxStatus} from "contracts/state-transition/chain-deps/facets/Mailbox.sol";
 import {Bridgehub} from "contracts/bridgehub/Bridgehub.sol";
+import {IBridgehub} from "contracts/bridgehub/IBridgehub.sol";
 import {MerkleTreeNoSort} from "test/foundry/l1/unit/concrete/common/libraries/Merkle/MerkleTreeNoSort.sol";
+import {MessageHashing} from "contracts/common/libraries/MessageHashing.sol";
+import {IMailbox} from "contracts/state-transition/chain-interfaces/IMailbox.sol";
+import {IGetters} from "contracts/state-transition/chain-interfaces/IGetters.sol";
+import {UtilsFacet} from "foundry-test/l1/unit/concrete/Utils/UtilsFacet.sol";
 
 contract MailboxL2LogsProve is MailboxTest {
     bytes32[] elements;
@@ -285,6 +290,89 @@ contract MailboxL2LogsProve is MailboxTest {
         assertEq(ret, true);
     }
 
+    function checkRecursiveLeafProof(RecursiveProofInfo memory proofInfo) internal returns (bool) {
+        address secondDiamondProxy = deployDiamondProxy();
+
+        IMailbox secondMailbox = IMailbox(secondDiamondProxy);
+        UtilsFacet secondUtils = UtilsFacet(secondDiamondProxy);
+        IGetters secondGetters = IGetters(secondDiamondProxy);
+
+        uint256 secondBatchNumber = secondGetters.getTotalBatchesExecuted();
+
+        (bytes32[] memory proof, bytes32 requiredRoot) = _composeRecursiveProof(
+            RecursiveProofInfo({
+                leaf: proofInfo.leaf,
+                logProof: proofInfo.logProof,
+                leafProofMask: proofInfo.leafProofMask,
+                // We override it since it is only known here
+                batchNumber: batchNumber,
+                batchProof: proofInfo.batchProof,
+                batchLeafProofMask: proofInfo.batchLeafProofMask,
+                // We override it since it is only known here
+                settlementLayerBatchNumber: secondBatchNumber,
+                settlementLayerBatchRootMask: proofInfo.settlementLayerBatchRootMask,
+                settlementLayerChainId: proofInfo.settlementLayerChainId,
+                chainIdProof: proofInfo.chainIdProof
+            })
+        );
+        utilsFacet.util_setL2LogsRootHash(secondBatchNumber, requiredRoot);
+
+        vm.mockCall(
+            address(bridgehub),
+            abi.encodeCall(IBridgehub.whitelistedSettlementLayers, (proofInfo.settlementLayerChainId)),
+            abi.encode(true)
+        );
+        vm.mockCall(
+            address(bridgehub),
+            abi.encodeCall(IBridgehub.getZKChain, (proofInfo.settlementLayerChainId)),
+            abi.encode(secondDiamondProxy)
+        );
+
+        return mailboxFacet.proveL2LeafInclusion(batchNumber, proofInfo.leafProofMask, proofInfo.leaf, proof);
+    }
+
+    function test_successRecursiveProof() external {
+        assertTrue(
+            checkRecursiveLeafProof(
+                RecursiveProofInfo({
+                    leaf: bytes32(0),
+                    logProof: bytes32Arr(2, bytes32(0), bytes32(uint256(1))),
+                    leafProofMask: 2,
+                    // We override it since it is only known here
+                    batchNumber: 0,
+                    batchProof: bytes32Arr(2, bytes32(uint256(1)), bytes32(uint256(1))),
+                    batchLeafProofMask: 1,
+                    // We override it since it is only known here
+                    settlementLayerBatchNumber: 0,
+                    settlementLayerBatchRootMask: 3,
+                    settlementLayerChainId: 255,
+                    chainIdProof: bytes32Arr(2, bytes32(uint256(1)), bytes32(uint256(0)))
+                })
+            )
+        );
+    }
+
+    function test_successRecursiveProofZeroLength() external {
+        assertTrue(
+            checkRecursiveLeafProof(
+                RecursiveProofInfo({
+                    leaf: bytes32(0),
+                    logProof: bytes32Arr(2, bytes32(0), bytes32(uint256(1))),
+                    leafProofMask: 2,
+                    // We override it since it is only known here
+                    batchNumber: 0,
+                    batchProof: bytes32Arr(0, bytes32(0), bytes32(0)),
+                    batchLeafProofMask: 0,
+                    // We override it since it is only known here
+                    settlementLayerBatchNumber: 0,
+                    settlementLayerBatchRootMask: 3,
+                    settlementLayerChainId: 255,
+                    chainIdProof: bytes32Arr(2, bytes32(uint256(1)), bytes32(uint256(0)))
+                })
+            )
+        );
+    }
+
     /// @notice Proves L1 to L2 transaction status and cross-checks new and old encoding
     function _proveL1ToL2TransactionStatus(
         bytes32 _l2TxHash,
@@ -379,13 +467,92 @@ contract MailboxL2LogsProve is MailboxTest {
         return retOldEncoding;
     }
 
+    function _composeMetadata(uint256 proofLen, uint256 batchProofLen, bool finalNode) internal pure returns (bytes32) {
+        return
+            bytes32(
+                bytes.concat(
+                    bytes1(0x01),
+                    bytes1(uint8(proofLen)),
+                    bytes1(uint8(batchProofLen)),
+                    bytes1(uint8(finalNode ? 1 : 0)),
+                    bytes28(0)
+                )
+            );
+    }
+
     /// @notice Appends the proof metadata to the log proof as if the proof is for a batch that settled on L1.
     function _appendProofMetadata(bytes32[] memory logProof) internal returns (bytes32[] memory result) {
         result = new bytes32[](logProof.length + 1);
 
-        result[0] = bytes32(bytes.concat(bytes1(0x01), bytes1(uint8(logProof.length)), bytes30(0x00)));
+        result[0] = _composeMetadata(logProof.length, 0, true);
         for (uint256 i = 0; i < logProof.length; i++) {
             result[i + 1] = logProof[i];
         }
     }
+
+    // Just quicker to type than creating new bytes32[] each time,
+    function bytes32Arr(uint256 length, bytes32 elem1, bytes32 elem2) internal pure returns (bytes32[] memory result) {
+        result = new bytes32[](length);
+        if (length > 0) {
+            result[0] = elem1;
+        }
+        if (length > 1) {
+            result[1] = elem2;
+        }
+    }
+
+    struct RecursiveProofInfo {
+        bytes32 leaf;
+        bytes32[] logProof;
+        uint256 leafProofMask;
+        uint256 batchNumber;
+        bytes32[] batchProof;
+        uint256 batchLeafProofMask;
+        uint256 settlementLayerBatchNumber;
+        uint256 settlementLayerBatchRootMask;
+        uint256 settlementLayerChainId;
+        bytes32[] chainIdProof;
+    }
+
+    function _composeRecursiveProof(
+        RecursiveProofInfo memory info
+    ) internal returns (bytes32[] memory proof, bytes32 chainBRoot) {
+        uint256 ptr;
+        proof = new bytes32[](1 + info.logProof.length + 1 + info.batchProof.length + 2 + 1 + info.chainIdProof.length);
+        proof[ptr++] = _composeMetadata(info.logProof.length, info.batchProof.length, false);
+        copyBytes32(proof, info.logProof, ptr);
+        ptr += info.logProof.length;
+
+        bytes32 batchSettlementRoot = Merkle.calculateRootMemory(info.logProof, info.leafProofMask, info.leaf);
+
+        bytes32 batchLeafHash = MessageHashing.batchLeafHash(batchSettlementRoot, info.batchNumber);
+
+        proof[ptr++] = bytes32(uint256(info.batchLeafProofMask));
+        copyBytes32(proof, info.batchProof, ptr);
+        ptr += info.batchProof.length;
+
+        bytes32 chainIdRoot = Merkle.calculateRootMemory(info.batchProof, info.batchLeafProofMask, batchLeafHash);
+
+        bytes32 chainIdLeaf = MessageHashing.chainIdLeafHash(chainIdRoot, gettersFacet.getChainId());
+
+        uint256 settlementLayerPackedBatchInfo = (info.settlementLayerBatchNumber << 128) +
+            (info.settlementLayerBatchRootMask);
+        proof[ptr++] = bytes32(settlementLayerPackedBatchInfo);
+        proof[ptr++] = bytes32(info.settlementLayerChainId);
+
+        proof[ptr++] = _composeMetadata(info.chainIdProof.length, 0, true);
+        copyBytes32(proof, info.chainIdProof, ptr);
+        ptr += info.chainIdProof.length;
+
+        // Just in case
+        require(proof.length == ptr, "Incorrect ptr");
+
+        chainBRoot = Merkle.calculateRootMemory(info.chainIdProof, info.settlementLayerBatchRootMask, chainIdLeaf);
+    }
+
+    function copyBytes32(bytes32[] memory to, bytes32[] memory from, uint256 pos) internal pure {
+        for (uint256 i = 0; i < from.length; i++) {
+            to[pos + i] = from[i];
+        }
+    }
 }

l1-contracts/test/foundry/l1/unit/concrete/state-transition/chain-deps/facets/Mailbox/_Mailbox_Shared.t.sol

@@ -23,7 +23,7 @@ contract MailboxTest is Test {
     address diamondProxy;
     address bridgehub;
 
-    function setupDiamondProxy() public virtual {
+    function deployDiamondProxy() internal returns (address proxy) {
         sender = makeAddr("sender");
         bridgehub = makeAddr("bridgehub");
         vm.deal(sender, 100 ether);
@@ -48,12 +48,17 @@ contract MailboxTest is Test {
             selectors: Utils.getGettersSelectors()
         });
 
-        diamondProxy = Utils.makeDiamondProxy(facetCuts, testnetVerifier);
+        proxy = Utils.makeDiamondProxy(facetCuts, testnetVerifier);
+        utilsFacet = UtilsFacet(proxy);
+        utilsFacet.util_setBridgehub(bridgehub);
+    }
+
+    function setupDiamondProxy() public {
+        address diamondProxy = deployDiamondProxy();
+
         mailboxFacet = IMailbox(diamondProxy);
         utilsFacet = UtilsFacet(diamondProxy);
         gettersFacet = IGetters(diamondProxy);
-
-        utilsFacet.util_setBridgehub(bridgehub);
     }
 
     // add this to be excluded from coverage report