final cleanup

Author: benesjan

noir-projects/noir-contracts/contracts/protocol/auth_registry_contract/src/main.nr

@@ -96,18 +96,18 @@ pub contract AuthRegistry {
         let storage: Storage<PublicContext> = Storage::init(context);
         // MACRO CODE END
 
-        assert(false == storage.reject_all.at(on_behalf_of).read(), "rejecting all");
+        assert_eq(false, storage.reject_all.at(on_behalf_of).read(), "rejecting all");
 
         let message_hash = compute_authwit_message_hash(
-            avm::sender(),
+            context.maybe_msg_sender().unwrap(),
             context.chain_id(),
             context.version(),
             inner_hash,
         );
 
         let authorized = storage.approved_actions.at(on_behalf_of).at(message_hash).read();
 
-        assert(true == authorized, "unauthorized");
+        assert_eq(true, authorized, "unauthorized");
         storage.approved_actions.at(on_behalf_of).at(message_hash).write(false);
 
         IS_VALID_SELECTOR

noir-projects/noir-contracts/contracts/protocol/contract_class_registry_contract/src/main.nr

@@ -16,6 +16,7 @@ pub contract ContractClassRegistry {
         protocol::{
             constants::{
                 CONTRACT_CLASS_REGISTRY_BYTECODE_CAPSULE_SLOT,
+                MAX_PACKED_BYTECODE_SIZE_PER_PRIVATE_FUNCTION_IN_FIELDS,
                 MAX_PACKED_BYTECODE_SIZE_PER_UTILITY_FUNCTION_IN_FIELDS,
             },
             contract_class_id::ContractClassId,
@@ -53,12 +54,12 @@ pub contract ContractClassRegistry {
                 .unwrap()
         };
         // Compute and check the public bytecode commitment
-        let computed_public_bytecode_commitment: Field =
+        let computed_public_bytecode_commitment =
             compute_public_bytecode_commitment(packed_public_bytecode);
         assert_eq(computed_public_bytecode_commitment, public_bytecode_commitment);
 
         // Compute contract class id from preimage
-        let contract_class_id: ContractClassId = ContractClassId::compute(
+        let contract_class_id = ContractClassId::compute(
             artifact_hash,
             private_functions_root,
             public_bytecode_commitment,
@@ -131,7 +132,7 @@ pub contract ContractClassRegistry {
         // 1. Unlike public functions, we don't need execution guarantees for private functions.
         // 2. This broadcast simply provides convenient bytecode sharing vs offchain distribution.
         // 3. Computing the VK for private bytecode in-circuit is not possible, so we can't do better.
-        let private_bytecode: [Field; 3000] = unsafe {
+        let private_bytecode: [Field; MAX_PACKED_BYTECODE_SIZE_PER_PRIVATE_FUNCTION_IN_FIELDS] = unsafe {
             capsules::load(
                 context.this_address(),
                 CONTRACT_CLASS_REGISTRY_BYTECODE_CAPSULE_SLOT,

noir-projects/noir-contracts/contracts/protocol/contract_instance_registry_contract/src/main.nr

@@ -136,24 +136,28 @@ pub contract ContractInstanceRegistry {
         context.assert_nullifier_exists(nullifier_existence_request);
 
         // Determine the deployer based on universal_deploy flag
-        let deployer: AztecAddress = if universal_deploy {
+        let deployer = if universal_deploy {
             AztecAddress::zero()
         } else {
             context.maybe_msg_sender().unwrap()
         };
 
-        // Compute the contract address
-        let partial_address: PartialAddress =
+        let partial_address =
             PartialAddress::compute(contract_class_id, salt, initialization_hash, deployer);
 
-        // Validate public keys are on curve
+        // Note: Out of all the public keys, only the Ivpk_m is checked to be on the curve
+        // (implicitly as part of the `add` operation of deriving the address). This at least
+        // protects the AVM against DoS attacks.
+        // For thoroughness, we also ensure the other points are on the curve:
         validate_on_curve(public_keys.npk_m.inner);
         validate_on_curve(public_keys.ovpk_m.inner);
         validate_on_curve(public_keys.tpk_m.inner);
 
-        let address: AztecAddress = AztecAddress::compute(public_keys, partial_address);
+        let address = AztecAddress::compute(public_keys, partial_address);
 
-        // Push nullifier to mark the contract as deployed
+        // Emit the address as a nullifier:
+        // - to demonstrate that this contract instance hasn't been published before
+        // - to enable apps to read that this contract instance has been published.
         context.push_nullifier(address.to_field());
 
         // Emit the deployment event
@@ -167,24 +171,20 @@ pub contract ContractInstanceRegistry {
             deployer,
             version: 1,
         };
-        let payload: [Field; 15] = event.serialize_non_standard();
+        let payload = event.serialize_non_standard();
         debug_log_format("ContractInstancePublished: {}", payload);
-        let padded_log: [Field; 18] = payload.concat([0; 3]);
-        let length: u32 = payload.len();
+        let padded_log = payload.concat([0; 3]);
+        let length = payload.len();
         context.emit_private_log(padded_log, length);
 
         // MACRO CODE START
         context.finish()
         // MACRO CODE END
     }
 
-    /**
-     * Updates the contract class ID for the caller's deployed contract instance.
-     *
-     * @param new_contract_class_id The new contract class ID to update to
-     */
     #[aztec::macros::internals_functions_generation::abi_attributes::abi_public]
     unconstrained fn update(new_contract_class_id: ContractClassId) {
+        // MACRO CODE START
         let context: PublicContext = PublicContext::new(
             || -> Field {
                 let serialized_args: [Field; 1] =
@@ -193,16 +193,19 @@ pub contract ContractInstanceRegistry {
             },
         );
         let storage: Storage<PublicContext> = Storage::init(context);
+        // MACRO CODE END
 
-        let address: AztecAddress = avm::sender();
+        let address = msg_sender().unwrap();
 
-        // Verify that the caller (msg.sender) is a deployed contract
+        // Safety: we're using the nullifier's existence as a guarantee of the availability of the contract's
+        // information through publishing, which is safe - we just need this information to be _eventually_ available.
         assert(
             context.nullifier_exists_unsafe(address.to_field(), context.this_address()),
             "msg.sender is not deployed",
         );
 
-        // Verify that the new contract class is registered
+        // Safety: we're using the nullifier's existence as a guarantee of the availability of the new contract class'
+        // information through registration, which is safe - we just need this information to be _eventually_ available.
         assert(
             context.nullifier_exists_unsafe(
                 new_contract_class_id.to_field(),
@@ -211,14 +214,12 @@ pub contract ContractInstanceRegistry {
             "New contract class is not registered",
         );
 
-        // Schedule the contract class update
         let scheduled_value_update = storage
             .updated_class_ids
             .at(address)
             .schedule_and_return_value_change(new_contract_class_id);
         let (prev_contract_class_id, timestamp_of_change) = scheduled_value_update.get_previous();
 
-        // Emit the update event
         let event = ContractInstanceUpdated {
             CONTRACT_INSTANCE_UPDATED_MAGIC_VALUE,
             address,
@@ -229,33 +230,29 @@ pub contract ContractInstanceRegistry {
         context.emit_public_log(event);
     }
 
-    /**
-     * Sets a new update delay for the caller's contract instance.
-     *
-     * @param new_update_delay The new delay value (must be >= MINIMUM_UPDATE_DELAY)
-     */
     #[aztec::macros::internals_functions_generation::abi_attributes::abi_public]
     unconstrained fn set_update_delay(new_update_delay: u64) {
+        // MACRO CODE START
         let context: PublicContext = PublicContext::new(
             || -> Field {
                 let serialized_args: [Field; 1] = avm::calldata_copy(1, <u64 as Serialize>::N);
                 hash_args(serialized_args)
             },
         );
         let storage: Storage<PublicContext> = Storage::init(context);
+        // MACRO CODE END
 
-        let msg_sender: AztecAddress = avm::sender();
+        let msg_sender = context.maybe_msg_sender().unwrap();
 
-        // Verify that the caller (msg.sender) is a deployed contract
+        // Safety: we're using the nullifier's existence as a guarantee of the availability of the contract's
+        // information through publishing, which is safe - we just need this information to be _eventually_ available.
         assert(
             context.nullifier_exists_unsafe(msg_sender.to_field(), context.this_address()),
             "msg.sender is not deployed",
         );
 
-        // Verify the new delay meets the minimum requirement
         assert(new_update_delay >= MINIMUM_UPDATE_DELAY, "New update delay is too low");
 
-        // Schedule the delay change
         storage.updated_class_ids.at(msg_sender).schedule_delay_change(new_update_delay);
     }
 
@@ -265,16 +262,18 @@ pub contract ContractInstanceRegistry {
      * @return The current update delay value
      */
     #[aztec::macros::internals_functions_generation::abi_attributes::abi_public]
+    #[aztec::macros::internals_functions_generation::abi_attributes::abi_view]
     unconstrained fn get_update_delay() -> pub u64 {
+        // MACRO CODE START
         let context: PublicContext = PublicContext::new(
             || -> Field {
                 let serialized_args: [Field; 0] = avm::calldata_copy(1, 0);
                 hash_args(serialized_args)
             },
         );
         let storage: Storage<PublicContext> = Storage::init(context);
-
         assert(context.is_static_call(), "Function get_update_delay can only be called statically");
+        // MACRO CODE END
 
         storage.updated_class_ids.at(avm::sender()).get_current_delay()
     }

noir-projects/noir-contracts/contracts/protocol/fee_juice_contract/src/main.nr

@@ -138,7 +138,7 @@ pub contract FeeJuice {
         );
         // MACRO CODE END
 
-        let new_balance: u128 = storage.balances.at(to).read().add(amount);
+        let new_balance = storage.balances.at(to).read().add(amount);
         storage.balances.at(to).write(new_balance);
     }