fix: set side effect hwm in child private calls

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;
 use private_context::PrivateContext;

noir-projects/aztec-nr/aztec/src/context/private_context.nr

@@ -347,18 +347,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));
@@ -383,8 +372,6 @@ impl PrivateContext {
             );
         }
 
-        // crate::oracle::debug_log::debug_log_array_with_prefix("Private call stack item", item.serialize());
-
         self.private_call_stack_hashes.push(item.hash());
 
         item.public_inputs.return_values

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 }
+}

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<DeployOptions, 'fee' | 'skipClassRegistration' | 'skipPublicDeployment'>;
+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,

yarn-project/end-to-end/src/e2e_account_init_fees.test.ts

@@ -4,6 +4,7 @@ import {
   type DebugLogger,
   ExtendedNote,
   Fr,
+  type FunctionCall,
   NativeFeePaymentMethod,
   Note,
   PrivateFeePaymentMethod,
@@ -12,10 +13,18 @@ import {
   type TxHash,
   TxStatus,
   type Wallet,
+  computeAuthWitMessageHash,
   computeMessageSecretHash,
   generatePublicKey,
 } from '@aztec/aztec.js';
-import { type AztecAddress, CompleteAddress, Fq, getContractClassFromArtifact } from '@aztec/circuits.js';
+import {
+  type AztecAddress,
+  CompleteAddress,
+  Fq,
+  FunctionData,
+  FunctionSelector,
+  getContractClassFromArtifact,
+} from '@aztec/circuits.js';
 import {
   TokenContract as BananaCoin,
   FPCContract,
@@ -232,7 +241,7 @@ describe('e2e_fees_account_init', () => {
       });
     });
 
-    describe('public through an FPC', () => {
+    describe('publicly through an FPC', () => {
       let mintedPublicBananas: bigint;
 
       beforeEach(async () => {
@@ -270,9 +279,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 () => {
@@ -335,3 +369,36 @@ describe('e2e_fees_account_init', () => {
     await ctx.pxe.addNote(extendedNote);
   }
 });
+
+class BuggedSetupFeePaymentMethod extends PublicFeePaymentMethod {
+  getFunctionCalls(maxFee: Fr): Promise<FunctionCall[]> {
+    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],
+      },
+    ]);
+  }
+}

yarn-project/sequencer-client/src/sequencer/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 [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;
         },