Skip to content

Commit

Permalink
Apply suggestions from code review
Browse files Browse the repository at this point in the history
Co-authored-by: Nicolás Venturo <[email protected]>
  • Loading branch information
sklppy88 and nventuro committed Aug 13, 2024
1 parent b0cf048 commit bc3fffb
Show file tree
Hide file tree
Showing 7 changed files with 17 additions and 18 deletions.
4 changes: 2 additions & 2 deletions yarn-project/aztec-node/src/aztec-node/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ export class AztecNodeService implements AztecNode {
const timer = new Timer();
this.log.info(`Received tx ${tx.getTxHash()}`);

if ((await this.validateTx(tx)) === false) {
if (!(await this.isValidTx(tx))) {
this.metrics.receivedTx(timer.ms(), false);
return;
}
Expand Down Expand Up @@ -768,7 +768,7 @@ export class AztecNodeService implements AztecNode {
);
}

public async validateTx(tx: Tx): Promise<boolean> {
public async isValidTx(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`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ export type SimulateMethodOptions = {
from?: AztecAddress;
/** Gas settings for the simulation. */
gasSettings?: GasSettings;
/** Simulate without validating tx against current state. */
offline?: boolean;
/** Option to throw an error if simulation does not produce a valid transaction. */
skipTxValidation?: boolean;
};

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

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

// 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
4 changes: 2 additions & 2 deletions yarn-project/aztec.js/src/wallet/base_wallet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,9 @@ export abstract class BaseWallet implements Wallet {
txRequest: TxExecutionRequest,
simulatePublic: boolean,
msgSender?: AztecAddress,
offline?: boolean,
skipTxValidation?: boolean,
): Promise<SimulatedTx> {
return this.pxe.simulateTx(txRequest, simulatePublic, msgSender, offline, this.scopes);
return this.pxe.simulateTx(txRequest, simulatePublic, msgSender, skipTxValidation, this.scopes);
}
sendTx(tx: Tx): Promise<TxHash> {
return this.pxe.sendTx(tx);
Expand Down
7 changes: 4 additions & 3 deletions yarn-project/circuit-types/src/interfaces/aztec-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -317,11 +317,12 @@ 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.
* Returns true if the transaction is valid for inclusion at the current state. Valid transactions can be
* made invalid by *other* transactions if e.g. they emit the same nullifiers, or come become invalid
* due to e.g. the max_block_number property.
* @param tx - The transaction to validate for correctness.
*/
validateTx(tx: Tx): Promise<boolean>;
isValidTx(tx: Tx): Promise<boolean>;

/**
* Updates the configuration of this node.
Expand Down
4 changes: 2 additions & 2 deletions yarn-project/circuit-types/src/interfaces/pxe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +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 skipTxValidation - (Optional) If false, this function throws if the transaction is unable to be included in a block at the 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 @@ -191,7 +191,7 @@ export interface PXE {
txRequest: TxExecutionRequest,
simulatePublic: boolean,
msgSender?: AztecAddress,
offline?: boolean,
skipTxValidation?: boolean,
scopes?: AztecAddress[],
): Promise<SimulatedTx>;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ describe('e2e_voting_contract', () => {
// We try simulating voting again, but our TX is invalid because it will emit duplicate nullifiers
await expect(
votingContract.methods.cast_vote(candidate).simulate({
offline: false,
skipTxValidation: false,
}),
).rejects.toThrow('The simulated transaction is unable to be added to state and is invalid.');

Expand Down
8 changes: 3 additions & 5 deletions yarn-project/pxe/src/pxe_service/pxe_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,7 @@ export class PXEService implements PXE {
txRequest: TxExecutionRequest,
simulatePublic: boolean,
msgSender: AztecAddress | undefined = undefined,
offline: boolean = true,
skipTxValidation: boolean = true,
scopes?: AztecAddress[],
): Promise<SimulatedTx> {
return await this.jobQueue.put(async () => {
Expand All @@ -506,10 +506,8 @@ export class PXEService implements PXE {
simulatedTx.publicOutput = await this.#simulatePublicCalls(simulatedTx.tx);
}

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

if (!isValidTx) {
if (!skipTxValidation) {
if (!(await this.node.isValidTx(simulatedTx.tx))) {
throw new Error('The simulated transaction is unable to be added to state and is invalid.');
}
}
Expand Down

0 comments on commit bc3fffb

Please sign in to comment.