From 6e919b83344ff4a71acff35de879e1e7e48acefc Mon Sep 17 00:00:00 2001 From: LHerskind Date: Thu, 29 Aug 2024 08:45:49 +0000 Subject: [PATCH] feat: moving fee payout + make proof submission sequential --- l1-contracts/src/core/Rollup.sol | 94 ++++++------------- l1-contracts/src/core/interfaces/IRollup.sol | 2 - l1-contracts/src/core/libraries/Errors.sol | 1 + l1-contracts/test/Rollup.t.sol | 68 ++++++++++---- .../end-to-end/src/e2e_fees/failures.test.ts | 12 ++- .../end-to-end/src/e2e_fees/fees_test.ts | 5 +- .../src/e2e_fees/private_payments.test.ts | 7 ++ 7 files changed, 102 insertions(+), 87 deletions(-) diff --git a/l1-contracts/src/core/Rollup.sol b/l1-contracts/src/core/Rollup.sol index c15862dfe461..d67f3d8c914f 100644 --- a/l1-contracts/src/core/Rollup.sol +++ b/l1-contracts/src/core/Rollup.sol @@ -39,7 +39,6 @@ contract Rollup is Leonidas, IRollup, ITestRollup { bytes32 archive; bytes32 blockHash; uint128 slotNumber; - bool isProven; } // @note The number of slots within which a block must be proven @@ -97,8 +96,7 @@ contract Rollup is Leonidas, IRollup, ITestRollup { blocks[0] = BlockLog({ archive: bytes32(Constants.GENESIS_ARCHIVE_ROOT), blockHash: bytes32(0), - slotNumber: 0, - isProven: true + slotNumber: 0 }); pendingBlockCount = 1; provenBlockCount = 1; @@ -151,11 +149,7 @@ contract Rollup is Leonidas, IRollup, ITestRollup { onlyOwner { if (blockNumber > provenBlockCount && blockNumber <= pendingBlockCount) { - for (uint256 i = provenBlockCount; i < blockNumber; i++) { - blocks[i].isProven = true; - emit L2ProofVerified(i, "CHEAT"); - } - _progressState(); + provenBlockCount = blockNumber; } assumeProvenUntilBlockNumber = blockNumber; } @@ -274,6 +268,12 @@ contract Rollup is Leonidas, IRollup, ITestRollup { revert Errors.Rollup__TryingToProveNonExistingBlock(); } + // @note This implicitly also ensures that we have not already proven, since + // the value `provenBlockCount` is incremented at the end of this function + if (header.globalVariables.blockNumber != provenBlockCount) { + revert Errors.Rollup__NonSequentialProving(); + } + bytes32 expectedLastArchive = blocks[header.globalVariables.blockNumber - 1].archive; // We do it this way to provide better error messages than passing along the storage values // TODO(#4148) Proper genesis state. If the state is empty, we allow anything for now. @@ -356,24 +356,22 @@ contract Rollup is Leonidas, IRollup, ITestRollup { revert Errors.Rollup__InvalidProof(); } - blocks[header.globalVariables.blockNumber].isProven = true; + provenBlockCount += 1; - _progressState(); + for (uint256 i = 0; i < 32; i++) { + address coinbase = address(uint160(uint256(publicInputs[25 + i * 2]))); + uint256 fees = uint256(publicInputs[26 + i * 2]); + if (coinbase != address(0) && fees > 0) { + // @note This will currently fail if there are insufficient funds in the bridge + // which WILL happen for the old version after an upgrade where the bridge follow. + // Consider allowing a failure. See #7938. + FEE_JUICE_PORTAL.distributeFees(coinbase, fees); + } + } emit L2ProofVerified(header.globalVariables.blockNumber, _proverId); } - /** - * @notice Get the `isProven` flag for the block number - * - * @param _blockNumber - The block number to check - * - * @return bool - True if proven, false otherwise - */ - function isBlockProven(uint256 _blockNumber) external view override(IRollup) returns (bool) { - return blocks[_blockNumber].isProven; - } - /** * @notice Get the archive root of a specific block * @@ -476,8 +474,7 @@ contract Rollup is Leonidas, IRollup, ITestRollup { blocks[pendingBlockCount++] = BlockLog({ archive: _archive, blockHash: _blockHash, - slotNumber: header.globalVariables.slotNumber.toUint128(), - isProven: false + slotNumber: header.globalVariables.slotNumber.toUint128() }); // @note The block number here will always be >=1 as the genesis block is at 0 @@ -494,22 +491,20 @@ contract Rollup is Leonidas, IRollup, ITestRollup { header.globalVariables.blockNumber, header.contentCommitment.outHash, l2ToL1TreeMinHeight ); - // @note This should be addressed at the time of proving if sequential proving or at the time of - // inclusion into the proven chain otherwise. See #7622. - if (header.globalVariables.coinbase != address(0) && header.totalFees > 0) { - // @note This will currently fail if there are insufficient funds in the bridge - // which WILL happen for the old version after an upgrade where the bridge follow. - // Consider allowing a failure. See #7938. - FEE_JUICE_PORTAL.distributeFees(header.globalVariables.coinbase, header.totalFees); - } - emit L2BlockProcessed(header.globalVariables.blockNumber); // Automatically flag the block as proven if we have cheated and set assumeProvenUntilBlockNumber. if (header.globalVariables.blockNumber < assumeProvenUntilBlockNumber) { - blocks[header.globalVariables.blockNumber].isProven = true; + provenBlockCount += 1; + + if (header.globalVariables.coinbase != address(0) && header.totalFees > 0) { + // @note This will currently fail if there are insufficient funds in the bridge + // which WILL happen for the old version after an upgrade where the bridge follow. + // Consider allowing a failure. See #7938. + FEE_JUICE_PORTAL.distributeFees(header.globalVariables.coinbase, header.totalFees); + } + emit L2ProofVerified(header.globalVariables.blockNumber, "CHEAT"); - _progressState(); } } @@ -537,37 +532,6 @@ contract Rollup is Leonidas, IRollup, ITestRollup { return blocks[pendingBlockCount - 1].archive; } - /** - * @notice Progresses the state of the proven chain as far as possible - * - * @dev Emits `ProgressedState` if the state is progressed - * - * @dev Will continue along the pending chain as long as the blocks are proven - * stops at the first unproven block. - * - * @dev Have a potentially unbounded gas usage. @todo Will need a bounded version, such that it cannot be - * used as a DOS vector. - */ - function _progressState() internal { - if (pendingBlockCount == provenBlockCount) { - // We are already up to date - return; - } - - uint256 cachedProvenBlockCount = provenBlockCount; - - for (; cachedProvenBlockCount < pendingBlockCount; cachedProvenBlockCount++) { - if (!blocks[cachedProvenBlockCount].isProven) { - break; - } - } - - if (cachedProvenBlockCount > provenBlockCount) { - provenBlockCount = cachedProvenBlockCount; - emit ProgressedState(provenBlockCount, pendingBlockCount); - } - } - /** * @notice Validates the header for submission * diff --git a/l1-contracts/src/core/interfaces/IRollup.sol b/l1-contracts/src/core/interfaces/IRollup.sol index 4e129e052b58..3d8985935621 100644 --- a/l1-contracts/src/core/interfaces/IRollup.sol +++ b/l1-contracts/src/core/interfaces/IRollup.sol @@ -18,7 +18,6 @@ interface ITestRollup { interface IRollup { event L2BlockProcessed(uint256 indexed blockNumber); event L2ProofVerified(uint256 indexed blockNumber, bytes32 indexed proverId); - event ProgressedState(uint256 provenBlockCount, uint256 pendingBlockCount); event PrunedPending(uint256 provenBlockCount, uint256 pendingBlockCount); function canProposeAtTime(uint256 _ts, address _proposer, bytes32 _archive) @@ -81,6 +80,5 @@ interface IRollup { // ) external; function archive() external view returns (bytes32); - function isBlockProven(uint256 _blockNumber) external view returns (bool); function archiveAt(uint256 _blockNumber) external view returns (bytes32); } diff --git a/l1-contracts/src/core/libraries/Errors.sol b/l1-contracts/src/core/libraries/Errors.sol index b7ff5d9b4e6a..db826807b5ec 100644 --- a/l1-contracts/src/core/libraries/Errors.sol +++ b/l1-contracts/src/core/libraries/Errors.sol @@ -61,6 +61,7 @@ library Errors { error Rollup__UnavailableTxs(bytes32 txsHash); // 0x414906c3 error Rollup__NothingToPrune(); // 0x850defd3 error Rollup__NotReadyToPrune(uint256 currentSlot, uint256 prunableAt); // 0x9fdf1614 + error Rollup__NonSequentialProving(); // 0x1e5be132 // Registry error Registry__RollupNotRegistered(address rollup); // 0xa1fee4cf diff --git a/l1-contracts/test/Rollup.t.sol b/l1-contracts/test/Rollup.t.sol index 07c67385a809..0ad6665a404e 100644 --- a/l1-contracts/test/Rollup.t.sol +++ b/l1-contracts/test/Rollup.t.sol @@ -78,6 +78,27 @@ 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.process(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++) { @@ -105,7 +126,7 @@ contract RollupTest is DecoderBase { _testBlock("mixed_block_1", false); uint256 currentSlot = rollup.getCurrentSlot(); - (,, uint128 slot,) = rollup.blocks(1); + (,, uint128 slot) = rollup.blocks(1); uint256 prunableAt = uint256(slot) + rollup.TIMELINESS_PROVING_IN_SLOTS(); vm.expectRevert( @@ -127,7 +148,7 @@ contract RollupTest is DecoderBase { // Even if we end up reverting block 1, we should still see the same root in the inbox. bytes32 inboxRoot2 = inbox.getRoot(2); - (,, uint128 slot,) = rollup.blocks(1); + (,, uint128 slot) = rollup.blocks(1); uint256 prunableAt = uint256(slot) + rollup.TIMELINESS_PROVING_IN_SLOTS(); uint256 timeOfPrune = rollup.getTimestampForSlot(prunableAt); @@ -198,6 +219,14 @@ 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.process(header, archive, bytes32(0)); + assertEq(portalERC20.balanceOf(coinbase), 0, "invalid coinbase balance"); + vm.expectRevert( abi.encodeWithSelector( IERC20Errors.ERC20InsufficientBalance.selector, @@ -206,15 +235,13 @@ contract RollupTest is DecoderBase { feeAmount ) ); - rollup.process(header, archive, bytes32(0)); - - address coinbase = data.decodedHeader.globalVariables.coinbase; - uint256 coinbaseBalance = portalERC20.balanceOf(coinbase); - assertEq(coinbaseBalance, 0, "invalid initial coinbase balance"); + rollup.submitBlockRootProof(header, archive, bytes32(0), "", ""); + assertEq(portalERC20.balanceOf(coinbase), 0, "invalid coinbase balance"); portalERC20.mint(address(feeJuicePortal), feeAmount - portalBalance); - rollup.process(header, archive, bytes32(0)); + // When the block is proven we should have received the funds + rollup.submitBlockRootProof(header, archive, bytes32(0), "", ""); assertEq(portalERC20.balanceOf(coinbase), feeAmount, "invalid coinbase balance"); } @@ -237,9 +264,18 @@ contract RollupTest is DecoderBase { function testConsecutiveMixedBlocksNonSequentialProof() public setUpFor("mixed_block_1") { _testBlock("mixed_block_1", false); - _testBlock("mixed_block_2", true); - assertTrue(rollup.isBlockProven(2), "Block 2 is not proven"); + DecoderBase.Data memory data = load("mixed_block_2").block; + bytes memory header = data.header; + bytes32 archive = data.archive; + bytes memory body = data.body; + + vm.warp(max(block.timestamp, data.decodedHeader.globalVariables.timestamp)); + availabilityOracle.publish(body); + rollup.process(header, archive, bytes32(0)); + + vm.expectRevert(abi.encodeWithSelector(Errors.Rollup__NonSequentialProving.selector)); + rollup.submitBlockRootProof(header, archive, bytes32(0), "", ""); assertEq(rollup.pendingBlockCount(), 3, "Invalid pending block count"); assertEq(rollup.provenBlockCount(), 1, "Invalid proven block count"); @@ -366,9 +402,8 @@ contract RollupTest is DecoderBase { function testSubmitProofInvalidArchive() public setUpFor("empty_block_1") { _testBlock("empty_block_1", false); - _testBlock("empty_block_2", false); - DecoderBase.Data memory data = load("empty_block_2").block; + DecoderBase.Data memory data = load("empty_block_1").block; bytes memory header = data.header; bytes32 archive = data.archive; @@ -379,7 +414,7 @@ contract RollupTest is DecoderBase { vm.expectRevert( abi.encodeWithSelector( - Errors.Rollup__InvalidArchive.selector, rollup.archiveAt(1), 0xdeadbeef + Errors.Rollup__InvalidArchive.selector, rollup.archiveAt(0), 0xdeadbeef ) ); rollup.submitBlockRootProof(header, archive, bytes32(0), "", ""); @@ -433,12 +468,11 @@ contract RollupTest is DecoderBase { rollup.process(header, archive, bytes32(0)); if (_submitProof) { + uint256 pre = rollup.provenBlockCount(); + rollup.submitBlockRootProof(header, archive, bytes32(0), "", ""); - assertTrue( - rollup.isBlockProven(full.block.decodedHeader.globalVariables.blockNumber), - "Block not proven" - ); + assertEq(pre + 1, rollup.provenBlockCount(), "Block not proven"); } bytes32 l2ToL1MessageTreeRoot; diff --git a/yarn-project/end-to-end/src/e2e_fees/failures.test.ts b/yarn-project/end-to-end/src/e2e_fees/failures.test.ts index ca5efa6ac078..253cada5a316 100644 --- a/yarn-project/end-to-end/src/e2e_fees/failures.test.ts +++ b/yarn-project/end-to-end/src/e2e_fees/failures.test.ts @@ -8,6 +8,7 @@ import { PublicFeePaymentMethod, TxStatus, computeSecretHash, + sleep, } from '@aztec/aztec.js'; import { Gas, GasSettings } from '@aztec/circuits.js'; import { FunctionType } from '@aztec/foundation/abi'; @@ -94,9 +95,16 @@ describe('e2e_fees failures', () => { .wait({ dontThrowOnRevert: true }); expect(txReceipt.status).toBe(TxStatus.APP_LOGIC_REVERTED); + + // We wait until the block is proven since that is when the payout happens. + const bn = await t.aztecNode.getBlockNumber(); + while ((await t.aztecNode.getProvenBlockNumber()) < bn) { + await sleep(1000); + } + const feeAmount = txReceipt.transactionFee!; - const newSequencerL1Gas = await t.getCoinbaseBalance(); - expect(newSequencerL1Gas).toEqual(currentSequencerL1Gas + feeAmount); + const newSequencerL1FeeAssetBalance = await t.getCoinbaseBalance(); + expect(newSequencerL1FeeAssetBalance).toEqual(currentSequencerL1Gas + feeAmount); // and thus we paid the fee await expectMapping( diff --git a/yarn-project/end-to-end/src/e2e_fees/fees_test.ts b/yarn-project/end-to-end/src/e2e_fees/fees_test.ts index 0c02c841bb21..ff0c18a29781 100644 --- a/yarn-project/end-to-end/src/e2e_fees/fees_test.ts +++ b/yarn-project/end-to-end/src/e2e_fees/fees_test.ts @@ -27,6 +27,7 @@ import { TokenContract, } from '@aztec/noir-contracts.js'; import { getCanonicalFeeJuice } from '@aztec/protocol-contracts/fee-juice'; +import { type ProverNode } from '@aztec/prover-node'; import { getContract } from 'viem'; @@ -53,6 +54,7 @@ export class FeesTest { public logger: DebugLogger; public pxe!: PXE; public aztecNode!: AztecNode; + public proverNode!: ProverNode; public aliceWallet!: AccountWallet; public aliceAddress!: AztecAddress; @@ -167,9 +169,10 @@ export class FeesTest { await this.snapshotManager.snapshot( 'initial_accounts', addAccounts(3, this.logger), - async ({ accountKeys }, { pxe, aztecNode, aztecNodeConfig }) => { + async ({ accountKeys }, { pxe, aztecNode, aztecNodeConfig, proverNode }) => { this.pxe = pxe; this.aztecNode = aztecNode; + this.proverNode = proverNode; const accountManagers = accountKeys.map(ak => getSchnorrAccount(pxe, ak[0], ak[1], 1)); await Promise.all(accountManagers.map(a => a.register())); this.wallets = await Promise.all(accountManagers.map(a => a.getWallet())); diff --git a/yarn-project/end-to-end/src/e2e_fees/private_payments.test.ts b/yarn-project/end-to-end/src/e2e_fees/private_payments.test.ts index 3987c8137d77..52c3abfe6406 100644 --- a/yarn-project/end-to-end/src/e2e_fees/private_payments.test.ts +++ b/yarn-project/end-to-end/src/e2e_fees/private_payments.test.ts @@ -6,6 +6,7 @@ import { PrivateFeePaymentMethod, type TxReceipt, computeSecretHash, + sleep, } from '@aztec/aztec.js'; import { type GasSettings } from '@aztec/circuits.js'; import { type TokenContract as BananaCoin, FPCContract } from '@aztec/noir-contracts.js'; @@ -138,6 +139,12 @@ describe('e2e_fees private_payment', () => { * TODO(6583): update this comment properly now that public execution consumes gas */ + // We wait until the block is proven since that is when the payout happens. + const bn = await t.aztecNode.getBlockNumber(); + while ((await t.aztecNode.getProvenBlockNumber()) < bn) { + await sleep(1000); + } + // expect(tx.transactionFee).toEqual(200032492n); await expect(t.getCoinbaseBalance()).resolves.toEqual(InitialSequencerL1Gas + tx.transactionFee!); const [feeAmount, refundAmount] = getFeeAndRefund(tx);