Skip to content

Commit

Permalink
fix: enforce parity of sequencer tx validation and node tx validation (
Browse files Browse the repository at this point in the history
…#7951)

Part of #4781 by having parity between sequencer tx validation and node
tx validation.

Note that we are using the validators from the sequencer, and they
should match. We are omitting `phases` and `gas` tx validator which is
in the sequencer and not here is because those tx validators are
customizable by the sequencer and not uniform between all sequencers.

---------

Co-authored-by: Nicolás Venturo <nicolas.venturo@gmail.com>
  • Loading branch information
sklppy88 and nventuro authored Aug 30, 2024
1 parent 4c6fe1a commit c7eaf92
Show file tree
Hide file tree
Showing 8 changed files with 262 additions and 96 deletions.
17 changes: 9 additions & 8 deletions yarn-project/aztec-node/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,9 @@
"../package.common.json"
],
"jest": {
"moduleNameMapper": {
"^(\\.{1,2}/.*)\\.[cm]?js$": "$1"
},
"testRegex": "./src/.*\\.test\\.(js|mjs|ts)$",
"rootDir": "./src",
"extensionsToTreatAsEsm": [
".ts"
],
"transform": {
"^.+\\.tsx?$": [
"@swc/jest",
Expand All @@ -43,9 +41,11 @@
}
]
},
"extensionsToTreatAsEsm": [
".ts"
],
"moduleNameMapper": {
"^(\\.{1,2}/.*)\\.[cm]?js$": "$1"
},
"testRegex": "./src/.*\\.test\\.(js|mjs|ts)$",
"rootDir": "./src",
"reporters": [
[
"default",
Expand Down Expand Up @@ -83,6 +83,7 @@
"@types/jest": "^29.5.0",
"@types/node": "^18.7.23",
"jest": "^29.5.0",
"jest-mock-extended": "^3.0.3",
"ts-node": "^10.9.1",
"typescript": "^5.0.4"
},
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/aztec-node/src/aztec-node/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { readFileSync } from 'fs';
import { dirname, resolve } from 'path';
import { fileURLToPath } from 'url';

export { sequencerClientConfigMappings, SequencerClientConfig } from '@aztec/sequencer-client';
export { sequencerClientConfigMappings, SequencerClientConfig };

/**
* The configuration the aztec node.
Expand Down
216 changes: 216 additions & 0 deletions yarn-project/aztec-node/src/aztec-node/server.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,216 @@
import { TestCircuitVerifier } from '@aztec/bb-prover';
import {
type AztecNode,
type L1ToL2MessageSource,
type L2BlockSource,
type L2LogsSource,
MerkleTreeId,
type MerkleTreeOperations,
mockTxForRollup,
} from '@aztec/circuit-types';
import { AztecAddress, EthAddress, Fr, GasFees, GlobalVariables, MaxBlockNumber } from '@aztec/circuits.js';
import { type AztecLmdbStore } from '@aztec/kv-store/lmdb';
import { type P2P } from '@aztec/p2p';
import { type GlobalVariableBuilder } from '@aztec/sequencer-client';
import { NoopTelemetryClient } from '@aztec/telemetry-client/noop';
import { type ContractDataSource } from '@aztec/types/contracts';
import { type WorldStateSynchronizer } from '@aztec/world-state';

import { type MockProxy, mock, mockFn } from 'jest-mock-extended';

import { type AztecNodeConfig, getConfigEnvVars } from './config.js';
import { AztecNodeService } from './server.js';

describe('aztec node', () => {
let p2p: MockProxy<P2P>;
let globalVariablesBuilder: MockProxy<GlobalVariableBuilder>;
let merkleTreeOps: MockProxy<MerkleTreeOperations>;

let lastBlockNumber: number;

let node: AztecNode;

const chainId = new Fr(12345);
const version = Fr.ZERO;
const coinbase = EthAddress.random();
const feeRecipient = AztecAddress.random();
const gasFees = GasFees.empty();

beforeEach(() => {
lastBlockNumber = 0;

p2p = mock<P2P>();

globalVariablesBuilder = mock<GlobalVariableBuilder>();
merkleTreeOps = mock<MerkleTreeOperations>();

const worldState = mock<WorldStateSynchronizer>({
getLatest: () => merkleTreeOps,
});

const l2BlockSource = mock<L2BlockSource>({
getBlockNumber: mockFn().mockResolvedValue(lastBlockNumber),
});

const l2LogsSource = mock<L2LogsSource>();

const l1ToL2MessageSource = mock<L1ToL2MessageSource>();

// all txs use the same allowed FPC class
const contractSource = mock<ContractDataSource>();

const store = mock<AztecLmdbStore>();

const aztecNodeConfig: AztecNodeConfig = getConfigEnvVars();

node = new AztecNodeService(
{
...aztecNodeConfig,
l1Contracts: {
...aztecNodeConfig.l1Contracts,
rollupAddress: EthAddress.ZERO,
registryAddress: EthAddress.ZERO,
inboxAddress: EthAddress.ZERO,
outboxAddress: EthAddress.ZERO,
availabilityOracleAddress: EthAddress.ZERO,
},
},
p2p,
l2BlockSource,
l2LogsSource,
l2LogsSource,
contractSource,
l1ToL2MessageSource,
worldState,
undefined,
31337,
1,
globalVariablesBuilder,
store,
new TestCircuitVerifier(),
new NoopTelemetryClient(),
);
});

describe('tx validation', () => {
it('tests that the node correctly validates double spends', async () => {
const txs = [mockTxForRollup(0x10000), mockTxForRollup(0x20000)];
txs.forEach(tx => {
tx.data.constants.txContext.chainId = chainId;
});
const doubleSpendTx = txs[0];
const doubleSpendWithExistingTx = txs[1];

const mockedGlobalVariables = new GlobalVariables(
chainId,
version,
new Fr(lastBlockNumber + 1),
new Fr(1),
Fr.ZERO,
coinbase,
feeRecipient,
gasFees,
);

globalVariablesBuilder.buildGlobalVariables
.mockResolvedValueOnce(mockedGlobalVariables)
.mockResolvedValueOnce(mockedGlobalVariables);

expect(await node.isValidTx(doubleSpendTx)).toBe(true);

// 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);

globalVariablesBuilder.buildGlobalVariables
.mockResolvedValueOnce(mockedGlobalVariables)
.mockResolvedValueOnce(mockedGlobalVariables);

expect(await node.isValidTx(doubleSpendWithExistingTx)).toBe(true);

// 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();
merkleTreeOps.findLeafIndex.mockImplementation((treeId: MerkleTreeId, value: any) => {
return Promise.resolve(
treeId === MerkleTreeId.NULLIFIER_TREE && value.equals(doubleSpendNullifier) ? 1n : undefined,
);
});

expect(await node.isValidTx(doubleSpendWithExistingTx)).toBe(false);
});

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

const mockedGlobalVariables = new GlobalVariables(
chainId,
version,
new Fr(lastBlockNumber + 1),
new Fr(1),
Fr.ZERO,
coinbase,
feeRecipient,
gasFees,
);

globalVariablesBuilder.buildGlobalVariables
.mockResolvedValueOnce(mockedGlobalVariables)
.mockResolvedValueOnce(mockedGlobalVariables);

expect(await node.isValidTx(tx)).toBe(true);

// 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);

expect(await node.isValidTx(tx)).toBe(false);
});

it('tests that the node correctly validates max block numbers', async () => {
const txs = [mockTxForRollup(0x10000), mockTxForRollup(0x20000), mockTxForRollup(0x30000)];
txs.forEach(tx => {
tx.data.constants.txContext.chainId = chainId;
});

const noMaxBlockNumberMetadata = txs[0];
const invalidMaxBlockNumberMetadata = txs[1];
const validMaxBlockNumberMetadata = txs[2];

invalidMaxBlockNumberMetadata.data.forRollup!.rollupValidationRequests = {
maxBlockNumber: new MaxBlockNumber(true, new Fr(1)),
getSize: () => 1,
toBuffer: () => Fr.ZERO.toBuffer(),
};

validMaxBlockNumberMetadata.data.forRollup!.rollupValidationRequests = {
maxBlockNumber: new MaxBlockNumber(true, new Fr(5)),
getSize: () => 1,
toBuffer: () => Fr.ZERO.toBuffer(),
};

const mockedGlobalVariables = new GlobalVariables(
chainId,
version,
new Fr(lastBlockNumber + 5),
new Fr(1),
Fr.ZERO,
coinbase,
feeRecipient,
gasFees,
);

globalVariablesBuilder.buildGlobalVariables
.mockResolvedValueOnce(mockedGlobalVariables)
.mockResolvedValueOnce(mockedGlobalVariables)
.mockResolvedValueOnce(mockedGlobalVariables);

// Default tx with no max block number should be valid
expect(await node.isValidTx(noMaxBlockNumberMetadata)).toBe(true);
// Tx with max block number < current block number should be invalid
expect(await node.isValidTx(invalidMaxBlockNumberMetadata)).toBe(false);
// Tx with max block number >= current block number should be valid
expect(await node.isValidTx(validMaxBlockNumberMetadata)).toBe(true);
});
});
});
51 changes: 33 additions & 18 deletions yarn-project/aztec-node/src/aztec-node/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { createArchiver } from '@aztec/archiver';
import { BBCircuitVerifier, TestCircuitVerifier } from '@aztec/bb-prover';
import {
type AztecNode,
type ClientProtocolCircuitVerifier,
type FromLogType,
type GetUnencryptedLogsResponse,
type L1ToL2MessageSource,
Expand All @@ -24,7 +25,6 @@ import {
type TxHash,
TxReceipt,
TxStatus,
type TxValidator,
partitionReverts,
} from '@aztec/circuit-types';
import {
Expand Down Expand Up @@ -57,8 +57,15 @@ import { getCanonicalFeeJuice } from '@aztec/protocol-contracts/fee-juice';
import { getCanonicalInstanceDeployer } from '@aztec/protocol-contracts/instance-deployer';
import { getCanonicalKeyRegistryAddress } from '@aztec/protocol-contracts/key-registry';
import { getCanonicalMultiCallEntrypointAddress } from '@aztec/protocol-contracts/multi-call-entrypoint';
import { AggregateTxValidator, DataTxValidator, GlobalVariableBuilder, SequencerClient } from '@aztec/sequencer-client';
import { PublicProcessorFactory, WASMSimulator, createSimulationProvider } from '@aztec/simulator';
import {
AggregateTxValidator,
DataTxValidator,
DoubleSpendTxValidator,
GlobalVariableBuilder,
MetadataTxValidator,
SequencerClient,
} from '@aztec/sequencer-client';
import { PublicProcessorFactory, WASMSimulator, WorldStateDB, createSimulationProvider } from '@aztec/simulator';
import { type TelemetryClient } from '@aztec/telemetry-client';
import { NoopTelemetryClient } from '@aztec/telemetry-client/noop';
import {
Expand All @@ -72,7 +79,6 @@ import { MerkleTrees, type WorldStateSynchronizer, createWorldStateSynchronizer

import { type AztecNodeConfig, getPackageInfo } from './config.js';
import { NodeMetrics } from './node_metrics.js';
import { MetadataTxValidator } from './tx_validator/tx_metadata_validator.js';
import { TxProofValidator } from './tx_validator/tx_proof_validator.js';

/**
Expand All @@ -97,7 +103,7 @@ export class AztecNodeService implements AztecNode {
protected readonly version: number,
protected readonly globalVariableBuilder: GlobalVariableBuilder,
protected readonly merkleTreesDb: AztecKVStore,
private txValidator: TxValidator,
private proofVerifier: ClientProtocolCircuitVerifier,
private telemetry: TelemetryClient,
private log = createDebugLogger('aztec:node'),
) {
Expand Down Expand Up @@ -158,11 +164,6 @@ export class AztecNodeService implements AztecNode {
await Promise.all([p2pClient.start(), worldStateSynchronizer.start()]);

const proofVerifier = config.realProofs ? await BBCircuitVerifier.new(config) : new TestCircuitVerifier();
const txValidator = new AggregateTxValidator(
new DataTxValidator(),
new MetadataTxValidator(config.l1ChainId),
new TxProofValidator(proofVerifier),
);

const simulationProvider = await createSimulationProvider(config, log);

Expand Down Expand Up @@ -197,7 +198,7 @@ export class AztecNodeService implements AztecNode {
config.version,
new GlobalVariableBuilder(config),
store,
txValidator,
proofVerifier,
telemetry,
log,
);
Expand Down Expand Up @@ -762,7 +763,26 @@ export class AztecNodeService implements AztecNode {
}

public async isValidTx(tx: Tx): Promise<boolean> {
const [_, invalidTxs] = await this.txValidator.validateTxs([tx]);
const blockNumber = (await this.blockSource.getBlockNumber()) + 1;

const newGlobalVariables = await this.globalVariableBuilder.buildGlobalVariables(
new Fr(blockNumber),
// We only need chainId and block number, thus coinbase and fee recipient can be set to 0.
EthAddress.ZERO,
AztecAddress.ZERO,
);

// 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 txValidator = new AggregateTxValidator(
new DataTxValidator(),
new MetadataTxValidator(newGlobalVariables),
new DoubleSpendTxValidator(new WorldStateDB(this.worldStateSynchronizer.getLatest())),
new TxProofValidator(this.proofVerifier),
);

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

Expand All @@ -777,12 +797,7 @@ export class AztecNodeService implements AztecNode {
this.sequencer?.updateSequencerConfig(config);

if (newConfig.realProofs !== this.config.realProofs) {
const proofVerifier = config.realProofs ? await BBCircuitVerifier.new(newConfig) : new TestCircuitVerifier();

this.txValidator = new AggregateTxValidator(
new MetadataTxValidator(this.l1ChainId),
new TxProofValidator(proofVerifier),
);
this.proofVerifier = config.realProofs ? await BBCircuitVerifier.new(newConfig) : new TestCircuitVerifier();
}

this.config = newConfig;
Expand Down
Loading

0 comments on commit c7eaf92

Please sign in to comment.