Skip to content

Commit

Permalink
feat: l1-publisher cleanup (#8148)
Browse files Browse the repository at this point in the history
Doing some cleanup after #7850, looking to address the issue of #8153
and #8110.

- #8153
- Addressed by including a "watcher" as part of the setup, which will
push us to the next slot if there is already a block proposed for the
current one.
- #8110
- Updates the logic in the contract such that we can deal with
"simulating" in the future, and use this from the sequencer.

Gets rid of the `time_traveler` from the l1-publisher, now lives in the
watcher which is used in tests.

Issues related to slot duration is still to be addressed, so the name of
this branch got slightly funky.

Taking over the extra check added in #8204 since i) they are related and
ii) the pain of going through CI made me do it.
  • Loading branch information
LHerskind authored and codygunton committed Aug 30, 2024
1 parent 82bd181 commit 2689086
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 45 deletions.
30 changes: 27 additions & 3 deletions l1-contracts/src/core/Rollup.sol
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ contract Rollup is Leonidas, IRollup, ITestRollup {
}

/**
* @notice propose an incoming L2 block with signatures
* @notice Processes an incoming L2 block with signatures
*
* @param _header - The L2 block header
* @param _archive - A root of the archive tree after the L2 block is applied
Expand All @@ -475,7 +475,8 @@ contract Rollup is Leonidas, IRollup, ITestRollup {
blocks[pendingBlockCount++] = BlockLog({
archive: _archive,
blockHash: _blockHash,
slotNumber: header.globalVariables.slotNumber.toUint128()
slotNumber: header.globalVariables.slotNumber.toUint128(),
isProven: false
});

// @note The block number here will always be >=1 as the genesis block is at 0
Expand Down Expand Up @@ -556,6 +557,29 @@ contract Rollup is Leonidas, IRollup, ITestRollup {
);
}

/**
* @notice Validates the header for submission
*
* @param _header - The proposed block header
* @param _signatures - The signatures for the attestations
* @param _digest - The digest that signatures signed
* @param _currentTime - The time of execution
* @dev - This value is provided to allow for simple simulation of future
* @param _flags - Flags specific to the execution, whether certain checks should be skipped
*/
function _validateHeader(
HeaderLib.Header memory _header,
SignatureLib.Signature[] memory _signatures,
bytes32 _digest,
uint256 _currentTime,
DataStructures.ExecutionFlags memory _flags
) internal view {
_validateHeaderForSubmissionBase(_header, _currentTime, _flags);
_validateHeaderForSubmissionSequencerSelection(
_header.globalVariables.slotNumber, _signatures, _digest, _currentTime, _flags
);
}

/**
* @notice Validate a header for submission to the pending chain (sequencer selection checks)
*
Expand Down Expand Up @@ -611,7 +635,7 @@ contract Rollup is Leonidas, IRollup, ITestRollup {
revert Errors.Rollup__InvalidEpoch(currentEpoch, epochNumber);
}

_proposePendingBlock(_slot, _signatures, _digest, _flags);
_processPendingBlock(_slot, _signatures, _digest, _flags);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion l1-contracts/src/core/sequencer_selection/Leonidas.sol
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ contract Leonidas is Ownable, ILeonidas {
* @param _signatures - The signatures of the committee members
* @param _digest - The digest of the block
*/
function _proposePendingBlock(
function _processPendingBlock(
uint256 _slot,
SignatureLib.Signature[] memory _signatures,
bytes32 _digest,
Expand Down
29 changes: 0 additions & 29 deletions l1-contracts/test/Rollup.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -78,27 +78,6 @@ contract RollupTest is DecoderBase {
_;
}

function testRevertProveTwice() public setUpFor("mixed_block_1") {
DecoderBase.Data memory data = load("mixed_block_1").block;
bytes memory header = data.header;
bytes32 archive = data.archive;
bytes memory body = data.body;

// Progress time as necessary
vm.warp(max(block.timestamp, data.decodedHeader.globalVariables.timestamp));
availabilityOracle.publish(body);

// We jump to the time of the block. (unless it is in the past)
vm.warp(max(block.timestamp, data.decodedHeader.globalVariables.timestamp));

rollup.propose(header, archive, bytes32(0));

rollup.submitBlockRootProof(header, archive, bytes32(0), "", "");

vm.expectRevert(abi.encodeWithSelector(Errors.Rollup__NonSequentialProving.selector));
rollup.submitBlockRootProof(header, archive, bytes32(0), "", "");
}

function testTimestamp() public setUpFor("mixed_block_1") {
// Ensure that the timestamp of the current slot is never in the future.
for (uint256 i = 0; i < 100; i++) {
Expand Down Expand Up @@ -219,14 +198,6 @@ contract RollupTest is DecoderBase {
// We jump to the time of the block. (unless it is in the past)
vm.warp(max(block.timestamp, data.decodedHeader.globalVariables.timestamp));

address coinbase = data.decodedHeader.globalVariables.coinbase;
uint256 coinbaseBalance = portalERC20.balanceOf(coinbase);
assertEq(coinbaseBalance, 0, "invalid initial coinbase balance");

// Assert that balance have NOT been increased by proposing the block
rollup.propose(header, archive, bytes32(0));
assertEq(portalERC20.balanceOf(coinbase), 0, "invalid coinbase balance");

vm.expectRevert(
abi.encodeWithSelector(
IERC20Errors.ERC20InsufficientBalance.selector,
Expand Down
18 changes: 11 additions & 7 deletions yarn-project/sequencer-client/src/publisher/l1-publisher.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,10 +166,10 @@ describe('L1Publisher', () => {
`0x${body.toString('hex')}`,
] as const;
if (!L1Publisher.SKIP_SIMULATION) {
expect(rollupContractSimulate.propose).toHaveBeenCalledWith(args, { account: account });
expect(rollupContractSimulate.publishAndProcess).toHaveBeenCalledWith(args, { account: account });
}
expect(rollupContractWrite.propose).toHaveBeenCalledWith(args, { account: account });
expect(publicClient.getTransactionReceipt).toHaveBeenCalledWith({ hash: proposeTxHash });
expect(rollupContractWrite.publishAndProcess).toHaveBeenCalledWith(args, { account: account });
expect(publicClient.getTransactionReceipt).toHaveBeenCalledWith({ hash: publishAndProcessTxHash });
});

it('publishes l2 block to l1 (already published body)', async () => {
Expand All @@ -188,9 +188,9 @@ describe('L1Publisher', () => {
`0x${blockHash.toString('hex')}`,
] as const;
if (!L1Publisher.SKIP_SIMULATION) {
expect(rollupContractSimulate.propose).toHaveBeenCalledWith(args, { account });
expect(rollupContractSimulate.process).toHaveBeenCalledWith(args, { account });
}
expect(rollupContractWrite.propose).toHaveBeenCalledWith(args, { account });
expect(rollupContractWrite.process).toHaveBeenCalledWith(args, { account });
expect(publicClient.getTransactionReceipt).toHaveBeenCalledWith({ hash: processTxHash });
});

Expand Down Expand Up @@ -220,15 +220,19 @@ describe('L1Publisher', () => {
rollupContractRead.validateHeader.mockRejectedValueOnce(new Error('Test error'));
}

if (L1Publisher.SKIP_SIMULATION) {
rollupContractRead.validateHeader.mockRejectedValueOnce(new Error('Test error'));
}

const result = await publisher.processL2Block(l2Block);

expect(result).toEqual(false);
if (!L1Publisher.SKIP_SIMULATION) {
expect(rollupContractSimulate.propose).toHaveBeenCalledTimes(1);
expect(rollupContractSimulate.publishAndProcess).toHaveBeenCalledTimes(1);
}
expect(rollupContractRead.validateHeader).toHaveBeenCalledTimes(1);

expect(rollupContractWrite.propose).toHaveBeenCalledTimes(0);
expect(rollupContractWrite.publishAndProcess).toHaveBeenCalledTimes(0);
});

it('does not retry if sending a publish and propose tx fails', async () => {
Expand Down
10 changes: 5 additions & 5 deletions yarn-project/sequencer-client/src/publisher/l1-publisher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ export class L1Publisher {
] as const;

if (!L1Publisher.SKIP_SIMULATION) {
await this.rollupContract.simulate.propose(args, { account: this.account });
await this.rollupContract.simulate.process(args, { account: this.account });
}

return await this.rollupContract.write.propose(args, {
Expand All @@ -480,9 +480,9 @@ export class L1Publisher {
] as const;

if (!L1Publisher.SKIP_SIMULATION) {
await this.rollupContract.simulate.propose(args, { account: this.account });
await this.rollupContract.simulate.process(args, { account: this.account });
}
return await this.rollupContract.write.propose(args, {
return await this.rollupContract.write.process(args, {
account: this.account,
});
}
Expand All @@ -507,7 +507,7 @@ export class L1Publisher {
] as const;

if (!L1Publisher.SKIP_SIMULATION) {
await this.rollupContract.simulate.propose(args, {
await this.rollupContract.simulate.publishAndProcess(args, {
account: this.account,
});
}
Expand All @@ -524,7 +524,7 @@ export class L1Publisher {
] as const;

if (!L1Publisher.SKIP_SIMULATION) {
await this.rollupContract.simulate.propose(args, {
await this.rollupContract.simulate.publishAndProcess(args, {
account: this.account,
});
}
Expand Down

0 comments on commit 2689086

Please sign in to comment.