Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fixing private voting by correctly throwing an error if simulation fails #7840

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions yarn-project/aztec-node/src/aztec-node/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,12 @@ import { createProverClient } from '@aztec/prover-client';
import {
AggregateTxValidator,
DataTxValidator,
DoubleSpendTxValidator,
type GlobalVariableBuilder,
SequencerClient,
getGlobalVariableBuilder,
} from '@aztec/sequencer-client';
import { PublicProcessorFactory, WASMSimulator, createSimulationProvider } from '@aztec/simulator';
import { PublicProcessorFactory, WASMSimulator, WorldStateDB, createSimulationProvider } from '@aztec/simulator';
import { type TelemetryClient } from '@aztec/telemetry-client';
import { NoopTelemetryClient } from '@aztec/telemetry-client/noop';
import {
Expand Down Expand Up @@ -165,6 +166,7 @@ export class AztecNodeService implements AztecNode {
new DataTxValidator(),
new MetadataTxValidator(config.l1ChainId),
new TxProofValidator(proofVerifier),
new DoubleSpendTxValidator(new WorldStateDB(worldStateSynchronizer.getLatest())),
);

const simulationProvider = await createSimulationProvider(config, log);
Expand Down Expand Up @@ -339,9 +341,7 @@ export class AztecNodeService implements AztecNode {
const timer = new Timer();
this.log.info(`Received tx ${tx.getTxHash()}`);

const [_, invalidTxs] = await this.txValidator.validateTxs([tx]);
if (invalidTxs.length > 0) {
this.log.warn(`Rejecting tx ${tx.getTxHash()} because of validation errors`);
if ((await this.validateTx(tx)) === false) {
this.metrics.receivedTx(timer.ms(), false);
return;
}
Expand Down Expand Up @@ -768,6 +768,17 @@ export class AztecNodeService implements AztecNode {
);
}

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

return false;
}

return true;
}

public async setConfig(config: Partial<SequencerConfig & ProverConfig>): Promise<void> {
const newConfig = { ...this.config, ...config };
this.sequencer?.updateSequencerConfig(config);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ export type SimulateMethodOptions = {
from?: AztecAddress;
/** Gas settings for the simulation. */
gasSettings?: GasSettings;
/** Simulate without validating tx against current state. */
offline?: boolean;
};

/**
Expand Down Expand Up @@ -93,7 +95,7 @@ export class ContractFunctionInteraction extends BaseContractInteraction {
}

const txRequest = await this.create();
const simulatedTx = await this.wallet.simulateTx(txRequest, true, options?.from);
const simulatedTx = await this.wallet.simulateTx(txRequest, true, options?.from, options?.offline);

// As account entrypoints are private, for private functions we retrieve the return values from the first nested call
// since we're interested in the first set of values AFTER the account entrypoint
Expand Down
9 changes: 7 additions & 2 deletions yarn-project/aztec.js/src/wallet/base_wallet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,13 @@ export abstract class BaseWallet implements Wallet {
proveTx(txRequest: TxExecutionRequest, simulatePublic: boolean): Promise<Tx> {
return this.pxe.proveTx(txRequest, simulatePublic, this.scopes);
}
simulateTx(txRequest: TxExecutionRequest, simulatePublic: boolean, msgSender?: AztecAddress): Promise<SimulatedTx> {
return this.pxe.simulateTx(txRequest, simulatePublic, msgSender, this.scopes);
simulateTx(
txRequest: TxExecutionRequest,
simulatePublic: boolean,
msgSender?: AztecAddress,
offline?: boolean,
): Promise<SimulatedTx> {
return this.pxe.simulateTx(txRequest, simulatePublic, msgSender, offline, this.scopes);
}
sendTx(tx: Tx): Promise<TxHash> {
return this.pxe.sendTx(tx);
Expand Down
7 changes: 7 additions & 0 deletions yarn-project/circuit-types/src/interfaces/aztec-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,13 @@ export interface AztecNode {
**/
simulatePublicCalls(tx: Tx): Promise<PublicSimulationOutput>;

/**
* Validates the correctness of the execution, namely that a transaction is valid if and
* only if the transaction can be added to a valid block at the current state.
* @param tx - The transaction to validate for correctness.
*/
validateTx(tx: Tx): Promise<boolean>;

/**
* Updates the configuration of this node.
* @param config - Updated configuration to be merged with the current one.
Expand Down
2 changes: 2 additions & 0 deletions yarn-project/circuit-types/src/interfaces/pxe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ export interface PXE {
* @param txRequest - An authenticated tx request ready for simulation
* @param simulatePublic - Whether to simulate the public part of the transaction.
* @param msgSender - (Optional) The message sender to use for the simulation.
* @param offline - (Optional) Whether to simulate without validating tx against current state.
* @param scopes - (Optional) The accounts whose notes we can access in this call. Currently optional and will default to all.
* @returns A simulated transaction object that includes a transaction that is potentially ready
* to be sent to the network for execution, along with public and private return values.
Expand All @@ -190,6 +191,7 @@ export interface PXE {
txRequest: TxExecutionRequest,
simulatePublic: boolean,
msgSender?: AztecAddress,
offline?: boolean,
scopes?: AztecAddress[],
): Promise<SimulatedTx>;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,11 @@ describe('e2e_voting_contract', () => {
await crossDelay();

// We try simulating voting again, but our TX is invalid because it will emit duplicate nullifiers
await expect(votingContract.methods.cast_vote(candidate).simulate()).rejects.toThrow(
'The simulated transaction is unable to be added to state and is invalid.',
);
await expect(
votingContract.methods.cast_vote(candidate).simulate({
offline: false,
}),
).rejects.toThrow('The simulated transaction is unable to be added to state and is invalid.');

// We try voting again, but our TX is dropped due to trying to emit duplicate nullifiers
await expect(votingContract.methods.cast_vote(candidate).send().wait()).rejects.toThrow(
Expand Down
9 changes: 9 additions & 0 deletions yarn-project/pxe/src/pxe_service/pxe_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,7 @@ export class PXEService implements PXE {
txRequest: TxExecutionRequest,
simulatePublic: boolean,
msgSender: AztecAddress | undefined = undefined,
offline: boolean = true,
scopes?: AztecAddress[],
): Promise<SimulatedTx> {
return await this.jobQueue.put(async () => {
Expand All @@ -505,6 +506,14 @@ export class PXEService implements PXE {
simulatedTx.publicOutput = await this.#simulatePublicCalls(simulatedTx.tx);
}

if (!offline) {
const isValidTx = await this.node.validateTx(simulatedTx.tx);

if (!isValidTx) {
throw new Error('The simulated transaction is unable to be added to state and is invalid.');
}
}

// We log only if the msgSender is undefined, as simulating with a different msgSender
// is unlikely to be a real transaction, and likely to be only used to read data.
// Meaning that it will not necessarily have produced a nullifier (and thus have no TxHash)
Expand Down
1 change: 1 addition & 0 deletions yarn-project/sequencer-client/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ export * from './publisher/index.js';
export * from './sequencer/index.js';
export * from './tx_validator/aggregate_tx_validator.js';
export * from './tx_validator/data_validator.js';
export * from './tx_validator/double_spend_validator.js';

// Used by the node to simulate public parts of transactions. Should these be moved to a shared library?
export * from './global_variable_builder/index.js';
Loading