From 7237519e7c4c5d53ea316c6301f7f5046c5e3ddb Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Mon, 10 Aug 2026 11:40:05 -0300 Subject: [PATCH 1/3] chore: add tmp folder in yarn-project (#25130) Forward port of #23932 --- yarn-project/.gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/yarn-project/.gitignore b/yarn-project/.gitignore index c1a51324410..bb1e117fb9b 100644 --- a/yarn-project/.gitignore +++ b/yarn-project/.gitignore @@ -61,6 +61,7 @@ cli-wallet/test/data cli/src/config/generated ethereum/src/generated slasher/src/generated +tmp/ .claude/settings.local.json .yarn/* From b30cd3793a71d64a3fe954fed8d5df546db34b8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Bene=C5=A1?= Date: Mon, 10 Aug 2026 17:44:26 +0200 Subject: [PATCH 2/3] refactor: rename `GasPrice` to `FeesPerGas` in l1 tx utils (#25152) The original name was very misleading as gas price has a real meaning - that's the gas price paid for when the L1 tx is mined. For this reason I am renaming `GasPrice` as `FeesPerGas`, following viem's naming for the same EIP-1559 values. Old naming is handled by the stores so this should be fine to merge. --- .../docs/resources/migration_notes.md | 77 ++++++++---- .../ethereum/src/deploy_l1_contract.ts | 6 +- .../ethereum/src/l1_tx_utils/README.md | 4 +- ...er.test.ts => fees_per_gas_ladder.test.ts} | 16 +-- .../l1_tx_utils/l1_tx_timeout_error.test.ts | 6 +- .../src/l1_tx_utils/l1_tx_utils.test.ts | 114 +++++++++--------- .../ethereum/src/l1_tx_utils/l1_tx_utils.ts | 84 ++++++------- .../src/l1_tx_utils/readonly_l1_tx_utils.ts | 22 ++-- .../ethereum/src/l1_tx_utils/types.ts | 18 +-- .../node-lib/src/metrics/l1_tx_metrics.ts | 10 +- .../node-lib/src/stores/l1_tx_store.test.ts | 26 +++- .../node-lib/src/stores/l1_tx_store.ts | 43 ++++--- .../src/prover-node-publisher.test.ts | 6 +- .../prover-node/src/prover-node-publisher.ts | 6 +- .../l1_tx_failed_store/failed_tx_store.ts | 25 ++-- .../src/publisher/sequencer-publisher.test.ts | 12 +- .../src/publisher/sequencer-publisher.ts | 4 +- 17 files changed, 274 insertions(+), 205 deletions(-) rename yarn-project/ethereum/src/l1_tx_utils/{gas_price_ladder.test.ts => fees_per_gas_ladder.test.ts} (87%) diff --git a/docs/docs-developers/docs/resources/migration_notes.md b/docs/docs-developers/docs/resources/migration_notes.md index 2fac2e8b8ce..8078dd7e139 100644 --- a/docs/docs-developers/docs/resources/migration_notes.md +++ b/docs/docs-developers/docs/resources/migration_notes.md @@ -58,8 +58,8 @@ Nine `DOM_SEP__*` domain separators that used to live in the protocol constants Two remain public, at a new path: -| Constant | Old path | New path | -| --- | --- | --- | +| Constant | Old path | New path | +| ---------------------------------- | ---------------------------- | --------------------------- | | `DOM_SEP__PARTIAL_NOTE_COMMITMENT` | `aztec::protocol::constants` | `aztec::note::partial_note` | | `DOM_SEP__NOTE_COMPLETION_LOG_TAG` | `aztec::protocol::constants` | `aztec::note::partial_note` | @@ -72,6 +72,39 @@ The other seven are now crate-internal (`pub(crate)`) and can no longer be impor **Impact**: Contracts that use aztec-nr's high-level APIs (notes, authwit, state variables, message delivery, ECDH) are unaffected, since these separators are applied internally. A contract that imported one of these constants directly must either switch to the new `aztec::note::partial_note` path (for the two public ones) or, for the now-internal ones, call the corresponding aztec-nr helper instead of recomputing the hash by hand. The generated TypeScript `DomainSeparator` enum in `@aztec/constants` / `@aztec/stdlib` likewise no longer contains the seven removed members (their values were unused in TypeScript). +### [Aztec Node] `GasPrice` renamed to `FeesPerGas` in `@aztec/ethereum` + +The `GasPrice` interface exported from `@aztec/ethereum/l1-tx-utils` is now `FeesPerGas`, matching viem's name for the same thing, and the methods and fields carrying it are renamed to match. These values were never a price paid: they are the EIP-1559 caps a transaction is sent with (`maxFeePerGas`, `maxPriorityFeePerGas`, `maxFeePerBlobGas`), and what the transaction actually pays is decided at inclusion. The fields inside the type are unchanged. + +| Old | New | +| ------------------------------------------- | --------------------------------------------- | +| `GasPrice` | `FeesPerGas` | +| `ReadOnlyL1TxUtils.getGasPrice()` | `ReadOnlyL1TxUtils.getFeesPerGas()` | +| `L1TxState.gasPrice` | `L1TxState.feesPerGas` | +| `L1TxState.gasPriceHistory` | `L1TxState.feesPerGasHistory` | +| `TimedOutTxState.gasPriceHistory` | `TimedOutTxState.feesPerGasHistory` | +| `TimedOutTxState.finalGasPrice` | `TimedOutTxState.finalFeesPerGas` | +| `L1Deployer.sendTransaction()` → `gasPrice` | `L1Deployer.sendTransaction()` → `feesPerGas` | + +**Migration:** + +```diff +- import type { GasPrice } from '@aztec/ethereum/l1-tx-utils'; ++ import type { FeesPerGas } from '@aztec/ethereum/l1-tx-utils'; + +- const gasPrice = await l1TxUtils.getGasPrice(gasConfig, true, 0); ++ const feesPerGas = await l1TxUtils.getFeesPerGas(gasConfig, true, 0); + + const { state } = await l1TxUtils.sendTransaction(request); +- logger.info(`Sent at ${state.gasPrice.maxFeePerGas}`); ++ logger.info(`Sent at ${state.feesPerGas.maxFeePerGas}`); +``` + +**Impact**: A rename with no behavior change. Two on-disk formats move with it, neither of which needs operator action: + +- The node's L1 transaction state store writes the new key but still reads states written under the old one, so a node restarting on this version keeps monitoring transactions it sent before the upgrade. +- Failed-L1-transaction debug records (written when `L1_TX_FAILED_STORE` is set) now use `sentFeesPerGas` and `sentFeesPerGasLadder` instead of `sentGasPrice` and `sentGasPriceLadder`. Records written before the upgrade keep the old keys, and parsing one through `FailedL1TxSchema` returns it without those fields, though the values are still present in the stored JSON under the old names. + ## 5.1.0 ### [Aztec.nr] Canonical HandshakeRegistry re-pinned at a new address @@ -119,10 +152,10 @@ Note types declared in their own module are unaffected, since they are already ` The `history::note` helpers that recompute a note's nullifier have been renamed with a `local_` prefix and now assert that the note belongs to the executing contract: -| Old | New | -| --- | --- | -| `assert_note_was_valid_by` | `assert_local_note_was_valid_by` | -| `assert_note_was_nullified_by` | `assert_local_note_was_nullified_by` | +| Old | New | +| ---------------------------------- | ---------------------------------------- | +| `assert_note_was_valid_by` | `assert_local_note_was_valid_by` | +| `assert_note_was_nullified_by` | `assert_local_note_was_nullified_by` | | `assert_note_was_not_nullified_by` | `assert_local_note_was_not_nullified_by` | These helpers derive the note's nullifier from the executing contract's app-siloed nullifier key, and it is not possible for a contract to retrieve the app-siloed nullifier for a different contract, as this would constitute leakage of key material. These helpers incorrectly used the local contract's key siloing, which for a note of a different contract resulted in the non-inclusion check passing unconditionally and wrongly reporting a nullified note as not nullified. The helpers now assert `contract_address == context.this_address()` and fail with "Note nullification history is only supported for the executing contract's own notes" otherwise. @@ -142,7 +175,7 @@ Previously, connecting a PXE or embedded wallet to a different or redeployed rol **Impact**: The first start after upgrading to this version begins with a fresh, empty store; the pre-upgrade data is not deleted. On Node.js environments (lmdb-v2) pre-upgrade data stays at `/` while new per-identity `pxe_data` stores live under `/-stores/`. The embedded Node.js wallet previously stored data in cwd-relative, rollup-address-suffixed directories instead (`pxe_data_/pxe_data` for the PXE store, `wallet_data_/wallet_data` for the wallet store): if you used it before this release, that is where the old data lives. The embedded wallet now defaults its data root to `aztec-wallet-data/`, with per-identity wallet stores under `/wallet_data-stores/` on Node.js and OPFS store names prefixed `wallet_data_` in the browser. Browser apps can enumerate and clean up `pxe_data` and `wallet_data` stores for networks no longer in use with the new `listStores()` / `deleteStore()` utilities: ```ts -import { deleteStore, listStores } from '@aztec/kv-store/sqlite-opfs'; +import { deleteStore, listStores } from "@aztec/kv-store/sqlite-opfs"; const names = await listStores(); await deleteStore(names[0]); @@ -262,7 +295,7 @@ Registering classes and instances are now separate, unvalidated operations. `reg + await pxe.registerContract(instance); ``` - If you were calling it without an artifact, just drop the wrapping object: `pxe.registerContract({ instance })` becomes `pxe.registerContract(instance)`. The `wallet.registerContract(instance, artifact?, secretKeyOrKeys?)` convenience is unchanged and performs both registrations for you. +If you were calling it without an artifact, just drop the wrapping object: `pxe.registerContract({ instance })` becomes `pxe.registerContract(instance)`. The `wallet.registerContract(instance, artifact?, secretKeyOrKeys?)` convenience is unchanged and performs both registrations for you. - To make a new class's code available after an onchain upgrade, register the new artifact instead of calling `updateContract`: @@ -271,9 +304,10 @@ Registering classes and instances are now separate, unvalidated operations. `reg + await pxe.registerContractClass(newArtifact); ``` - The new class is used automatically once the upgrade takes effect on chain; no further PXE action is needed. Registering it beforehand is harmless: until the update activates, the node still resolves the contract's current class to the previous one, so it keeps running its old code. +The new class is used automatically once the upgrade takes effect on chain; no further PXE action is needed. Registering it beforehand is harmless: until the update activates, the node still resolves the contract's current class to the previous one, so it keeps running its old code. - `pxe.getContractInstance(address)` and `wallet.getContractMetadata(address).instance` now return the contract's **address preimage**, which no longer includes `currentContractClassId`. + ### [Aztec.js] `AccountWithSecretKey` removed, read account keys from the `AccountManager` or PXE `AccountWithSecretKey` was a thin wrapper that bundled an account's transaction signer with its master secret key, used mainly to print or export the secret. It has been removed, and `AccountManager.getAccount()` now returns the plain `Account` signer. The wrapper's extra methods are no longer available on that value: @@ -294,7 +328,7 @@ Registering classes and instances are now separate, unvalidated operations. `reg To do what `AccountWithSecretKey` was meant for (exporting an account into a separate PXE or wallet), account registration accepts a full set of master secret keys instead of only a single seed. `wallet.registerContract(instance, artifact?, secretKeyOrKeys?)` takes either an `Fr` seed (as before) or a `MasterSecretKeys` object (exported from `@aztec/aztec.js/keys`), for an account whose privacy keys were generated independently rather than from one seed. -The PXE never receives the seed nor the message-signing and fallback secret keys: it is not trusted to hold them. The wallet derives the account's privacy keys and passes the PXE only the four privacy secret keys (nullifier-hiding, incoming-viewing, outgoing-viewing, tagging) plus the message-signing and fallback *public* keys. Accordingly, `pxe.getAccountSecretKeys(address)` returns only those four privacy secret keys. +The PXE never receives the seed nor the message-signing and fallback secret keys: it is not trusted to hold them. The wallet derives the account's privacy keys and passes the PXE only the four privacy secret keys (nullifier-hiding, incoming-viewing, outgoing-viewing, tagging) plus the message-signing and fallback _public_ keys. Accordingly, `pxe.getAccountSecretKeys(address)` returns only those four privacy secret keys. **Impact**: Importing `AccountWithSecretKey`, or calling `getSecretKey()`/`getEncryptionSecret()` on the result of `getAccount()`, no longer compiles. The signer `getAccount()` returns is otherwise unchanged, and passing a single `Fr` or a `MasterSecretKeys` to `wallet.registerContract` keeps working. @@ -393,21 +427,21 @@ If you must stay on IndexedDB for now, import from the deprecated entrypoint ins The PXE methods for registering tagging-secret sources have been replaced by a single set that takes a `TaggingSecretSource` discriminated union. `registerSender`/`getSenders`/`removeSender` and `registerSharedSecret`/`removeSharedSecret` are gone; use `registerTaggingSecretSource`/`removeTaggingSecretSource`/`getTaggingSecretSources` instead. The `Wallet` interface (`wallet.registerSender`, `getAddressBook`) is unchanged, so this only affects code that talks to a `PXE` instance directly. -| Before | After | -| --- | --- | -| `pxe.registerSender(address)` | `pxe.registerTaggingSecretSource({ kind: 'address-derived', sender: address })` | -| `pxe.removeSender(address)` | `pxe.removeTaggingSecretSource({ kind: 'address-derived', sender: address })` | -| `pxe.getSenders()` | `pxe.getTaggingSecretSources({ kind: 'address-derived' })` | +| Before | After | +| --------------------------------------------- | ---------------------------------------------------------------------------------- | +| `pxe.registerSender(address)` | `pxe.registerTaggingSecretSource({ kind: 'address-derived', sender: address })` | +| `pxe.removeSender(address)` | `pxe.removeTaggingSecretSource({ kind: 'address-derived', sender: address })` | +| `pxe.getSenders()` | `pxe.getTaggingSecretSources({ kind: 'address-derived' })` | | `pxe.registerSharedSecret(recipient, secret)` | `pxe.registerTaggingSecretSource({ kind: 'arbitrary-secret', recipient, secret })` | -| `pxe.removeSharedSecret(recipient, secret)` | `pxe.removeTaggingSecretSource({ kind: 'arbitrary-secret', recipient, secret })` | +| `pxe.removeSharedSecret(recipient, secret)` | `pxe.removeTaggingSecretSource({ kind: 'arbitrary-secret', recipient, secret })` | ### [Aztec.js] Unchecked `AztecAddress` constructors renamed with an `Unsafe` suffix The synchronous `AztecAddress` constructors that build an address from a raw value do not verify that the value is a valid address (the x-coordinate of a point on the Grumpkin curve, which is what allows it to be encrypted to). An invalid value is accepted silently and only fails later, when a transaction is sent. To make this obvious at the call site, they now carry an `Unsafe` suffix: -| Before | After | -| --- | --- | -| `AztecAddress.fromField` | `AztecAddress.fromFieldUnsafe` | +| Before | After | +| ------------------------- | ------------------------------- | +| `AztecAddress.fromField` | `AztecAddress.fromFieldUnsafe` | | `AztecAddress.fromBigInt` | `AztecAddress.fromBigIntUnsafe` | | `AztecAddress.fromNumber` | `AztecAddress.fromNumberUnsafe` | | `AztecAddress.fromString` | `AztecAddress.fromStringUnsafe` | @@ -453,7 +487,7 @@ A client factory `createProverNodeAdminClient(url, versions?, fetch?, apiKey?)` ### [Aztec.nr] `ContractInstance.contract_class_id` renamed to `original_contract_class_id` -The `contract_class_id` field of the `ContractInstance` struct (returned by `get_contract_instance`) has been renamed to `original_contract_class_id`. The struct is the contract's *address preimage*, so this field is the class id the contract was deployed with: for contracts whose class was later updated via the `ContractInstanceRegistry`, it is NOT the class currently executing. The rename makes that explicit. +The `contract_class_id` field of the `ContractInstance` struct (returned by `get_contract_instance`) has been renamed to `original_contract_class_id`. The struct is the contract's _address preimage_, so this field is the class id the contract was deployed with: for contracts whose class was later updated via the `ContractInstanceRegistry`, it is NOT the class currently executing. The rename makes that explicit. **Migration:** @@ -467,7 +501,7 @@ Note that this value is not available during public execution, which only has ac ### [Aztec.nr] `get_contract_instance_class_id_avm` renamed to `get_contract_instance_current_class_id_avm` -The AVM contract-instance class id getter has been renamed to make explicit that it returns the *current* class id, i.e. it reflects updates performed via the `ContractInstanceRegistry`. +The AVM contract-instance class id getter has been renamed to make explicit that it returns the _current_ class id, i.e. it reflects updates performed via the `ContractInstanceRegistry`. **Migration:** @@ -1582,7 +1616,6 @@ The empire slashing model has been removed. Only the tally-based slashing model `slashMinPenaltyPercentage` and `slashMaxPenaltyPercentage` removed from `SlasherConfig`. - ### [Aztec Node] `getTxByHash`, `getTxsByHash` and `getPendingTxs` no longer return tx proofs by default `AztecNode.getTxByHash`, `AztecNode.getTxsByHash` and `AztecNode.getPendingTxs` (also exposed on the P2P API) now take an optional `GetTxByHashOptions` argument with an `includeProof` flag. The proof is stripped from returned txs unless `includeProof: true` is passed, cutting roughly 35-52KB per tx over the wire. diff --git a/yarn-project/ethereum/src/deploy_l1_contract.ts b/yarn-project/ethereum/src/deploy_l1_contract.ts index 28228a3c3bf..dcce1d8b957 100644 --- a/yarn-project/ethereum/src/deploy_l1_contract.ts +++ b/yarn-project/ethereum/src/deploy_l1_contract.ts @@ -26,7 +26,7 @@ import { RegisterNewRollupVersionPayloadArtifact } from './l1_artifacts.js'; import { type L1TxUtilsConfig, getL1TxUtilsConfigEnvVars } from './l1_tx_utils/config.js'; import { createL1TxUtils } from './l1_tx_utils/factory.js'; import type { L1TxUtils } from './l1_tx_utils/l1_tx_utils.js'; -import type { GasPrice, L1TxConfig, L1TxRequest } from './l1_tx_utils/types.js'; +import type { FeesPerGas, L1TxConfig, L1TxRequest } from './l1_tx_utils/types.js'; import type { ExtendedViemWalletClient } from './types.js'; import { formatViemError } from './utils.js'; @@ -131,11 +131,11 @@ export class L1Deployer { sendTransaction( tx: L1TxRequest, options?: L1TxConfig, - ): Promise<{ txHash: Hex; gasLimit: bigint; gasPrice: GasPrice }> { + ): Promise<{ txHash: Hex; gasLimit: bigint; feesPerGas: FeesPerGas }> { return this.l1TxUtils.sendTransaction(tx, options).then(({ txHash, state }) => ({ txHash, gasLimit: state.gasLimit, - gasPrice: state.gasPrice, + feesPerGas: state.feesPerGas, })); } } diff --git a/yarn-project/ethereum/src/l1_tx_utils/README.md b/yarn-project/ethereum/src/l1_tx_utils/README.md index d5e82f74b60..f63c88cc0c9 100644 --- a/yarn-project/ethereum/src/l1_tx_utils/README.md +++ b/yarn-project/ethereum/src/l1_tx_utils/README.md @@ -128,7 +128,7 @@ def send_and_monitor_tx(tx_request): # Speed up if stalled and have retries left if time_since_last_sent > stall_time and attempts < max_attempts: - replacement_tx = make_tx(tx_request, nonce, bump_gas_price(state.gasPrice)) + replacement_tx = make_tx(tx_request, nonce, bump_fees_per_gas(state.feesPerGas)) state.status = is_cancel_tx ? 'cancelled' : 'speed-up' state.tx_hashes.push(send_tx(replacement_tx)) state.last_sent_at = now @@ -144,7 +144,7 @@ def attempt_tx_cancellation(state): return # Send noop tx with same nonce but higher gas - cancel_tx = make_noop_tx(state.nonce, bump_gas_price(state.gasPrice)) + cancel_tx = make_noop_tx(state.nonce, bump_fees_per_gas(state.feesPerGas)) state.cancelTxHashes.push(send_tx(cancel_tx)) state.status = 'cancelled' diff --git a/yarn-project/ethereum/src/l1_tx_utils/gas_price_ladder.test.ts b/yarn-project/ethereum/src/l1_tx_utils/fees_per_gas_ladder.test.ts similarity index 87% rename from yarn-project/ethereum/src/l1_tx_utils/gas_price_ladder.test.ts rename to yarn-project/ethereum/src/l1_tx_utils/fees_per_gas_ladder.test.ts index 69d3ff6d81d..c5a4f70bc13 100644 --- a/yarn-project/ethereum/src/l1_tx_utils/gas_price_ladder.test.ts +++ b/yarn-project/ethereum/src/l1_tx_utils/fees_per_gas_ladder.test.ts @@ -19,7 +19,7 @@ import { type L1TxRequest, L1TxTimeoutError } from './types.js'; const MNEMONIC = 'test test test test test test test test test test test junk'; const WEI_CONST = 1_000_000_000n; -const logger = createLogger('ethereum:test:gas-price-ladder'); +const logger = createLogger('ethereum:test:fees-per-gas-ladder'); const REQUEST: L1TxRequest = { to: '0x1234567890123456789012345678901234567890', @@ -49,7 +49,7 @@ async function runUntilTimeout(gasUtils: L1TxUtils, cheatCodes: EthCheatCodes): return promise; } -describe('L1TxUtils gas-price ladder capture (integration)', () => { +describe('L1TxUtils fees-per-gas ladder capture (integration)', () => { const initialBaseFee = WEI_CONST; // 1 gwei let l1Client: ExtendedViemWalletClient; let anvil: Anvil; @@ -96,7 +96,7 @@ describe('L1TxUtils gas-price ladder capture (integration)', () => { await anvil.stop().catch(err => createLogger('cleanup').error(err)); }, 10_000); - it('captures the escalating gas-price ladder and surfaces it on timeout', async () => { + it('captures the escalating fees-per-gas ladder and surfaces it on timeout', async () => { const gasUtils = makeGasUtils({ maxSpeedUpAttempts: 3 }); const err = await runUntilTimeout(gasUtils, cheatCodes); @@ -107,18 +107,18 @@ describe('L1TxUtils gas-price ladder capture (integration)', () => { const { txState } = err as L1TxTimeoutError; // Initial send plus at least one speed-up. - expect(txState.gasPriceHistory).toBeDefined(); - expect(txState.gasPriceHistory!.length).toBeGreaterThanOrEqual(2); + expect(txState.feesPerGasHistory).toBeDefined(); + expect(txState.feesPerGasHistory!.length).toBeGreaterThanOrEqual(2); // Priority fees strictly escalate across the ladder. - const priorityFees = txState.gasPriceHistory!.map(g => g.maxPriorityFeePerGas); + const priorityFees = txState.feesPerGasHistory!.map(g => g.maxPriorityFeePerGas); for (let i = 1; i < priorityFees.length; i++) { expect(priorityFees[i]).toBeGreaterThan(priorityFees[i - 1]); } // attempts == number of sent txs == ladder length; other snapshot fields are populated. - expect(txState.attempts).toBe(txState.gasPriceHistory!.length); - expect(txState.finalGasPrice.maxPriorityFeePerGas).toBe(priorityFees[priorityFees.length - 1]); + expect(txState.attempts).toBe(txState.feesPerGasHistory!.length); + expect(txState.finalFeesPerGas.maxPriorityFeePerGas).toBe(priorityFees[priorityFees.length - 1]); expect(txState.gasLimit).toBeGreaterThan(0n); expect(txState.nonce).toBeGreaterThanOrEqual(0); diff --git a/yarn-project/ethereum/src/l1_tx_utils/l1_tx_timeout_error.test.ts b/yarn-project/ethereum/src/l1_tx_utils/l1_tx_timeout_error.test.ts index 742d8aed8e9..78e249b9bb4 100644 --- a/yarn-project/ethereum/src/l1_tx_utils/l1_tx_timeout_error.test.ts +++ b/yarn-project/ethereum/src/l1_tx_utils/l1_tx_timeout_error.test.ts @@ -4,11 +4,11 @@ import { L1TxTimeoutError, type TimedOutTxState } from './types.js'; describe('L1TxTimeoutError', () => { const txState: TimedOutTxState = { - gasPriceHistory: [ + feesPerGasHistory: [ { maxFeePerGas: 100n, maxPriorityFeePerGas: 1n }, { maxFeePerGas: 120n, maxPriorityFeePerGas: 2n }, ], - finalGasPrice: { maxFeePerGas: 120n, maxPriorityFeePerGas: 2n }, + finalFeesPerGas: { maxFeePerGas: 120n, maxPriorityFeePerGas: 2n }, attempts: 2, nonce: 7, gasLimit: 21_000n, @@ -22,7 +22,7 @@ describe('L1TxTimeoutError', () => { expect(err instanceof Error).toBe(true); }); - it('carries the gas-price ladder snapshot and message', () => { + it('carries the fees-per-gas ladder snapshot and message', () => { const err = new L1TxTimeoutError('timed out', txState); expect(err.message).toBe('timed out'); expect(err.txState).toEqual(txState); 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 7ac26ccb95f..0d7ac8129c6 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 @@ -332,7 +332,7 @@ describe('L1TxUtils', () => { cancelTxHashes: [], status: TxUtilsState.SENT, gasLimit: estimatedGas, - gasPrice: { + feesPerGas: { maxFeePerGas: originalMaxFeePerGas, maxPriorityFeePerGas: originalMaxPriorityFeePerGas, maxFeePerBlobGas: WEI_CONST * 20n, @@ -455,7 +455,7 @@ describe('L1TxUtils', () => { l1Client.estimateMaxPriorityFeePerGas = () => Promise.resolve(mockBasePriorityFee); try { - const gasPrice = await gasUtils['getGasPrice'](); + const feesPerGas = await gasUtils['getFeesPerGas'](); // With default config, priority fee should be bumped by 20% const expectedPriorityFee = (mockBasePriorityFee * 120n) / 100n; @@ -463,8 +463,8 @@ describe('L1TxUtils', () => { // Base fee should be bumped for potential stalls (1.125^(stallTimeMs/12000) = ~1.125 for default config) const expectedMaxFee = (WEI_CONST * 1125n) / 1000n + expectedPriorityFee; - expect(gasPrice.maxPriorityFeePerGas).toBe(expectedPriorityFee); - expect(gasPrice.maxFeePerGas).toBe(expectedMaxFee); + expect(feesPerGas.maxPriorityFeePerGas).toBe(expectedPriorityFee); + expect(feesPerGas.maxFeePerGas).toBe(expectedMaxFee); } finally { // Restore original method l1Client.estimateMaxPriorityFeePerGas = originalEstimate; @@ -489,11 +489,11 @@ describe('L1TxUtils', () => { minimumPriorityFeePerGas: 0, }); - const gasPrice = await gasUtils['getGasPrice'](undefined, true); + const feesPerGas = await gasUtils['getFeesPerGas'](undefined, true); // With ceiling division: (1n * 1125n + 999n) / 1000n = 2n - expect(gasPrice.maxFeePerGas).toBe(2n); - expect(gasPrice.maxFeePerBlobGas).toBe(2n); + expect(feesPerGas.maxFeePerGas).toBe(2n); + expect(feesPerGas.maxFeePerBlobGas).toBe(2n); // Verify compounding works across multiple iterations gasUtils.updateConfig({ @@ -503,12 +503,12 @@ describe('L1TxUtils', () => { minimumPriorityFeePerGas: 0, }); - const gasPrice2 = await gasUtils['getGasPrice'](undefined, true); + const feesPerGas2 = await gasUtils['getFeesPerGas'](undefined, true); // Iteration 1: ceil(1 * 1125 / 1000) = 2 // Iteration 2: ceil(2 * 1125 / 1000) = ceil(2.25) = 3 - expect(gasPrice2.maxFeePerGas).toBe(3n); - expect(gasPrice2.maxFeePerBlobGas).toBe(3n); + expect(feesPerGas2.maxFeePerGas).toBe(3n); + expect(feesPerGas2.maxFeePerBlobGas).toBe(3n); } finally { l1Client.getBlobBaseFee = originalGetBlobBaseFee; l1Client.estimateMaxPriorityFeePerGas = originalEstimate; @@ -519,17 +519,17 @@ describe('L1TxUtils', () => { await cheatCodes.setNextBlockBaseFeePerGas(WEI_CONST); await cheatCodes.evmMine(); - const initialGasPrice = await gasUtils['getGasPrice'](); + const initialFeesPerGas = await gasUtils['getFeesPerGas'](); // Get retry gas price for 2nd attempt - const retryGasPrice = await gasUtils['getGasPrice'](undefined, false, 1, initialGasPrice); + const retryFeesPerGas = await gasUtils['getFeesPerGas'](undefined, false, 1, initialFeesPerGas); // With default config, retry should bump fees by 50% - const expectedPriorityFee = (initialGasPrice.maxPriorityFeePerGas * 150n) / 100n; - const expectedMaxFee = (initialGasPrice.maxFeePerGas * 150n) / 100n; + const expectedPriorityFee = (initialFeesPerGas.maxPriorityFeePerGas * 150n) / 100n; + const expectedMaxFee = (initialFeesPerGas.maxFeePerGas * 150n) / 100n; - expect(retryGasPrice.maxPriorityFeePerGas).toBe(expectedPriorityFee); - expect(retryGasPrice.maxFeePerGas).toBe(expectedMaxFee); + expect(retryFeesPerGas.maxPriorityFeePerGas).toBe(expectedPriorityFee); + expect(retryFeesPerGas.maxFeePerGas).toBe(expectedMaxFee); }); it('handles minimum priority fee with fractional part', async () => { @@ -550,12 +550,12 @@ describe('L1TxUtils', () => { priorityFeeBumpPercentage: 0, // No bump to make test clearer }); - const gasPrice = await gasUtils['getGasPrice'](); + const feesPerGas = await gasUtils['getFeesPerGas'](); // Priority fee should be at least 1.5 gwei = 1_500_000_000 wei (the minimum) const expectedMinimumFee = BigInt(Math.trunc(1.5 * Number(WEI_CONST))); - expect(gasPrice.maxPriorityFeePerGas).toBe(expectedMinimumFee); - expect(gasPrice.maxPriorityFeePerGas).toBe(1_500_000_000n); + expect(feesPerGas.maxPriorityFeePerGas).toBe(expectedMinimumFee); + expect(feesPerGas.maxPriorityFeePerGas).toBe(1_500_000_000n); // Test with full 9 decimal places (2.123456789 gwei) gasUtils.updateConfig({ @@ -564,11 +564,11 @@ describe('L1TxUtils', () => { priorityFeeBumpPercentage: 0, // No bump to make test clearer }); - const gasPrice2 = await gasUtils['getGasPrice'](); + const feesPerGas2 = await gasUtils['getFeesPerGas'](); const expectedMinimumFee2 = BigInt(Math.trunc(2.123456789 * Number(WEI_CONST))); - expect(gasPrice2.maxPriorityFeePerGas).toBe(expectedMinimumFee2); - expect(gasPrice2.maxPriorityFeePerGas).toBe(2_123_456_789n); + expect(feesPerGas2.maxPriorityFeePerGas).toBe(expectedMinimumFee2); + expect(feesPerGas2.maxPriorityFeePerGas).toBe(2_123_456_789n); } finally { // Restore original method l1Client.estimateMaxPriorityFeePerGas = originalEstimate; @@ -593,12 +593,12 @@ describe('L1TxUtils', () => { priorityFeeBumpPercentage: 0, // No bump to make test clearer }); - const gasPrice = await gasUtils['getGasPrice'](); + const feesPerGas = await gasUtils['getFeesPerGas'](); // Network fee (5 gwei) should be used since it exceeds the minimum (1 gwei) const minimumFee = WEI_CONST; // 1 gwei - expect(gasPrice.maxPriorityFeePerGas).toBeGreaterThan(minimumFee); - expect(gasPrice.maxPriorityFeePerGas).toBe(mockHighPriorityFee); + expect(feesPerGas.maxPriorityFeePerGas).toBeGreaterThan(minimumFee); + expect(feesPerGas.maxPriorityFeePerGas).toBe(mockHighPriorityFee); } finally { // Restore original method l1Client.estimateMaxPriorityFeePerGas = originalEstimate; @@ -623,12 +623,12 @@ describe('L1TxUtils', () => { priorityFeeBumpPercentage: 20, // 20% bump }); - const gasPrice = await gasUtils['getGasPrice'](); + const feesPerGas = await gasUtils['getFeesPerGas'](); // Expected: network fee (3 gwei) with 20% bump = 3.6 gwei const expectedFee = (mockNetworkFee * 120n) / 100n; - expect(gasPrice.maxPriorityFeePerGas).toBe(expectedFee); - expect(gasPrice.maxPriorityFeePerGas).toBe(3_600_000_000n); + expect(feesPerGas.maxPriorityFeePerGas).toBe(expectedFee); + expect(feesPerGas.maxPriorityFeePerGas).toBe(3_600_000_000n); } finally { // Restore original method l1Client.estimateMaxPriorityFeePerGas = originalEstimate; @@ -650,12 +650,12 @@ describe('L1TxUtils', () => { await cheatCodes.setNextBlockBaseFeePerGas(highBaseFee); await cheatCodes.evmMine(); - const gasPrice = await gasUtils['getGasPrice'](); + const feesPerGas = await gasUtils['getFeesPerGas'](); // Max fee should be capped at 500.5 gwei = 500_500_000_000 wei const expectedMaxGwei = BigInt(Math.trunc(500.5 * Number(WEI_CONST))); - expect(gasPrice.maxFeePerGas).toBe(expectedMaxGwei); - expect(gasPrice.maxFeePerGas).toBe(500_500_000_000n); + expect(feesPerGas.maxFeePerGas).toBe(expectedMaxGwei); + expect(feesPerGas.maxFeePerGas).toBe(500_500_000_000n); // Test with more complex fractional value (123.456789 gwei) gasUtils.updateConfig({ @@ -663,12 +663,12 @@ describe('L1TxUtils', () => { maxGwei: 123.456789, }); - const gasPrice2 = await gasUtils['getGasPrice'](); + const feesPerGas2 = await gasUtils['getFeesPerGas'](); // Should cap at 123.456789 gwei = 123_456_789_000 wei const expectedMaxGwei2 = BigInt(Math.trunc(123.456789 * Number(WEI_CONST))); - expect(gasPrice2.maxFeePerGas).toBe(expectedMaxGwei2); - expect(gasPrice2.maxFeePerGas).toBe(123_456_789_000n); + expect(feesPerGas2.maxFeePerGas).toBe(expectedMaxGwei2); + expect(feesPerGas2.maxFeePerGas).toBe(123_456_789_000n); }); it('handles maxBlobGwei with fractional part', async () => { @@ -690,12 +690,12 @@ describe('L1TxUtils', () => { // Mock high blob base fee to trigger the max cap (200 gwei > 100.5 gwei cap) l1Client.getBlobBaseFee = () => Promise.resolve(WEI_CONST * 200n); - const gasPrice = await gasUtils['getGasPrice'](undefined, true); + const feesPerGas = await gasUtils['getFeesPerGas'](undefined, true); // Max blob fee should be capped at 100.5 gwei = 100_500_000_000 wei const expectedMaxBlobGwei = BigInt(Math.trunc(100.5 * Number(WEI_CONST))); - expect(gasPrice.maxFeePerBlobGas).toBe(expectedMaxBlobGwei); - expect(gasPrice.maxFeePerBlobGas).toBe(100_500_000_000n); + expect(feesPerGas.maxFeePerBlobGas).toBe(expectedMaxBlobGwei); + expect(feesPerGas.maxFeePerBlobGas).toBe(100_500_000_000n); // Test with more complex fractional value (250.123456 gwei) gasUtils.updateConfig({ @@ -707,12 +707,12 @@ describe('L1TxUtils', () => { // Use higher base fee to trigger this cap (300 gwei > 250.123456 gwei cap) l1Client.getBlobBaseFee = () => Promise.resolve(WEI_CONST * 300n); - const gasPrice2 = await gasUtils['getGasPrice'](undefined, true); + const feesPerGas2 = await gasUtils['getFeesPerGas'](undefined, true); // Should cap at 250.123456 gwei = 250_123_456_000 wei const expectedMaxBlobGwei2 = BigInt(Math.trunc(250.123456 * Number(WEI_CONST))); - expect(gasPrice2.maxFeePerBlobGas).toBe(expectedMaxBlobGwei2); - expect(gasPrice2.maxFeePerBlobGas).toBe(250_123_456_000n); + expect(feesPerGas2.maxFeePerBlobGas).toBe(expectedMaxBlobGwei2); + expect(feesPerGas2.maxFeePerBlobGas).toBe(250_123_456_000n); } finally { // Restore original method l1Client.getBlobBaseFee = originalGetBlobBaseFee; @@ -725,17 +725,17 @@ describe('L1TxUtils', () => { priorityFeeRetryBumpPercentage: 5, // Set lower than minimum 10% }); - const initialGasPrice = await gasUtils['getGasPrice'](); + const initialFeesPerGas = await gasUtils['getFeesPerGas'](); // Get retry gas price with attempt = 1 - const retryGasPrice = await gasUtils['getGasPrice'](undefined, false, 1, initialGasPrice); + const retryFeesPerGas = await gasUtils['getFeesPerGas'](undefined, false, 1, initialFeesPerGas); // Should use 10% minimum bump even though config specified 5% - const expectedPriorityFee = (initialGasPrice.maxPriorityFeePerGas * 110n) / 100n; - const expectedMaxFee = (initialGasPrice.maxFeePerGas * 110n) / 100n; + const expectedPriorityFee = (initialFeesPerGas.maxPriorityFeePerGas * 110n) / 100n; + const expectedMaxFee = (initialFeesPerGas.maxFeePerGas * 110n) / 100n; - expect(retryGasPrice.maxPriorityFeePerGas).toBe(expectedPriorityFee); - expect(retryGasPrice.maxFeePerGas).toBe(expectedMaxFee); + expect(retryFeesPerGas.maxPriorityFeePerGas).toBe(expectedPriorityFee); + expect(retryFeesPerGas.maxFeePerGas).toBe(expectedMaxFee); }); it('uses competitive fee from pending txs and fee history on retry attempts', async () => { @@ -776,21 +776,21 @@ describe('L1TxUtils', () => { l1Client.estimateMaxPriorityFeePerGas = () => Promise.resolve(WEI_CONST); // 1 gwei try { - const initialGasPrice = await gasUtils['getGasPrice'](); + const initialFeesPerGas = await gasUtils['getFeesPerGas'](); // Get retry gas price - should use competitive fee from pending (3 gwei at 75th percentile) - const retryGasPrice = await gasUtils['getGasPrice'](undefined, false, 1, initialGasPrice); + const retryFeesPerGas = await gasUtils['getFeesPerGas'](undefined, false, 1, initialFeesPerGas); // Competitive fee should be: max(network=1gwei, historical=2gwei, pending=3gwei) = 3gwei, then bumped by 50% const expectedCompetitiveFee = (mockPendingFee * 150n) / 100n; // 4.5 gwei // The minimum bump from initial would be initialPriority * 1.5 - const minBump = (initialGasPrice.maxPriorityFeePerGas * 150n) / 100n; + const minBump = (initialFeesPerGas.maxPriorityFeePerGas * 150n) / 100n; // Final should be max of competitive and minimum bump const expectedPriorityFee = expectedCompetitiveFee > minBump ? expectedCompetitiveFee : minBump; - expect(retryGasPrice.maxPriorityFeePerGas).toBeGreaterThanOrEqual(expectedPriorityFee); + expect(retryFeesPerGas.maxPriorityFeePerGas).toBeGreaterThanOrEqual(expectedPriorityFee); } finally { // Restore original methods l1Client.getBlock = originalGetBlock; @@ -812,18 +812,18 @@ describe('L1TxUtils', () => { l1Client.estimateMaxPriorityFeePerGas = () => Promise.resolve(mockBasePriorityFee); try { - const initialGasPrice = await gasUtils['getGasPrice'](); + const initialFeesPerGas = await gasUtils['getFeesPerGas'](); // Get retry gas price - should fallback to network estimate when fee history fails - const retryGasPrice = await gasUtils['getGasPrice'](undefined, false, 1, initialGasPrice); + const retryFeesPerGas = await gasUtils['getFeesPerGas'](undefined, false, 1, initialFeesPerGas); // Should still get a valid price using network estimate fallback - expect(retryGasPrice.maxPriorityFeePerGas).toBeGreaterThan(0n); - expect(retryGasPrice.maxFeePerGas).toBeGreaterThan(0n); + expect(retryFeesPerGas.maxPriorityFeePerGas).toBeGreaterThan(0n); + expect(retryFeesPerGas.maxFeePerGas).toBeGreaterThan(0n); // Should be at least the minimum bump - const minBump = (initialGasPrice.maxPriorityFeePerGas * 150n) / 100n; - expect(retryGasPrice.maxPriorityFeePerGas).toBeGreaterThanOrEqual(minBump); + const minBump = (initialFeesPerGas.maxPriorityFeePerGas * 150n) / 100n; + expect(retryFeesPerGas.maxPriorityFeePerGas).toBeGreaterThanOrEqual(minBump); } finally { // Restore original methods l1Client.getFeeHistory = originalGetFeeHistory; @@ -1934,7 +1934,7 @@ describe('L1TxUtils', () => { // Verify all read-only methods are available expect(l1TxUtils.getBlock).toBeDefined(); expect(l1TxUtils.getBlockNumber).toBeDefined(); - expect(l1TxUtils.getGasPrice).toBeDefined(); + expect(l1TxUtils.getFeesPerGas).toBeDefined(); expect(l1TxUtils.estimateGas).toBeDefined(); expect(l1TxUtils.getTransactionStats).toBeDefined(); expect(l1TxUtils.simulate).toBeDefined(); 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 5d96204e1d7..1f5b9d8abfd 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 @@ -271,13 +271,13 @@ export class L1TxUtils extends ReadOnlyL1TxUtils { } this.logger.trace(`Computed gas limit ${gasLimit}`, { gasLimit, ...request }); - const gasPrice = await this.getGasPrice(gasConfig, !!blobInputs); + const feesPerGas = await this.getFeesPerGas(gasConfig, !!blobInputs); const now = await this.checkInterruptedOrTimedOut(gasConfig); let txHash: Hex; let nonce: number; - let baseState: Pick; + let baseState: Pick; await this.sendMutex.acquire(); try { @@ -287,7 +287,7 @@ export class L1TxUtils extends ReadOnlyL1TxUtils { nonce = this.lastSentNonce !== undefined && chainNonce <= this.lastSentNonce ? this.lastSentNonce + 1 : chainNonce; - baseState = { request, gasLimit, blobInputs, gasPrice, nonce }; + baseState = { request, gasLimit, blobInputs, feesPerGas, nonce }; const txData = this.makeTxData(baseState, { isCancelTx: false }); const signedRequest = await this.prepareSignedTransaction(txData); @@ -307,7 +307,7 @@ export class L1TxUtils extends ReadOnlyL1TxUtils { txConfigOverrides: gasConfigOverrides ?? {}, sentAtL1Ts: now, lastSentAtL1Ts: now, - gasPriceHistory: [baseState.gasPrice], + feesPerGasHistory: [baseState.feesPerGas], }; // And persist it @@ -330,9 +330,9 @@ export class L1TxUtils extends ReadOnlyL1TxUtils { account, sentAt: now, gasLimit, - maxFeePerGas: formatGwei(gasPrice.maxFeePerGas), - maxPriorityFeePerGas: formatGwei(gasPrice.maxPriorityFeePerGas), - ...(gasPrice.maxFeePerBlobGas && { maxFeePerBlobGas: formatGwei(gasPrice.maxFeePerBlobGas) }), + maxFeePerGas: formatGwei(feesPerGas.maxFeePerGas), + maxPriorityFeePerGas: formatGwei(feesPerGas.maxPriorityFeePerGas), + ...(feesPerGas.maxFeePerBlobGas && { maxFeePerBlobGas: formatGwei(feesPerGas.maxFeePerBlobGas) }), isBlobTx: !!blobInputs, txConfig: gasConfigOverrides, }); @@ -504,20 +504,20 @@ export class L1TxUtils extends ReadOnlyL1TxUtils { // Speed up the transaction if it appears to be stuck (exceeded stall time and still have retry attempts) const attempts = txHashes.length; if (timePassed >= stallTimeMs && attempts <= maxSpeedUpAttempts) { - const newGasPrice = await this.getGasPrice(gasConfig, isBlobTx, attempts, state.gasPrice); - state.gasPrice = newGasPrice; - state.gasPriceHistory?.push(newGasPrice); + const newFeesPerGas = await this.getFeesPerGas(gasConfig, isBlobTx, attempts, state.feesPerGas); + state.feesPerGas = newFeesPerGas; + state.feesPerGasHistory?.push(newFeesPerGas); this.logger.debug( `Tx ${currentTxHash} with nonce ${nonce} from ${account} appears stuck. ` + `Attempting speed-up ${attempts}/${maxSpeedUpAttempts} ` + - `with new priority fee ${formatGwei(newGasPrice.maxPriorityFeePerGas)} gwei.`, + `with new priority fee ${formatGwei(newFeesPerGas.maxPriorityFeePerGas)} gwei.`, { account, nonce, - maxFeePerGas: formatGwei(newGasPrice.maxFeePerGas), - maxPriorityFeePerGas: formatGwei(newGasPrice.maxPriorityFeePerGas), - ...(newGasPrice.maxFeePerBlobGas && { maxFeePerBlobGas: formatGwei(newGasPrice.maxFeePerBlobGas) }), + maxFeePerGas: formatGwei(newFeesPerGas.maxFeePerGas), + maxPriorityFeePerGas: formatGwei(newFeesPerGas.maxPriorityFeePerGas), + ...(newFeesPerGas.maxFeePerBlobGas && { maxFeePerBlobGas: formatGwei(newFeesPerGas.maxFeePerBlobGas) }), }, ); @@ -532,10 +532,10 @@ export class L1TxUtils extends ReadOnlyL1TxUtils { nonce, account, gasLimit, - maxFeePerGas: formatGwei(newGasPrice.maxFeePerGas), - maxPriorityFeePerGas: formatGwei(newGasPrice.maxPriorityFeePerGas), + maxFeePerGas: formatGwei(newFeesPerGas.maxFeePerGas), + maxPriorityFeePerGas: formatGwei(newFeesPerGas.maxPriorityFeePerGas), txConfig: state.txConfigOverrides, - ...(newGasPrice.maxFeePerBlobGas && { maxFeePerBlobGas: formatGwei(newGasPrice.maxFeePerBlobGas) }), + ...(newFeesPerGas.maxFeePerBlobGas && { maxFeePerBlobGas: formatGwei(newFeesPerGas.maxFeePerBlobGas) }), }, ); @@ -558,7 +558,7 @@ export class L1TxUtils extends ReadOnlyL1TxUtils { timePassed, isBlobTx, isCancelTx, - ...pick(state.gasPrice, 'maxFeePerGas', 'maxPriorityFeePerGas', 'maxFeePerBlobGas'), + ...pick(state.feesPerGas, 'maxFeePerGas', 'maxPriorityFeePerGas', 'maxFeePerBlobGas'), ...pick( gasConfig, 'txUnseenConsideredDroppedMs', @@ -623,13 +623,13 @@ export class L1TxUtils extends ReadOnlyL1TxUtils { * and an empty blob input if the original tx also had blobs. */ private makeTxData( - state: Pick, + state: Pick, opts: { isCancelTx: boolean }, ): PrepareTransactionRequestRequest { - const { request, gasLimit, blobInputs, gasPrice, nonce } = state; + const { request, gasLimit, blobInputs, feesPerGas, nonce } = state; const isBlobTx = blobInputs !== undefined; - const baseTxOpts = { nonce, ...pick(gasPrice, 'maxFeePerGas', 'maxPriorityFeePerGas') }; + const baseTxOpts = { nonce, ...pick(feesPerGas, 'maxFeePerGas', 'maxPriorityFeePerGas') }; if (opts.isCancelTx) { const baseTxData = { @@ -640,7 +640,7 @@ export class L1TxUtils extends ReadOnlyL1TxUtils { ...baseTxOpts, }; - return isBlobTx ? { ...baseTxData, ...this.makeEmptyBlobInputs(gasPrice.maxFeePerBlobGas!) } : baseTxData; + return isBlobTx ? { ...baseTxData, ...this.makeEmptyBlobInputs(feesPerGas.maxFeePerBlobGas!) } : baseTxData; } const baseTxData = { @@ -649,7 +649,7 @@ export class L1TxUtils extends ReadOnlyL1TxUtils { gas: gasLimit, }; - return blobInputs ? { ...baseTxData, ...blobInputs, maxFeePerBlobGas: gasPrice.maxFeePerBlobGas! } : baseTxData; + return blobInputs ? { ...baseTxData, ...blobInputs, maxFeePerBlobGas: feesPerGas.maxFeePerBlobGas! } : baseTxData; } /** Returns when all monitor loops have stopped. */ @@ -680,10 +680,10 @@ export class L1TxUtils extends ReadOnlyL1TxUtils { return { receipt, state }; } catch (err) { if (err instanceof TimeoutError) { - // Snapshot the ladder now: the fire-and-forget cancellation mutates state.gasPrice moments later. + // Snapshot the ladder now: the fire-and-forget cancellation mutates state.feesPerGas moments later. throw new L1TxTimeoutError(err.message, { - gasPriceHistory: state.gasPriceHistory ? [...state.gasPriceHistory] : undefined, - finalGasPrice: state.gasPrice, + feesPerGasHistory: state.feesPerGasHistory ? [...state.feesPerGasHistory] : undefined, + finalFeesPerGas: state.feesPerGas, attempts: state.txHashes.length, nonce: state.nonce, gasLimit: state.gasLimit, @@ -702,14 +702,14 @@ export class L1TxUtils extends ReadOnlyL1TxUtils { ): Promise<{ gasUsed: bigint; result: `0x${string}` }> { const blockOverrides = { ..._blockOverrides }; const gasConfig = merge(this.config, _gasConfig); - const gasPrice = await this.getGasPrice(gasConfig, false); + const feesPerGas = await this.getFeesPerGas(gasConfig, false); const call: any = { to: request.to!, data: request.data, from: request.from ?? this.getSenderAddress().toString(), - maxFeePerGas: gasPrice.maxFeePerGas, - maxPriorityFeePerGas: gasPrice.maxPriorityFeePerGas, + maxFeePerGas: feesPerGas.maxFeePerGas, + maxPriorityFeePerGas: feesPerGas.maxPriorityFeePerGas, gas: request.gas ?? MAX_L1_TX_LIMIT, }; @@ -728,7 +728,7 @@ export class L1TxUtils extends ReadOnlyL1TxUtils { */ protected async attemptTxCancellation(state: L1TxState): Promise { const isBlobTx = state.blobInputs !== undefined; - const { nonce, gasPrice: previousGasPrice } = state; + const { nonce, feesPerGas: previousFeesPerGas } = state; const account = this.getSenderAddress().toString(); // Do not send cancellation if interrupted @@ -741,18 +741,18 @@ export class L1TxUtils extends ReadOnlyL1TxUtils { return; } - // Resolve the pending nonce and cancellation gas price up front. These are read-only RPC calls, so + // Resolve the pending nonce and cancellation fees per gas up front. These are read-only RPC calls, so // they are safe to retry; a transient RPC failure here must not permanently abandon the cancellation // (which would leave the nonce stuck and the tx state stranded short of CANCELLED). We retry with a - // real backoff before giving up, unlike getGasPrice's tight internal retry which can be exhausted by + // real backoff before giving up, unlike getFeesPerGas's tight internal retry which can be exhausted by // a brief node hiccup. The retried block performs no state-changing send, so retrying cannot // double-send the cancellation tx. - const { currentNonce, cancelGasPrice } = await retry( + const { currentNonce, cancelFeesPerGas } = await retry( async () => { const currentNonce = await this.client.getTransactionCount({ address: account, blockTag: 'pending' }); if (currentNonce >= nonce) { - // Get gas price with higher priority fee for cancellation - const cancelGasPrice = await this.getGasPrice( + // Get fees per gas with higher priority fee for cancellation + const cancelFeesPerGas = await this.getFeesPerGas( { ...this.config, // Use high bump for cancellation to ensure it replaces the original tx @@ -760,18 +760,18 @@ export class L1TxUtils extends ReadOnlyL1TxUtils { }, isBlobTx, state.txHashes.length, - previousGasPrice, + previousFeesPerGas, ); - return { currentNonce, cancelGasPrice }; + return { currentNonce, cancelFeesPerGas }; } - return { currentNonce, cancelGasPrice: undefined }; + return { currentNonce, cancelFeesPerGas: undefined }; }, `Preparing cancellation for L1 tx from account ${account} with nonce ${nonce}`, makeBackoff(CANCELLATION_PREP_RETRY_BACKOFF_S), this.logger, ); - if (cancelGasPrice === undefined) { + if (cancelFeesPerGas === undefined) { this.logger.verbose( `Not sending cancellation for L1 tx from account ${account} with nonce ${nonce} as it is dropped`, { nonce, account, currentNonce }, @@ -780,7 +780,7 @@ export class L1TxUtils extends ReadOnlyL1TxUtils { return; } - const { maxFeePerGas, maxPriorityFeePerGas, maxFeePerBlobGas } = cancelGasPrice; + const { maxFeePerGas, maxPriorityFeePerGas, maxFeePerBlobGas } = cancelFeesPerGas; this.logger.verbose( `Attempting to cancel L1 ${isBlobTx ? 'blob' : 'vanilla'} transaction from account ${account} with nonce ${nonce} after time out`, { @@ -791,7 +791,7 @@ export class L1TxUtils extends ReadOnlyL1TxUtils { ); // Send 0-value tx to self with higher gas price - state.gasPrice = cancelGasPrice; + state.feesPerGas = cancelFeesPerGas; state.lastSentAtL1Ts = new Date(await this.getL1Timestamp()); const txData = this.makeTxData(state, { isCancelTx: true }); @@ -803,7 +803,7 @@ export class L1TxUtils extends ReadOnlyL1TxUtils { this.logger.warn(`Sent cancellation tx ${cancelTxHash} for timed out tx from ${account} with nonce ${nonce}`, { nonce, - cancelGasPrice, + cancelFeesPerGas, isBlobTx, txHashes: state.txHashes, }); diff --git a/yarn-project/ethereum/src/l1_tx_utils/readonly_l1_tx_utils.ts b/yarn-project/ethereum/src/l1_tx_utils/readonly_l1_tx_utils.ts index cea21480f4f..a53dd688174 100644 --- a/yarn-project/ethereum/src/l1_tx_utils/readonly_l1_tx_utils.ts +++ b/yarn-project/ethereum/src/l1_tx_utils/readonly_l1_tx_utils.ts @@ -34,7 +34,7 @@ import { WEI_CONST, } from './constants.js'; import { P75AllTxsPriorityFeeStrategy, type PriorityFeeStrategy } from './fee-strategies/index.js'; -import type { GasPrice, L1BlobInputs, L1TxRequest, TransactionStats } from './types.js'; +import type { FeesPerGas, L1BlobInputs, L1TxRequest, TransactionStats } from './types.js'; import { getCalldataGasUsage, tryGetCustomErrorNameContractFunction } from './utils.js'; // Change this to the current strategy we want to use @@ -75,14 +75,14 @@ export class ReadOnlyL1TxUtils { } /** - * Gets the current gas price with bounds checking + * Gets the EIP-1559 fees per gas to send with, derived from the current network fees and bounded by the config */ - public async getGasPrice( + public async getFeesPerGas( gasConfigOverrides?: L1TxUtilsConfig, isBlobTx: boolean = false, attempt: number = 0, - previousGasPrice?: typeof attempt extends 0 ? never : GasPrice, - ): Promise { + previousFeesPerGas?: typeof attempt extends 0 ? never : FeesPerGas, + ): Promise { const gasConfig = merge(this.config, gasConfigOverrides); // Execute strategy - it handles all RPC calls internally and returns everything we need @@ -148,8 +148,8 @@ export class ReadOnlyL1TxUtils { // Calculate minimum required fees based on previous attempt // multiply by 100 & divide by 100 to maintain some precision const minPriorityFee = - (previousGasPrice!.maxPriorityFeePerGas * (100_00n + BigInt(bumpPercentage * 1_00))) / 100_00n; - const minMaxFee = (previousGasPrice!.maxFeePerGas * (100_00n + BigInt(bumpPercentage * 1_00))) / 100_00n; + (previousFeesPerGas!.maxPriorityFeePerGas * (100_00n + BigInt(bumpPercentage * 1_00))) / 100_00n; + const minMaxFee = (previousFeesPerGas!.maxFeePerGas * (100_00n + BigInt(bumpPercentage * 1_00))) / 100_00n; // Apply bump percentage to competitive fee const competitivePriorityFee = (priorityFee * (100_00n + BigInt(configBump * 1_00))) / 100_00n; @@ -200,14 +200,14 @@ export class ReadOnlyL1TxUtils { // Ensure priority fee doesn't exceed max fee const maxPriorityFeePerGas = priorityFee > maxFeePerGas ? maxFeePerGas : priorityFee; - if (attempt > 0 && previousGasPrice?.maxFeePerBlobGas) { + if (attempt > 0 && previousFeesPerGas?.maxFeePerBlobGas) { const bumpPercentage = gasConfig.priorityFeeRetryBumpPercentage! > MIN_BLOB_REPLACEMENT_BUMP_PERCENTAGE ? gasConfig.priorityFeeRetryBumpPercentage! : MIN_BLOB_REPLACEMENT_BUMP_PERCENTAGE; // calculate min blob fee based on previous attempt - const minBlobFee = (previousGasPrice.maxFeePerBlobGas * (100_00n + BigInt(bumpPercentage * 1_00))) / 100_00n; + const minBlobFee = (previousFeesPerGas.maxFeePerBlobGas * (100_00n + BigInt(bumpPercentage * 1_00))) / 100_00n; // use max between current network values and min required values maxFeePerBlobGas = maxFeePerBlobGas > minBlobFee ? maxFeePerBlobGas : minBlobFee; @@ -248,12 +248,12 @@ export class ReadOnlyL1TxUtils { // Use 2x buffer for maxFeePerBlobGas to avoid stale fees and to pass EIP-4844 validation (even if it is a gas estimation call). // 1. maxFeePerBlobGas >= blobBaseFee // 2. account balance >= gas * maxFeePerGas + maxFeePerBlobGas * blobCount + value - const gasPrice = await this.getGasPrice(gasConfig, true, 0); + const feesPerGas = await this.getFeesPerGas(gasConfig, true, 0); initialEstimate = await this.client.estimateGas({ account, ...request, ..._blobInputs, - maxFeePerBlobGas: gasPrice.maxFeePerBlobGas! * 2n, + maxFeePerBlobGas: feesPerGas.maxFeePerBlobGas! * 2n, gas: MAX_L1_TX_LIMIT, blockTag: 'latest', }); diff --git a/yarn-project/ethereum/src/l1_tx_utils/types.ts b/yarn-project/ethereum/src/l1_tx_utils/types.ts index 44d24548b35..9c88228118d 100644 --- a/yarn-project/ethereum/src/l1_tx_utils/types.ts +++ b/yarn-project/ethereum/src/l1_tx_utils/types.ts @@ -22,7 +22,7 @@ export interface L1BlobInputs { maxFeePerBlobGas?: bigint; } -export interface GasPrice { +export interface FeesPerGas { maxFeePerGas: bigint; maxPriorityFeePerGas: bigint; maxFeePerBlobGas?: bigint; @@ -55,13 +55,13 @@ export type L1TxState = { txHashes: Hex[]; cancelTxHashes: Hex[]; gasLimit: bigint; - gasPrice: GasPrice; + feesPerGas: FeesPerGas; /** - * Prices used for each attempt (initial send followed by each speed-up), in order. Always set on + * Fees per gas used for each attempt (initial send followed by each speed-up), in order. Always set on * newly sent txs; optional because states restored from the state store predate the field. * In-memory only — not persisted by the state store. */ - gasPriceHistory?: GasPrice[]; + feesPerGasHistory?: FeesPerGas[]; txConfigOverrides: L1TxConfig; request: L1TxRequest; status: TxUtilsState; @@ -93,10 +93,10 @@ export class DroppedTransactionError extends Error { /** Snapshot of what a timed-out L1 tx tried to pay, taken when the timeout is raised. */ export type TimedOutTxState = { - /** Prices used across the initial send and each speed-up, in order (undefined only for restored states). */ - gasPriceHistory?: GasPrice[]; - /** The last price the tx was sent at before timing out. */ - finalGasPrice: GasPrice; + /** Fees per gas used across the initial send and each speed-up, in order (undefined only for restored states). */ + feesPerGasHistory?: FeesPerGas[]; + /** The last fees per gas the tx was sent with before timing out. */ + finalFeesPerGas: FeesPerGas; /** Number of send attempts (initial + speed-ups). */ attempts: number; nonce: number; @@ -105,7 +105,7 @@ export type TimedOutTxState = { /** * Thrown by sendAndMonitorTransaction when a tx times out. Subclasses TimeoutError so existing - * `instanceof TimeoutError` checks keep working, while carrying the gas-price ladder for diagnostics. + * `instanceof TimeoutError` checks keep working, while carrying the fees-per-gas ladder for diagnostics. */ export class L1TxTimeoutError extends TimeoutError { constructor( diff --git a/yarn-project/node-lib/src/metrics/l1_tx_metrics.ts b/yarn-project/node-lib/src/metrics/l1_tx_metrics.ts index 22af22e0975..dbcfc372ee5 100644 --- a/yarn-project/node-lib/src/metrics/l1_tx_metrics.ts +++ b/yarn-project/node-lib/src/metrics/l1_tx_metrics.ts @@ -92,12 +92,12 @@ export class L1TxMetrics implements IL1TxMetrics { const attempts = isCancelTx ? state.cancelTxHashes.length : state.txHashes.length; this.txAttemptsUntilMined.record(attempts, attributes); - // Record gas prices at end state, converted from wei to gwei to match metric unit definitions + // Record fees per gas at end state, converted from wei to gwei to match metric unit definitions const weiToGwei = 1e9; - const maxPriorityFeeGwei = Number(state.gasPrice.maxPriorityFeePerGas) / weiToGwei; - const maxFeeGwei = Number(state.gasPrice.maxFeePerGas) / weiToGwei; - const blobFeeGwei = state.gasPrice.maxFeePerBlobGas - ? Number(state.gasPrice.maxFeePerBlobGas) / weiToGwei + const maxPriorityFeeGwei = Number(state.feesPerGas.maxPriorityFeePerGas) / weiToGwei; + const maxFeeGwei = Number(state.feesPerGas.maxFeePerGas) / weiToGwei; + const blobFeeGwei = state.feesPerGas.maxFeePerBlobGas + ? Number(state.feesPerGas.maxFeePerBlobGas) / weiToGwei : undefined; this.maxPriorityFeeHistogram.record(maxPriorityFeeGwei, attributes); diff --git a/yarn-project/node-lib/src/stores/l1_tx_store.test.ts b/yarn-project/node-lib/src/stores/l1_tx_store.test.ts index 06508d5b497..76529e0d303 100644 --- a/yarn-project/node-lib/src/stores/l1_tx_store.test.ts +++ b/yarn-project/node-lib/src/stores/l1_tx_store.test.ts @@ -32,7 +32,7 @@ describe('L1TxStore', () => { txHashes: [`0xabc${nonce}` as Hex], cancelTxHashes: [], gasLimit: 21000n, - gasPrice: { + feesPerGas: { maxFeePerGas: 1000000000n, maxPriorityFeePerGas: 1000000n, }, @@ -126,14 +126,14 @@ describe('L1TxStore', () => { const account = '0xabc123'; const state = createMockState(1); state.gasLimit = 123456789012345n; - state.gasPrice.maxFeePerGas = 987654321098765n; + state.feesPerGas.maxFeePerGas = 987654321098765n; state.request.value = 5000000000000000000n; await store.saveState(account, state); const loaded = await store.loadStates(account); expect(loaded[0].gasLimit).toBe(123456789012345n); - expect(loaded[0].gasPrice.maxFeePerGas).toBe(987654321098765n); + expect(loaded[0].feesPerGas.maxFeePerGas).toBe(987654321098765n); expect(loaded[0].request.value).toBe(5000000000000000000n); }); @@ -407,6 +407,24 @@ describe('L1TxStore', () => { }); }); + describe('legacy gasPrice key', () => { + it('loads states written under the legacy gasPrice key', async () => { + const account = '0xabc123'; + const saved = await store.saveState(account, createMockState(1)); + + // Rewrite the stored state into the pre-rename shape, where fees per gas were spelled `gasPrice`. + const states = kvStore.openMap('l1_tx_states'); + const key = `${account}-${saved.id.toString().padStart(10, '0')}`; + const { feesPerGas, ...rest } = JSON.parse((await states.getAsync(key))!); + await states.set(key, JSON.stringify({ ...rest, gasPrice: feesPerGas })); + + const loaded = await store.loadStates(account); + + expect(loaded).toHaveLength(1); + expect(loaded[0].feesPerGas).toEqual(saved.feesPerGas); + }); + }); + describe('serialize/deserialize roundtrip', () => { it('should preserve all state fields through serialization', async () => { const account = '0xabc123'; @@ -417,7 +435,7 @@ describe('L1TxStore', () => { txHashes: ['0xhash1' as Hex, '0xhash2' as Hex], cancelTxHashes: ['0xcancel1' as Hex], gasLimit: 123456n, - gasPrice: { + feesPerGas: { maxFeePerGas: 50000000000n, maxPriorityFeePerGas: 2000000000n, maxFeePerBlobGas: 1000000000n, diff --git a/yarn-project/node-lib/src/stores/l1_tx_store.ts b/yarn-project/node-lib/src/stores/l1_tx_store.ts index d434a2c6dcf..e3ce788a441 100644 --- a/yarn-project/node-lib/src/stores/l1_tx_store.ts +++ b/yarn-project/node-lib/src/stores/l1_tx_store.ts @@ -16,9 +16,9 @@ interface SerializableL1TxRequest { } /** - * Serializable version of GasPrice for storage. + * Serializable version of FeesPerGas for storage. */ -interface SerializableGasPrice { +interface SerializableFeesPerGas { maxFeePerGas: string; maxPriorityFeePerGas: string; maxFeePerBlobGas?: string; @@ -57,7 +57,7 @@ interface SerializableL1TxState { txHashes: string[]; cancelTxHashes: string[]; gasLimit: string; - gasPrice: SerializableGasPrice; + feesPerGas: SerializableFeesPerGas; txConfigOverrides: SerializableL1TxConfig; request: SerializableL1TxRequest; status: number; @@ -69,6 +69,16 @@ interface SerializableL1TxState { blobMetadata?: SerializableBlobMetadata; } +/** + * Shape accepted when reading a state back. States written before the rename spelled `feesPerGas` as `gasPrice`, so + * both are optional here and `deserializeState` takes whichever is present. Nothing writes `gasPrice` any more; drop + * it once no store can hold states predating the rename. + */ +type StoredL1TxState = Omit & { + feesPerGas?: SerializableFeesPerGas; + gasPrice?: SerializableFeesPerGas; +}; + /** * Serializable blob inputs for separate storage. */ @@ -167,7 +177,7 @@ export class L1TxStore implements IL1TxStore { const stateId = parseInt(stateIdStr, 10); try { - const serialized: SerializableL1TxState = JSON.parse(stateJson); + const serialized: StoredL1TxState = JSON.parse(stateJson); // Load blobs if they exist let blobInputs: L1BlobInputs | undefined; @@ -207,7 +217,7 @@ export class L1TxStore implements IL1TxStore { } try { - const serialized: SerializableL1TxState = JSON.parse(stateJson); + const serialized: StoredL1TxState = JSON.parse(stateJson); // Load blobs if they exist let blobInputs: L1BlobInputs | undefined; @@ -300,10 +310,10 @@ export class L1TxStore implements IL1TxStore { txHashes: state.txHashes, cancelTxHashes: state.cancelTxHashes, gasLimit: state.gasLimit.toString(), - gasPrice: { - maxFeePerGas: state.gasPrice.maxFeePerGas.toString(), - maxPriorityFeePerGas: state.gasPrice.maxPriorityFeePerGas.toString(), - maxFeePerBlobGas: state.gasPrice.maxFeePerBlobGas?.toString(), + feesPerGas: { + maxFeePerGas: state.feesPerGas.maxFeePerGas.toString(), + maxPriorityFeePerGas: state.feesPerGas.maxPriorityFeePerGas.toString(), + maxFeePerBlobGas: state.feesPerGas.maxFeePerBlobGas?.toString(), }, txConfigOverrides, request: { @@ -325,7 +335,12 @@ export class L1TxStore implements IL1TxStore { /** * Deserializes a stored state back to L1TxState. */ - private deserializeState(stored: SerializableL1TxState, blobInputs?: L1BlobInputs): L1TxState { + private deserializeState(stored: StoredL1TxState, blobInputs?: L1BlobInputs): L1TxState { + const feesPerGas = stored.feesPerGas ?? stored.gasPrice; + if (!feesPerGas) { + throw new Error(`State ${stored.id} has no fees per gas`); + } + const txConfigOverrides: L1TxConfig = { ...stored.txConfigOverrides, gasLimit: stored.txConfigOverrides.gasLimit !== undefined ? BigInt(stored.txConfigOverrides.gasLimit) : undefined, @@ -348,10 +363,10 @@ export class L1TxStore implements IL1TxStore { txHashes: stored.txHashes as `0x${string}`[], cancelTxHashes: stored.cancelTxHashes as `0x${string}`[], gasLimit: BigInt(stored.gasLimit), - gasPrice: { - maxFeePerGas: BigInt(stored.gasPrice.maxFeePerGas), - maxPriorityFeePerGas: BigInt(stored.gasPrice.maxPriorityFeePerGas), - maxFeePerBlobGas: stored.gasPrice.maxFeePerBlobGas ? BigInt(stored.gasPrice.maxFeePerBlobGas) : undefined, + feesPerGas: { + maxFeePerGas: BigInt(feesPerGas.maxFeePerGas), + maxPriorityFeePerGas: BigInt(feesPerGas.maxPriorityFeePerGas), + maxFeePerBlobGas: feesPerGas.maxFeePerBlobGas ? BigInt(feesPerGas.maxFeePerBlobGas) : undefined, }, txConfigOverrides, request: { diff --git a/yarn-project/prover-node/src/prover-node-publisher.test.ts b/yarn-project/prover-node/src/prover-node-publisher.test.ts index ef62ca0d97a..f7a1e248dc1 100644 --- a/yarn-project/prover-node/src/prover-node-publisher.test.ts +++ b/yarn-project/prover-node/src/prover-node-publisher.test.ts @@ -256,7 +256,7 @@ describe('prover-node-publisher', () => { jest.spyOn(l1Utils, 'getSenderAddress').mockReturnValue(EthAddress.random()); jest.spyOn(l1Utils, 'estimateGas').mockResolvedValue(500_000n); jest - .spyOn(l1Utils, 'getGasPrice') + .spyOn(l1Utils, 'getFeesPerGas') .mockResolvedValue({ maxFeePerGas: 20_000_000_000n, maxPriorityFeePerGas: 1_000_000_000n }); (l1Utils as any).client = { getBlock: jest @@ -284,7 +284,7 @@ describe('prover-node-publisher', () => { }); expect(l1Utils.estimateGas).toHaveBeenCalled(); - expect(l1Utils.getGasPrice).toHaveBeenCalled(); + expect(l1Utils.getFeesPerGas).toHaveBeenCalled(); expect(l1Utils.sendAndMonitorTransaction).not.toHaveBeenCalled(); }); @@ -339,7 +339,7 @@ describe('prover-node-publisher', () => { jest.spyOn(l1Utils, 'getSenderAddress').mockReturnValue(EthAddress.random()); jest.spyOn(l1Utils, 'sendAndMonitorTransaction').mockResolvedValue({ - state: { gasPrice: {} as any } as any, + state: { feesPerGas: {} as any } as any, receipt: { status: 'reverted', effectiveGasPrice: 1n, diff --git a/yarn-project/prover-node/src/prover-node-publisher.ts b/yarn-project/prover-node/src/prover-node-publisher.ts index 0ff328189de..eebeeff0108 100644 --- a/yarn-project/prover-node/src/prover-node-publisher.ts +++ b/yarn-project/prover-node/src/prover-node-publisher.ts @@ -220,14 +220,14 @@ export class ProverNodePublisher { const data = this.encodeSubmitEpochProofCalldata(args); const senderAddress = this.l1TxUtils.getSenderAddress(); - const [gasLimit, gasPrice, latestBlock] = await Promise.all([ + const [gasLimit, feesPerGas, latestBlock] = await Promise.all([ this.l1TxUtils.estimateGas(senderAddress.toString() as `0x${string}`, { to: this.proofSubmissionTarget, data }), - this.l1TxUtils.getGasPrice(), + this.l1TxUtils.getFeesPerGas(), this.l1TxUtils.client.getBlock({ blockTag: 'latest' }), ]); const baseFeePerGas = latestBlock.baseFeePerGas ?? 0n; - const { maxPriorityFeePerGas } = gasPrice; + const { maxPriorityFeePerGas } = feesPerGas; const effectiveFeePerGas = baseFeePerGas + maxPriorityFeePerGas; const estimatedTotalFee = gasLimit * effectiveFeePerGas; diff --git a/yarn-project/sequencer-client/src/publisher/l1_tx_failed_store/failed_tx_store.ts b/yarn-project/sequencer-client/src/publisher/l1_tx_failed_store/failed_tx_store.ts index a78d67aea69..1ce1b3dfe0f 100644 --- a/yarn-project/sequencer-client/src/publisher/l1_tx_failed_store/failed_tx_store.ts +++ b/yarn-project/sequencer-client/src/publisher/l1_tx_failed_store/failed_tx_store.ts @@ -1,5 +1,5 @@ import type { WindowBlockFees } from '@aztec/ethereum/l1-fee-analysis'; -import type { GasPrice } from '@aztec/ethereum/l1-tx-utils'; +import type { FeesPerGas } from '@aztec/ethereum/l1-tx-utils'; import type { Branded } from '@aztec/foundation/branded-types'; import { type ZodFor, schemas } from '@aztec/foundation/schemas'; @@ -55,22 +55,25 @@ export type FailedL1Tx = { }; /** Gas pricing info at time of failure for underpricing diagnosis. */ gasInfo?: { - /** Gas prices the tx was sent with (present for revert/send-error/timeout, not simulation). */ - sentGasPrice?: GasPrice; + /** + * Fees per gas the tx was sent with (present for revert/send-error/timeout, not simulation). Records written before + * this field was renamed spell it `sentGasPrice`, and parse back without it. + */ + sentFeesPerGas?: FeesPerGas; /** Gas limit used or estimated. */ gasLimit?: bigint; /** Nonce used for the sent tx. */ nonce?: number; /** - * For timeouts: the escalating gas prices used across the initial send and each speed-up retry, - * in order. Compare against windowBlocks[].minIncludedPriorityFee to see if any attempt cleared the bar. + * For timeouts: the escalating fees per gas used across the initial send and each speed-up retry, in order. + * Compare against windowBlocks[].minIncludedPriorityFee to see if any attempt cleared the bar. */ - sentGasPriceLadder?: GasPrice[]; + sentFeesPerGasLadder?: FeesPerGas[]; /** Number of send attempts (initial send + speed-ups). */ attempts?: number; /** * Per-block fee data for the L1 blocks the tx could have been included in (the target L2 slot's - * inclusion window), in chronological order. Compare sentGasPrice against these to see whether + * inclusion window), in chronological order. Compare sentFeesPerGas against these to see whether * the tx was underpriced for each block it competed for. May be a partial or empty list if the * window was not yet mined when the failure was recorded (e.g. an early send failure). */ @@ -89,11 +92,11 @@ export type FailedL1Tx = { const hexSchema = schemas.HexStringWith0x; -const gasPriceSchema = z.object({ +const feesPerGasSchema = z.object({ maxFeePerGas: schemas.BigInt, maxPriorityFeePerGas: schemas.BigInt, maxFeePerBlobGas: schemas.BigInt.optional(), -}) satisfies ZodFor; +}) satisfies ZodFor; const windowBlockFeesSchema = z.object({ blockNumber: schemas.BigInt, @@ -138,10 +141,10 @@ export const FailedL1TxSchema: ZodFor = z.object({ }), gasInfo: z .object({ - sentGasPrice: gasPriceSchema.optional(), + sentFeesPerGas: feesPerGasSchema.optional(), gasLimit: schemas.BigInt.optional(), nonce: z.number().optional(), - sentGasPriceLadder: z.array(gasPriceSchema).optional(), + sentFeesPerGasLadder: z.array(feesPerGasSchema).optional(), attempts: z.number().optional(), windowBlocks: z.array(windowBlockFeesSchema).optional(), }) diff --git a/yarn-project/sequencer-client/src/publisher/sequencer-publisher.test.ts b/yarn-project/sequencer-client/src/publisher/sequencer-publisher.test.ts index 4fa19d92c57..dfd5bf06ad7 100644 --- a/yarn-project/sequencer-client/src/publisher/sequencer-publisher.test.ts +++ b/yarn-project/sequencer-client/src/publisher/sequencer-publisher.test.ts @@ -209,7 +209,7 @@ describe('SequencerPublisher', () => { l1TxUtils.sendAndMonitorTransaction.mockResolvedValue({ receipt: proposeTxReceipt, - state: { gasPrice: { maxFeePerGas: 1n, maxPriorityFeePerGas: 1n } } as any, + state: { feesPerGas: { maxFeePerGas: 1n, maxPriorityFeePerGas: 1n } } as any, }); (l1TxUtils as any).estimateGas.mockResolvedValue(GAS_GUESS); (l1TxUtils as any).simulate.mockResolvedValue({ gasUsed: 1_000_000n, result: '0x' }); @@ -1176,7 +1176,7 @@ describe('SequencerPublisher', () => { expect(governanceProposerContract.getPayloadProposalStatus).toHaveBeenCalledTimes(2); }); - describe('failed-tx store: timeout gas-price ladder (integration)', () => { + describe('failed-tx store: timeout fees-per-gas ladder (integration)', () => { let storeDir: string; let storedPublisher: SequencerPublisher; @@ -1209,13 +1209,13 @@ describe('SequencerPublisher', () => { await rm(storeDir, { recursive: true, force: true }); }); - it('writes the gas-price ladder into the failed-tx record when a propose times out', async () => { + it('writes the fees-per-gas ladder into the failed-tx record when a propose times out', async () => { const ladder = { - gasPriceHistory: [ + feesPerGasHistory: [ { maxFeePerGas: 100n, maxPriorityFeePerGas: 1n }, { maxFeePerGas: 150n, maxPriorityFeePerGas: 3n }, ], - finalGasPrice: { maxFeePerGas: 150n, maxPriorityFeePerGas: 3n }, + finalFeesPerGas: { maxFeePerGas: 150n, maxPriorityFeePerGas: 3n }, attempts: 2, nonce: 5, gasLimit: 21_000n, @@ -1234,7 +1234,7 @@ describe('SequencerPublisher', () => { const record = await waitForFailedTxRecord(join(storeDir, 'timeout')); expect(record.failureType).toBe('timeout'); - expect(record.gasInfo?.sentGasPriceLadder).toEqual([ + expect(record.gasInfo?.sentFeesPerGasLadder).toEqual([ { maxFeePerGas: 100n, maxPriorityFeePerGas: 1n }, { maxFeePerGas: 150n, maxPriorityFeePerGas: 3n }, ]); diff --git a/yarn-project/sequencer-client/src/publisher/sequencer-publisher.ts b/yarn-project/sequencer-client/src/publisher/sequencer-publisher.ts index 9b97e0bfedb..d34b47ebcff 100644 --- a/yarn-project/sequencer-client/src/publisher/sequencer-publisher.ts +++ b/yarn-project/sequencer-client/src/publisher/sequencer-publisher.ts @@ -614,7 +614,7 @@ export class SequencerPublisher { timing: this.computeTimingInfo(currentL2Slot), gasInfo: timeoutState ? { - sentGasPriceLadder: timeoutState.gasPriceHistory, + sentFeesPerGasLadder: timeoutState.feesPerGasHistory, attempts: timeoutState.attempts, gasLimit: timeoutState.gasLimit, nonce: timeoutState.nonce, @@ -790,7 +790,7 @@ export class SequencerPublisher { }, gasInfo: err.txState ? { - sentGasPrice: err.txState.gasPrice, + sentFeesPerGas: err.txState.feesPerGas, gasLimit: err.txState.gasLimit, nonce: err.txState.nonce, } From 7f3dad975a6119dd689fe26ca58854a01c0d83dc Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 12 Aug 2026 12:23:46 +0100 Subject: [PATCH 3/3] feat: improve mempool performance when chain tip finalises (#25189) Port #25148 to next. --------- Co-authored-by: Santiago Palladino --- release-image/Dockerfile.dockerignore | 1 + .../src/store/contract_instance_store.ts | 12 +- .../aztec-node/src/aztec-node/server.ts | 8 +- .../src/verifier/batch_chonk_verifier.ts | 5 + .../src/verifier/queued_chonk_verifier.ts | 3 +- .../end-to-end/src/spartan/n_tps.test.ts | 10 + .../src/spartan/n_tps_prove.test.ts | 10 + .../src/spartan/utils/gossip_metrics.ts | 108 +++++++++ .../end-to-end/src/spartan/utils/index.ts | 3 + .../foundation/src/iterable/first.test.ts | 40 ++++ yarn-project/foundation/src/iterable/first.ts | 14 ++ yarn-project/foundation/src/iterable/index.ts | 1 + .../src/deprecated/indexeddb/multi_map.ts | 4 +- .../tx_pool_v2/archive/tx_archive.ts | 42 +++- .../mem_pools/tx_pool_v2/instrumentation.ts | 27 +++ .../src/mem_pools/tx_pool_v2/tx_pool_v2.ts | 103 ++++++--- .../tx_pool_v2_finalization.test.ts | 218 ++++++++++++++++++ .../mem_pools/tx_pool_v2/tx_pool_v2_impl.ts | 91 +++++--- .../src/services/libp2p/instrumentation.ts | 8 + .../p2p/src/services/libp2p/libp2p_service.ts | 55 ++++- .../telemetry-client/src/attributes.ts | 6 + yarn-project/telemetry-client/src/metrics.ts | 25 ++ 22 files changed, 699 insertions(+), 95 deletions(-) create mode 100644 yarn-project/end-to-end/src/spartan/utils/gossip_metrics.ts create mode 100644 yarn-project/foundation/src/iterable/first.test.ts create mode 100644 yarn-project/foundation/src/iterable/first.ts create mode 100644 yarn-project/p2p/src/mem_pools/tx_pool_v2/tx_pool_v2_finalization.test.ts diff --git a/release-image/Dockerfile.dockerignore b/release-image/Dockerfile.dockerignore index 16f26933391..17839d9aa66 100644 --- a/release-image/Dockerfile.dockerignore +++ b/release-image/Dockerfile.dockerignore @@ -30,6 +30,7 @@ !/yarn-project/noir-protocol-circuits-types/artifacts/ !/yarn-project/protocol-contracts/artifacts/ !/yarn-project/standard-contracts/artifacts/ +!/yarn-project/standard-contracts/artifacts-historical/ !/yarn-project/noir-contracts.js/artifacts/ !/yarn-project/noir-test-contracts.js/artifacts/ !/yarn-project/simulator/artifacts/ diff --git a/yarn-project/archiver/src/store/contract_instance_store.ts b/yarn-project/archiver/src/store/contract_instance_store.ts index dd58039cb97..426d877d6d8 100644 --- a/yarn-project/archiver/src/store/contract_instance_store.ts +++ b/yarn-project/archiver/src/store/contract_instance_store.ts @@ -1,4 +1,5 @@ import type { Fr } from '@aztec/foundation/curves/bn254'; +import { first } from '@aztec/foundation/iterable'; import type { AztecAsyncKVStore, AztecAsyncMap } from '@aztec/kv-store'; import { isProtocolContract } from '@aztec/protocol-contracts'; import type { AztecAddress } from '@aztec/stdlib/aztec-address'; @@ -134,19 +135,18 @@ export class ContractInstanceStore { async getCurrentContractInstanceClassId(address: AztecAddress, timestamp: UInt64, originalClassId: Fr): Promise { // We need to find the last update before the given timestamp - const queryResult = await this.#contractInstanceUpdates - .valuesAsync({ + const serializedUpdate = await first( + this.#contractInstanceUpdates.valuesAsync({ reverse: true, start: this.getUpdateKey(address, 0n), // Make sure we only look at updates for this contract end: this.getUpdateKey(address, timestamp + 1n), // No update can match this key since it doesn't have a log index. We want the highest key <= timestamp limit: 1, - }) - .next(); - if (queryResult.done) { + }), + ); + if (serializedUpdate === undefined) { return originalClassId; } - const serializedUpdate = queryResult.value; const update = SerializableContractInstanceUpdate.fromBuffer(serializedUpdate); if (timestamp < update.timestampOfChange) { return update.prevContractClassId.isZero() ? originalClassId : update.prevContractClassId; diff --git a/yarn-project/aztec-node/src/aztec-node/server.ts b/yarn-project/aztec-node/src/aztec-node/server.ts index 526557ca861..31c2b17eb2d 100644 --- a/yarn-project/aztec-node/src/aztec-node/server.ts +++ b/yarn-project/aztec-node/src/aztec-node/server.ts @@ -16,6 +16,7 @@ import { import { compactArray, pick, unique } from '@aztec/foundation/collection'; import { Fr } from '@aztec/foundation/curves/bn254'; import { EthAddress } from '@aztec/foundation/eth-address'; +import { first } from '@aztec/foundation/iterable'; import { BadRequestError } from '@aztec/foundation/json-rpc'; import { type Logger, createLogger } from '@aztec/foundation/log'; import { retryUntil } from '@aztec/foundation/retry'; @@ -463,11 +464,8 @@ export class AztecNodeService implements AztecNode, AztecNodeAdmin, AztecNodeDeb } public async getMaxPriorityFees(): Promise { - for await (const tx of this.p2pClient.iteratePendingTxs({ includeProof: false })) { - return tx.getGasSettings().maxPriorityFeesPerGas; - } - - return GasFees.from({ feePerDaGas: 0n, feePerL2Gas: 0n }); + const tx = await first(this.p2pClient.iteratePendingTxs({ includeProof: false })); + return tx ? tx.getGasSettings().maxPriorityFeesPerGas : GasFees.from({ feePerDaGas: 0n, feePerL2Gas: 0n }); } /** diff --git a/yarn-project/bb-prover/src/verifier/batch_chonk_verifier.ts b/yarn-project/bb-prover/src/verifier/batch_chonk_verifier.ts index 147042b2c9b..edd65f480ab 100644 --- a/yarn-project/bb-prover/src/verifier/batch_chonk_verifier.ts +++ b/yarn-project/bb-prover/src/verifier/batch_chonk_verifier.ts @@ -6,6 +6,7 @@ import { Timer } from '@aztec/foundation/timer'; import { ProtocolCircuitVks } from '@aztec/noir-protocol-circuits-types/server/vks'; import type { ClientProtocolCircuitVerifier, IVCProofVerificationResult } from '@aztec/stdlib/interfaces/server'; import type { Tx } from '@aztec/stdlib/tx'; +import { getTelemetryClient } from '@aztec/telemetry-client'; import { Unpackr } from 'msgpackr'; import { execFile } from 'node:child_process'; @@ -16,6 +17,7 @@ import * as path from 'node:path'; import { promisify } from 'node:util'; import type { BBConfig } from '../config.js'; +import { IVCVerifierMetrics } from './queued_chonk_verifier.js'; const execFileAsync = promisify(execFile); const RESULT_TIMEOUT_MS = 5 * 60 * 1000; @@ -58,6 +60,7 @@ export class BatchChonkVerifier implements ClientProtocolCircuitVerifier { private sendQueue: SerialQueue; private fifoReader: FifoFrameReader; private logger = createLogger('bb-prover:batch_chonk_verifier'); + private metrics: IVCVerifierMetrics; /** Maps artifact name to VK index in the batch verifier. */ private vkIndexMap = new Map(); /** Bound cleanup handler for process exit signals. */ @@ -75,6 +78,7 @@ export class BatchChonkVerifier implements ClientProtocolCircuitVerifier { this.fifoReader = new FifoFrameReader(); this.sendQueue = new SerialQueue(); this.sendQueue.start(1); + this.metrics = new IVCVerifierMetrics(getTelemetryClient(), `BatchChonkVerifier-${label}`); } /** Create and start a BatchChonkVerifier using the protocol circuit VKs. */ @@ -311,6 +315,7 @@ export class BatchChonkVerifier implements ClientProtocolCircuitVerifier { const totalDurationMs = pending.totalTimer.ms(); const ivcResult: IVCProofVerificationResult = { valid, durationMs, totalDurationMs }; + this.metrics.recordIVCVerification(ivcResult); if (!valid) { this.logger.warn(`Proof verification failed for request_id=${result.request_id}: ${result.error_message}`); diff --git a/yarn-project/bb-prover/src/verifier/queued_chonk_verifier.ts b/yarn-project/bb-prover/src/verifier/queued_chonk_verifier.ts index c7d62565fa6..7fa09b6a216 100644 --- a/yarn-project/bb-prover/src/verifier/queued_chonk_verifier.ts +++ b/yarn-project/bb-prover/src/verifier/queued_chonk_verifier.ts @@ -16,7 +16,8 @@ import { import { createHistogram } from 'node:perf_hooks'; -class IVCVerifierMetrics { +/** Records verification timing and failure metrics for an IVC (chonk) proof verifier. */ +export class IVCVerifierMetrics { private ivcVerificationHistogram: Histogram; private ivcTotalVerificationHistogram: Histogram; private ivcFailureCount: UpDownCounter; 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 3dfe0a0d94d..ecaa0f1fee7 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 @@ -38,6 +38,7 @@ import { getRPCEndpoint, hasDeployedHelmRelease, installChaosMeshChart, + logGossipTxValidationMetrics, setupEnvironment, startPortForwardForPrometeheus, uninstallChaosMesh, @@ -127,6 +128,15 @@ describe('sustained N TPS test', () => { afterAll(async () => { logger.info('Collecting benchmark metrics and cleaning up...'); + + // Log the gossip tx validation breakdown (per-stage timings, tx pool queue stats, chonk verifier + // timings) so slow gossip validations observed during the run can be attributed to a stage. + if (prometheusClient) { + await logGossipTxValidationMetrics(prometheusClient, config.NAMESPACE, TEST_DURATION_SECONDS + 60, logger).catch( + err => logger.warn(`Failed to scrape gossip validation metrics: ${err}`, { err }), + ); + } + if (process.env.BENCH_OUTPUT) { for (const topic of Object.values(TopicType)) { try { diff --git a/yarn-project/end-to-end/src/spartan/n_tps_prove.test.ts b/yarn-project/end-to-end/src/spartan/n_tps_prove.test.ts index 3fb22650cca..125e0b296d1 100644 --- a/yarn-project/end-to-end/src/spartan/n_tps_prove.test.ts +++ b/yarn-project/end-to-end/src/spartan/n_tps_prove.test.ts @@ -35,6 +35,7 @@ import { type ServiceEndpoint, getEthereumEndpoint, getExternalIP, + logGossipTxValidationMetrics, setupEnvironment, startPortForwardForPrometeheus, } from './utils.js'; @@ -148,6 +149,15 @@ describe(`prove ${TARGET_TPS}TPS test`, () => { server: new URL(`http://127.0.0.1:${freshPromForward.port}`), }); + // Log the gossip tx validation breakdown (per-stage timings, tx pool queue stats, chonk + // verifier timings) so slow gossip validations during the run can be attributed to a stage. + await logGossipTxValidationMetrics( + prometheusClient, + config.NAMESPACE, + epochDurationSeconds + SLOTS_BUFFER * slotDurationSeconds, + logger, + ).catch(err => logger.warn(`Failed to scrape gossip validation metrics: ${err}`, { err })); + const endSnapshot = await captureMetricsSnapshot(prometheusClient, logger); // Helper to compute delta, clamping negative values to 0 (handles pod restarts) diff --git a/yarn-project/end-to-end/src/spartan/utils/gossip_metrics.ts b/yarn-project/end-to-end/src/spartan/utils/gossip_metrics.ts new file mode 100644 index 00000000000..f151b072afc --- /dev/null +++ b/yarn-project/end-to-end/src/spartan/utils/gossip_metrics.ts @@ -0,0 +1,108 @@ +import type { Logger } from '@aztec/foundation/log'; + +import type { PrometheusClient } from '../../quality_of_service/prometheus_client.js'; + +/** Runs a PromQL query and returns each series as a label-record plus value. Returns [] on error. */ +async function queryVector( + prometheus: PrometheusClient, + query: string, + logger: Logger, +): Promise<{ labels: Record; value: number }[]> { + try { + const resp = await prometheus.queryRaw(query); + if (resp.status !== 'success' || resp.data.resultType !== 'vector') { + logger.warn(`Unexpected Prometheus response for query`, { query, resp }); + return []; + } + return resp.data.result.map(({ metric, value }) => ({ + labels: (metric ?? {}) as Record, + value: parseFloat(value[1]), + })); + } catch (err) { + logger.warn(`Failed to run Prometheus query: ${err}`, { query }); + return []; + } +} + +/** Formats a grouped vector result as a { labelValue: value } record for structured logging. */ +function toRecord(series: { labels: Record; value: number }[], label: string): Record { + const out: Record = {}; + for (const { labels, value } of series) { + out[labels[label] ?? 'unknown'] = Math.round(value * 100) / 100; + } + return out; +} + +/** + * Scrapes and logs gossip tx validation timing breakdowns from Prometheus: per-stage validation + * durations, tx pool serial queue wait/execution times, and chonk (IVC) proof verifier timings. + * Used by the spartan TPS benchmarks to attribute slow gossip validations to a specific stage. + */ +export async function logGossipTxValidationMetrics( + prometheus: PrometheusClient, + namespace: string, + windowSeconds: number, + logger: Logger, +): Promise { + const ns = `k8s_namespace_name="${namespace}"`; + const window = `[${Math.max(60, Math.ceil(windowSeconds))}s]`; + + const stageQuantile = (perc: string) => + `histogram_quantile(${perc}, sum(rate(aztec_p2p_gossip_tx_validation_stage_duration_milliseconds_bucket{${ns}}${window})) by (le, aztec_p2p_tx_validation_stage))`; + const stageAvg = () => + `sum(rate(aztec_p2p_gossip_tx_validation_stage_duration_milliseconds_sum{${ns}}${window})) by (aztec_p2p_tx_validation_stage) / ` + + `sum(rate(aztec_p2p_gossip_tx_validation_stage_duration_milliseconds_count{${ns}}${window})) by (aztec_p2p_tx_validation_stage)`; + const validationQuantile = (perc: string) => + `histogram_quantile(${perc}, sum(rate(aztec_p2p_gossip_message_validation_duration_milliseconds_bucket{${ns}}${window})) by (le, aztec_gossip_topic_name))`; + const queueQuantile = (metric: string, perc: string) => + `topk(10, histogram_quantile(${perc}, sum(rate(${metric}{${ns}}${window})) by (le, aztec_mempool_operation)))`; + const ivcQuantile = (metric: string, perc: string) => + `histogram_quantile(${perc}, sum(rate(${metric}{${ns}}${window})) by (le))`; + + const stageLabel = 'aztec_p2p_tx_validation_stage'; + const topicLabel = 'aztec_gossip_topic_name'; + const operationLabel = 'aztec_mempool_operation'; + + const [stageP50, stageP95, stageAvgs, validationP95, slowCount] = await Promise.all([ + queryVector(prometheus, stageQuantile('0.50'), logger), + queryVector(prometheus, stageQuantile('0.95'), logger), + queryVector(prometheus, stageAvg(), logger), + queryVector(prometheus, validationQuantile('0.95'), logger), + queryVector(prometheus, `sum(aztec_p2p_gossip_slow_validation_count{${ns}}) by (${topicLabel})`, logger), + ]); + + logger.info('Gossip tx validation stage timings (ms)', { + stageP50: toRecord(stageP50, stageLabel), + stageP95: toRecord(stageP95, stageLabel), + stageAvg: toRecord(stageAvgs, stageLabel), + validationP95ByTopic: toRecord(validationP95, topicLabel), + slowValidationCountByTopic: toRecord(slowCount, topicLabel), + }); + + const [queueWaitP95, queueExecutionP95, queueLengthMax] = await Promise.all([ + queryVector(prometheus, queueQuantile('aztec_mempool_tx_pool_v2_queue_wait_milliseconds_bucket', '0.95'), logger), + queryVector( + prometheus, + queueQuantile('aztec_mempool_tx_pool_v2_queue_execution_milliseconds_bucket', '0.95'), + logger, + ), + queryVector(prometheus, `max(max_over_time(aztec_mempool_tx_pool_v2_queue_length{${ns}}${window}))`, logger), + ]); + + logger.info('Tx pool serial queue stats (ms)', { + queueWaitP95: toRecord(queueWaitP95, operationLabel), + queueExecutionP95: toRecord(queueExecutionP95, operationLabel), + queueLengthMax: queueLengthMax[0]?.value, + }); + + const [ivcVerifyP95, ivcTotalP95] = await Promise.all([ + queryVector(prometheus, ivcQuantile('aztec_ivc_verifier_time_milliseconds_bucket', '0.95'), logger), + queryVector(prometheus, ivcQuantile('aztec_ivc_verifier_total_time_milliseconds_bucket', '0.95'), logger), + ]); + + // A large gap between total (queue + verify) and verify indicates a pile-up in the verifier queue. + logger.info('Chonk (IVC) proof verifier timings (ms)', { + verifyP95: ivcVerifyP95[0]?.value, + totalP95: ivcTotalP95[0]?.value, + }); +} diff --git a/yarn-project/end-to-end/src/spartan/utils/index.ts b/yarn-project/end-to-end/src/spartan/utils/index.ts index c9a05844c18..8d88da5a814 100644 --- a/yarn-project/end-to-end/src/spartan/utils/index.ts +++ b/yarn-project/end-to-end/src/spartan/utils/index.ts @@ -69,3 +69,6 @@ export { ChainHealth, type ChainHealthSnapshot } from './health.js'; // Pod log extraction export { type BlockBuiltLogEntry, fetchBlockBuiltLogs } from './pod_logs.js'; + +// Gossip validation metrics scraping +export { logGossipTxValidationMetrics } from './gossip_metrics.js'; diff --git a/yarn-project/foundation/src/iterable/first.test.ts b/yarn-project/foundation/src/iterable/first.test.ts new file mode 100644 index 00000000000..d23ebc9197c --- /dev/null +++ b/yarn-project/foundation/src/iterable/first.test.ts @@ -0,0 +1,40 @@ +import { first } from './index.js'; + +describe('first iterable', () => { + it('returns the first entry of a sync iterable', async () => { + await expect(first([3, 2, 1])).resolves.toEqual(3); + }); + + it('returns the first entry of an async iterable', async () => { + const generator = (async function* (): AsyncGenerator { + yield* [3, 2, 1]; + })(); + + await expect(first(generator)).resolves.toEqual(3); + }); + + it('returns undefined on an empty iterable', async () => { + await expect(first([])).resolves.toBeUndefined(); + await expect( + first( + (async function* (): AsyncGenerator { + /* yields nothing */ + })(), + ), + ).resolves.toBeUndefined(); + }); + + it('closes the underlying iterator after consuming the first entry', async () => { + let closed = false; + const generator = (async function* (): AsyncGenerator { + try { + yield* [3, 2, 1]; + } finally { + closed = true; + } + })(); + + await expect(first(generator)).resolves.toEqual(3); + expect(closed).toBe(true); + }); +}); diff --git a/yarn-project/foundation/src/iterable/first.ts b/yarn-project/foundation/src/iterable/first.ts new file mode 100644 index 00000000000..df86a4cfb11 --- /dev/null +++ b/yarn-project/foundation/src/iterable/first.ts @@ -0,0 +1,14 @@ +/** + * Returns the first entry of an iterable, or undefined if it yields none, closing the underlying + * iterator either way. Use this instead of calling `.next()` once and abandoning the iterator: + * an abandoned generator never runs its finally blocks, so any resource it holds is leaked — e.g. + * a kv-store iterator's LMDB cursor, where enough leaks deadlock the store. + */ +export async function first( + iterator: Iterable | AsyncIterableIterator | AsyncIterable | IterableIterator, +): Promise { + for await (const i of iterator) { + return i; + } + return undefined; +} diff --git a/yarn-project/foundation/src/iterable/index.ts b/yarn-project/foundation/src/iterable/index.ts index ca18f4de0be..69ac269ff25 100644 --- a/yarn-project/foundation/src/iterable/index.ts +++ b/yarn-project/foundation/src/iterable/index.ts @@ -3,5 +3,6 @@ export * from './filter.js'; export * from './sort.js'; export * from './take.js'; export * from './all.js'; +export * from './first.js'; export * from './peek.js'; export * from './toArray.js'; diff --git a/yarn-project/kv-store/src/deprecated/indexeddb/multi_map.ts b/yarn-project/kv-store/src/deprecated/indexeddb/multi_map.ts index c0ce3dcfd48..5cc0143eb6e 100644 --- a/yarn-project/kv-store/src/deprecated/indexeddb/multi_map.ts +++ b/yarn-project/kv-store/src/deprecated/indexeddb/multi_map.ts @@ -1,3 +1,5 @@ +import { first } from '@aztec/foundation/iterable'; + import { hash } from 'ohash'; import type { Key, Value } from '../../interfaces/common.js'; @@ -35,7 +37,7 @@ export class IndexedDBAztecMultiMap // Instead, we iterate in reverse order to get the last inserted entry const index = this.db.index('keyCount'); const rangeQuery = IDBKeyRange.upperBound([this.container, this.normalizeKey(key), Number.MAX_SAFE_INTEGER]); - const maxEntry = (await index.iterate(rangeQuery, 'prevunique').next()).value; + const maxEntry = await first(index.iterate(rangeQuery, 'prevunique')); const count = maxEntry?.value?.keyCount ?? 0; await this.db.put({ value: val, diff --git a/yarn-project/p2p/src/mem_pools/tx_pool_v2/archive/tx_archive.ts b/yarn-project/p2p/src/mem_pools/tx_pool_v2/archive/tx_archive.ts index baf7686356c..b23f6928aeb 100644 --- a/yarn-project/p2p/src/mem_pools/tx_pool_v2/archive/tx_archive.ts +++ b/yarn-project/p2p/src/mem_pools/tx_pool_v2/archive/tx_archive.ts @@ -1,3 +1,4 @@ +import { first } from '@aztec/foundation/iterable'; import { type Logger, createLogger } from '@aztec/foundation/log'; import type { AztecAsyncKVStore, AztecAsyncMap } from '@aztec/kv-store'; import { Tx, TxHash } from '@aztec/stdlib/tx'; @@ -46,8 +47,23 @@ export class TxArchive { * Archives transactions, stripping their proofs. * Evicts oldest transactions if the limit is exceeded. */ - async archiveTxs(txs: Tx[]): Promise { - if (!this.isEnabled() || txs.length === 0) { + archiveTxs(txs: Tx[]): Promise { + if (!this.isEnabled()) { + return Promise.resolve(); + } + + return this.archiveTxBuffers( + txs.map(tx => ({ txHash: tx.getTxHash().toString(), buffer: tx.withoutProof().toBuffer() })), + ); + } + + /** + * Archives already-serialized proof-less tx buffers, avoiding any deserialization. This is the + * hot path used at finalization time, where the pool already stores txs proof-stripped. + * Evicts oldest transactions if the limit is exceeded. + */ + async archiveTxBuffers(entries: { txHash: string; buffer: Buffer }[]): Promise { + if (!this.isEnabled() || entries.length === 0) { return; } @@ -57,7 +73,12 @@ export class TxArchive { let headIdx = await this.getHeadIndex(); let tailIdx = await this.getTailIndex(); - for (const tx of txs) { + for (const { txHash, buffer } of entries) { + // Skip txs that are already archived + if (await this.#txs.hasAsync(txHash)) { + continue; + } + // Evict oldest entries if at capacity while (headIdx - tailIdx >= this.#limit) { const txHashToEvict = await this.#indices.getAsync(tailIdx); @@ -68,15 +89,12 @@ export class TxArchive { tailIdx++; } - // Archive the transaction with stripped proof - const archivedTx = tx.withoutProof(); - const txHash = tx.getTxHash().toString(); - await this.#txs.set(txHash, archivedTx.toBuffer()); + await this.#txs.set(txHash, buffer); await this.#indices.set(headIdx, txHash); headIdx++; } - this.#log.debug(`Archived ${txs.length} txs, total: ${headIdx - tailIdx}`); + this.#log.debug(`Archived ${entries.length} txs, total: ${headIdx - tailIdx}`); }); } catch (error) { this.#log.error('Error archiving transactions', { error }); @@ -101,12 +119,12 @@ export class TxArchive { } private async getHeadIndex(): Promise { - const entry = await this.#indices.entriesAsync({ limit: 1, reverse: true }).next(); - return (entry.value?.[0] ?? -1) + 1; + const entry = await first(this.#indices.entriesAsync({ limit: 1, reverse: true })); + return entry === undefined ? 0 : entry[0] + 1; } private async getTailIndex(): Promise { - const entry = await this.#indices.entriesAsync({ limit: 1 }).next(); - return entry.value?.[0] ?? 0; + const entry = await first(this.#indices.entriesAsync({ limit: 1 })); + return entry === undefined ? 0 : entry[0]; } } diff --git a/yarn-project/p2p/src/mem_pools/tx_pool_v2/instrumentation.ts b/yarn-project/p2p/src/mem_pools/tx_pool_v2/instrumentation.ts index 6ec711bb826..9dac77baecf 100644 --- a/yarn-project/p2p/src/mem_pools/tx_pool_v2/instrumentation.ts +++ b/yarn-project/p2p/src/mem_pools/tx_pool_v2/instrumentation.ts @@ -1,5 +1,6 @@ import { Attributes, + type Histogram, type Meter, Metrics, type ObservableGauge, @@ -67,3 +68,29 @@ export class TxPoolV2Instrumentation { this.#missingPreviouslyEvictedCounter.add(count); } } + +/** + * Instrumentation for the tx pool serial queue: how long operations wait behind other queued work, + * how long they take to execute once running, and the current queue depth. All pool operations share + * a single serial queue, so contention here directly delays gossip tx validation. + */ +export class TxPoolQueueInstrumentation { + #queueWait: Histogram; + #queueExecution: Histogram; + + constructor(telemetry: TelemetryClient, getQueueLength: () => number) { + const meter: Meter = telemetry.getMeter('TxPoolQueue'); + this.#queueWait = meter.createHistogram(Metrics.MEMPOOL_TX_POOL_V2_QUEUE_WAIT); + this.#queueExecution = meter.createHistogram(Metrics.MEMPOOL_TX_POOL_V2_QUEUE_EXECUTION); + const queueLength = meter.createObservableGauge(Metrics.MEMPOOL_TX_POOL_V2_QUEUE_LENGTH); + queueLength.addCallback((result: ObservableResult) => { + result.observe(getQueueLength()); + }); + } + + record(operation: string, waitMs: number, executionMs: number) { + const attributes = { [Attributes.MEMPOOL_OPERATION]: operation }; + this.#queueWait.record(Math.ceil(waitMs), attributes); + this.#queueExecution.record(Math.ceil(executionMs), attributes); + } +} 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 b7094ec40b0..46b4a08b87c 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 @@ -1,7 +1,8 @@ import { SlotNumber } from '@aztec/foundation/branded-types'; +import { chunk } from '@aztec/foundation/collection'; import { type Logger, createLogger } from '@aztec/foundation/log'; import { SerialQueue } from '@aztec/foundation/queue'; -import { DateProvider } from '@aztec/foundation/timer'; +import { DateProvider, Timer } from '@aztec/foundation/timer'; import type { TypedEventEmitter } from '@aztec/foundation/types'; import type { AztecAsyncKVStore } from '@aztec/kv-store'; import type { L2Block, L2BlockId } from '@aztec/stdlib/block'; @@ -11,6 +12,7 @@ import { type TelemetryClient, getTelemetryClient } from '@aztec/telemetry-clien import EventEmitter from 'node:events'; import { PoolInstrumentation, PoolName } from '../instrumentation.js'; +import { TxPoolQueueInstrumentation } from './instrumentation.js'; import type { AddTxsResult, PoolReadAccess, @@ -21,7 +23,7 @@ import type { } from './interfaces.js'; import type { TxState } from './tx_metadata.js'; import type { MinedTxInfo } from './tx_pool_v2_impl.js'; -import { TxPoolV2Impl } from './tx_pool_v2_impl.js'; +import { FINALIZE_BLOCK_CHUNK_SIZE, TxPoolV2Impl } from './tx_pool_v2_impl.js'; /** * Implementation of TxPoolV2 with explicit state management. @@ -31,6 +33,9 @@ import { TxPoolV2Impl } from './tx_pool_v2_impl.js'; */ export class AztecKVTxPoolV2 extends (EventEmitter as new () => TypedEventEmitter) implements TxPoolV2 { #queue: SerialQueue; + #queueMetrics: TxPoolQueueInstrumentation; + /** Serializes finalizations so their chunked #queue items never interleave with each other. */ + #finalizationQueue = new SerialQueue(); #impl: TxPoolV2Impl; #metrics?: PoolInstrumentation; #store: AztecAsyncKVStore; @@ -53,6 +58,7 @@ export class AztecKVTxPoolV2 extends (EventEmitter as new () => TypedEventEmitte this.#telemetry = telemetry; this.#log = log; this.#queue = new SerialQueue(); + this.#queueMetrics = new TxPoolQueueInstrumentation(telemetry, () => this.#queue.length()); // Create callbacks that the impl uses to notify us about events and metrics const callbacks = { @@ -84,106 +90,146 @@ export class AztecKVTxPoolV2 extends (EventEmitter as new () => TypedEventEmitte // PUBLIC API - All methods queue to the implementation // ============================================================================ + /** + * Enqueues an operation on the serial queue, recording how long it waited behind other queued + * work and how long it took to execute once running. + */ + #run(operation: string, fn: () => T | Promise): Promise> { + const waitTimer = new Timer(); + return this.#queue.put(async () => { + const waitMs = waitTimer.ms(); + const executionTimer = new Timer(); + try { + return await fn(); + } finally { + this.#queueMetrics.record(operation, waitMs, executionTimer.ms()); + } + }); + } + // === Core Operations === addPendingTxs(txs: Tx[], opts: { source?: string; feeComparisonOnly?: boolean } = {}): Promise { - return this.#queue.put(() => this.#impl.addPendingTxs(txs, opts)); + return this.#run('addPendingTxs', () => this.#impl.addPendingTxs(txs, opts)); } canAddPendingTx(tx: Tx): Promise<'accepted' | 'ignored'> { - return this.#queue.put(() => this.#impl.canAddPendingTx(tx)); + return this.#run('canAddPendingTx', () => this.#impl.canAddPendingTx(tx)); } addProtectedTxs(txs: Tx[], block: BlockHeader, opts: { source?: string } = {}): Promise { - return this.#queue.put(() => this.#impl.addProtectedTxs(txs, block, opts)); + return this.#run('addProtectedTxs', () => this.#impl.addProtectedTxs(txs, block, opts)); } protectTxs(txHashes: TxHash[], block: BlockHeader): Promise { - return this.#queue.put(() => this.#impl.protectTxs(txHashes, block)); + return this.#run('protectTxs', () => this.#impl.protectTxs(txHashes, block)); } addMinedTxs(txs: Tx[], block: BlockHeader, opts: { source?: string } = {}): Promise { - return this.#queue.put(() => this.#impl.addMinedTxs(txs, block, opts)); + return this.#run('addMinedTxs', () => this.#impl.addMinedTxs(txs, block, opts)); } // === State Transition Handlers === handleMinedBlock(block: L2Block): Promise { - return this.#queue.put(() => this.#impl.handleMinedBlock(block)); + return this.#run('handleMinedBlock', () => this.#impl.handleMinedBlock(block)); } prepareForSlot(slotNumber: SlotNumber): Promise { - return this.#queue.put(() => this.#impl.prepareForSlot(slotNumber)); + return this.#run('prepareForSlot', () => this.#impl.prepareForSlot(slotNumber)); } unprotectTxs(txHashes: TxHash[], slotNumber: SlotNumber): Promise { - return this.#queue.put(() => this.#impl.unprotectTxs(txHashes, slotNumber)); + return this.#run('unprotectTxs', () => this.#impl.unprotectTxs(txHashes, slotNumber)); } handlePrunedBlocks(latestBlock: L2BlockId, options?: { deleteAllTxs?: boolean }): Promise { - return this.#queue.put(() => this.#impl.handlePrunedBlocks(latestBlock, options)); + return this.#run('handlePrunedBlocks', () => this.#impl.handlePrunedBlocks(latestBlock, options)); } handleFailedExecution(txHashes: TxHash[]): Promise { - return this.#queue.put(() => this.#impl.handleFailedExecution(txHashes)); + return this.#run('handleFailedExecution', () => this.#impl.handleFailedExecution(txHashes)); } + /** + * Handles a finalized block by archiving and deleting the mined txs it finalizes. The work is + * split into chunk-sized serial-queue items rather than one long-running item, so gossip-driven + * pool operations (canAddPendingTx / addPendingTxs) interleave with finalization instead of + * stalling behind an entire epoch's worth of tx processing. Finalizations run on their own serial + * queue so two concurrent calls never interleave their chunks with each other. + */ handleFinalizedBlock(block: BlockHeader): Promise { - return this.#queue.put(() => this.#impl.handleFinalizedBlock(block)); + return this.#finalizationQueue.put(() => this.#handleFinalizedBlock(block)); + } + + async #handleFinalizedBlock(block: BlockHeader): Promise { + const { cutoffBlock, txHashes } = await this.#run('prepareFinalization', () => + this.#impl.prepareFinalization(block), + ); + const batches = chunk(txHashes, FINALIZE_BLOCK_CHUNK_SIZE); + for (const batch of batches) { + await this.#run('archiveFinalizedTxs', () => this.#impl.archiveFinalizedTxs(batch)); + } + for (const batch of batches) { + await this.#run('deleteFinalizedTxs', () => this.#impl.deleteFinalizedTxs(batch, cutoffBlock)); + } + await this.#run('completeFinalization', () => + this.#impl.completeFinalization(txHashes, cutoffBlock, block.globalVariables.blockNumber), + ); } // === Queries === getTxByHash(txHash: TxHash, opts?: { includeProof?: boolean }): Promise { - return this.#queue.put(() => this.#impl.getTxByHash(txHash, opts)); + return this.#run('getTxByHash', () => this.#impl.getTxByHash(txHash, opts)); } getTxsByHash(txHashes: TxHash[], opts?: { includeProof?: boolean }): Promise<(Tx | undefined)[]> { - return this.#queue.put(() => this.#impl.getTxsByHash(txHashes, opts)); + return this.#run('getTxsByHash', () => this.#impl.getTxsByHash(txHashes, opts)); } hasTxs(txHashes: TxHash[]): Promise { - return this.#queue.put(() => this.#impl.hasTxs(txHashes)); + return this.#run('hasTxs', () => this.#impl.hasTxs(txHashes)); } getTxStatus(txHash: TxHash): Promise { - return this.#queue.put(() => Promise.resolve(this.#impl.getTxStatus(txHash))); + return this.#run('getTxStatus', () => this.#impl.getTxStatus(txHash)); } getPendingTxHashes(): Promise { - return this.#queue.put(() => Promise.resolve(this.#impl.getPendingTxHashes())); + return this.#run('getPendingTxHashes', () => this.#impl.getPendingTxHashes()); } getEligiblePendingTxHashes(): Promise { - return this.#queue.put(() => Promise.resolve(this.#impl.getEligiblePendingTxHashes())); + return this.#run('getEligiblePendingTxHashes', () => this.#impl.getEligiblePendingTxHashes()); } getPendingTxCount(): Promise { - return this.#queue.put(() => Promise.resolve(this.#impl.getPendingTxCount())); + return this.#run('getPendingTxCount', () => this.#impl.getPendingTxCount()); } hasEligiblePendingTxs(minCount: number): Promise { - return this.#queue.put(() => Promise.resolve(this.#impl.hasEligiblePendingTxs(minCount))); + return this.#run('hasEligiblePendingTxs', () => this.#impl.hasEligiblePendingTxs(minCount)); } getMinedTxHashes(): Promise<[TxHash, L2BlockId][]> { - return this.#queue.put(() => Promise.resolve(this.#impl.getMinedTxHashes())); + return this.#run('getMinedTxHashes', () => this.#impl.getMinedTxHashes()); } getMinedTxCount(): Promise { - return this.#queue.put(() => Promise.resolve(this.#impl.getMinedTxCount())); + return this.#run('getMinedTxCount', () => this.#impl.getMinedTxCount()); } isEmpty(): Promise { - return this.#queue.put(() => Promise.resolve(this.#impl.isEmpty())); + return this.#run('isEmpty', () => this.#impl.isEmpty()); } getArchivedTxByHash(txHash: TxHash): Promise { - return this.#queue.put(() => this.#impl.getArchivedTxByHash(txHash)); + return this.#run('getArchivedTxByHash', () => this.#impl.getArchivedTxByHash(txHash)); } getLowestPriorityPending(limit: number): Promise { - return this.#queue.put(() => Promise.resolve(this.#impl.getLowestPriorityPending(limit))); + return this.#run('getLowestPriorityPending', () => this.#impl.getLowestPriorityPending(limit)); } /** Returns read-only access to the pool. Used for testing. */ @@ -212,8 +258,9 @@ export class AztecKVTxPoolV2 extends (EventEmitter as new () => TypedEventEmitte this.#log.info('Starting transaction pool...'); - // Start the serial queue + // Start the serial queues this.#queue.start(); + this.#finalizationQueue.start(); this.#started = true; // Setup metrics - created after queue is started so callbacks can safely queue @@ -245,6 +292,8 @@ export class AztecKVTxPoolV2 extends (EventEmitter as new () => TypedEventEmitte async stop(): Promise { if (this.#started) { + // End the finalization queue first: an in-flight finalization still needs #queue to run its chunks. + await this.#finalizationQueue.end(); await this.#queue.end(); this.#started = false; } diff --git a/yarn-project/p2p/src/mem_pools/tx_pool_v2/tx_pool_v2_finalization.test.ts b/yarn-project/p2p/src/mem_pools/tx_pool_v2/tx_pool_v2_finalization.test.ts new file mode 100644 index 00000000000..5a7fe1042c4 --- /dev/null +++ b/yarn-project/p2p/src/mem_pools/tx_pool_v2/tx_pool_v2_finalization.test.ts @@ -0,0 +1,218 @@ +import { BlockNumber, CheckpointNumber, IndexWithinCheckpoint, SlotNumber } from '@aztec/foundation/branded-types'; +import { Fr } from '@aztec/foundation/curves/bn254'; +import { createLogger } from '@aztec/foundation/log'; +import { promiseWithResolvers } from '@aztec/foundation/promise'; +import { Timer } from '@aztec/foundation/timer'; +import { openTmpStore } from '@aztec/kv-store/lmdb-v2'; +import { RevertCode } from '@aztec/stdlib/avm'; +import { AztecAddress } from '@aztec/stdlib/aztec-address'; +import { Body, L2Block, type L2BlockSource } from '@aztec/stdlib/block'; +import { GasFees } from '@aztec/stdlib/gas'; +import type { MerkleTreeReadOperations, WorldStateSynchronizer } from '@aztec/stdlib/interfaces/server'; +import { mockTx } from '@aztec/stdlib/testing'; +import { + AppendOnlyTreeSnapshot, + MerkleTreeId, + PublicDataTreeLeaf, + PublicDataTreeLeafPreimage, +} from '@aztec/stdlib/trees'; +import { BlockHeader, GlobalVariables, type Tx, TxEffect, type TxValidator } from '@aztec/stdlib/tx'; + +import { jest } from '@jest/globals'; +import { type MockProxy, mock } from 'jest-mock-extended'; + +import type { TxMetaData } from './tx_metadata.js'; +import { AztecKVTxPoolV2 } from './tx_pool_v2.js'; +import { TxPoolV2Impl } from './tx_pool_v2_impl.js'; + +const alwaysValidValidator: TxValidator = { + validateTx: () => Promise.resolve({ result: 'valid' }), +}; + +jest.setTimeout(300_000); + +// Reproduction for A-1656: gossip tx validation on mainnet stalls for 10-40s whenever the pool +// finalizes an epoch's worth of mined txs. Finalization runs as a single serial-queue item, so a +// concurrent canAddPendingTx / addPendingTxs (issued by gossip validation) waits for the whole +// bulk operation to complete. This test measures that wait and bounds it. +describe('TxPoolV2 finalization stall', () => { + const logger = createLogger('p2p:tx_pool_v2:finalization_stall_test'); + + // Matches the tx counts seen finalized per epoch tick on mainnet (~100-200). + const MINED_TX_COUNT = 128; + // Upper bound for how long a gossip-validation pool operation may wait behind maintenance work. + const MAX_STALL_MS = 1_000; + + const feePayers = [ + AztecAddress.fromBigIntUnsafe(1n), + AztecAddress.fromBigIntUnsafe(2n), + AztecAddress.fromBigIntUnsafe(3n), + ]; + + let mockL2BlockSource: MockProxy; + let mockWorldState: MockProxy; + let db: MockProxy; + let minedTxs: Tx[]; + let incomingTx: Tx; + + const makeHeader = (n: number) => + BlockHeader.empty({ + globalVariables: GlobalVariables.empty({ + blockNumber: BlockNumber(n), + slotNumber: SlotNumber(n), + }), + }); + + const makeBlock = (txs: Tx[], header: BlockHeader): L2Block => { + const txEffects = txs.map(tx => { + const nullifiers = tx.data.getNonEmptyNullifiers(); + return new TxEffect(RevertCode.OK, tx.getTxHash(), Fr.ZERO, [], nullifiers, [], [], [], [], []); + }); + const body = new Body(txEffects); + const archive = new AppendOnlyTreeSnapshot(Fr.random(), header.globalVariables.blockNumber + 1); + return new L2Block( + archive, + header, + body, + CheckpointNumber(Number(header.globalVariables.blockNumber)), + IndexWithinCheckpoint(0), + ); + }; + + // Large public calldata mimics the complex public-heavy txs seen on mainnet when the stalls occurred. + const createTxBatch = (count: number, startSeed = 1): Promise => + Promise.all( + Array.from({ length: count }, (_, i) => + mockTx((startSeed + i) * 100, { + publicCalldataSize: 1000, + maxPriorityFeesPerGas: new GasFees(((startSeed + i) % 100) + 1, ((startSeed + i) % 100) + 1), + feePayer: feePayers[(startSeed + i) % feePayers.length], + }), + ), + ); + + const createPool = async (archivedTxLimit: number) => { + const store = await openTmpStore('p2p-finalization-stall'); + const archiveStore = await openTmpStore('archive-finalization-stall'); + const pool = new AztecKVTxPoolV2( + store, + archiveStore, + { + l2BlockSource: mockL2BlockSource, + worldStateSynchronizer: mockWorldState, + createTxValidator: () => Promise.resolve(alwaysValidValidator), + checkAllowedSetupCalls: () => Promise.resolve(true), + blockMinFeesProvider: { getCurrentMinFees: () => Promise.resolve(GasFees.empty()) }, + }, + undefined, + { archivedTxLimit }, + ); + await pool.start(); + const cleanup = async () => { + await pool.stop(); + await store.delete(); + await archiveStore.delete(); + }; + return { pool, cleanup }; + }; + + beforeAll(async () => { + minedTxs = await createTxBatch(MINED_TX_COUNT); + [incomingTx] = await createTxBatch(1, (MINED_TX_COUNT + 1) * 100); + logger.info(`Created ${MINED_TX_COUNT} mined txs of ${minedTxs[0].toBuffer().length} bytes each`); + }); + + beforeEach(() => { + mockL2BlockSource = mock(); + mockL2BlockSource.getTxEffect.mockResolvedValue(undefined); + + mockWorldState = mock(); + db = mock(); + mockWorldState.getCommitted.mockReturnValue(db); + mockWorldState.getSnapshot.mockReturnValue(db); + db.getPreviousValueIndex.mockImplementation((_tree, slot) => + Promise.resolve({ index: slot, alreadyPresent: true }), + ); + db.getLeafPreimage.mockImplementation((tree, index) => + Promise.resolve( + tree === MerkleTreeId.PUBLIC_DATA_TREE + ? new PublicDataTreeLeafPreimage( + new PublicDataTreeLeaf(new Fr(index), new Fr(BigInt('1000000000000000000000000'))), + Fr.ONE, + 1n, + ) + : undefined, + ), + ); + db.findLeafIndices.mockImplementation((_tree, leaves) => + Promise.resolve((leaves as Fr[]).map((_, i) => BigInt(i + 1))), + ); + }); + + // Fills the pool with mined txs, kicks off finalization, and issues the pool calls that gossip + // validation depends on once finalization is executing on the serial queue. handleFinalizedBlock + // defers its first queue item to a later microtask, so without gating on an archive chunk having + // started the gossip ops would enter the queue ahead of finalization and measure nothing. + // Returns how long each waited, the finalize time, and which of the two finished first. + const measureStallDuringFinalization = async (archivedTxLimit: number) => { + const { pool, cleanup } = await createPool(archivedTxLimit); + // Resolved when the first archive chunk starts executing, i.e. finalization holds the queue. + // archiveFinalizedTxs runs per chunk even when archiving is disabled. + const archiveChunkStarted = promiseWithResolvers(); + const originalArchiveFinalizedTxs = TxPoolV2Impl.prototype.archiveFinalizedTxs; + const archiveSpy = jest.spyOn(TxPoolV2Impl.prototype, 'archiveFinalizedTxs').mockImplementation(function ( + this: TxPoolV2Impl, + txHashes: string[], + ) { + archiveChunkStarted.resolve(); + return originalArchiveFinalizedTxs.call(this, txHashes); + }); + try { + await pool.addPendingTxs(minedTxs); + await pool.handleMinedBlock(makeBlock(minedTxs, makeHeader(1))); + + const finalizeTimer = new Timer(); + const finalizeDone = pool.handleFinalizedBlock(makeHeader(1)).then(() => 'finalization' as const); + + await archiveChunkStarted.promise; + + const precheckTimer = new Timer(); + const precheckPromise = pool.canAddPendingTx(incomingTx).then(() => precheckTimer.ms()); + + const addTimer = new Timer(); + const addPromise = pool.addPendingTxs([incomingTx], { source: 'gossip' }).then(() => addTimer.ms()); + + const gossipDone = Promise.all([precheckPromise, addPromise]).then(() => 'gossip ops' as const); + const finishedFirst = await Promise.race([gossipDone, finalizeDone]); + + const [precheckMs, addMs] = await Promise.all([precheckPromise, addPromise]); + await finalizeDone; + const finalizeMs = finalizeTimer.ms(); + + logger.info( + `Finalized ${MINED_TX_COUNT} txs in ${Math.round(finalizeMs)}ms with archivedTxLimit=${archivedTxLimit} ` + + `(canAddPendingTx waited ${Math.round(precheckMs)}ms, addPendingTxs waited ${Math.round(addMs)}ms, ` + + `${finishedFirst} finished first)`, + { finalizeMs, precheckMs, addMs, archivedTxLimit, finishedFirst }, + ); + return { finalizeMs, precheckMs, addMs, finishedFirst }; + } finally { + archiveSpy.mockRestore(); + await cleanup(); + } + }; + + it('does not stall gossip pool operations while finalizing mined txs with archiving disabled', async () => { + const { precheckMs, addMs, finishedFirst } = await measureStallDuringFinalization(0); + expect(finishedFirst).toEqual('gossip ops'); + expect(precheckMs).toBeLessThan(MAX_STALL_MS); + expect(addMs).toBeLessThan(MAX_STALL_MS); + }); + + it('does not stall gossip pool operations while finalizing mined txs with archiving enabled', async () => { + const { precheckMs, addMs, finishedFirst } = await measureStallDuringFinalization(10_000); + expect(finishedFirst).toEqual('gossip ops'); + expect(precheckMs).toBeLessThan(MAX_STALL_MS); + expect(addMs).toBeLessThan(MAX_STALL_MS); + }); +}); 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 ddd014c4955..15b31242e77 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 @@ -42,11 +42,12 @@ import { type TxMetaData, type TxState, buildTxMetaData, checkNullifierConflict import { TxPoolIndices } from './tx_pool_indices.js'; /** - * Maximum number of full transactions to load into memory at once when finalizing a block. - * Bounds peak memory while archiving and hard-deleting mined txs (~23k txs/epoch at 10 TPS would - * otherwise OOM the node). + * Maximum number of finalized txs to archive or hard-delete per serial-queue item. Bounds both + * peak memory (~23k txs/epoch at 10 TPS would otherwise OOM the node) and, more importantly, how + * long finalization occupies the pool's serial queue per item: gossip tx validation waits on that + * queue, so each item must stay well under the gossipsub mcache eviction window. */ -const FINALIZE_BLOCK_CHUNK_SIZE = 100; +export const FINALIZE_BLOCK_CHUNK_SIZE = 16; /** * Callbacks for the implementation to notify the outer class about events and metrics. @@ -725,45 +726,71 @@ export class TxPoolV2Impl { this.#log.info(`Deleted ${txHashes.length} failed txs`, { txHashes: txHashes.map(h => h.toString()) }); } - async handleFinalizedBlock(block: BlockHeader): Promise { - const blockNumber = block.globalVariables.blockNumber; - + /** + * Resolves what a finalized block event should process: the cutoff block and the mined txs at or + * before it. The wrapper feeds the result to archiveFinalizedTxs / finalizeTxs in chunks, each as + * its own serial-queue item, so gossip-driven pool operations can interleave with finalization + * instead of stalling behind an entire epoch's worth of tx processing. + */ + async prepareFinalization(block: BlockHeader): Promise<{ cutoffBlock: BlockNumber; txHashes: string[] }> { // Hold finalized txs for a configurable margin behind the finalized tip so a prover still // proving an epoch with already-finalized blocks isn't starved of its txs. 0 deletes at the finalized tip. const cutoffBlock = await this.#finalizationCutoffBlock(block); + return { cutoffBlock, txHashes: this.#indices.findTxsMinedAtOrBefore(cutoffBlock) }; + } - // Step 1: Find mined txs at or before the cutoff block - const minedTxsToFinalize = this.#indices.findTxsMinedAtOrBefore(cutoffBlock); - - // Step 2: Archive in chunks if archiving is enabled. Hydrating an entire epoch's worth of - // mined txs at once would OOM under load. When archiving is disabled there is no need to hydrate the txs at all. - if (this.#archive.isEnabled()) { - for (let i = 0; i < minedTxsToFinalize.length; i += FINALIZE_BLOCK_CHUNK_SIZE) { - const chunk = minedTxsToFinalize.slice(i, i + FINALIZE_BLOCK_CHUNK_SIZE); - const txsToArchive: Tx[] = []; - for (const txHashStr of chunk) { - const buffer = await this.#txsDB.getAsync(txHashStr); - if (buffer) { - txsToArchive.push(Tx.fromBuffer(buffer)); - } - } - if (txsToArchive.length > 0) { - await this.#archive.archiveTxs(txsToArchive); - } + /** + * Copies the given finalized txs into the archive. The pool stores txs proof-stripped, which is + * exactly the archive format, so buffers are copied as-is without deserialization. + */ + async archiveFinalizedTxs(txHashes: string[]): Promise { + if (!this.#archive.isEnabled()) { + return; + } + const entries: { txHash: string; buffer: Buffer }[] = []; + for (const txHash of txHashes) { + const buffer = await this.#txsDB.getAsync(txHash); + if (buffer) { + entries.push({ txHash, buffer }); } } + await this.#archive.archiveTxBuffers(entries); + } + + /** + * Deletes a batch of finalized mined txs from the active pool. Callers may invoke this in + * chunks: a crash between chunks leaves some finalized txs mined, and the next finalized-block + * event lists and deletes them again (the operation is idempotent), so per-chunk atomicity is + * sufficient. Since other pool operations may interleave between the finalization plan being + * computed and this call, each tx is re-checked to still be mined at or before the cutoff. + */ + async deleteFinalizedTxs(txHashes: string[], cutoffBlock: BlockNumber): Promise { + const stillFinalized = txHashes.filter(txHash => { + const minedBlock = this.#indices.getMetadata(txHash)?.minedL2BlockId?.number; + return minedBlock !== undefined && minedBlock <= cutoffBlock; + }); + if (stillFinalized.length === 0) { + return; + } + await this.#store.transactionAsync(async () => { + await this.#deleteTxsBatch(stillFinalized); + }); + } - // Step 3: Delete mined txs from the active pool and finalize soft-deleted txs in one - // transaction. Only tx hashes are touched here, so memory is bounded and atomicity is preserved. + /** Finalizes soft-deleted txs up to the cutoff and logs the completed finalization. */ + async completeFinalization( + txHashes: string[], + cutoffBlock: BlockNumber, + finalizedBlockNumber: BlockNumber, + ): Promise { await this.#store.transactionAsync(async () => { - await this.#deleteTxsBatch(minedTxsToFinalize); await this.#deletedPool.finalizeBlock(cutoffBlock); }); - if (minedTxsToFinalize.length > 0) { - this.#log.info(`Finalized ${minedTxsToFinalize.length} mined txs from blocks up to ${cutoffBlock}`, { - txHashes: minedTxsToFinalize, - finalizedBlockNumber: blockNumber, + if (txHashes.length > 0) { + this.#log.info(`Finalized ${txHashes.length} mined txs from blocks up to ${cutoffBlock}`, { + txHashes, + finalizedBlockNumber, cutoffBlock, }); } diff --git a/yarn-project/p2p/src/services/libp2p/instrumentation.ts b/yarn-project/p2p/src/services/libp2p/instrumentation.ts index 7ec5e0a9a89..c0be0c07f1e 100644 --- a/yarn-project/p2p/src/services/libp2p/instrumentation.ts +++ b/yarn-project/p2p/src/services/libp2p/instrumentation.ts @@ -19,6 +19,7 @@ export class P2PInstrumentation { private messageLatency: Histogram; private txReceivedCount: UpDownCounter; private slowValidationCount: UpDownCounter; + private txValidationStageDuration: Histogram; private aggLatencyHisto = new Map(); private aggValidationHisto = new Map(); @@ -47,6 +48,8 @@ export class P2PInstrumentation { this.messageLatency = meter.createHistogram(Metrics.P2P_GOSSIP_MESSAGE_LATENCY); + this.txValidationStageDuration = meter.createHistogram(Metrics.P2P_GOSSIP_TX_VALIDATION_STAGE_DURATION); + this.txReceivedCount = createUpDownCounterWithDefault(meter, Metrics.P2P_GOSSIP_TX_RECEIVED_COUNT); this.slowValidationCount = createUpDownCounterWithDefault(meter, Metrics.P2P_GOSSIP_SLOW_VALIDATION_COUNT, { @@ -101,6 +104,11 @@ export class P2PInstrumentation { this.slowValidationCount.add(1, { [Attributes.TOPIC_NAME]: topicName }); } + /** Records the duration of a single stage of gossiped tx validation. */ + public recordTxValidationStage(stage: string, ms: number) { + this.txValidationStageDuration.record(Math.ceil(ms), { [Attributes.TX_VALIDATION_STAGE]: stage }); + } + public incMessagePrevalidationStatus(passed: boolean, topicName: TopicType | undefined) { this.messagePrevalidationCount.add(1, { [Attributes.TOPIC_NAME]: topicName, [Attributes.OK]: passed }); } diff --git a/yarn-project/p2p/src/services/libp2p/libp2p_service.ts b/yarn-project/p2p/src/services/libp2p/libp2p_service.ts index ddeb5d15901..53daa419f36 100644 --- a/yarn-project/p2p/src/services/libp2p/libp2p_service.ts +++ b/yarn-project/p2p/src/services/libp2p/libp2p_service.ts @@ -1007,6 +1007,7 @@ export class LibP2PService extends WithTracer implements P2PService { msgId: string, source: PeerId, topicType: TopicType, + stageTimings?: Record, ): Promise> { // Default to reject result with a penalty if validation function throws an error let resultAndObj: ReceivedMessageValidationResult = { @@ -1027,12 +1028,18 @@ export class LibP2PService extends WithTracer implements P2PService { this.logger.warn( `Gossip validation for ${topicType} took ${validationTimeMs}ms, approaching mcache eviction window of ${mcacheWindowMs}ms. ` + `Message forwarding may be skipped if validation exceeds the window.`, - { msgId, source: source.toString(), topicType, validationTimeMs, mcacheWindowMs }, + { msgId, source: source.toString(), topicType, validationTimeMs, mcacheWindowMs, stageTimings }, ); } if (resultAndObj.result === TopicValidatorResult.Accept) { - this.logger.debug(`Message ${topicType} accepted by validator`, { msgId, source: source.toString(), topicType }); + this.logger.debug(`Message ${topicType} accepted by validator`, { + msgId, + source: source.toString(), + topicType, + validationTimeMs, + stageTimings, + }); this.instrumentation.recordMessageValidation(topicType, timer); } else if (resultAndObj.result === TopicValidatorResult.Reject) { this.logger.warn(`Message ${topicType} rejected by validator with severity ${resultAndObj.severity}`, { @@ -1064,18 +1071,38 @@ export class LibP2PService extends WithTracer implements P2PService { } protected async handleGossipedTx(payloadData: Buffer, msgId: string, source: PeerId) { + // Per-stage timings so slow validations can be attributed to a specific stage (see A-1656). + const stageTimings: Record = {}; + const timed = async (stage: string, fn: () => T | Promise): Promise => { + const timer = new Timer(); + try { + return await fn(); + } finally { + const ms = timer.ms(); + stageTimings[stage] = Math.ceil(ms); + this.instrumentation.recordTxValidationStage(stage, ms); + } + }; + const validationFunc: () => Promise> = async () => { - const tx = this.tryDeserialize(() => Tx.fromBuffer(payloadData), msgId, source); + const tx = await timed('deserialize', () => this.tryDeserialize(() => Tx.fromBuffer(payloadData), msgId, source)); if (!tx) { return { result: TopicValidatorResult.Reject, severity: PeerErrorSeverity.LowToleranceError }; } - const currentBlockNumber = await this.archiver.getBlockNumber(); - const { ts: nextSlotTimestamp } = this.epochCache.getEpochAndSlotInNextL1Slot(); + // Stage 1 setup: chain tip, epoch info, gas fees, and L1 constants reads + const { currentBlockNumber, firstStageValidators } = await timed('fast_validation_setup', async () => { + const currentBlockNumber = await this.archiver.getBlockNumber(); + const { ts: nextSlotTimestamp } = this.epochCache.getEpochAndSlotInNextL1Slot(); + const firstStageValidators = await this.createFirstStageMessageValidators( + currentBlockNumber, + nextSlotTimestamp, + ); + return { currentBlockNumber, firstStageValidators }; + }); // Stage 1: fast validators (metadata, data, timestamps, double-spend, gas, phases, block header) - const firstStageValidators = await this.createFirstStageMessageValidators(currentBlockNumber, nextSlotTimestamp); - const firstStageOutcome = await this.runValidations(tx, firstStageValidators); + const firstStageOutcome = await timed('fast_validation', () => this.runValidations(tx, firstStageValidators)); if (!firstStageOutcome.allPassed) { const { name } = firstStageOutcome.failure; let { severity } = firstStageOutcome.failure; @@ -1097,7 +1124,7 @@ export class LibP2PService extends WithTracer implements P2PService { } // Pool pre-check: see if the pool would accept this tx before doing expensive proof verification - const canAdd = await this.mempools.txPool.canAddPendingTx(tx); + const canAdd = await timed('pool_precheck', () => this.mempools.txPool.canAddPendingTx(tx)); if (canAdd === 'ignored') { this.logger.verbose(`Ignoring gossiped tx ${tx.getTxHash().toString()}: pool pre-check returned ignored`, { source: source.toString(), @@ -1107,7 +1134,7 @@ export class LibP2PService extends WithTracer implements P2PService { // Stage 2: expensive proof verification const secondStageValidators = this.createSecondStageMessageValidators(); - const secondStageOutcome = await this.runValidations(tx, secondStageValidators); + const secondStageOutcome = await timed('proof_verify', () => this.runValidations(tx, secondStageValidators)); if (!secondStageOutcome.allPassed) { const { severity, name } = secondStageOutcome.failure; this.logger.verbose(`Rejecting gossiped tx ${tx.getTxHash().toString()}: stage 2 validation failed`, { @@ -1120,7 +1147,7 @@ export class LibP2PService extends WithTracer implements P2PService { // Pool add: persist the tx const txHash = tx.getTxHash(); - const addResult = await this.mempools.txPool.addPendingTxs([tx], { source: 'gossip' }); + const addResult = await timed('pool_add', () => this.mempools.txPool.addPendingTxs([tx], { source: 'gossip' })); const wasAccepted = addResult.accepted.some(h => h.equals(txHash)); const wasIgnored = addResult.ignored.some(h => h.equals(txHash)); @@ -1144,7 +1171,13 @@ export class LibP2PService extends WithTracer implements P2PService { } }; - const { result, obj: tx } = await this.validateReceivedMessage(validationFunc, msgId, source, TopicType.tx); + const { result, obj: tx } = await this.validateReceivedMessage( + validationFunc, + msgId, + source, + TopicType.tx, + stageTimings, + ); if (result !== TopicValidatorResult.Accept || !tx) { return; } diff --git a/yarn-project/telemetry-client/src/attributes.ts b/yarn-project/telemetry-client/src/attributes.ts index 8a262c03efe..e934bf03c87 100644 --- a/yarn-project/telemetry-client/src/attributes.ts +++ b/yarn-project/telemetry-client/src/attributes.ts @@ -133,6 +133,12 @@ export const NODEJS_EVENT_LOOP_STATE = 'nodejs.eventloop.state'; export const TOPIC_NAME = 'aztec.gossip.topic_name'; +/** Stage of gossiped tx validation (deserialize, fast_validation_setup, fast_validation, pool_precheck, proof_verify, pool_add) */ +export const TX_VALIDATION_STAGE = 'aztec.p2p.tx_validation_stage'; + +/** Operation type enqueued on the mempool serial queue */ +export const MEMPOOL_OPERATION = 'aztec.mempool.operation'; + /** The reason a transaction was evicted from the tx pool */ export const TX_POOL_EVICTION_REASON = 'aztec.mempool.eviction_reason'; diff --git a/yarn-project/telemetry-client/src/metrics.ts b/yarn-project/telemetry-client/src/metrics.ts index 6255f44aeda..1bdb0217a68 100644 --- a/yarn-project/telemetry-client/src/metrics.ts +++ b/yarn-project/telemetry-client/src/metrics.ts @@ -204,6 +204,24 @@ export const MEMPOOL_TX_POOL_V2_METADATA_MEMORY: MetricDefinition = { valueType: ValueType.INT, }; +export const MEMPOOL_TX_POOL_V2_QUEUE_WAIT: MetricDefinition = { + name: 'aztec.mempool.tx_pool_v2.queue_wait', + description: 'Time an operation waits in the tx pool serial queue before executing, keyed by mempool.operation', + unit: 'ms', + valueType: ValueType.INT, +}; +export const MEMPOOL_TX_POOL_V2_QUEUE_EXECUTION: MetricDefinition = { + name: 'aztec.mempool.tx_pool_v2.queue_execution', + description: 'Time an operation spends executing in the tx pool serial queue, keyed by mempool.operation', + unit: 'ms', + valueType: ValueType.INT, +}; +export const MEMPOOL_TX_POOL_V2_QUEUE_LENGTH: MetricDefinition = { + name: 'aztec.mempool.tx_pool_v2.queue_length', + description: 'Number of operations waiting in the tx pool serial queue', + valueType: ValueType.INT, +}; + export const MEMPOOL_TX_POOL_V2_DUPLICATE_ADD: MetricDefinition = { name: 'aztec.mempool.tx_pool_v2.duplicate_add', description: 'Transactions received via addPendingTxs that were already in the pool', @@ -1022,6 +1040,13 @@ export const P2P_GOSSIP_SLOW_VALIDATION_COUNT: MetricDefinition = { valueType: ValueType.INT, }; +export const P2P_GOSSIP_TX_VALIDATION_STAGE_DURATION: MetricDefinition = { + name: 'aztec.p2p.gossip.tx_validation_stage_duration', + description: 'Duration of each stage of gossiped tx validation, keyed by the tx_validation_stage attribute', + unit: 'ms', + valueType: ValueType.INT, +}; + export const PUBLIC_PROCESSOR_TX_DURATION: MetricDefinition = { name: 'aztec.public_processor.tx_duration', description: 'Duration to process a public transaction',