fix(p2p): move gas-limit admission out of GasTxValidator

yarn-project/p2p/src/mem_pools/tx_pool_v2/tx_pool_v2.test.ts

@@ -32,7 +32,8 @@ import { getTelemetryClient } from '@aztec/telemetry-client';
 import { type MockProxy, mock } from 'jest-mock-extended';
 
 import { AggregateTxValidator } from '../../msg_validators/tx_validator/aggregate_tx_validator.js';
-import { GasLimitsValidator, MaxFeePerGasValidator } from '../../msg_validators/tx_validator/gas_validator.js';
+import { GasLimitsValidator } from '../../msg_validators/tx_validator/gas_limits_validator.js';
+import { MaxFeePerGasValidator } from '../../msg_validators/tx_validator/gas_validator.js';
 import { AllowedSetupCallsMetaValidator } from '../../msg_validators/tx_validator/phases_validator.js';
 import type { TxMetaData } from './tx_metadata.js';
 import { AztecKVTxPoolV2 } from './tx_pool_v2.js';

yarn-project/p2p/src/msg_validators/tx_validator/README.md

@@ -19,7 +19,7 @@ Unsolicited transactions from any peer. Fully validated in two stages with a poo
 
 | Step | What runs | On failure |
 |------|-----------|------------|
-| **Stage 1** (fast) | TxPermitted, Data, Metadata, Timestamp, DoubleSpend, Gas, Phases, BlockHeader | Penalize peer, reject tx |
+| **Stage 1** (fast) | TxPermitted, Data, Metadata, Timestamp, DoubleSpend, GasLimits, Gas, Phases, BlockHeader | Penalize peer, reject tx |
 | **Pool pre-check** | `canAddPendingTx` — checks for duplicates, pool capacity | Ignore tx (no penalty) |
 | **Stage 2** (slow) | Proof verification | Penalize peer, reject tx |
 | **Pool add** | `addPendingTxs` | Accept, ignore, or reject |
@@ -34,6 +34,7 @@ Each stage-1 and stage-2 validator is paired with a `PeerErrorSeverity`. If a va
 Unsolicited transactions from a local wallet/PXE. Runs the full set of checks as a single aggregate validator:
 
 - TxPermitted, Size, Data, Metadata, Timestamp, DoubleSpend, Phases, BlockHeader
+- GasLimits (skipped for simulations — gas estimation submits limits above the per-tx maximum)
 - Gas (optional — skipped when `skipFeeEnforcement` is set)
 - Proof verification (optional — skipped for simulations when no verifier is provided)
 
@@ -56,7 +57,7 @@ State-dependent checks are deferred to either the block building validator (for
 Transactions already in the pool, about to be sequenced into a block. Re-validates against the current state of the block being built. **This is where invalid txs that entered via req/resp or block proposals are caught** — their invalidity is reported as part of block validation/attestation.
 
 Runs:
-- Timestamp, DoubleSpend, Phases, Gas, BlockHeader
+- Timestamp, DoubleSpend, Phases, GasLimits, Gas, BlockHeader
 
 Does **not** run:
 - Proof, Data — already verified on entry (by gossip, RPC, or req/resp validators)
@@ -91,8 +92,8 @@ The `AllowedSetupCallsMetaValidator` checks a precomputed boolean flag (`TxMetaD
 | `MetadataTxValidator` | Chain ID, rollup version, protocol contracts hash, VK tree root | 4.18 us |
 | `TimestampTxValidator` | Transaction has not expired (expiration timestamp vs next slot) | 1.56 us |
 | `DoubleSpendTxValidator` | Nullifiers do not already exist in the nullifier tree | 106.08 us |
-| `GasTxValidator` | Gas limits are within bounds (delegates to `GasLimitsValidator`), max fee per gas meets current block fees (delegates to `MaxFeePerGasValidator`), and fee payer has sufficient FeeJuice balance | 1.02 ms |
-| `GasLimitsValidator` | Gas limits are >= fixed minimums and <= AVM max processable L2 gas. Used standalone in pool migration; also called internally by `GasTxValidator` | 3–10 us |
+| `GasTxValidator` | Max fee per gas meets current block fees (delegates to `MaxFeePerGasValidator`), and fee payer has sufficient FeeJuice balance | 1.02 ms |
+| `GasLimitsValidator` | Gas limits are >= fixed minimums and <= AVM max processable L2 gas (optionally clamped further by network admission limits). Sole owner of declared gas-limit validation | 3–10 us |
 | `MaxFeePerGasValidator` | Max fee per gas >= current block gas fees on both dimensions (DA and L2). Used standalone in pool migration; also called internally by `GasTxValidator` | 3–10 us |
 | `PhasesTxValidator` | Public function calls in setup phase are on the allow list | 10.12–13.12 us |
 | `AllowedSetupCallsMetaValidator` | Checks the precomputed `allowedSetupCalls` flag on `TxMetaData`. Used in pool migration instead of the full `PhasesTxValidator` | — |
@@ -109,16 +110,17 @@ The `AllowedSetupCallsMetaValidator` checks a precomputed boolean flag (`TxMetaD
 | Metadata | Stage 1 | Yes | Yes | — | — |
 | Timestamp | Stage 1 | Yes | — | Yes | Yes |
 | DoubleSpend | Stage 1 | Yes | — | Yes | Yes |
-| Gas (balance + limits) | Stage 1 | Optional* | — | Yes | — |
-| GasLimits (standalone) | — | — | — | — | Yes |
+| Gas (fee balance) | Stage 1 | Optional* | — | Yes | — |
+| GasLimits | Stage 1 | Yes*** | — | Yes | Yes |
 | MaxFeePerGas (standalone) | — | — | — | — | Yes |
 | Phases | Stage 1 | Yes | — | Yes | — |
 | AllowedSetupCalls | — | — | — | — | Yes |
 | BlockHeader | Stage 1 | Yes | — | Yes | Yes |
 | Proof | Stage 2 | Optional** | Yes | — | — |
 
-\* Gas balance check is skipped when `skipFeeEnforcement` is set (testing/dev). `GasTxValidator` internally delegates to `GasLimitsValidator` and `MaxFeePerGasValidator` as its first steps, so gas limits and fee-per-gas are checked wherever `GasTxValidator` runs. Pool migration uses `GasLimitsValidator` and `MaxFeePerGasValidator` standalone because it doesn't need the balance check.
+\* Gas balance check is skipped when `skipFeeEnforcement` is set (testing/dev). `GasTxValidator` internally delegates to `MaxFeePerGasValidator` as its first step, so fee-per-gas is checked wherever `GasTxValidator` runs. Pool migration uses `MaxFeePerGasValidator` standalone because it doesn't need the balance check. Declared gas-limit validation is owned solely by `GasLimitsValidator`.
 \** Proof verification is skipped for simulations (no verifier provided).
+\*** Skipped for simulations: gas estimation submits limits above the per-tx maximum, and the wallet clamps the real tx to the admission limit afterward.
 
 The gas-limit bounds `GasLimitsValidator` enforces here — the per-tx protocol maxima and the network admission limits — are documented in [`stdlib/src/gas/README.md`](../../../../stdlib/src/gas/README.md) under "Gas and Data Limits".
 

yarn-project/p2p/src/msg_validators/tx_validator/factory.test.ts

@@ -1,14 +1,16 @@
+import { MAX_PROCESSABLE_L2_GAS, MAX_TX_DA_GAS } from '@aztec/constants';
 import { BlockNumber } from '@aztec/foundation/branded-types';
 import { Fr } from '@aztec/foundation/curves/bn254';
 import type { ContractDataSource } from '@aztec/stdlib/contract';
-import { GasFees } from '@aztec/stdlib/gas';
+import { Gas, GasFees, GasSettings } from '@aztec/stdlib/gas';
 import type {
   ClientProtocolCircuitVerifier,
   MerkleTreeReadOperations,
   WorldStateSynchronizer,
 } from '@aztec/stdlib/interfaces/server';
 import { PeerErrorSeverity } from '@aztec/stdlib/p2p';
-import type { GlobalVariables } from '@aztec/stdlib/tx';
+import { mockTx } from '@aztec/stdlib/testing';
+import { type GlobalVariables, TX_ERROR_GAS_LIMIT_TOO_HIGH } from '@aztec/stdlib/tx';
 
 import { type MockProxy, mock } from 'jest-mock-extended';
 
@@ -26,7 +28,8 @@ import {
   createTxValidatorForOnDemandReceivedTxs,
   createTxValidatorForTransactionsEnteringPendingTxPool,
 } from './factory.js';
-import { GasLimitsValidator, GasTxValidator, MaxFeePerGasValidator } from './gas_validator.js';
+import { GasLimitsValidator } from './gas_limits_validator.js';
+import { GasTxValidator, MaxFeePerGasValidator } from './gas_validator.js';
 import { MetadataTxValidator } from './metadata_validator.js';
 import { AllowedSetupCallsMetaValidator, PhasesTxValidator } from './phases_validator.js';
 import { SizeTxValidator } from './size_validator.js';
@@ -39,6 +42,17 @@ function getValidatorNames(aggregate: AggregateTxValidator<unknown>): string[] {
   return aggregate.validators.map(v => v.constructor.name);
 }
 
+/** A tx with no public calls, carrying the given gas settings. */
+async function mockPrivateTxWithGasSettings(gasSettings: GasSettings) {
+  const tx = await mockTx(1, {
+    numberOfNonRevertiblePublicCallRequests: 0,
+    numberOfRevertiblePublicCallRequests: 0,
+    hasPublicTeardownCallRequest: false,
+  });
+  tx.data.constants.txContext.gasSettings = gasSettings;
+  return tx;
+}
+
 describe('Validator factory functions', () => {
   let synchronizer: MockProxy<WorldStateSynchronizer>;
   let contractSource: MockProxy<ContractDataSource>;
@@ -72,12 +86,68 @@ describe('Validator factory functions', () => {
         'phasesValidator',
         'blockHeaderValidator',
         'doubleSpendValidator',
+        'gasLimitsValidator',
         'gasValidator',
         'dataValidator',
         'contractInstanceValidator',
       ]);
     });
 
+    it('forwards the network admission limits to the gas limits validator', async () => {
+      const maxTxL2Gas = Math.floor(MAX_PROCESSABLE_L2_GAS / 2);
+      const validators = createFirstStageTxValidationsForGossipedTransactions(
+        0n,
+        BlockNumber(2),
+        synchronizer,
+        new GasFees(1, 1),
+        1,
+        2,
+        Fr.ZERO,
+        contractSource,
+        true,
+        [],
+        undefined,
+        { maxTxL2Gas },
+      );
+
+      // Over the network admission limit but under the protocol ceiling, so only forwarded opts can reject it.
+      const tx = await mockPrivateTxWithGasSettings(
+        GasSettings.fallback({ gasLimits: new Gas(MAX_TX_DA_GAS, maxTxL2Gas + 1), maxFeesPerGas: new GasFees(1, 1) }),
+      );
+      const result = await validators.gasLimitsValidator.validator.validateTx(tx);
+      expect(result.result).toBe('invalid');
+      expect((result as { reason: string[] }).reason[0]).toContain(TX_ERROR_GAS_LIMIT_TOO_HIGH);
+    });
+
+    it('forwards the network DA admission limit to the gas limits validator', async () => {
+      const maxTxDAGas = Math.floor(MAX_TX_DA_GAS / 2);
+      const validators = createFirstStageTxValidationsForGossipedTransactions(
+        0n,
+        BlockNumber(2),
+        synchronizer,
+        new GasFees(1, 1),
+        1,
+        2,
+        Fr.ZERO,
+        contractSource,
+        true,
+        [],
+        undefined,
+        { maxTxDAGas },
+      );
+
+      // Over the network DA admission limit but under the protocol DA ceiling.
+      const tx = await mockPrivateTxWithGasSettings(
+        GasSettings.fallback({
+          gasLimits: new Gas(maxTxDAGas + 1, MAX_PROCESSABLE_L2_GAS),
+          maxFeesPerGas: new GasFees(1, 1),
+        }),
+      );
+      const result = await validators.gasLimitsValidator.validator.validateTx(tx);
+      expect(result.result).toBe('invalid');
+      expect((result as { reason: string[] }).reason[0]).toContain(TX_ERROR_GAS_LIMIT_TOO_HIGH);
+    });
+
     it('does not include a proof validator', () => {
       const validators = createFirstStageTxValidationsForGossipedTransactions(
         0n,
@@ -116,6 +186,7 @@ describe('Validator factory functions', () => {
       expect(validators.dataValidator.severity).toBe(PeerErrorSeverity.MidToleranceError);
       expect(validators.metadataValidator.severity).toBe(PeerErrorSeverity.MidToleranceError);
       expect(validators.doubleSpendValidator.severity).toBe(PeerErrorSeverity.MidToleranceError);
+      expect(validators.gasLimitsValidator.severity).toBe(PeerErrorSeverity.MidToleranceError);
       expect(validators.gasValidator.severity).toBe(PeerErrorSeverity.MidToleranceError);
       expect(validators.phasesValidator.severity).toBe(PeerErrorSeverity.MidToleranceError);
     });
@@ -245,14 +316,14 @@ describe('Validator factory functions', () => {
 
       const aggregate = validator as AggregateTxValidator<unknown>;
       const names = getValidatorNames(aggregate);
-      // Declared gas-limit admission is not fee enforcement, so it stays even with fees skipped.
+      // Gas-limit validation is not fee enforcement, so it stays even with fees skipped.
       expect(names).toContain(GasLimitsValidator.name);
       expect(names).not.toContain(GasTxValidator.name);
       expect(names).toContain(TxProofValidator.name);
     });
 
-    it('excludes the gas-limits admission validator during simulation', () => {
-      // Gas estimation submits intentionally-inflated forEstimation limits, so the admission limit must not
+    it('excludes the gas-limits validator during simulation', () => {
+      // Gas estimation submits intentionally-inflated forEstimation limits, so gas-limit validation must not
       // reject the estimation tx; the wallet clamps the real tx afterward.
       const validator = createTxValidatorForAcceptingTxsOverRPC(db, contractSource, undefined, {
         l1ChainId: 1,
@@ -270,6 +341,42 @@ describe('Validator factory functions', () => {
       expect(getValidatorNames(aggregate)).not.toContain(GasLimitsValidator.name);
     });
 
+    describe('gas-limit validation', () => {
+      // Estimation limits exceed the per-tx protocol maximum by construction, so whether the tx is rejected is
+      // decided solely by isSimulation; skipFeeEnforcement must not affect it. The aggregate collects reasons
+      // from every validator, so asserting on the specific error is robust to other validators failing on the
+      // mocked db.
+      it.each`
+        isSimulation | skipFeeEnforcement | rejected
+        ${false}     | ${false}           | ${true}
+        ${false}     | ${true}            | ${true}
+        ${true}      | ${false}           | ${false}
+        ${true}      | ${true}            | ${false}
+      `(
+        'isSimulation=$isSimulation, skipFeeEnforcement=$skipFeeEnforcement: over-limit tx rejected=$rejected',
+        async ({ isSimulation, skipFeeEnforcement, rejected }) => {
+          db.findLeafIndices.mockResolvedValue([]);
+          const validator = createTxValidatorForAcceptingTxsOverRPC(db, contractSource, undefined, {
+            l1ChainId: 1,
+            rollupVersion: 2,
+            setupAllowList: [],
+            gasFees: new GasFees(1, 1),
+            skipFeeEnforcement,
+            isSimulation,
+            timestamp: 100n,
+            blockNumber: BlockNumber(5),
+            txsPermitted: true,
+          });
+          const tx = await mockPrivateTxWithGasSettings(
+            GasSettings.forEstimation({ maxFeesPerGas: new GasFees(1, 1) }),
+          );
+          const result = await validator.validateTx(tx);
+          const reasons = result.result === 'invalid' ? result.reason : [];
+          expect(reasons.some(r => r.includes(TX_ERROR_GAS_LIMIT_TOO_HIGH))).toBe(rejected);
+        },
+      );
+    });
+
     it('excludes proof validator when no verifier is provided', () => {
       const validator = createTxValidatorForAcceptingTxsOverRPC(db, contractSource, undefined, {
         l1ChainId: 1,
@@ -309,10 +416,24 @@ describe('Validator factory functions', () => {
         PhasesTxValidator.name,
         BlockHeaderTxValidator.name,
         DoubleSpendTxValidator.name,
+        GasLimitsValidator.name,
         GasTxValidator.name,
       ]);
     });
 
+    it('rejects declared gas limits above the protocol ceiling', async () => {
+      // Block proposal txs get only well-formedness checks on receipt; this is where an over-declared limit must
+      // be caught, or execution would trip the simulator's MAX_PROCESSABLE_L2_GAS assertion.
+      db.findLeafIndices.mockResolvedValue([]);
+      const result = createTxValidatorForBlockBuilding(db, contractSource, globalVariables, []);
+
+      const tx = await mockPrivateTxWithGasSettings(GasSettings.forEstimation({ maxFeesPerGas: new GasFees(1, 1) }));
+      const validationResult = await result.preprocessValidator!.validateTx(tx);
+      expect(validationResult.result).toBe('invalid');
+      const reasons = (validationResult as { reason: string[] }).reason;
+      expect(reasons.some(r => r.includes(TX_ERROR_GAS_LIMIT_TOO_HIGH))).toBe(true);
+    });
+
     it('returns a nullifierCache alongside the preprocessValidator', () => {
       const result = createTxValidatorForBlockBuilding(db, contractSource, globalVariables, []);
 

yarn-project/p2p/src/msg_validators/tx_validator/factory.ts

@@ -57,7 +57,8 @@ import { CachedTxValidator } from './cached_tx_validator.js';
 import { ContractInstanceTxValidator } from './contract_instance_validator.js';
 import { DataTxValidator } from './data_validator.js';
 import { DoubleSpendTxValidator, type NullifierSource } from './double_spend_validator.js';
-import { GasLimitsValidator, GasTxValidator, MaxFeePerGasValidator } from './gas_validator.js';
+import { GasLimitsValidator } from './gas_limits_validator.js';
+import { GasTxValidator, MaxFeePerGasValidator } from './gas_validator.js';
 import { MetadataTxValidator } from './metadata_validator.js';
 import { NullifierCache } from './nullifier_cache.js';
 import { AllowedSetupCallsMetaValidator, PhasesTxValidator } from './phases_validator.js';
@@ -86,7 +87,8 @@ export interface TransactionValidator {
  * without consulting the pool or running proof verification.
  *
  * The `doubleSpendValidator` failure is special-cased by the caller (`handleGossipedTx`)
- * to determine severity based on how recently the nullifier appeared.
+ * to determine severity based on how recently the nullifier appeared. The caller reports the
+ * first failing entry among equally severe ones.
  */
 export function createFirstStageTxValidationsForGossipedTransactions(
   timestamp: UInt64,
@@ -156,13 +158,16 @@ export function createFirstStageTxValidationsForGossipedTransactions(
       ),
       severity: PeerErrorSeverity.MidToleranceError, // This is handled specifically at the point of rejection by considering a recent window where it may have been valid
     },
+    gasLimitsValidator: {
+      validator: new GasLimitsValidator<Tx>({ ...gasLimitOpts, bindings }),
+      severity: PeerErrorSeverity.MidToleranceError,
+    },
     gasValidator: {
       validator: new GasTxValidator(
         new DatabasePublicStateSource(merkleTree),
         ProtocolContractAddress.FeeJuice,
         gasFees,
         bindings,
-        gasLimitOpts,
       ),
       severity: PeerErrorSeverity.MidToleranceError,
     },
@@ -343,8 +348,7 @@ export function createTxValidatorForAcceptingTxsOverRPC(
   // skipped during simulation: gas estimation submits intentionally-inflated `forEstimation` limits (above
   // the per-tx max) and the wallet clamps the real tx to the admission limit afterward, so enforcing the
   // limit on the estimation tx would reject a valid estimation. The fee-balance check below stays behind
-  // `skipFeeEnforcement`, and GasTxValidator is constructed without the limit opts so it does not re-run
-  // this same check.
+  // `skipFeeEnforcement`, and GasTxValidator does not re-run this same check.
   if (!isSimulation) {
     validators.push(new GasLimitsValidator<Tx>({ maxTxL2Gas, maxTxDAGas, bindings }));
   }
@@ -416,6 +420,9 @@ function createTxValidatorForValidatingAgainstCurrentState(
     new PhasesTxValidator(contractDataSource, setupAllowList, globalVariables.timestamp, bindings),
     new BlockHeaderTxValidator(archiveSource, bindings),
     new DoubleSpendTxValidator(nullifierSource, bindings),
+    // No limit opts: enforce only the per-tx protocol ceiling. Network admission limits are relay policy and
+    // must not invalidate a proposed block.
+    new GasLimitsValidator<Tx>({ bindings }),
     new GasTxValidator(publicStateSource, ProtocolContractAddress.FeeJuice, globalVariables.gasFees, bindings),
   );
 }