Skip to content

Commit

Permalink
Merge pull request #74 from OffchainLabs/batch-poster-manager
Browse files Browse the repository at this point in the history
Add a batch poster manager for the sequencer inbox
  • Loading branch information
gzeoneth authored Jan 24, 2024
2 parents b4c739f + 14d2cd4 commit c755485
Show file tree
Hide file tree
Showing 8 changed files with 172 additions and 23 deletions.
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);
}
_;
}

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_;
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 @@ -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
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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(),
Expand All @@ -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, {
Expand All @@ -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'
Expand All @@ -346,6 +353,7 @@ const setup = async () => {
delayedBridge: rollupCreatedEvent.bridge,
delayedInbox: rollupCreatedEvent.inboxAddress,
bridge,
batchPosterManager,
upgradeExecutorAddress: rollupCreatedEvent.upgradeExecutor,
adminproxy: rollupCreatedEvent.adminProxy,
}
Expand Down Expand Up @@ -524,6 +532,7 @@ describe('ArbRollup', () => {
bridge: bridgeContract,
admin: adminI,
validators: validatorsI,
batchPosterManager: batchPosterManagerI,
upgradeExecutorAddress,
adminproxy: adminproxyAddress,
} = await setup()
Expand All @@ -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 () {
Expand Down Expand Up @@ -1125,6 +1135,7 @@ describe('ArbRollup', () => {
rollupUser: rollupUserContract,
admin: adminI,
validators: validatorsI,
batchPosterManager: batchPosterManagerI,
upgradeExecutorAddress,
} = await setup()
rollupAdmin = rollupAdminContract
Expand All @@ -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 () {
Expand Down Expand Up @@ -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()'
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 @@ -225,6 +225,7 @@ describe('SequencerInboxForceInclude', async () => {
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

0 comments on commit c755485

Please sign in to comment.