Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion noir-projects/aztec-nr/aztec/src/oracle/version.nr
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Comment thread
nventuro marked this conversation as resolved.
/// 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 {
Expand All @@ -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(),
}
}

Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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() {
Expand All @@ -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();
// The this will result in the exact same address preimage and hence same address, and so the initialization
Comment thread
nventuro marked this conversation as resolved.
Outdated
// 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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@ pub unconstrained fn deploy<let M: u32, let N: u32, let P: u32>(
initializer: str<P>,
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)
}

Expand Down Expand Up @@ -116,6 +118,8 @@ pub unconstrained fn deploy_oracle<let N: u32, let P: u32>(
initializer: str<P>,
args: [Field],
secret: Field,
salt: Field,
deployer: AztecAddress,
) -> [Field; CONTRACT_INSTANCE_LENGTH] {}

#[oracle(aztec_txe_createAccount)]
Expand Down
29 changes: 26 additions & 3 deletions noir-projects/aztec-nr/aztec/src/test/helpers/utils.nr
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ pub struct ContractDeployment<let O: u32> {
pub env: TestEnvironment,
pub path: str<O>,
pub secret: Field,
pub salt: Field,
}

impl<let O: u32> ContractDeployment<O> {
Expand Down Expand Up @@ -38,7 +39,14 @@ impl<let O: u32> ContractDeployment<O> {
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
Expand Down Expand Up @@ -77,7 +85,14 @@ impl<let O: u32> ContractDeployment<O> {
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
Expand All @@ -96,7 +111,15 @@ impl<let O: u32> ContractDeployment<O> {
/// 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()
}
}

Expand Down
2 changes: 1 addition & 1 deletion yarn-project/pxe/src/oracle_version.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 6 additions & 3 deletions yarn-project/txe/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
type ForeignCallResult,
ForeignCallResultSchema,
type ForeignCallSingle,
addressFromSingle,
fromArray,
fromSingle,
toSingle,
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand Down
Loading