From 775f80ce89f6d88b1b9d14573fa1973d11a5d130 Mon Sep 17 00:00:00 2001 From: Mitchell Tracy Date: Wed, 6 Mar 2024 19:29:15 -0500 Subject: [PATCH] adds basic support for reversions of public functions --- .vscode/settings.json | 10 +- docs/docs/developers/tutorials/testing.md | 8 +- .../src/core/libraries/ConstantsGen.sol | 2 +- .../aztec/src/context/private_context.nr | 3 +- .../aztec/src/context/public_context.nr | 3 +- .../crates/public-kernel-lib/src/common.nr | 32 +- .../src/public_kernel_app_logic.nr | 39 +- .../src/public_kernel_setup.nr | 1 + .../src/public_kernel_teardown.nr | 15 +- .../public_kernel_circuit_public_inputs.nr | 1 + ...ic_kernel_circuit_public_inputs_builder.nr | 4 +- .../types/src/abis/public_call_stack_item.nr | 4 +- .../src/abis/public_circuit_public_inputs.nr | 6 +- .../crates/types/src/constants.nr | 2 +- .../types/src/tests/kernel_data_builder.nr | 16 +- .../public_circuit_public_inputs_builder.nr | 4 +- .../aztec-node/src/aztec-node/server.ts | 9 +- yarn-project/circuit-types/src/body.ts | 12 + .../circuit-types/src/interfaces/pxe.ts | 1 + yarn-project/circuit-types/src/mocks.ts | 6 +- .../circuit-types/src/public_data_write.ts | 4 + yarn-project/circuit-types/src/tx_effect.ts | 33 +- yarn-project/circuits.js/src/constants.gen.ts | 2 +- .../public_call_stack_item.test.ts.snap | 64 +-- .../public_circuit_public_inputs.test.ts.snap | 122 ++--- .../kernel/combined_accumulated_data.ts | 62 +++ .../public_kernel_circuit_public_inputs.ts | 34 +- .../public_circuit_public_inputs.test.ts | 2 +- .../structs/public_circuit_public_inputs.ts | 12 +- .../circuits.js/src/structs/side_effects.ts | 4 + .../circuits.js/src/tests/factories.ts | 2 + .../src/e2e_deploy_contract.test.ts | 20 +- yarn-project/end-to-end/src/e2e_fees.test.ts | 248 ++++++--- .../end-to-end/src/e2e_token_contract.test.ts | 28 +- .../src/guides/dapp_testing.test.ts | 14 +- .../end-to-end/src/shared/uniswap_l1_l2.ts | 2 +- yarn-project/foundation/src/array/array.ts | 11 + .../foundation/src/serialize/free_funcs.ts | 9 +- .../src/type_conversion.ts | 3 + .../block_builder/solo_block_builder.test.ts | 53 +- .../src/block_builder/solo_block_builder.ts | 38 +- yarn-project/sequencer-client/src/index.ts | 14 +- .../src/sequencer/abstract_phase_manager.ts | 63 +-- .../src/sequencer/app_logic_phase_manager.ts | 34 +- .../sequencer-client/src/sequencer/index.ts | 2 +- .../src/sequencer/processed_tx.ts | 164 +++++- .../src/sequencer/public_processor.test.ts | 486 ++++++++++++------ .../src/sequencer/public_processor.ts | 38 +- .../src/sequencer/sequencer.ts | 1 - .../src/sequencer/setup_phase_manager.ts | 24 +- .../src/sequencer/teardown_phase_manager.ts | 23 +- .../src/avm/temporary_executor_migration.ts | 3 + yarn-project/simulator/src/index.ts | 3 +- .../simulator/src/public/execution.ts | 26 +- yarn-project/simulator/src/public/executor.ts | 67 ++- .../simulator/src/public/index.test.ts | 49 +- .../src/public/public_execution_context.ts | 2 +- 57 files changed, 1273 insertions(+), 671 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 02a3ca363954..2e610b828f9d 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -126,7 +126,9 @@ "editor.defaultFormatter": "esbenp.prettier-vscode" }, // Now when a new file is pasted in /yellow-paper/docs/readme.md, the image file is created at /yellow-paper/docs/images/readme/image.png. - "markdown.copyFiles.destination": {"/yellow-paper/**/*": "images/${documentBaseName}/"}, + "markdown.copyFiles.destination": { + "/yellow-paper/**/*": "images/${documentBaseName}/" + }, /////////////////////////////////////// // C++/Circuits settings /////////////////////////////////////// @@ -153,11 +155,9 @@ "C_Cpp.default.enableConfigurationSquiggles": false, "C_Cpp.formatting": "disabled", "C_Cpp.vcpkg.enabled": false, - "C_Cpp.default.includePath": [ - "barretenberg/cpp/src" - ], + "C_Cpp.default.includePath": ["barretenberg/cpp/src"], "rust-analyzer.linkedProjects": [ "noir/noir-repo/Cargo.toml", "avm-transpiler/Cargo.toml" - ] + ] } diff --git a/docs/docs/developers/tutorials/testing.md b/docs/docs/developers/tutorials/testing.md index e1f9e867f14b..238438402e8a 100644 --- a/docs/docs/developers/tutorials/testing.md +++ b/docs/docs/developers/tutorials/testing.md @@ -130,18 +130,16 @@ Keep in mind that public function calls behave as in EVM blockchains, in that th #### A public call fails on the sequencer -We can ignore a local simulation error for a public function via the `skipPublicSimulation`. This will submit a failing call to the sequencer, who will then reject it and drop the transaction. +We can ignore a local simulation error for a public function via the `skipPublicSimulation`. This will submit a failing call to the sequencer, who will include the transaction, but without any side effects from our application logic. -#include_code pub-dropped /yarn-project/end-to-end/src/guides/dapp_testing.test.ts typescript - -If you run the snippet above, you'll see the following error in the Sandbox logs: +#include_code pub-reverted /yarn-project/end-to-end/src/guides/dapp_testing.test.ts typescript ``` WARN Error processing tx 06dc87c4d64462916ea58426ffcfaf20017880b353c9ec3e0f0ee5fab3ea923f: Assertion failed: Balance too low. ``` :::info -In the near future, transactions where a public function call fails will get mined but flagged as reverted, instead of dropped. +Presently, the transaction is included, but no additional information is included in the block to mark it as reverted. This will change in the near future. ::: ### State diff --git a/l1-contracts/src/core/libraries/ConstantsGen.sol b/l1-contracts/src/core/libraries/ConstantsGen.sol index d716ea644747..6026b49142b2 100644 --- a/l1-contracts/src/core/libraries/ConstantsGen.sol +++ b/l1-contracts/src/core/libraries/ConstantsGen.sol @@ -114,7 +114,7 @@ library Constants { uint256 internal constant PARTIAL_STATE_REFERENCE_LENGTH = 8; uint256 internal constant PRIVATE_CALL_STACK_ITEM_LENGTH = 223; uint256 internal constant PRIVATE_CIRCUIT_PUBLIC_INPUTS_LENGTH = 218; - uint256 internal constant PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH = 194; + uint256 internal constant PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH = 195; uint256 internal constant STATE_REFERENCE_LENGTH = 10; uint256 internal constant TX_CONTEXT_DATA_LENGTH = 11; uint256 internal constant TX_REQUEST_LENGTH = 17; 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 30c6d140273b..dffe714edb0a 100644 --- a/noir-projects/aztec-nr/aztec/src/context/private_context.nr +++ b/noir-projects/aztec-nr/aztec/src/context/private_context.nr @@ -465,7 +465,8 @@ impl PrivateContext { unencrypted_logs_hash: [0; NUM_FIELDS_PER_SHA256], unencrypted_log_preimages_length: 0, historical_header: Header::empty(), - prover_address: AztecAddress::zero() + prover_address: AztecAddress::zero(), + reverted: false }, is_execution_request: true }; diff --git a/noir-projects/aztec-nr/aztec/src/context/public_context.nr b/noir-projects/aztec-nr/aztec/src/context/public_context.nr index 50a25dd96b97..f8816f231790 100644 --- a/noir-projects/aztec-nr/aztec/src/context/public_context.nr +++ b/noir-projects/aztec-nr/aztec/src/context/public_context.nr @@ -157,7 +157,8 @@ impl PublicContext { unencrypted_logs_hash, unencrypted_log_preimages_length, historical_header: self.inputs.historical_header, - prover_address: self.prover_address + prover_address: self.prover_address, + reverted: false }; pub_circuit_pub_inputs } diff --git a/noir-projects/noir-protocol-circuits/crates/public-kernel-lib/src/common.nr b/noir-projects/noir-protocol-circuits/crates/public-kernel-lib/src/common.nr index 0d1b854a0809..0f9476bfa8d0 100644 --- a/noir-projects/noir-protocol-circuits/crates/public-kernel-lib/src/common.nr +++ b/noir-projects/noir-protocol-circuits/crates/public-kernel-lib/src/common.nr @@ -39,6 +39,14 @@ pub fn validate_inputs(public_call: PublicCallData) { assert(public_call.bytecode_hash != 0, "Bytecode hash cannot be zero"); } +pub fn initialize_reverted_flag( + previous_kernel: PublicKernelData, + public_call: PublicCallData, + circuit_outputs: &mut PublicKernelCircuitPublicInputsBuilder +) { + circuit_outputs.reverted = previous_kernel.public_inputs.reverted | public_call.call_stack_item.public_inputs.reverted; +} + pub fn initialize_end_values( previous_kernel: PublicKernelData, circuit_outputs: &mut PublicKernelCircuitPublicInputsBuilder @@ -50,21 +58,23 @@ pub fn initialize_end_values( // functions within this circuit: let start = previous_kernel.public_inputs.end; - circuit_outputs.end.new_note_hashes = array_to_bounded_vec(start.new_note_hashes); - circuit_outputs.end.new_nullifiers = array_to_bounded_vec(start.new_nullifiers); + if circuit_outputs.reverted == false { + circuit_outputs.end.new_note_hashes = array_to_bounded_vec(start.new_note_hashes); + circuit_outputs.end.new_nullifiers = array_to_bounded_vec(start.new_nullifiers); - circuit_outputs.end.private_call_stack = array_to_bounded_vec(start.private_call_stack); - circuit_outputs.end.public_call_stack = array_to_bounded_vec(start.public_call_stack); - circuit_outputs.end.new_l2_to_l1_msgs = array_to_bounded_vec(start.new_l2_to_l1_msgs); + circuit_outputs.end.private_call_stack = array_to_bounded_vec(start.private_call_stack); + circuit_outputs.end.public_call_stack = array_to_bounded_vec(start.public_call_stack); + circuit_outputs.end.new_l2_to_l1_msgs = array_to_bounded_vec(start.new_l2_to_l1_msgs); - circuit_outputs.end.public_data_update_requests = array_to_bounded_vec(start.public_data_update_requests); - circuit_outputs.end.public_data_reads = array_to_bounded_vec(start.public_data_reads); + circuit_outputs.end.public_data_update_requests = array_to_bounded_vec(start.public_data_update_requests); + circuit_outputs.end.public_data_reads = array_to_bounded_vec(start.public_data_reads); - // Public kernel does not modify encrypted logs values --> we just copy them to output - circuit_outputs.end.encrypted_logs_hash = start.encrypted_logs_hash; - circuit_outputs.end.encrypted_log_preimages_length = start.encrypted_log_preimages_length; + // Public kernel does not modify encrypted logs values --> we just copy them to output + circuit_outputs.end.encrypted_logs_hash = start.encrypted_logs_hash; + circuit_outputs.end.encrypted_log_preimages_length = start.encrypted_log_preimages_length; - circuit_outputs.end.new_contracts = array_to_bounded_vec(previous_kernel.public_inputs.end.new_contracts); + circuit_outputs.end.new_contracts = array_to_bounded_vec(previous_kernel.public_inputs.end.new_contracts); + } let start_non_revertible = previous_kernel.public_inputs.end_non_revertible; circuit_outputs.end_non_revertible.new_note_hashes = array_to_bounded_vec(start_non_revertible.new_note_hashes); diff --git a/noir-projects/noir-protocol-circuits/crates/public-kernel-lib/src/public_kernel_app_logic.nr b/noir-projects/noir-protocol-circuits/crates/public-kernel-lib/src/public_kernel_app_logic.nr index 7337baaa4091..810e32dcf37f 100644 --- a/noir-projects/noir-protocol-circuits/crates/public-kernel-lib/src/public_kernel_app_logic.nr +++ b/noir-projects/noir-protocol-circuits/crates/public-kernel-lib/src/public_kernel_app_logic.nr @@ -2,6 +2,7 @@ use dep::types::abis::public_call_data::PublicCallData; use dep::types::abis::kernel_data::{PublicKernelData}; use dep::types::PublicKernelCircuitPublicInputs; use dep::types::abis::kernel_circuit_public_inputs::PublicKernelCircuitPublicInputsBuilder; +use dep::types::utils::arrays::array_to_bounded_vec; use crate::common; use dep::std::unsafe; @@ -22,6 +23,7 @@ impl PublicKernelAppLogicCircuitPrivateInputs { fn public_kernel_app_logic(self) -> PublicKernelCircuitPublicInputs { // construct the circuit outputs let mut public_inputs: PublicKernelCircuitPublicInputsBuilder = unsafe::zeroed(); + common::initialize_reverted_flag(self.previous_kernel, self.public_call, &mut public_inputs); // initialise the end state with our provided previous kernel state common::initialize_end_values(self.previous_kernel, &mut public_inputs); @@ -32,18 +34,31 @@ impl PublicKernelAppLogicCircuitPrivateInputs { // validate the inputs unique to having a previous public kernel self.validate_inputs(); - // Pops the item from the call stack and validates it against the current execution. - let call_request = public_inputs.end.public_call_stack.pop(); - common::validate_call_against_request(self.public_call, call_request); - - common::update_public_end_values(self.public_call, &mut public_inputs); - - common::accumulate_unencrypted_logs( - self.public_call, - self.previous_kernel.public_inputs.end.unencrypted_logs_hash, - self.previous_kernel.public_inputs.end.unencrypted_log_preimages_length, - &mut public_inputs - ); + if public_inputs.reverted == false { + // Pops the item from the call stack and validates it against the current execution. + let call_request = public_inputs.end.public_call_stack.pop(); + common::validate_call_against_request(self.public_call, call_request); + common::update_public_end_values(self.public_call, &mut public_inputs); + common::accumulate_unencrypted_logs( + self.public_call, + self.previous_kernel.public_inputs.end.unencrypted_logs_hash, + self.previous_kernel.public_inputs.end.unencrypted_log_preimages_length, + &mut public_inputs + ); + } else { + let mut remaining_calls = array_to_bounded_vec(self.previous_kernel.public_inputs.end.public_call_stack); + let reverted_call_request = remaining_calls.pop(); + // even though we reverted, we still need to make sure the correct call was made + // but don't do the full `validate_call_against_request` because + // that makes a bunch of assertions that we don't want to make + // e.g. that msg_sender is self in the case of internal. + // We don't want to make those checks because we already know we reverted, + // and we aren't updating the public end values, so we want this kernel circuit to solve. + // So just check that the call request is the same as the one we expected. + assert( + reverted_call_request.hash == self.public_call.call_stack_item.hash(), "calculated public_kernel_inputs_hash does not match provided public_kernel_inputs_hash at the top of the call stack" + ); + } public_inputs.to_inner() } diff --git a/noir-projects/noir-protocol-circuits/crates/public-kernel-lib/src/public_kernel_setup.nr b/noir-projects/noir-protocol-circuits/crates/public-kernel-lib/src/public_kernel_setup.nr index 444c47189f1c..448860bb4c17 100644 --- a/noir-projects/noir-protocol-circuits/crates/public-kernel-lib/src/public_kernel_setup.nr +++ b/noir-projects/noir-protocol-circuits/crates/public-kernel-lib/src/public_kernel_setup.nr @@ -38,6 +38,7 @@ impl PublicKernelSetupCircuitPrivateInputs { fn public_kernel_setup(self) -> PublicKernelCircuitPublicInputs { // construct the circuit outputs let mut public_inputs: PublicKernelCircuitPublicInputsBuilder = unsafe::zeroed(); + common::initialize_reverted_flag(self.previous_kernel, self.public_call, &mut public_inputs); // initialise the end state with our provided previous kernel state common::initialize_end_values(self.previous_kernel, &mut public_inputs); diff --git a/noir-projects/noir-protocol-circuits/crates/public-kernel-lib/src/public_kernel_teardown.nr b/noir-projects/noir-protocol-circuits/crates/public-kernel-lib/src/public_kernel_teardown.nr index 6460db3d1081..ae6af32e33fb 100644 --- a/noir-projects/noir-protocol-circuits/crates/public-kernel-lib/src/public_kernel_teardown.nr +++ b/noir-projects/noir-protocol-circuits/crates/public-kernel-lib/src/public_kernel_teardown.nr @@ -26,6 +26,7 @@ impl PublicKernelTeardownCircuitPrivateInputs { fn public_kernel_teardown(self) -> PublicKernelCircuitPublicInputs { // construct the circuit outputs let mut public_inputs: PublicKernelCircuitPublicInputsBuilder = unsafe::zeroed(); + common::initialize_reverted_flag(self.previous_kernel, self.public_call, &mut public_inputs); // initialise the end state with our provided previous kernel state common::initialize_end_values(self.previous_kernel, &mut public_inputs); @@ -42,12 +43,14 @@ impl PublicKernelTeardownCircuitPrivateInputs { common::update_public_end_non_revertible_values(self.public_call, &mut public_inputs); - common::accumulate_unencrypted_logs( - self.public_call, - self.previous_kernel.public_inputs.end.unencrypted_logs_hash, - self.previous_kernel.public_inputs.end.unencrypted_log_preimages_length, - &mut public_inputs - ); + if public_inputs.reverted == false { + common::accumulate_unencrypted_logs( + self.public_call, + self.previous_kernel.public_inputs.end.unencrypted_logs_hash, + self.previous_kernel.public_inputs.end.unencrypted_log_preimages_length, + &mut public_inputs + ); + } let mut output = public_inputs.to_inner(); diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/abis/kernel_circuit_public_inputs/public_kernel_circuit_public_inputs.nr b/noir-projects/noir-protocol-circuits/crates/types/src/abis/kernel_circuit_public_inputs/public_kernel_circuit_public_inputs.nr index a581a992d19e..3d87bfd443f4 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/abis/kernel_circuit_public_inputs/public_kernel_circuit_public_inputs.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/abis/kernel_circuit_public_inputs/public_kernel_circuit_public_inputs.nr @@ -13,4 +13,5 @@ struct PublicKernelCircuitPublicInputs { needs_setup: bool, needs_app_logic: bool, needs_teardown: bool, + reverted: bool, } diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/abis/kernel_circuit_public_inputs/public_kernel_circuit_public_inputs_builder.nr b/noir-projects/noir-protocol-circuits/crates/types/src/abis/kernel_circuit_public_inputs/public_kernel_circuit_public_inputs_builder.nr index 022d7bf7fc22..2bcc215e6e7d 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/abis/kernel_circuit_public_inputs/public_kernel_circuit_public_inputs_builder.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/abis/kernel_circuit_public_inputs/public_kernel_circuit_public_inputs_builder.nr @@ -11,6 +11,7 @@ struct PublicKernelCircuitPublicInputsBuilder { end_non_revertible: AccumulatedNonRevertibleDataBuilder, end: AccumulatedRevertibleDataBuilder, constants: CombinedConstantData, + reverted: bool, } impl PublicKernelCircuitPublicInputsBuilder { @@ -24,7 +25,8 @@ impl PublicKernelCircuitPublicInputsBuilder { constants: self.constants, needs_setup: end_non_revertible.needs_setup(), needs_app_logic: end.needs_app_logic(), - needs_teardown: end_non_revertible.needs_teardown() + needs_teardown: end_non_revertible.needs_teardown(), + reverted: self.reverted } } } diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/abis/public_call_stack_item.nr b/noir-projects/noir-protocol-circuits/crates/types/src/abis/public_call_stack_item.nr index 6ea95afbd7e8..bf448d685a1c 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/abis/public_call_stack_item.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/abis/public_call_stack_item.nr @@ -69,7 +69,7 @@ mod tests { let call_stack_item = PublicCallStackItem { contract_address, public_inputs, is_execution_request: true, function_data }; // Value from public_call_stack_item.test.ts "Computes a callstack item request hash" test - assert_eq(call_stack_item.hash(), 0x2812dfeffdb7553fbbdd27c03fbdf61e3aa9bab3209db39f78838508ad892803); + assert_eq(call_stack_item.hash(), 0x2f3c7c0a0a0611feaba860e7c1022b18b6a9da1db41e3b4a65e535553e371765); } #[test] @@ -86,6 +86,6 @@ mod tests { let call_stack_item = PublicCallStackItem { contract_address, public_inputs, is_execution_request: false, function_data }; // Value from public_call_stack_item.test.ts "Computes a callstack item hash" test - assert_eq(call_stack_item.hash(), 0x1f71c0d6bd03e409df694549b6aa83d706cfe55427152e6ec443ec64fa62d3a0); + assert_eq(call_stack_item.hash(), 0x21a57c25d064f611e94bcbd6729cdf7d4194c98e8c58ad4703c6315dfbd7e1d9); } } diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/abis/public_circuit_public_inputs.nr b/noir-projects/noir-protocol-circuits/crates/types/src/abis/public_circuit_public_inputs.nr index 92968c80264d..a8402f14a64f 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/abis/public_circuit_public_inputs.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/abis/public_circuit_public_inputs.nr @@ -38,6 +38,8 @@ struct PublicCircuitPublicInputs{ historical_header: Header, prover_address: AztecAddress, + + reverted: bool, } impl Eq for PublicCircuitPublicInputs { @@ -73,6 +75,7 @@ impl Serialize for PublicCircuitPublicInput fields.push(self.unencrypted_log_preimages_length); fields.extend_from_array(self.historical_header.serialize()); fields.push(self.prover_address.to_field()); + fields.push(self.reverted as Field); fields.storage } } @@ -95,6 +98,7 @@ impl Deserialize for PublicCircuitPublicInp unencrypted_log_preimages_length: reader.read(), historical_header: reader.read_struct(Header::deserialize), prover_address: reader.read_struct(AztecAddress::deserialize), + reverted: reader.read() as bool, }; reader.finish(); @@ -122,5 +126,5 @@ fn empty_hash() { let hash = inputs.hash(); // Value from public_circuit_public_inputs.test.ts "computes empty item hash" test - assert_eq(hash, 0x0d43290c164ebc3d80d4d17f1939482d9d01ad503cebceb8c665d2bd96597a68); + assert_eq(hash, 0x022c1c742521fb12c0d40c223b953d6499be7de29d6cc62f80ae141bbf4cd9a3); } diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr b/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr index b9089b7d6f4c..9626a6a80e48 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr @@ -168,7 +168,7 @@ global PRIVATE_CALL_STACK_ITEM_LENGTH: u64 = 223; // constant as well PRIVATE_CALL_STACK_ITEM_LENGTH global PRIVATE_CIRCUIT_PUBLIC_INPUTS_LENGTH: u64 = 218; // Change this ONLY if you have changed the PublicCircuitPublicInputs structure. -global PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH: u64 = 194; +global PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH: u64 = 195; global STATE_REFERENCE_LENGTH: u64 = 10; // 2 for snap + 8 for partial global TX_CONTEXT_DATA_LENGTH: u64 = 11; global TX_REQUEST_LENGTH: u64 = 17; diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/tests/kernel_data_builder.nr b/noir-projects/noir-protocol-circuits/crates/types/src/tests/kernel_data_builder.nr index 31704bb1b6ea..f11f1862bf46 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/tests/kernel_data_builder.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/tests/kernel_data_builder.nr @@ -35,7 +35,8 @@ struct PreviousKernelDataBuilder { vk_index: u32, vk_path: [Field; VK_TREE_HEIGHT], sideffect_counter: u32, - min_revertible_side_effect_counter: u32 + min_revertible_side_effect_counter: u32, + reverted: bool } impl PreviousKernelDataBuilder { @@ -69,7 +70,8 @@ impl PreviousKernelDataBuilder { vk_index: 0, vk_path: [0; VK_TREE_HEIGHT], sideffect_counter: 2, - min_revertible_side_effect_counter: 2 + min_revertible_side_effect_counter: 2, + reverted: false } } @@ -144,6 +146,11 @@ impl PreviousKernelDataBuilder { first_nullifier + nullifier_index as Field } + fn get_mock_nullifier_value_non_revertible(self, nullifier_index: u64) -> Field { + let first_nullifier = self.end_non_revertible.new_nullifiers.get(0); + first_nullifier.value + nullifier_index as Field + } + pub fn append_new_nullifiers_from_private(&mut self, num_extra_nullifier: u64) { // in private kernel, the nullifiers have not yet been partitioned // (that is part of the job of the private kernel tail) @@ -169,7 +176,7 @@ impl PreviousKernelDataBuilder { if i <= num_extra_nullifier { self.end.new_nullifiers.push( SideEffectLinkedToNoteHash { - value: self.get_mock_nullifier_value(index_offset + i), + value: self.get_mock_nullifier_value_non_revertible(index_offset + i), note_hash: 0, counter: self.next_sideffect_counter() } @@ -292,7 +299,8 @@ impl PreviousKernelDataBuilder { constants: CombinedConstantData { historical_header: self.historical_header, tx_context: self.tx_context }, needs_setup: end_non_revertible.needs_setup(), needs_app_logic: end.needs_app_logic(), - needs_teardown: end_non_revertible.needs_teardown() + needs_teardown: end_non_revertible.needs_teardown(), + reverted: self.reverted }; PublicKernelData { public_inputs, proof: self.proof, vk: self.vk, vk_index: self.vk_index, vk_path: self.vk_path } diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/tests/public_circuit_public_inputs_builder.nr b/noir-projects/noir-protocol-circuits/crates/types/src/tests/public_circuit_public_inputs_builder.nr index 8e4e7923f1f6..f6e8c0a0cc06 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/tests/public_circuit_public_inputs_builder.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/tests/public_circuit_public_inputs_builder.nr @@ -27,6 +27,7 @@ struct PublicCircuitPublicInputsBuilder { unencrypted_log_preimages_length: Field, historical_header: Header, prover_address: AztecAddress, + reverted: bool, } impl PublicCircuitPublicInputsBuilder { @@ -51,7 +52,8 @@ impl PublicCircuitPublicInputsBuilder { unencrypted_logs_hash: self.unencrypted_logs_hash, unencrypted_log_preimages_length: self.unencrypted_log_preimages_length, historical_header: self.historical_header, - prover_address: self.prover_address + prover_address: self.prover_address, + reverted: self.reverted } } } diff --git a/yarn-project/aztec-node/src/aztec-node/server.ts b/yarn-project/aztec-node/src/aztec-node/server.ts index 3de03460588d..aeb6588e1bfa 100644 --- a/yarn-project/aztec-node/src/aztec-node/server.ts +++ b/yarn-project/aztec-node/src/aztec-node/server.ts @@ -52,6 +52,7 @@ import { SequencerClient, WASMSimulator, getGlobalVariableBuilder, + partitionReverts, } from '@aztec/sequencer-client'; import { ContractClassPublic, ContractInstanceWithAddress } from '@aztec/types/contracts'; import { @@ -609,10 +610,16 @@ export class AztecNodeService implements AztecNode { new WASMSimulator(), ); const processor = await publicProcessorFactory.create(prevHeader, newGlobalVariables); - const [, failedTxs] = await processor.process([tx]); + const [processedTxs, failedTxs] = await processor.process([tx]); if (failedTxs.length) { + this.log.warn(`Simulated tx ${tx.getTxHash()} fails: ${failedTxs[0].error}`); throw failedTxs[0].error; } + const { reverted } = partitionReverts(processedTxs); + if (reverted.length) { + this.log.warn(`Simulated tx ${tx.getTxHash()} reverts: ${reverted[0].revertReason}`); + throw reverted[0].revertReason; + } this.log.info(`Simulated tx ${tx.getTxHash()} succeeds`); } diff --git a/yarn-project/circuit-types/src/body.ts b/yarn-project/circuit-types/src/body.ts index 9d1904b16d2d..b77c2160a17e 100644 --- a/yarn-project/circuit-types/src/body.ts +++ b/yarn-project/circuit-types/src/body.ts @@ -6,6 +6,8 @@ import { sha256 } from '@aztec/foundation/crypto'; import { Fr } from '@aztec/foundation/fields'; import { BufferReader, Tuple, serializeToBuffer } from '@aztec/foundation/serialize'; +import { inspect } from 'util'; + export class Body { constructor( public l1ToL2Messages: Tuple, @@ -34,6 +36,16 @@ export class Body { ); } + [inspect.custom]() { + // print non empty l2ToL1Messages and txEffects + const l1ToL2Messages = this.l1ToL2Messages.filter(h => !h.isZero()); + + return `Body { + l1ToL2Messages: ${inspect(l1ToL2Messages)}, + txEffects: ${inspect(this.txEffects)}, +}`; + } + /** * Computes the transactions effects hash for the L2 block * This hash is also computed in the `AvailabilityOracle` and the `Circuit`. diff --git a/yarn-project/circuit-types/src/interfaces/pxe.ts b/yarn-project/circuit-types/src/interfaces/pxe.ts index 62f07cfdbae8..46915f8ce2e0 100644 --- a/yarn-project/circuit-types/src/interfaces/pxe.ts +++ b/yarn-project/circuit-types/src/interfaces/pxe.ts @@ -124,6 +124,7 @@ export interface PXE { * @param simulatePublic - Whether to simulate the public part of the transaction. * @returns A transaction ready to be sent to the network for execution. * @throws If the code for the functions executed in this transaction has not been made available via `addContracts`. + * Also throws if simulatePublic is true and public simulation reverts. */ simulateTx(txRequest: TxExecutionRequest, simulatePublic: boolean): Promise; diff --git a/yarn-project/circuit-types/src/mocks.ts b/yarn-project/circuit-types/src/mocks.ts index e82e59428b08..c34dde0eed03 100644 --- a/yarn-project/circuit-types/src/mocks.ts +++ b/yarn-project/circuit-types/src/mocks.ts @@ -31,12 +31,12 @@ export function makeEmptyLogs(): TxL2Logs { export const randomTxHash = (): TxHash => new TxHash(randomBytes(32)); -export const mockTx = (seed = 1) => { +export const mockTx = (seed = 1, logs = true) => { const tx = new Tx( makePrivateKernelTailCircuitPublicInputs(seed), new Proof(Buffer.alloc(0)), - TxL2Logs.random(8, 3), // 8 priv function invocations creating 3 encrypted logs each - TxL2Logs.random(11, 2), // 8 priv + 3 pub function invocations creating 2 unencrypted logs each + logs ? TxL2Logs.random(8, 3) : TxL2Logs.empty(), // 8 priv function invocations creating 3 encrypted logs each + logs ? TxL2Logs.random(11, 2) : TxL2Logs.empty(), // 8 priv + 3 pub function invocations creating 2 unencrypted logs each times(MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX, makePublicCallRequest), times(MAX_NEW_CONTRACTS_PER_TX, () => ExtendedContractData.random()) as Tuple< ExtendedContractData, diff --git a/yarn-project/circuit-types/src/public_data_write.ts b/yarn-project/circuit-types/src/public_data_write.ts index 2b96e6777e84..51edff938681 100644 --- a/yarn-project/circuit-types/src/public_data_write.ts +++ b/yarn-project/circuit-types/src/public_data_write.ts @@ -93,4 +93,8 @@ export class PublicDataWrite { static random(): PublicDataWrite { return new PublicDataWrite(Fr.random(), Fr.random()); } + + static isEmpty(data: PublicDataWrite): boolean { + return data.isEmpty(); + } } diff --git a/yarn-project/circuit-types/src/tx_effect.ts b/yarn-project/circuit-types/src/tx_effect.ts index c67708dee9b8..2fe7b957fc83 100644 --- a/yarn-project/circuit-types/src/tx_effect.ts +++ b/yarn-project/circuit-types/src/tx_effect.ts @@ -7,10 +7,12 @@ import { MAX_NEW_NULLIFIERS_PER_TX, MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX, } from '@aztec/circuits.js'; -import { makeTuple } from '@aztec/foundation/array'; +import { assertRightPadded, makeTuple } from '@aztec/foundation/array'; import { padArrayEnd } from '@aztec/foundation/collection'; import { sha256 } from '@aztec/foundation/crypto'; -import { BufferReader, Tuple, serializeArrayOfBufferableToVector } from '@aztec/foundation/serialize'; +import { BufferReader, Tuple, assertLength, serializeArrayOfBufferableToVector } from '@aztec/foundation/serialize'; + +import { inspect } from 'util'; export class TxEffect { constructor( @@ -97,10 +99,22 @@ export class TxEffect { } hash() { + assertLength(this.noteHashes, MAX_NEW_NOTE_HASHES_PER_TX); + assertRightPadded(this.noteHashes, Fr.ZERO); const noteHashesBuffer = Buffer.concat(this.noteHashes.map(x => x.toBuffer())); + + assertLength(this.nullifiers, MAX_NEW_NULLIFIERS_PER_TX); + assertRightPadded(this.nullifiers, Fr.ZERO); const nullifiersBuffer = Buffer.concat(this.nullifiers.map(x => x.toBuffer())); + + assertLength(this.l2ToL1Msgs, MAX_NEW_L2_TO_L1_MSGS_PER_TX); + assertRightPadded(this.l2ToL1Msgs, Fr.ZERO); const newL2ToL1MsgsBuffer = Buffer.concat(this.l2ToL1Msgs.map(x => x.toBuffer())); + + assertLength(this.publicDataWrites, MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX); + assertRightPadded(this.publicDataWrites, PublicDataWrite.empty()); const publicDataUpdateRequestsBuffer = Buffer.concat(this.publicDataWrites.map(x => x.toBuffer())); + const encryptedLogsHashKernel0 = this.encryptedLogs.hash(); const unencryptedLogsHashKernel0 = this.unencryptedLogs.hash(); @@ -148,6 +162,21 @@ export class TxEffect { return this.toBuffer().toString('hex'); } + [inspect.custom]() { + // print out the non-empty fields + + return `TxEffect { + note hashes: [${this.noteHashes.map(h => h.toString()).join(', ')}], + nullifiers: [${this.nullifiers.map(h => h.toString()).join(', ')}], + l2ToL1Msgs: [${this.l2ToL1Msgs.map(h => h.toString()).join(', ')}], + publicDataWrites: [${this.publicDataWrites.map(h => h.toString()).join(', ')}], + contractLeaves: [${this.contractLeaves.map(h => h.toString()).join(', ')}], + contractData: [${this.contractData.map(h => h.toString()).join(', ')}], + encryptedLogs: ${JSON.stringify(this.encryptedLogs.toJSON())}, + unencryptedLogs: ${JSON.stringify(this.unencryptedLogs.toJSON())} + }`; + } + /** * Deserializes an TxEffect object from a string. * @param str - String to deserialize. diff --git a/yarn-project/circuits.js/src/constants.gen.ts b/yarn-project/circuits.js/src/constants.gen.ts index 3a7db288043e..ffeb74bf57ec 100644 --- a/yarn-project/circuits.js/src/constants.gen.ts +++ b/yarn-project/circuits.js/src/constants.gen.ts @@ -99,7 +99,7 @@ export const NULLIFIER_KEY_VALIDATION_REQUEST_CONTEXT_LENGTH = 5; export const PARTIAL_STATE_REFERENCE_LENGTH = 8; export const PRIVATE_CALL_STACK_ITEM_LENGTH = 223; export const PRIVATE_CIRCUIT_PUBLIC_INPUTS_LENGTH = 218; -export const PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH = 194; +export const PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH = 195; export const STATE_REFERENCE_LENGTH = 10; export const TX_CONTEXT_DATA_LENGTH = 11; export const TX_REQUEST_LENGTH = 17; diff --git a/yarn-project/circuits.js/src/structs/__snapshots__/public_call_stack_item.test.ts.snap b/yarn-project/circuits.js/src/structs/__snapshots__/public_call_stack_item.test.ts.snap index 735f7ea94fd6..e74aa51e4b0c 100644 --- a/yarn-project/circuits.js/src/structs/__snapshots__/public_call_stack_item.test.ts.snap +++ b/yarn-project/circuits.js/src/structs/__snapshots__/public_call_stack_item.test.ts.snap @@ -1,46 +1,46 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`PublicCallStackItem Computes a callstack item hash 1`] = `"0x1f71c0d6bd03e409df694549b6aa83d706cfe55427152e6ec443ec64fa62d3a0"`; +exports[`PublicCallStackItem Computes a callstack item hash 1`] = `"0x21a57c25d064f611e94bcbd6729cdf7d4194c98e8c58ad4703c6315dfbd7e1d9"`; -exports[`PublicCallStackItem Computes a callstack item request hash 1`] = `"0x2812dfeffdb7553fbbdd27c03fbdf61e3aa9bab3209db39f78838508ad892803"`; +exports[`PublicCallStackItem Computes a callstack item request hash 1`] = `"0x2f3c7c0a0a0611feaba860e7c1022b18b6a9da1db41e3b4a65e535553e371765"`; exports[`PublicCallStackItem computes hash 1`] = ` Fr { - "asBigInt": 5564031247356020353810998760199482863796790393190610135965142067359455090525n, + "asBigInt": 10303122325746643826583747593622617947092841409264941610766787655996494469123n, "asBuffer": { "data": [ - 12, - 77, - 33, - 77, - 64, - 203, - 33, - 253, - 25, - 21, - 66, - 225, 22, - 94, + 199, + 92, + 79, + 4, + 250, + 62, + 84, + 206, + 222, + 239, 25, - 36, - 156, - 159, - 227, - 54, - 40, - 115, - 108, - 155, + 153, 82, - 234, - 144, - 55, - 50, - 211, - 151, - 93, + 66, + 3, + 105, + 117, + 249, + 128, + 101, + 228, + 187, + 37, + 196, + 173, + 44, + 162, + 71, + 186, + 88, + 3, ], "type": "Buffer", }, diff --git a/yarn-project/circuits.js/src/structs/__snapshots__/public_circuit_public_inputs.test.ts.snap b/yarn-project/circuits.js/src/structs/__snapshots__/public_circuit_public_inputs.test.ts.snap index 87eca1e4dd22..c79421fa0624 100644 --- a/yarn-project/circuits.js/src/structs/__snapshots__/public_circuit_public_inputs.test.ts.snap +++ b/yarn-project/circuits.js/src/structs/__snapshots__/public_circuit_public_inputs.test.ts.snap @@ -2,41 +2,41 @@ exports[`PublicCircuitPublicInputs computes empty item hash 1`] = ` Fr { - "asBigInt": 5998729082391453463848740365500417056769011535294882115251337368669961943656n, + "asBigInt": 982563348178838876008170273361844612000980410060290093415555892275290364323n, "asBuffer": { "data": [ - 13, - 67, - 41, + 2, + 44, + 28, + 116, + 37, + 33, + 251, + 18, + 192, + 212, 12, - 22, - 78, - 188, + 34, + 59, + 149, 61, - 128, - 212, - 209, - 127, - 25, - 57, - 72, - 45, + 100, + 153, + 190, + 125, + 226, 157, - 1, - 173, - 80, - 60, - 235, - 206, - 184, + 108, 198, - 101, - 210, - 189, - 150, - 89, - 122, - 104, + 47, + 128, + 174, + 20, + 27, + 191, + 76, + 217, + 163, ], "type": "Buffer", }, @@ -45,41 +45,41 @@ Fr { exports[`PublicCircuitPublicInputs hash matches snapshot 1`] = ` Fr { - "asBigInt": 888846281962498247810067532853731271075966629879817122598695403485138176580n, + "asBigInt": 1913686165740086250096887774400945438599678344243836938248205905816240806835n, "asBuffer": { "data": [ - 1, - 247, - 17, - 180, - 245, - 241, - 13, - 223, - 202, - 143, - 57, - 70, - 74, - 0, - 106, - 174, - 207, - 10, - 115, - 35, - 110, - 39, - 45, + 4, + 59, + 27, 164, - 201, - 195, - 236, - 184, - 208, - 18, - 202, - 68, + 246, + 232, + 134, + 146, + 93, + 64, + 64, + 188, + 181, + 94, + 93, + 129, + 25, + 220, + 166, + 209, + 217, + 189, + 205, + 188, + 37, + 22, + 205, + 207, + 149, + 88, + 91, + 179, ], "type": "Buffer", }, diff --git a/yarn-project/circuits.js/src/structs/kernel/combined_accumulated_data.ts b/yarn-project/circuits.js/src/structs/kernel/combined_accumulated_data.ts index 246f48b89831..0332423c6342 100644 --- a/yarn-project/circuits.js/src/structs/kernel/combined_accumulated_data.ts +++ b/yarn-project/circuits.js/src/structs/kernel/combined_accumulated_data.ts @@ -1,8 +1,11 @@ import { makeTuple } from '@aztec/foundation/array'; import { padArrayEnd } from '@aztec/foundation/collection'; import { Fr } from '@aztec/foundation/fields'; +import { createDebugOnlyLogger } from '@aztec/foundation/log'; import { BufferReader, Tuple, serializeToBuffer } from '@aztec/foundation/serialize'; +import { inspect } from 'util'; + import { MAX_NEW_CONTRACTS_PER_TX, MAX_NEW_L2_TO_L1_MSGS_PER_CALL, @@ -34,6 +37,8 @@ import { ReadRequestContext } from '../read_request.js'; import { SideEffect, SideEffectLinkedToNoteHash } from '../side_effects.js'; import { NewContractData } from './new_contract_data.js'; +const log = createDebugOnlyLogger('aztec:combined_accumulated_data'); + /** * Read operations from the public state tree. */ @@ -314,7 +319,13 @@ export class CombinedAccumulatedData { public static recombine( nonRevertible: PublicAccumulatedNonRevertibleData, revertible: PublicAccumulatedRevertibleData, + reverted: boolean, ): CombinedAccumulatedData { + if (reverted && !revertible.isEmpty()) { + log(inspect(revertible)); + throw new Error('Revertible data should be empty if the transaction is reverted'); + } + const newNoteHashes = padArrayEnd( [...nonRevertible.newNoteHashes, ...revertible.newNoteHashes].filter(x => !x.isEmpty()), SideEffect.empty(), @@ -474,6 +485,47 @@ export class PublicAccumulatedRevertibleData { return this.toBuffer().toString('hex'); } + isEmpty(): boolean { + return ( + this.noteHashReadRequests.every(x => x.isEmpty()) && + this.nullifierReadRequests.every(x => x.isEmpty()) && + this.nullifierKeyValidationRequests.every(x => x.isEmpty()) && + this.newNoteHashes.every(x => x.isEmpty()) && + this.newNullifiers.every(x => x.isEmpty()) && + this.privateCallStack.every(x => x.isEmpty()) && + this.publicCallStack.every(x => x.isEmpty()) && + this.newL2ToL1Msgs.every(x => x.isZero()) && + this.encryptedLogsHash.every(x => x.isZero()) && + this.unencryptedLogsHash.every(x => x.isZero()) && + this.encryptedLogPreimagesLength.isZero() && + this.unencryptedLogPreimagesLength.isZero() && + this.newContracts.every(x => x.isEmpty()) && + this.publicDataUpdateRequests.every(x => x.isEmpty()) && + this.publicDataReads.every(x => x.isEmpty()) + ); + } + + [inspect.custom]() { + // print out the non-empty fields + return `PublicAccumulatedRevertibleData { + noteHashReadRequests: [${this.noteHashReadRequests.map(h => h.toString()).join(', ')}], + nullifierReadRequests: [${this.nullifierReadRequests.map(h => h.toString()).join(', ')}], + nullifierKeyValidationRequests: [${this.nullifierKeyValidationRequests.map(h => h.toString()).join(', ')}], + newNoteHashes: [${this.newNoteHashes.map(h => h.toString()).join(', ')}], + newNullifiers: [${this.newNullifiers.map(h => h.toString()).join(', ')}], + privateCallStack: [${this.privateCallStack.map(h => h.toString()).join(', ')}], + publicCallStack: [${this.publicCallStack.map(h => h.toString()).join(', ')}], + newL2ToL1Msgs: [${this.newL2ToL1Msgs.map(h => h.toString()).join(', ')}], + encryptedLogsHash: [${this.encryptedLogsHash.map(h => h.toString()).join(', ')}], + unencryptedLogsHash: [${this.unencryptedLogsHash.map(h => h.toString()).join(', ')}], + encryptedLogPreimagesLength: ${this.encryptedLogPreimagesLength} + unencryptedLogPreimagesLength: ${this.unencryptedLogPreimagesLength} + newContracts: [${this.newContracts.map(h => h.toString()).join(', ')}], + publicDataUpdateRequests: [${this.publicDataUpdateRequests.map(h => h.toString()).join(', ')}], + publicDataReads: [${this.publicDataReads.map(h => h.toString()).join(', ')}], +}`; + } + /** * Deserializes from a buffer or reader, corresponding to a write in cpp. * @param buffer - Buffer or reader to read from. @@ -781,4 +833,14 @@ export class PublicAccumulatedNonRevertibleData { makeTuple(MAX_NON_REVERTIBLE_PUBLIC_DATA_READS_PER_TX, PublicDataRead.empty), ); } + + [inspect.custom]() { + return `PublicAccumulatedNonRevertibleData { + newNoteHashes: [${this.newNoteHashes.map(h => h.toString()).join(', ')}], + newNullifiers: [${this.newNullifiers.map(h => h.toString()).join(', ')}], + publicCallStack: [${this.publicCallStack.map(h => h.toString()).join(', ')}], + publicDataUpdateRequests: [${this.publicDataUpdateRequests.map(h => h.toString()).join(', ')}], + publicDataReads: [${this.publicDataReads.map(h => h.toString()).join(', ')}], +}`; + } } diff --git a/yarn-project/circuits.js/src/structs/kernel/public_kernel_circuit_public_inputs.ts b/yarn-project/circuits.js/src/structs/kernel/public_kernel_circuit_public_inputs.ts index 28cb85c063ff..690ecabbb82e 100644 --- a/yarn-project/circuits.js/src/structs/kernel/public_kernel_circuit_public_inputs.ts +++ b/yarn-project/circuits.js/src/structs/kernel/public_kernel_circuit_public_inputs.ts @@ -1,5 +1,7 @@ import { BufferReader, serializeToBuffer } from '@aztec/foundation/serialize'; +import { inspect } from 'util'; + import { AggregationObject } from '../aggregation_object.js'; import { CombinedAccumulatedData, @@ -44,6 +46,10 @@ export class PublicKernelCircuitPublicInputs { * Indicates whether the teardown kernel is needed. */ public needsTeardown: boolean, + /** + * Indicates whether execution of the public circuit reverted. + */ + public reverted: boolean, ) {} toBuffer() { @@ -55,12 +61,17 @@ export class PublicKernelCircuitPublicInputs { this.needsSetup, this.needsAppLogic, this.needsTeardown, + this.reverted, ); } get combinedData() { + if (this.needsSetup || this.needsAppLogic || this.needsTeardown) { + throw new Error('Cannot combine data when the circuit is not finished'); + } + if (!this.combined) { - this.combined = CombinedAccumulatedData.recombine(this.endNonRevertibleData, this.end); + this.combined = CombinedAccumulatedData.recombine(this.endNonRevertibleData, this.end, this.reverted); } return this.combined; } @@ -80,6 +91,7 @@ export class PublicKernelCircuitPublicInputs { reader.readBoolean(), reader.readBoolean(), reader.readBoolean(), + reader.readBoolean(), ); } @@ -89,9 +101,23 @@ export class PublicKernelCircuitPublicInputs { PublicAccumulatedNonRevertibleData.empty(), PublicAccumulatedRevertibleData.empty(), CombinedConstantData.empty(), - true, - true, - true, + false, + false, + false, + false, ); } + + [inspect.custom]() { + return `PublicKernelCircuitPublicInputs { + aggregationObject: ${this.aggregationObject}, + endNonRevertibleData: ${inspect(this.endNonRevertibleData)}, + end: ${inspect(this.end)}, + constants: ${this.constants}, + needsSetup: ${this.needsSetup}, + needsAppLogic: ${this.needsAppLogic}, + needsTeardown: ${this.needsTeardown}, + reverted: ${this.reverted} + }`; + } } diff --git a/yarn-project/circuits.js/src/structs/public_circuit_public_inputs.test.ts b/yarn-project/circuits.js/src/structs/public_circuit_public_inputs.test.ts index 1030dfec73ea..a69c18ad6ecc 100644 --- a/yarn-project/circuits.js/src/structs/public_circuit_public_inputs.test.ts +++ b/yarn-project/circuits.js/src/structs/public_circuit_public_inputs.test.ts @@ -35,6 +35,6 @@ describe('PublicCircuitPublicInputs', () => { expect(hash).toMatchSnapshot(); // Value used in empty_hash test in public_circuit_public_inputs.nr - // console.log("hash", hash.toString()); + // console.log('hash', hash.toString()); }); }); diff --git a/yarn-project/circuits.js/src/structs/public_circuit_public_inputs.ts b/yarn-project/circuits.js/src/structs/public_circuit_public_inputs.ts index b04124320481..bad67169449d 100644 --- a/yarn-project/circuits.js/src/structs/public_circuit_public_inputs.ts +++ b/yarn-project/circuits.js/src/structs/public_circuit_public_inputs.ts @@ -87,6 +87,11 @@ export class PublicCircuitPublicInputs { * Address of the prover. */ public proverAddress: AztecAddress, + + /** + * Flag indicating if the call was reverted. + */ + public reverted: boolean, ) {} /** @@ -117,6 +122,7 @@ export class PublicCircuitPublicInputs { Fr.ZERO, Header.empty(), AztecAddress.ZERO, + false, ); } @@ -138,7 +144,8 @@ export class PublicCircuitPublicInputs { isFrArrayEmpty(this.unencryptedLogsHash) && this.unencryptedLogPreimagesLength.isZero() && this.historicalHeader.isEmpty() && - this.proverAddress.isZero() + this.proverAddress.isZero() && + this.reverted === false ); } @@ -162,6 +169,7 @@ export class PublicCircuitPublicInputs { fields.unencryptedLogPreimagesLength, fields.historicalHeader, fields.proverAddress, + fields.reverted, ] as const; } @@ -204,6 +212,7 @@ export class PublicCircuitPublicInputs { reader.readObject(Fr), reader.readObject(Header), reader.readObject(AztecAddress), + reader.readBoolean(), ); } @@ -224,6 +233,7 @@ export class PublicCircuitPublicInputs { reader.readField(), Header.fromFields(reader), AztecAddress.fromFields(reader), + reader.readBoolean(), ); } diff --git a/yarn-project/circuits.js/src/structs/side_effects.ts b/yarn-project/circuits.js/src/structs/side_effects.ts index 98fdaa715e93..9e09b15255a3 100644 --- a/yarn-project/circuits.js/src/structs/side_effects.ts +++ b/yarn-project/circuits.js/src/structs/side_effects.ts @@ -113,6 +113,10 @@ export class SideEffectLinkedToNoteHash implements SideEffectType { public counter: Fr, ) {} + toString(): string { + return `value=${this.value.toString()} noteHash=${this.noteHash.toString()} counter=${this.counter.toString()}`; + } + /** * Serialize this as a buffer. * @returns The buffer. diff --git a/yarn-project/circuits.js/src/tests/factories.ts b/yarn-project/circuits.js/src/tests/factories.ts index 3f5297457cdb..6736fb61e248 100644 --- a/yarn-project/circuits.js/src/tests/factories.ts +++ b/yarn-project/circuits.js/src/tests/factories.ts @@ -433,6 +433,7 @@ export function makePublicCircuitPublicInputs( fr(seed + 0x902), makeHeader(seed + 0xa00, undefined), makeAztecAddress(seed + 0xb01), + false, // reverted ); } @@ -453,6 +454,7 @@ export function makePublicKernelCircuitPublicInputs( true, true, true, + false, ); } diff --git a/yarn-project/end-to-end/src/e2e_deploy_contract.test.ts b/yarn-project/end-to-end/src/e2e_deploy_contract.test.ts index 39051bb7c0bd..dffd844c7410 100644 --- a/yarn-project/end-to-end/src/e2e_deploy_contract.test.ts +++ b/yarn-project/end-to-end/src/e2e_deploy_contract.test.ts @@ -13,6 +13,7 @@ import { Fr, PXE, SignerlessWallet, + TxStatus, Wallet, getContractClassFromArtifact, getContractInstanceFromDeployParams, @@ -146,16 +147,18 @@ describe('e2e_deploy_contract', () => { const [goodTxPromiseResult, badTxReceiptResult] = await Promise.allSettled([goodTx.wait(), badTx.wait()]); expect(goodTxPromiseResult.status).toBe('fulfilled'); - expect(badTxReceiptResult.status).toBe('rejected'); + expect(badTxReceiptResult.status).toBe('fulfilled'); // but reverted const [goodTxReceipt, badTxReceipt] = await Promise.all([goodTx.getReceipt(), badTx.getReceipt()]); expect(goodTxReceipt.blockNumber).toEqual(expect.any(Number)); - expect(badTxReceipt.blockNumber).toBeUndefined(); + // the bad transaction is included + expect(badTxReceipt.blockNumber).toEqual(expect.any(Number)); await expect(pxe.getContractData(goodDeploy.getInstance().address)).resolves.toBeDefined(); await expect(pxe.getExtendedContractData(goodDeploy.getInstance().address)).resolves.toBeDefined(); + // but did not deploy the contract await expect(pxe.getContractData(badDeploy.getInstance().address)).resolves.toBeUndefined(); await expect(pxe.getExtendedContractData(badDeploy.getInstance().address)).resolves.toBeUndefined(); } finally { @@ -362,12 +365,13 @@ describe('e2e_deploy_contract', () => { }, 30_000); it('refuses to call a public function with init check if the instance is not initialized', async () => { - await expect( - contract.methods - .increment_public_value(AztecAddress.random(), 10) - .send({ skipPublicSimulation: true }) - .wait(), - ).rejects.toThrow(/dropped/i); + // TODO(#4972) check for reverted flag + const receipt = await contract.methods + .increment_public_value(AztecAddress.random(), 10) + .send({ skipPublicSimulation: true }) + .wait(); + + expect(receipt.status).toEqual(TxStatus.MINED); }, 30_000); it('calls a public function with init check after initialization', async () => { diff --git a/yarn-project/end-to-end/src/e2e_fees.test.ts b/yarn-project/end-to-end/src/e2e_fees.test.ts index d6e25bb8a84f..5aeb83d3443d 100644 --- a/yarn-project/end-to-end/src/e2e_fees.test.ts +++ b/yarn-project/end-to-end/src/e2e_fees.test.ts @@ -1,24 +1,39 @@ import { AztecAddress, + DebugLogger, ExtendedNote, Fr, FunctionSelector, Note, PrivateFeePaymentMethod, + PublicFeePaymentMethod, TxHash, computeMessageSecretHash, } from '@aztec/aztec.js'; -import { decodeFunctionSignature } from '@aztec/foundation/abi'; -import { TokenContract as BananaCoin, FPCContract, GasTokenContract } from '@aztec/noir-contracts.js'; +import { ContractArtifact, decodeFunctionSignature } from '@aztec/foundation/abi'; +import { + TokenContract as BananaCoin, + FPCContract, + GasTokenContract, + SchnorrAccountContract, +} from '@aztec/noir-contracts.js'; import { jest } from '@jest/globals'; -import { BalancesFn, EndToEndContext, expectMapping, getBalancesFn, setup } from './fixtures/utils.js'; +import { + BalancesFn, + EndToEndContext, + expectMapping, + getBalancesFn, + publicDeployAccounts, + setup, +} from './fixtures/utils.js'; import { GasPortalTestingHarnessFactory, IGasBridgingTestHarness } from './shared/gas_portal_test_harness.js'; const TOKEN_NAME = 'BananaCoin'; const TOKEN_SYMBOL = 'BC'; const TOKEN_DECIMALS = 18n; +const BRIDGED_FPC_GAS = 500n; jest.setTimeout(100_000); @@ -40,7 +55,19 @@ describe('e2e_fees', () => { process.env.PXE_URL = ''; e2eContext = await setup(3); - const { wallets, accounts, logger, aztecNode, pxe, deployL1ContractsValues } = e2eContext; + const { accounts, logger, aztecNode, pxe, deployL1ContractsValues, wallets } = e2eContext; + + logFunctionSignatures(BananaCoin.artifact, logger); + logFunctionSignatures(FPCContract.artifact, logger); + logFunctionSignatures(GasTokenContract.artifact, logger); + logFunctionSignatures(SchnorrAccountContract.artifact, logger); + + await aztecNode.setConfig({ + feeRecipient: accounts.at(-1)!.address, + }); + + aliceAddress = accounts.at(0)!.address; + sequencerAddress = accounts.at(-1)!.address; gasBridgeTestHarness = await GasPortalTestingHarnessFactory.create({ pxeService: pxe, @@ -53,82 +80,146 @@ describe('e2e_fees', () => { gasTokenContract = gasBridgeTestHarness.l2Token; - BananaCoin.artifact.functions.forEach(fn => { - const sig = decodeFunctionSignature(fn.name, fn.parameters); - logger(`Function ${sig} and the selector: ${FunctionSelector.fromNameAndParameters(fn.name, fn.parameters)}`); - }); - FPCContract.artifact.functions.forEach(fn => { - const sig = decodeFunctionSignature(fn.name, fn.parameters); - logger(`Function ${sig} and the selector: ${FunctionSelector.fromNameAndParameters(fn.name, fn.parameters)}`); - }); - GasTokenContract.artifact.functions.forEach(fn => { - const sig = decodeFunctionSignature(fn.name, fn.parameters); - logger(`Function ${sig} and the selector: ${FunctionSelector.fromNameAndParameters(fn.name, fn.parameters)}`); - }); - - await aztecNode.setConfig({ - feeRecipient: accounts.at(-1)!.address, - }); - - aliceAddress = accounts.at(0)!.address; - sequencerAddress = accounts.at(-1)!.address; - }, 30_000); - - const InitialFPCGas = 500n; - beforeEach(async () => { - bananaCoin = await BananaCoin.deploy( - e2eContext.wallets[0], - e2eContext.accounts[0], - TOKEN_NAME, - TOKEN_SYMBOL, - TOKEN_DECIMALS, - ) + bananaCoin = await BananaCoin.deploy(wallets[0], accounts[0], TOKEN_NAME, TOKEN_SYMBOL, TOKEN_DECIMALS) .send() .deployed(); - e2eContext.logger(`BananaCoin deployed at ${bananaCoin.address}`); + logger(`BananaCoin deployed at ${bananaCoin.address}`); - bananaFPC = await FPCContract.deploy(e2eContext.wallets[0], bananaCoin.address, gasTokenContract.address) - .send() - .deployed(); - e2eContext.logger(`bananaPay deployed at ${bananaFPC.address}`); - await gasBridgeTestHarness.bridgeFromL1ToL2(InitialFPCGas + 1n, InitialFPCGas, bananaFPC.address); + bananaFPC = await FPCContract.deploy(wallets[0], bananaCoin.address, gasTokenContract.address).send().deployed(); + logger(`bananaPay deployed at ${bananaFPC.address}`); + await publicDeployAccounts(wallets[0], accounts); + + await gasBridgeTestHarness.bridgeFromL1ToL2(BRIDGED_FPC_GAS, BRIDGED_FPC_GAS, bananaFPC.address); - gasBalances = getBalancesFn('⛽', gasTokenContract.methods.balance_of_public, e2eContext.logger); - bananaPublicBalances = getBalancesFn('🍌.public', bananaCoin.methods.balance_of_public, e2eContext.logger); - bananaPrivateBalances = getBalancesFn('🍌.private', bananaCoin.methods.balance_of_private, e2eContext.logger); + bananaPublicBalances = getBalancesFn('🍌.public', bananaCoin.methods.balance_of_public, logger); + bananaPrivateBalances = getBalancesFn('🍌.private', bananaCoin.methods.balance_of_private, logger); + gasBalances = getBalancesFn('⛽', gasTokenContract.methods.balance_of_public, logger); await expectMapping(bananaPrivateBalances, [aliceAddress, bananaFPC.address, sequencerAddress], [0n, 0n, 0n]); await expectMapping(bananaPublicBalances, [aliceAddress, bananaFPC.address, sequencerAddress], [0n, 0n, 0n]); - await expectMapping(gasBalances, [aliceAddress, bananaFPC.address, sequencerAddress], [0n, InitialFPCGas, 0n]); - }, 100_000); + await expectMapping(gasBalances, [aliceAddress, bananaFPC.address, sequencerAddress], [0n, BRIDGED_FPC_GAS, 0n]); + }); + + it('reverts transactions but still pays fees using PublicFeePaymentMethod', async () => { + const OutrageousPublicAmountAliceDoesNotHave = 10000n; + const PublicMintedAlicePublicBananas = 1000n; + const FeeAmount = 1n; + const RefundAmount = 2n; + const MaxFee = FeeAmount + RefundAmount; + const { wallets } = e2eContext; + + const [initialAlicePrivateBananas, initialFPCPrivateBananas] = await bananaPrivateBalances( + aliceAddress, + bananaFPC.address, + ); + const [initialAlicePublicBananas, initialFPCPublicBananas] = await bananaPublicBalances( + aliceAddress, + bananaFPC.address, + ); + const [initialAliceGas, initialFPCGas, initialSequencerGas] = await gasBalances( + aliceAddress, + bananaFPC.address, + sequencerAddress, + ); + + await bananaCoin.methods.mint_public(aliceAddress, PublicMintedAlicePublicBananas).send().wait(); + // if we simulate locally, it throws an error + await expect( + bananaCoin.methods + .transfer_public(aliceAddress, sequencerAddress, OutrageousPublicAmountAliceDoesNotHave, 0) + .send({ + fee: { + maxFee: MaxFee, + paymentMethod: new PublicFeePaymentMethod(bananaCoin.address, bananaFPC.address, wallets[0]), + }, + }) + .wait(), + ).rejects.toThrow(/attempt to subtract with underflow 'hi == high'/); + + // we did not pay the fee, because we did not submit the TX + await expectMapping( + bananaPrivateBalances, + [aliceAddress, bananaFPC.address, sequencerAddress], + [initialAlicePrivateBananas, initialFPCPrivateBananas, 0n], + ); + await expectMapping( + bananaPublicBalances, + [aliceAddress, bananaFPC.address, sequencerAddress], + [initialAlicePublicBananas + PublicMintedAlicePublicBananas, initialFPCPublicBananas, 0n], + ); + await expectMapping( + gasBalances, + [aliceAddress, bananaFPC.address, sequencerAddress], + [initialAliceGas, initialFPCGas, initialSequencerGas], + ); + + // if we skip simulation, it includes the failed TX + await bananaCoin.methods + .transfer_public(aliceAddress, sequencerAddress, OutrageousPublicAmountAliceDoesNotHave, 0) + .send({ + skipPublicSimulation: true, + fee: { + maxFee: MaxFee, + paymentMethod: new PublicFeePaymentMethod(bananaCoin.address, bananaFPC.address, wallets[0]), + }, + }) + .wait(); + + // and thus we paid the fee + await expectMapping( + bananaPrivateBalances, + [aliceAddress, bananaFPC.address, sequencerAddress], + [initialAlicePrivateBananas, initialFPCPrivateBananas, 0n], + ); + await expectMapping( + bananaPublicBalances, + [aliceAddress, bananaFPC.address, sequencerAddress], + [initialAlicePublicBananas + PublicMintedAlicePublicBananas - FeeAmount, initialFPCPublicBananas + FeeAmount, 0n], + ); + await expectMapping( + gasBalances, + [aliceAddress, bananaFPC.address, sequencerAddress], + [initialAliceGas, initialFPCGas - FeeAmount, initialSequencerGas + FeeAmount], + ); + + // TODO(#4712) - demonstrate reverts with the PrivateFeePaymentMethod. + // Can't do presently because all logs are "revertible" so we lose notes that get broadcasted during unshielding. + }); it('mint banana privately, pay privately with banana via FPC', async () => { - const PrivateInitialBananasAmount = 100n; - const MintedBananasAmount = 1000n; + const PrivateMintedBananasAmount = 100n; + const AppLogicMintedBananasAmount = 1000n; const FeeAmount = 1n; const RefundAmount = 2n; const MaxFee = FeeAmount + RefundAmount; - const { wallets, accounts } = e2eContext; + const { wallets } = e2eContext; - // Mint bananas privately - const secret = Fr.random(); - const secretHash = computeMessageSecretHash(secret); - const receipt = await bananaCoin.methods.mint_private(PrivateInitialBananasAmount, secretHash).send().wait(); + const [initialAlicePrivateBananas, initialFPCPrivateBananas] = await bananaPrivateBalances( + aliceAddress, + bananaFPC.address, + ); + const [initialAlicePublicBananas, initialFPCPublicBananas] = await bananaPublicBalances( + aliceAddress, + bananaFPC.address, + ); + const [initialAliceGas, initialFPCGas, initialSequencerGas] = await gasBalances( + aliceAddress, + bananaFPC.address, + sequencerAddress, + ); - // Setup auth wit - await addPendingShieldNoteToPXE(0, PrivateInitialBananasAmount, secretHash, receipt.txHash); - const txClaim = bananaCoin.methods.redeem_shield(accounts[0].address, PrivateInitialBananasAmount, secret).send(); - const receiptClaim = await txClaim.wait({ debug: true }); - const { visibleNotes } = receiptClaim.debugInfo!; - expect(visibleNotes[0].note.items[0].toBigInt()).toBe(PrivateInitialBananasAmount); + await mintPrivate(PrivateMintedBananasAmount, aliceAddress); await expectMapping( bananaPrivateBalances, [aliceAddress, bananaFPC.address, sequencerAddress], - [PrivateInitialBananasAmount, 0n, 0n], + [initialAlicePrivateBananas + PrivateMintedBananasAmount, initialFPCPrivateBananas, 0n], + ); + await expectMapping( + bananaPublicBalances, + [aliceAddress, bananaFPC.address, sequencerAddress], + [initialAlicePublicBananas, initialFPCPublicBananas, 0n], ); - await expectMapping(bananaPublicBalances, [aliceAddress, bananaFPC.address, sequencerAddress], [0n, 0n, 0n]); - await expectMapping(gasBalances, [aliceAddress, bananaFPC.address, sequencerAddress], [0n, InitialFPCGas, 0n]); /** * PRIVATE SETUP @@ -154,7 +245,7 @@ describe('e2e_fees', () => { * */ await bananaCoin.methods - .mint_public(aliceAddress, MintedBananasAmount) + .mint_public(aliceAddress, AppLogicMintedBananasAmount) .send({ fee: { maxFee: MaxFee, @@ -166,19 +257,44 @@ describe('e2e_fees', () => { await expectMapping( bananaPrivateBalances, [aliceAddress, bananaFPC.address, sequencerAddress], - [PrivateInitialBananasAmount - MaxFee, 0n, 0n], + [initialAlicePrivateBananas + PrivateMintedBananasAmount - MaxFee, initialFPCPrivateBananas, 0n], ); await expectMapping( bananaPublicBalances, [aliceAddress, bananaFPC.address, sequencerAddress], - [MintedBananasAmount + RefundAmount, MaxFee - RefundAmount, 0n], + [ + initialAlicePublicBananas + AppLogicMintedBananasAmount + RefundAmount, + initialFPCPublicBananas + MaxFee - RefundAmount, + 0n, + ], ); await expectMapping( gasBalances, [aliceAddress, bananaFPC.address, sequencerAddress], - [0n, InitialFPCGas - FeeAmount, FeeAmount], + [initialAliceGas, initialFPCGas - FeeAmount, initialSequencerGas + FeeAmount], ); - }, 100_000); + }); + + function logFunctionSignatures(artifact: ContractArtifact, logger: DebugLogger) { + artifact.functions.forEach(fn => { + const sig = decodeFunctionSignature(fn.name, fn.parameters); + logger(`${FunctionSelector.fromNameAndParameters(fn.name, fn.parameters)} => ${artifact.name}.${sig} `); + }); + } + + const mintPrivate = async (amount: bigint, address: AztecAddress) => { + // Mint bananas privately + const secret = Fr.random(); + const secretHash = computeMessageSecretHash(secret); + const receipt = await bananaCoin.methods.mint_private(amount, secretHash).send().wait(); + + // Setup auth wit + await addPendingShieldNoteToPXE(0, amount, secretHash, receipt.txHash); + const txClaim = bananaCoin.methods.redeem_shield(address, amount, secret).send(); + const receiptClaim = await txClaim.wait({ debug: true }); + const { visibleNotes } = receiptClaim.debugInfo!; + expect(visibleNotes[0].note.items[0].toBigInt()).toBe(amount); + }; const addPendingShieldNoteToPXE = async (accountIndex: number, amount: bigint, secretHash: Fr, txHash: TxHash) => { const storageSlot = new Fr(5); // The storage slot of `pending_shields` is 5. diff --git a/yarn-project/end-to-end/src/e2e_token_contract.test.ts b/yarn-project/end-to-end/src/e2e_token_contract.test.ts index 4d732410d6b5..8a000a73af75 100644 --- a/yarn-project/end-to-end/src/e2e_token_contract.test.ts +++ b/yarn-project/end-to-end/src/e2e_token_contract.test.ts @@ -101,30 +101,22 @@ describe('e2e_token_contract', () => { }); describe('name', () => { - it('private', async () => { - const t = toString(await asset.methods.un_get_name().view()); - expect(t).toBe(TOKEN_NAME); - - const tx = reader.methods.check_name_private(asset.address, TOKEN_NAME).send(); - const receipt = await tx.wait(); - expect(receipt.status).toBe(TxStatus.MINED); - - await expect(reader.methods.check_name_private(asset.address, 'WRONG_NAME').simulate()).rejects.toThrowError( - "Cannot satisfy constraint 'name.is_eq(_what)'", - ); - }); - - it('public', async () => { + it.each([ + ['private', 'check_name_private' as const, "Cannot satisfy constraint 'name.is_eq(_what)'"], + [ + 'public', + 'check_name_public' as const, + "Failed to solve brillig function, reason: explicit trap hit in brillig 'name.is_eq(_what)'", + ], + ])('name - %s', async (_type, method, errorMessage) => { const t = toString(await asset.methods.un_get_name().view()); expect(t).toBe(TOKEN_NAME); - const tx = reader.methods.check_name_public(asset.address, TOKEN_NAME).send(); + const tx = reader.methods[method](asset.address, TOKEN_NAME).send(); const receipt = await tx.wait(); expect(receipt.status).toBe(TxStatus.MINED); - await expect(reader.methods.check_name_public(asset.address, 'WRONG_NAME').simulate()).rejects.toThrowError( - "Failed to solve brillig function, reason: explicit trap hit in brillig 'name.is_eq(_what)'", - ); + await expect(reader.methods[method](asset.address, 'WRONG_NAME').simulate()).rejects.toThrow(errorMessage); }); }); diff --git a/yarn-project/end-to-end/src/guides/dapp_testing.test.ts b/yarn-project/end-to-end/src/guides/dapp_testing.test.ts index d4ffb9b529ae..9bf90e5e4ae6 100644 --- a/yarn-project/end-to-end/src/guides/dapp_testing.test.ts +++ b/yarn-project/end-to-end/src/guides/dapp_testing.test.ts @@ -6,6 +6,7 @@ import { Fr, Note, PXE, + TxStatus, computeMessageSecretHash, createPXEClient, waitForPXE, @@ -250,11 +251,16 @@ describe('guides/dapp/testing', () => { // docs:end:local-pub-fails }, 30_000); - it('asserts a transaction with a failing public call is dropped (until we get public reverts)', async () => { - // docs:start:pub-dropped + // TODO(#4972) update to show the transaction is included but reverted + it('asserts a transaction with a failing public call is included (with no state changes)', async () => { + // docs:start:pub-reverted const call = token.methods.transfer_public(owner.getAddress(), recipient.getAddress(), 1000n, 0); - await expect(call.send({ skipPublicSimulation: true }).wait()).rejects.toThrowError(/dropped/); - // docs:end:pub-dropped + const receipt = await call.send({ skipPublicSimulation: true }).wait(); + expect(receipt.status).toEqual(TxStatus.MINED); + const ownerPublicBalanceSlot = cheats.aztec.computeSlotInMap(6n, owner.getAddress()); + const balance = await pxe.getPublicStorageAt(token.address, ownerPublicBalanceSlot); + expect(balance.value).toEqual(100n); + // docs:end:pub-reverted }, 30_000); }); }); diff --git a/yarn-project/end-to-end/src/shared/uniswap_l1_l2.ts b/yarn-project/end-to-end/src/shared/uniswap_l1_l2.ts index dd2d892da776..6bdaf5caaae2 100644 --- a/yarn-project/end-to-end/src/shared/uniswap_l1_l2.ts +++ b/yarn-project/end-to-end/src/shared/uniswap_l1_l2.ts @@ -574,7 +574,7 @@ export const uniswapL1L2TestSuite = ( await ownerWallet.setPublicAuth(swapMessageHash, true).send().wait(); // Swap! - await expect(action.simulate()).rejects.toThrowError( + await expect(action.simulate()).rejects.toThrow( "Assertion failed: Message not authorized by account 'is_valid == true'", ); }); diff --git a/yarn-project/foundation/src/array/array.ts b/yarn-project/foundation/src/array/array.ts index 59ad84b30c6b..47dd21e9fd75 100644 --- a/yarn-project/foundation/src/array/array.ts +++ b/yarn-project/foundation/src/array/array.ts @@ -117,3 +117,14 @@ export function assertPermutation( seenValue.add(index); } } + +export function assertRightPadded(arr: T[], emptyElt: T) { + let seenEmpty = false; + for (let i = 0; i < arr.length; i++) { + if (arr[i] === emptyElt) { + seenEmpty = true; + } else if (seenEmpty) { + throw new Error(`Non-empty element at index [${i}] after empty element`); + } + } +} diff --git a/yarn-project/foundation/src/serialize/free_funcs.ts b/yarn-project/foundation/src/serialize/free_funcs.ts index 3ff2ea133a5b..5c7aa27e48ec 100644 --- a/yarn-project/foundation/src/serialize/free_funcs.ts +++ b/yarn-project/foundation/src/serialize/free_funcs.ts @@ -1,4 +1,5 @@ import { Fr } from '../fields/fields.js'; +import { Tuple } from './types.js'; /** * Convert a boolean value to its corresponding byte representation in a Buffer of size 1. @@ -139,9 +140,13 @@ export function from2Fields(field1: Fr, field2: Fr): Buffer { const buf2 = field2.toBuffer(); // Remove the padding (first 16 bytes) from each buffer - const originalPart1 = buf1.slice(16, 32); - const originalPart2 = buf2.slice(16, 32); + const originalPart1 = buf1.subarray(Fr.SIZE_IN_BYTES / 2, Fr.SIZE_IN_BYTES); + const originalPart2 = buf2.subarray(Fr.SIZE_IN_BYTES / 2, Fr.SIZE_IN_BYTES); // Concatenate the two parts to form the original buffer return Buffer.concat([originalPart1, originalPart2]); } + +export function fromFieldsTuple(fields: Tuple): Buffer { + return from2Fields(fields[0], fields[1]); +} diff --git a/yarn-project/noir-protocol-circuits-types/src/type_conversion.ts b/yarn-project/noir-protocol-circuits-types/src/type_conversion.ts index ab4badfafa7a..09424a14f9b4 100644 --- a/yarn-project/noir-protocol-circuits-types/src/type_conversion.ts +++ b/yarn-project/noir-protocol-circuits-types/src/type_conversion.ts @@ -1189,6 +1189,7 @@ export function mapPublicKernelCircuitPublicInputsToNoir( needs_setup: inputs.needsSetup, needs_app_logic: inputs.needsAppLogic, needs_teardown: inputs.needsTeardown, + reverted: inputs.reverted, }; } @@ -1410,6 +1411,7 @@ export function mapPublicKernelCircuitPublicInputsFromNoir( inputs.needs_setup, inputs.needs_app_logic, inputs.needs_teardown, + inputs.reverted, ); } @@ -1561,6 +1563,7 @@ export function mapPublicCircuitPublicInputsToNoir( historical_header: mapHeaderToNoir(publicInputs.historicalHeader), prover_address: mapAztecAddressToNoir(publicInputs.proverAddress), + reverted: publicInputs.reverted, }; } /** diff --git a/yarn-project/sequencer-client/src/block_builder/solo_block_builder.test.ts b/yarn-project/sequencer-client/src/block_builder/solo_block_builder.test.ts index c9a0131c6b39..f516983fdc67 100644 --- a/yarn-project/sequencer-client/src/block_builder/solo_block_builder.test.ts +++ b/yarn-project/sequencer-client/src/block_builder/solo_block_builder.test.ts @@ -1,13 +1,10 @@ import { Body, - ContractData, ExtendedContractData, L2Block, MerkleTreeId, - PublicDataWrite, Tx, TxEffect, - TxL2Logs, makeEmptyLogs, mockTx, } from '@aztec/circuit-types'; @@ -19,15 +16,12 @@ import { Fr, GlobalVariables, Header, - MAX_NEW_CONTRACTS_PER_TX, MAX_NEW_L2_TO_L1_MSGS_PER_TX, - MAX_NEW_NOTE_HASHES_PER_TX, MAX_NEW_NULLIFIERS_PER_TX, MAX_NON_REVERTIBLE_NOTE_HASHES_PER_TX, MAX_NON_REVERTIBLE_NULLIFIERS_PER_TX, MAX_NON_REVERTIBLE_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX, MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX, - MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX, MAX_REVERTIBLE_NOTE_HASHES_PER_TX, MAX_REVERTIBLE_NULLIFIERS_PER_TX, MAX_REVERTIBLE_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX, @@ -40,7 +34,6 @@ import { PublicDataUpdateRequest, PublicKernelCircuitPublicInputs, RootRollupPublicInputs, - SideEffect, SideEffectLinkedToNoteHash, StateReference, } from '@aztec/circuits.js'; @@ -58,10 +51,11 @@ import { import { makeTuple, range } from '@aztec/foundation/array'; import { toBufferBE } from '@aztec/foundation/bigint-buffer'; import { padArrayEnd, times } from '@aztec/foundation/collection'; -import { Tuple, to2Fields } from '@aztec/foundation/serialize'; +import { to2Fields } from '@aztec/foundation/serialize'; import { openTmpStore } from '@aztec/kv-store/utils'; import { MerkleTreeOperations, MerkleTrees } from '@aztec/world-state'; +import { jest } from '@jest/globals'; import { MockProxy, mock } from 'jest-mock-extended'; import { type MemDown, default as memdown } from 'memdown'; @@ -72,6 +66,7 @@ import { ProcessedTx, makeEmptyProcessedTx as makeEmptyProcessedTxFromHistoricalTreeRoots, makeProcessedTx, + toTxEffect, } from '../sequencer/processed_tx.js'; import { WASMSimulator } from '../simulator/acvm_wasm.js'; import { RollupSimulator } from '../simulator/index.js'; @@ -206,6 +201,9 @@ describe('sequencer/solo_block_builder', () => { const buildMockSimulatorInputs = async () => { const kernelOutput = makePrivateKernelTailCircuitPublicInputs(); kernelOutput.constants.historicalHeader = await expectsDb.buildInitialHeader(); + kernelOutput.needsAppLogic = false; + kernelOutput.needsSetup = false; + kernelOutput.needsTeardown = false; const tx = makeProcessedTx( new Tx( @@ -223,39 +221,18 @@ describe('sequencer/solo_block_builder', () => { // Calculate what would be the tree roots after the first tx and update mock circuit output await updateExpectedTreesFromTxs([txs[0]]); baseRollupOutputLeft.end = await getPartialStateReference(); + baseRollupOutputLeft.calldataHash = to2Fields(toTxEffect(tx).hash()); // Same for the tx on the right await updateExpectedTreesFromTxs([txs[1]]); baseRollupOutputRight.end = await getPartialStateReference(); + baseRollupOutputRight.calldataHash = to2Fields(toTxEffect(tx).hash()); // Update l1 to l2 message tree await updateL1ToL2MessageTree(mockL1ToL2Messages); // Collect all new nullifiers, commitments, and contracts from all txs in this block - const txEffects: TxEffect[] = txs.map( - tx => - new TxEffect( - tx.data.combinedData.newNoteHashes.map((c: SideEffect) => c.value) as Tuple< - Fr, - typeof MAX_NEW_NOTE_HASHES_PER_TX - >, - tx.data.combinedData.newNullifiers.map((n: SideEffectLinkedToNoteHash) => n.value) as Tuple< - Fr, - typeof MAX_NEW_NULLIFIERS_PER_TX - >, - tx.data.combinedData.newL2ToL1Msgs, - tx.data.combinedData.publicDataUpdateRequests.map(t => new PublicDataWrite(t.leafSlot, t.newValue)) as Tuple< - PublicDataWrite, - typeof MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX - >, - tx.data.combinedData.newContracts.map(cd => cd.hash()) as Tuple, - tx.data.combinedData.newContracts.map( - cd => new ContractData(cd.contractAddress, cd.portalContractAddress), - ) as Tuple, - tx.encryptedLogs || new TxL2Logs([]), - tx.unencryptedLogs || new TxL2Logs([]), - ), - ); + const txEffects: TxEffect[] = txs.map(tx => toTxEffect(tx)); const body = new Body(padArrayEnd(mockL1ToL2Messages, Fr.ZERO, NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP), txEffects); // We are constructing the block here just to get body hash/calldata hash so we can pass in an empty archive and header @@ -278,9 +255,21 @@ describe('sequencer/solo_block_builder', () => { }; describe('mock simulator', () => { + beforeAll(() => { + jest.spyOn(TxEffect.prototype, 'hash').mockImplementation(() => { + return Buffer.alloc(32, 0); + }); + }); + + afterAll(() => { + jest.restoreAllMocks(); + }); + beforeEach(() => { // Create instance to test builder = new SoloBlockBuilder(builderDb, vks, simulator, prover); + // since we now assert on the hash of the tx effect while running the base rollup, + // we need to mock the hash function to return a constant value }); it('builds an L2 block using mock simulator', async () => { diff --git a/yarn-project/sequencer-client/src/block_builder/solo_block_builder.ts b/yarn-project/sequencer-client/src/block_builder/solo_block_builder.ts index 760bec97c1b0..42045d6416e5 100644 --- a/yarn-project/sequencer-client/src/block_builder/solo_block_builder.ts +++ b/yarn-project/sequencer-client/src/block_builder/solo_block_builder.ts @@ -1,4 +1,4 @@ -import { Body, ContractData, L2Block, MerkleTreeId, PublicDataWrite, TxEffect, TxL2Logs } from '@aztec/circuit-types'; +import { Body, L2Block, MerkleTreeId, TxEffect } from '@aztec/circuit-types'; import { CircuitSimulationStats } from '@aztec/circuit-types/stats'; import { ARCHIVE_HEIGHT, @@ -7,13 +7,10 @@ import { BaseRollupInputs, CONTRACT_SUBTREE_HEIGHT, CONTRACT_SUBTREE_SIBLING_PATH_LENGTH, - CombinedAccumulatedData, ConstantRollupData, GlobalVariables, L1_TO_L2_MSG_SUBTREE_HEIGHT, L1_TO_L2_MSG_SUBTREE_SIBLING_PATH_LENGTH, - MAX_NEW_CONTRACTS_PER_TX, - MAX_NEW_NOTE_HASHES_PER_TX, MAX_NEW_NULLIFIERS_PER_TX, MAX_PUBLIC_DATA_READS_PER_TX, MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX, @@ -40,8 +37,6 @@ import { RollupTypes, RootRollupInputs, RootRollupPublicInputs, - SideEffect, - SideEffectLinkedToNoteHash, StateDiffHints, StateReference, VK_TREE_HEIGHT, @@ -57,10 +52,11 @@ import { elapsed } from '@aztec/foundation/timer'; import { MerkleTreeOperations } from '@aztec/world-state'; import chunk from 'lodash.chunk'; +import { inspect } from 'util'; import { VerificationKeys } from '../mocks/verification_keys.js'; import { RollupProver } from '../prover/index.js'; -import { ProcessedTx } from '../sequencer/processed_tx.js'; +import { ProcessedTx, toTxEffect } from '../sequencer/processed_tx.js'; import { RollupSimulator } from '../simulator/index.js'; import { BlockBuilder } from './index.js'; import { TreeNames } from './types.js'; @@ -107,30 +103,7 @@ export class SoloBlockBuilder implements BlockBuilder { const [circuitsOutput, proof] = await this.runCircuits(globalVariables, txs, newL1ToL2Messages); // Collect all new nullifiers, commitments, and contracts from all txs in this block - const txEffects: TxEffect[] = txs.map( - tx => - new TxEffect( - tx.data.combinedData.newNoteHashes.map((c: SideEffect) => c.value) as Tuple< - Fr, - typeof MAX_NEW_NOTE_HASHES_PER_TX - >, - tx.data.combinedData.newNullifiers.map((n: SideEffectLinkedToNoteHash) => n.value) as Tuple< - Fr, - typeof MAX_NEW_NULLIFIERS_PER_TX - >, - tx.data.combinedData.newL2ToL1Msgs, - tx.data.combinedData.publicDataUpdateRequests.map(t => new PublicDataWrite(t.leafSlot, t.newValue)) as Tuple< - PublicDataWrite, - typeof MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX - >, - tx.data.combinedData.newContracts.map(cd => cd.hash()) as Tuple, - tx.data.combinedData.newContracts.map( - cd => new ContractData(cd.contractAddress, cd.portalContractAddress), - ) as Tuple, - tx.encryptedLogs || new TxL2Logs([]), - tx.unencryptedLogs || new TxL2Logs([]), - ), - ); + const txEffects: TxEffect[] = txs.map(tx => toTxEffect(tx)); const blockBody = new Body(padArrayEnd(newL1ToL2Messages, Fr.ZERO, NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP), txEffects); @@ -141,6 +114,7 @@ export class SoloBlockBuilder implements BlockBuilder { }); if (!l2Block.body.getTxsEffectsHash().equals(circuitsOutput.header.contentCommitment.txsEffectsHash)) { + this.debug(inspect(blockBody)); throw new Error( `Txs effects hash mismatch, ${l2Block.body .getTxsEffectsHash() @@ -479,7 +453,7 @@ export class SoloBlockBuilder implements BlockBuilder { protected getKernelDataFor(tx: ProcessedTx): RollupKernelData { const inputs = new RollupKernelCircuitPublicInputs( tx.data.aggregationObject, - CombinedAccumulatedData.recombine(tx.data.endNonRevertibleData, tx.data.end), + tx.data.combinedData, tx.data.constants, ); return new RollupKernelData( diff --git a/yarn-project/sequencer-client/src/index.ts b/yarn-project/sequencer-client/src/index.ts index adae5cf85f2f..7d4538bf4765 100644 --- a/yarn-project/sequencer-client/src/index.ts +++ b/yarn-project/sequencer-client/src/index.ts @@ -1,17 +1,17 @@ -export * from './sequencer/index.js'; -export * from './config.js'; -export * from './publisher/index.js'; export * from './client/index.js'; +export * from './config.js'; export * from './mocks/verification_keys.js'; +export * from './publisher/index.js'; +export * from './sequencer/index.js'; // Used by the node to simulate public parts of transactions. Should these be moved to a shared library? -export * from './sequencer/public_processor.js'; export * from './global_variable_builder/index.js'; +export * from './sequencer/public_processor.js'; // Used by publisher test in e2e -export { RealRollupCircuitSimulator } from './simulator/rollup.js'; -export { EmptyRollupProver } from './prover/empty.js'; export { SoloBlockBuilder } from './block_builder/solo_block_builder.js'; +export { EmptyRollupProver } from './prover/empty.js'; +export { makeEmptyProcessedTx, makeProcessedTx, partitionReverts } from './sequencer/processed_tx.js'; export { WASMSimulator } from './simulator/acvm_wasm.js'; +export { RealRollupCircuitSimulator } from './simulator/rollup.js'; export { SimulationProvider } from './simulator/simulation_provider.js'; -export { makeProcessedTx, makeEmptyProcessedTx } from './sequencer/processed_tx.js'; diff --git a/yarn-project/sequencer-client/src/sequencer/abstract_phase_manager.ts b/yarn-project/sequencer-client/src/sequencer/abstract_phase_manager.ts index b8e9960d1a08..071fd5a464f7 100644 --- a/yarn-project/sequencer-client/src/sequencer/abstract_phase_manager.ts +++ b/yarn-project/sequencer-client/src/sequencer/abstract_phase_manager.ts @@ -1,4 +1,4 @@ -import { FunctionL2Logs, MerkleTreeId, Tx } from '@aztec/circuit-types'; +import { FunctionL2Logs, MerkleTreeId, SimulationError, Tx } from '@aztec/circuit-types'; import { AztecAddress, CallRequest, @@ -21,8 +21,6 @@ import { MembershipWitness, PrivateKernelTailCircuitPublicInputs, Proof, - PublicAccumulatedNonRevertibleData, - PublicAccumulatedRevertibleData, PublicCallData, PublicCallRequest, PublicCallStackItem, @@ -102,6 +100,10 @@ export abstract class AbstractPhaseManager { * the proof of the public kernel circuit for this phase */ publicKernelProof: Proof; + /** + * revert reason, if any + */ + revertReason: SimulationError | undefined; }>; abstract rollback(tx: Tx, err: unknown): Promise; @@ -158,55 +160,18 @@ export abstract class AbstractPhaseManager { return calls; } - public static getKernelOutputAndProof( - tx: Tx, - publicKernelPublicInput?: PublicKernelCircuitPublicInputs, - previousPublicKernelProof?: Proof, - ): { - /** - * the output of the public kernel circuit for this phase - */ - publicKernelPublicInput: PublicKernelCircuitPublicInputs; - /** - * the proof of the public kernel circuit for this phase - */ - publicKernelProof: Proof; - } { - if (publicKernelPublicInput && previousPublicKernelProof) { - return { - publicKernelPublicInput: publicKernelPublicInput, - publicKernelProof: previousPublicKernelProof, - }; - } else { - const publicKernelPublicInput = new PublicKernelCircuitPublicInputs( - tx.data.aggregationObject, - PublicAccumulatedNonRevertibleData.fromPrivateAccumulatedNonRevertibleData(tx.data.endNonRevertibleData), - PublicAccumulatedRevertibleData.fromPrivateAccumulatedRevertibleData(tx.data.end), - tx.data.constants, - tx.data.needsSetup, - tx.data.needsAppLogic, - tx.data.needsTeardown, - ); - const publicKernelProof = previousPublicKernelProof || tx.proof; - return { - publicKernelPublicInput, - publicKernelProof, - }; - } - } - protected async processEnqueuedPublicCalls( tx: Tx, previousPublicKernelOutput: PublicKernelCircuitPublicInputs, previousPublicKernelProof: Proof, - ): Promise<[PublicKernelCircuitPublicInputs, Proof, FunctionL2Logs[]]> { + ): Promise<[PublicKernelCircuitPublicInputs, Proof, FunctionL2Logs[], SimulationError | undefined]> { let kernelOutput = previousPublicKernelOutput; let kernelProof = previousPublicKernelProof; const enqueuedCalls = this.extractEnqueuedPublicCalls(tx); if (!enqueuedCalls || !enqueuedCalls.length) { - return [kernelOutput, kernelProof, []]; + return [kernelOutput, kernelProof, [], undefined]; } const newUnencryptedFunctionLogs: FunctionL2Logs[] = []; @@ -243,6 +208,17 @@ export abstract class AbstractPhaseManager { [kernelOutput, kernelProof] = await this.runKernelCircuit(callData, kernelOutput, kernelProof); + if (kernelOutput.reverted && this.phase === PublicKernelPhase.APP_LOGIC) { + this.log.debug( + `Reverting on ${result.execution.contractAddress.toString()}:${functionSelector} with reason: ${ + result.revertReason + }`, + ); + // halt immediately if the public kernel circuit has reverted. + // return no logs, as they should not go on-chain. + return [kernelOutput, kernelProof, [], result.revertReason]; + } + if (!enqueuedExecutionResult) { enqueuedExecutionResult = result; } @@ -255,7 +231,7 @@ export abstract class AbstractPhaseManager { // TODO(#3675): This should be done in a public kernel circuit removeRedundantPublicDataWrites(kernelOutput); - return [kernelOutput, kernelProof, newUnencryptedFunctionLogs]; + return [kernelOutput, kernelProof, newUnencryptedFunctionLogs, undefined]; } protected async runKernelCircuit( @@ -334,6 +310,7 @@ export abstract class AbstractPhaseManager { unencryptedLogsHash, unencryptedLogPreimagesLength, historicalHeader: this.historicalHeader, + reverted: result.reverted, }); } diff --git a/yarn-project/sequencer-client/src/sequencer/app_logic_phase_manager.ts b/yarn-project/sequencer-client/src/sequencer/app_logic_phase_manager.ts index 653aed17ff63..8ddbb329df95 100644 --- a/yarn-project/sequencer-client/src/sequencer/app_logic_phase_manager.ts +++ b/yarn-project/sequencer-client/src/sequencer/app_logic_phase_manager.ts @@ -27,42 +27,32 @@ export class AppLogicPhaseManager extends AbstractPhaseManager { super(db, publicExecutor, publicKernel, publicProver, globalVariables, historicalHeader, phase); } - async handle( + override async handle( tx: Tx, previousPublicKernelOutput: PublicKernelCircuitPublicInputs, previousPublicKernelProof: Proof, - ): Promise<{ - /** - * the output of the public kernel circuit for this phase - */ - publicKernelOutput: PublicKernelCircuitPublicInputs; - /** - * the proof of the public kernel circuit for this phase - */ - publicKernelProof: Proof; - }> { + ) { // add new contracts to the contracts db so that their functions may be found and called // TODO(#4073): This is catching only private deployments, when we add public ones, we'll // have to capture contracts emitted in that phase as well. // TODO(@spalladino): Should we allow emitting contracts in the fee preparation phase? this.log(`Processing tx ${tx.getTxHash()}`); await this.publicContractsDB.addNewContracts(tx); - this.log(`Executing enqueued public calls for tx ${tx.getTxHash()}`); - const [publicKernelOutput, publicKernelProof, newUnencryptedFunctionLogs] = await this.processEnqueuedPublicCalls( - tx, - previousPublicKernelOutput, - previousPublicKernelProof, - ); - tx.unencryptedLogs.addFunctionLogs(newUnencryptedFunctionLogs); + const [publicKernelOutput, publicKernelProof, newUnencryptedFunctionLogs, revertReason] = + await this.processEnqueuedPublicCalls(tx, previousPublicKernelOutput, previousPublicKernelProof); - // commit the state updates from this transaction - await this.publicStateDB.commit(); + if (revertReason) { + await this.rollback(tx, revertReason); + } else { + tx.unencryptedLogs.addFunctionLogs(newUnencryptedFunctionLogs); + await this.publicStateDB.commit(); + } - return { publicKernelOutput, publicKernelProof }; + return { publicKernelOutput, publicKernelProof, revertReason }; } async rollback(tx: Tx, err: unknown): Promise { - this.log.warn(`Error processing tx ${tx.getTxHash()}: ${err}`); + this.log.warn(`Rolling back changes from ${tx.getTxHash()}`); // remove contracts on failure await this.publicContractsDB.removeNewContracts(tx); // rollback any state updates from this failed transaction diff --git a/yarn-project/sequencer-client/src/sequencer/index.ts b/yarn-project/sequencer-client/src/sequencer/index.ts index 4e1a69cbc9b2..459a5cab42ff 100644 --- a/yarn-project/sequencer-client/src/sequencer/index.ts +++ b/yarn-project/sequencer-client/src/sequencer/index.ts @@ -1,2 +1,2 @@ -export * from './sequencer.js'; export * from './config.js'; +export * from './sequencer.js'; diff --git a/yarn-project/sequencer-client/src/sequencer/processed_tx.ts b/yarn-project/sequencer-client/src/sequencer/processed_tx.ts index 206c190b04a6..daa2239cdcbd 100644 --- a/yarn-project/sequencer-client/src/sequencer/processed_tx.ts +++ b/yarn-project/sequencer-client/src/sequencer/processed_tx.ts @@ -1,13 +1,29 @@ -import { ExtendedContractData, Tx, TxHash, TxL2Logs } from '@aztec/circuit-types'; +import { + ContractData, + ExtendedContractData, + PublicDataWrite, + SimulationError, + Tx, + TxEffect, + TxHash, + TxL2Logs, +} from '@aztec/circuit-types'; import { Fr, Header, + MAX_NEW_CONTRACTS_PER_TX, + MAX_NEW_NOTE_HASHES_PER_TX, + MAX_NEW_NULLIFIERS_PER_TX, + MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX, Proof, PublicAccumulatedNonRevertibleData, PublicAccumulatedRevertibleData, PublicKernelCircuitPublicInputs, + SideEffect, + SideEffectLinkedToNoteHash, makeEmptyProof, } from '@aztec/circuits.js'; +import { Tuple, fromFieldsTuple } from '@aztec/foundation/serialize'; /** * Represents a tx that has been processed by the sequencer public processor, @@ -26,8 +42,39 @@ export type ProcessedTx = Pick { + if (isRevertedTx(tx)) { + reverted.push(tx); + } else { + nonReverted.push(tx); + } + return { reverted, nonReverted }; + }, + { reverted: [], nonReverted: [] } as ReturnType, + ); +} + /** * Represents a tx that failed to be processed by the sequencer public processor. */ @@ -42,31 +89,73 @@ export type FailedTx = { error: Error; }; +/** + * + * @param tx - the TX being procesed + * @param publicKernelPublicInput - the output of the public kernel circuit, unless we just came from private + * @param publicKernelProof - the proof of the public kernel circuit, unless we just came from private + * @returns PublicKernelCircuitPublicInputs, either passed through from the input or converted from the output of the TX, + * and Proof, either passed through from the input or the proof of the TX + */ +export function getPreviousOutputAndProof( + tx: Tx, + publicKernelPublicInput?: PublicKernelCircuitPublicInputs, + publicKernelProof?: Proof, +): { + /** + * the output of the public kernel circuit for this phase + */ + publicKernelPublicInput: PublicKernelCircuitPublicInputs; + /** + * the proof of the public kernel circuit for this phase + */ + previousProof: Proof; +} { + if (publicKernelPublicInput && publicKernelProof) { + return { + publicKernelPublicInput, + previousProof: publicKernelProof, + }; + } else { + const publicKernelPublicInput = new PublicKernelCircuitPublicInputs( + tx.data.aggregationObject, + PublicAccumulatedNonRevertibleData.fromPrivateAccumulatedNonRevertibleData(tx.data.endNonRevertibleData), + PublicAccumulatedRevertibleData.fromPrivateAccumulatedRevertibleData(tx.data.end), + tx.data.constants, + tx.data.needsSetup, + tx.data.needsAppLogic, + tx.data.needsTeardown, + false, // reverted + ); + return { + publicKernelPublicInput, + previousProof: publicKernelProof || tx.proof, + }; + } +} + /** * Makes a processed tx out of source tx. * @param tx - Source tx. * @param kernelOutput - Output of the kernel circuit simulation for this tx. * @param proof - Proof of the kernel circuit for this tx. */ -export function makeProcessedTx(tx: Tx, kernelOutput?: PublicKernelCircuitPublicInputs, proof?: Proof): ProcessedTx { +export function makeProcessedTx( + tx: Tx, + kernelOutput?: PublicKernelCircuitPublicInputs, + proof?: Proof, + revertReason?: SimulationError, +): ProcessedTx { + const { publicKernelPublicInput, previousProof } = getPreviousOutputAndProof(tx, kernelOutput, proof); return { hash: tx.getTxHash(), - data: - kernelOutput ?? - new PublicKernelCircuitPublicInputs( - tx.data.aggregationObject, - PublicAccumulatedNonRevertibleData.fromPrivateAccumulatedNonRevertibleData(tx.data.endNonRevertibleData), - PublicAccumulatedRevertibleData.fromPrivateAccumulatedRevertibleData(tx.data.end), - tx.data.constants, - tx.data.needsSetup, - tx.data.needsAppLogic, - tx.data.needsTeardown, - ), - proof: proof ?? tx.proof, - encryptedLogs: tx.encryptedLogs, - unencryptedLogs: tx.unencryptedLogs, - newContracts: tx.newContracts, + data: publicKernelPublicInput, + proof: previousProof, + encryptedLogs: revertReason ? new TxL2Logs([]) : tx.encryptedLogs, + unencryptedLogs: revertReason ? new TxL2Logs([]) : tx.unencryptedLogs, + newContracts: revertReason ? [ExtendedContractData.empty()] : tx.newContracts, isEmpty: false, + revertReason, }; } @@ -90,5 +179,46 @@ export function makeEmptyProcessedTx(header: Header, chainId: Fr, version: Fr): proof: emptyProof, newContracts: [ExtendedContractData.empty()], isEmpty: true, + revertReason: undefined, }; } + +export function toTxEffect(tx: ProcessedTx): TxEffect { + return new TxEffect( + tx.data.combinedData.newNoteHashes.map((c: SideEffect) => c.value) as Tuple, + tx.data.combinedData.newNullifiers.map((n: SideEffectLinkedToNoteHash) => n.value) as Tuple< + Fr, + typeof MAX_NEW_NULLIFIERS_PER_TX + >, + tx.data.combinedData.newL2ToL1Msgs, + tx.data.combinedData.publicDataUpdateRequests.map(t => new PublicDataWrite(t.leafSlot, t.newValue)) as Tuple< + PublicDataWrite, + typeof MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX + >, + tx.data.combinedData.newContracts.map(cd => cd.hash()) as Tuple, + tx.data.combinedData.newContracts.map( + cd => new ContractData(cd.contractAddress, cd.portalContractAddress), + ) as Tuple, + tx.encryptedLogs || new TxL2Logs([]), + tx.unencryptedLogs || new TxL2Logs([]), + ); +} + +function validateProcessedTxLogs(tx: ProcessedTx): void { + const unencryptedLogs = tx.unencryptedLogs || new TxL2Logs([]); + const kernelUnencryptedLogsHash = fromFieldsTuple(tx.data.combinedData.unencryptedLogsHash); + if (!unencryptedLogs.hash().equals(kernelUnencryptedLogsHash)) { + throw new Error( + `Unencrypted logs hash mismatch. Expected ${unencryptedLogs + .hash() + .toString('hex')}, got ${kernelUnencryptedLogsHash.toString('hex')}. + Processed: ${JSON.stringify(unencryptedLogs.toJSON())} + Kernel Length: ${tx.data.combinedData.unencryptedLogPreimagesLength}`, + ); + } +} + +export function validateProcessedTx(tx: ProcessedTx): void { + validateProcessedTxLogs(tx); + // TODO: validate other fields +} diff --git a/yarn-project/sequencer-client/src/sequencer/public_processor.test.ts b/yarn-project/sequencer-client/src/sequencer/public_processor.test.ts index 8affece38133..9e6bc64a850c 100644 --- a/yarn-project/sequencer-client/src/sequencer/public_processor.test.ts +++ b/yarn-project/sequencer-client/src/sequencer/public_processor.test.ts @@ -2,6 +2,7 @@ import { ExtendedContractData, FunctionCall, FunctionL2Logs, + PublicDataWrite, SiblingPath, SimulationError, Tx, @@ -23,11 +24,11 @@ import { MAX_PRIVATE_CALL_STACK_LENGTH_PER_TX, MAX_REVERTIBLE_PUBLIC_CALL_STACK_LENGTH_PER_TX, PUBLIC_DATA_TREE_HEIGHT, + PrivateKernelTailCircuitPublicInputs, Proof, PublicAccumulatedNonRevertibleData, PublicAccumulatedRevertibleData, PublicCallRequest, - PublicDataUpdateRequest, PublicKernelCircuitPublicInputs, makeEmptyProof, } from '@aztec/circuits.js'; @@ -52,7 +53,7 @@ import { WASMSimulator } from '../simulator/acvm_wasm.js'; import { PublicKernelCircuitSimulator } from '../simulator/index.js'; import { ContractsDataSourcePublicDB, WorldStatePublicDB } from '../simulator/public_executor.js'; import { RealPublicKernelCircuitSimulator } from '../simulator/public_kernel.js'; -import { ProcessedTx } from './processed_tx.js'; +import { ProcessedTx, toTxEffect } from './processed_tx.js'; import { PublicProcessor } from './public_processor.js'; describe('public_processor', () => { @@ -100,8 +101,11 @@ describe('public_processor', () => { }); it('skips txs without public execution requests', async function () { - const tx = mockTx(); + const seed = 1; + const includeLogs = false; + const tx = mockTx(seed, includeLogs); tx.data.end.publicCallStack = makeTuple(MAX_REVERTIBLE_PUBLIC_CALL_STACK_LENGTH_PER_TX, CallRequest.empty); + tx.data.end.unencryptedLogsHash = [Fr.ZERO, Fr.ZERO]; tx.data.endNonRevertibleData.publicCallStack = makeTuple( MAX_NON_REVERTIBLE_PUBLIC_CALL_STACK_LENGTH_PER_TX, CallRequest.empty, @@ -116,7 +120,7 @@ describe('public_processor', () => { expect(processed.length).toBe(1); const p = processed[0]; - const e = { + const e: ProcessedTx = { hash, data: new PublicKernelCircuitPublicInputs( tx.data.aggregationObject, @@ -126,12 +130,14 @@ describe('public_processor', () => { tx.data.needsSetup, tx.data.needsAppLogic, tx.data.needsTeardown, + false, // reverted ), proof: tx.proof, encryptedLogs: tx.encryptedLogs, unencryptedLogs: tx.unencryptedLogs, newContracts: tx.newContracts, isEmpty: false, + revertReason: undefined, }; // Jest is complaining that the two objects are not equal, but they are. @@ -150,15 +156,17 @@ describe('public_processor', () => { }); it('returns failed txs without aborting entire operation', async function () { - publicExecutor.simulate.mockRejectedValue(new Error(`Failed`)); + publicExecutor.simulate.mockRejectedValue(new SimulationError(`Failed`, [])); - const tx = mockTx(); + const tx = mockTx(1, false); + tx.data.needsSetup = false; const [processed, failed] = await processor.process([tx]); expect(processed).toEqual([]); expect(failed[0].tx).toEqual(tx); - expect(publicWorldStateDB.commit).toHaveBeenCalledTimes(1); - expect(publicWorldStateDB.rollback).toHaveBeenCalledTimes(1); + expect(failed[0].error).toEqual(new SimulationError(`Failed`, [])); + expect(publicWorldStateDB.commit).toHaveBeenCalledTimes(0); + expect(publicWorldStateDB.rollback).toHaveBeenCalledTimes(0); }); }); @@ -203,8 +211,9 @@ describe('public_processor', () => { MAX_NON_REVERTIBLE_PUBLIC_CALL_STACK_LENGTH_PER_TX, CallRequest.empty, ); + kernelOutput.end.unencryptedLogsHash = [Fr.ZERO, Fr.ZERO]; - const tx = new Tx(kernelOutput, proof, TxL2Logs.random(2, 3), TxL2Logs.random(3, 2), publicCallRequests, [ + const tx = new Tx(kernelOutput, proof, TxL2Logs.empty(), TxL2Logs.empty(), publicCallRequests, [ ExtendedContractData.random(), ]); @@ -214,7 +223,9 @@ describe('public_processor', () => { publicExecutor.simulate.mockImplementation(execution => { for (const request of publicCallRequests) { if (execution.contractAddress.equals(request.contractAddress)) { - return Promise.resolve(makePublicExecutionResultFromRequest(request)); + const result = PublicExecutionResultBuilder.fromPublicCallRequest({ request }).build(); + // result.unencryptedLogs = tx.unencryptedLogs.functionLogs[0]; + return Promise.resolve(result); } } throw new Error(`Unexpected execution request: ${execution}`); @@ -245,6 +256,7 @@ describe('public_processor', () => { MAX_NON_REVERTIBLE_PUBLIC_CALL_STACK_LENGTH_PER_TX, CallRequest.empty, ); + kernelOutput.end.unencryptedLogsHash = [Fr.ZERO, Fr.ZERO]; kernelOutput.needsSetup = false; kernelOutput.needsTeardown = false; @@ -252,20 +264,22 @@ describe('public_processor', () => { const tx = new Tx( kernelOutput, proof, - TxL2Logs.random(2, 3), - TxL2Logs.random(3, 2), + TxL2Logs.empty(), + TxL2Logs.empty(), [callRequest], [ExtendedContractData.random()], ); - const publicExecutionResult = makePublicExecutionResultFromRequest(callRequest); - publicExecutionResult.nestedExecutions = [ - makePublicExecutionResult(publicExecutionResult.execution.contractAddress, { - to: makeAztecAddress(30), - functionData: new FunctionData(makeSelector(5), false, false, false), - args: new Array(ARGS_LENGTH).fill(Fr.ZERO), - }), - ]; + const publicExecutionResult = PublicExecutionResultBuilder.fromPublicCallRequest({ + request: callRequest, + nestedExecutions: [ + PublicExecutionResultBuilder.fromFunctionCall({ + from: callRequest.contractAddress, + tx: makeFunctionCall(), + }).build(), + ], + }).build(); + publicExecutor.simulate.mockResolvedValue(publicExecutionResult); const [processed, failed] = await processor.process([tx]); @@ -278,50 +292,121 @@ describe('public_processor', () => { expect(publicWorldStateDB.rollback).toHaveBeenCalledTimes(0); }); - it('rolls back db updates on failed public execution', async function () { - const callRequest: PublicCallRequest = makePublicCallRequest(0x100); - const callStackItem = callRequest.toCallRequest(); + it('rolls back app logic db updates on failed public execution, but persists setup/teardown', async function () { + const baseContractAddressSeed = 0x200; + const baseContractAddress = makeAztecAddress(baseContractAddressSeed); + const callRequests: PublicCallRequest[] = [ + baseContractAddressSeed, + baseContractAddressSeed, + baseContractAddressSeed, + ].map(makePublicCallRequest); + callRequests[0].callContext.startSideEffectCounter = 2; + callRequests[1].callContext.startSideEffectCounter = 3; + callRequests[2].callContext.startSideEffectCounter = 4; const kernelOutput = makePrivateKernelTailCircuitPublicInputs(0x10); - kernelOutput.end.publicCallStack = padArrayEnd( - [callStackItem], - CallRequest.empty(), - MAX_REVERTIBLE_PUBLIC_CALL_STACK_LENGTH_PER_TX, - ); - kernelOutput.end.privateCallStack = padArrayEnd([], CallRequest.empty(), MAX_PRIVATE_CALL_STACK_LENGTH_PER_TX); - kernelOutput.endNonRevertibleData.publicCallStack = makeTuple( - MAX_NON_REVERTIBLE_PUBLIC_CALL_STACK_LENGTH_PER_TX, - CallRequest.empty, - ); + kernelOutput.end.unencryptedLogsHash = [Fr.ZERO, Fr.ZERO]; + + addKernelPublicCallStack(kernelOutput, { + setupCalls: [callRequests[0]], + appLogicCalls: [callRequests[2]], + teardownCall: callRequests[1], + }); + const tx = new Tx( kernelOutput, proof, - TxL2Logs.random(2, 3), - TxL2Logs.random(3, 2), - [callRequest], + TxL2Logs.empty(), + TxL2Logs.empty(), + // reverse because `enqueuedPublicFunctions` expects the last element to be the front of the queue + callRequests.slice().reverse(), [ExtendedContractData.random()], ); - tx.data.needsSetup = false; - tx.data.needsTeardown = false; + const contractSlotA = fr(0x100); + const contractSlotB = fr(0x150); + const contractSlotC = fr(0x200); - const publicExecutionResult = makePublicExecutionResultFromRequest(callRequest); - publicExecutionResult.nestedExecutions = [ - makePublicExecutionResult(publicExecutionResult.execution.contractAddress, { - to: makeAztecAddress(30), - functionData: new FunctionData(makeSelector(5), false, false, false), - args: new Array(ARGS_LENGTH).fill(Fr.ZERO), - }), + let simulatorCallCount = 0; + const simulatorResults: PublicExecutionResult[] = [ + // Setup + PublicExecutionResultBuilder.fromPublicCallRequest({ + request: callRequests[0], + contractStorageUpdateRequests: [new ContractStorageUpdateRequest(contractSlotA, fr(0x101))], + }).build(), + + // App Logic + PublicExecutionResultBuilder.fromPublicCallRequest({ + request: callRequests[2], + nestedExecutions: [ + PublicExecutionResultBuilder.fromFunctionCall({ + from: callRequests[1].contractAddress, + tx: makeFunctionCall(baseContractAddress, makeSelector(5)), + contractStorageUpdateRequests: [ + new ContractStorageUpdateRequest(contractSlotA, fr(0x102)), + new ContractStorageUpdateRequest(contractSlotB, fr(0x151)), + ], + }).build(), + PublicExecutionResultBuilder.fromFunctionCall({ + from: callRequests[1].contractAddress, + tx: makeFunctionCall(baseContractAddress, makeSelector(5)), + revertReason: new SimulationError('Simulation Failed', []), + }).build(), + ], + }).build(), + + // Teardown + PublicExecutionResultBuilder.fromPublicCallRequest({ + request: callRequests[1], + nestedExecutions: [ + PublicExecutionResultBuilder.fromFunctionCall({ + from: callRequests[1].contractAddress, + tx: makeFunctionCall(baseContractAddress, makeSelector(5)), + contractStorageUpdateRequests: [new ContractStorageUpdateRequest(contractSlotC, fr(0x201))], + }).build(), + ], + }).build(), ]; - publicExecutor.simulate.mockRejectedValueOnce(new SimulationError('Simulation Failed', [])); + + publicExecutor.simulate.mockImplementation(execution => { + if (simulatorCallCount < simulatorResults.length) { + return Promise.resolve(simulatorResults[simulatorCallCount++]); + } else { + throw new Error(`Unexpected execution request: ${execution}, call count: ${simulatorCallCount}`); + } + }); + + const setupSpy = jest.spyOn(publicKernel, 'publicKernelCircuitSetup'); + const appLogicSpy = jest.spyOn(publicKernel, 'publicKernelCircuitAppLogic'); + const teardownSpy = jest.spyOn(publicKernel, 'publicKernelCircuitTeardown'); const [processed, failed] = await processor.process([tx]); - expect(failed).toHaveLength(1); - expect(processed).toHaveLength(0); - expect(publicExecutor.simulate).toHaveBeenCalledTimes(1); + expect(processed).toHaveLength(1); + expect(processed).toEqual([expectedTxByHash(tx)]); + expect(failed).toHaveLength(0); + + expect(setupSpy).toHaveBeenCalledTimes(1); + expect(appLogicSpy).toHaveBeenCalledTimes(2); + expect(teardownSpy).toHaveBeenCalledTimes(2); + expect(publicExecutor.simulate).toHaveBeenCalledTimes(3); + expect(publicWorldStateDB.commit).toHaveBeenCalledTimes(2); expect(publicWorldStateDB.rollback).toHaveBeenCalledTimes(1); - expect(publicWorldStateDB.commit).toHaveBeenCalledTimes(0); + + expect(arrayNonEmptyLength(processed[0].data.combinedData.publicCallStack, i => i.isEmpty())).toEqual(0); + + const txEffect = toTxEffect(processed[0]); + expect(arrayNonEmptyLength(txEffect.publicDataWrites, PublicDataWrite.isEmpty)).toEqual(2); + expect(txEffect.publicDataWrites[0]).toEqual( + new PublicDataWrite(computePublicDataTreeLeafSlot(baseContractAddress, contractSlotA), fr(0x101)), + ); + expect(txEffect.publicDataWrites[1]).toEqual( + new PublicDataWrite(computePublicDataTreeLeafSlot(baseContractAddress, contractSlotC), fr(0x201)), + ); + expect(txEffect.contractData[0].isEmpty()).toBe(true); + expect(txEffect.contractLeaves[0].isZero()).toBe(true); + expect(txEffect.encryptedLogs.getTotalLogCount()).toBe(0); + expect(txEffect.unencryptedLogs.getTotalLogCount()).toBe(0); }); it('runs a tx with setup and teardown phases', async function () { @@ -338,92 +423,69 @@ describe('public_processor', () => { const kernelOutput = makePrivateKernelTailCircuitPublicInputs(0x10); - // the first two calls are non-revertible - // the first is for setup, the second is for teardown - kernelOutput.endNonRevertibleData.publicCallStack = padArrayEnd( - // this is a stack, so the first item is the last call - // and callRequests is in the order of the calls - [callRequests[1].toCallRequest(), callRequests[0].toCallRequest()], - CallRequest.empty(), - MAX_NON_REVERTIBLE_PUBLIC_CALL_STACK_LENGTH_PER_TX, - ); - - kernelOutput.end.publicCallStack = padArrayEnd( - [callRequests[2].toCallRequest()], - CallRequest.empty(), - MAX_REVERTIBLE_PUBLIC_CALL_STACK_LENGTH_PER_TX, - ); - kernelOutput.end.privateCallStack = padArrayEnd([], CallRequest.empty(), MAX_PRIVATE_CALL_STACK_LENGTH_PER_TX); + kernelOutput.end.unencryptedLogsHash = [Fr.ZERO, Fr.ZERO]; + addKernelPublicCallStack(kernelOutput, { + setupCalls: [callRequests[0]], + appLogicCalls: [callRequests[2]], + teardownCall: callRequests[1], + }); const tx = new Tx( kernelOutput, proof, - TxL2Logs.random(2, 3), - TxL2Logs.random(3, 2), + TxL2Logs.empty(), + TxL2Logs.empty(), // reverse because `enqueuedPublicFunctions` expects the last element to be the front of the queue callRequests.slice().reverse(), [ExtendedContractData.random()], ); - // const enqueuedExecutionContractAddress = makeAztecAddress(30); - const enqueuedExecutionContractAddress = baseContractAddress; + // const baseContractAddress = makeAztecAddress(30); const contractSlotA = fr(0x100); const contractSlotB = fr(0x150); const contractSlotC = fr(0x200); let simulatorCallCount = 0; - - publicExecutor.simulate.mockImplementation(execution => { - let executionResult: PublicExecutionResult; - - // first call is for setup - if (simulatorCallCount === 0) { - executionResult = makePublicExecutionResultFromRequest(callRequests[0]); - } - // second call is for app logic - else if (simulatorCallCount === 1) { - // which is the call enqueued last chronologically - executionResult = makePublicExecutionResultFromRequest(callRequests[2]); - executionResult.contractStorageUpdateRequests = [ + const simulatorResults: PublicExecutionResult[] = [ + // Setup + PublicExecutionResultBuilder.fromPublicCallRequest({ request: callRequests[0] }).build(), + + // App Logic + PublicExecutionResultBuilder.fromPublicCallRequest({ + request: callRequests[2], + contractStorageUpdateRequests: [ new ContractStorageUpdateRequest(contractSlotA, fr(0x101)), new ContractStorageUpdateRequest(contractSlotB, fr(0x151)), - ]; - } - // third call is for teardown - else if (simulatorCallCount === 2) { - // which is the call enqueued second chronologically - executionResult = makePublicExecutionResultFromRequest(callRequests[1]); - // if this is the call for teardown, enqueue additional call - executionResult.nestedExecutions = [ - makePublicExecutionResult( - executionResult.execution.contractAddress, - { - to: enqueuedExecutionContractAddress, - functionData: new FunctionData(makeSelector(5), false, false, false), - args: new Array(ARGS_LENGTH).fill(Fr.ZERO), - }, - [], - [ + ], + }).build(), + + // Teardown + PublicExecutionResultBuilder.fromPublicCallRequest({ + request: callRequests[1], + nestedExecutions: [ + PublicExecutionResultBuilder.fromFunctionCall({ + from: callRequests[1].contractAddress, + tx: makeFunctionCall(baseContractAddress, makeSelector(5)), + contractStorageUpdateRequests: [ new ContractStorageUpdateRequest(contractSlotA, fr(0x101)), new ContractStorageUpdateRequest(contractSlotC, fr(0x201)), ], - ), - makePublicExecutionResult( - executionResult.execution.contractAddress, - { - to: enqueuedExecutionContractAddress, - functionData: new FunctionData(makeSelector(6), false, false, false), - args: new Array(ARGS_LENGTH).fill(Fr.ZERO), - }, - [], - [new ContractStorageUpdateRequest(contractSlotA, fr(0x102))], - ), - ]; + }).build(), + PublicExecutionResultBuilder.fromFunctionCall({ + from: callRequests[1].contractAddress, + tx: makeFunctionCall(baseContractAddress, makeSelector(5)), + contractStorageUpdateRequests: [new ContractStorageUpdateRequest(contractSlotA, fr(0x102))], + }).build(), + ], + }).build(), + ]; + + publicExecutor.simulate.mockImplementation(execution => { + if (simulatorCallCount < simulatorResults.length) { + return Promise.resolve(simulatorResults[simulatorCallCount++]); } else { throw new Error(`Unexpected execution request: ${execution}, call count: ${simulatorCallCount}`); } - simulatorCallCount++; - return Promise.resolve(executionResult); }); const setupSpy = jest.spyOn(publicKernel, 'publicKernelCircuitSetup'); @@ -442,64 +504,158 @@ describe('public_processor', () => { expect(publicExecutor.simulate).toHaveBeenCalledTimes(3); expect(publicWorldStateDB.commit).toHaveBeenCalledTimes(3); expect(publicWorldStateDB.rollback).toHaveBeenCalledTimes(0); - expect( - arrayNonEmptyLength(processed[0].data.combinedData.publicDataUpdateRequests, PublicDataUpdateRequest.isEmpty), - ).toEqual(3); - expect(processed[0].data.combinedData.publicDataUpdateRequests[0]).toEqual( - new PublicDataUpdateRequest(computePublicDataTreeLeafSlot(baseContractAddress, contractSlotA), fr(0x102)), + + const txEffect = toTxEffect(processed[0]); + expect(arrayNonEmptyLength(txEffect.publicDataWrites, PublicDataWrite.isEmpty)).toEqual(3); + expect(txEffect.publicDataWrites[0]).toEqual( + new PublicDataWrite(computePublicDataTreeLeafSlot(baseContractAddress, contractSlotA), fr(0x102)), ); - expect(processed[0].data.combinedData.publicDataUpdateRequests[1]).toEqual( - new PublicDataUpdateRequest( - computePublicDataTreeLeafSlot(enqueuedExecutionContractAddress, contractSlotB), - fr(0x151), - ), + expect(txEffect.publicDataWrites[1]).toEqual( + new PublicDataWrite(computePublicDataTreeLeafSlot(baseContractAddress, contractSlotB), fr(0x151)), ); - expect(processed[0].data.combinedData.publicDataUpdateRequests[2]).toEqual( - new PublicDataUpdateRequest( - computePublicDataTreeLeafSlot(enqueuedExecutionContractAddress, contractSlotC), - fr(0x201), - ), + expect(txEffect.publicDataWrites[2]).toEqual( + new PublicDataWrite(computePublicDataTreeLeafSlot(baseContractAddress, contractSlotC), fr(0x201)), ); + expect(txEffect.encryptedLogs.getTotalLogCount()).toBe(0); + expect(txEffect.unencryptedLogs.getTotalLogCount()).toBe(0); }); }); }); -function makePublicExecutionResultFromRequest(item: PublicCallRequest): PublicExecutionResult { - return { - execution: item, - nestedExecutions: [], - returnValues: [new Fr(1n)], - newNoteHashes: [], - newL2ToL1Messages: [], - newNullifiers: [], - contractStorageReads: [], - contractStorageUpdateRequests: [], - unencryptedLogs: new FunctionL2Logs([]), - }; +class PublicExecutionResultBuilder { + private _execution: PublicExecution; + private _nestedExecutions: PublicExecutionResult[] = []; + private _contractStorageUpdateRequests: ContractStorageUpdateRequest[] = []; + private _returnValues: Fr[] = []; + private _reverted = false; + private _revertReason: SimulationError | undefined = undefined; + + constructor(execution: PublicExecution) { + this._execution = execution; + } + + static fromPublicCallRequest({ + request, + returnValues = [new Fr(1n)], + nestedExecutions = [], + contractStorageUpdateRequests = [], + }: { + request: PublicCallRequest; + returnValues?: Fr[]; + nestedExecutions?: PublicExecutionResult[]; + contractStorageUpdateRequests?: ContractStorageUpdateRequest[]; + }): PublicExecutionResultBuilder { + const builder = new PublicExecutionResultBuilder(request); + + builder.withNestedExecutions(...nestedExecutions); + builder.withContractStorageUpdateRequest(...contractStorageUpdateRequests); + builder.withReturnValues(...returnValues); + + return builder; + } + + static fromFunctionCall({ + from, + tx, + returnValues = [new Fr(1n)], + nestedExecutions = [], + contractStorageUpdateRequests = [], + revertReason, + }: { + from: AztecAddress; + tx: FunctionCall; + returnValues?: Fr[]; + nestedExecutions?: PublicExecutionResult[]; + contractStorageUpdateRequests?: ContractStorageUpdateRequest[]; + revertReason?: SimulationError; + }) { + const builder = new PublicExecutionResultBuilder({ + callContext: new CallContext(from, tx.to, EthAddress.ZERO, tx.functionData.selector, false, false, false, 0), + contractAddress: tx.to, + functionData: tx.functionData, + args: tx.args, + }); + + builder.withNestedExecutions(...nestedExecutions); + builder.withContractStorageUpdateRequest(...contractStorageUpdateRequests); + builder.withReturnValues(...returnValues); + if (revertReason) { + builder.withReverted(revertReason); + } + + return builder; + } + + withNestedExecutions(...nested: PublicExecutionResult[]): PublicExecutionResultBuilder { + this._nestedExecutions.push(...nested); + return this; + } + + withContractStorageUpdateRequest(...request: ContractStorageUpdateRequest[]): PublicExecutionResultBuilder { + this._contractStorageUpdateRequests.push(...request); + return this; + } + + withReturnValues(...values: Fr[]): PublicExecutionResultBuilder { + this._returnValues.push(...values); + return this; + } + + withReverted(reason: SimulationError): PublicExecutionResultBuilder { + this._reverted = true; + this._revertReason = reason; + return this; + } + + build(): PublicExecutionResult { + return { + execution: this._execution, + nestedExecutions: this._nestedExecutions, + contractStorageUpdateRequests: this._contractStorageUpdateRequests, + returnValues: this._returnValues, + newNoteHashes: [], + newNullifiers: [], + newL2ToL1Messages: [], + contractStorageReads: [], + unencryptedLogs: new FunctionL2Logs([]), + reverted: this._reverted, + revertReason: this._revertReason, + }; + } } -function makePublicExecutionResult( - from: AztecAddress, - tx: FunctionCall, - nestedExecutions: PublicExecutionResult[] = [], - contractStorageUpdateRequests: ContractStorageUpdateRequest[] = [], -): PublicExecutionResult { - const callContext = new CallContext(from, tx.to, EthAddress.ZERO, tx.functionData.selector, false, false, false, 0); - const execution: PublicExecution = { - callContext, - contractAddress: tx.to, - functionData: tx.functionData, - args: tx.args, - }; - return { - execution, - nestedExecutions, - contractStorageUpdateRequests, - returnValues: [], - newNoteHashes: [], - newNullifiers: [], - newL2ToL1Messages: [], - contractStorageReads: [], - unencryptedLogs: new FunctionL2Logs([]), - }; +const makeFunctionCall = ( + to = makeAztecAddress(30), + selector = makeSelector(5), + args = new Array(ARGS_LENGTH).fill(Fr.ZERO), +) => ({ + to, + functionData: new FunctionData(selector, false, false, false), + args, +}); + +function addKernelPublicCallStack( + kernelOutput: PrivateKernelTailCircuitPublicInputs, + calls: { + setupCalls: PublicCallRequest[]; + appLogicCalls: PublicCallRequest[]; + teardownCall: PublicCallRequest; + }, +) { + // the first two calls are non-revertible + // the first is for setup, the second is for teardown + kernelOutput.endNonRevertibleData.publicCallStack = padArrayEnd( + // this is a stack, so the first item is the last call + // and callRequests is in the order of the calls + [calls.teardownCall.toCallRequest(), ...calls.setupCalls.map(c => c.toCallRequest())], + CallRequest.empty(), + MAX_NON_REVERTIBLE_PUBLIC_CALL_STACK_LENGTH_PER_TX, + ); + + kernelOutput.end.publicCallStack = padArrayEnd( + calls.appLogicCalls.map(c => c.toCallRequest()), + CallRequest.empty(), + MAX_REVERTIBLE_PUBLIC_CALL_STACK_LENGTH_PER_TX, + ); + kernelOutput.end.privateCallStack = padArrayEnd([], CallRequest.empty(), MAX_PRIVATE_CALL_STACK_LENGTH_PER_TX); } diff --git a/yarn-project/sequencer-client/src/sequencer/public_processor.ts b/yarn-project/sequencer-client/src/sequencer/public_processor.ts index c53b9b47ea88..8c479cfac47b 100644 --- a/yarn-project/sequencer-client/src/sequencer/public_processor.ts +++ b/yarn-project/sequencer-client/src/sequencer/public_processor.ts @@ -1,4 +1,4 @@ -import { ContractDataSource, L1ToL2MessageSource, Tx } from '@aztec/circuit-types'; +import { ContractDataSource, L1ToL2MessageSource, SimulationError, Tx } from '@aztec/circuit-types'; import { TxSequencerProcessingStats } from '@aztec/circuit-types/stats'; import { GlobalVariables, Header } from '@aztec/circuits.js'; import { createDebugLogger } from '@aztec/foundation/log'; @@ -14,7 +14,14 @@ import { RealPublicKernelCircuitSimulator } from '../simulator/public_kernel.js' import { SimulationProvider } from '../simulator/simulation_provider.js'; import { AbstractPhaseManager } from './abstract_phase_manager.js'; import { PhaseManagerFactory } from './phase_manager_factory.js'; -import { FailedTx, ProcessedTx, makeEmptyProcessedTx, makeProcessedTx } from './processed_tx.js'; +import { + FailedTx, + ProcessedTx, + getPreviousOutputAndProof, + makeEmptyProcessedTx, + makeProcessedTx, + validateProcessedTx, +} from './processed_tx.js'; /** * Creates new instances of PublicProcessor given the provided merkle tree db and contract data source. @@ -99,17 +106,15 @@ export class PublicProcessor { this.publicStateDB, ); this.log(`Beginning processing in phase ${phase?.phase} for tx ${tx.getTxHash()}`); - let { publicKernelPublicInput, publicKernelProof } = AbstractPhaseManager.getKernelOutputAndProof( - tx, - undefined, - undefined, - ); + let { publicKernelPublicInput, previousProof: proof } = getPreviousOutputAndProof(tx, undefined, undefined); + let revertReason: SimulationError | undefined; const timer = new Timer(); try { while (phase) { - const output = await phase.handle(tx, publicKernelPublicInput, publicKernelProof); + const output = await phase.handle(tx, publicKernelPublicInput, proof); publicKernelPublicInput = output.publicKernelOutput; - publicKernelProof = output.publicKernelProof; + proof = output.publicKernelProof; + revertReason ??= output.revertReason; phase = PhaseManagerFactory.phaseFromOutput( publicKernelPublicInput, phase, @@ -124,7 +129,9 @@ export class PublicProcessor { ); } - const processedTransaction = makeProcessedTx(tx, publicKernelPublicInput, publicKernelProof); + const processedTransaction = makeProcessedTx(tx, publicKernelPublicInput, proof, revertReason); + validateProcessedTx(processedTransaction); + result.push(processedTransaction); this.log(`Processed public part of ${tx.data.endNonRevertibleData.newNullifiers[0].value}`, { @@ -135,9 +142,14 @@ export class PublicProcessor { 0, ...tx.getStats(), } satisfies TxSequencerProcessingStats); - } catch (err) { - const failedTx = await phase!.rollback(tx, err); - failed.push(failedTx); + } catch (err: any) { + const errorMessage = err instanceof Error ? err.message : 'Unknown error'; + this.log.warn(`Failed to process tx ${tx.getTxHash()}: ${errorMessage}`); + + failed.push({ + tx, + error: err instanceof Error ? err : new Error(errorMessage), + }); } } diff --git a/yarn-project/sequencer-client/src/sequencer/sequencer.ts b/yarn-project/sequencer-client/src/sequencer/sequencer.ts index 4a34b685751a..47ce5804eb5b 100644 --- a/yarn-project/sequencer-client/src/sequencer/sequencer.ts +++ b/yarn-project/sequencer-client/src/sequencer/sequencer.ts @@ -180,7 +180,6 @@ export class Sequencer { this.log.info(`Building block ${newBlockNumber} with ${validTxs.length} transactions`); this.state = SequencerState.CREATING_BLOCK; - // Process txs and drop the ones that fail processing // We create a fresh processor each time to reset any cached state (eg storage writes) const processor = await this.publicProcessorFactory.create(historicalHeader, newGlobalVariables); const [publicProcessorDuration, [processedTxs, failedTxs]] = await elapsed(() => processor.process(validTxs)); 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 efac1e362028..070783c1127a 100644 --- a/yarn-project/sequencer-client/src/sequencer/setup_phase_manager.ts +++ b/yarn-project/sequencer-client/src/sequencer/setup_phase_manager.ts @@ -27,34 +27,20 @@ export class SetupPhaseManager extends AbstractPhaseManager { super(db, publicExecutor, publicKernel, publicProver, globalVariables, historicalHeader, phase); } - // this is a no-op for now - async handle( + override async handle( tx: Tx, previousPublicKernelOutput: PublicKernelCircuitPublicInputs, previousPublicKernelProof: Proof, - ): Promise<{ - /** - * the output of the public kernel circuit for this phase - */ - publicKernelOutput: PublicKernelCircuitPublicInputs; - /** - * the proof of the public kernel circuit for this phase - */ - publicKernelProof: Proof; - }> { + ) { this.log(`Processing tx ${tx.getTxHash()}`); - this.log(`Executing enqueued public calls for tx ${tx.getTxHash()}`); - const [publicKernelOutput, publicKernelProof, newUnencryptedFunctionLogs] = await this.processEnqueuedPublicCalls( - tx, - previousPublicKernelOutput, - previousPublicKernelProof, - ); + const [publicKernelOutput, publicKernelProof, newUnencryptedFunctionLogs, revertReason] = + await this.processEnqueuedPublicCalls(tx, previousPublicKernelOutput, previousPublicKernelProof); tx.unencryptedLogs.addFunctionLogs(newUnencryptedFunctionLogs); // commit the state updates from this transaction await this.publicStateDB.commit(); - return { publicKernelOutput, publicKernelProof }; + return { publicKernelOutput, publicKernelProof, revertReason }; } async rollback(tx: Tx, err: unknown): Promise { diff --git a/yarn-project/sequencer-client/src/sequencer/teardown_phase_manager.ts b/yarn-project/sequencer-client/src/sequencer/teardown_phase_manager.ts index 48cc3d8b0b76..f466218fa590 100644 --- a/yarn-project/sequencer-client/src/sequencer/teardown_phase_manager.ts +++ b/yarn-project/sequencer-client/src/sequencer/teardown_phase_manager.ts @@ -27,33 +27,20 @@ export class TeardownPhaseManager extends AbstractPhaseManager { super(db, publicExecutor, publicKernel, publicProver, globalVariables, historicalHeader, phase); } - async handle( + override async handle( tx: Tx, previousPublicKernelOutput: PublicKernelCircuitPublicInputs, previousPublicKernelProof: Proof, - ): Promise<{ - /** - * the output of the public kernel circuit for this phase - */ - publicKernelOutput: PublicKernelCircuitPublicInputs; - /** - * the proof of the public kernel circuit for this phase - */ - publicKernelProof: Proof; - }> { + ) { this.log(`Processing tx ${tx.getTxHash()}`); - this.log(`Executing enqueued public calls for tx ${tx.getTxHash()}`); - const [publicKernelOutput, publicKernelProof, newUnencryptedFunctionLogs] = await this.processEnqueuedPublicCalls( - tx, - previousPublicKernelOutput, - previousPublicKernelProof, - ); + const [publicKernelOutput, publicKernelProof, newUnencryptedFunctionLogs, revertReason] = + await this.processEnqueuedPublicCalls(tx, previousPublicKernelOutput, previousPublicKernelProof); tx.unencryptedLogs.addFunctionLogs(newUnencryptedFunctionLogs); // commit the state updates from this transaction await this.publicStateDB.commit(); - return { publicKernelOutput, publicKernelProof }; + return { publicKernelOutput, publicKernelProof, revertReason }; } async rollback(tx: Tx, err: unknown): Promise { diff --git a/yarn-project/simulator/src/avm/temporary_executor_migration.ts b/yarn-project/simulator/src/avm/temporary_executor_migration.ts index ad66c6b98855..dc6af29bc2bf 100644 --- a/yarn-project/simulator/src/avm/temporary_executor_migration.ts +++ b/yarn-project/simulator/src/avm/temporary_executor_migration.ts @@ -10,6 +10,7 @@ import { } from '@aztec/circuits.js'; import { Fr } from '@aztec/foundation/fields'; +import { createSimulationError } from '../common/errors.js'; import { PublicExecution, PublicExecutionResult } from '../public/execution.js'; import { AvmExecutionEnvironment } from './avm_execution_environment.js'; import { AvmContractCallResults } from './avm_message_call_result.js'; @@ -105,5 +106,7 @@ export function temporaryConvertAvmResults( returnValues, nestedExecutions, unencryptedLogs, + reverted: result.reverted, + revertReason: result.revertReason ? createSimulationError(result.revertReason) : undefined, }; } diff --git a/yarn-project/simulator/src/index.ts b/yarn-project/simulator/src/index.ts index e166c532bdd2..83725c7d2be8 100644 --- a/yarn-project/simulator/src/index.ts +++ b/yarn-project/simulator/src/index.ts @@ -1,3 +1,4 @@ -export * from './client/index.js'; export * from './acvm/index.js'; +export * from './client/index.js'; +export * from './common/index.js'; export * from './public/index.js'; diff --git a/yarn-project/simulator/src/public/execution.ts b/yarn-project/simulator/src/public/execution.ts index 6d6bca3ee1cb..83cf088f8cd0 100644 --- a/yarn-project/simulator/src/public/execution.ts +++ b/yarn-project/simulator/src/public/execution.ts @@ -1,12 +1,11 @@ -import { FunctionL2Logs } from '@aztec/circuit-types'; +import { FunctionL2Logs, SimulationError } from '@aztec/circuit-types'; import { AztecAddress, - CallContext, ContractStorageRead, ContractStorageUpdateRequest, Fr, - FunctionData, L2ToL1Message, + PublicCallRequest, PublicDataRead, PublicDataUpdateRequest, SideEffect, @@ -39,21 +38,20 @@ export interface PublicExecutionResult { * Note: These are preimages to `unencryptedLogsHash`. */ unencryptedLogs: FunctionL2Logs; + /** + * Whether the execution reverted. + */ + reverted: boolean; + /** + * The revert reason if the execution reverted. + */ + revertReason: SimulationError | undefined; } /** * The execution of a public function. */ -export interface PublicExecution { - /** Address of the contract being executed. */ - contractAddress: AztecAddress; - /** Function of the contract being called. */ - functionData: FunctionData; - /** Arguments for the call. */ - args: Fr[]; - /** Context of the call. */ - callContext: CallContext; -} +export type PublicExecution = Pick; /** * Returns if the input is a public execution result and not just a public execution. @@ -63,7 +61,7 @@ export interface PublicExecution { export function isPublicExecutionResult( input: PublicExecution | PublicExecutionResult, ): input is PublicExecutionResult { - return !!(input as PublicExecutionResult).execution; + return 'execution' in input && input.execution !== undefined; } /** diff --git a/yarn-project/simulator/src/public/executor.ts b/yarn-project/simulator/src/public/executor.ts index 814570d016b0..2f8c430db904 100644 --- a/yarn-project/simulator/src/public/executor.ts +++ b/yarn-project/simulator/src/public/executor.ts @@ -1,3 +1,4 @@ +import { FunctionL2Logs } from '@aztec/circuit-types'; import { GlobalVariables, Header, PublicCircuitPublicInputs } from '@aztec/circuits.js'; import { createDebugLogger } from '@aztec/foundation/log'; @@ -11,10 +12,10 @@ import { temporaryConvertAvmResults, temporaryCreateAvmExecutionEnvironment, } from '../avm/temporary_executor_migration.js'; +import { AcirSimulator } from '../client/simulator.js'; import { ExecutionError, createSimulationError } from '../common/errors.js'; import { SideEffectCounter } from '../common/index.js'; import { PackedArgsCache } from '../common/packed_args_cache.js'; -import { AcirSimulator } from '../index.js'; import { CommitmentsDB, PublicContractsDB, PublicStateDB } from './db.js'; import { PublicExecution, PublicExecutionResult, checkValidStaticCall } from './execution.js'; import { PublicExecutionContext } from './public_execution_context.js'; @@ -25,6 +26,7 @@ import { PublicExecutionContext } from './public_execution_context.js'; export async function executePublicFunction( context: PublicExecutionContext, acir: Buffer, + nested: boolean, log = createDebugLogger('aztec:simulator:public_execution'), ): Promise { const execution = context.execution; @@ -34,9 +36,19 @@ export async function executePublicFunction( const initialWitness = context.getInitialWitness(); const acvmCallback = new Oracle(context); - const { partialWitness } = await acvm(await AcirSimulator.getSolver(), acir, initialWitness, acvmCallback).catch( - (err: Error) => { - throw new ExecutionError( + const { partialWitness, reverted, revertReason } = await acvm( + await AcirSimulator.getSolver(), + acir, + initialWitness, + acvmCallback, + ) + .then(result => ({ + partialWitness: result.partialWitness, + reverted: false, + revertReason: undefined, + })) + .catch((err: Error) => { + const ee = new ExecutionError( err.message, { contractAddress, @@ -45,8 +57,41 @@ export async function executePublicFunction( extractCallStack(err), { cause: err }, ); - }, - ); + + if (nested) { + // If we're nested, throw the error so the parent can handle it + throw ee; + } else { + return { + partialWitness: undefined, + reverted: true, + revertReason: createSimulationError(ee), + }; + } + }); + if (reverted) { + if (!revertReason) { + throw new Error('Reverted but no revert reason'); + } + + return { + execution, + returnValues: [], + newNoteHashes: [], + newL2ToL1Messages: [], + newNullifiers: [], + contractStorageReads: [], + contractStorageUpdateRequests: [], + nestedExecutions: [], + unencryptedLogs: FunctionL2Logs.empty(), + reverted, + revertReason, + }; + } + + if (!partialWitness) { + throw new Error('No partial witness returned from ACVM'); + } const returnWitness = extractReturnWitness(acir, partialWitness); const { @@ -86,6 +131,8 @@ export async function executePublicFunction( returnValues, nestedExecutions, unencryptedLogs, + reverted: false, + revertReason: undefined, }; } @@ -130,13 +177,7 @@ export class PublicExecutor { this.commitmentsDb, ); - let executionResult; - - try { - executionResult = await executePublicFunction(context, acir); - } catch (err) { - throw createSimulationError(err instanceof Error ? err : new Error('Unknown error during public execution')); - } + const executionResult = await executePublicFunction(context, acir, false /** nested */); if (executionResult.execution.callContext.isStaticCall) { checkValidStaticCall( diff --git a/yarn-project/simulator/src/public/index.test.ts b/yarn-project/simulator/src/public/index.test.ts index 3c8396547c95..098c72dc096e 100644 --- a/yarn-project/simulator/src/public/index.test.ts +++ b/yarn-project/simulator/src/public/index.test.ts @@ -240,9 +240,9 @@ describe('ACIR public execution simulator', () => { const recipientBalance = new Fr(20n); mockStore(senderBalance, recipientBalance); - await expect(executor.simulate(execution, GlobalVariables.empty())).rejects.toThrowError( - 'Assertion failed: attempt to subtract with underflow', - ); + const { reverted, revertReason } = await executor.simulate(execution, GlobalVariables.empty()); + expect(reverted).toBe(true); + expect(revertReason?.message).toMatch('Assertion failed: attempt to subtract with underflow'); }); }); }); @@ -304,7 +304,10 @@ describe('ACIR public execution simulator', () => { ); if (isInternal === undefined) { - await expect(executor.simulate(execution, globalVariables)).rejects.toThrowError(/Method not found -/); + const { reverted, revertReason } = await executor.simulate(execution, globalVariables); + + expect(reverted).toBe(true); + expect(revertReason?.message).toMatch('Method not found -'); } else { const result = await executor.simulate(execution, globalVariables); @@ -544,7 +547,9 @@ describe('ACIR public execution simulator', () => { const execution: PublicExecution = { contractAddress, functionData, args, callContext }; executor = new PublicExecutor(publicState, publicContracts, commitmentsDb, header); - await expect(executor.simulate(execution, globalVariables)).rejects.toThrowError('Message not in state'); + const { revertReason, reverted } = await executor.simulate(execution, globalVariables); + expect(reverted).toBe(true); + expect(revertReason?.message).toMatch(`Message not in state`); }); it('Invalid recipient', async () => { @@ -559,7 +564,9 @@ describe('ACIR public execution simulator', () => { const execution: PublicExecution = { contractAddress, functionData, args, callContext }; executor = new PublicExecutor(publicState, publicContracts, commitmentsDb, header); - await expect(executor.simulate(execution, globalVariables)).rejects.toThrowError('Message not in state'); + const { revertReason, reverted } = await executor.simulate(execution, globalVariables); + expect(reverted).toBe(true); + expect(revertReason?.message).toMatch(`Message not in state`); }); it('Invalid sender', async () => { @@ -574,7 +581,9 @@ describe('ACIR public execution simulator', () => { const execution: PublicExecution = { contractAddress, functionData, args, callContext }; executor = new PublicExecutor(publicState, publicContracts, commitmentsDb, header); - await expect(executor.simulate(execution, globalVariables)).rejects.toThrowError('Message not in state'); + const { revertReason, reverted } = await executor.simulate(execution, globalVariables); + expect(reverted).toBe(true); + expect(revertReason?.message).toMatch(`Message not in state`); }); it('Invalid chainid', async () => { @@ -589,7 +598,9 @@ describe('ACIR public execution simulator', () => { const execution: PublicExecution = { contractAddress, functionData, args, callContext }; executor = new PublicExecutor(publicState, publicContracts, commitmentsDb, header); - await expect(executor.simulate(execution, globalVariables)).rejects.toThrowError('Message not in state'); + const { revertReason, reverted } = await executor.simulate(execution, globalVariables); + expect(reverted).toBe(true); + expect(revertReason?.message).toMatch(`Message not in state`); }); it('Invalid version', async () => { @@ -604,7 +615,9 @@ describe('ACIR public execution simulator', () => { const execution: PublicExecution = { contractAddress, functionData, args, callContext }; executor = new PublicExecutor(publicState, publicContracts, commitmentsDb, header); - await expect(executor.simulate(execution, globalVariables)).rejects.toThrowError('Message not in state'); + const { revertReason, reverted } = await executor.simulate(execution, globalVariables); + expect(reverted).toBe(true); + expect(revertReason?.message).toMatch(`Message not in state`); }); it('Invalid Content', async () => { @@ -620,7 +633,9 @@ describe('ACIR public execution simulator', () => { const execution: PublicExecution = { contractAddress, functionData, args, callContext }; executor = new PublicExecutor(publicState, publicContracts, commitmentsDb, header); - await expect(executor.simulate(execution, globalVariables)).rejects.toThrowError('Message not in state'); + const { revertReason, reverted } = await executor.simulate(execution, globalVariables); + expect(reverted).toBe(true); + expect(revertReason?.message).toMatch(`Message not in state`); }); it('Invalid secret', async () => { @@ -636,7 +651,9 @@ describe('ACIR public execution simulator', () => { const execution: PublicExecution = { contractAddress, functionData, args, callContext }; executor = new PublicExecutor(publicState, publicContracts, commitmentsDb, header); - await expect(executor.simulate(execution, globalVariables)).rejects.toThrowError('Message not in state'); + const { revertReason, reverted } = await executor.simulate(execution, globalVariables); + expect(reverted).toBe(true); + expect(revertReason?.message).toMatch(`Message not in state`); }); }); }); @@ -723,9 +740,9 @@ describe('ACIR public execution simulator', () => { const execution: PublicExecution = { contractAddress, functionData, args, callContext }; executor = new PublicExecutor(publicState, publicContracts, commitmentsDb, header); - await expect(executor.simulate(execution, globalVariables)).rejects.toThrowError( - `Invalid ${description.toLowerCase()}`, - ); + const { revertReason, reverted } = await executor.simulate(execution, globalVariables); + expect(reverted).toBe(true); + expect(revertReason?.message).toMatch(`Invalid ${description.toLowerCase()}`); }); }); }); @@ -773,7 +790,9 @@ describe('ACIR public execution simulator', () => { const execution: PublicExecution = { contractAddress, functionData, args, callContext }; executor = new PublicExecutor(publicState, publicContracts, commitmentsDb, header); - await expect(executor.simulate(execution, GlobalVariables.empty())).rejects.toThrowError('Invalid header hash'); + const { revertReason, reverted } = await executor.simulate(execution, GlobalVariables.empty()); + expect(reverted).toBe(true); + expect(revertReason?.message).toMatch(`Invalid header hash`); }); }); }); diff --git a/yarn-project/simulator/src/public/public_execution_context.ts b/yarn-project/simulator/src/public/public_execution_context.ts index 3b53fd2490ba..597b04563584 100644 --- a/yarn-project/simulator/src/public/public_execution_context.ts +++ b/yarn-project/simulator/src/public/public_execution_context.ts @@ -211,7 +211,7 @@ export class PublicExecutionContext extends TypedOracle { this.log, ); - const childExecutionResult = await executePublicFunction(context, acir); + const childExecutionResult = await executePublicFunction(context, acir, true /** nested */); if (isStaticCall) { checkValidStaticCall(