feat: merge-train/fairies (#21182)

BEGIN_COMMIT_OVERRIDE
fix: empty undefined offchain messages for batch calls (#21157)
feat: add aztecaddress::is_valid (#21072)
feat: add ergonomic conversions for Noir's `Option<T>` (#21107)
chore: use returns `true` for boolean fns (#21186)
feat: add salt and secret params to env.deploy (#21183)
END_COMMIT_OVERRIDE

Author: AztecBot

noir-projects/aztec-nr/aztec/src/note/note_metadata.nr

@@ -68,21 +68,21 @@ impl NoteMetadata {
         }
     }
 
-    /// Returns true if the note is pending **and** from the same phase, i.e. if it's been created in the current
+    /// Returns `true` if the note is pending **and** from the same phase, i.e. if it's been created in the current
     /// transaction during the current execution phase (either non-revertible or revertible).
     pub fn is_pending_same_phase(self) -> bool {
         self.stage == NoteStage.PENDING_SAME_PHASE
     }
 
-    /// Returns true if the note is pending **and** from the previous phase, i.e. if it's been created in the current
+    /// Returns `true` if the note is pending **and** from the previous phase, i.e. if it's been created in the current
     /// transaction during an execution phase prior to the current one. Because private execution only has two phases
     /// with strict ordering, this implies that the note was created in the non-revertible phase, and that the current
     /// phase is the revertible phase.
     pub fn is_pending_previous_phase(self) -> bool {
         self.stage == NoteStage.PENDING_PREVIOUS_PHASE
     }
 
-    /// Returns true if the note is settled, i.e. if it's been created in a prior transaction and is therefore already
+    /// Returns `true` if the note is settled, i.e. if it's been created in a prior transaction and is therefore already
     /// in the note hash tree.
     pub fn is_settled(self) -> bool {
         self.stage == NoteStage.SETTLED

noir-projects/aztec-nr/aztec/src/oracle/nullifiers.nr

@@ -14,7 +14,7 @@ pub fn notify_created_nullifier(inner_nullifier: Field) {
 #[oracle(aztec_prv_notifyCreatedNullifier)]
 unconstrained fn notify_created_nullifier_oracle(_inner_nullifier: Field) {}
 
-/// Returns true if the nullifier has been emitted in the same transaction, i.e. if [`notify_created_nullifier`] has
+/// Returns `true` if the nullifier has been emitted in the same transaction, i.e. if [`notify_created_nullifier`] has
 /// been
 /// called for this inner nullifier from the contract with the specified address.
 ///
@@ -29,7 +29,7 @@ pub unconstrained fn is_nullifier_pending(inner_nullifier: Field, contract_addre
 #[oracle(aztec_prv_isNullifierPending)]
 unconstrained fn is_nullifier_pending_oracle(_inner_nullifier: Field, _contract_address: AztecAddress) -> bool {}
 
-/// Returns true if the nullifier exists. Note that a `true` value can be constrained by proving existence of the
+/// Returns `true` if the nullifier exists. Note that a `true` value can be constrained by proving existence of the
 /// nullifier, but a `false` value should not be relied upon since other transactions may emit this nullifier before
 /// the current transaction is included in a block. While this might seem of little use at first, certain design
 /// patterns benefit from this abstraction (see e.g. `PrivateMutable`).

noir-projects/aztec-nr/aztec/src/oracle/version.nr

@@ -5,7 +5,7 @@
 /// @dev Whenever a contract function or Noir test is run, the `aztec_utl_assertCompatibleOracleVersion` oracle is
 /// called
 /// and if the oracle version is incompatible an error is thrown.
-pub global ORACLE_VERSION: Field = 13;
+pub global ORACLE_VERSION: Field = 14;
 
 /// Asserts that the version of the oracle is compatible with the version expected by the contract.
 pub fn assert_compatible_oracle_version() {

noir-projects/aztec-nr/aztec/src/state_vars/private_immutable.nr

@@ -143,7 +143,7 @@ where
         self.compute_initialization_nullifier(secret)
     }
 
-    /// Returns whether this PrivateImmutable has been initialized.
+    /// Returns `true` if this PrivateImmutable has been initialized.
     pub unconstrained fn is_initialized(self) -> bool {
         let nullifier = self.get_initialization_nullifier();
         check_nullifier_exists(nullifier)

noir-projects/aztec-nr/aztec/src/state_vars/public_immutable.nr

@@ -214,7 +214,7 @@ impl<T> PublicImmutable<T, PublicContext> {
         WithHash::public_storage_read(self.context, self.storage_slot)
     }
 
-    /// Returns true if the `PublicImmutable` has been initialized.
+    /// Returns `true` if the `PublicImmutable` has been initialized.
     ///
     /// ## Examples:
     ///
@@ -263,7 +263,7 @@ impl<T> PublicImmutable<T, UtilityContext> {
         WithHash::utility_public_storage_read(self.context, self.storage_slot)
     }
 
-    /// Returns true if the `PublicImmutable` has been initialized.
+    /// Returns `true` if the `PublicImmutable` has been initialized.
     pub unconstrained fn is_initialized(self) -> bool {
         let nullifier = self.compute_initialization_inner_nullifier();
         check_nullifier_exists(nullifier)

noir-projects/aztec-nr/aztec/src/state_vars/single_private_immutable.nr

@@ -158,7 +158,7 @@ where
         self.compute_initialization_nullifier(secret)
     }
 
-    /// Returns whether this SinglePrivateImmutable has been initialized.
+    /// Returns `true` if this SinglePrivateImmutable has been initialized.
     pub unconstrained fn is_initialized(self) -> bool {
         let nullifier = self.get_initialization_nullifier();
         check_nullifier_exists(nullifier)

noir-projects/aztec-nr/aztec/src/state_vars/single_private_mutable.nr

@@ -249,7 +249,7 @@ where
         self.compute_initialization_nullifier(secret)
     }
 
-    /// Returns whether this SinglePrivateImmutable has been initialized.
+    /// Returns `true` if this SinglePrivateMutable has been initialized.
     pub unconstrained fn is_initialized(self) -> bool {
         let nullifier = self.get_initialization_nullifier();
         check_nullifier_exists(nullifier)

noir-projects/aztec-nr/aztec/src/state_vars/single_use_claim.nr

@@ -132,7 +132,7 @@ impl SingleUseClaim<&mut PrivateContext> {
 }
 
 impl SingleUseClaim<UtilityContext> {
-    /// Returns whether an owner has claimed this single use claim.
+    /// Returns `true` if an owner has claimed this single use claim.
     pub unconstrained fn has_claimed(self) -> bool {
         let owner_nhk_app = get_nhk_app(get_public_keys(self.owner).npk_m.hash());
         check_nullifier_exists(self.compute_nullifier(owner_nhk_app))

noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr

@@ -76,16 +76,17 @@ impl Counter {
 /// }
 /// ```
 pub struct TestEnvironment {
-    // The secrets to be used for light and contract account creation, as well as contract deployments. By keeping
-    // track of the last used secret we can issue new ones automatically without requiring the user to provide
+    // The secrets and salt to be used for light and contract account creation, as well as contract deployments. By
+    // keeping track of the last used secret we can issue new ones automatically without requiring the user to provide
     // different ones.
     //
-    // Additionally, having the secrets be deterministic for each set of accounts and all concurrent tests results in
-    // TXE being able to maximize cache usage and not have to recompute account addresses and contract artifacts, which
-    // are relatively expensive operations.
+    // Additionally, having the secrets and salt be deterministic for each set of accounts and all concurrent tests
+    // results in TXE being able to maximize cache usage and not have to recompute account addresses and contract
+    // artifacts, which are relatively expensive operations.
     light_account_secret: Counter,
     contract_account_secret: Counter,
     contract_deployment_secret: Counter,
+    contract_deployment_salt: Counter,
 }
 
 /// Configuration values for [`TestEnvironment::private_context_opts`]. Meant to be used by calling `new` and then
@@ -138,6 +139,38 @@ struct EventDiscoveryOptions {
     contract_address: Option<AztecAddress>,
 }
 
+/// Configuration values for [`TestEnvironment::deploy_opts`]. Meant to be used by calling `new` and then chaining
+/// methods setting each value, e.g.:
+/// ```noir
+/// env.deploy_opts(DeployOptions::new().with_salt(42).with_secret(100), "MyContract").without_initializer();
+/// ```
+pub struct DeployOptions {
+    salt: Option<Field>,
+    secret: Option<Field>,
+}
+
+impl DeployOptions {
+    /// Creates a new `DeployOptions` with default values, i.e. the same as if using the `deploy` method instead of
+    /// `deploy_opts`. Use the `with_salt` and `with_secret` methods to set the desired configuration values.
+    pub fn new() -> Self {
+        Self { salt: Option::none(), secret: Option::none() }
+    }
+
+    /// Sets the deployment salt. The salt affects the resulting contract address: the same contract deployed with
+    /// different salts will have different addresses.
+    pub fn with_salt(&mut self, salt: Field) -> Self {
+        self.salt = Option::some(salt);
+        *self
+    }
+
+    /// Sets the secret used for key derivation. The secret affects the contract's public keys and therefore its
+    /// address: the same contract deployed with different secrets will have different addresses.
+    pub fn with_secret(&mut self, secret: Field) -> Self {
+        self.secret = Option::some(secret);
+        *self
+    }
+}
+
 impl TestEnvironment {
     /// Creates a new `TestEnvironment`. This function should only be called once per test.
     pub unconstrained fn new() -> Self {
@@ -150,6 +183,9 @@ impl TestEnvironment {
             light_account_secret: Counter::new(),
             contract_account_secret: Counter::new_with_offset(1_000),
             contract_deployment_secret: Counter::new_with_offset(2_000),
+            // The salt counter does not need an offset because keys (derived from secret) and salt are unrelated: a
+            // collision between a salt and a secret value has no effect on the resulting contract address.
+            contract_deployment_salt: Counter::new(),
         }
     }
 
@@ -477,7 +513,19 @@ impl TestEnvironment {
     /// );
     /// ```
     pub unconstrained fn deploy<let N: u32>(&mut self, path: str<N>) -> ContractDeployment<N> {
-        ContractDeployment { env: *self, path, secret: self.contract_deployment_secret.next() }
+        self.deploy_opts(DeployOptions::new(), path)
+    }
+
+    /// Variant of `deploy` which allows specifying configuration values via `DeployOptions`, such as a custom salt
+    /// or secret. If not specified, the salt and secret default to values from internal counters.
+    pub unconstrained fn deploy_opts<let N: u32>(
+        &mut self,
+        opts: DeployOptions,
+        path: str<N>,
+    ) -> ContractDeployment<N> {
+        let secret = opts.secret.unwrap_or_else(|| self.contract_deployment_secret.next());
+        let salt = opts.salt.unwrap_or_else(|| self.contract_deployment_salt.next());
+        ContractDeployment { env: *self, path, secret, salt }
     }
 
     /// Performs a private contract function call, including the processing of any nested private calls and enqueued

noir-projects/aztec-nr/aztec/src/test/helpers/test_environment/test/deployment.nr

@@ -1,4 +1,4 @@
-use crate::test::helpers::test_environment::TestEnvironment;
+use crate::test::helpers::test_environment::{DeployOptions, TestEnvironment};
 
 #[test]
 unconstrained fn deploy_does_not_repeat_addresses() {
@@ -15,3 +15,43 @@ unconstrained fn deploy_does_not_repeat_addresses() {
     // This at least does test that the basics of the deployment mechanism work.
     assert(first_contract != second_contract);
 }
+
+#[test(should_fail_with = "NullifierTree")]
+unconstrained fn deploy_with_same_salt_and_secret_fails() {
+    let mut env = TestEnvironment::new();
+
+    let opts = DeployOptions::new().with_salt(42).with_secret(100);
+
+    let _ = env.deploy_opts(opts, "../noir-contracts/@test_contract/Test").without_initializer();
+    // This will result in the exact same address preimage and hence same address, and so the initialization
+    // nullifiers will be duplicated.
+    let _ = env.deploy_opts(opts, "../noir-contracts/@test_contract/Test").without_initializer();
+}
+
+#[test]
+unconstrained fn deploy_with_same_salt_does_not_repeat_addresses() {
+    let mut env = TestEnvironment::new();
+
+    let first_contract = env
+        .deploy_opts(DeployOptions::new().with_salt(42).with_secret(100), "../noir-contracts/@test_contract/Test")
+        .without_initializer();
+    let second_contract = env
+        .deploy_opts(DeployOptions::new().with_salt(42).with_secret(200), "../noir-contracts/@test_contract/Test")
+        .without_initializer();
+
+    assert(first_contract != second_contract);
+}
+
+#[test]
+unconstrained fn deploy_with_same_secret_does_not_repeat_addresses() {
+    let mut env = TestEnvironment::new();
+
+    let first_contract = env
+        .deploy_opts(DeployOptions::new().with_salt(42).with_secret(100), "../noir-contracts/@test_contract/Test")
+        .without_initializer();
+    let second_contract = env
+        .deploy_opts(DeployOptions::new().with_salt(43).with_secret(100), "../noir-contracts/@test_contract/Test")
+        .without_initializer();
+
+    assert(first_contract != second_contract);
+}