diff --git a/yarn-project/archiver/src/archiver-store.test.ts b/yarn-project/archiver/src/archiver-store.test.ts index d5cd45f3a3fa..950b90008ea8 100644 --- a/yarn-project/archiver/src/archiver-store.test.ts +++ b/yarn-project/archiver/src/archiver-store.test.ts @@ -432,4 +432,116 @@ describe('Archiver Store', () => { expect(result).toEqual([]); }); }); + + describe('rollbackTo', () => { + beforeEach(() => { + publicClient.getBlock.mockImplementation( + (args: { blockNumber?: bigint } = {}) => + Promise.resolve({ number: args.blockNumber ?? 0n, hash: `0x${'0'.repeat(64)}` }) as any, + ); + }); + + it('rejects rollback to a block that is not at a checkpoint boundary', async () => { + const genesisArchive = new AppendOnlyTreeSnapshot(new Fr(GENESIS_ARCHIVE_ROOT), 1); + // Checkpoint 1: 3 blocks (1, 2, 3). Checkpoint 2: 3 blocks (4, 5, 6). + const testCheckpoints = await makeChainedCheckpoints(2, { + previousArchive: genesisArchive, + blocksPerCheckpoint: 3, + }); + await archiverStore.addCheckpoints(testCheckpoints); + + // Block 1 is not at a checkpoint boundary (checkpoint 1 ends at block 3) + await expect(archiver.rollbackTo(BlockNumber(1))).rejects.toThrow( + /not at a checkpoint boundary.*Use block 3 to roll back to this checkpoint.*or block 0 to roll back to the previous one/, + ); + + // Block 2 is also not at a checkpoint boundary + await expect(archiver.rollbackTo(BlockNumber(2))).rejects.toThrow( + /not at a checkpoint boundary.*Use block 3 to roll back to this checkpoint.*or block 0 to roll back to the previous one/, + ); + }); + + it('allows rollback to the last block of a checkpoint and updates sync points', async () => { + const genesisArchive = new AppendOnlyTreeSnapshot(new Fr(GENESIS_ARCHIVE_ROOT), 1); + // Checkpoint 1: 3 blocks (1, 2, 3), L1 block 10. Checkpoint 2: 3 blocks (4, 5, 6), L1 block 20. + const testCheckpoints = await makeChainedCheckpoints(2, { + previousArchive: genesisArchive, + blocksPerCheckpoint: 3, + }); + await archiverStore.addCheckpoints(testCheckpoints); + + // Block 3 is the last block of checkpoint 1 — should succeed + await archiver.rollbackTo(BlockNumber(3)); + + expect(await archiver.getSynchedCheckpointNumber()).toEqual(CheckpointNumber(1)); + + // Verify sync points are set to checkpoint 1's L1 block number (10) + const synchPoint = await archiverStore.getSynchPoint(); + expect(synchPoint.blocksSynchedTo).toEqual(10n); + expect(synchPoint.messagesSynchedTo?.l1BlockNumber).toEqual(10n); + }); + + it('includes correct boundary info in error for mid-checkpoint rollback', async () => { + const genesisArchive = new AppendOnlyTreeSnapshot(new Fr(GENESIS_ARCHIVE_ROOT), 1); + // Checkpoint 1: 2 blocks (1, 2). Checkpoint 2: 3 blocks (3, 4, 5). + const checkpoints1 = await makeChainedCheckpoints(1, { + previousArchive: genesisArchive, + blocksPerCheckpoint: 2, + }); + const checkpoints2 = await makeChainedCheckpoints(1, { + previousArchive: checkpoints1[0].checkpoint.blocks.at(-1)!.archive, + startCheckpointNumber: CheckpointNumber(2), + startBlockNumber: 3, + startL1BlockNumber: 20, + blocksPerCheckpoint: 3, + }); + await archiverStore.addCheckpoints([...checkpoints1, ...checkpoints2]); + + // Block 3 is the first of checkpoint 2 (spans 3-5) + // Should suggest block 5 (end of this checkpoint) or block 2 (end of previous) + await expect(archiver.rollbackTo(BlockNumber(3))).rejects.toThrow( + /Checkpoint 2 spans blocks 3 to 5.*Use block 5 to roll back to this checkpoint.*or block 2 to roll back to the previous one/, + ); + }); + + it('rolls back proven checkpoint number when target is before proven block', async () => { + const genesisArchive = new AppendOnlyTreeSnapshot(new Fr(GENESIS_ARCHIVE_ROOT), 1); + // Checkpoint 1: blocks 1-2, Checkpoint 2: blocks 3-4, Checkpoint 3: blocks 5-6 + const testCheckpoints = await makeChainedCheckpoints(3, { + previousArchive: genesisArchive, + blocksPerCheckpoint: 2, + }); + await archiverStore.addCheckpoints(testCheckpoints); + + // Mark checkpoint 2 as proven + await archiverStore.setProvenCheckpointNumber(CheckpointNumber(2)); + expect(await archiver.getProvenCheckpointNumber()).toEqual(CheckpointNumber(2)); + + // Roll back to block 2 (end of checkpoint 1), which is before proven block 4 + await archiver.rollbackTo(BlockNumber(2)); + + expect(await archiver.getSynchedCheckpointNumber()).toEqual(CheckpointNumber(1)); + expect(await archiver.getProvenCheckpointNumber()).toEqual(CheckpointNumber(1)); + }); + + it('preserves proven checkpoint number when target is after proven block', async () => { + const genesisArchive = new AppendOnlyTreeSnapshot(new Fr(GENESIS_ARCHIVE_ROOT), 1); + // Checkpoint 1: blocks 1-2, Checkpoint 2: blocks 3-4, Checkpoint 3: blocks 5-6 + const testCheckpoints = await makeChainedCheckpoints(3, { + previousArchive: genesisArchive, + blocksPerCheckpoint: 2, + }); + await archiverStore.addCheckpoints(testCheckpoints); + + // Mark checkpoint 1 as proven + await archiverStore.setProvenCheckpointNumber(CheckpointNumber(1)); + expect(await archiver.getProvenCheckpointNumber()).toEqual(CheckpointNumber(1)); + + // Roll back to block 4 (end of checkpoint 2), which is after proven block 2 + await archiver.rollbackTo(BlockNumber(4)); + + expect(await archiver.getSynchedCheckpointNumber()).toEqual(CheckpointNumber(2)); + expect(await archiver.getProvenCheckpointNumber()).toEqual(CheckpointNumber(1)); + }); + }); }); diff --git a/yarn-project/archiver/src/archiver.ts b/yarn-project/archiver/src/archiver.ts index 4aec6f3c9e69..de82a0482186 100644 --- a/yarn-project/archiver/src/archiver.ts +++ b/yarn-project/archiver/src/archiver.ts @@ -399,7 +399,6 @@ export class Archiver extends ArchiverDataSourceBase implements L2BlockSink, Tra } public async rollbackTo(targetL2BlockNumber: BlockNumber): Promise { - // TODO(pw/mbps): This still assumes 1 block per checkpoint const currentBlocks = await this.getL2Tips(); const currentL2Block = currentBlocks.proposed.number; const currentProvenBlock = currentBlocks.proven.block.number; @@ -411,8 +410,25 @@ export class Archiver extends ArchiverDataSourceBase implements L2BlockSink, Tra if (!targetL2Block) { throw new Error(`Target L2 block ${targetL2BlockNumber} not found`); } - const targetL1BlockNumber = targetL2Block.l1.blockNumber; const targetCheckpointNumber = targetL2Block.checkpointNumber; + + // Rollback operates at checkpoint granularity: the target block must be the last block of its checkpoint. + const checkpointData = await this.store.getCheckpointData(targetCheckpointNumber); + if (checkpointData) { + const lastBlockInCheckpoint = BlockNumber(checkpointData.startBlock + checkpointData.blockCount - 1); + if (targetL2BlockNumber !== lastBlockInCheckpoint) { + const previousCheckpointBoundary = + checkpointData.startBlock > 1 ? BlockNumber(checkpointData.startBlock - 1) : BlockNumber(0); + throw new Error( + `Target L2 block ${targetL2BlockNumber} is not at a checkpoint boundary. ` + + `Checkpoint ${targetCheckpointNumber} spans blocks ${checkpointData.startBlock} to ${lastBlockInCheckpoint}. ` + + `Use block ${lastBlockInCheckpoint} to roll back to this checkpoint, ` + + `or block ${previousCheckpointBoundary} to roll back to the previous one.`, + ); + } + } + + const targetL1BlockNumber = targetL2Block.l1.blockNumber; const targetL1Block = await this.publicClient.getBlock({ blockNumber: targetL1BlockNumber, includeTransactions: false, @@ -431,13 +447,14 @@ export class Archiver extends ArchiverDataSourceBase implements L2BlockSink, Tra await this.store.setCheckpointSynchedL1BlockNumber(targetL1BlockNumber); await this.store.setMessageSynchedL1Block({ l1BlockNumber: targetL1BlockNumber, l1BlockHash: targetL1BlockHash }); if (targetL2BlockNumber < currentProvenBlock) { - this.log.info(`Clearing proven L2 block number`); - await this.updater.setProvenCheckpointNumber(CheckpointNumber.ZERO); + this.log.info(`Rolling back proven L2 checkpoint to ${targetCheckpointNumber}`); + await this.updater.setProvenCheckpointNumber(targetCheckpointNumber); } // TODO(palla/reorg): Set the finalized block when we add support for it. + // const currentFinalizedBlock = currentBlocks.finalized.block.number; // if (targetL2BlockNumber < currentFinalizedBlock) { - // this.log.info(`Clearing finalized L2 block number`); - // await this.store.setFinalizedL2BlockNumber(0); + // this.log.info(`Rolling back finalized L2 checkpoint to ${targetCheckpointNumber}`); + // await this.updater.setFinalizedCheckpointNumber(targetCheckpointNumber); // } } } diff --git a/yarn-project/end-to-end/src/spartan/n_tps.test.ts b/yarn-project/end-to-end/src/spartan/n_tps.test.ts index 72a3d3002955..d7a774b5fc3b 100644 --- a/yarn-project/end-to-end/src/spartan/n_tps.test.ts +++ b/yarn-project/end-to-end/src/spartan/n_tps.test.ts @@ -90,10 +90,10 @@ const mempoolTxMinedDelayQuery = (perc: string) => const mempoolAttestationMinedDelayQuery = (perc: string) => `histogram_quantile(${perc}, sum(rate(aztec_mempool_attestations_mined_delay_milliseconds_bucket{k8s_namespace_name="${config.NAMESPACE}"}[1m])) by (le))`; -const peerCountQuery = () => `avg(aztec_peer_manager_peer_count{k8s_namespace_name="${config.NAMESPACE}"})`; +const peerCountQuery = () => `avg(aztec_peer_manager_peer_count_peers{k8s_namespace_name="${config.NAMESPACE}"})`; -const peerConnectionDurationQuery = (perc: string) => - `histogram_quantile(${perc}, sum(rate(aztec_peer_manager_peer_connection_duration_milliseconds_bucket{k8s_namespace_name="${config.NAMESPACE}"}[1m])) by (le))`; +const peerConnectionDurationQuery = (perc: string, windowSeconds: number) => + `histogram_quantile(${perc}, sum(rate(aztec_peer_manager_peer_connection_duration_milliseconds_bucket{k8s_namespace_name="${config.NAMESPACE}"}[${windowSeconds}s])) by (le))`; describe('sustained N TPS test', () => { jest.setTimeout(60 * 60 * 1000 * 10); // 10 hours @@ -168,8 +168,8 @@ describe('sustained N TPS test', () => { try { const [avgCount, durationP50, durationP95] = await Promise.all([ prometheusClient.querySingleValue(peerCountQuery()), - prometheusClient.querySingleValue(peerConnectionDurationQuery('0.50')), - prometheusClient.querySingleValue(peerConnectionDurationQuery('0.95')), + prometheusClient.querySingleValue(peerConnectionDurationQuery('0.50', TEST_DURATION_SECONDS + 60)), + prometheusClient.querySingleValue(peerConnectionDurationQuery('0.95', TEST_DURATION_SECONDS + 60)), ]); metrics.recordPeerStats(avgCount, durationP50, durationP95); logger.debug('Scraped peer stats', { avgCount, durationP50, durationP95 }); @@ -384,7 +384,7 @@ describe('sustained N TPS test', () => { const tx = await (config.REAL_VERIFIER ? submitProven(wallet, fee) : submitUnproven(wallet, fee)); const t1 = performance.now(); - metrics.recordSentTx(tx, `high_value_${highValueTps}tps`); + metrics.recordSentTx(tx, 'tx_inclusion_time'); const txHash = await tx.send({ wait: NO_WAIT }); const t2 = performance.now(); @@ -461,8 +461,8 @@ describe('sustained N TPS test', () => { logger.warn(`Failed transaction ${idx + 1}: ${result.error}`); }); - const highValueGroup = `high_value_${highValueTps}tps`; - const inclusionStats = metrics.inclusionTimeInSeconds(highValueGroup); + const txInclusionGroup = 'tx_inclusion_time'; + const inclusionStats = metrics.inclusionTimeInSeconds(txInclusionGroup); logger.info(`Transaction inclusion summary: ${successCount} succeeded, ${failureCount} failed`); logger.info('Inclusion time stats', inclusionStats); }); diff --git a/yarn-project/end-to-end/src/spartan/tx_metrics.ts b/yarn-project/end-to-end/src/spartan/tx_metrics.ts index 6234641a9276..20d536432ceb 100644 --- a/yarn-project/end-to-end/src/spartan/tx_metrics.ts +++ b/yarn-project/end-to-end/src/spartan/tx_metrics.ts @@ -296,7 +296,7 @@ export class TxInclusionMetrics { value: stats.mean, }, { - name: `${group}/median_inclusion`, + name: `${group}/p50_inclusion`, unit: 's', value: stats.median, }, diff --git a/yarn-project/ethereum/src/l1_tx_utils/l1_tx_utils.test.ts b/yarn-project/ethereum/src/l1_tx_utils/l1_tx_utils.test.ts index 828a351bf5fb..593dd1831a3d 100644 --- a/yarn-project/ethereum/src/l1_tx_utils/l1_tx_utils.test.ts +++ b/yarn-project/ethereum/src/l1_tx_utils/l1_tx_utils.test.ts @@ -147,7 +147,7 @@ describe('L1TxUtils', () => { address: l1Client.account.address, }); - // Next send fails at sendRawTransaction (e.g. network error) + // Next send fails at sendRawTransaction (e.g. network error / 429) const originalSendRawTransaction = l1Client.sendRawTransaction.bind(l1Client); using _sendSpy = jest .spyOn(l1Client, 'sendRawTransaction') @@ -163,6 +163,29 @@ describe('L1TxUtils', () => { expect((await l1Client.getTransaction({ hash: txHash })).nonce).toBe(expectedNonce); }, 30_000); + it('bumps nonce when getTransactionCount returns a stale value after a successful send', async () => { + // Send a successful tx first to advance the chain nonce + await gasUtils.sendAndMonitorTransaction(request); + + const expectedNonce = await l1Client.getTransactionCount({ + blockTag: 'pending', + address: l1Client.account.address, + }); + + // Simulate a stale fallback RPC node that returns the pre-send nonce + const originalGetTransactionCount = l1Client.getTransactionCount.bind(l1Client); + using _spy = jest + .spyOn(l1Client, 'getTransactionCount') + .mockImplementationOnce(() => Promise.resolve(expectedNonce - 1)) // stale: one behind + .mockImplementation(originalGetTransactionCount); + + // Despite the stale count, the send should use lastSentNonce+1 = expectedNonce + const { txHash, state } = await gasUtils.sendTransaction(request); + + expect(state.nonce).toBe(expectedNonce); + expect((await l1Client.getTransaction({ hash: txHash })).nonce).toBe(expectedNonce); + }, 30_000); + // Regression for TMNT-312 it('speed-up of blob tx sets non-zero maxFeePerBlobGas', async () => { await cheatCodes.setAutomine(false); diff --git a/yarn-project/ethereum/src/l1_tx_utils/l1_tx_utils.ts b/yarn-project/ethereum/src/l1_tx_utils/l1_tx_utils.ts index 48b8dfc41aa5..f6292311fc7e 100644 --- a/yarn-project/ethereum/src/l1_tx_utils/l1_tx_utils.ts +++ b/yarn-project/ethereum/src/l1_tx_utils/l1_tx_utils.ts @@ -45,6 +45,8 @@ const MAX_L1_TX_STATES = 32; export class L1TxUtils extends ReadOnlyL1TxUtils { protected txs: L1TxState[] = []; + /** Last nonce successfully sent to the chain. Used as a lower bound when a fallback RPC node returns a stale count. */ + private lastSentNonce: number | undefined; /** Tx delayer for testing. Only set when enableDelayer config is true. */ public delayer?: Delayer; /** KZG instance for blob operations. */ @@ -105,6 +107,11 @@ export class L1TxUtils extends ReadOnlyL1TxUtils { this.metrics?.recordMinedTx(l1TxState, new Date(l1Timestamp)); } else if (newState === TxUtilsState.NOT_MINED) { this.metrics?.recordDroppedTx(l1TxState); + // The tx was dropped: the chain nonce reverted to l1TxState.nonce, so our lower bound is + // no longer valid. Clear it so the next send fetches the real nonce from the chain. + if (this.lastSentNonce === l1TxState.nonce) { + this.lastSentNonce = undefined; + } } // Update state in the store @@ -246,7 +253,11 @@ export class L1TxUtils extends ReadOnlyL1TxUtils { ); } - const nonce = await this.client.getTransactionCount({ address: account, blockTag: 'pending' }); + const chainNonce = await this.client.getTransactionCount({ address: account, blockTag: 'pending' }); + // If a fallback RPC node returns a stale count (lower than what we last sent), use our + // local lower bound to avoid sending a duplicate of an already-pending transaction. + const nonce = + this.lastSentNonce !== undefined && chainNonce <= this.lastSentNonce ? this.lastSentNonce + 1 : chainNonce; const baseState = { request, gasLimit, blobInputs, gasPrice, nonce }; const txData = this.makeTxData(baseState, { isCancelTx: false }); @@ -254,6 +265,8 @@ export class L1TxUtils extends ReadOnlyL1TxUtils { // Send the new tx const signedRequest = await this.prepareSignedTransaction(txData); const txHash = await this.client.sendRawTransaction({ serializedTransaction: signedRequest }); + // Update after tx is sent successfully + this.lastSentNonce = nonce; // Create the new state for monitoring const l1TxState: L1TxState = { diff --git a/yarn-project/foundation/src/config/config.test.ts b/yarn-project/foundation/src/config/config.test.ts index c21021c8d067..a4e0f7ad17b2 100644 --- a/yarn-project/foundation/src/config/config.test.ts +++ b/yarn-project/foundation/src/config/config.test.ts @@ -1,6 +1,6 @@ import { jest } from '@jest/globals'; -import { type ConfigMappingsType, getConfigFromMappings, numberConfigHelper } from './index.js'; +import { type ConfigMappingsType, bigintConfigHelper, getConfigFromMappings, numberConfigHelper } from './index.js'; describe('Config', () => { describe('getConfigFromMappings', () => { @@ -131,4 +131,37 @@ describe('Config', () => { }); }); }); + + describe('bigintConfigHelper', () => { + it('parses plain integer strings', () => { + const { parseEnv } = bigintConfigHelper(); + expect(parseEnv!('123')).toBe(123n); + expect(parseEnv!('0')).toBe(0n); + expect(parseEnv!('200000000000000000000000')).toBe(200000000000000000000000n); + }); + + it('parses scientific notation', () => { + const { parseEnv } = bigintConfigHelper(); + expect(parseEnv!('1e+23')).toBe(100000000000000000000000n); + expect(parseEnv!('2E+23')).toBe(200000000000000000000000n); + expect(parseEnv!('1e23')).toBe(100000000000000000000000n); + expect(parseEnv!('5e18')).toBe(5000000000000000000n); + }); + + it('parses scientific notation with decimal mantissa', () => { + const { parseEnv } = bigintConfigHelper(); + expect(parseEnv!('1.5e10')).toBe(15000000000n); + expect(parseEnv!('2.5e5')).toBe(250000n); + }); + + it('returns default value for empty string', () => { + const { parseEnv } = bigintConfigHelper(42n); + expect(parseEnv!('')).toBe(42n); + }); + + it('throws for non-integer scientific notation results', () => { + const { parseEnv } = bigintConfigHelper(); + expect(() => parseEnv!('1e-3')).toThrow(); + }); + }); }); diff --git a/yarn-project/foundation/src/config/index.ts b/yarn-project/foundation/src/config/index.ts index 7ed7d3ef6ceb..8962dea3b1b0 100644 --- a/yarn-project/foundation/src/config/index.ts +++ b/yarn-project/foundation/src/config/index.ts @@ -177,6 +177,21 @@ export function bigintConfigHelper(defaultVal?: bigint): Pick { + Pick { /** A flag dictating whether the P2P subsystem should be enabled. */ p2pEnabled: boolean; diff --git a/yarn-project/p2p/src/mem_pools/attestation_pool/attestation_pool.ts b/yarn-project/p2p/src/mem_pools/attestation_pool/attestation_pool.ts index 827d91ce84ea..e61456ef5cbf 100644 --- a/yarn-project/p2p/src/mem_pools/attestation_pool/attestation_pool.ts +++ b/yarn-project/p2p/src/mem_pools/attestation_pool/attestation_pool.ts @@ -359,11 +359,10 @@ export class AttestationPool { } const address = sender.toString(); + const ownKey = this.getAttestationKey(slotNumber, proposalId, address); - await this.checkpointAttestations.set( - this.getAttestationKey(slotNumber, proposalId, address), - attestation.toBuffer(), - ); + await this.checkpointAttestations.set(ownKey, attestation.toBuffer()); + this.metrics.trackMempoolItemAdded(ownKey); this.log.debug(`Added own checkpoint attestation for slot ${slotNumber} from ${address}`, { signature: attestation.signature.toString(), @@ -429,6 +428,7 @@ export class AttestationPool { const attestationEndKey = new Fr(oldestSlot).toString(); for await (const key of this.checkpointAttestations.keysAsync({ end: attestationEndKey })) { await this.checkpointAttestations.delete(key); + this.metrics.trackMempoolItemRemoved(key); numberOfAttestations++; } @@ -526,6 +526,7 @@ export class AttestationPool { // Add the attestation await this.checkpointAttestations.set(key, attestation.toBuffer()); + this.metrics.trackMempoolItemAdded(key); // Track this attestation in the per-signer-per-slot index for duplicate detection const slotSignerKey = this.getSlotSignerKey(slotNumber, signerAddress); diff --git a/yarn-project/p2p/src/mem_pools/instrumentation.ts b/yarn-project/p2p/src/mem_pools/instrumentation.ts index bfd3c7b64ac7..d76d2c30ad4a 100644 --- a/yarn-project/p2p/src/mem_pools/instrumentation.ts +++ b/yarn-project/p2p/src/mem_pools/instrumentation.ts @@ -73,7 +73,7 @@ export class PoolInstrumentation { private defaultAttributes; private meter: Meter; - private txAddedTimestamp: Map = new Map(); + private mempoolItemAddedTimestamp: Map = new Map(); constructor( telemetry: TelemetryClient, @@ -114,22 +114,26 @@ export class PoolInstrumentation { } public transactionsAdded(transactions: Tx[]) { - const timestamp = Date.now(); - for (const transaction of transactions) { - this.txAddedTimestamp.set(transaction.txHash.toBigInt(), timestamp); - } + transactions.forEach(tx => this.trackMempoolItemAdded(tx.txHash.toBigInt())); } public transactionsRemoved(hashes: Iterable | Iterable) { - const timestamp = Date.now(); for (const hash of hashes) { - const key = BigInt(hash); - const addedAt = this.txAddedTimestamp.get(key); - if (addedAt !== undefined) { - this.txAddedTimestamp.delete(key); - if (addedAt < timestamp) { - this.minedDelay.record(timestamp - addedAt); - } + this.trackMempoolItemRemoved(BigInt(hash)); + } + } + + public trackMempoolItemAdded(key: bigint | string): void { + this.mempoolItemAddedTimestamp.set(key, Date.now()); + } + + public trackMempoolItemRemoved(key: bigint | string): void { + const timestamp = Date.now(); + const addedAt = this.mempoolItemAddedTimestamp.get(key); + if (addedAt !== undefined) { + this.mempoolItemAddedTimestamp.delete(key); + if (addedAt < timestamp) { + this.minedDelay.record(timestamp - addedAt); } } } diff --git a/yarn-project/p2p/src/mem_pools/tx_pool_v2/tx_pool_v2.ts b/yarn-project/p2p/src/mem_pools/tx_pool_v2/tx_pool_v2.ts index 4f65a0d6b0ae..69b91bd95c52 100644 --- a/yarn-project/p2p/src/mem_pools/tx_pool_v2/tx_pool_v2.ts +++ b/yarn-project/p2p/src/mem_pools/tx_pool_v2/tx_pool_v2.ts @@ -58,6 +58,9 @@ export class AztecKVTxPoolV2 extends (EventEmitter as new () => TypedEventEmitte const hashes = txHashes.map(h => (typeof h === 'string' ? TxHash.fromString(h) : TxHash.fromBigInt(h))); this.emit('txs-removed', { txHashes: hashes }); }, + onTxsMined: (txHashes: string[]) => { + this.#metrics?.transactionsRemoved(txHashes); + }, }; // Create the implementation diff --git a/yarn-project/p2p/src/mem_pools/tx_pool_v2/tx_pool_v2_impl.ts b/yarn-project/p2p/src/mem_pools/tx_pool_v2/tx_pool_v2_impl.ts index 88f6e887b9a8..15f6eb4b051e 100644 --- a/yarn-project/p2p/src/mem_pools/tx_pool_v2/tx_pool_v2_impl.ts +++ b/yarn-project/p2p/src/mem_pools/tx_pool_v2/tx_pool_v2_impl.ts @@ -45,6 +45,7 @@ import { TxPoolIndices } from './tx_pool_indices.js'; export interface TxPoolV2Callbacks { onTxsAdded: (txs: Tx[], opts: { source?: string }) => void; onTxsRemoved: (txHashes: string[] | bigint[]) => void; + onTxsMined: (txHashes: string[]) => void; } /** @@ -498,6 +499,10 @@ export class TxPoolV2Impl { await this.#evictionManager.evictAfterNewBlock(block.header, nullifiers, feePayers); }); + if (found.length > 0) { + this.#callbacks.onTxsMined(found.map(m => m.txHash)); + } + this.#log.info(`Marked ${found.length} txs as mined in block ${blockId.number}`); } diff --git a/yarn-project/p2p/src/msg_validators/proposal_validator/block_proposal_validator.ts b/yarn-project/p2p/src/msg_validators/proposal_validator/block_proposal_validator.ts index 5de0076aea39..a481256e9f37 100644 --- a/yarn-project/p2p/src/msg_validators/proposal_validator/block_proposal_validator.ts +++ b/yarn-project/p2p/src/msg_validators/proposal_validator/block_proposal_validator.ts @@ -4,7 +4,7 @@ import type { BlockProposal, P2PValidator } from '@aztec/stdlib/p2p'; import { ProposalValidator } from '../proposal_validator/proposal_validator.js'; export class BlockProposalValidator extends ProposalValidator implements P2PValidator { - constructor(epochCache: EpochCacheInterface, opts: { txsPermitted: boolean }) { + constructor(epochCache: EpochCacheInterface, opts: { txsPermitted: boolean; maxTxsPerBlock?: number }) { super(epochCache, opts, 'p2p:block_proposal_validator'); } } diff --git a/yarn-project/p2p/src/msg_validators/proposal_validator/checkpoint_proposal_validator.ts b/yarn-project/p2p/src/msg_validators/proposal_validator/checkpoint_proposal_validator.ts index 763912a04814..74804fe45d21 100644 --- a/yarn-project/p2p/src/msg_validators/proposal_validator/checkpoint_proposal_validator.ts +++ b/yarn-project/p2p/src/msg_validators/proposal_validator/checkpoint_proposal_validator.ts @@ -7,7 +7,7 @@ export class CheckpointProposalValidator extends ProposalValidator implements P2PValidator { - constructor(epochCache: EpochCacheInterface, opts: { txsPermitted: boolean }) { + constructor(epochCache: EpochCacheInterface, opts: { txsPermitted: boolean; maxTxsPerBlock?: number }) { super(epochCache, opts, 'p2p:checkpoint_proposal_validator'); } } diff --git a/yarn-project/p2p/src/msg_validators/proposal_validator/proposal_validator.ts b/yarn-project/p2p/src/msg_validators/proposal_validator/proposal_validator.ts index f6fb7102e537..a926d1f3c144 100644 --- a/yarn-project/p2p/src/msg_validators/proposal_validator/proposal_validator.ts +++ b/yarn-project/p2p/src/msg_validators/proposal_validator/proposal_validator.ts @@ -9,10 +9,16 @@ export abstract class ProposalValidator this.maxTxsPerBlock) { + this.logger.warn( + `Penalizing peer for proposal with ${proposal.txHashes.length} transaction(s) when max is ${this.maxTxsPerBlock}`, + ); + return { result: 'reject', severity: PeerErrorSeverity.MidToleranceError }; + } + // Embedded txs must be listed in txHashes const hashSet = new Set(proposal.txHashes.map(h => h.toString())); const missingTxHashes = diff --git a/yarn-project/p2p/src/msg_validators/proposal_validator/proposal_validator_test_suite.ts b/yarn-project/p2p/src/msg_validators/proposal_validator/proposal_validator_test_suite.ts index 6aa79230f8cd..e58a007a3de7 100644 --- a/yarn-project/p2p/src/msg_validators/proposal_validator/proposal_validator_test_suite.ts +++ b/yarn-project/p2p/src/msg_validators/proposal_validator/proposal_validator_test_suite.ts @@ -1,4 +1,5 @@ import type { EpochCacheInterface } from '@aztec/epoch-cache'; +import { NoCommitteeError } from '@aztec/ethereum/contracts'; import type { Secp256k1Signer } from '@aztec/foundation/crypto/secp256k1-signer'; import type { EthAddress } from '@aztec/foundation/eth-address'; import { @@ -9,12 +10,13 @@ import { } from '@aztec/stdlib/p2p'; import type { TxHash } from '@aztec/stdlib/tx'; +import { jest } from '@jest/globals'; import type { MockProxy } from 'jest-mock-extended'; export interface ProposalValidatorTestParams { validatorFactory: ( epochCache: EpochCacheInterface, - opts: { txsPermitted: boolean }, + opts: { txsPermitted: boolean; maxTxsPerBlock?: number }, ) => { validate: (proposal: TProposal) => Promise }; makeProposal: (options?: any) => Promise; makeHeader: (epochNumber: number | bigint, slotNumber: number | bigint, blockNumber: number | bigint) => any; @@ -105,6 +107,26 @@ export function sharedProposalValidatorTests { + const currentProposer = getSigner(); + const header = makeHeader(1, 100, 100); + const mockProposal = await makeProposal({ + blockHeader: header, + lastBlockHeader: header, + signer: currentProposer, + }); + + // Override getSender to return undefined (invalid signature) + jest.spyOn(mockProposal as any, 'getSender').mockReturnValue(undefined); + + mockGetProposer(getAddress(currentProposer), getAddress()); + const result = await validator.validate(mockProposal); + expect(result).toEqual({ result: 'reject', severity: PeerErrorSeverity.MidToleranceError }); + + // Should not try to resolve proposer if signature is invalid + expect(epochCache.getProposerAttesterAddressInSlot).not.toHaveBeenCalled(); + }); + it('returns mid tolerance error if proposer is not current proposer for current slot', async () => { const currentProposer = getSigner(); const nextProposer = getSigner(); @@ -152,6 +174,34 @@ export function sharedProposalValidatorTests { + const currentProposer = getSigner(); + const header = makeHeader(1, 100, 100); + const mockProposal = await makeProposal({ + blockHeader: header, + lastBlockHeader: header, + signer: currentProposer, + }); + + epochCache.getProposerAttesterAddressInSlot.mockResolvedValue(undefined); + const result = await validator.validate(mockProposal); + expect(result).toEqual({ result: 'accept' }); + }); + + it('returns low tolerance error when getProposerAttesterAddressInSlot throws NoCommitteeError', async () => { + const currentProposer = getSigner(); + const header = makeHeader(1, 100, 100); + const mockProposal = await makeProposal({ + blockHeader: header, + lastBlockHeader: header, + signer: currentProposer, + }); + + epochCache.getProposerAttesterAddressInSlot.mockRejectedValue(new NoCommitteeError()); + const result = await validator.validate(mockProposal); + expect(result).toEqual({ result: 'reject', severity: PeerErrorSeverity.LowToleranceError }); + }); + it('returns undefined if proposal is valid for current slot and proposer', async () => { const currentProposer = getSigner(); const nextProposer = getSigner(); @@ -226,5 +276,98 @@ export function sharedProposalValidatorTests { + it('returns mid tolerance error if embedded txs are not listed in txHashes', async () => { + const currentProposer = getSigner(); + const txHashes = getTxHashes(2); + const header = makeHeader(1, 100, 100); + const mockProposal = await makeProposal({ + blockHeader: header, + lastBlockHeader: header, + signer: currentProposer, + txHashes, + }); + + // Create a fake tx whose hash is NOT in txHashes + const fakeTxHash = getTxHashes(1)[0]; + const fakeTx = { getTxHash: () => fakeTxHash, validateTxHash: () => Promise.resolve(true) }; + Object.defineProperty(mockProposal, 'txs', { get: () => [fakeTx], configurable: true }); + + mockGetProposer(getAddress(currentProposer), getAddress()); + const result = await validator.validate(mockProposal); + expect(result).toEqual({ result: 'reject', severity: PeerErrorSeverity.MidToleranceError }); + }); + + it('returns low tolerance error if embedded tx has invalid tx hash', async () => { + const currentProposer = getSigner(); + const txHashes = getTxHashes(2); + const header = makeHeader(1, 100, 100); + const mockProposal = await makeProposal({ + blockHeader: header, + lastBlockHeader: header, + signer: currentProposer, + txHashes, + }); + + // Create a fake tx whose hash IS in txHashes but validateTxHash returns false + const fakeTx = { getTxHash: () => txHashes[0], validateTxHash: () => Promise.resolve(false) }; + Object.defineProperty(mockProposal, 'txs', { get: () => [fakeTx], configurable: true }); + + mockGetProposer(getAddress(currentProposer), getAddress()); + const result = await validator.validate(mockProposal); + expect(result).toEqual({ result: 'reject', severity: PeerErrorSeverity.LowToleranceError }); + }); + }); + + describe('maxTxsPerBlock validation', () => { + it('rejects proposal when txHashes exceed maxTxsPerBlock', async () => { + const validatorWithMaxTxs = validatorFactory(epochCache, { txsPermitted: true, maxTxsPerBlock: 2 }); + const currentProposer = getSigner(); + const header = makeHeader(1, 100, 100); + const mockProposal = await makeProposal({ + blockHeader: header, + lastBlockHeader: header, + signer: currentProposer, + txHashes: getTxHashes(3), + }); + + mockGetProposer(getAddress(currentProposer), getAddress()); + const result = await validatorWithMaxTxs.validate(mockProposal); + expect(result).toEqual({ result: 'reject', severity: PeerErrorSeverity.MidToleranceError }); + }); + + it('accepts proposal when txHashes count equals maxTxsPerBlock', async () => { + const validatorWithMaxTxs = validatorFactory(epochCache, { txsPermitted: true, maxTxsPerBlock: 2 }); + const currentProposer = getSigner(); + const header = makeHeader(1, 100, 100); + const mockProposal = await makeProposal({ + blockHeader: header, + lastBlockHeader: header, + signer: currentProposer, + txHashes: getTxHashes(2), + }); + + mockGetProposer(getAddress(currentProposer), getAddress()); + const result = await validatorWithMaxTxs.validate(mockProposal); + expect(result).toEqual({ result: 'accept' }); + }); + + it('accepts proposal when maxTxsPerBlock is not set (unlimited)', async () => { + // Default validator has no maxTxsPerBlock + const currentProposer = getSigner(); + const header = makeHeader(1, 100, 100); + const mockProposal = await makeProposal({ + blockHeader: header, + lastBlockHeader: header, + signer: currentProposer, + txHashes: getTxHashes(10), + }); + + mockGetProposer(getAddress(currentProposer), getAddress()); + const result = await validator.validate(mockProposal); + expect(result).toEqual({ result: 'accept' }); + }); + }); }); } diff --git a/yarn-project/p2p/src/services/libp2p/libp2p_service.ts b/yarn-project/p2p/src/services/libp2p/libp2p_service.ts index 0122f322ebc0..8da82a7d195b 100644 --- a/yarn-project/p2p/src/services/libp2p/libp2p_service.ts +++ b/yarn-project/p2p/src/services/libp2p/libp2p_service.ts @@ -222,9 +222,13 @@ export class LibP2PService extends WithTracer implements P2PService { this.protocolVersion, ); - this.blockProposalValidator = new BlockProposalValidator(epochCache, { txsPermitted: !config.disableTransactions }); + this.blockProposalValidator = new BlockProposalValidator(epochCache, { + txsPermitted: !config.disableTransactions, + maxTxsPerBlock: config.maxTxsPerBlock, + }); this.checkpointProposalValidator = new CheckpointProposalValidator(epochCache, { txsPermitted: !config.disableTransactions, + maxTxsPerBlock: config.maxTxsPerBlock, }); this.checkpointAttestationValidator = config.fishermanMode ? new FishermanAttestationValidator(epochCache, mempools.attestationPool, telemetry) diff --git a/yarn-project/p2p/src/services/peer-manager/metrics.ts b/yarn-project/p2p/src/services/peer-manager/metrics.ts index 06cd513727db..2e1f198611a4 100644 --- a/yarn-project/p2p/src/services/peer-manager/metrics.ts +++ b/yarn-project/p2p/src/services/peer-manager/metrics.ts @@ -18,6 +18,7 @@ export class PeerManagerMetrics { private sentGoodbyes: UpDownCounter; private receivedGoodbyes: UpDownCounter; private peerCount: Gauge; + private healthyPeerCount: Gauge; private lowScoreDisconnects: UpDownCounter; private peerConnectionDuration: Histogram; @@ -49,6 +50,7 @@ export class PeerManagerMetrics { goodbyeReasonAttrs, ); this.peerCount = meter.createGauge(Metrics.PEER_MANAGER_PEER_COUNT); + this.healthyPeerCount = meter.createGauge(Metrics.PEER_MANAGER_HEALTHY_PEER_COUNT); this.lowScoreDisconnects = createUpDownCounterWithDefault(meter, Metrics.PEER_MANAGER_LOW_SCORE_DISCONNECTS, { [Attributes.P2P_PEER_SCORE_STATE]: ['Banned', 'Disconnect'], }); @@ -67,6 +69,10 @@ export class PeerManagerMetrics { this.peerCount.record(count); } + public recordHealthyPeerCount(count: number) { + this.healthyPeerCount.record(count); + } + public recordLowScoreDisconnect(scoreState: 'Banned' | 'Disconnect') { this.lowScoreDisconnects.add(1, { [Attributes.P2P_PEER_SCORE_STATE]: scoreState }); } @@ -79,6 +85,7 @@ export class PeerManagerMetrics { const connectedAt = this.peerConnectedAt.get(id.toString()); if (connectedAt) { this.peerConnectionDuration.record(Date.now() - connectedAt); + this.peerConnectedAt.delete(id.toString()); } } } diff --git a/yarn-project/p2p/src/services/peer-manager/peer_manager.ts b/yarn-project/p2p/src/services/peer-manager/peer_manager.ts index 336f726b6dec..669f0e149a9c 100644 --- a/yarn-project/p2p/src/services/peer-manager/peer_manager.ts +++ b/yarn-project/p2p/src/services/peer-manager/peer_manager.ts @@ -515,7 +515,8 @@ export class PeerManager implements PeerManagerInterface { ...this.peerScoring.getStats(), }); - this.metrics.recordPeerCount(healthyConnections.length); + this.metrics.recordPeerCount(connections.length); + this.metrics.recordHealthyPeerCount(healthyConnections.length); // Exit if no peers to connect if (peersToConnect <= 0) { diff --git a/yarn-project/sequencer-client/src/config.ts b/yarn-project/sequencer-client/src/config.ts index 469651fba387..61dcb2344c17 100644 --- a/yarn-project/sequencer-client/src/config.ts +++ b/yarn-project/sequencer-client/src/config.ts @@ -13,6 +13,7 @@ import { type P2PConfig, p2pConfigMappings } from '@aztec/p2p/config'; import { AztecAddress } from '@aztec/stdlib/aztec-address'; import { type ChainConfig, + DEFAULT_MAX_TXS_PER_BLOCK, type SequencerConfig, chainConfigMappings, sharedSequencerConfigMappings, @@ -37,7 +38,7 @@ export type { SequencerConfig }; */ export const DefaultSequencerConfig: ResolvedSequencerConfig = { sequencerPollingIntervalMS: 500, - maxTxsPerBlock: 32, + maxTxsPerBlock: DEFAULT_MAX_TXS_PER_BLOCK, minTxsPerBlock: 1, buildCheckpointIfEmpty: false, publishTxsWithProposals: false, @@ -77,11 +78,6 @@ export const sequencerConfigMappings: ConfigMappingsType = { description: 'The number of ms to wait between polling for checking to build on the next slot.', ...numberConfigHelper(DefaultSequencerConfig.sequencerPollingIntervalMS), }, - maxTxsPerBlock: { - env: 'SEQ_MAX_TX_PER_BLOCK', - description: 'The maximum number of txs to include in a block.', - ...numberConfigHelper(DefaultSequencerConfig.maxTxsPerBlock), - }, minTxsPerBlock: { env: 'SEQ_MIN_TX_PER_BLOCK', description: 'The minimum number of txs to include in a block.', diff --git a/yarn-project/stdlib/src/config/sequencer-config.ts b/yarn-project/stdlib/src/config/sequencer-config.ts index 7619cdce7e68..31d0eca9458a 100644 --- a/yarn-project/stdlib/src/config/sequencer-config.ts +++ b/yarn-project/stdlib/src/config/sequencer-config.ts @@ -1,7 +1,10 @@ -import type { ConfigMappingsType } from '@aztec/foundation/config'; +import { type ConfigMappingsType, numberConfigHelper } from '@aztec/foundation/config'; import type { SequencerConfig } from '../interfaces/configs.js'; +/** Default maximum number of transactions per block. */ +export const DEFAULT_MAX_TXS_PER_BLOCK = 32; + /** * Partial sequencer config mappings for fields that need to be shared across packages. * The full sequencer config mappings remain in sequencer-client, but shared fields @@ -9,7 +12,7 @@ import type { SequencerConfig } from '../interfaces/configs.js'; * to avoid duplication. */ export const sharedSequencerConfigMappings: ConfigMappingsType< - Pick + Pick > = { blockDurationMs: { env: 'SEQ_BLOCK_DURATION_MS', @@ -26,4 +29,9 @@ export const sharedSequencerConfigMappings: ConfigMappingsType< parseEnv: (val: string) => (val ? parseInt(val, 10) : 0), defaultValue: 0, }, + maxTxsPerBlock: { + env: 'SEQ_MAX_TX_PER_BLOCK', + description: 'The maximum number of txs to include in a block.', + ...numberConfigHelper(DEFAULT_MAX_TXS_PER_BLOCK), + }, }; diff --git a/yarn-project/stdlib/src/interfaces/validator.ts b/yarn-project/stdlib/src/interfaces/validator.ts index 09851cfbb98c..608d08520758 100644 --- a/yarn-project/stdlib/src/interfaces/validator.ts +++ b/yarn-project/stdlib/src/interfaces/validator.ts @@ -62,7 +62,7 @@ export type ValidatorClientConfig = ValidatorHASignerConfig & { }; export type ValidatorClientFullConfig = ValidatorClientConfig & - Pick & + Pick & Pick< SlasherConfig, 'slashBroadcastedInvalidBlockPenalty' | 'slashDuplicateProposalPenalty' | 'slashDuplicateAttestationPenalty' @@ -93,6 +93,7 @@ export const ValidatorClientFullConfigSchema = zodFor WatcherEmitter) const metrics = new ValidatorMetrics(telemetry); const blockProposalValidator = new BlockProposalValidator(epochCache, { txsPermitted: !config.disableTransactions, + maxTxsPerBlock: config.maxTxsPerBlock, }); const blockProposalHandler = new BlockProposalHandler( checkpointsBuilder,