adds basic support for reversions of public functions

.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"
-  ] 
+  ]
 }

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

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;

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

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
     }

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);

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()
     }

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);

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();
 

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

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

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);
     }
 }

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<PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH> 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<PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH> 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);
 }

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;

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 }

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

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`);
   }