From 0046b36e751f93b1d185d0a8a86752a66c543e1e Mon Sep 17 00:00:00 2001 From: Harry Kalodner <412311+hkalodner@users.noreply.github.com> Date: Thu, 20 Aug 2026 15:14:15 -0400 Subject: [PATCH 1/3] Pin the parent chain hash via ArbSys on an Arbitrum host chain createNewAssertion recorded the next assertion's terminal as blockhash(block.number - 1). On an Arbitrum host chain that resolves through ProcessingHook.L1BlockHash to ArbOS's L1 block hash ring buffer, so it stored an L1 hash where afterMELState.parentChainBlockHash is a hash of this chain's own parent. The equality check in createNewAssertion then cannot pass, and because block.number - 1 sits inside the 256-block window it returns a plausible non-zero hash rather than reverting. This makes MEL unusable on any L3. Use ArbSys.arbBlockHash(arbBlockNumber() - 1) on an Arbitrum host, matching the branch this function already takes for _assertionCreatedAtArbSysBlock and the one SequencerInbox takes for keyset creation blocks. One block back is well inside arbBlockHash's 256-block window. Pairs with the nitro-side fix on fix/mel-l3-correctness. Co-Authored-By: Claude Opus 5 (1M context) --- src/rollup/RollupCore.sol | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/rollup/RollupCore.sol b/src/rollup/RollupCore.sol index d342f8ca..c0fdb7ed 100644 --- a/src/rollup/RollupCore.sol +++ b/src/rollup/RollupCore.sol @@ -506,8 +506,18 @@ abstract contract RollupCore is IRollupCore, PausableUpgradeable { "ASSERTION_SEEN" ); - // Next assertion will have to process messages from blocks up to the previous one - bytes32 nextParentChainBlockHash = blockhash(block.number - 1); + // Next assertion will have to process messages from blocks up to the previous one. + // On an Arbitrum host chain, blockhash() resolves against the L1 block hash ring + // buffer, so it would pin an L1 hash where afterMELState.parentChainBlockHash is a + // hash of this chain's parent -- the equality check above could never pass. ArbSys + // gives the parent chain hash; one block back is well inside its 256-block window. + bytes32 nextParentChainBlockHash; + if (_hostChainIsArbitrum) { + ArbSys arbSys = ArbSys(address(100)); + nextParentChainBlockHash = arbSys.arbBlockHash(arbSys.arbBlockNumber() - 1); + } else { + nextParentChainBlockHash = blockhash(block.number - 1); + } // state updates AssertionNode memory newAssertion = AssertionNodeLib.createAssertion( From b7685c7bb6bb33a6dbcca7f471daaf6fe70e1d01 Mon Sep 17 00:00:00 2001 From: Harry Kalodner <412311+hkalodner@users.noreply.github.com> Date: Fri, 21 Aug 2026 00:32:43 -0400 Subject: [PATCH 2/3] Pin the genesis terminal via ArbSys too, and test the branch Review feedback: the fix was incomplete. createNewAssertion was changed but RollupAdminLogic.initialize still computed nextParentChainBlockHash from blockhash(block.number - 1), so on an Arbitrum host chain the genesis assertion pinned an L1 ring-buffer hash and the first post-genesis assertion would still fail BAD_PARENT_CHAIN_BLOCK_HASH. The two have to agree -- genesis pins the terminal its first child must extend from -- so fixing only one of them fixes nothing. Extract _nextParentChainBlockHash() into RollupCore and call it from both, rather than copying the conditional. RollupAdminLogic already inherits RollupCore and uses _hostChainIsArbitrum twenty-five lines below the bug; the immutable is initialized at construction, so it is available during initialize. Also adds the test the branch never had. ArbitrumChecker.runningOnArbitrum returns false under Foundry unless ArbSys answers, and because _hostChainIsArbitrum is an immutable the mock has to be in place before deployment, not after. A RollupAdminLogic-derived harness exposes the helper, which is enough to exercise both branches without standing up a rollup. Reverting the fix fails the Arbitrum case and leaves the off-Arbitrum one passing, so it discriminates. 70 existing RollupTest cases still pass. Note tests need FOUNDRY_PROFILE=test; the default profile sets skip = ['test/*']. Co-Authored-By: Claude Opus 5 (1M context) --- src/rollup/RollupAdminLogic.sol | 5 +- src/rollup/RollupCore.sol | 36 ++++++++---- test/foundry/RollupParentChainHash.t.sol | 71 ++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 13 deletions(-) create mode 100644 test/foundry/RollupParentChainHash.t.sol diff --git a/src/rollup/RollupAdminLogic.sol b/src/rollup/RollupAdminLogic.sol index 2cf1144d..fe54a9ab 100644 --- a/src/rollup/RollupAdminLogic.sol +++ b/src/rollup/RollupAdminLogic.sol @@ -74,7 +74,10 @@ contract RollupAdminLogic is RollupCore, IRollupAdmin, DoubleLogicUUPSUpgradeabl afterStateHash: config.genesisAssertionState.hash() }); - bytes32 nextParentChainBlockHash = blockhash(block.number - 1); + // Must match createNewAssertion's: this pins the terminal the first post-genesis + // assertion has to extend from, and on an Arbitrum host chain blockhash() would pin + // an L1 hash that MEL's parentChainBlockHash can never equal. + bytes32 nextParentChainBlockHash = _nextParentChainBlockHash(); AssertionNode memory initialAssertion = AssertionNodeLib.createAssertion( true, RollupLib.configHash({ diff --git a/src/rollup/RollupCore.sol b/src/rollup/RollupCore.sol index c0fdb7ed..8951f829 100644 --- a/src/rollup/RollupCore.sol +++ b/src/rollup/RollupCore.sol @@ -416,6 +416,28 @@ abstract contract RollupCore is IRollupCore, PausableUpgradeable { delete _stakerMap[stakerAddress]; } + /** + * @dev The parent chain block hash to pin as the terminal an assertion's child must + * extend from. Compared against afterMELState.parentChainBlockHash when that child + * is created, so it has to be a hash of *this* chain's parent chain. + * + * On an Arbitrum host chain blockhash() resolves against the L1 block hash ring + * buffer, so it would pin an L1 hash where MEL supplies a hash of the host chain's + * parent, and the equality check could never pass. ArbSys gives the right one; a + * single block back is well inside its 256-block window. + * + * Used by both createNewAssertion and RollupAdminLogic.initialize — the genesis + * assertion pins the terminal for the first post-genesis assertion, so both have to + * agree or that first assertion cannot be created. + */ + function _nextParentChainBlockHash() internal view returns (bytes32) { + if (_hostChainIsArbitrum) { + ArbSys arbSys = ArbSys(address(100)); + return arbSys.arbBlockHash(arbSys.arbBlockNumber() - 1); + } + return blockhash(block.number - 1); + } + function createNewAssertion( AssertionInputs calldata assertion, bytes32 prevAssertionHash, @@ -506,18 +528,8 @@ abstract contract RollupCore is IRollupCore, PausableUpgradeable { "ASSERTION_SEEN" ); - // Next assertion will have to process messages from blocks up to the previous one. - // On an Arbitrum host chain, blockhash() resolves against the L1 block hash ring - // buffer, so it would pin an L1 hash where afterMELState.parentChainBlockHash is a - // hash of this chain's parent -- the equality check above could never pass. ArbSys - // gives the parent chain hash; one block back is well inside its 256-block window. - bytes32 nextParentChainBlockHash; - if (_hostChainIsArbitrum) { - ArbSys arbSys = ArbSys(address(100)); - nextParentChainBlockHash = arbSys.arbBlockHash(arbSys.arbBlockNumber() - 1); - } else { - nextParentChainBlockHash = blockhash(block.number - 1); - } + // Next assertion will have to process messages from blocks up to the previous one + bytes32 nextParentChainBlockHash = _nextParentChainBlockHash(); // state updates AssertionNode memory newAssertion = AssertionNodeLib.createAssertion( diff --git a/test/foundry/RollupParentChainHash.t.sol b/test/foundry/RollupParentChainHash.t.sol new file mode 100644 index 00000000..2fc8a3c1 --- /dev/null +++ b/test/foundry/RollupParentChainHash.t.sol @@ -0,0 +1,71 @@ +// Copyright 2026, Offchain Labs, Inc. +// For license information, see https://github.com/OffchainLabs/nitro-contracts/blob/main/LICENSE.md +// SPDX-License-Identifier: BUSL-1.1 + +pragma solidity ^0.8.0; + +import "forge-std/Test.sol"; +import "../../src/rollup/RollupAdminLogic.sol"; +import "../../src/precompiles/ArbSys.sol"; + +/// Exposes the internal helper both createNewAssertion and RollupAdminLogic.initialize use to +/// pin the terminal an assertion's child must extend from. RollupAdminLogic is concrete and +/// inherits RollupCore's `_hostChainIsArbitrum` immutable, so deploying one is enough to +/// exercise the branch without standing up a whole rollup. +contract NextParentChainBlockHashHarness is RollupAdminLogic { + function exposedNextParentChainBlockHash() external view returns (bytes32) { + return _nextParentChainBlockHash(); + } +} + +contract RollupParentChainHashTest is Test { + uint256 constant ARB_BLOCK = 12_345; + bytes32 constant ARB_BLOCK_HASH = keccak256("arbBlockHash(12344)"); + + /// `_hostChainIsArbitrum` is an immutable initialized from ArbitrumChecker at construction, + /// so ArbSys has to answer before the harness is deployed, not after. + function _mockArbitrumHost() internal { + vm.mockCall( + address(100), + abi.encodeWithSelector(ArbSys.arbOSVersion.selector), + abi.encode(uint256(11)) + ); + } + + function testUsesArbSysOnArbitrumHostChain() public { + _mockArbitrumHost(); + NextParentChainBlockHashHarness harness = new NextParentChainBlockHashHarness(); + + vm.mockCall( + address(100), + abi.encodeWithSelector(ArbSys.arbBlockNumber.selector), + abi.encode(ARB_BLOCK) + ); + vm.mockCall( + address(100), + abi.encodeWithSelector(ArbSys.arbBlockHash.selector, ARB_BLOCK - 1), + abi.encode(ARB_BLOCK_HASH) + ); + + assertEq( + harness.exposedNextParentChainBlockHash(), + ARB_BLOCK_HASH, + "must pin arbBlockHash(arbBlockNumber() - 1), not the L1 ring buffer" + ); + // The bug this guards: blockhash() on an Arbitrum host resolves against the L1 ring + // buffer, so MEL's parentChainBlockHash could never equal it. + assertTrue( + harness.exposedNextParentChainBlockHash() != blockhash(block.number - 1), + "fixture must make the two sources differ, or this test proves nothing" + ); + } + + function testUsesBlockhashOffArbitrum() public { + NextParentChainBlockHashHarness harness = new NextParentChainBlockHashHarness(); + assertEq( + harness.exposedNextParentChainBlockHash(), + blockhash(block.number - 1), + "off an Arbitrum host chain the terminal is still blockhash(block.number - 1)" + ); + } +} From b42e8911f60d7d3808328839af65ccdcf107996c Mon Sep 17 00:00:00 2001 From: Harry Kalodner <412311+hkalodner@users.noreply.github.com> Date: Fri, 21 Aug 2026 00:37:38 -0400 Subject: [PATCH 3/3] Record why the same-block guard uses different units Swept for other places the new helper belongs. There are none -- every producer of nextParentChainBlockHash now routes through it, and the two remaining references consume a stored value rather than computing one (decreaseBaseStake reconstructs an existing configHash from a caller-supplied hash; EdgeChallengeManager reads prevConfig). The one site that invites a second look is the SAME_BLOCK_ASSERTION guard, which counts L1 blocks while the hash it protects now advances with arbBlockNumber(). That mismatch is real and it is sound: every host block carries exactly one block.number, so two assertions in the same host block always see the same one and are rejected, while many host blocks can share a block.number, so the guard can only reject assertions whose hashes would in fact have differed. Over-rejects, never under-rejects, at the cost of waiting for the next L1 block on a fast-block host. Comment only -- no behaviour change. Recorded so the next reader does not have to derive it. Co-Authored-By: Claude Opus 5 (1M context) --- src/rollup/RollupCore.sol | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/rollup/RollupCore.sol b/src/rollup/RollupCore.sol index 8951f829..573fae79 100644 --- a/src/rollup/RollupCore.sol +++ b/src/rollup/RollupCore.sol @@ -482,6 +482,15 @@ abstract contract RollupCore is IRollupCore, PausableUpgradeable { { // We want to prevent multiple assertions from being created in the same block, as this would allow them to have the same `nextParentChainBlockHash`, // which would be an already processed block hash by the time the assertions are created. + // + // On an Arbitrum host chain these are deliberately different units: block.number and + // createdAtBlock are L1 block numbers, while _nextParentChainBlockHash() advances with + // arbBlockNumber(). That is still sound, and in the safe direction. Every host block + // carries exactly one block.number, so two assertions in the same host block always + // see the same one and are rejected; many host blocks can share a block.number, so the + // check can only reject assertions whose hashes would in fact have differed. It over- + // rejects, never under-rejects. The cost is waiting for the next L1 block on a + // fast-block host, far below any real assertion cadence. require((block.number - prevAssertion.createdAtBlock) >= 1, "SAME_BLOCK_ASSERTION"); // This new assertion consumes the messages from prevParentChainBlockHash to afterParentChainBlockHash