diff --git a/noir-projects/aztec-nr/aztec/src/oracle/version.nr b/noir-projects/aztec-nr/aztec/src/oracle/version.nr index 673822ce14a6..e3040821aa8a 100644 --- a/noir-projects/aztec-nr/aztec/src/oracle/version.nr +++ b/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() { diff --git a/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr b/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr index c48e8b5c023c..eed98c35fbb6 100644 --- a/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr +++ b/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, } +/// 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, + secret: Option, +} + +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(&mut self, path: str) -> ContractDeployment { - 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( + &mut self, + opts: DeployOptions, + path: str, + ) -> ContractDeployment { + 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 diff --git a/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment/test/deployment.nr b/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment/test/deployment.nr index 38ae1adfc641..06dbd33264ba 100644 --- a/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment/test/deployment.nr +++ b/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); +} diff --git a/noir-projects/aztec-nr/aztec/src/test/helpers/txe_oracles.nr b/noir-projects/aztec-nr/aztec/src/test/helpers/txe_oracles.nr index 608352e06d6b..2b36c86ebae3 100644 --- a/noir-projects/aztec-nr/aztec/src/test/helpers/txe_oracles.nr +++ b/noir-projects/aztec-nr/aztec/src/test/helpers/txe_oracles.nr @@ -18,8 +18,10 @@ pub unconstrained fn deploy( initializer: str

, args: [Field; M], secret: Field, + salt: Field, + deployer: AztecAddress, ) -> ContractInstance { - let instance_fields = deploy_oracle(path, initializer, args, secret); + let instance_fields = deploy_oracle(path, initializer, args, secret, salt, deployer); ContractInstance::deserialize(instance_fields) } @@ -116,6 +118,8 @@ pub unconstrained fn deploy_oracle( initializer: str

, args: [Field], secret: Field, + salt: Field, + deployer: AztecAddress, ) -> [Field; CONTRACT_INSTANCE_LENGTH] {} #[oracle(aztec_txe_createAccount)] diff --git a/noir-projects/aztec-nr/aztec/src/test/helpers/utils.nr b/noir-projects/aztec-nr/aztec/src/test/helpers/utils.nr index d5e0db6b7aa8..32806b7141f3 100644 --- a/noir-projects/aztec-nr/aztec/src/test/helpers/utils.nr +++ b/noir-projects/aztec-nr/aztec/src/test/helpers/utils.nr @@ -6,6 +6,7 @@ pub struct ContractDeployment { pub env: TestEnvironment, pub path: str, pub secret: Field, + pub salt: Field, } impl ContractDeployment { @@ -38,7 +39,14 @@ impl ContractDeployment { let name = initializer_call.name; let selector = initializer_call.selector; let args = initializer_call.args; - let instance = txe_oracles::deploy(self.path, name, args, self.secret); + let instance = txe_oracles::deploy( + self.path, + name, + args, + self.secret, + self.salt, + AztecAddress::zero(), + ); // initializer_call does not actually have the target_contract value set - it is created with the helper // `interface` function created by `generate_contract_interface` in the aztec macros - it represents a call to @@ -77,7 +85,14 @@ impl ContractDeployment { let name = initializer_call.name; let selector = initializer_call.selector; let args = initializer_call.args; - let instance = txe_oracles::deploy(self.path, name, args, self.secret); + let instance = txe_oracles::deploy( + self.path, + name, + args, + self.secret, + self.salt, + AztecAddress::zero(), + ); // initializer_call does not actually have the target_contract value set - it is created with the helper // `interface` function created by `generate_contract_interface` in the aztec macros - it represents a call to @@ -96,7 +111,15 @@ impl ContractDeployment { /// contains a commitment to the lack of initialization arguments as per the protocol rules. Initializers can only /// be invoked by using the `with_private_initializer` or `with_public_initializer` functions. pub unconstrained fn without_initializer(self) -> AztecAddress { - txe_oracles::deploy(self.path, "", [], self.secret).to_address() + txe_oracles::deploy( + self.path, + "", + [], + self.secret, + self.salt, + AztecAddress::zero(), + ) + .to_address() } } diff --git a/yarn-project/pxe/src/oracle_version.ts b/yarn-project/pxe/src/oracle_version.ts index 898b3e631450..e282867c911f 100644 --- a/yarn-project/pxe/src/oracle_version.ts +++ b/yarn-project/pxe/src/oracle_version.ts @@ -4,7 +4,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. -export const ORACLE_VERSION = 13; +export const ORACLE_VERSION = 14; /// This hash is computed as by hashing the Oracle interface and it is used to detect when the Oracle interface changes, /// which in turn implies that you need to update the ORACLE_VERSION constant in this file and in diff --git a/yarn-project/txe/src/index.ts b/yarn-project/txe/src/index.ts index 182058ddf8ac..408813fe153e 100644 --- a/yarn-project/txe/src/index.ts +++ b/yarn-project/txe/src/index.ts @@ -32,6 +32,7 @@ import { type ForeignCallResult, ForeignCallResultSchema, type ForeignCallSingle, + addressFromSingle, fromArray, fromSingle, toSingle, @@ -105,6 +106,8 @@ class TXEDispatcher { const decodedArgs = fromArray(inputs[3] as ForeignCallArray); const secret = fromSingle(inputs[4] as ForeignCallSingle); + const salt = fromSingle(inputs[5] as ForeignCallSingle); + const deployer = addressFromSingle(inputs[6] as ForeignCallSingle); const publicKeys = secret.equals(Fr.ZERO) ? PublicKeys.default() : (await deriveKeys(secret)).publicKeys; const publicKeysHash = await publicKeys.hash(); @@ -135,7 +138,7 @@ class TXEDispatcher { const cacheKey = `${contractDirectory ?? ''}-${contractFilename}-${initializer}-${decodedArgs .map(arg => arg.toString()) - .join('-')}-${publicKeysHash}-${fileHash}`; + .join('-')}-${publicKeysHash}-${salt}-${deployer}-${fileHash}`; let instance; let artifact: ContractArtifactWithHash; @@ -161,10 +164,10 @@ class TXEDispatcher { const computedInstance = await getContractInstanceFromInstantiationParams(computedArtifact, { constructorArgs: decodedArgs, skipArgsDecoding: true, - salt: Fr.ONE, + salt, publicKeys, constructorArtifact: initializer ? initializer : undefined, - deployer: AztecAddress.ZERO, + deployer, }); const result = { artifact: computedArtifact, instance: computedInstance }; TXEArtifactsCache.set(cacheKey, result);