Merge pull request #74 from OffchainLabs/batch-poster-manager

gzeoneth · web-flow · commit c7554852a7c4 · 2024-01-24T23:55:48.000+08:00
Add a batch poster manager for the sequencer inbox
diff --git a/src/bridge/ISequencerInbox.sol b/src/bridge/ISequencerInbox.sol
@@ -91,6 +91,10 @@ interface ISequencerInbox is IDelayedMessageProvider {
 
     function maxDataSize() external view returns (uint256);
 
+    /// @notice The batch poster manager has the ability to change the batch poster addresses
+    ///         This enables the batch poster to do key rotation
+    function batchPosterManager() external view returns (address);
+
     struct DasKeySetInfo {
         bool isValidKeyset;
         uint64 creationBlock;
@@ -210,9 +214,16 @@ interface ISequencerInbox is IDelayedMessageProvider {
      */
     function setIsSequencer(address addr, bool isSequencer_) external;
 
+    /**
+     * @notice Updates the batch poster manager, the address which has the ability to rotate batch poster keys
+     * @param newBatchPosterManager The new batch poster manager to be set
+     */
+    function setBatchPosterManager(address newBatchPosterManager) external;
+
+    /// @notice Allows the rollup owner to sync the rollup address
+    function updateRollupAddress() external;
+
     // ---------- initializer ----------
 
     function initialize(IBridge bridge_, MaxTimeVariation calldata maxTimeVariation_) external;
-
-    function updateRollupAddress() external;
 }
diff --git a/src/bridge/SequencerInbox.sol b/src/bridge/SequencerInbox.sol
@@ -22,6 +22,7 @@ import {
     AlreadyValidDASKeyset,
     NoSuchKeyset,
     NotForked,
+    NotBatchPosterManager,
     RollupNotChanged,
     DataBlobsNotSupported,
     InitParamZero,
@@ -87,6 +88,7 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
     uint256 internal constant GAS_PER_BLOB = 1 << 17;
 
     IOwnable public rollup;
+
     mapping(address => bool) public isBatchPoster;
 
     // we previously stored the max time variation in a (uint,uint,uint,uint) struct here
@@ -99,6 +101,13 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
         _;
     }
 
+    modifier onlyRollupOwnerOrBatchPosterManager() {
+        if (msg.sender != rollup.owner() && msg.sender != batchPosterManager) {
+            revert NotBatchPosterManager(msg.sender);
+        }
+        _;
+    }
+
     mapping(address => bool) public isSequencer;
     IReader4844 public immutable reader4844;
 
@@ -108,6 +117,9 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
     uint64 internal delaySeconds;
     uint64 internal futureSeconds;
 
+    /// @inheritdoc ISequencerInbox
+    address public batchPosterManager;
+
     // On L1 this should be set to 117964: 90% of Geth's 128KB tx size limit, leaving ~13KB for proving
     uint256 public immutable maxDataSize;
     uint256 internal immutable deployTimeChainId = block.chainid;
@@ -720,7 +732,10 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
     }
 
     /// @inheritdoc ISequencerInbox
-    function setIsBatchPoster(address addr, bool isBatchPoster_) external onlyRollupOwner {
+    function setIsBatchPoster(address addr, bool isBatchPoster_)
+        external
+        onlyRollupOwnerOrBatchPosterManager
+    {
         isBatchPoster[addr] = isBatchPoster_;
         emit OwnerFunctionCalled(1);
     }
@@ -756,9 +771,18 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
     }
 
     /// @inheritdoc ISequencerInbox
-    function setIsSequencer(address addr, bool isSequencer_) external onlyRollupOwner {
+    function setIsSequencer(address addr, bool isSequencer_)
+        external
+        onlyRollupOwnerOrBatchPosterManager
+    {
         isSequencer[addr] = isSequencer_;
-        emit OwnerFunctionCalled(4);
+        emit OwnerFunctionCalled(4); // Owner in this context can also be batch poster manager
+    }
+
+    /// @inheritdoc ISequencerInbox
+    function setBatchPosterManager(address newBatchPosterManager) external onlyRollupOwner {
+        batchPosterManager = newBatchPosterManager;
+        emit OwnerFunctionCalled(5);
     }
 
     function isValidKeysetHash(bytes32 ksHash) external view returns (bool) {
diff --git a/src/libraries/Error.sol b/src/libraries/Error.sol
@@ -176,6 +176,9 @@ error AlreadyValidDASKeyset(bytes32);
 /// @dev Tried to use or invalidate an already invalid Data Availability Service keyset
 error NoSuchKeyset(bytes32);
 
+/// @dev Thrown when the provided address is not the designated batch poster manager
+error NotBatchPosterManager(address);
+
 /// @dev Thrown when a data blob feature is attempted to be used on a chain that doesnt support it
 error DataBlobsNotSupported();
 
diff --git a/src/rollup/RollupCreator.sol b/src/rollup/RollupCreator.sol
@@ -35,12 +35,14 @@ contract RollupCreator is Ownable {
 
     struct RollupDeploymentParams {
         Config config;
-        address batchPoster;
         address[] validators;
         uint256 maxDataSize;
         address nativeToken;
         bool deployFactoriesToL2;
         uint256 maxFeePerGasForRetryables;
+        //// @dev The address of the batch poster, not used when set to zero address
+        address[] batchPosters;
+        address batchPosterManager;
     }
 
     BridgeCreator public bridgeCreator;
@@ -186,9 +188,12 @@ contract RollupCreator is Ownable {
             })
         );
 
-        // setting batch poster, if the address provided is not zero address
-        if (deployParams.batchPoster != address(0)) {
-            bridgeContracts.sequencerInbox.setIsBatchPoster(deployParams.batchPoster, true);
+        // Setting batch posters and batch poster manager
+        for (uint256 i = 0; i < deployParams.batchPosters.length; i++) {
+            bridgeContracts.sequencerInbox.setIsBatchPoster(deployParams.batchPosters[i], true);
+        }
+        if (deployParams.batchPosterManager != address(0)) {
+            bridgeContracts.sequencerInbox.setBatchPosterManager(deployParams.batchPosterManager);
         }
 
         // Call setValidator on the newly created rollup contract just if validator set is not empty
diff --git a/test/contract/arbRollup.spec.ts b/test/contract/arbRollup.spec.ts
@@ -86,6 +86,7 @@ const dummy4844Reader = '0x0000000000000000000000000000000000000089'
 
 // let rollup: RollupContract
 let rollup: RollupContract
+let batchPosterManager: Signer
 let rollupUser: RollupUserLogic
 let rollupAdmin: RollupAdminLogic
 let bridge: Bridge
@@ -132,6 +133,7 @@ const setup = async () => {
   const val3 = accounts[4]
   const val4 = accounts[5]
   sequencer = accounts[6]
+  const batchPosterManager = accounts[7]
 
   const oneStep0Fac = (await ethers.getContractFactory(
     'OneStepProver0'
@@ -285,7 +287,7 @@ const setup = async () => {
 
   const deployParams = {
     config: await getDefaultConfig(),
-    batchPoster: await sequencer.getAddress(),
+    batchPosters: [await sequencer.getAddress()],
     validators: [
       await val1.getAddress(),
       await val2.getAddress(),
@@ -296,6 +298,7 @@ const setup = async () => {
     nativeToken: ethers.constants.AddressZero,
     deployFactoriesToL2: true,
     maxFeePerGasForRetryables: maxFeePerGas,
+    batchPosterManager: await batchPosterManager.getAddress(),
   }
 
   const response = await rollupCreator.createRollup(deployParams, {
@@ -322,6 +325,10 @@ const setup = async () => {
     )) as SequencerInbox__factory
   ).attach(rollupCreatedEvent.sequencerInbox)
 
+  await sequencerInbox
+    .connect(await impersonateAccount(rollupCreatedEvent.upgradeExecutor))
+    .setBatchPosterManager(await batchPosterManager.getAddress())
+
   challengeManager = (
     (await ethers.getContractFactory(
       'ChallengeManager'
@@ -346,6 +353,7 @@ const setup = async () => {
     delayedBridge: rollupCreatedEvent.bridge,
     delayedInbox: rollupCreatedEvent.inboxAddress,
     bridge,
+    batchPosterManager,
     upgradeExecutorAddress: rollupCreatedEvent.upgradeExecutor,
     adminproxy: rollupCreatedEvent.adminProxy,
   }
@@ -524,6 +532,7 @@ describe('ArbRollup', () => {
       bridge: bridgeContract,
       admin: adminI,
       validators: validatorsI,
+      batchPosterManager: batchPosterManagerI,
       upgradeExecutorAddress,
       adminproxy: adminproxyAddress,
     } = await setup()
@@ -535,6 +544,7 @@ describe('ArbRollup', () => {
     upgradeExecutor = upgradeExecutorAddress
     adminproxy = adminproxyAddress
     rollup = new RollupContract(rollupUser.connect(validators[0]))
+    batchPosterManager = batchPosterManagerI
   })
 
   it('should only initialize once', async function () {
@@ -1125,6 +1135,7 @@ describe('ArbRollup', () => {
       rollupUser: rollupUserContract,
       admin: adminI,
       validators: validatorsI,
+      batchPosterManager: batchPosterManagerI,
       upgradeExecutorAddress,
     } = await setup()
     rollupAdmin = rollupAdminContract
@@ -1133,6 +1144,7 @@ describe('ArbRollup', () => {
     validators = validatorsI
     upgradeExecutor = upgradeExecutorAddress
     rollup = new RollupContract(rollupUser.connect(validators[0]))
+    batchPosterManager = batchPosterManagerI
   })
 
   it('should stake on initial node again', async function () {
@@ -1383,6 +1395,81 @@ describe('ArbRollup', () => {
     ).to.eq('view')
   })
 
+  it('can set is sequencer', async function () {
+    const testAddress = await accounts[9].getAddress()
+    expect(await sequencerInbox.isSequencer(testAddress)).to.be.false
+    await expect(
+      sequencerInbox.setIsSequencer(testAddress, true)
+    ).to.revertedWith(
+      `NotBatchPosterManager("${await sequencerInbox.signer.getAddress()}")`
+    )
+    expect(await sequencerInbox.isSequencer(testAddress)).to.be.false
+
+    await (
+      await sequencerInbox
+        .connect(batchPosterManager)
+        .setIsSequencer(testAddress, true)
+    ).wait()
+
+    expect(await sequencerInbox.isSequencer(testAddress)).to.be.true
+
+    await (
+      await sequencerInbox
+        .connect(batchPosterManager)
+        .setIsSequencer(testAddress, false)
+    ).wait()
+
+    expect(await sequencerInbox.isSequencer(testAddress)).to.be.false
+  })
+
+  it('can set a batch poster', async function () {
+    const testAddress = await accounts[9].getAddress()
+    expect(await sequencerInbox.isBatchPoster(testAddress)).to.be.false
+    await expect(
+      sequencerInbox.setIsBatchPoster(testAddress, true)
+    ).to.revertedWith(
+      `NotBatchPosterManager("${await sequencerInbox.signer.getAddress()}")`
+    )
+    expect(await sequencerInbox.isBatchPoster(testAddress)).to.be.false
+
+    await (
+      await sequencerInbox
+        .connect(batchPosterManager)
+        .setIsBatchPoster(testAddress, true)
+    ).wait()
+
+    expect(await sequencerInbox.isBatchPoster(testAddress)).to.be.true
+
+    await (
+      await sequencerInbox
+        .connect(batchPosterManager)
+        .setIsBatchPoster(testAddress, false)
+    ).wait()
+
+    expect(await sequencerInbox.isBatchPoster(testAddress)).to.be.false
+  })
+
+  it('can set batch poster manager', async function () {
+    const testManager = await accounts[8].getAddress()
+    expect(await sequencerInbox.batchPosterManager()).to.eq(
+      await batchPosterManager.getAddress()
+    )
+    await expect(
+      sequencerInbox.connect(accounts[8]).setBatchPosterManager(testManager)
+    ).to.revertedWith(`NotOwner("${testManager}", "${upgradeExecutor}")`)
+    expect(await sequencerInbox.batchPosterManager()).to.eq(
+      await batchPosterManager.getAddress()
+    )
+
+    await (
+      await sequencerInbox
+        .connect(await impersonateAccount(upgradeExecutor))
+        .setBatchPosterManager(testManager)
+    ).wait()
+
+    expect(await sequencerInbox.batchPosterManager()).to.eq(testManager)
+  })
+
   it('should fail the chainid fork check', async function () {
     await expect(sequencerInbox.removeDelayAfterFork()).to.revertedWith(
       'NotForked()'
diff --git a/test/contract/sequencerInboxForceInclude.spec.ts b/test/contract/sequencerInboxForceInclude.spec.ts
@@ -225,6 +225,7 @@ describe('SequencerInboxForceInclude', async () => {
     const dummyRollup = accounts[2]
     const rollupOwner = accounts[3]
     const batchPoster = accounts[4]
+    const batchPosterManager = accounts[5]
 
     const rollupMockFac = (await ethers.getContractFactory(
       'RollupMock'
diff --git a/test/foundry/RollupCreator.t.sol b/test/foundry/RollupCreator.t.sol
diff --git a/test/storage/SequencerInbox b/test/storage/SequencerInbox
