From e320a7eef7d3a80d5563c2b564f5fc29da43aded Mon Sep 17 00:00:00 2001 From: Michael de Hoog Date: Tue, 7 Jan 2025 10:38:25 -1000 Subject: [PATCH] Fix OutputOracle.getL2OutputIndexAfter (#49) --- contracts/src/OutputOracle.sol | 20 ++++---- contracts/test/OutputOracle.t.sol | 84 +++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 9 deletions(-) create mode 100644 contracts/test/OutputOracle.t.sol diff --git a/contracts/src/OutputOracle.sol b/contracts/src/OutputOracle.sol index 4fe1b11..40c6e9d 100644 --- a/contracts/src/OutputOracle.sol +++ b/contracts/src/OutputOracle.sol @@ -156,6 +156,12 @@ contract OutputOracle is Initializable, ISemver { /// @notice Returns the index of the L2 output that checkpoints a given L2 block number. /// Uses a binary search to find the first output greater than or equal to the given /// block. + /// This is provided for backwards compatibility with the op-stack. However, because + /// this contract is modified to only store a fixed number of outputs, it is recommended + /// to use the `latestOutputIndex` rather than the index returned from this function, + /// as it will stay valid for longer. + /// If your withdrawal is old, the output at the index selected by this function may be + /// overwritten by the next proposal. /// @param _l2BlockNumber L2 block number to find a checkpoint for. /// @return Index of the first checkpoint that commits to the given L2 block number. function getL2OutputIndexAfter(uint256 _l2BlockNumber) public view returns (uint256) { @@ -165,24 +171,20 @@ contract OutputOracle is Initializable, ISemver { "L2OutputOracle: cannot get output for a block that has not been proposed" ); - // Make sure there's at least one output proposed. - require(l2Outputs.length > 0, "L2OutputOracle: cannot get output as no outputs have been proposed yet"); - // Find the output via binary search, guaranteed to exist. - uint256 lo = 0; - uint256 hi = l2Outputs.length; - uint256 offset = latestOutputIndex + 1 % l2Outputs.length; + uint256 offset = nextOutputIndex(); + uint256 lo = offset; + uint256 hi = offset + l2Outputs.length; while (lo < hi) { uint256 mid = (lo + hi) / 2; - uint256 m = (mid + offset) % l2Outputs.length; - if (l2Outputs[m].l2BlockNumber < _l2BlockNumber) { + if (l2Outputs[mid % l2Outputs.length].l2BlockNumber < _l2BlockNumber) { lo = mid + 1; } else { hi = mid; } } - return lo; + return lo % l2Outputs.length; } /// @notice Returns the L2 output proposal that checkpoints a given L2 block number. diff --git a/contracts/test/OutputOracle.t.sol b/contracts/test/OutputOracle.t.sol new file mode 100644 index 0000000..1e807f9 --- /dev/null +++ b/contracts/test/OutputOracle.t.sol @@ -0,0 +1,84 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.15; + +import {Test, console} from "forge-std/Test.sol"; +import {ICertManager} from "@nitro-validator/ICertManager.sol"; +import {ProxyAdmin} from "@eth-optimism-bedrock/src/universal/ProxyAdmin.sol"; + +import "../src/ResolvingProxyFactory.sol"; +import "../src/SystemConfigGlobal.sol"; +import "../src/SystemConfigOwnable.sol"; +import "../src/OutputOracle.sol"; + +contract OutputOracleTest is Test { + OutputOracle internal outputOracle; + + function setUp() public { + ProxyAdmin admin = new ProxyAdmin(address(this)); + SystemConfigGlobal scgImpl = new SystemConfigGlobal(ICertManager(address(0))); + SystemConfigGlobal scg = + SystemConfigGlobal(ResolvingProxyFactory.setupProxy(address(scgImpl), address(admin), 0x00)); + scg.initialize({_owner: address(this)}); + scg.setProposer(address(this)); + OutputOracle outputOracleImpl = new OutputOracle({_systemConfigGlobal: scg, _maxOutputCount: 6}); + outputOracle = OutputOracle(ResolvingProxyFactory.setupProxy(address(outputOracleImpl), address(admin), 0x00)); + outputOracle.initialize({ + _systemConfig: SystemConfigOwnable(address(0)), + _configHash: bytes32(0), + _genesisOutputRoot: bytes32(0), + _proofsEnabled: false + }); + } + + function test_getL2OutputIndexAfter() public { + // only genesis proposed + assertEq(outputOracle.getL2OutputIndexAfter(0), 0); + vm.expectRevert(bytes("L2OutputOracle: cannot get output for a block that has not been proposed")); + outputOracle.getL2OutputIndexAfter(1); + + // propose block 100 (index 1) + outputOracle.proposeL2Output(bytes32(uint256(1)), 100, 0, ""); + assertEq(outputOracle.getL2OutputIndexAfter(0), 0); + assertEq(outputOracle.getL2OutputIndexAfter(1), 1); + assertEq(outputOracle.getL2OutputIndexAfter(100), 1); + vm.expectRevert(bytes("L2OutputOracle: cannot get output for a block that has not been proposed")); + outputOracle.getL2OutputIndexAfter(101); + + // propose block 200 (index 2) + outputOracle.proposeL2Output(bytes32(uint256(2)), 200, 0, ""); + assertEq(outputOracle.getL2OutputIndexAfter(0), 0); + assertEq(outputOracle.getL2OutputIndexAfter(1), 1); + assertEq(outputOracle.getL2OutputIndexAfter(100), 1); + assertEq(outputOracle.getL2OutputIndexAfter(101), 2); + assertEq(outputOracle.getL2OutputIndexAfter(200), 2); + vm.expectRevert(bytes("L2OutputOracle: cannot get output for a block that has not been proposed")); + outputOracle.getL2OutputIndexAfter(201); + + // propose blocks 300 (3), 400 (4), 500 (5), 600 (0), 700 (1), 800 (2) + outputOracle.proposeL2Output(bytes32(uint256(3)), 300, 0, ""); + outputOracle.proposeL2Output(bytes32(uint256(4)), 400, 0, ""); + outputOracle.proposeL2Output(bytes32(uint256(5)), 500, 0, ""); + outputOracle.proposeL2Output(bytes32(uint256(6)), 600, 0, ""); + outputOracle.proposeL2Output(bytes32(uint256(7)), 700, 0, ""); + outputOracle.proposeL2Output(bytes32(uint256(7)), 800, 0, ""); + assertEq(outputOracle.getL2OutputIndexAfter(0), 3); + assertEq(outputOracle.getL2OutputIndexAfter(1), 3); + assertEq(outputOracle.getL2OutputIndexAfter(100), 3); + assertEq(outputOracle.getL2OutputIndexAfter(101), 3); + assertEq(outputOracle.getL2OutputIndexAfter(200), 3); + assertEq(outputOracle.getL2OutputIndexAfter(201), 3); + assertEq(outputOracle.getL2OutputIndexAfter(300), 3); + assertEq(outputOracle.getL2OutputIndexAfter(301), 4); + assertEq(outputOracle.getL2OutputIndexAfter(400), 4); + assertEq(outputOracle.getL2OutputIndexAfter(401), 5); + assertEq(outputOracle.getL2OutputIndexAfter(500), 5); + assertEq(outputOracle.getL2OutputIndexAfter(501), 0); + assertEq(outputOracle.getL2OutputIndexAfter(600), 0); + assertEq(outputOracle.getL2OutputIndexAfter(601), 1); + assertEq(outputOracle.getL2OutputIndexAfter(700), 1); + assertEq(outputOracle.getL2OutputIndexAfter(701), 2); + assertEq(outputOracle.getL2OutputIndexAfter(800), 2); + vm.expectRevert(bytes("L2OutputOracle: cannot get output for a block that has not been proposed")); + outputOracle.getL2OutputIndexAfter(801); + } +}