Merge pull request #146 from OffchainLabs/develop

release: v1.2.1

.github/workflows/contract-tests.yml

@@ -88,6 +88,9 @@ jobs:
       - name: Test Storage Layouts
         run: yarn run test:storage
 
+      - name: Test function signatures
+        run: yarn run test:signatures
+
       - name: Run coverage
         run: yarn hardhat coverage --testfiles "test/contract/*.spec.ts"
 

.gitignore

@@ -5,6 +5,7 @@ deployments/
 /test/prover/proofs/*.json
 /test/prover/spec-proofs/*.json
 /test/storage/*-old
+/test/signatures/*-old
 scripts/config.ts
 forge-cache/
 out/

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",
@@ -34,6 +34,7 @@
     "test:4844": "DISABLE_GAS_REPORTER=true hardhat --network hardhat test test/contract/*.spec.4844.ts",
     "test:compatibility": "yarn run build:0.6 && yarn run build:0.7",
     "test:storage": "./test/storage/test.bash",
+    "test:signatures": "./test/signatures/test-sigs.bash",
     "test:e2e": "hardhat test test/e2e/*.ts",
     "postinstall": "patch-package",
     "deploy-factory": "hardhat run scripts/deployment.ts",

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()

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(

src/bridge/SequencerInbox.sol

@@ -10,15 +10,13 @@ import {
     BadPostUpgradeInit,
     NotOrigin,
     DataTooLarge,
-    NotRollup,
     DelayedBackwards,
     DelayedTooFar,
     ForceIncludeBlockTooSoon,
     ForceIncludeTimeTooSoon,
     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);
     }
 

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();

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;
     }
 

test/contract/arbRollup.spec.ts

@@ -534,7 +534,6 @@ describe('ArbRollup', () => {
       validators: validatorsI,
       batchPosterManager: batchPosterManagerI,
       upgradeExecutorAddress,
-      adminproxy: adminproxyAddress,
     } = await setup()
     rollupAdmin = rollupAdminContract
     rollupUser = rollupUserContract