From 47686876af99bcfceca55256d22fda020fec9c3c Mon Sep 17 00:00:00 2001 From: Alex Gherghisan Date: Tue, 9 Apr 2024 08:13:09 +0000 Subject: [PATCH] fix: set side effect hwm in child private calls --- noir-projects/aztec-nr/aztec/src/context.nr | 1 + .../aztec/src/context/private_context.nr | 13 +--- .../aztec-nr/aztec/src/context/utils.nr | 3 + .../aztec.js/src/account_manager/index.ts | 6 +- .../src/flakey_e2e_account_init_fees.test.ts | 73 ++++++++++++++++++- .../src/public/setup_phase_manager.ts | 2 + 6 files changed, 83 insertions(+), 15 deletions(-) create mode 100644 noir-projects/aztec-nr/aztec/src/context/utils.nr diff --git a/noir-projects/aztec-nr/aztec/src/context.nr b/noir-projects/aztec-nr/aztec/src/context.nr index c7d79f2e24b..67a7580c078 100644 --- a/noir-projects/aztec-nr/aztec/src/context.nr +++ b/noir-projects/aztec-nr/aztec/src/context.nr @@ -6,6 +6,7 @@ mod public_context; mod avm_context; mod interface; mod gas; +mod utils; use interface::{ ContextInterface, PrivateCallInterface, PublicCallInterface, PrivateVoidCallInterface, diff --git a/noir-projects/aztec-nr/aztec/src/context/private_context.nr b/noir-projects/aztec-nr/aztec/src/context/private_context.nr index 0fcdaa74bac..476e844b9fc 100644 --- a/noir-projects/aztec-nr/aztec/src/context/private_context.nr +++ b/noir-projects/aztec-nr/aztec/src/context/private_context.nr @@ -354,18 +354,7 @@ impl PrivateContext { assert_eq(item.public_inputs.start_side_effect_counter, self.side_effect_counter); self.side_effect_counter = item.public_inputs.end_side_effect_counter + 1; - // TODO (fees) figure out why this crashes the prover and enable it - // we need this in order to pay fees inside child call contexts - // assert( - // (item.public_inputs.min_revertible_side_effect_counter == 0 as u32) - // | (item.public_inputs.min_revertible_side_effect_counter - // > self.min_revertible_side_effect_counter) - // ); - - // if item.public_inputs.min_revertible_side_effect_counter - // > self.min_revertible_side_effect_counter { - // self.min_revertible_side_effect_counter = item.public_inputs.min_revertible_side_effect_counter; - // } + self.min_revertible_side_effect_counter = crate::context::utils::max(item.public_inputs.min_revertible_side_effect_counter, self.min_revertible_side_effect_counter); assert(contract_address.eq(item.contract_address)); assert(function_selector.eq(item.function_data.selector)); diff --git a/noir-projects/aztec-nr/aztec/src/context/utils.nr b/noir-projects/aztec-nr/aztec/src/context/utils.nr new file mode 100644 index 00000000000..8bc7b0b5524 --- /dev/null +++ b/noir-projects/aztec-nr/aztec/src/context/utils.nr @@ -0,0 +1,3 @@ +pub fn max(a: u32, b: u32) -> u32 { + if a > b { a } else { b } +} diff --git a/yarn-project/aztec.js/src/account_manager/index.ts b/yarn-project/aztec.js/src/account_manager/index.ts index 251e7d40bb3..ae13f0bb967 100644 --- a/yarn-project/aztec.js/src/account_manager/index.ts +++ b/yarn-project/aztec.js/src/account_manager/index.ts @@ -18,7 +18,10 @@ import { DeployAccountSentTx } from './deploy_account_sent_tx.js'; /** * Options to deploy an account contract. */ -export type DeployAccountOptions = Pick; +export type DeployAccountOptions = Pick< + DeployOptions, + 'fee' | 'skipClassRegistration' | 'skipPublicDeployment' | 'skipPublicSimulation' +>; /** * Manages a user account. Provides methods for calculating the account's address, deploying the account contract, @@ -167,6 +170,7 @@ export class AccountManager { .then(deployMethod => deployMethod.send({ contractAddressSalt: this.salt, + skipPublicSimulation: opts?.skipPublicSimulation ?? false, skipClassRegistration: opts?.skipClassRegistration ?? true, skipPublicDeployment: opts?.skipPublicDeployment ?? true, skipInitialization: false, diff --git a/yarn-project/end-to-end/src/flakey_e2e_account_init_fees.test.ts b/yarn-project/end-to-end/src/flakey_e2e_account_init_fees.test.ts index 1cb3ab1ba33..f434d058c13 100644 --- a/yarn-project/end-to-end/src/flakey_e2e_account_init_fees.test.ts +++ b/yarn-project/end-to-end/src/flakey_e2e_account_init_fees.test.ts @@ -4,6 +4,7 @@ import { type DebugLogger, ExtendedNote, Fr, + type FunctionCall, NativeFeePaymentMethod, Note, PrivateFeePaymentMethod, @@ -12,10 +13,19 @@ import { type TxHash, TxStatus, type Wallet, + computeAuthWitMessageHash, computeMessageSecretHash, generatePublicKey, } from '@aztec/aztec.js'; -import { type AztecAddress, CompleteAddress, DimensionGasSettings, Fq, GasSettings } from '@aztec/circuits.js'; +import { + type AztecAddress, + CompleteAddress, + DimensionGasSettings, + Fq, + FunctionData, + FunctionSelector, + GasSettings, +} from '@aztec/circuits.js'; import { TokenContract as BananaCoin, FPCContract, @@ -234,7 +244,7 @@ describe('e2e_fees_account_init', () => { }); }); - describe('public through an FPC', () => { + describe('publicly through an FPC', () => { let mintedPublicBananas: bigint; beforeEach(async () => { @@ -272,9 +282,34 @@ describe('e2e_fees_account_init', () => { sequencersInitialGas + actualFee, ]); }); + + it('failing to pay the fee drops the transaction', async () => { + await expect( + bobsAccountManager + .deploy({ + skipPublicSimulation: true, + skipPublicDeployment: false, + fee: { + maxFee, + paymentMethod: new BuggedSetupFeePaymentMethod( + bananaCoin.address, + bananaFPC.address, + await bobsAccountManager.getWallet(), + ), + }, + }) + .wait(), + ).rejects.toThrow('Tx dropped by P2P node'); + + await expect( + ctx.aztecNode.getContract(bobsAccountManager.getCompleteAddress().address), + ).resolves.toBeUndefined(); + }); }); }); + // TODO (alexg) add test to validate bugged account contracts can't grief the sequencer. Requires #5649 + describe('another account pays the fee', () => { describe('in the gas token', () => { beforeEach(async () => { @@ -337,3 +372,37 @@ describe('e2e_fees_account_init', () => { await ctx.pxe.addNote(extendedNote); } }); + +class BuggedSetupFeePaymentMethod extends PublicFeePaymentMethod { + override getFunctionCalls(gasSettings: GasSettings): Promise { + const maxFee = gasSettings.getFeeLimit(); + const nonce = Fr.random(); + const messageHash = computeAuthWitMessageHash( + this.paymentContract, + this.wallet.getChainId(), + this.wallet.getVersion(), + { + args: [this.wallet.getAddress(), this.paymentContract, maxFee, nonce], + functionData: new FunctionData( + FunctionSelector.fromSignature('transfer_public((Field),(Field),Field,Field)'), + false, + ), + to: this.asset, + }, + ); + + const tooMuchFee = new Fr(maxFee.toBigInt() * 2n); + + return Promise.resolve([ + this.wallet.setPublicAuthWit(messageHash, true).request(), + { + to: this.getPaymentContract(), + functionData: new FunctionData( + FunctionSelector.fromSignature('fee_entrypoint_public(Field,(Field),Field)'), + true, + ), + args: [tooMuchFee, this.asset, nonce], + }, + ]); + } +} diff --git a/yarn-project/simulator/src/public/setup_phase_manager.ts b/yarn-project/simulator/src/public/setup_phase_manager.ts index 9c21f610a16..80bdff3f9d9 100644 --- a/yarn-project/simulator/src/public/setup_phase_manager.ts +++ b/yarn-project/simulator/src/public/setup_phase_manager.ts @@ -35,10 +35,12 @@ export class SetupPhaseManager extends AbstractPhaseManager { previousPublicKernelProof: Proof, ) { this.log.verbose(`Processing tx ${tx.getTxHash()}`); + await this.publicContractsDB.addNewContracts(tx); const [kernelInputs, publicKernelOutput, publicKernelProof, newUnencryptedFunctionLogs, revertReason] = await this.processEnqueuedPublicCalls(tx, previousPublicKernelOutput, previousPublicKernelProof).catch( // the abstract phase manager throws if simulation gives error in a non-revertible phase async err => { + await this.publicContractsDB.removeNewContracts(tx); await this.publicStateDB.rollbackToCommit(); throw err; },