feat: merge-train/avm

barretenberg/cpp/pil/vm2/bitwise.pil

@@ -142,17 +142,16 @@ start_sha256 * (1 - start_sha256) = 0;
 // Crucial constraint to prevent the vulnerability described in PR #19875 consisting
 // of maliciously setting start_keccak/start_sha256=1 on error rows (sel=1, err=1)
 // and forging arbitrary bitwise operation results.
-// keccak and sh256 only enforce that tag_a = U64/U32 respectively. An attacker is free to
+// keccak and sha256 only enforce that tag_a = U64/U32 respectively. An attacker is free to
 // set tag_b to any tag value and use the resulting tag mismatch error to return an invalid result
-// Since error rows have sel=1, we gate by err instead of (1 - sel).
 #[BITW_NO_EXTERNAL_START_ON_ERROR]
 (start_keccak + start_sha256) * err = 0;
 
 // To support dynamically sized memory operands we use a counter against a lookup
 // This decrementing counter goes from [TAG_LEN, 0] where TAG_LEN is the number of bytes in the
 // corresponding integer. i.e. TAG_LEN is between 1 (U1/U8) and 16 (U128).
 // Consistency can be achieved with a lookup table between the tag and precomputed.tag_byte_length
-pol commit ctr; // when err=1, last=1 forces ctr in {0,1} via BITW_LAST_FOR_CTR_ONE; 0 is optimal
+pol commit ctr;
 
 // The error flags for this subtrace
 pol commit sel_tag_ff_err; // @boolean
@@ -163,7 +162,7 @@ sel_tag_mismatch_err * (1 - sel_tag_mismatch_err) = 0;
 // Consolidated error flag
 pol commit err; // @boolean (by definition)
 err = 1 - (1 - sel_tag_mismatch_err) * (1 - sel_tag_ff_err);
-#[LAST_ON_ERROR] // end = 1 if err = 1
+#[END_ON_ERROR] // end = 1 if err = 1
 err * (end - 1) = 0;
 
 // Error can only manifest on start rows (where tag checks are evaluated).

barretenberg/cpp/pil/vm2/bytecode/contract_instance_retrieval.pil

@@ -1,79 +1,121 @@
+include "../constants_gen.pil";
+include "../ff_gt.pil";
+include "../precomputed.pil";
+include "../public_inputs.pil";
+include "../trees/indexed_tree_check.pil";
 include "address_derivation.pil";
 include "update_check.pil";
-include "../ff_gt.pil";
 
 /**
  * Contract Instance Retrieval gadget.
  * For use by execution (GetContractInstance opcode) and by bytecode retrieval.
- *
  * Proves the existence of a deployed contract instance for the provided address,
  * along with its derivation from its parts (deployer, class id, init hash).
- * Does so via interactions with the following auxiliary gadgets:
- *     - Nullifier check: check (non)existence of the contract address nullifier.
- *     - Address derivation: constrain the derivation of the address from its parts (deployer, class id, init hash).
- *     - Update checking: enforce that the class id provided is the _current_ one.
  *
- * Usage (as from opcode or bytecode retrieval):
- * sel {
- *     // inputs
- *     execution_or_bc_retrieval.address,
- *     execution_or_bc_retrieval.nullifier_tree_root,
- *     execution_or_bc_retrieval.public_data_tree_root
- *     // outputs
- *     execution_or_bc_retrieval.exists
- *     execution_or_bc_retrieval.deployer_addr, // situational
- *     execution_or_bc_retrieval.current_class_id,
- *     execution_or_bc_retrieval.init_hash // situational
- * } in contract_instance_retrieval.sel {
- *     // inputs
- *     contract_instance_retrieval.address,
- *     contract_instance_retrieval.nullifier_tree_root,
- *     contract_instance_retrieval.public_data_tree_root,
- *     // outputs
- *     contract_instance_retrieval.exists,
- *     contract_instance_retrieval.deployer_addr, // situational
- *     contract_instance_retrieval.current_class_id,
- *     contract_instance_retrieval.init_hash // situational
- * };
+ * GADGET LOGIC:
+ *     The gadget first classifies the address as protocol or non-protocol, then follows
+ *     one of two paths to determine existence and validate the instance.
+ *
+ *     For all addresses:
+ *     - Classify via range check: address in [1, MAX_PROTOCOL_CONTRACTS] => protocol contract.
+ *     - Force all instance members to 0 if the contract does not exist.
+ *     - If the contract exists, constrain address derivation from instance parts (salt, deployer,
+ *       original class id, init hash, public keys). Salt and public keys are hinted.
+ *
+ *     For protocol contracts:
+ *     - Read derived address from the public inputs column at a computed index.
+ *     - Existence is determined by whether the derived address is non-zero.
+ *     - No nullifier check or update check is performed.
+ *
+ *     For non-protocol contracts:
+ *     - Check (non)existence of the deployment nullifier (the contract address itself) in the
+ *       nullifier tree, siloed under the deployer protocol contract address.
+ *     - If the contract exists, enforce that the current class id is up-to-date via update check.
  *
- * Note: the missing instance member "salt" is not needed in the lookup and can instead just be hinted to the instance retrieval gadget.
+ * PRECONDITIONS:
+ *     - The caller must constrain nullifier_tree_root and public_data_tree_root to the correct
+ *       tree roots for the current context.
+ *     - The contract database must be consistent with the nullifier tree: if a deployment nullifier
+ *       exists for a non-protocol address, the contract instance must be present in the DB.
+ *     - The contract database must be consistent with the protocol contracts table: a protocol
+ *       contract slot has a derived address if and only if the instance is present in the DB.
  *
- * Note: Forces instance members to 0 if the instance doesn't exist.
+ * USAGE:
+ *     This gadget is a lookup destination for get_contract_instance.pil and bc_retrieval.pil.
+ *     The "salt" member is not needed in the lookup — it is hinted to the retrieval gadget.
+ *
+ *     sel {
+ *         // inputs
+ *         caller.address,
+ *         caller.nullifier_tree_root,
+ *         caller.public_data_tree_root
+ *         // outputs
+ *         caller.exists,
+ *         caller.deployer_addr,   // situational - only if caller needs it
+ *         caller.current_class_id,
+ *         caller.init_hash        // situational - only if caller needs it
+ *     } in contract_instance_retrieval.sel {
+ *         contract_instance_retrieval.address,
+ *         contract_instance_retrieval.nullifier_tree_root,
+ *         contract_instance_retrieval.public_data_tree_root,
+ *         contract_instance_retrieval.exists,
+ *         contract_instance_retrieval.deployer_addr,
+ *         contract_instance_retrieval.current_class_id,
+ *         contract_instance_retrieval.init_hash
+ *     };
+ *
+ *     Situational columns (deployer_addr, init_hash) can be omitted if the caller doesn't need
+ *     them. When omitted, they are only hinted for address derivation. This is secure because
+ *     incorrect values would break derivation of the given address.
+ *
+ * TRACE SHAPE:
+ *     1 row per retrieval.
+ *
+ * ERROR HANDLING:
+ *     This gadget does not have error conditions. Errors (invalid enum, out-of-bounds writes)
+ *     are handled by the calling opcode gadget (get_contract_instance.pil).
+ *
+ * INTERACTIONS:
+ *     - ff_gt.pil: Determine if address is in protocol contract range. (#[CHECK_PROTOCOL_ADDRESS_RANGE])
+ *     - public_inputs.pil: Read derived address for protocol contracts. (#[READ_DERIVED_ADDRESS_FROM_PUBLIC_INPUTS])
+ *     - indexed_tree_check.pil: Check (non)existence of deployment nullifier. (#[DEPLOYMENT_NULLIFIER_READ])
+ *     - address_derivation.pil: Constrain address derivation from instance parts. (#[ADDRESS_DERIVATION])
+ *     - update_check.pil: Enforce current class id is up-to-date. (#[UPDATE_CHECK])
  */
 namespace contract_instance_retrieval;
 
+    #[skippable_if]
+    sel = 0;
+
     pol commit sel; // @boolean
     sel * (1 - sel) = 0;
 
-    // No relations will be checked if this identity is satisfied.
-    #[skippable_if]
-    sel = 0;
+    // ==== Inputs and Outputs ====
 
-    ////////////////////////////////////////////////////////////////////////////
-    // I/O
-    pol commit address; // contract address.
-    pol commit exists; // @boolean the contract instance exists (its address nullifier exists)
+    pol commit address; // Contract address.
+    pol commit exists; // @boolean — the contract instance exists (its address nullifier exists, or for protocol contracts, its derived address in PI is nonzero).
     exists * (1 - exists) = 0;
 
-    // address instance members.
+    // Address instance members.
     // See barretenberg/cpp/src/barretenberg/vm2/common/aztec_types.hpp
     pol commit salt; // HINTED!
     pol commit deployer_addr;
     pol commit current_class_id;
     pol commit original_class_id; // HINTED!
     pol commit init_hash;
 
-    // current state
-    // These should be looked up and constrained by the caller.
+    // Current state — these should be looked up and constrained by the caller.
     pol commit nullifier_tree_root;
     pol commit public_data_tree_root;
-    // end I/O
-    ////////////////////////////////////////////////////////////////////////////
+
+    // ==== Hinted and Internal Columns ====
 
     // The address that is checked in address derivation.
+    // This comes from public inputs for protocol contracts,
+    // or is just assigned "address" (from input) for non-protocol contracts.
     pol commit derived_address;
 
-    // public keys (all hinted)
+    // Public keys (all hinted).
     pol commit nullifier_key_x;
     pol commit nullifier_key_y;
     pol commit incoming_viewing_key_x;
@@ -83,29 +125,43 @@ namespace contract_instance_retrieval;
     pol commit tagging_key_x;
     pol commit tagging_key_y;
 
-    // TODO: Remove this as a column when we can lookup with constants
+    // ==== Determine if It Is a Protocol Contract (<= max) ====
+
+    // Lookup constant support: Can be removed when we support constants in lookups.
     pol commit deployer_protocol_contract_address;
+    #[DEPLOYER_PROTOCOL_CONTRACT_ADDRESS_CONSTANT]
     sel * (constants.CONTRACT_INSTANCE_REGISTRY_CONTRACT_ADDRESS - deployer_protocol_contract_address) = 0;
 
-    // Indicates if the instance belongs to a protocol contract
+    // Indicates if the instance belongs to a protocol contract.
     pol commit is_protocol_contract; // @boolean (by lookup on active rows)
-    // TODO: review safety of is_protocol_contract being underconstrained on inactive rows
+    // Note that is_protocol_contract is not constrained to be boolean or nonzero on inactive rows.
+    // This should be fine as it is not used as the target selector for lookups into this trace
+    // and is not used to activate any interactions that create side effects (memory writes, public inputs writes).
+    // Worst-case, a malicious actor could prove an extra properly constrained address-derivation which should be harmless.
 
-    // Canonical Addresses can be in the range of 1 <= address <= MAX_PROTOCOL_CONTRACTS
+    // Canonical addresses can be in the range of 1 <= address <= MAX_PROTOCOL_CONTRACTS.
     pol commit max_protocol_contracts;
     max_protocol_contracts = sel * constants.MAX_PROTOCOL_CONTRACTS;
 
     pol commit address_sub_one;
     address_sub_one = sel * (address - 1);
+
+    // Checks whether address is in the protocol contract range by looking up a > b comparison.
+    // When sel = 1: is_protocol_contract = 1 iff MAX_PROTOCOL_CONTRACTS > (address - 1),
+    // i.e., address <= MAX_PROTOCOL_CONTRACTS.
     #[CHECK_PROTOCOL_ADDRESS_RANGE]
     sel { max_protocol_contracts, address_sub_one, is_protocol_contract }
     in
-    ff_gt.sel_gt { ff_gt.a, ff_gt.b, ff_gt.result};
+    ff_gt.sel_gt { ff_gt.a, ff_gt.b, ff_gt.result };
 
-    // Constrain exists and derived address for protocol contracts
+    // ==== Constrain "exists" and "derived_address" via Public Inputs (Protocol Contracts) ====
 
-    // Compute the protocol contract derived address index in the PI column as `base_offset + contract_index = base_offset + protocol_address - 1`
+    // Compute the protocol contract derived address index in the PI column as:
+    // base_offset + contract_index = base_offset + protocol_address - 1.
+    // Only constrained when is_protocol_contract=1; free otherwise — should be secure because
+    // it is only used in the PI lookup which is also gated by is_protocol_contract.
     pol commit derived_address_pi_index;
+    #[DERIVED_ADDRESS_PI_INDEX]
     is_protocol_contract * (constants.AVM_PUBLIC_INPUTS_PROTOCOL_CONTRACTS_ROW_IDX + address_sub_one - derived_address_pi_index) = 0;
 
     #[READ_DERIVED_ADDRESS_FROM_PUBLIC_INPUTS]
@@ -117,30 +173,36 @@ namespace contract_instance_retrieval;
         public_inputs.cols[0]
     };
 
-    pol commit protocol_contract_derived_address_inv;
+    // Only constrained when is_protocol_contract=1; free otherwise — should be secure because
+    // it is only used in the zero-check which is also gated by is_protocol_contract.
+    pol commit protocol_contract_derived_address_inv; // @zero-check
     pol NOT_EXISTS = 1 - exists;
 
-    // If the protocol contract derived address is zero, the protocol contract does not exist. Otherwise, it exists.
+    // Zero-check with error support: exists iff derived_address != 0.
+    // See https://github.com/AztecProtocol/aztec-packages/blob/next/barretenberg/cpp/pil/vm2/docs/recipes.md#with-error-support.
+    #[PROTOCOL_CONTRACT_EXISTS_CHECK]
     is_protocol_contract * (derived_address * (NOT_EXISTS * (1 - protocol_contract_derived_address_inv) + protocol_contract_derived_address_inv) - 1 + NOT_EXISTS) = 0;
 
-    // Constrain exists and derived address for non-protocol contracts via nullifier check
+    // ==== Constrain "exists" and "derived_address" via Nullifier Check (Non-protocol Contracts) ====
 
     pol commit should_check_nullifier; // @boolean (by definition)
     // Protocol contracts do not have an address nullifier in the nullifier tree.
     should_check_nullifier = sel * (1 - is_protocol_contract);
 
     // Lookup constant support: Can be removed when we support constants in lookups.
     pol commit nullifier_tree_height;
+    #[NULLIFIER_TREE_HEIGHT_CONSTANT]
     should_check_nullifier * (nullifier_tree_height - constants.NULLIFIER_TREE_HEIGHT) = 0;
 
     // Lookup constant support: Can be removed when we support constants in lookups.
     pol commit siloing_separator;
+    #[SILOING_SEPARATOR_CONSTANT]
     should_check_nullifier * (siloing_separator - constants.DOM_SEP__SILOED_NULLIFIER) = 0;
 
-    // Nullifier existence check (deployment nullifier read)
+    // Nullifier existence check (deployment nullifier read).
     #[DEPLOYMENT_NULLIFIER_READ]
     should_check_nullifier {
-        exists, // does the contract address nullifier exist? gates later lookups....
+        exists, // does the contract address nullifier exist?
         address, // the deployment nullifier
         nullifier_tree_root,
         nullifier_tree_height,
@@ -157,11 +219,14 @@ namespace contract_instance_retrieval;
         indexed_tree_check.address
     };
 
-    // For protocol contracts we retrieve the derived address from the protocol contract trace, else use the input address
+    // For non-protocol contracts, derived address just comes from this gadget's `address` input,
+    // unlike protocol contracts where it is looked up from public inputs above.
     #[UNCHANGED_ADDRESS_NON_PROTOCOL]
     sel * (1 - is_protocol_contract) * (derived_address - address) = 0;
 
-    // Force members to 0 if the instance doesn't exist
+    // ==== If Exists, Constrain Address Derivation ====
+
+    // Force members to 0 if the instance doesn't exist.
     #[INSTANCE_MEMBER_SALT_IS_ZERO_IF_DNE]
     sel * (1 - exists) * salt = 0; // technically not needed since salt is hinted, but good for consistency
     #[INSTANCE_MEMBER_DEPLOYER_IS_ZERO_IF_DNE]
@@ -173,7 +238,7 @@ namespace contract_instance_retrieval;
     #[INSTANCE_MEMBER_INIT_HASH_IS_ZERO_IF_DNE]
     sel * (1 - exists) * init_hash = 0;
 
-    // Address derivation lookup (only if the nullifier exists or for protocol contract instances)
+    // Address derivation lookup (only if the nullifier exists or for protocol contract instances).
     #[ADDRESS_DERIVATION]
     exists {
         derived_address,
@@ -205,10 +270,13 @@ namespace contract_instance_retrieval;
         address_derivation.tagging_key_y
     };
 
-    // Enforce that the class id provided is the _current_ one (only when nullifier exists)
-    // If the address nullifier doesn't exist (which excludes protocol contract instances), there is no need to check!
+    // ==== Constrain That "current" Class ID Truly Is Current (Non-protocol Contracts That Exist) ====
+
+    // Enforce that the class id provided is the _current_/active one. Only check when nullifier exists.
+    // If the address nullifier doesn't exist (which excludes protocol contract instances), there is no need to check.
     pol commit should_check_for_update; // @boolean (by definition)
     should_check_for_update = should_check_nullifier * exists;
+
     #[UPDATE_CHECK]
     should_check_for_update {
         address,

barretenberg/cpp/pil/vm2/execution.pil

@@ -537,7 +537,7 @@ sel_gas_sstore * (written_slots_tree_siloing_separator - constants.DOM_SEP__PUBL
 #[CHECK_WRITTEN_STORAGE_SLOT]
 sel_gas_sstore {
     dynamic_da_gas_factor,
-    register[1], // value
+    register[1], // slot
     prev_written_public_data_slots_tree_root,
     written_slots_tree_height,
     sel_gas_sstore, // sel_silo = 1