diff --git a/package.json b/package.json index eecf0d2b..3e46caeb 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@arbitrum/nitro-contracts", - "version": "1.2.0", + "version": "1.2.1", "description": "Layer 2 precompiles and rollup for Arbitrum Nitro", "author": "Offchain Labs, Inc.", "license": "BUSL-1.1", diff --git a/scripts/upgrade/deploy4844.ts b/scripts/upgrade/deploy4844.ts index e8823c2e..986636e5 100644 --- a/scripts/upgrade/deploy4844.ts +++ b/scripts/upgrade/deploy4844.ts @@ -7,58 +7,96 @@ import { deployContract, verifyContract } from '../deploymentUtils' import { maxDataSize, isUsingFeeToken } from '../config' async function main() { - const [signer] = await ethers.getSigners() - const overrides : Overrides = { - maxFeePerGas: ethers.utils.parseUnits('30', 'gwei'), - maxPriorityFeePerGas: ethers.utils.parseUnits('0.001', 'gwei') - } + const [signer] = await ethers.getSigners() + const overrides: Overrides = { + maxFeePerGas: ethers.utils.parseUnits('30', 'gwei'), + maxPriorityFeePerGas: ethers.utils.parseUnits('0.001', 'gwei'), + } - const contractFactory = new ContractFactory( - IReader4844__factory.abi, - Reader4844Bytecode, - signer - ) - const reader4844 = await contractFactory.deploy(overrides) - await reader4844.deployed() - console.log(`Reader4844 deployed at ${reader4844.address}`) + const contractFactory = new ContractFactory( + IReader4844__factory.abi, + Reader4844Bytecode, + signer + ) + const reader4844 = await contractFactory.deploy(overrides) + await reader4844.deployed() + console.log(`Reader4844 deployed at ${reader4844.address}`) - // skip verification on deployment - const sequencerInbox = await deployContract('SequencerInbox', signer, [ - maxDataSize, - reader4844.address, - isUsingFeeToken - ], false, overrides) - // SequencerInbox logic do not need to be initialized - const prover0 = await deployContract('OneStepProver0', signer, [], false, overrides) - const proverMem = await deployContract('OneStepProverMemory', signer, [], false, overrides) - const proverMath = await deployContract('OneStepProverMath', signer, [], false, overrides) - const proverHostIo = await deployContract('OneStepProverHostIo', signer, [], false, overrides) - const osp: Contract = await deployContract('OneStepProofEntry', signer, [ + // skip verification on deployment + const sequencerInbox = await deployContract( + 'SequencerInbox', + signer, + [maxDataSize, reader4844.address, isUsingFeeToken], + false, + overrides + ) + // SequencerInbox logic do not need to be initialized + const prover0 = await deployContract( + 'OneStepProver0', + signer, + [], + false, + overrides + ) + const proverMem = await deployContract( + 'OneStepProverMemory', + signer, + [], + false, + overrides + ) + const proverMath = await deployContract( + 'OneStepProverMath', + signer, + [], + false, + overrides + ) + const proverHostIo = await deployContract( + 'OneStepProverHostIo', + signer, + [], + false, + overrides + ) + const osp: Contract = await deployContract( + 'OneStepProofEntry', + signer, + [ prover0.address, proverMem.address, proverMath.address, proverHostIo.address, - ], false, overrides) - const challengeManager = await deployContract('ChallengeManager', signer, [], false, overrides) - // ChallengeManager logic do not need to be initialized + ], + false, + overrides + ) + const challengeManager = await deployContract( + 'ChallengeManager', + signer, + [], + false, + overrides + ) + // ChallengeManager logic do not need to be initialized - // verify - await verifyContract('SequencerInbox', sequencerInbox.address, [ - maxDataSize, - reader4844.address, - isUsingFeeToken - ]) - await verifyContract('OneStepProver0', prover0.address, []) - await verifyContract('OneStepProverMemory', proverMem.address, []) - await verifyContract('OneStepProverMath', proverMath.address, []) - await verifyContract('OneStepProverHostIo', proverHostIo.address, []) - await verifyContract('OneStepProofEntry', osp.address, [ - prover0.address, - proverMem.address, - proverMath.address, - proverHostIo.address, - ]) - await verifyContract('ChallengeManager', challengeManager.address, []) + // verify + await verifyContract('SequencerInbox', sequencerInbox.address, [ + maxDataSize, + reader4844.address, + isUsingFeeToken, + ]) + await verifyContract('OneStepProver0', prover0.address, []) + await verifyContract('OneStepProverMemory', proverMem.address, []) + await verifyContract('OneStepProverMath', proverMath.address, []) + await verifyContract('OneStepProverHostIo', proverHostIo.address, []) + await verifyContract('OneStepProofEntry', osp.address, [ + prover0.address, + proverMem.address, + proverMath.address, + proverHostIo.address, + ]) + await verifyContract('ChallengeManager', challengeManager.address, []) } main() diff --git a/src/bridge/ISequencerInbox.sol b/src/bridge/ISequencerInbox.sol index 45f2028e..47db30f0 100644 --- a/src/bridge/ISequencerInbox.sol +++ b/src/bridge/ISequencerInbox.sol @@ -12,10 +12,10 @@ import "./IBridge.sol"; interface ISequencerInbox is IDelayedMessageProvider { struct MaxTimeVariation { - uint64 delayBlocks; - uint64 futureBlocks; - uint64 delaySeconds; - uint64 futureSeconds; + uint256 delayBlocks; + uint256 futureBlocks; + uint256 delaySeconds; + uint256 futureSeconds; } event SequencerBatchDelivered( diff --git a/src/bridge/SequencerInbox.sol b/src/bridge/SequencerInbox.sol index 58249dae..16a65e77 100644 --- a/src/bridge/SequencerInbox.sol +++ b/src/bridge/SequencerInbox.sol @@ -10,7 +10,6 @@ import { BadPostUpgradeInit, NotOrigin, DataTooLarge, - NotRollup, DelayedBackwards, DelayedTooFar, ForceIncludeBlockTooSoon, @@ -18,7 +17,6 @@ import { IncorrectMessagePreimage, NotBatchPoster, BadSequencerNumber, - DataNotAuthenticated, AlreadyValidDASKeyset, NoSuchKeyset, NotForked, @@ -27,12 +25,10 @@ import { DataBlobsNotSupported, InitParamZero, MissingDataHashes, - InvalidBlobMetadata, NotOwner, - RollupNotChanged, - EmptyBatchData, InvalidHeaderFlag, NativeTokenMismatch, + BadMaxTimeVariation, Deprecated } from "../libraries/Error.sol"; import "./IBridge.sol"; @@ -93,7 +89,7 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox // we previously stored the max time variation in a (uint,uint,uint,uint) struct here // solhint-disable-next-line var-name-mixedcase - uint256[4] private __LEGACY_MAX_TIME_VARIATION; + ISequencerInbox.MaxTimeVariation private __LEGACY_MAX_TIME_VARIATION; mapping(bytes32 => DasKeySetInfo) public dasKeySetInfo; @@ -152,32 +148,32 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox // Assuming we would not upgrade from a version that have MaxTimeVariation all set to zero // If that is the case, postUpgradeInit do not need to be called if ( - __LEGACY_MAX_TIME_VARIATION[0] == 0 && - __LEGACY_MAX_TIME_VARIATION[1] == 0 && - __LEGACY_MAX_TIME_VARIATION[2] == 0 && - __LEGACY_MAX_TIME_VARIATION[3] == 0 + __LEGACY_MAX_TIME_VARIATION.delayBlocks == 0 && + __LEGACY_MAX_TIME_VARIATION.futureBlocks == 0 && + __LEGACY_MAX_TIME_VARIATION.delaySeconds == 0 && + __LEGACY_MAX_TIME_VARIATION.futureSeconds == 0 ) { revert AlreadyInit(); } if ( - __LEGACY_MAX_TIME_VARIATION[0] > type(uint64).max || - __LEGACY_MAX_TIME_VARIATION[1] > type(uint64).max || - __LEGACY_MAX_TIME_VARIATION[2] > type(uint64).max || - __LEGACY_MAX_TIME_VARIATION[3] > type(uint64).max + __LEGACY_MAX_TIME_VARIATION.delayBlocks > type(uint64).max || + __LEGACY_MAX_TIME_VARIATION.futureBlocks > type(uint64).max || + __LEGACY_MAX_TIME_VARIATION.delaySeconds > type(uint64).max || + __LEGACY_MAX_TIME_VARIATION.futureSeconds > type(uint64).max ) { revert BadPostUpgradeInit(); } - delayBlocks = uint64(__LEGACY_MAX_TIME_VARIATION[0]); - futureBlocks = uint64(__LEGACY_MAX_TIME_VARIATION[1]); - delaySeconds = uint64(__LEGACY_MAX_TIME_VARIATION[2]); - futureSeconds = uint64(__LEGACY_MAX_TIME_VARIATION[3]); + delayBlocks = uint64(__LEGACY_MAX_TIME_VARIATION.delayBlocks); + futureBlocks = uint64(__LEGACY_MAX_TIME_VARIATION.futureBlocks); + delaySeconds = uint64(__LEGACY_MAX_TIME_VARIATION.delaySeconds); + futureSeconds = uint64(__LEGACY_MAX_TIME_VARIATION.futureSeconds); - __LEGACY_MAX_TIME_VARIATION[0] = 0; - __LEGACY_MAX_TIME_VARIATION[1] = 0; - __LEGACY_MAX_TIME_VARIATION[2] = 0; - __LEGACY_MAX_TIME_VARIATION[3] = 0; + __LEGACY_MAX_TIME_VARIATION.delayBlocks = 0; + __LEGACY_MAX_TIME_VARIATION.futureBlocks = 0; + __LEGACY_MAX_TIME_VARIATION.delaySeconds = 0; + __LEGACY_MAX_TIME_VARIATION.futureSeconds = 0; } function initialize( @@ -201,10 +197,8 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox bridge = bridge_; rollup = bridge_.rollup(); - delayBlocks = maxTimeVariation_.delayBlocks; - futureBlocks = maxTimeVariation_.futureBlocks; - delaySeconds = maxTimeVariation_.delaySeconds; - futureSeconds = maxTimeVariation_.futureSeconds; + + _setMaxTimeVariation(maxTimeVariation_); } /// @notice Allows the rollup owner to sync the rollup address @@ -718,15 +712,29 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox return bridge.sequencerMessageCount(); } + function _setMaxTimeVariation(ISequencerInbox.MaxTimeVariation memory maxTimeVariation_) + internal + { + if ( + maxTimeVariation_.delayBlocks > type(uint64).max || + maxTimeVariation_.futureBlocks > type(uint64).max || + maxTimeVariation_.delaySeconds > type(uint64).max || + maxTimeVariation_.futureSeconds > type(uint64).max + ) { + revert BadMaxTimeVariation(); + } + delayBlocks = uint64(maxTimeVariation_.delayBlocks); + futureBlocks = uint64(maxTimeVariation_.futureBlocks); + delaySeconds = uint64(maxTimeVariation_.delaySeconds); + futureSeconds = uint64(maxTimeVariation_.futureSeconds); + } + /// @inheritdoc ISequencerInbox function setMaxTimeVariation(ISequencerInbox.MaxTimeVariation memory maxTimeVariation_) external onlyRollupOwner { - delayBlocks = maxTimeVariation_.delayBlocks; - futureBlocks = maxTimeVariation_.futureBlocks; - delaySeconds = maxTimeVariation_.delaySeconds; - futureSeconds = maxTimeVariation_.futureSeconds; + _setMaxTimeVariation(maxTimeVariation_); emit OwnerFunctionCalled(0); } diff --git a/src/libraries/Error.sol b/src/libraries/Error.sol index 7c2860fe..0d5a2a8d 100644 --- a/src/libraries/Error.sol +++ b/src/libraries/Error.sol @@ -167,9 +167,6 @@ error BadSequencerNumber(uint256 stored, uint256 received); /// @dev The sequence message number provided to this message was inconsistent with the previous one error BadSequencerMessageNumber(uint256 stored, uint256 received); -/// @dev The batch data has the inbox authenticated bit set, but the batch data was not authenticated by the inbox -error DataNotAuthenticated(); - /// @dev Tried to create an already valid Data Availability Service keyset error AlreadyValidDASKeyset(bytes32); @@ -188,15 +185,9 @@ error InitParamZero(string name); /// @dev Thrown when data hashes where expected but not where present on the tx error MissingDataHashes(); -/// @dev Thrown when the data blob meta data is invalid -error InvalidBlobMetadata(); - /// @dev Thrown when rollup is not updated with updateRollupAddress error RollupNotChanged(); -/// @dev Batch data was empty when non empty was expected -error EmptyBatchData(); - /// @dev Unsupported header flag was provided error InvalidHeaderFlag(bytes1); @@ -205,3 +196,6 @@ error NativeTokenMismatch(); /// @dev Thrown when a deprecated function is called error Deprecated(); + +/// @dev Thrown when any component of maxTimeVariation is over uint64 +error BadMaxTimeVariation(); diff --git a/src/mocks/SequencerInboxStub.sol b/src/mocks/SequencerInboxStub.sol index e78d9479..49b31702 100644 --- a/src/mocks/SequencerInboxStub.sol +++ b/src/mocks/SequencerInboxStub.sol @@ -19,10 +19,10 @@ contract SequencerInboxStub is SequencerInbox { ) SequencerInbox(maxDataSize_, reader4844_, isUsingFeeToken_) { bridge = bridge_; rollup = IOwnable(msg.sender); - delayBlocks = maxTimeVariation_.delayBlocks; - futureBlocks = maxTimeVariation_.futureBlocks; - delaySeconds = maxTimeVariation_.delaySeconds; - futureSeconds = maxTimeVariation_.futureSeconds; + delayBlocks = uint64(maxTimeVariation_.delayBlocks); + futureBlocks = uint64(maxTimeVariation_.futureBlocks); + delaySeconds = uint64(maxTimeVariation_.delaySeconds); + futureSeconds = uint64(maxTimeVariation_.futureSeconds); isBatchPoster[sequencer_] = true; } diff --git a/test/contract/arbRollup.spec.ts b/test/contract/arbRollup.spec.ts index 3571c948..26ae4a16 100644 --- a/test/contract/arbRollup.spec.ts +++ b/test/contract/arbRollup.spec.ts @@ -534,7 +534,6 @@ describe('ArbRollup', () => { validators: validatorsI, batchPosterManager: batchPosterManagerI, upgradeExecutorAddress, - adminproxy: adminproxyAddress, } = await setup() rollupAdmin = rollupAdminContract rollupUser = rollupUserContract diff --git a/test/foundry/SequencerInbox.t.sol b/test/foundry/SequencerInbox.t.sol index 5bad2eb6..92902d68 100644 --- a/test/foundry/SequencerInbox.t.sol +++ b/test/foundry/SequencerInbox.t.sol @@ -566,4 +566,49 @@ contract SequencerInboxTest is Test { abi.encodeWithSelector(SequencerInbox.postUpgradeInit.selector) ); } + + function testSetMaxTimeVariation( + uint256 delayBlocks, + uint256 futureBlocks, + uint256 delaySeconds, + uint256 futureSeconds + ) public { + vm.assume(delayBlocks <= uint256(type(uint64).max)); + vm.assume(futureBlocks <= uint256(type(uint64).max)); + vm.assume(delaySeconds <= uint256(type(uint64).max)); + vm.assume(futureSeconds <= uint256(type(uint64).max)); + (SequencerInbox seqInbox, ) = deployRollup(false); + vm.prank(rollupOwner); + seqInbox.setMaxTimeVariation( + ISequencerInbox.MaxTimeVariation({ + delayBlocks: delayBlocks, + futureBlocks: futureBlocks, + delaySeconds: delaySeconds, + futureSeconds: futureSeconds + }) + ); + } + + function testSetMaxTimeVariationOverflow( + uint256 delayBlocks, + uint256 futureBlocks, + uint256 delaySeconds, + uint256 futureSeconds + ) public { + vm.assume(delayBlocks > uint256(type(uint64).max)); + vm.assume(futureBlocks > uint256(type(uint64).max)); + vm.assume(delaySeconds > uint256(type(uint64).max)); + vm.assume(futureSeconds > uint256(type(uint64).max)); + (SequencerInbox seqInbox, ) = deployRollup(false); + vm.expectRevert(abi.encodeWithSelector(BadMaxTimeVariation.selector)); + vm.prank(rollupOwner); + seqInbox.setMaxTimeVariation( + ISequencerInbox.MaxTimeVariation({ + delayBlocks: delayBlocks, + futureBlocks: futureBlocks, + delaySeconds: delaySeconds, + futureSeconds: futureSeconds + }) + ); + } } diff --git a/test/signatures/BridgeCreator b/test/signatures/BridgeCreator index c26bbbaf..33340999 100644 --- a/test/signatures/BridgeCreator +++ b/test/signatures/BridgeCreator @@ -1,5 +1,5 @@ { - "createBridge(address,address,address,(uint64,uint64,uint64,uint64))": "871be055", + "createBridge(address,address,address,(uint256,uint256,uint256,uint256))": "b4a5cc34", "erc20BasedTemplates()": "76768ab9", "ethBasedTemplates()": "11f02227", "owner()": "8da5cb5b", diff --git a/test/signatures/RollupAdminLogic b/test/signatures/RollupAdminLogic index d00e3a1a..65a3167e 100644 --- a/test/signatures/RollupAdminLogic +++ b/test/signatures/RollupAdminLogic @@ -19,7 +19,7 @@ "getStaker(address)": "a23c44b1", "getStakerAddress(uint64)": "6ddd3744", "inbox()": "fb0e722b", - "initialize((uint64,uint64,address,uint256,bytes32,address,address,uint256,string,uint64,(uint64,uint64,uint64,uint64)),(address,address,address,address,address,address,address,address,address,address))": "27aa19bd", + "initialize((uint64,uint64,address,uint256,bytes32,address,address,uint256,string,uint64,(uint256,uint256,uint256,uint256)),(address,address,address,address,address,address,address,address,address,address))": "28ff127a", "isStaked(address)": "6177fd18", "isStakedOnLatestConfirmed(address)": "dcd030aa", "isValidator(address)": "facd743b", diff --git a/test/signatures/RollupCreator b/test/signatures/RollupCreator index 60586510..6c6f761a 100644 --- a/test/signatures/RollupCreator +++ b/test/signatures/RollupCreator @@ -1,7 +1,7 @@ { "bridgeCreator()": "f860cefa", "challengeManagerTemplate()": "9c683d10", - "createRollup(((uint64,uint64,address,uint256,bytes32,address,address,uint256,string,uint64,(uint64,uint64,uint64,uint64)),address[],uint256,address,bool,uint256,address[],address))": "25fb4420", + "createRollup(((uint64,uint64,address,uint256,bytes32,address,address,uint256,string,uint64,(uint256,uint256,uint256,uint256)),address[],uint256,address,bool,uint256,address[],address))": "331f9b0b", "l2FactoriesDeployer()": "ac0425bc", "osp()": "f26a62c6", "owner()": "8da5cb5b", diff --git a/test/signatures/SequencerInbox b/test/signatures/SequencerInbox index 9a2e7a6c..18c3db02 100644 --- a/test/signatures/SequencerInbox +++ b/test/signatures/SequencerInbox @@ -17,7 +17,7 @@ "forceInclusion(uint256,uint8,uint64[2],uint256,address,bytes32)": "f1981578", "getKeysetCreationBlock(bytes32)": "258f0495", "inboxAccs(uint256)": "d9dd67ab", - "initialize(address,(uint64,uint64,uint64,uint64))": "280c0bfa", + "initialize(address,(uint256,uint256,uint256,uint256))": "1f7a92b2", "invalidateKeysetHash(bytes32)": "84420860", "isBatchPoster(address)": "71c3e6fe", "isSequencer(address)": "6d46e987", @@ -32,7 +32,7 @@ "setBatchPosterManager(address)": "1ff64790", "setIsBatchPoster(address,bool)": "6e7df3e7", "setIsSequencer(address,bool)": "1f956632", - "setMaxTimeVariation((uint64,uint64,uint64,uint64))": "1184ed67", + "setMaxTimeVariation((uint256,uint256,uint256,uint256))": "b31761f8", "setValidKeyset(bytes)": "d1ce8da8", "totalDelayedMessagesRead()": "7fa3a40e", "updateRollupAddress()": "6ae71f12" diff --git a/test/storage/SequencerInbox b/test/storage/SequencerInbox index 54db5485..c53e775e 100644 --- a/test/storage/SequencerInbox +++ b/test/storage/SequencerInbox @@ -4,7 +4,7 @@ | bridge | contract IBridge | 1 | 0 | 20 | src/bridge/SequencerInbox.sol:SequencerInbox | | rollup | contract IOwnable | 2 | 0 | 20 | src/bridge/SequencerInbox.sol:SequencerInbox | | isBatchPoster | mapping(address => bool) | 3 | 0 | 32 | src/bridge/SequencerInbox.sol:SequencerInbox | -| __LEGACY_MAX_TIME_VARIATION | uint256[4] | 4 | 0 | 128 | src/bridge/SequencerInbox.sol:SequencerInbox | +| __LEGACY_MAX_TIME_VARIATION | struct ISequencerInbox.MaxTimeVariation | 4 | 0 | 128 | src/bridge/SequencerInbox.sol:SequencerInbox | | dasKeySetInfo | mapping(bytes32 => struct ISequencerInbox.DasKeySetInfo) | 8 | 0 | 32 | src/bridge/SequencerInbox.sol:SequencerInbox | | isSequencer | mapping(address => bool) | 9 | 0 | 32 | src/bridge/SequencerInbox.sol:SequencerInbox | | delayBlocks | uint64 | 10 | 0 | 8 | src/bridge/SequencerInbox.sol:SequencerInbox |