From a5d749be97e4a999f42e9d3bce45d982a6a2f5ee 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/authwit/src/account.nr | 6 +++-- .../aztec/src/context/private_context.nr | 22 ++++++++++++++----- .../src/sequencer/public_processor.ts | 5 +++++ .../src/sequencer/setup_phase_manager.ts | 3 +++ 4 files changed, 28 insertions(+), 8 deletions(-) diff --git a/noir-projects/aztec-nr/authwit/src/account.nr b/noir-projects/aztec-nr/authwit/src/account.nr index 0bc16963e25..a293dcd5f23 100644 --- a/noir-projects/aztec-nr/authwit/src/account.nr +++ b/noir-projects/aztec-nr/authwit/src/account.nr @@ -77,13 +77,15 @@ impl AccountActions { let fee_hash = fee_payload.hash(); assert(valid_fn(private_context, fee_hash)); fee_payload.execute_calls(private_context); + + dep::aztec::oracle::debug_log::debug_log("About to capture min revertible side effect counter"); private_context.capture_min_revertible_side_effect_counter(); } // docs:start:spend_private_authwit pub fn spend_private_authwit(self, inner_hash: Field) -> Field { let context = self.context.private.unwrap(); - // The `inner_hash` is "siloed" with the `msg_sender` to ensure that only it can + // The `inner_hash` is "siloed" with the `msg_sender` to ensure that only it can // consume the message. // This ensures that contracts cannot consume messages that are not intended for them. let message_hash = compute_outer_authwit_hash( @@ -102,7 +104,7 @@ impl AccountActions { // docs:start:spend_public_authwit pub fn spend_public_authwit(self, inner_hash: Field) -> Field { let context = self.context.public.unwrap(); - // The `inner_hash` is "siloed" with the `msg_sender` to ensure that only it can + // The `inner_hash` is "siloed" with the `msg_sender` to ensure that only it can // consume the message. // This ensures that contracts cannot consume messages that are not intended for them. let message_hash = compute_outer_authwit_hash( 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 c3ec9a75604..128cabaa64b 100644 --- a/noir-projects/aztec-nr/aztec/src/context/private_context.nr +++ b/noir-projects/aztec-nr/aztec/src/context/private_context.nr @@ -190,6 +190,10 @@ impl PrivateContext { pub fn capture_min_revertible_side_effect_counter(&mut self) { self.min_revertible_side_effect_counter = self.side_effect_counter; + crate::oracle::debug_log::debug_log_format( + "Captured min revertible counter {0}", + [self.min_revertible_side_effect_counter as Field] + ); } // docs:start:max-block-number @@ -347,18 +351,24 @@ 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; + let min_revertible_side_effect_counter = item.public_inputs.min_revertible_side_effect_counter; + // 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 + // (min_revertible_side_effect_counter == 0 as u32) + // | (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; - // } + if min_revertible_side_effect_counter > self.min_revertible_side_effect_counter { + self.min_revertible_side_effect_counter = min_revertible_side_effect_counter; + + crate::oracle::debug_log::debug_log_format( + "Updated min revertible counter to {0}", + [self.min_revertible_side_effect_counter as Field] + ); + } assert(contract_address.eq(item.contract_address)); assert(function_selector.eq(item.function_data.selector)); diff --git a/yarn-project/sequencer-client/src/sequencer/public_processor.ts b/yarn-project/sequencer-client/src/sequencer/public_processor.ts index 0d03d43d6d9..d65cf9229d8 100644 --- a/yarn-project/sequencer-client/src/sequencer/public_processor.ts +++ b/yarn-project/sequencer-client/src/sequencer/public_processor.ts @@ -151,6 +151,11 @@ export class PublicProcessor { private async processTxWithPublicCalls(tx: Tx): Promise<[ProcessedTx, ProcessReturnValues | undefined]> { let returnValues: ProcessReturnValues = undefined; + this.log( + `tx=${tx.getTxHash().toString()} needs setup=${tx.data.forPublic?.needsSetup} needs app=${ + tx.data.forPublic?.needsAppLogic + } needs teardown=${tx.data.forPublic?.needsTeardown}`, + ); let phase: AbstractPhaseManager | undefined = PhaseManagerFactory.phaseFromTx( tx, this.db, diff --git a/yarn-project/sequencer-client/src/sequencer/setup_phase_manager.ts b/yarn-project/sequencer-client/src/sequencer/setup_phase_manager.ts index 43e434d9b02..b9fbf2830cb 100644 --- a/yarn-project/sequencer-client/src/sequencer/setup_phase_manager.ts +++ b/yarn-project/sequencer-client/src/sequencer/setup_phase_manager.ts @@ -35,10 +35,13 @@ export class SetupPhaseManager extends AbstractPhaseManager { previousPublicKernelProof: Proof, ) { this.log(`Processing tx ${tx.getTxHash()}`); + // fix for #5614 + 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; },