Skip to content

Commit

Permalink
feat: Validate block proposal txs iteratively
Browse files Browse the repository at this point in the history
  • Loading branch information
spalladino committed Dec 24, 2024
1 parent ab3f318 commit da8056a
Show file tree
Hide file tree
Showing 41 changed files with 843 additions and 930 deletions.
31 changes: 19 additions & 12 deletions yarn-project/aztec-node/src/aztec-node/server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
type WorldStateSynchronizer,
mockTxForRollup,
} from '@aztec/circuit-types';
import { type ContractDataSource, EthAddress, Fr, MaxBlockNumber } from '@aztec/circuits.js';
import { type ContractDataSource, EthAddress, Fr, GasFees, MaxBlockNumber } from '@aztec/circuits.js';
import { type P2P } from '@aztec/p2p';
import { type GlobalVariableBuilder } from '@aztec/sequencer-client';
import { NoopTelemetryClient } from '@aztec/telemetry-client/noop';
Expand All @@ -37,8 +37,9 @@ describe('aztec node', () => {
p2p = mock<P2P>();

globalVariablesBuilder = mock<GlobalVariableBuilder>();
merkleTreeOps = mock<MerkleTreeReadOperations>();
globalVariablesBuilder.getCurrentBaseFees.mockResolvedValue(new GasFees(0, 0));

merkleTreeOps = mock<MerkleTreeReadOperations>();
merkleTreeOps.findLeafIndices.mockImplementation((_treeId: MerkleTreeId, _value: any[]) => {
return Promise.resolve([undefined]);
});
Expand Down Expand Up @@ -99,14 +100,14 @@ describe('aztec node', () => {
const doubleSpendWithExistingTx = txs[1];
lastBlockNumber += 1;

expect(await node.isValidTx(doubleSpendTx)).toBe(true);
expect(await node.isValidTx(doubleSpendTx)).toEqual({ result: 'valid' });

// We push a duplicate nullifier that was created in the same transaction
doubleSpendTx.data.forRollup!.end.nullifiers.push(doubleSpendTx.data.forRollup!.end.nullifiers[0]);

expect(await node.isValidTx(doubleSpendTx)).toBe(false);
expect(await node.isValidTx(doubleSpendTx)).toEqual({ result: 'invalid', reason: ['Duplicate nullifier in tx'] });

expect(await node.isValidTx(doubleSpendWithExistingTx)).toBe(true);
expect(await node.isValidTx(doubleSpendWithExistingTx)).toEqual({ result: 'valid' });

// We make a nullifier from `doubleSpendWithExistingTx` a part of the nullifier tree, so it gets rejected as double spend
const doubleSpendNullifier = doubleSpendWithExistingTx.data.forRollup!.end.nullifiers[0].toBuffer();
Expand All @@ -116,20 +117,23 @@ describe('aztec node', () => {
);
});

expect(await node.isValidTx(doubleSpendWithExistingTx)).toBe(false);
expect(await node.isValidTx(doubleSpendWithExistingTx)).toEqual({
result: 'invalid',
reason: ['Existing nullifier'],
});
lastBlockNumber = 0;
});

it('tests that the node correctly validates chain id', async () => {
const tx = mockTxForRollup(0x10000);
tx.data.constants.txContext.chainId = chainId;

expect(await node.isValidTx(tx)).toBe(true);
expect(await node.isValidTx(tx)).toEqual({ result: 'valid' });

// We make the chain id on the tx not equal to the configured chain id
tx.data.constants.txContext.chainId = new Fr(1n + chainId.value);
tx.data.constants.txContext.chainId = new Fr(1n + chainId.toBigInt());

expect(await node.isValidTx(tx)).toBe(false);
expect(await node.isValidTx(tx)).toEqual({ result: 'invalid', reason: ['Incorrect chain id'] });
});

it('tests that the node correctly validates max block numbers', async () => {
Expand Down Expand Up @@ -159,11 +163,14 @@ describe('aztec node', () => {
lastBlockNumber = 3;

// Default tx with no max block number should be valid
expect(await node.isValidTx(noMaxBlockNumberMetadata)).toBe(true);
expect(await node.isValidTx(noMaxBlockNumberMetadata)).toEqual({ result: 'valid' });
// Tx with max block number < current block number should be invalid
expect(await node.isValidTx(invalidMaxBlockNumberMetadata)).toBe(false);
expect(await node.isValidTx(invalidMaxBlockNumberMetadata)).toEqual({
result: 'invalid',
reason: ['Invalid block number'],
});
// Tx with max block number >= current block number should be valid
expect(await node.isValidTx(validMaxBlockNumberMetadata)).toBe(true);
expect(await node.isValidTx(validMaxBlockNumberMetadata)).toEqual({ result: 'valid' });
});
});
});
65 changes: 25 additions & 40 deletions yarn-project/aztec-node/src/aztec-node/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import {
NullifierMembershipWitness,
type NullifierWithBlockSource,
P2PClientType,
type ProcessedTx,
type ProverConfig,
PublicDataWitness,
PublicSimulationOutput,
Expand All @@ -29,7 +28,7 @@ import {
TxReceipt,
type TxScopedL2Log,
TxStatus,
type TxValidator,
type TxValidationResult,
type WorldStateSynchronizer,
tryStop,
} from '@aztec/circuit-types';
Expand Down Expand Up @@ -63,17 +62,14 @@ import { DateProvider, Timer } from '@aztec/foundation/timer';
import { type AztecKVStore } from '@aztec/kv-store';
import { openTmpStore } from '@aztec/kv-store/lmdb';
import { SHA256Trunc, StandardTree, UnbalancedTree } from '@aztec/merkle-tree';
import {
AggregateTxValidator,
DataTxValidator,
DoubleSpendTxValidator,
MetadataTxValidator,
type P2P,
TxProofValidator,
createP2PClient,
} from '@aztec/p2p';
import { type P2P, createP2PClient } from '@aztec/p2p';
import { ProtocolContractAddress } from '@aztec/protocol-contracts';
import { GlobalVariableBuilder, type L1Publisher, SequencerClient } from '@aztec/sequencer-client';
import {
GlobalVariableBuilder,
type L1Publisher,
SequencerClient,
createValidatorForAcceptingTxs,
} from '@aztec/sequencer-client';
import { PublicProcessorFactory } from '@aztec/simulator';
import { Attributes, type TelemetryClient, type Traceable, type Tracer, trackSpan } from '@aztec/telemetry-client';
import { NoopTelemetryClient } from '@aztec/telemetry-client/noop';
Expand Down Expand Up @@ -397,15 +393,19 @@ export class AztecNodeService implements AztecNode, Traceable {
*/
public async sendTx(tx: Tx) {
const timer = new Timer();
this.log.info(`Received tx ${tx.getTxHash()}`);
const txHash = tx.getTxHash().toString();

if (!(await this.isValidTx(tx))) {
const valid = await this.isValidTx(tx);
if (valid.result !== 'valid') {
const reason = valid.reason.join(', ');
this.metrics.receivedTx(timer.ms(), false);
return;
this.log.warn(`Invalid tx ${txHash}: ${reason}`, { txHash });
throw new Error(`Invalid tx: ${reason}`);
}

await this.p2pClient!.sendTx(tx);
this.metrics.receivedTx(timer.ms(), true);
this.log.info(`Received tx ${tx.getTxHash()}`, { txHash });
}

public async getTxReceipt(txHash: TxHash): Promise<TxReceipt> {
Expand Down Expand Up @@ -857,34 +857,19 @@ export class AztecNodeService implements AztecNode, Traceable {
}
}

public async isValidTx(tx: Tx, isSimulation: boolean = false): Promise<boolean> {
public async isValidTx(tx: Tx, isSimulation: boolean = false): Promise<TxValidationResult> {
const blockNumber = (await this.blockSource.getBlockNumber()) + 1;
const db = this.worldStateSynchronizer.getCommitted();
// These validators are taken from the sequencer, and should match.
// The reason why `phases` and `gas` tx validator is in the sequencer and not here is because
// those tx validators are customizable by the sequencer.
const txValidators: TxValidator<Tx | ProcessedTx>[] = [
new DataTxValidator(),
new MetadataTxValidator(new Fr(this.l1ChainId), new Fr(blockNumber)),
new DoubleSpendTxValidator({
getNullifierIndices: nullifiers => db.findLeafIndices(MerkleTreeId.NULLIFIER_TREE, nullifiers),
}),
];

if (!isSimulation) {
txValidators.push(new TxProofValidator(this.proofVerifier));
}

const txValidator = new AggregateTxValidator(...txValidators);

const [_, invalidTxs] = await txValidator.validateTxs([tx]);
if (invalidTxs.length > 0) {
this.log.warn(`Rejecting tx ${tx.getTxHash()} because of validation errors`);

return false;
}
const verifier = isSimulation ? this.proofVerifier : undefined;
const validator = createValidatorForAcceptingTxs(db, this.contractDataSource, verifier, {
blockNumber,
l1ChainId: this.l1ChainId,
enforceFees: !!this.config.enforceFees,
setupAllowList: this.config.allowedInSetup ?? [],
gasFees: await this.getCurrentBaseFees(),
});

return true;
return await validator.validateTx(tx);
}

public async setConfig(config: Partial<SequencerConfig & ProverConfig>): Promise<void> {
Expand Down
14 changes: 10 additions & 4 deletions yarn-project/circuit-types/src/interfaces/aztec-node.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import { MerkleTreeId } from '../merkle_tree_id.js';
import { EpochProofQuote } from '../prover_coordination/epoch_proof_quote.js';
import { PublicDataWitness } from '../public_data_witness.js';
import { SiblingPath } from '../sibling_path/sibling_path.js';
import { type TxValidationResult } from '../tx/index.js';
import { PublicSimulationOutput } from '../tx/public_simulation_output.js';
import { Tx } from '../tx/tx.js';
import { TxHash } from '../tx/tx_hash.js';
Expand Down Expand Up @@ -293,9 +294,14 @@ describe('AztecNodeApiSchema', () => {
expect(response).toBeInstanceOf(PublicSimulationOutput);
});

it('isValidTx', async () => {
it('isValidTx(valid)', async () => {
const response = await context.client.isValidTx(Tx.random(), true);
expect(response).toEqual({ result: 'valid' });
});

it('isValidTx(invalid)', async () => {
const response = await context.client.isValidTx(Tx.random());
expect(response).toBe(true);
expect(response).toEqual({ result: 'invalid', reason: ['Invalid'] });
});

it('setConfig', async () => {
Expand Down Expand Up @@ -559,9 +565,9 @@ class MockAztecNode implements AztecNode {
expect(tx).toBeInstanceOf(Tx);
return Promise.resolve(PublicSimulationOutput.random());
}
isValidTx(tx: Tx, _isSimulation?: boolean | undefined): Promise<boolean> {
isValidTx(tx: Tx, isSimulation?: boolean | undefined): Promise<TxValidationResult> {
expect(tx).toBeInstanceOf(Tx);
return Promise.resolve(true);
return Promise.resolve(isSimulation ? { result: 'valid' } : { result: 'invalid', reason: ['Invalid'] });
}
setConfig(config: Partial<SequencerConfig & ProverConfig>): Promise<void> {
expect(config.coinbase).toBeInstanceOf(EthAddress);
Expand Down
13 changes: 10 additions & 3 deletions yarn-project/circuit-types/src/interfaces/aztec-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,14 @@ import { MerkleTreeId } from '../merkle_tree_id.js';
import { EpochProofQuote } from '../prover_coordination/epoch_proof_quote.js';
import { PublicDataWitness } from '../public_data_witness.js';
import { SiblingPath } from '../sibling_path/index.js';
import { PublicSimulationOutput, Tx, TxHash, TxReceipt } from '../tx/index.js';
import {
PublicSimulationOutput,
Tx,
TxHash,
TxReceipt,
type TxValidationResult,
TxValidationResultSchema,
} from '../tx/index.js';
import { TxEffect } from '../tx_effect.js';
import { type SequencerConfig, SequencerConfigSchema } from './configs.js';
import { type L2BlockNumber, L2BlockNumberSchema } from './l2_block_number.js';
Expand Down Expand Up @@ -395,7 +402,7 @@ export interface AztecNode
* @param tx - The transaction to validate for correctness.
* @param isSimulation - True if the transaction is a simulated one without generated proofs. (Optional)
*/
isValidTx(tx: Tx, isSimulation?: boolean): Promise<boolean>;
isValidTx(tx: Tx, isSimulation?: boolean): Promise<TxValidationResult>;

/**
* Updates the configuration of this node.
Expand Down Expand Up @@ -567,7 +574,7 @@ export const AztecNodeApiSchema: ApiSchemaFor<AztecNode> = {

simulatePublicCalls: z.function().args(Tx.schema, optional(z.boolean())).returns(PublicSimulationOutput.schema),

isValidTx: z.function().args(Tx.schema, optional(z.boolean())).returns(z.boolean()),
isValidTx: z.function().args(Tx.schema, optional(z.boolean())).returns(TxValidationResultSchema),

setConfig: z.function().args(SequencerConfigSchema.merge(ProverConfigSchema).partial()).returns(z.void()),

Expand Down
6 changes: 6 additions & 0 deletions yarn-project/circuit-types/src/interfaces/configs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ export interface SequencerConfig {
maxTxsPerBlock?: number;
/** The minimum number of txs to include in a block. */
minTxsPerBlock?: number;
/** The maximum L2 block gas. */
maxL2BlockGas?: number;
/** The maximum DA block gas. */
maxDABlockGas?: number;
/** Recipient of block reward. */
coinbase?: EthAddress;
/** Address to receive fees. */
Expand Down Expand Up @@ -53,6 +57,8 @@ export const SequencerConfigSchema = z.object({
transactionPollingIntervalMS: z.number().optional(),
maxTxsPerBlock: z.number().optional(),
minTxsPerBlock: z.number().optional(),
maxL2BlockGas: z.number().optional(),
maxDABlockGas: z.number().optional(),
coinbase: schemas.EthAddress.optional(),
feeRecipient: schemas.AztecAddress.optional(),
acvmWorkingDirectory: z.string().optional(),
Expand Down
16 changes: 16 additions & 0 deletions yarn-project/circuit-types/src/tx/tx.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import {
ClientIvcProof,
Fr,
PrivateKernelTailCircuitPublicInputs,
PrivateLog,
type PrivateToPublicAccumulatedData,
type ScopedLogHash,
} from '@aztec/circuits.js';
Expand Down Expand Up @@ -230,6 +232,20 @@ export class Tx extends Gossipable {
);
}

/**
* Estimates the tx size based on its private effects. Note that the actual size of the tx
* after processing will probably be larger, as public execution would generate more data.
*/
getEstimatedPrivateTxEffectsSize() {
return (
this.unencryptedLogs.getSerializedLength() +
this.contractClassLogs.getSerializedLength() +
this.data.getNonEmptyNoteHashes().length * Fr.SIZE_IN_BYTES +
this.data.getNonEmptyNullifiers().length * Fr.SIZE_IN_BYTES +
this.data.getNonEmptyPrivateLogs().length * PrivateLog.SIZE_IN_BYTES
);
}

/**
* Convenience function to get a hash out of a tx or a tx-like.
* @param tx - Tx-like object.
Expand Down
10 changes: 3 additions & 7 deletions yarn-project/circuit-types/src/tx/validator/empty_validator.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
import { type AnyTx, type TxValidator } from './tx_validator.js';
import { type AnyTx, type TxValidationResult, type TxValidator } from './tx_validator.js';

export class EmptyTxValidator<T extends AnyTx = AnyTx> implements TxValidator<T> {
public validateTxs(txs: T[]): Promise<[validTxs: T[], invalidTxs: T[], skippedTxs: T[]]> {
return Promise.resolve([txs, [], []]);
}

public validateTx(_tx: T): Promise<boolean> {
return Promise.resolve(true);
public validateTx(_tx: T): Promise<TxValidationResult> {
return Promise.resolve({ result: 'valid' });
}
}
18 changes: 16 additions & 2 deletions yarn-project/circuit-types/src/tx/validator/tx_validator.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,23 @@
import { type ZodFor } from '@aztec/foundation/schemas';

import { z } from 'zod';

import { type ProcessedTx } from '../processed_tx.js';
import { type Tx } from '../tx.js';

export type AnyTx = Tx | ProcessedTx;

export type TxValidationResult =
| { result: 'valid' }
| { result: 'invalid'; reason: string[] }
| { result: 'skipped'; reason: string[] };

export interface TxValidator<T extends AnyTx = AnyTx> {
validateTx(tx: T): Promise<boolean>;
validateTxs(txs: T[]): Promise<[validTxs: T[], invalidTxs: T[], skippedTxs?: T[]]>;
validateTx(tx: T): Promise<TxValidationResult>;
}

export const TxValidationResultSchema = z.discriminatedUnion('result', [
z.object({ result: z.literal('valid'), reason: z.array(z.string()).optional() }),
z.object({ result: z.literal('invalid'), reason: z.array(z.string()) }),
z.object({ result: z.literal('skipped'), reason: z.array(z.string()) }),
]) satisfies ZodFor<TxValidationResult>;
5 changes: 5 additions & 0 deletions yarn-project/circuit-types/src/tx_effect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,11 @@ export class TxEffect {
]);
}

/** Returns the size of this tx effect in bytes as serialized onto DA. */
getDASize() {
return this.toBlobFields().length * Fr.SIZE_IN_BYTES;
}

/**
* Deserializes the TxEffect object from a Buffer.
* @param buffer - Buffer or BufferReader object to deserialize.
Expand Down
5 changes: 5 additions & 0 deletions yarn-project/circuits.js/src/structs/gas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ export class Gas {
return new Gas(Math.ceil(this.daGas * scalar), Math.ceil(this.l2Gas * scalar));
}

/** Returns true if any of this instance's dimensions is greater than the corresponding on the other. */
gtAny(other: Gas) {
return this.daGas > other.daGas || this.l2Gas > other.l2Gas;
}

computeFee(gasFees: GasFees) {
return GasDimensions.reduce(
(acc, dimension) => acc.add(gasFees.get(dimension).mul(new Fr(this.get(dimension)))),
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/end-to-end/src/e2e_block_building.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ describe('e2e_block_building', () => {
// This will leave the sequencer with just 2s to build the block, so it shouldn't be
// able to squeeze in more than 10 txs in each. This is sensitive to the time it takes
// to pick up and validate the txs, so we may need to bump it to work on CI.
sequencer.sequencer.timeTable[SequencerState.WAITING_FOR_TXS] = 2;
sequencer.sequencer.timeTable[SequencerState.INITIALIZING_PROPOSAL] = 2;
sequencer.sequencer.timeTable[SequencerState.CREATING_BLOCK] = 2;
sequencer.sequencer.processTxTime = 1;

Expand Down
2 changes: 2 additions & 0 deletions yarn-project/foundation/src/config/env_var.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,8 @@ export type EnvVar =
| 'SEQ_MAX_BLOCK_SIZE_IN_BYTES'
| 'SEQ_MAX_TX_PER_BLOCK'
| 'SEQ_MIN_TX_PER_BLOCK'
| 'SEQ_MAX_DA_BLOCK_GAS'
| 'SEQ_MAX_L2_BLOCK_GAS'
| 'SEQ_PUBLISH_RETRY_INTERVAL_MS'
| 'SEQ_PUBLISHER_PRIVATE_KEY'
| 'SEQ_REQUIRED_CONFIRMATIONS'
Expand Down
Loading

0 comments on commit da8056a

Please sign in to comment.