Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a batch poster manager for the sequencer inbox #74

Merged
merged 21 commits into from
Jan 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions src/bridge/ISequencerInbox.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
30 changes: 27 additions & 3 deletions src/bridge/SequencerInbox.sol
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
AlreadyValidDASKeyset,
NoSuchKeyset,
NotForked,
NotBatchPosterManager,
RollupNotChanged,
DataBlobsNotSupported,
InitParamZero,
Expand Down Expand Up @@ -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
Expand All @@ -99,6 +101,13 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
_;
}

modifier onlyRollupOwnerOrBatchPosterManager() {
if (msg.sender != rollup.owner() && msg.sender != batchPosterManager) {
revert NotBatchPosterManager(msg.sender);
gzeoneth marked this conversation as resolved.
Show resolved Hide resolved
}
_;
}

mapping(address => bool) public isSequencer;
IReader4844 public immutable reader4844;

Expand All @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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_;
gzeoneth marked this conversation as resolved.
Show resolved Hide resolved
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) {
Expand Down
3 changes: 3 additions & 0 deletions src/libraries/Error.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
13 changes: 9 additions & 4 deletions src/rollup/RollupCreator.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
89 changes: 88 additions & 1 deletion test/contract/arbRollup.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/

/* eslint-env node, mocha */
import { ethers, network, run } from 'hardhat'

Check warning on line 18 in test/contract/arbRollup.spec.ts

View workflow job for this annotation

GitHub Actions / Contract tests

'run' is defined but never used. Allowed unused vars must match /^_/u
import { Signer } from '@ethersproject/abstract-signer'
import { BigNumberish, BigNumber } from '@ethersproject/bignumber'
import { BytesLike } from '@ethersproject/bytes'
Expand Down Expand Up @@ -86,6 +86,7 @@

// let rollup: RollupContract
let rollup: RollupContract
let batchPosterManager: Signer
let rollupUser: RollupUserLogic
let rollupAdmin: RollupAdminLogic
let bridge: Bridge
Expand All @@ -96,7 +97,7 @@
let sequencer: Signer
let challengeManager: ChallengeManager
let upgradeExecutor: string
let adminproxy: string

Check warning on line 100 in test/contract/arbRollup.spec.ts

View workflow job for this annotation

GitHub Actions / Contract tests

'adminproxy' is assigned a value but never used. Allowed unused vars must match /^_/u

async function getDefaultConfig(
_confirmPeriodBlocks = confirmationPeriodBlocks
Expand Down Expand Up @@ -132,6 +133,7 @@
const val3 = accounts[4]
const val4 = accounts[5]
sequencer = accounts[6]
const batchPosterManager = accounts[7]

const oneStep0Fac = (await ethers.getContractFactory(
'OneStepProver0'
Expand Down Expand Up @@ -285,7 +287,7 @@

const deployParams = {
config: await getDefaultConfig(),
batchPoster: await sequencer.getAddress(),
batchPosters: [await sequencer.getAddress()],
validators: [
await val1.getAddress(),
await val2.getAddress(),
Expand All @@ -296,6 +298,7 @@
nativeToken: ethers.constants.AddressZero,
deployFactoriesToL2: true,
maxFeePerGasForRetryables: maxFeePerGas,
batchPosterManager: await batchPosterManager.getAddress(),
}

const response = await rollupCreator.createRollup(deployParams, {
Expand All @@ -322,6 +325,10 @@
)) as SequencerInbox__factory
).attach(rollupCreatedEvent.sequencerInbox)

await sequencerInbox
.connect(await impersonateAccount(rollupCreatedEvent.upgradeExecutor))
.setBatchPosterManager(await batchPosterManager.getAddress())

challengeManager = (
(await ethers.getContractFactory(
'ChallengeManager'
Expand All @@ -346,6 +353,7 @@
delayedBridge: rollupCreatedEvent.bridge,
delayedInbox: rollupCreatedEvent.inboxAddress,
bridge,
batchPosterManager,
upgradeExecutorAddress: rollupCreatedEvent.upgradeExecutor,
adminproxy: rollupCreatedEvent.adminProxy,
}
Expand Down Expand Up @@ -524,6 +532,7 @@
bridge: bridgeContract,
admin: adminI,
validators: validatorsI,
batchPosterManager: batchPosterManagerI,
upgradeExecutorAddress,
adminproxy: adminproxyAddress,
} = await setup()
Expand All @@ -535,6 +544,7 @@
upgradeExecutor = upgradeExecutorAddress
adminproxy = adminproxyAddress
rollup = new RollupContract(rollupUser.connect(validators[0]))
batchPosterManager = batchPosterManagerI
})

it('should only initialize once', async function () {
Expand Down Expand Up @@ -1125,6 +1135,7 @@
rollupUser: rollupUserContract,
admin: adminI,
validators: validatorsI,
batchPosterManager: batchPosterManagerI,
upgradeExecutorAddress,
} = await setup()
rollupAdmin = rollupAdminContract
Expand All @@ -1133,6 +1144,7 @@
validators = validatorsI
upgradeExecutor = upgradeExecutorAddress
rollup = new RollupContract(rollupUser.connect(validators[0]))
batchPosterManager = batchPosterManagerI
})

it('should stake on initial node again', async function () {
Expand Down Expand Up @@ -1383,6 +1395,81 @@
).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()'
Expand Down
1 change: 1 addition & 0 deletions test/contract/sequencerInboxForceInclude.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -222,9 +222,10 @@
const admin = accounts[0]
const adminAddr = await admin.getAddress()
const user = accounts[1]
const dummyRollup = accounts[2]

Check warning on line 225 in test/contract/sequencerInboxForceInclude.spec.ts

View workflow job for this annotation

GitHub Actions / Contract tests

'dummyRollup' is assigned a value but never used. Allowed unused vars must match /^_/u
const rollupOwner = accounts[3]
const batchPoster = accounts[4]
const batchPosterManager = accounts[5]

Check warning on line 228 in test/contract/sequencerInboxForceInclude.spec.ts

View workflow job for this annotation

GitHub Actions / Contract tests

'batchPosterManager' is assigned a value but never used. Allowed unused vars must match /^_/u

const rollupMockFac = (await ethers.getContractFactory(
'RollupMock'
Expand Down
Loading
Loading