From bce1eea90deb8fabf2324554681628144f4ee08d Mon Sep 17 00:00:00 2001 From: Maddiaa <47148561+Maddiaa0@users.noreply.github.com> Date: Wed, 11 Sep 2024 15:57:49 +0100 Subject: [PATCH] feat: validators ensure transactions live in their p2p pool before attesting (#8410) --- l1-contracts/src/core/Rollup.sol | 6 +- l1-contracts/src/core/interfaces/IRollup.sol | 1 + .../src/core/sequencer_selection/Leonidas.sol | 3 - l1-contracts/test/Rollup.t.sol | 25 +++-- l1-contracts/test/sparta/Sparta.t.sol | 9 +- .../archiver/src/archiver/archiver.test.ts | 2 +- .../archiver/src/archiver/eth_log_handlers.ts | 2 +- .../src/p2p/block_attestation.ts | 21 +++- .../circuit-types/src/p2p/block_proposal.ts | 20 +++- .../circuit-types/src/p2p/block_utils.ts | 34 ++++++ yarn-project/circuit-types/src/p2p/index.ts | 1 + yarn-project/circuit-types/src/p2p/mocks.ts | 10 +- .../composed/integration_l1_publisher.test.ts | 2 + .../end-to-end/src/e2e_p2p_network.test.ts | 85 +++++++++++++-- .../foundation/src/buffer/buffer32.ts | 9 ++ yarn-project/foundation/src/config/env_var.ts | 2 + .../p2p/src/attestation_pool/mocks.ts | 9 +- yarn-project/p2p/src/client/p2p_client.ts | 49 ++++++++- .../p2p/src/service/libp2p_service.ts | 2 +- .../p2p/src/service/reqresp/interface.ts | 6 +- .../p2p/src/service/reqresp/reqresp.ts | 5 + yarn-project/p2p/src/service/service.ts | 2 +- .../src/publisher/l1-publisher.test.ts | 1 + .../src/publisher/l1-publisher.ts | 19 +++- .../sequencer-client/src/publisher/utils.ts | 15 +++ .../src/sequencer/sequencer.test.ts | 46 +++++--- .../src/sequencer/sequencer.ts | 37 ++++--- yarn-project/validator-client/package.json | 1 + yarn-project/validator-client/src/config.ts | 24 ++++- .../src/duties/validation_service.ts | 16 +-- .../validator-client/src/errors/index.ts | 1 + .../src/errors/validator.error.ts | 19 ++++ .../validator-client/src/validator.test.ts | 70 ++++++++++++ .../validator-client/src/validator.ts | 100 +++++++++++++----- yarn-project/yarn.lock | 3 +- 35 files changed, 537 insertions(+), 120 deletions(-) create mode 100644 yarn-project/circuit-types/src/p2p/block_utils.ts create mode 100644 yarn-project/sequencer-client/src/publisher/utils.ts create mode 100644 yarn-project/validator-client/src/errors/index.ts create mode 100644 yarn-project/validator-client/src/errors/validator.error.ts create mode 100644 yarn-project/validator-client/src/validator.test.ts diff --git a/l1-contracts/src/core/Rollup.sol b/l1-contracts/src/core/Rollup.sol index 0a21a2c5d29..03f8181ddf2 100644 --- a/l1-contracts/src/core/Rollup.sol +++ b/l1-contracts/src/core/Rollup.sol @@ -191,6 +191,7 @@ contract Rollup is Leonidas, IRollup, ITestRollup { bytes calldata _header, bytes32 _archive, bytes32 _blockHash, + bytes32[] memory _txHashes, SignatureLib.Signature[] memory _signatures, bytes calldata _body ) external override(IRollup) { @@ -198,11 +199,13 @@ contract Rollup is Leonidas, IRollup, ITestRollup { // Decode and validate header HeaderLib.Header memory header = HeaderLib.decode(_header); + + bytes32 digest = keccak256(abi.encode(_archive, _txHashes)); setupEpoch(); _validateHeader({ _header: header, _signatures: _signatures, - _digest: _archive, + _digest: digest, _currentTime: block.timestamp, _txEffectsHash: txsEffectsHash, _flags: DataStructures.ExecutionFlags({ignoreDA: false, ignoreSignatures: false}) @@ -419,6 +422,7 @@ contract Rollup is Leonidas, IRollup, ITestRollup { revert Errors.Rollup__SlotAlreadyInChain(lastSlot, slot); } + // Make sure that the proposer is up to date bytes32 tipArchive = archive(); if (tipArchive != _archive) { revert Errors.Rollup__InvalidArchive(tipArchive, _archive); diff --git a/l1-contracts/src/core/interfaces/IRollup.sol b/l1-contracts/src/core/interfaces/IRollup.sol index 8fff6e6a5bd..ce99bd64921 100644 --- a/l1-contracts/src/core/interfaces/IRollup.sol +++ b/l1-contracts/src/core/interfaces/IRollup.sol @@ -39,6 +39,7 @@ interface IRollup { bytes calldata _header, bytes32 _archive, bytes32 _blockHash, + bytes32[] memory _txHashes, SignatureLib.Signature[] memory _signatures, bytes calldata _body ) external; diff --git a/l1-contracts/src/core/sequencer_selection/Leonidas.sol b/l1-contracts/src/core/sequencer_selection/Leonidas.sol index 1d1d26b61bd..ad9550c794f 100644 --- a/l1-contracts/src/core/sequencer_selection/Leonidas.sol +++ b/l1-contracts/src/core/sequencer_selection/Leonidas.sol @@ -152,9 +152,6 @@ contract Leonidas is Ownable, ILeonidas { /** * @notice Get the validator set for the current epoch - * - * @dev Makes a call to setupEpoch under the hood, this should ONLY be called as a view function, and not from within - * this contract. * @return The validator set for the current epoch */ function getCurrentEpochCommittee() external view override(ILeonidas) returns (address[] memory) { diff --git a/l1-contracts/test/Rollup.t.sol b/l1-contracts/test/Rollup.t.sol index a772efccc80..e9b1f899eaa 100644 --- a/l1-contracts/test/Rollup.t.sol +++ b/l1-contracts/test/Rollup.t.sol @@ -81,11 +81,12 @@ contract RollupTest is DecoderBase { bytes memory header = data.header; bytes32 archive = data.archive; bytes memory body = data.body; + bytes32[] memory txHashes = new bytes32[](0); // 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), signatures, body); + rollup.propose(header, archive, bytes32(0), txHashes, signatures, body); rollup.submitBlockRootProof(header, archive, bytes32(0), "", ""); @@ -186,6 +187,7 @@ contract RollupTest is DecoderBase { bytes memory header = data.header; bytes32 archive = data.archive; bytes memory body = data.body; + bytes32[] memory txHashes = new bytes32[](0); // Progress time as necessary vm.warp(max(block.timestamp, data.decodedHeader.globalVariables.timestamp)); @@ -206,7 +208,7 @@ contract RollupTest is DecoderBase { assertEq(coinbaseBalance, 0, "invalid initial coinbase balance"); // Assert that balance have NOT been increased by proposing the block - rollup.propose(header, archive, bytes32(0), signatures, body); + rollup.propose(header, archive, bytes32(0), txHashes, signatures, body); assertEq(portalERC20.balanceOf(coinbase), 0, "invalid coinbase balance"); vm.expectRevert( @@ -251,9 +253,10 @@ contract RollupTest is DecoderBase { bytes memory header = data.header; bytes32 archive = data.archive; bytes memory body = data.body; + bytes32[] memory txHashes = new bytes32[](0); vm.warp(max(block.timestamp, data.decodedHeader.globalVariables.timestamp)); - rollup.propose(header, archive, bytes32(0), signatures, body); + rollup.propose(header, archive, bytes32(0), txHashes, signatures, body); vm.expectRevert(abi.encodeWithSelector(Errors.Rollup__NonSequentialProving.selector)); rollup.submitBlockRootProof(header, archive, bytes32(0), "", ""); @@ -282,6 +285,7 @@ contract RollupTest is DecoderBase { bytes memory header = data.header; bytes32 archive = data.archive; bytes memory body = data.body; + bytes32[] memory txHashes = new bytes32[](0); assembly { // TODO: Hardcoding offsets in the middle of tests is annoying to say the least. @@ -289,7 +293,7 @@ contract RollupTest is DecoderBase { } vm.expectRevert(abi.encodeWithSelector(Errors.Rollup__InvalidBlockNumber.selector, 1, 0x420)); - rollup.propose(header, archive, bytes32(0), signatures, body); + rollup.propose(header, archive, bytes32(0), txHashes, signatures, body); } function testRevertInvalidChainId() public setUpFor("empty_block_1") { @@ -297,14 +301,14 @@ contract RollupTest is DecoderBase { bytes memory header = data.header; bytes32 archive = data.archive; bytes memory body = data.body; + bytes32[] memory txHashes = new bytes32[](0); assembly { - // TODO: Hardcoding offsets in the middle of tests is annoying to say the least. mstore(add(header, add(0x20, 0x0134)), 0x420) } vm.expectRevert(abi.encodeWithSelector(Errors.Rollup__InvalidChainId.selector, 31337, 0x420)); - rollup.propose(header, archive, bytes32(0), signatures, body); + rollup.propose(header, archive, bytes32(0), txHashes, signatures, body); } function testRevertInvalidVersion() public setUpFor("empty_block_1") { @@ -312,13 +316,14 @@ contract RollupTest is DecoderBase { bytes memory header = data.header; bytes32 archive = data.archive; bytes memory body = data.body; + bytes32[] memory txHashes = new bytes32[](0); assembly { mstore(add(header, add(0x20, 0x0154)), 0x420) } vm.expectRevert(abi.encodeWithSelector(Errors.Rollup__InvalidVersion.selector, 1, 0x420)); - rollup.propose(header, archive, bytes32(0), signatures, body); + rollup.propose(header, archive, bytes32(0), txHashes, signatures, body); } function testRevertInvalidTimestamp() public setUpFor("empty_block_1") { @@ -326,6 +331,7 @@ contract RollupTest is DecoderBase { bytes memory header = data.header; bytes32 archive = data.archive; bytes memory body = data.body; + bytes32[] memory txHashes = new bytes32[](0); uint256 realTs = data.decodedHeader.globalVariables.timestamp; uint256 badTs = realTs + 1; @@ -337,7 +343,7 @@ contract RollupTest is DecoderBase { } vm.expectRevert(abi.encodeWithSelector(Errors.Rollup__InvalidTimestamp.selector, realTs, badTs)); - rollup.propose(header, archive, bytes32(0), signatures, body); + rollup.propose(header, archive, bytes32(0), txHashes, signatures, body); } function testBlocksWithAssumeProven() public setUpFor("mixed_block_1") { @@ -418,6 +424,7 @@ contract RollupTest is DecoderBase { bytes32 archive = full.block.archive; bytes memory body = full.block.body; uint32 numTxs = full.block.numTxs; + bytes32[] memory txHashes = new bytes32[](0); // Overwrite some timestamps if needed if (_slotNumber != 0) { @@ -436,7 +443,7 @@ contract RollupTest is DecoderBase { _populateInbox(full.populate.sender, full.populate.recipient, full.populate.l1ToL2Content); - rollup.propose(header, archive, bytes32(0), signatures, body); + rollup.propose(header, archive, bytes32(0), txHashes, signatures, body); if (_submitProof) { uint256 pre = rollup.provenBlockCount(); diff --git a/l1-contracts/test/sparta/Sparta.t.sol b/l1-contracts/test/sparta/Sparta.t.sol index e165d8b7bec..dc559ede666 100644 --- a/l1-contracts/test/sparta/Sparta.t.sol +++ b/l1-contracts/test/sparta/Sparta.t.sol @@ -171,14 +171,17 @@ contract SpartaTest is DecoderBase { rollup.setupEpoch(); + bytes32[] memory txHashes = new bytes32[](0); + if (_signatureCount > 0 && ree.proposer != address(0)) { address[] memory validators = rollup.getEpochCommittee(rollup.getCurrentEpoch()); ree.needed = validators.length * 2 / 3 + 1; SignatureLib.Signature[] memory signatures = new SignatureLib.Signature[](_signatureCount); + bytes32 digest = keccak256(abi.encode(archive, txHashes)); for (uint256 i = 0; i < _signatureCount; i++) { - signatures[i] = createSignature(validators[i], archive); + signatures[i] = createSignature(validators[i], digest); } if (_expectRevert) { @@ -208,7 +211,7 @@ contract SpartaTest is DecoderBase { } vm.prank(ree.proposer); - rollup.propose(header, archive, bytes32(0), signatures, body); + rollup.propose(header, archive, bytes32(0), txHashes, signatures, body); if (ree.shouldRevert) { return; @@ -216,7 +219,7 @@ contract SpartaTest is DecoderBase { } else { SignatureLib.Signature[] memory signatures = new SignatureLib.Signature[](0); - rollup.propose(header, archive, bytes32(0), signatures, body); + rollup.propose(header, archive, bytes32(0), txHashes, signatures, body); } assertEq(_expectRevert, ree.shouldRevert, "Does not match revert expectation"); diff --git a/yarn-project/archiver/src/archiver/archiver.test.ts b/yarn-project/archiver/src/archiver/archiver.test.ts index 95847d27b6a..9b9232b0ddb 100644 --- a/yarn-project/archiver/src/archiver/archiver.test.ts +++ b/yarn-project/archiver/src/archiver/archiver.test.ts @@ -277,7 +277,7 @@ function makeRollupTx(l2Block: L2Block) { const input = encodeFunctionData({ abi: RollupAbi, functionName: 'propose', - args: [header, archive, blockHash, [], body], + args: [header, archive, blockHash, [], [], body], }); return { input } as Transaction; } diff --git a/yarn-project/archiver/src/archiver/eth_log_handlers.ts b/yarn-project/archiver/src/archiver/eth_log_handlers.ts index f0fdf6a1075..fa218fb2086 100644 --- a/yarn-project/archiver/src/archiver/eth_log_handlers.ts +++ b/yarn-project/archiver/src/archiver/eth_log_handlers.ts @@ -87,7 +87,7 @@ async function getBlockFromRollupTx( if (!(functionName === 'propose')) { throw new Error(`Unexpected method called ${functionName}`); } - const [headerHex, archiveRootHex, , , bodyHex] = args! as readonly [Hex, Hex, Hex, ViemSignature[], Hex]; + const [headerHex, archiveRootHex, , , , bodyHex] = args! as readonly [Hex, Hex, Hex, Hex[], ViemSignature[], Hex]; const header = Header.fromBuffer(Buffer.from(hexToBytes(headerHex))); const blockBody = Body.fromBuffer(Buffer.from(hexToBytes(bodyHex))); diff --git a/yarn-project/circuit-types/src/p2p/block_attestation.ts b/yarn-project/circuit-types/src/p2p/block_attestation.ts index fd38d691172..e38effdb368 100644 --- a/yarn-project/circuit-types/src/p2p/block_attestation.ts +++ b/yarn-project/circuit-types/src/p2p/block_attestation.ts @@ -5,6 +5,8 @@ import { BufferReader, serializeToBuffer } from '@aztec/foundation/serialize'; import { recoverMessageAddress } from 'viem'; +import { TxHash } from '../tx/tx_hash.js'; +import { get0xStringHashedSignaturePayload, getSignaturePayload } from './block_utils.js'; import { Gossipable } from './gossipable.js'; import { Signature } from './signature.js'; import { TopicType, createTopicString } from './topic_type.js'; @@ -31,6 +33,7 @@ export class BlockAttestation extends Gossipable { public readonly header: Header, // TODO(https://github.com/AztecProtocol/aztec-packages/pull/7727#discussion_r1713670830): temporary public readonly archive: Fr, + public readonly txHashes: TxHash[], /** The signature of the block attester */ public readonly signature: Signature, ) { @@ -53,8 +56,9 @@ export class BlockAttestation extends Gossipable { async getSender() { if (!this.sender) { // Recover the sender from the attestation + const hashed = get0xStringHashedSignaturePayload(this.archive, this.txHashes); const address = await recoverMessageAddress({ - message: { raw: this.p2pMessageIdentifier().to0xString() }, + message: { raw: hashed }, signature: this.signature.to0xString(), }); // Cache the sender for later use @@ -64,16 +68,25 @@ export class BlockAttestation extends Gossipable { return this.sender; } + getPayload(): Buffer { + return getSignaturePayload(this.archive, this.txHashes); + } + toBuffer(): Buffer { - return serializeToBuffer([this.header, this.archive, this.signature]); + return serializeToBuffer([this.header, this.archive, this.txHashes.length, this.txHashes, this.signature]); } static fromBuffer(buf: Buffer | BufferReader): BlockAttestation { const reader = BufferReader.asReader(buf); - return new BlockAttestation(reader.readObject(Header), reader.readObject(Fr), reader.readObject(Signature)); + return new BlockAttestation( + reader.readObject(Header), + reader.readObject(Fr), + reader.readArray(reader.readNumber(), TxHash), + reader.readObject(Signature), + ); } static empty(): BlockAttestation { - return new BlockAttestation(Header.empty(), Fr.ZERO, Signature.empty()); + return new BlockAttestation(Header.empty(), Fr.ZERO, [], Signature.empty()); } } diff --git a/yarn-project/circuit-types/src/p2p/block_proposal.ts b/yarn-project/circuit-types/src/p2p/block_proposal.ts index 5c77de9b5e9..8164e755117 100644 --- a/yarn-project/circuit-types/src/p2p/block_proposal.ts +++ b/yarn-project/circuit-types/src/p2p/block_proposal.ts @@ -6,6 +6,7 @@ import { BufferReader, serializeToBuffer } from '@aztec/foundation/serialize'; import { recoverMessageAddress } from 'viem'; import { TxHash } from '../tx/tx_hash.js'; +import { get0xStringHashedSignaturePayload, getHashedSignaturePayload, getSignaturePayload } from './block_utils.js'; import { Gossipable } from './gossipable.js'; import { Signature } from './signature.js'; import { TopicType, createTopicString } from './topic_type.js'; @@ -49,14 +50,27 @@ export class BlockProposal extends Gossipable { return BlockProposalHash.fromField(this.archive); } + static async createProposalFromSigner( + header: Header, + archive: Fr, + txs: TxHash[], + payloadSigner: (payload: Buffer) => Promise, + ) { + const hashed = getHashedSignaturePayload(archive, txs); + const sig = await payloadSigner(hashed); + + return new BlockProposal(header, archive, txs, sig); + } + /**Get Sender * Lazily evaluate the sender of the proposal; result is cached */ async getSender() { if (!this.sender) { // performance note(): this signature method requires another hash behind the scenes + const hashed = get0xStringHashedSignaturePayload(this.archive, this.txs); const address = await recoverMessageAddress({ - message: { raw: this.p2pMessageIdentifier().to0xString() }, + message: { raw: hashed }, signature: this.signature.to0xString(), }); // Cache the sender for later use @@ -66,6 +80,10 @@ export class BlockProposal extends Gossipable { return this.sender; } + getPayload() { + return getSignaturePayload(this.archive, this.txs); + } + toBuffer(): Buffer { return serializeToBuffer([this.header, this.archive, this.txs.length, this.txs, this.signature]); } diff --git a/yarn-project/circuit-types/src/p2p/block_utils.ts b/yarn-project/circuit-types/src/p2p/block_utils.ts new file mode 100644 index 00000000000..cd1c5a48842 --- /dev/null +++ b/yarn-project/circuit-types/src/p2p/block_utils.ts @@ -0,0 +1,34 @@ +import { keccak256 as keccak256Buffer } from '@aztec/foundation/crypto'; +import { type Fr } from '@aztec/foundation/fields'; + +import { encodeAbiParameters, keccak256 as keccak2560xString, parseAbiParameters } from 'viem'; + +import { type TxHash } from '../tx/tx_hash.js'; + +/** + * Get the payload for the signature of the block proposal + * @param archive - The archive of the block + * @param txs - The transactions in the block + * @returns The payload for the signature of the block proposal + */ +export function getSignaturePayload(archive: Fr, txs: TxHash[]) { + const abi = parseAbiParameters('bytes32, bytes32[]'); + const txArray = txs.map(tx => tx.to0xString()); + const encodedData = encodeAbiParameters(abi, [archive.toString(), txArray] as const); + + return Buffer.from(encodedData.slice(2), 'hex'); +} + +/** + * Get the hashed payload for the signature of the block proposal + * @param archive - The archive of the block + * @param txs - The transactions in the block + * @returns The hashed payload for the signature of the block proposal + */ +export function getHashedSignaturePayload(archive: Fr, txs: TxHash[]): Buffer { + return keccak256Buffer(getSignaturePayload(archive, txs)); +} + +export function get0xStringHashedSignaturePayload(archive: Fr, txs: TxHash[]): `0x${string}` { + return keccak2560xString(getSignaturePayload(archive, txs)); +} diff --git a/yarn-project/circuit-types/src/p2p/index.ts b/yarn-project/circuit-types/src/p2p/index.ts index a8a7f011fd0..e6c268523c1 100644 --- a/yarn-project/circuit-types/src/p2p/index.ts +++ b/yarn-project/circuit-types/src/p2p/index.ts @@ -4,3 +4,4 @@ export * from './interface.js'; export * from './gossipable.js'; export * from './topic_type.js'; export * from './signature.js'; +export * from './block_utils.js'; diff --git a/yarn-project/circuit-types/src/p2p/mocks.ts b/yarn-project/circuit-types/src/p2p/mocks.ts index f85ba76a6ae..2e4b9495d66 100644 --- a/yarn-project/circuit-types/src/p2p/mocks.ts +++ b/yarn-project/circuit-types/src/p2p/mocks.ts @@ -7,6 +7,7 @@ import { generatePrivateKey, privateKeyToAccount } from 'viem/accounts'; import { TxHash } from '../tx/tx_hash.js'; import { BlockAttestation } from './block_attestation.js'; import { BlockProposal } from './block_proposal.js'; +import { get0xStringHashedSignaturePayload } from './block_utils.js'; import { Signature } from './signature.js'; export const makeBlockProposal = async (signer?: PrivateKeyAccount): Promise => { @@ -15,7 +16,8 @@ export const makeBlockProposal = async (signer?: PrivateKeyAccount): Promise TxHash.random()); - const signature = Signature.from0xString(await signer.signMessage({ message: { raw: archive.toString() } })); + const hash = get0xStringHashedSignaturePayload(archive, txs); + const signature = Signature.from0xString(await signer.signMessage({ message: { raw: hash } })); return new BlockProposal(blockHeader, archive, txs, signature); }; @@ -26,9 +28,11 @@ export const makeBlockAttestation = async (signer?: PrivateKeyAccount): Promise< const blockHeader = makeHeader(1); const archive = Fr.random(); - const signature = Signature.from0xString(await signer.signMessage({ message: { raw: archive.toString() } })); + const txs = [0, 1, 2, 3, 4, 5].map(() => TxHash.random()); + const hash = get0xStringHashedSignaturePayload(archive, txs); + const signature = Signature.from0xString(await signer.signMessage({ message: { raw: hash } })); - return new BlockAttestation(blockHeader, archive, signature); + return new BlockAttestation(blockHeader, archive, txs, signature); }; export const randomSigner = (): PrivateKeyAccount => { diff --git a/yarn-project/end-to-end/src/composed/integration_l1_publisher.test.ts b/yarn-project/end-to-end/src/composed/integration_l1_publisher.test.ts index b978ecda0a5..21c8d5a7d5e 100644 --- a/yarn-project/end-to-end/src/composed/integration_l1_publisher.test.ts +++ b/yarn-project/end-to-end/src/composed/integration_l1_publisher.test.ts @@ -408,6 +408,7 @@ describe('L1Publisher integration', () => { `0x${block.archive.root.toBuffer().toString('hex')}`, `0x${block.header.hash().toBuffer().toString('hex')}`, [], + [], `0x${block.body.toBuffer().toString('hex')}`, ], }); @@ -509,6 +510,7 @@ describe('L1Publisher integration', () => { `0x${block.archive.root.toBuffer().toString('hex')}`, `0x${block.header.hash().toBuffer().toString('hex')}`, [], + [], `0x${block.body.toBuffer().toString('hex')}`, ], }); diff --git a/yarn-project/end-to-end/src/e2e_p2p_network.test.ts b/yarn-project/end-to-end/src/e2e_p2p_network.test.ts index 8e720410550..6caf8f6bec4 100644 --- a/yarn-project/end-to-end/src/e2e_p2p_network.test.ts +++ b/yarn-project/end-to-end/src/e2e_p2p_network.test.ts @@ -109,6 +109,13 @@ describe('e2e_p2p_network', () => { }); }); + const stopNodes = async (bootstrap: BootstrapNode, nodes: AztecNodeService[]) => { + for (const node of nodes) { + await node.stop(); + } + await bootstrap.stop(); + }; + afterEach(() => teardown()); afterAll(() => { @@ -136,7 +143,7 @@ describe('e2e_p2p_network', () => { ); // wait a bit for peers to discover each other - await sleep(2000); + await sleep(4000); for (const node of nodes) { const context = await createPXEServiceAndSubmitTransactions(node, NUM_TXS_PER_NODE); @@ -154,11 +161,73 @@ describe('e2e_p2p_network', () => { ); // shutdown all nodes. - for (const context of contexts) { - await context.node.stop(); - await context.pxeService.stop(); + await stopNodes(bootstrapNode, nodes); + }); + + // NOTE: If this test fails in a PR where the shuffling algorithm is changed, then it is failing as the node with + // the mocked p2p layer is being picked as the sequencer, and it does not have any transactions in it's mempool. + // If this is the case, then we should update the test to switch off the mempool of a different node. + // adjust `nodeToTurnOffTxGossip` in the test below. + it('should produce an attestation by requesting tx data over the p2p network', async () => { + /** + * Birds eye overview of the test + * 1. We spin up x nodes + * 2. We turn off receiving a tx via gossip from two of the nodes + * 3. We send a transactions and gossip it to other nodes + * 4. The disabled nodes will receive an attestation that it does not have the data for + * 5. They will request this data over the p2p layer + * 6. We receive all of the attestations that we need and we produce the block + * + * Note: we do not attempt to let this node produce a block, as it will not have received any transactions + * from the other pxes. + */ + + if (!bootstrapNodeEnr) { + throw new Error('Bootstrap node ENR is not available'); } - await bootstrapNode.stop(); + const contexts: NodeContext[] = []; + const nodes: AztecNodeService[] = await createNodes( + config, + PEER_ID_PRIVATE_KEYS, + bootstrapNodeEnr, + NUM_NODES, + BOOT_NODE_UDP_PORT, + ); + + // wait a bit for peers to discover each other + await sleep(4000); + + // Replace the p2p node implementation of some of the nodes with a spy such that it does not store transactions that are gossiped to it + // Original implementation of `processTxFromPeer` will store received transactions in the tx pool. + // We have chosen nodes 0,3 as they do not get chosen to be the sequencer in this test. + const nodeToTurnOffTxGossip = [0, 3]; + for (const nodeIndex of nodeToTurnOffTxGossip) { + jest + .spyOn((nodes[nodeIndex] as any).p2pClient.p2pService, 'processTxFromPeer') + .mockImplementation((): Promise => { + return Promise.resolve(); + }); + } + + // Only submit transactions to the first two nodes, so that we avoid our sequencer with a mocked p2p layer being picked to produce a block. + // If the shuffling algorithm changes, then this will need to be updated. + for (let i = 0; i < 2; i++) { + const context = await createPXEServiceAndSubmitTransactions(nodes[i], NUM_TXS_PER_NODE); + contexts.push(context); + } + + await Promise.all( + contexts.flatMap((context, i) => + context.txs.map(async (tx, j) => { + logger.info(`Waiting for tx ${i}-${j}: ${await tx.getTxHash()} to be mined`); + await tx.wait(); + logger.info(`Tx ${i}-${j}: ${await tx.getTxHash()} has been mined`); + return await tx.getTxHash(); + }), + ), + ); + + await stopNodes(bootstrapNode, nodes); }); it('should re-discover stored peers without bootstrap node', async () => { @@ -218,11 +287,7 @@ describe('e2e_p2p_network', () => { ); // shutdown all nodes. - // for (const context of contexts) { - for (const context of contexts) { - await context.node.stop(); - await context.pxeService.stop(); - } + await stopNodes(bootstrapNode, newNodes); }); // creates an instance of the PXE and submit a given number of transactions to it. diff --git a/yarn-project/foundation/src/buffer/buffer32.ts b/yarn-project/foundation/src/buffer/buffer32.ts index 399cdb009e3..736c0ada457 100644 --- a/yarn-project/foundation/src/buffer/buffer32.ts +++ b/yarn-project/foundation/src/buffer/buffer32.ts @@ -116,6 +116,15 @@ export class Buffer32 { return new Buffer32(Buffer.from(str, 'hex')); } + /** + * Converts a number into a Buffer32 object. + * @param num - The number to convert. + * @returns A new Buffer32 object. + */ + public static fromNumber(num: number): Buffer32 { + return new Buffer32(serializeBigInt(BigInt(num), Buffer32.SIZE)); + } + /** * Generates a random Buffer32. * @returns A new Buffer32 object. diff --git a/yarn-project/foundation/src/config/env_var.ts b/yarn-project/foundation/src/config/env_var.ts index 7b01983b743..be9b805d42a 100644 --- a/yarn-project/foundation/src/config/env_var.ts +++ b/yarn-project/foundation/src/config/env_var.ts @@ -109,6 +109,8 @@ export type EnvVar = | 'BOT_FLUSH_SETUP_TRANSACTIONS' | 'VALIDATOR_PRIVATE_KEY' | 'VALIDATOR_DISABLED' + | 'VALIDATOR_ATTESTATIONS_WAIT_TIMEOUT_MS' + | 'VALIDATOR_ATTESTATIONS_POOLING_INTERVAL_MS' | 'PROVER_NODE_DISABLE_AUTOMATIC_PROVING' | 'PROVER_NODE_MAX_PENDING_JOBS' | 'PROOF_VERIFIER_POLL_INTERVAL_MS' diff --git a/yarn-project/p2p/src/attestation_pool/mocks.ts b/yarn-project/p2p/src/attestation_pool/mocks.ts index 22e5da70a94..00194d52b96 100644 --- a/yarn-project/p2p/src/attestation_pool/mocks.ts +++ b/yarn-project/p2p/src/attestation_pool/mocks.ts @@ -1,6 +1,7 @@ -import { BlockAttestation, Signature } from '@aztec/circuit-types'; +import { BlockAttestation, Signature, TxHash } from '@aztec/circuit-types'; import { makeHeader } from '@aztec/circuits.js/testing'; import { Fr } from '@aztec/foundation/fields'; +import { serializeToBuffer } from '@aztec/foundation/serialize'; import { type PrivateKeyAccount } from 'viem'; import { generatePrivateKey, privateKeyToAccount } from 'viem/accounts'; @@ -25,9 +26,11 @@ export const mockAttestation = async (signer: PrivateKeyAccount, slot: number = // Use arbitrary numbers for all other than slot const header = makeHeader(1, 2, slot); const archive = Fr.random(); - const message = archive.toString(); + const txs = [0, 1, 2, 3, 4, 5].map(() => TxHash.random()); + + const message: `0x${string}` = `0x${serializeToBuffer([archive, txs]).toString('hex')}`; const sigString = await signer.signMessage({ message }); const signature = Signature.from0xString(sigString); - return new BlockAttestation(header, archive, signature); + return new BlockAttestation(header, archive, txs, signature); }; diff --git a/yarn-project/p2p/src/client/p2p_client.ts b/yarn-project/p2p/src/client/p2p_client.ts index 0c1890beeb8..9fec7856fae 100644 --- a/yarn-project/p2p/src/client/p2p_client.ts +++ b/yarn-project/p2p/src/client/p2p_client.ts @@ -70,7 +70,14 @@ export interface P2P { */ // REVIEW: https://github.com/AztecProtocol/aztec-packages/issues/7963 // ^ This pattern is not my favorite (md) - registerBlockProposalHandler(handler: (block: BlockProposal) => Promise): void; + registerBlockProposalHandler(handler: (block: BlockProposal) => Promise): void; + + /** + * Request a list of transactions from another peer by their tx hashes. + * @param txHashes - Hashes of the txs to query. + * @returns A list of transactions or undefined if the transactions are not found. + */ + requestTxs(txHashes: TxHash[]): Promise<(Tx | undefined)[]>; /** * Request a transaction from another peer by its tx hash. @@ -270,6 +277,7 @@ export class P2PClient implements P2P { } public broadcastProposal(proposal: BlockProposal): void { + this.log.verbose(`Broadcasting proposal ${proposal.p2pMessageIdentifier()} to peers`); return this.p2pService.propagate(proposal); } @@ -279,13 +287,44 @@ export class P2PClient implements P2P { // REVIEW: https://github.com/AztecProtocol/aztec-packages/issues/7963 // ^ This pattern is not my favorite (md) - public registerBlockProposalHandler(handler: (block: BlockProposal) => Promise): void { + public registerBlockProposalHandler(handler: (block: BlockProposal) => Promise): void { this.p2pService.registerBlockReceivedCallback(handler); } - public requestTxByHash(txHash: TxHash): Promise { - // Underlying I want to use the libp2p service to just have a request method where the subprotocol is defined here - return this.p2pService.sendRequest(TX_REQ_PROTOCOL, txHash); + /** + * Requests the transactions with the given hashes from the network. + * + * If a transaction can be retrieved, it will be returned, if not an undefined + * will be returned. In place. + * + * @param txHashes - The hashes of the transactions to request. + * @returns A promise that resolves to an array of transactions or undefined. + */ + public requestTxs(txHashes: TxHash[]): Promise<(Tx | undefined)[]> { + const requestPromises = txHashes.map(txHash => this.requestTxByHash(txHash)); + return Promise.all(requestPromises); + } + + /** + * Uses the Request Response protocol to request a transaction from the network. + * + * If the underlying request response protocol fails, then we return undefined. + * If it succeeds then we add the transaction to our transaction pool and return. + * + * @param txHash - The hash of the transaction to request. + * @returns A promise that resolves to a transaction or undefined. + */ + public async requestTxByHash(txHash: TxHash): Promise { + const tx = await this.p2pService.sendRequest(TX_REQ_PROTOCOL, txHash); + + this.log.debug(`Requested ${txHash.toString()} from peer | success = ${!!tx}`); + if (tx) { + // TODO(https://github.com/AztecProtocol/aztec-packages/issues/8485): This check is not sufficient to validate the transaction. We need to validate the entire proof. + // TODO(https://github.com/AztecProtocol/aztec-packages/issues/8483): alter peer scoring system for a validator that returns an invalid transcation + await this.txPool.addTxs([tx]); + } + + return tx; } /** diff --git a/yarn-project/p2p/src/service/libp2p_service.ts b/yarn-project/p2p/src/service/libp2p_service.ts index c62a694034f..caf57a821ea 100644 --- a/yarn-project/p2p/src/service/libp2p_service.ts +++ b/yarn-project/p2p/src/service/libp2p_service.ts @@ -390,7 +390,7 @@ export class LibP2PService implements P2PService { this.logger.verbose(`Sending message ${identifier} to peers`); const recipientsNum = await this.publishToTopic(parent.p2pTopic, message.toBuffer()); - this.logger.verbose(`Sent tx ${identifier} to ${recipientsNum} peers`); + this.logger.verbose(`Sent message ${identifier} to ${recipientsNum} peers`); } // Libp2p seems to hang sometimes if new peers are initiating connections. diff --git a/yarn-project/p2p/src/service/reqresp/interface.ts b/yarn-project/p2p/src/service/reqresp/interface.ts index 39f27b6268f..5ae61d0389d 100644 --- a/yarn-project/p2p/src/service/reqresp/interface.ts +++ b/yarn-project/p2p/src/service/reqresp/interface.ts @@ -3,9 +3,9 @@ import { Tx, TxHash } from '@aztec/circuit-types'; /* * Request Response Sub Protocols */ -export const PING_PROTOCOL = '/aztec/ping/0.1.0'; -export const STATUS_PROTOCOL = '/aztec/status/0.1.0'; -export const TX_REQ_PROTOCOL = '/aztec/tx_req/0.1.0'; +export const PING_PROTOCOL = '/aztec/req/ping/0.1.0'; +export const STATUS_PROTOCOL = '/aztec/req/status/0.1.0'; +export const TX_REQ_PROTOCOL = '/aztec/req/tx/0.1.0'; // Sum type for sub protocols export type ReqRespSubProtocol = typeof PING_PROTOCOL | typeof STATUS_PROTOCOL | typeof TX_REQ_PROTOCOL; diff --git a/yarn-project/p2p/src/service/reqresp/reqresp.ts b/yarn-project/p2p/src/service/reqresp/reqresp.ts index 2851c9e9fce..dada63424c2 100644 --- a/yarn-project/p2p/src/service/reqresp/reqresp.ts +++ b/yarn-project/p2p/src/service/reqresp/reqresp.ts @@ -96,8 +96,13 @@ export class ReqResp { ): Promise { try { const stream = await this.libp2p.dialProtocol(peerId, subProtocol); + this.logger.debug(`Stream opened with ${peerId.publicKey} for ${subProtocol}`); const result = await pipe([payload], stream, this.readMessage); + + await stream.close(); + this.logger.debug(`Stream closed with ${peerId.publicKey} for ${subProtocol}`); + return result; } catch (e) { this.logger.warn(`Failed to send request to peer ${peerId.publicKey}`); diff --git a/yarn-project/p2p/src/service/service.ts b/yarn-project/p2p/src/service/service.ts index 607927d3454..ce486e0b2bb 100644 --- a/yarn-project/p2p/src/service/service.ts +++ b/yarn-project/p2p/src/service/service.ts @@ -46,7 +46,7 @@ export interface P2PService { ): Promise | undefined>; // Leaky abstraction: fix https://github.com/AztecProtocol/aztec-packages/issues/7963 - registerBlockReceivedCallback(callback: (block: BlockProposal) => Promise): void; + registerBlockReceivedCallback(callback: (block: BlockProposal) => Promise): void; getEnr(): ENR | undefined; } diff --git a/yarn-project/sequencer-client/src/publisher/l1-publisher.test.ts b/yarn-project/sequencer-client/src/publisher/l1-publisher.test.ts index d06fdf32116..32cec46214c 100644 --- a/yarn-project/sequencer-client/src/publisher/l1-publisher.test.ts +++ b/yarn-project/sequencer-client/src/publisher/l1-publisher.test.ts @@ -123,6 +123,7 @@ describe('L1Publisher', () => { `0x${archive.toString('hex')}`, `0x${blockHash.toString('hex')}`, [], + [], `0x${body.toString('hex')}`, ] as const; expect(rollupContractWrite.propose).toHaveBeenCalledWith(args, { diff --git a/yarn-project/sequencer-client/src/publisher/l1-publisher.ts b/yarn-project/sequencer-client/src/publisher/l1-publisher.ts index e473f95329b..b92561f6808 100644 --- a/yarn-project/sequencer-client/src/publisher/l1-publisher.ts +++ b/yarn-project/sequencer-client/src/publisher/l1-publisher.ts @@ -1,4 +1,5 @@ -import { type L2Block, type Signature } from '@aztec/circuit-types'; +import { type L2Block, type Signature, type TxHash } from '@aztec/circuit-types'; +import { getHashedSignaturePayload } from '@aztec/circuit-types'; import { type L1PublishBlockStats, type L1PublishProofStats } from '@aztec/circuit-types/stats'; import { ETHEREUM_SLOT_DURATION, EthAddress, type Header, type Proof } from '@aztec/circuits.js'; import { createEthereumChain } from '@aztec/ethereum'; @@ -32,6 +33,7 @@ import type * as chains from 'viem/chains'; import { type PublisherConfig, type TxSenderConfig } from './config.js'; import { L1PublisherMetrics } from './l1-publisher-metrics.js'; +import { prettyLogVeimError } from './utils.js'; /** * Stats for a sent transaction. @@ -71,6 +73,8 @@ export type L1ProcessArgs = { blockHash: Buffer; /** L2 block body. */ body: Buffer; + /** L2 block tx hashes */ + txHashes: TxHash[]; /** Attestations */ attestations?: Signature[]; }; @@ -225,19 +229,21 @@ export class L1Publisher { * @param block - L2 block to publish. * @returns True once the tx has been confirmed and is successful, false on revert or interrupt, blocks otherwise. */ - public async processL2Block(block: L2Block, attestations?: Signature[]): Promise { + public async processL2Block(block: L2Block, attestations?: Signature[], txHashes?: TxHash[]): Promise { const ctx = { blockNumber: block.number, slotNumber: block.header.globalVariables.slotNumber.toBigInt(), blockHash: block.hash().toString(), }; - const processTxArgs = { + const digest = getHashedSignaturePayload(block.archive.root, txHashes ?? []); + const proposeTxArgs = { header: block.header.toBuffer(), archive: block.archive.root.toBuffer(), blockHash: block.header.hash().toBuffer(), body: block.body.toBuffer(), attestations, + txHashes: txHashes ?? [], }; // Publish body and propose block (if not already published) @@ -249,11 +255,11 @@ export class L1Publisher { // By simulation issue, I mean the fact that the block.timestamp is equal to the last block, not the next, which // make time consistency checks break. await this.validateBlockForSubmission(block.header, { - digest: block.archive.root.toBuffer(), + digest, signatures: attestations ?? [], }); - const txHash = await this.sendProposeTx(processTxArgs); + const txHash = await this.sendProposeTx(proposeTxArgs); if (!txHash) { this.log.info(`Failed to publish block ${block.number} to L1`, ctx); @@ -411,10 +417,12 @@ export class L1Publisher { const attestations = encodedData.attestations ? encodedData.attestations.map(attest => attest.toViemSignature()) : []; + const txHashes = encodedData.txHashes ? encodedData.txHashes.map(txHash => txHash.to0xString()) : []; const args = [ `0x${encodedData.header.toString('hex')}`, `0x${encodedData.archive.toString('hex')}`, `0x${encodedData.blockHash.toString('hex')}`, + txHashes, attestations, `0x${encodedData.body.toString('hex')}`, ] as const; @@ -424,6 +432,7 @@ export class L1Publisher { gas: gasGuesstimate, }); } catch (err) { + prettyLogVeimError(err, this.log); this.log.error(`Rollup publish failed`, err); return undefined; } diff --git a/yarn-project/sequencer-client/src/publisher/utils.ts b/yarn-project/sequencer-client/src/publisher/utils.ts new file mode 100644 index 00000000000..13842102a2c --- /dev/null +++ b/yarn-project/sequencer-client/src/publisher/utils.ts @@ -0,0 +1,15 @@ +import { type Logger } from '@aztec/foundation/log'; + +import { BaseError, ContractFunctionRevertedError } from 'viem'; + +export function prettyLogVeimError(err: any, logger: Logger) { + if (err instanceof BaseError) { + const revertError = err.walk(err => err instanceof ContractFunctionRevertedError); + if (revertError instanceof ContractFunctionRevertedError) { + const errorName = revertError.data?.errorName ?? ''; + const args = + revertError.metaMessages && revertError.metaMessages?.length > 1 ? revertError.metaMessages[1].trimStart() : ''; + logger.debug(`canProposeAtTime failed with "${errorName}${args}"`); + } + } +} diff --git a/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts b/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts index 462d6687dad..39fda5b0ae2 100644 --- a/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts +++ b/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts @@ -75,7 +75,7 @@ describe('sequencer', () => { const committee = [EthAddress.random()]; const getSignatures = () => [mockedSig]; const getAttestations = () => { - const attestation = new BlockAttestation(block.header, archive, mockedSig); + const attestation = new BlockAttestation(block.header, archive, [], mockedSig); (attestation as any).sender = committee[0]; return [attestation]; @@ -180,6 +180,7 @@ describe('sequencer', () => { it('builds a block out of a single tx', async () => { const tx = mockTxForRollup(); tx.data.constants.txContext.chainId = chainId; + const txHash = tx.getTxHash(); const result: ProvingSuccess = { status: PROVING_STATUS.SUCCESS, @@ -205,13 +206,14 @@ describe('sequencer', () => { ); // Ok, we have an issue that we never actually call the process L2 block expect(publisher.processL2Block).toHaveBeenCalledTimes(1); - expect(publisher.processL2Block).toHaveBeenCalledWith(block, getSignatures()); + expect(publisher.processL2Block).toHaveBeenCalledWith(block, getSignatures(), [txHash]); expect(blockSimulator.cancelBlock).toHaveBeenCalledTimes(0); }); it('builds a block when it is their turn', async () => { const tx = mockTxForRollup(); tx.data.constants.txContext.chainId = chainId; + const txHash = tx.getTxHash(); const result: ProvingSuccess = { status: PROVING_STATUS.SUCCESS, }; @@ -253,16 +255,19 @@ describe('sequencer', () => { mockedGlobalVariables, Array(NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP).fill(new Fr(0n)), ); - expect(publisher.processL2Block).toHaveBeenCalledWith(block, getSignatures()); + expect(publisher.processL2Block).toHaveBeenCalledWith(block, getSignatures(), [txHash]); expect(blockSimulator.cancelBlock).toHaveBeenCalledTimes(0); }); it('builds a block out of several txs rejecting double spends', async () => { + const doubleSpendTxIndex = 1; const txs = [mockTxForRollup(0x10000), mockTxForRollup(0x20000), mockTxForRollup(0x30000)]; txs.forEach(tx => { tx.data.constants.txContext.chainId = chainId; }); - const doubleSpendTx = txs[1]; + const validTxHashes = txs.filter((_, i) => i !== doubleSpendTxIndex).map(tx => tx.getTxHash()); + + const doubleSpendTx = txs[doubleSpendTxIndex]; const result: ProvingSuccess = { status: PROVING_STATUS.SUCCESS, }; @@ -293,17 +298,20 @@ describe('sequencer', () => { mockedGlobalVariables, Array(NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP).fill(new Fr(0n)), ); - expect(publisher.processL2Block).toHaveBeenCalledWith(block, getSignatures()); + expect(publisher.processL2Block).toHaveBeenCalledWith(block, getSignatures(), validTxHashes); expect(p2p.deleteTxs).toHaveBeenCalledWith([doubleSpendTx.getTxHash()]); expect(blockSimulator.cancelBlock).toHaveBeenCalledTimes(0); }); it('builds a block out of several txs rejecting incorrect chain ids', async () => { + const invalidChainTxIndex = 1; const txs = [mockTxForRollup(0x10000), mockTxForRollup(0x20000), mockTxForRollup(0x30000)]; txs.forEach(tx => { tx.data.constants.txContext.chainId = chainId; }); - const invalidChainTx = txs[1]; + const invalidChainTx = txs[invalidChainTxIndex]; + const validTxHashes = txs.filter((_, i) => i !== invalidChainTxIndex).map(tx => tx.getTxHash()); + const result: ProvingSuccess = { status: PROVING_STATUS.SUCCESS, }; @@ -329,16 +337,20 @@ describe('sequencer', () => { mockedGlobalVariables, Array(NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP).fill(new Fr(0n)), ); - expect(publisher.processL2Block).toHaveBeenCalledWith(block, getSignatures()); + expect(publisher.processL2Block).toHaveBeenCalledWith(block, getSignatures(), validTxHashes); expect(p2p.deleteTxs).toHaveBeenCalledWith([invalidChainTx.getTxHash()]); expect(blockSimulator.cancelBlock).toHaveBeenCalledTimes(0); }); it('builds a block out of several txs dropping the ones that go over max size', async () => { + const invalidTransactionIndex = 1; + const txs = [mockTxForRollup(0x10000), mockTxForRollup(0x20000), mockTxForRollup(0x30000)]; txs.forEach(tx => { tx.data.constants.txContext.chainId = chainId; }); + const validTxHashes = txs.filter((_, i) => i !== invalidTransactionIndex).map(tx => tx.getTxHash()); + const result: ProvingSuccess = { status: PROVING_STATUS.SUCCESS, }; @@ -354,8 +366,9 @@ describe('sequencer', () => { globalVariableBuilder.buildGlobalVariables.mockResolvedValueOnce(mockedGlobalVariables); // We make txs[1] too big to fit - (txs[1] as Writeable).unencryptedLogs = UnencryptedTxL2Logs.random(2, 4); - (txs[1].unencryptedLogs.functionLogs[0].logs[0] as Writeable).data = randomBytes(1024 * 1022); + (txs[invalidTransactionIndex] as Writeable).unencryptedLogs = UnencryptedTxL2Logs.random(2, 4); + (txs[invalidTransactionIndex].unencryptedLogs.functionLogs[0].logs[0] as Writeable).data = + randomBytes(1024 * 1022); await sequencer.initialSync(); await sequencer.work(); @@ -365,7 +378,7 @@ describe('sequencer', () => { mockedGlobalVariables, Array(NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP).fill(new Fr(0n)), ); - expect(publisher.processL2Block).toHaveBeenCalledWith(block, getSignatures()); + expect(publisher.processL2Block).toHaveBeenCalledWith(block, getSignatures(), validTxHashes); expect(blockSimulator.cancelBlock).toHaveBeenCalledTimes(0); }); @@ -401,11 +414,14 @@ describe('sequencer', () => { // block is not built with 3 txs p2p.getTxs.mockReturnValueOnce(txs.slice(0, 3)); + await sequencer.work(); expect(blockSimulator.startNewBlock).toHaveBeenCalledTimes(0); // block is built with 4 txs p2p.getTxs.mockReturnValueOnce(txs.slice(0, 4)); + const txHashes = txs.slice(0, 4).map(tx => tx.getTxHash()); + await sequencer.work(); expect(blockSimulator.startNewBlock).toHaveBeenCalledWith( 4, @@ -413,7 +429,7 @@ describe('sequencer', () => { Array(NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP).fill(new Fr(0n)), ); expect(publisher.processL2Block).toHaveBeenCalledTimes(1); - expect(publisher.processL2Block).toHaveBeenCalledWith(block, getSignatures()); + expect(publisher.processL2Block).toHaveBeenCalledWith(block, getSignatures(), txHashes); expect(blockSimulator.cancelBlock).toHaveBeenCalledTimes(0); }); @@ -464,7 +480,7 @@ describe('sequencer', () => { Array(NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP).fill(new Fr(0n)), ); expect(publisher.processL2Block).toHaveBeenCalledTimes(1); - expect(publisher.processL2Block).toHaveBeenCalledWith(block, getSignatures()); + expect(publisher.processL2Block).toHaveBeenCalledWith(block, getSignatures(), []); expect(blockSimulator.cancelBlock).toHaveBeenCalledTimes(0); }); @@ -506,7 +522,9 @@ describe('sequencer', () => { sequencer.flush(); // block is built with 3 txs - p2p.getTxs.mockReturnValueOnce(txs.slice(0, 3)); + const postFlushTxs = txs.slice(0, 3); + p2p.getTxs.mockReturnValueOnce(postFlushTxs); + const postFlushTxHashes = postFlushTxs.map(tx => tx.getTxHash()); await sequencer.work(); expect(blockSimulator.startNewBlock).toHaveBeenCalledTimes(1); expect(blockSimulator.startNewBlock).toHaveBeenCalledWith( @@ -516,7 +534,7 @@ describe('sequencer', () => { ); expect(publisher.processL2Block).toHaveBeenCalledTimes(1); - expect(publisher.processL2Block).toHaveBeenCalledWith(block, getSignatures()); + expect(publisher.processL2Block).toHaveBeenCalledWith(block, getSignatures(), postFlushTxHashes); expect(blockSimulator.cancelBlock).toHaveBeenCalledTimes(0); }); diff --git a/yarn-project/sequencer-client/src/sequencer/sequencer.ts b/yarn-project/sequencer-client/src/sequencer/sequencer.ts index 57dd47d59ff..611994cdaec 100644 --- a/yarn-project/sequencer-client/src/sequencer/sequencer.ts +++ b/yarn-project/sequencer-client/src/sequencer/sequencer.ts @@ -6,6 +6,7 @@ import { type ProcessedTx, Signature, Tx, + type TxHash, type TxValidator, } from '@aztec/circuit-types'; import { type AllowedElement, BlockProofError, PROVING_STATUS } from '@aztec/circuit-types/interfaces'; @@ -29,11 +30,10 @@ import { Attributes, type TelemetryClient, type Tracer, trackSpan } from '@aztec import { type ValidatorClient } from '@aztec/validator-client'; import { type WorldStateStatus, type WorldStateSynchronizer } from '@aztec/world-state'; -import { BaseError, ContractFunctionRevertedError } from 'viem'; - import { type BlockBuilderFactory } from '../block_builder/index.js'; import { type GlobalVariableBuilder } from '../global_variable_builder/global_builder.js'; import { type L1Publisher } from '../publisher/l1-publisher.js'; +import { prettyLogVeimError } from '../publisher/utils.js'; import { type TxValidatorFactory } from '../tx_validator/tx_validator_factory.js'; import { type SequencerConfig } from './config.js'; import { SequencerMetrics } from './metrics.js'; @@ -310,17 +310,7 @@ export class Sequencer { this.log.debug(`Can propose block ${proposalBlockNumber} at slot ${slot}`); return slot; } catch (err) { - if (err instanceof BaseError) { - const revertError = err.walk(err => err instanceof ContractFunctionRevertedError); - if (revertError instanceof ContractFunctionRevertedError) { - const errorName = revertError.data?.errorName ?? ''; - const args = - revertError.metaMessages && revertError.metaMessages?.length > 1 - ? revertError.metaMessages[1].trimStart() - : ''; - this.log.debug(`canProposeAtTime failed with "${errorName}${args}"`); - } - } + prettyLogVeimError(err, this.log); throw err; } } @@ -490,11 +480,15 @@ export class Sequencer { this.log.verbose(`Flushing completed`); } + const txHashes = validTxs.map(tx => tx.getTxHash()); + this.isFlushing = false; - const attestations = await this.collectAttestations(block); + this.log.verbose('Collecting attestations'); + const attestations = await this.collectAttestations(block, txHashes); + this.log.verbose('Attestations collected'); try { - await this.publishL2Block(block, attestations); + await this.publishL2Block(block, attestations, txHashes); this.metrics.recordPublishedBlock(workDuration); this.log.info( `Submitted rollup block ${block.number} with ${ @@ -512,11 +506,13 @@ export class Sequencer { this.isFlushing = true; } - protected async collectAttestations(block: L2Block): Promise { + protected async collectAttestations(block: L2Block, txHashes: TxHash[]): Promise { // TODO(https://github.com/AztecProtocol/aztec-packages/issues/7962): inefficient to have a round trip in here - this should be cached const committee = await this.publisher.getCurrentEpochCommittee(); + this.log.debug(`Attesting committee length ${committee.length}`); if (committee.length === 0) { + this.log.debug(`Attesting committee length is 0, skipping`); return undefined; } @@ -531,13 +527,16 @@ export class Sequencer { // TODO(https://github.com/AztecProtocol/aztec-packages/issues/7974): we do not have transaction[] lists in the block for now // Dont do anything with the proposals for now - just collect them - const proposal = await this.validatorClient.createBlockProposal(block.header, block.archive.root, []); + this.log.verbose('ATTEST | Creating block proposal'); + const proposal = await this.validatorClient.createBlockProposal(block.header, block.archive.root, txHashes); this.state = SequencerState.PUBLISHING_BLOCK_TO_PEERS; + this.log.verbose('Broadcasting block proposal to validators'); this.validatorClient.broadcastBlockProposal(proposal); this.state = SequencerState.WAITING_FOR_ATTESTATIONS; const attestations = await this.validatorClient.collectAttestations(proposal, numberOfRequiredAttestations); + this.log.verbose(`Collected attestations from validators, number of attestations: ${attestations.length}`); // note: the smart contract requires that the signatures are provided in the order of the committee return await orderAttestations(attestations, committee); @@ -550,11 +549,11 @@ export class Sequencer { @trackSpan('Sequencer.publishL2Block', block => ({ [Attributes.BLOCK_NUMBER]: block.number, })) - protected async publishL2Block(block: L2Block, attestations?: Signature[]) { + protected async publishL2Block(block: L2Block, attestations?: Signature[], txHashes?: TxHash[]) { // Publishes new block to the network and awaits the tx to be mined this.state = SequencerState.PUBLISHING_BLOCK; - const publishedL2Block = await this.publisher.processL2Block(block, attestations); + const publishedL2Block = await this.publisher.processL2Block(block, attestations, txHashes); if (publishedL2Block) { this.lastPublishedBlock = block.number; } else { diff --git a/yarn-project/validator-client/package.json b/yarn-project/validator-client/package.json index e89785cf9dc..23ca3b59106 100644 --- a/yarn-project/validator-client/package.json +++ b/yarn-project/validator-client/package.json @@ -72,6 +72,7 @@ "@types/jest": "^29.5.0", "@types/node": "^18.7.23", "jest": "^29.5.0", + "jest-mock-extended": "^3.0.7", "ts-node": "^10.9.1", "typescript": "^5.0.4" }, diff --git a/yarn-project/validator-client/src/config.ts b/yarn-project/validator-client/src/config.ts index 083beead6c1..241bffbfda1 100644 --- a/yarn-project/validator-client/src/config.ts +++ b/yarn-project/validator-client/src/config.ts @@ -1,5 +1,11 @@ +import { AZTEC_SLOT_DURATION } from '@aztec/circuits.js'; import { NULL_KEY } from '@aztec/ethereum'; -import { type ConfigMappingsType, booleanConfigHelper, getConfigFromMappings } from '@aztec/foundation/config'; +import { + type ConfigMappingsType, + booleanConfigHelper, + getConfigFromMappings, + numberConfigHelper, +} from '@aztec/foundation/config'; /** * The Validator Configuration @@ -10,6 +16,12 @@ export interface ValidatorClientConfig { /** Do not run the validator */ disableValidator: boolean; + + /** Interval between polling for new attestations from peers */ + attestationPoolingIntervalMs: number; + + /** Wait for attestations timeout */ + attestationWaitTimeoutMs: number; } export const validatorClientConfigMappings: ConfigMappingsType = { @@ -23,6 +35,16 @@ export const validatorClientConfigMappings: ConfigMappingsType { - // Note: just signing the archive for now - const archiveBuf = archive.toBuffer(); - const sig = await this.keyStore.sign(archiveBuf); + createBlockProposal(header: Header, archive: Fr, txs: TxHash[]): Promise { + const payloadSigner = (payload: Buffer) => this.keyStore.sign(payload); - return new BlockProposal(header, archive, txs, sig); + return BlockProposal.createProposalFromSigner(header, archive, txs, payloadSigner); } /** * Attest to the given block proposal constructed by the current sequencer * + * NOTE: This is just a blind signing. + * We assume that the proposal is valid and DA guarantees have been checked previously. + * * @param proposal - The proposal to attest to * @returns attestation */ async attestToProposal(proposal: BlockProposal): Promise { // TODO(https://github.com/AztecProtocol/aztec-packages/issues/7961): check that the current validator is correct - const buf = proposal.archive.toBuffer(); + const buf = keccak256(proposal.getPayload()); const sig = await this.keyStore.sign(buf); - return new BlockAttestation(proposal.header, proposal.archive, sig); + return new BlockAttestation(proposal.header, proposal.archive, proposal.txs, sig); } } diff --git a/yarn-project/validator-client/src/errors/index.ts b/yarn-project/validator-client/src/errors/index.ts new file mode 100644 index 00000000000..00a5c872c01 --- /dev/null +++ b/yarn-project/validator-client/src/errors/index.ts @@ -0,0 +1 @@ +export * from './validator.error.js'; diff --git a/yarn-project/validator-client/src/errors/validator.error.ts b/yarn-project/validator-client/src/errors/validator.error.ts new file mode 100644 index 00000000000..5cc3929962e --- /dev/null +++ b/yarn-project/validator-client/src/errors/validator.error.ts @@ -0,0 +1,19 @@ +import { type TxHash } from '@aztec/circuit-types/tx_hash'; + +export class ValidatorError extends Error { + constructor(message: string) { + super(message); + } +} + +export class AttestationTimeoutError extends ValidatorError { + constructor(numberOfRequiredAttestations: number, slot: bigint) { + super(`Timeout waiting for ${numberOfRequiredAttestations} attestations for slot, ${slot}`); + } +} + +export class TransactionsNotAvailableError extends ValidatorError { + constructor(txHashes: TxHash[]) { + super(`Transactions not available: ${txHashes.join(', ')}`); + } +} diff --git a/yarn-project/validator-client/src/validator.test.ts b/yarn-project/validator-client/src/validator.test.ts new file mode 100644 index 00000000000..4802656e5ce --- /dev/null +++ b/yarn-project/validator-client/src/validator.test.ts @@ -0,0 +1,70 @@ +/** + * Validation logic unit tests + */ +import { TxHash } from '@aztec/circuit-types'; +import { makeHeader } from '@aztec/circuits.js/testing'; +import { EthAddress } from '@aztec/foundation/eth-address'; +import { Fr } from '@aztec/foundation/fields'; +import { type P2P } from '@aztec/p2p'; + +import { describe, expect, it } from '@jest/globals'; +import { type MockProxy, mock } from 'jest-mock-extended'; +import { type PrivateKeyAccount, generatePrivateKey, privateKeyToAccount } from 'viem/accounts'; + +import { makeBlockProposal } from '../../circuit-types/src/p2p/mocks.js'; +import { AttestationTimeoutError, TransactionsNotAvailableError } from './errors/validator.error.js'; +import { ValidatorClient } from './validator.js'; + +describe('ValidationService', () => { + let validatorClient: ValidatorClient; + let p2pClient: MockProxy; + let validatorAccount: PrivateKeyAccount; + + beforeEach(() => { + p2pClient = mock(); + p2pClient.getAttestationsForSlot.mockImplementation(() => Promise.resolve([])); + + const validatorPrivateKey = generatePrivateKey(); + validatorAccount = privateKeyToAccount(validatorPrivateKey); + + const config = { + validatorPrivateKey: validatorPrivateKey, + attestationPoolingIntervalMs: 1000, + attestationWaitTimeoutMs: 1000, + disableValidator: false, + }; + validatorClient = ValidatorClient.new(config, p2pClient); + }); + + it('Should create a valid block proposal', async () => { + const header = makeHeader(); + const archive = Fr.random(); + const txs = [1, 2, 3, 4, 5].map(() => TxHash.random()); + + const blockProposal = await validatorClient.createBlockProposal(header, archive, txs); + + expect(blockProposal).toBeDefined(); + + const validatorAddress = EthAddress.fromString(validatorAccount.address); + expect(await blockProposal.getSender()).toEqual(validatorAddress); + }); + + it('Should a timeout if we do not collect enough attestations in time', async () => { + const proposal = await makeBlockProposal(); + + await expect(validatorClient.collectAttestations(proposal, 2)).rejects.toThrow(AttestationTimeoutError); + }); + + it('Should throw an error if the transactions are not available', async () => { + const proposal = await makeBlockProposal(); + + // mock the p2pClient.getTxStatus to return undefined for all transactions + p2pClient.getTxStatus.mockImplementation(() => undefined); + // Mock the p2pClient.requestTxs to return undefined for all transactions + p2pClient.requestTxs.mockImplementation(() => Promise.resolve([undefined])); + + await expect(validatorClient.ensureTransactionsAreAvailable(proposal)).rejects.toThrow( + TransactionsNotAvailableError, + ); + }); +}); diff --git a/yarn-project/validator-client/src/validator.ts b/yarn-project/validator-client/src/validator.ts index 7e44d06b9c2..1c8442ef367 100644 --- a/yarn-project/validator-client/src/validator.ts +++ b/yarn-project/validator-client/src/validator.ts @@ -7,6 +7,7 @@ import { type P2P } from '@aztec/p2p'; import { type ValidatorClientConfig } from './config.js'; import { ValidationService } from './duties/validation_service.js'; +import { AttestationTimeoutError, TransactionsNotAvailableError } from './errors/validator.error.js'; import { type ValidatorKeyStore } from './key_store/interface.js'; import { LocalKeyStore } from './key_store/local_key_store.js'; @@ -18,7 +19,6 @@ export interface Validator { createBlockProposal(header: Header, archive: Fr, txs: TxHash[]): Promise; attestToProposal(proposal: BlockProposal): void; - // TODO(md): possible abstraction leak broadcastBlockProposal(proposal: BlockProposal): void; collectAttestations(proposal: BlockProposal, numberOfRequiredAttestations: number): Promise; } @@ -26,11 +26,15 @@ export interface Validator { /** Validator Client */ export class ValidatorClient implements Validator { - private attestationPoolingIntervalMs: number = 1000; - private validationService: ValidationService; - constructor(keyStore: ValidatorKeyStore, private p2pClient: P2P, private log = createDebugLogger('aztec:validator')) { + constructor( + keyStore: ValidatorKeyStore, + private p2pClient: P2P, + private attestationPoolingIntervalMs: number, + private attestationWaitTimeoutMs: number, + private log = createDebugLogger('aztec:validator'), + ) { //TODO: We need to setup and store all of the currently active validators https://github.com/AztecProtocol/aztec-packages/issues/7962 this.validationService = new ValidationService(keyStore); @@ -40,7 +44,12 @@ export class ValidatorClient implements Validator { static new(config: ValidatorClientConfig, p2pClient: P2P) { const localKeyStore = new LocalKeyStore(config.validatorPrivateKey); - const validator = new ValidatorClient(localKeyStore, p2pClient); + const validator = new ValidatorClient( + localKeyStore, + p2pClient, + config.attestationPoolingIntervalMs, + config.attestationWaitTimeoutMs, + ); validator.registerBlockProposalHandler(); return validator; } @@ -54,16 +63,55 @@ export class ValidatorClient implements Validator { } public registerBlockProposalHandler() { - const handler = (block: BlockProposal): Promise => { - return this.validationService.attestToProposal(block); + const handler = (block: BlockProposal): Promise => { + return this.attestToProposal(block); }; this.p2pClient.registerBlockProposalHandler(handler); } - attestToProposal(proposal: BlockProposal) { + async attestToProposal(proposal: BlockProposal): Promise { + // Check that all of the tranasctions in the proposal are available in the tx pool before attesting + try { + await this.ensureTransactionsAreAvailable(proposal); + } catch (error: any) { + if (error instanceof TransactionsNotAvailableError) { + this.log.error(`Transactions not available, skipping attestation ${error.message}`); + } + return undefined; + } + this.log.debug(`Transactions available, attesting to proposal with ${proposal.txs.length} transactions`); + + // If the above function does not throw an error, then we can attest to the proposal return this.validationService.attestToProposal(proposal); } + /** + * Ensure that all of the transactions in the proposal are available in the tx pool before attesting + * + * 1. Check if the local tx pool contains all of the transactions in the proposal + * 2. If any transactions are not in the local tx pool, request them from the network + * 3. If we cannot retrieve them from the network, throw an error + * @param proposal - The proposal to attest to + */ + async ensureTransactionsAreAvailable(proposal: BlockProposal) { + const txHashes: TxHash[] = proposal.txs; + const transactionStatuses = await Promise.all(txHashes.map(txHash => this.p2pClient.getTxStatus(txHash))); + + const missingTxs = txHashes.filter((_, index) => !['pending', 'mined'].includes(transactionStatuses[index] ?? '')); + + if (missingTxs.length === 0) { + return; // All transactions are available + } + + this.log.verbose(`Missing ${missingTxs.length} transactions in the tx pool, requesting from the network`); + + const requestedTxs = await this.p2pClient.requestTxs(missingTxs); + if (requestedTxs.some(tx => tx === undefined)) { + this.log.error(`Failed to request transactions from the network: ${missingTxs.join(', ')}`); + throw new TransactionsNotAvailableError(missingTxs); + } + } + createBlockProposal(header: Header, archive: Fr, txs: TxHash[]): Promise { return this.validationService.createBlockProposal(header, archive, txs); } @@ -72,33 +120,37 @@ export class ValidatorClient implements Validator { this.p2pClient.broadcastProposal(proposal); } - // Target is temporarily hardcoded, for a test, but will be calculated from smart contract // TODO(https://github.com/AztecProtocol/aztec-packages/issues/7962) - // TODO(https://github.com/AztecProtocol/aztec-packages/issues/7976): require suitable timeouts async collectAttestations( proposal: BlockProposal, numberOfRequiredAttestations: number, ): Promise { - // Wait and poll the p2pClients attestation pool for this block - // until we have enough attestations - + // Wait and poll the p2pClient's attestation pool for this block until we have enough attestations const slot = proposal.header.globalVariables.slotNumber.toBigInt(); - this.log.info(`Waiting for ${numberOfRequiredAttestations} attestations for slot: ${slot}`); - const myAttestation = await this.attestToProposal(proposal); + const myAttestation = await this.validationService.attestToProposal(proposal); + + const startTime = Date.now(); - let attestations: BlockAttestation[] = []; - while (attestations.length < numberOfRequiredAttestations) { - attestations = [myAttestation, ...(await this.p2pClient.getAttestationsForSlot(slot))]; + while (true) { + const attestations = [myAttestation, ...(await this.p2pClient.getAttestationsForSlot(slot))]; - if (attestations.length < numberOfRequiredAttestations) { - this.log.verbose(`Waiting ${this.attestationPoolingIntervalMs}ms for more attestations...`); - await sleep(this.attestationPoolingIntervalMs); + if (attestations.length >= numberOfRequiredAttestations) { + this.log.info(`Collected all ${numberOfRequiredAttestations} attestations for slot, ${slot}`); + return attestations; + } + + const elapsedTime = Date.now() - startTime; + if (elapsedTime > this.attestationWaitTimeoutMs) { + this.log.error(`Timeout waiting for ${numberOfRequiredAttestations} attestations for slot, ${slot}`); + throw new AttestationTimeoutError(numberOfRequiredAttestations, slot); } - } - this.log.info(`Collected all attestations for slot, ${slot}`); - return attestations; + this.log.verbose( + `Collected ${attestations.length} attestations so far, waiting ${this.attestationPoolingIntervalMs}ms for more...`, + ); + await sleep(this.attestationPoolingIntervalMs); + } } } diff --git a/yarn-project/yarn.lock b/yarn-project/yarn.lock index 71ff91ca629..f1a9a1fb7f0 100644 --- a/yarn-project/yarn.lock +++ b/yarn-project/yarn.lock @@ -1223,6 +1223,7 @@ __metadata: "@types/jest": ^29.5.0 "@types/node": ^18.7.23 jest: ^29.5.0 + jest-mock-extended: ^3.0.7 koa: ^2.14.2 koa-router: ^12.0.0 ts-node: ^10.9.1 @@ -10806,7 +10807,7 @@ __metadata: languageName: node linkType: hard -"jest-mock-extended@npm:^3.0.3, jest-mock-extended@npm:^3.0.4, jest-mock-extended@npm:^3.0.5": +"jest-mock-extended@npm:^3.0.3, jest-mock-extended@npm:^3.0.4, jest-mock-extended@npm:^3.0.5, jest-mock-extended@npm:^3.0.7": version: 3.0.7 resolution: "jest-mock-extended@npm:3.0.7" dependencies: