diff --git a/noir-projects/aztec-nr/aztec/src/note/note_metadata.nr b/noir-projects/aztec-nr/aztec/src/note/note_metadata.nr index e006b361f654..8bd6c2bc36a7 100644 --- a/noir-projects/aztec-nr/aztec/src/note/note_metadata.nr +++ b/noir-projects/aztec-nr/aztec/src/note/note_metadata.nr @@ -68,13 +68,13 @@ 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. @@ -82,7 +82,7 @@ impl NoteMetadata { 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 diff --git a/noir-projects/aztec-nr/aztec/src/oracle/nullifiers.nr b/noir-projects/aztec-nr/aztec/src/oracle/nullifiers.nr index 323028dac4db..7c8e64a85574 100644 --- a/noir-projects/aztec-nr/aztec/src/oracle/nullifiers.nr +++ b/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`). 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/state_vars/private_immutable.nr b/noir-projects/aztec-nr/aztec/src/state_vars/private_immutable.nr index 8e3acdf200ba..ef84849f3ebb 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/private_immutable.nr +++ b/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) diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/public_immutable.nr b/noir-projects/aztec-nr/aztec/src/state_vars/public_immutable.nr index b001b4161c6e..da21ab7d3a1c 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/public_immutable.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/public_immutable.nr @@ -214,7 +214,7 @@ impl PublicImmutable { 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 PublicImmutable { 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) diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/single_private_immutable.nr b/noir-projects/aztec-nr/aztec/src/state_vars/single_private_immutable.nr index 8a83efad2e79..326a4843110d 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/single_private_immutable.nr +++ b/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) diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/single_private_mutable.nr b/noir-projects/aztec-nr/aztec/src/state_vars/single_private_mutable.nr index e2c0e05d7625..d286aa346b78 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/single_private_mutable.nr +++ b/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) diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/single_use_claim.nr b/noir-projects/aztec-nr/aztec/src/state_vars/single_use_claim.nr index 2edf233a6f9c..4945119afbc7 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/single_use_claim.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/single_use_claim.nr @@ -132,7 +132,7 @@ impl SingleUseClaim<&mut PrivateContext> { } impl SingleUseClaim { - /// 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)) 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/noir-projects/noir-contracts/Nargo.toml b/noir-projects/noir-contracts/Nargo.toml index a3c05bd05a9c..0e0f8c618ead 100644 --- a/noir-projects/noir-contracts/Nargo.toml +++ b/noir-projects/noir-contracts/Nargo.toml @@ -50,6 +50,7 @@ members = [ "contracts/test/note_getter_contract", "contracts/test/offchain_effect_contract", "contracts/test/only_self_contract", + "contracts/test/option_param_contract", "contracts/test/oracle_version_check_contract", "contracts/test/parent_contract", "contracts/test/pending_note_hashes_contract", diff --git a/noir-projects/noir-contracts/contracts/test/option_param_contract/Nargo.toml b/noir-projects/noir-contracts/contracts/test/option_param_contract/Nargo.toml new file mode 100644 index 000000000000..7f4a4e434f0d --- /dev/null +++ b/noir-projects/noir-contracts/contracts/test/option_param_contract/Nargo.toml @@ -0,0 +1,8 @@ +[package] +name = "option_param_contract" +authors = [""] +compiler_version = ">=0.25.0" +type = "contract" + +[dependencies] +aztec = { path = "../../../../aztec-nr/aztec" } diff --git a/noir-projects/noir-contracts/contracts/test/option_param_contract/src/main.nr b/noir-projects/noir-contracts/contracts/test/option_param_contract/src/main.nr new file mode 100644 index 000000000000..a30e26bbed67 --- /dev/null +++ b/noir-projects/noir-contracts/contracts/test/option_param_contract/src/main.nr @@ -0,0 +1,33 @@ +mod test; + +use aztec::macros::aztec; + +#[aztec] +pub contract OptionParam { + use aztec::{macros::functions::external, protocol::traits::{Deserialize, Serialize}}; + + #[derive(Serialize, Deserialize, Eq)] + pub struct SomeStruct { + pub w: Field, + pub x: bool, + pub y: u64, + pub z: i64, + } + + #[external("public")] + fn return_public_optional_struct(value: Option) -> Option { + value + } + + #[external("private")] + fn return_private_optional_struct(value: Option) -> Option { + value + } + + #[external("utility")] + unconstrained fn return_utility_optional_struct( + value: Option, + ) -> Option { + value + } +} diff --git a/noir-projects/noir-contracts/contracts/test/option_param_contract/src/test.nr b/noir-projects/noir-contracts/contracts/test/option_param_contract/src/test.nr new file mode 100644 index 000000000000..816f461b1448 --- /dev/null +++ b/noir-projects/noir-contracts/contracts/test/option_param_contract/src/test.nr @@ -0,0 +1,77 @@ +use crate::OptionParam; +use crate::OptionParam::SomeStruct; +use aztec::{protocol::constants::MAX_FIELD_VALUE, test::helpers::test_environment::TestEnvironment}; + +global U64_MAX: u64 = (2.pow_32(64) - 1) as u64; +global I64_MAX: i64 = (2.pow_32(63) - 1) as i64; + +#[test] +unconstrained fn passes_public_optional_struct_parameter() { + let mut env = TestEnvironment::new(); + let sender = env.create_light_account(); + + let option_param = OptionParam::at(env.deploy("OptionParam").without_initializer()); + + let none_value = + env.call_public(sender, option_param.return_public_optional_struct(Option::none())); + assert_eq(none_value, Option::none()); + + let some_value = env.call_public( + sender, + option_param.return_public_optional_struct(Option::some( + SomeStruct { w: MAX_FIELD_VALUE, x: true, y: U64_MAX, z: I64_MAX }, + )), + ); + assert_eq( + some_value, + Option::some( + SomeStruct { w: MAX_FIELD_VALUE, x: true, y: U64_MAX, z: I64_MAX }, + ), + ); +} + +#[test] +unconstrained fn passes_utility_optional_struct_parameter() { + let mut env = TestEnvironment::new(); + + let option_param = OptionParam::at(env.deploy("OptionParam").without_initializer()); + + let none_value = + env.execute_utility(option_param.return_utility_optional_struct(Option::none())); + assert_eq(none_value, Option::none()); + + let some_value = env.execute_utility(option_param.return_utility_optional_struct(Option::some( + SomeStruct { w: MAX_FIELD_VALUE, x: true, y: U64_MAX, z: I64_MAX }, + ))); + assert_eq( + some_value, + Option::some( + SomeStruct { w: MAX_FIELD_VALUE, x: true, y: U64_MAX, z: I64_MAX }, + ), + ); +} + +#[test] +unconstrained fn passes_private_optional_struct_parameter() { + let mut env = TestEnvironment::new(); + let sender = env.create_light_account(); + + let option_param = OptionParam::at(env.deploy("OptionParam").without_initializer()); + + let none_value = + env.call_private(sender, option_param.return_private_optional_struct(Option::none())); + assert_eq(none_value, Option::none()); + + let some_value = env.call_private( + sender, + option_param.return_private_optional_struct(Option::some( + SomeStruct { w: MAX_FIELD_VALUE, x: true, y: U64_MAX, z: I64_MAX }, + )), + ); + assert_eq( + some_value, + Option::some( + SomeStruct { w: MAX_FIELD_VALUE, x: true, y: U64_MAX, z: I64_MAX }, + ), + ); +} diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/address/aztec_address.nr b/noir-projects/noir-protocol-circuits/crates/types/src/address/aztec_address.nr index 3c1ac469c9af..38e063a62be9 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/address/aztec_address.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/address/aztec_address.nr @@ -49,19 +49,20 @@ impl AztecAddress { Self { inner: 0 } } - /// Returns an address's `AddressPoint`, which can be used to create shared secrets with the owner - /// of the address. If the address is invalid (i.e. it is not a properly derived Aztec address), then this - /// returns `Option::none()`, and no shared secrets can be created. - pub fn to_address_point(self) -> Option { - // We compute the address point by taking our address as x, and then solving for y in the - // equation which defines the grumpkin curve: - // y^2 = x^3 - 17; x = address - let x = self.inner; - let y_squared = x * x * x - 17; + /// Returns `true` if the address is valid. + /// + /// An invalid address is one that can be proven to not be correctly derived, meaning it contains no contract code, + /// public keys, etc., and can therefore not receive messages nor execute calls. + pub fn is_valid(self) -> bool { + self.get_y().is_some() + } - // An invalid AztecAddress is one for which no y coordinate satisfies the curve equation, which we'll - // identify by proving that the square root of y_squared does not exist. - sqrt(y_squared).map(|y| { + /// Returns an address's [`AddressPoint`]. + /// + /// This can be used to create shared secrets with the owner of the address. If the address is invalid (see + /// [`AztecAddress::is_valid`]) then this returns `Option::none()`, and no shared secrets can be created. + pub fn to_address_point(self) -> Option { + self.get_y().map(|y| { // If we get a negative y coordinate (y > (r - 1) / 2), we swap it to the // positive one (where y <= (r - 1) / 2) by negating it. let final_y = if Self::is_positive(y) { y } else { -y }; @@ -83,6 +84,24 @@ impl AztecAddress { y.lt(MID_PLUS_1) } + /// Returns one of the two possible y-coordinates. + /// + /// Not all `AztecAddresses` are valid, in which case there is no corresponding y-coordinate. This returns + /// `Option::none()` for invalid addresses. + /// + /// An `AztecAddress` is defined by an x-coordinate, for which two y-coordinates exist as solutions to the curve + /// equation. This function returns either of them. Note that an [`AddressPoint`] must **always** have a positive + /// y-coordinate - if trying to obtain the underlying point use [`AztecAddress::to_address_point`] instead. + fn get_y(self) -> Option { + // We compute the address point by taking our address as x, and then solving for y in the + // equation which defines the grumpkin curve: + // y^2 = x^3 - 17; x = address + let x = self.inner; + let y_squared = x * x * x - 17; + + sqrt(y_squared) + } + pub fn compute(public_keys: PublicKeys, partial_address: PartialAddress) -> AztecAddress { // // address = address_point.x @@ -265,6 +284,10 @@ fn serde() { fn to_address_point_valid() { // x = 8 where x^3 - 17 = 512 - 17 = 495, which is a residue in this field let address = AztecAddress { inner: 8 }; + + assert(address.get_y().is_some()); // We don't bother checking the result of get_y as it is only used internally + assert(address.is_valid()); + let maybe_point = address.to_address_point(); assert(maybe_point.is_some()); @@ -279,7 +302,10 @@ fn to_address_point_valid() { #[test] unconstrained fn to_address_point_invalid() { // x = 3 where x^3 - 17 = 27 - 17 = 10, which is a non-residue in this field - let address = AztecAddress { inner: 3 }; // - let maybe_point = address.to_address_point(); - assert(maybe_point.is_none()); + let address = AztecAddress { inner: 3 }; + + assert(address.get_y().is_none()); + assert(!address.is_valid()); + + assert(address.to_address_point().is_none()); } diff --git a/yarn-project/aztec.js/src/api/abi.ts b/yarn-project/aztec.js/src/api/abi.ts index abaedaba9b4c..56ff6448708a 100644 --- a/yarn-project/aztec.js/src/api/abi.ts +++ b/yarn-project/aztec.js/src/api/abi.ts @@ -31,6 +31,7 @@ export { type EventSelectorLike, type FieldLike, type FunctionSelectorLike, + type OptionLike, type U128Like, type WrappedFieldLike, } from '../utils/abi_types.js'; diff --git a/yarn-project/aztec.js/src/contract/batch_call.test.ts b/yarn-project/aztec.js/src/contract/batch_call.test.ts index bcb976b1729c..75d6ed0a3293 100644 --- a/yarn-project/aztec.js/src/contract/batch_call.test.ts +++ b/yarn-project/aztec.js/src/contract/batch_call.test.ts @@ -220,6 +220,22 @@ describe('BatchCall', () => { expect(results[1].result).toEqual(utilityResult2.result[0].toBigInt()); }); + it('should include empty offchainEffects and offchainMessages in utility call results', async () => { + const contractAddress = await AztecAddress.random(); + const utilityPayload = createUtilityExecutionPayload('view', [], contractAddress); + + batchCall = new BatchCall(wallet, [utilityPayload]); + + const utilityResult = UtilityExecutionResult.random(); + wallet.batch.mockResolvedValue([{ name: 'executeUtility', result: utilityResult }] as any); + + const results = await batchCall.simulate({ from: await AztecAddress.random() }); + + expect(results).toHaveLength(1); + expect(results[0].offchainEffects).toEqual([]); + expect(results[0].offchainMessages).toEqual([]); + }); + it('should handle only private/public calls using wallet.batch with simulateTx', async () => { const contractAddress1 = await AztecAddress.random(); const contractAddress2 = await AztecAddress.random(); diff --git a/yarn-project/aztec.js/src/contract/batch_call.ts b/yarn-project/aztec.js/src/contract/batch_call.ts index c1d5d183880c..66c668340687 100644 --- a/yarn-project/aztec.js/src/contract/batch_call.ts +++ b/yarn-project/aztec.js/src/contract/batch_call.ts @@ -6,6 +6,7 @@ import { BaseContractInteraction } from './base_contract_interaction.js'; import { type RequestInteractionOptions, type SimulateInteractionOptions, + emptyOffchainOutput, extractOffchainOutput, toSimulateOptions, } from './interaction_options.js'; @@ -111,7 +112,7 @@ export class BatchCall extends BaseContractInteraction { const rawReturnValues = (wrappedResult.result as UtilityExecutionResult).result; results[resultIndex] = { result: rawReturnValues ? decodeFromAbi(call.returnTypes, rawReturnValues) : [], - offchainEffects: [], + ...emptyOffchainOutput(), }; } } diff --git a/yarn-project/aztec.js/src/contract/contract.test.ts b/yarn-project/aztec.js/src/contract/contract.test.ts index 36519b918f7c..50394dc23226 100644 --- a/yarn-project/aztec.js/src/contract/contract.test.ts +++ b/yarn-project/aztec.js/src/contract/contract.test.ts @@ -110,6 +110,73 @@ describe('Contract Class', () => { debugSymbols: '', errorTypes: {}, }, + { + name: 'optionEcho', + isInitializer: false, + functionType: FunctionType.PRIVATE, + isOnlySelf: false, + isStatic: false, + parameters: [ + { + name: 'value', + type: { + kind: 'struct', + path: 'std::option::Option', + fields: [ + { + name: '_is_some', + type: { + kind: 'boolean', + }, + }, + { + name: '_value', + type: { + kind: 'field', + }, + }, + ], + }, + visibility: 'private', + }, + ], + returnTypes: [], + errorTypes: {}, + bytecode: Buffer.alloc(8, 0xfd), + verificationKey: Buffer.alloc(4064).toString('base64'), + debugSymbols: '', + }, + { + name: 'mixedParams', + isInitializer: false, + functionType: FunctionType.PRIVATE, + isOnlySelf: false, + isStatic: false, + parameters: [ + { + name: 'optValue', + type: { + kind: 'struct', + path: 'std::option::Option', + fields: [ + { name: '_is_some', type: { kind: 'boolean' } }, + { name: '_value', type: { kind: 'field' } }, + ], + }, + visibility: 'private', + }, + { + name: 'aField', + type: { kind: 'field' }, + visibility: 'private', + }, + ], + returnTypes: [], + errorTypes: {}, + bytecode: Buffer.alloc(8, 0xfe), + verificationKey: Buffer.alloc(4064).toString('base64'), + debugSymbols: '', + }, ], nonDispatchPublicFunctions: [], outputs: { @@ -160,4 +227,48 @@ describe('Contract Class', () => { ); expect(result).toBe(42n); }); + + it('allows nullish values for Option parameters', () => { + const fooContract = Contract.at(contractAddress, defaultArtifact, wallet); + + expect(() => fooContract.methods.optionEcho(undefined)).not.toThrow(); + expect(() => fooContract.methods.optionEcho(null)).not.toThrow(); + }); + + it('still rejects nullish values for non-Option parameters', () => { + const fooContract = Contract.at(contractAddress, defaultArtifact, wallet); + + expect(() => fooContract.methods.bar(undefined, 123n)).toThrow( + 'Null or undefined arguments are only allowed for Option parameters in bar(value: Field, value: Field). Received: (undefined, 123n).', + ); + expect(() => fooContract.methods.qux(null)).toThrow( + 'Null or undefined arguments are only allowed for Option parameters in qux(value: Field). Received: (null).', + ); + }); + + it('rejects nullish non-Option param even when Option param is valid', () => { + const fooContract = Contract.at(contractAddress, defaultArtifact, wallet); + + expect(() => fooContract.methods.mixedParams({ w: 1n }, undefined)).toThrow( + 'Null or undefined arguments are only allowed for Option parameters in mixedParams(optValue: Option, aField: Field). Received: ({ w: 1n }, undefined).', + ); + }); + + // Check basic formatting of null/undefined related errors + it.each([ + ['undefined', undefined, 'undefined'], + ['null', null, 'null'], + ['number', 42, '42'], + ['bigint', 123n, '123n'], + ['string', 'hello', 'hello'], + ['boolean', true, 'true'], + ['symbol', Symbol('test'), 'Symbol(test)'], + ['object', { a: 1n, b: 'x' }, '{ a: 1n, b: x }'], + ['array', [1n, 2n], '[1n, 2n]'], + ])('formats %s argument in error message', (_label, value, expectedFormatted) => { + const fooContract = Contract.at(contractAddress, defaultArtifact, wallet); + + // pass the test value first and undefined second to trigger the error whose message we want to test for + expect(() => fooContract.methods.bar(value, undefined)).toThrow(`Received: (${expectedFormatted}, undefined).`); + }); }); diff --git a/yarn-project/aztec.js/src/contract/contract_function_interaction.ts b/yarn-project/aztec.js/src/contract/contract_function_interaction.ts index 30d553d55151..e230aa1cddf8 100644 --- a/yarn-project/aztec.js/src/contract/contract_function_interaction.ts +++ b/yarn-project/aztec.js/src/contract/contract_function_interaction.ts @@ -1,10 +1,14 @@ import { + type ABIParameter, + type AbiType, type FunctionAbi, FunctionCall, FunctionSelector, FunctionType, + canBeMappedFromNullOrUndefined, decodeFromAbi, encodeArguments, + isOptionStruct, } from '@aztec/stdlib/abi'; import type { AuthWitness } from '@aztec/stdlib/auth-witness'; import { AztecAddress } from '@aztec/stdlib/aztec-address'; @@ -40,11 +44,32 @@ export class ContractFunctionInteraction extends BaseContractInteraction { private extraHashedArgs: HashedValues[] = [], ) { super(wallet, authWitnesses, capsules); - if (args.some(arg => arg === undefined || arg === null)) { - throw new Error(`All function interaction arguments must be defined and not null. Received: ${args}`); + // This may feel a bit ad-hoc here, so it warrants a comment. We accept Noir Option parameters, and it's natural + // to map JS's null/undefined to Noir Option's None. One possible way to deal with null/undefined arguments at this + // point in the codebase is to conclude that they are accepted since at least one Noir type (ie: Option) can be + // encoded from them. Then we would let `encode` deal with potential mismatches. I chose not to do that because of + // the pervasiveness of null/undefined in JS, and how easy it is to inadvertently pass it around. Having this check + // here allows us to fail at a point where the boundaries and intent are clear. + if (this.hasInvalidNullOrUndefinedArguments(args)) { + const signature = formatFunctionSignature(this.functionDao.name, this.functionDao.parameters); + const received = args.map(formatArg).join(', '); + throw new Error( + `Null or undefined arguments are only allowed for Option parameters in ${signature}. Received: (${received}).`, + ); } } + private hasInvalidNullOrUndefinedArguments(args: any[]) { + return args.some((arg, index) => { + if (arg !== undefined && arg !== null) { + return false; + } + + const parameterType = this.functionDao.parameters[index]?.type; + return !parameterType || !canBeMappedFromNullOrUndefined(parameterType); + }); + } + /** * Returns the encoded function call wrapped by this interaction * Useful when generating authwits @@ -204,3 +229,62 @@ export class ContractFunctionInteraction extends BaseContractInteraction { ); } } + +/** + * Render an AbiType as a human readable string + * */ +function formatAbiType(abiType: AbiType): string { + switch (abiType.kind) { + case 'field': + return 'Field'; + case 'boolean': + return 'bool'; + case 'integer': + return `${abiType.sign === 'signed' ? 'i' : 'u'}${abiType.width}`; + case 'string': + return `str<${abiType.length}>`; + case 'array': + return `[${formatAbiType(abiType.type)}; ${abiType.length}]`; + case 'struct': { + if (isOptionStruct(abiType)) { + const innerType = abiType.fields.find(f => f.name === '_value')!.type; + return `Option<${formatAbiType(innerType)}>`; + } + return `(${abiType.fields.map(f => `${f.name}: ${formatAbiType(f.type)}`).join(', ')})`; + } + case 'tuple': + return `(${abiType.fields.map(formatAbiType).join(', ')})`; + } +} + +/** + * Pretty print a function signature + */ +function formatFunctionSignature(name: string, parameters: ABIParameter[]): string { + const params = parameters.map(p => `${p.name}: ${formatAbiType(p.type)}`).join(', '); + return `${name}(${params})`; +} + +/** + * Non-exhaustive pretty print of JS args to display in error messages in this module + */ +function formatArg(arg: unknown): string { + if (arg === undefined) { + return 'undefined'; + } + if (arg === null) { + return 'null'; + } + if (typeof arg === 'bigint') { + return `${arg}n`; + } + if (Array.isArray(arg)) { + return `[${arg.map(formatArg).join(', ')}]`; + } + if (typeof arg === 'object') { + const entries = Object.entries(arg).map(([k, v]) => `${k}: ${formatArg(v)}`); + return `{ ${entries.join(', ')} }`; + } + // eslint-disable-next-line @typescript-eslint/no-base-to-string + return String(arg); +} diff --git a/yarn-project/aztec.js/src/utils/abi_types.ts b/yarn-project/aztec.js/src/utils/abi_types.ts index af2b8bfc70df..9c39481d54f6 100644 --- a/yarn-project/aztec.js/src/utils/abi_types.ts +++ b/yarn-project/aztec.js/src/utils/abi_types.ts @@ -23,3 +23,10 @@ export type U128Like = bigint | number; /** Any type that can be converted into a struct with a single `inner` field. */ export type WrappedFieldLike = { /** Wrapped value */ inner: FieldLike } | FieldLike; + +/** Noir `Option` lowered ABI shape, plus ergonomic direct `T | null | undefined` inputs. */ +export type OptionLike = + | T + | null + | undefined + | { /** Whether the option is populated */ _is_some: boolean; /** Wrapped value */ _value: T }; diff --git a/yarn-project/builder/src/contract-interface-gen/typescript.ts b/yarn-project/builder/src/contract-interface-gen/typescript.ts index ff0ec242710d..a8a427ab0d44 100644 --- a/yarn-project/builder/src/contract-interface-gen/typescript.ts +++ b/yarn-project/builder/src/contract-interface-gen/typescript.ts @@ -11,6 +11,7 @@ import { isBoundedVecStruct, isEthAddressStruct, isFunctionSelectorStruct, + isOptionStruct, isPublicKeysStruct, isWrappedFieldStruct, } from '@aztec/stdlib/abi'; @@ -55,6 +56,9 @@ function abiTypeToTypescript(type: ABIParameter['type']): string { // as a BoundedVec in the ArgumentsEncoder. return `${abiTypeToTypescript(type.fields[0].type)}`; } + if (isOptionStruct(type)) { + return `OptionLike<${abiTypeToTypescript(type.fields[1].type)}>`; + } return `{ ${type.fields.map(f => `${f.name}: ${abiTypeToTypescript(f.type)}`).join(', ')} }`; default: throw new Error(`Unknown type ${type.kind}`); @@ -305,7 +309,7 @@ export async function generateTypescriptContractInterface(input: ContractArtifac /* eslint-disable */ import { AztecAddress, CompleteAddress } from '@aztec/aztec.js/addresses'; -import { type AbiType, type AztecAddressLike, type ContractArtifact, EventSelector, decodeFromAbi, type EthAddressLike, type FieldLike, type FunctionSelectorLike, loadContractArtifact, loadContractArtifactForPublic, type NoirCompiledContract, type U128Like, type WrappedFieldLike } from '@aztec/aztec.js/abi'; +import { type AbiType, type AztecAddressLike, type ContractArtifact, EventSelector, decodeFromAbi, type EthAddressLike, type FieldLike, type FunctionSelectorLike, loadContractArtifact, loadContractArtifactForPublic, type NoirCompiledContract, type OptionLike, type U128Like, type WrappedFieldLike } from '@aztec/aztec.js/abi'; import { Contract, ContractBase, ContractFunctionInteraction, type ContractMethod, type ContractStorageLayout, DeployMethod } from '@aztec/aztec.js/contracts'; import { EthAddress } from '@aztec/aztec.js/addresses'; import { Fr, Point } from '@aztec/aztec.js/fields'; diff --git a/yarn-project/end-to-end/src/e2e_option_params.test.ts b/yarn-project/end-to-end/src/e2e_option_params.test.ts new file mode 100644 index 000000000000..99c373092636 --- /dev/null +++ b/yarn-project/end-to-end/src/e2e_option_params.test.ts @@ -0,0 +1,92 @@ +import { AztecAddress } from '@aztec/aztec.js/addresses'; +import type { Wallet } from '@aztec/aztec.js/wallet'; +import { MAX_FIELD_VALUE } from '@aztec/constants'; +import { OptionParamContract } from '@aztec/noir-test-contracts.js/OptionParam'; + +import { jest } from '@jest/globals'; + +import { setup } from './fixtures/utils.js'; + +const TIMEOUT = 120_000; + +const U64_MAX = 2n ** 64n - 1n; +const I64_MIN = -(2n ** 63n); + +describe('Option params', () => { + let contract: OptionParamContract; + let wallet: Wallet; + let defaultAccountAddress: AztecAddress; + let teardown: () => Promise; + + const someValue = { + w: MAX_FIELD_VALUE, + x: true, + y: U64_MAX, + z: I64_MIN, + }; + + jest.setTimeout(TIMEOUT); + + beforeAll(async () => { + ({ + teardown, + wallet, + accounts: [defaultAccountAddress], + } = await setup(1)); + + contract = (await OptionParamContract.deploy(wallet).send({ from: defaultAccountAddress })).contract; + }); + + afterAll(() => teardown()); + + it('accepts ergonomic Option params for public functions', async () => { + const { result } = await contract.methods + .return_public_optional_struct(undefined) + .simulate({ from: defaultAccountAddress }); + expect(result).toBeUndefined(); + + const { result: nullResult } = await contract.methods + .return_public_optional_struct(null) + .simulate({ from: defaultAccountAddress }); + expect(nullResult).toBeUndefined(); + + const { result: someResult } = await contract.methods + .return_public_optional_struct(someValue) + .simulate({ from: defaultAccountAddress }); + expect(someResult).toEqual(someValue); + }); + + it('accepts ergonomic Option params for utility functions', async () => { + const { result: undefinedResult } = await contract.methods + .return_utility_optional_struct(undefined) + .simulate({ from: defaultAccountAddress }); + expect(undefinedResult).toBeUndefined(); + + const { result: nullResult } = await contract.methods + .return_utility_optional_struct(null) + .simulate({ from: defaultAccountAddress }); + expect(nullResult).toBeUndefined(); + + const { result: someResult } = await contract.methods + .return_utility_optional_struct(someValue) + .simulate({ from: defaultAccountAddress }); + expect(someResult).toEqual(someValue); + }); + + it('accepts ergonomic Option params for private functions', async () => { + const { result: undefinedResult } = await contract.methods + .return_private_optional_struct(undefined) + .simulate({ from: defaultAccountAddress }); + expect(undefinedResult).toBeUndefined(); + + const { result: nullResult } = await contract.methods + .return_private_optional_struct(null) + .simulate({ from: defaultAccountAddress }); + expect(nullResult).toBeUndefined(); + + const { result: someResult } = await contract.methods + .return_private_optional_struct(someValue) + .simulate({ from: defaultAccountAddress }); + expect(someResult).toEqual(someValue); + }); +}); 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/stdlib/src/abi/decoder.test.ts b/yarn-project/stdlib/src/abi/decoder.test.ts index 70183ba2654b..03bd867cd98f 100644 --- a/yarn-project/stdlib/src/abi/decoder.test.ts +++ b/yarn-project/stdlib/src/abi/decoder.test.ts @@ -269,4 +269,60 @@ describe('decoder', () => { { x: 1n, y: 2n, is_infinite: false }, ]); }); + + it('decodes Option::Some as the wrapped value', () => { + const decoded = decodeFromAbi( + [ + { + kind: 'struct', + path: 'std::option::Option', + fields: [ + { name: '_is_some', type: { kind: 'boolean' } }, + { + name: '_value', + type: { + kind: 'struct', + path: 'Test::CustomStruct', + fields: [ + { name: 'w', type: { kind: 'field' } }, + { name: 'x', type: { kind: 'boolean' } }, + ], + }, + }, + ], + }, + ], + [new Fr(1n), new Fr(7n), new Fr(1n)], + ); + + expect(decoded).toEqual({ w: 7n, x: true }); + }); + + it('decodes Option::None as undefined', () => { + const decoded = decodeFromAbi( + [ + { + kind: 'struct', + path: 'std::option::Option', + fields: [ + { name: '_is_some', type: { kind: 'boolean' } }, + { + name: '_value', + type: { + kind: 'struct', + path: 'Test::CustomStruct', + fields: [ + { name: 'w', type: { kind: 'field' } }, + { name: 'x', type: { kind: 'boolean' } }, + ], + }, + }, + ], + }, + ], + [Fr.ZERO, new Fr(7n), new Fr(1n)], + ); + + expect(decoded).toBeUndefined(); + }); }); diff --git a/yarn-project/stdlib/src/abi/decoder.ts b/yarn-project/stdlib/src/abi/decoder.ts index 69f1ac106d81..a373a8ac05e1 100644 --- a/yarn-project/stdlib/src/abi/decoder.ts +++ b/yarn-project/stdlib/src/abi/decoder.ts @@ -2,12 +2,19 @@ import { Fr } from '@aztec/foundation/curves/bn254'; import { AztecAddress } from '../aztec-address/index.js'; import type { ABIParameter, ABIVariable, AbiType } from './abi.js'; -import { isAztecAddressStruct, parseSignedInt } from './utils.js'; +import { isAztecAddressStruct, isOptionStruct, parseSignedInt } from './utils.js'; /** * The type of our decoded ABI. */ -export type AbiDecoded = bigint | boolean | string | AztecAddress | AbiDecoded[] | { [key: string]: AbiDecoded }; +export type AbiDecoded = + | bigint + | boolean + | string + | AztecAddress + | AbiDecoded[] + | { [key: string]: AbiDecoded } + | undefined; /** * Decodes values using a provided ABI. @@ -51,6 +58,11 @@ class AbiDecoder { if (isAztecAddressStruct(abiType)) { return new AztecAddress(this.getNextField().toBuffer()); } + if (isOptionStruct(abiType)) { + const isSome = this.decodeNext(abiType.fields[0].type); + const value = this.decodeNext(abiType.fields[1].type); + return isSome ? value : undefined; + } for (const field of abiType.fields) { struct[field.name] = this.decodeNext(field.type); diff --git a/yarn-project/stdlib/src/abi/encoder.test.ts b/yarn-project/stdlib/src/abi/encoder.test.ts index c26e9ae78045..0bf1c64ab57e 100644 --- a/yarn-project/stdlib/src/abi/encoder.test.ts +++ b/yarn-project/stdlib/src/abi/encoder.test.ts @@ -160,6 +160,82 @@ describe('abi/encoder', () => { expect(encodeArguments(abi, [{ inner: value }])).toEqual([value]); }); + describe('option struct inputs', () => { + const optionStructAbi: FunctionAbi = { + name: 'test', + isInitializer: false, + functionType: FunctionType.PRIVATE, + isOnlySelf: false, + isStatic: false, + parameters: [ + { + name: 'value', + type: { + kind: 'struct', + path: 'std::option::Option', + fields: [ + { name: '_is_some', type: { kind: 'boolean' } }, + { + name: '_value', + type: { + kind: 'struct', + path: 'Test::CustomStruct', + fields: [ + { name: 'w', type: { kind: 'field' } }, + { name: 'x', type: { kind: 'boolean' } }, + { name: 'y', type: { kind: 'integer', sign: 'unsigned', width: 64 } }, + { name: 'z', type: { kind: 'integer', sign: 'signed', width: 64 } }, + ], + }, + }, + ], + }, + visibility: 'private', + }, + ], + returnTypes: [], + errorTypes: {}, + }; + + const someValue = { w: 1n, x: true, y: 2n, z: -3n }; + + it('encodes a direct value as Some', () => { + expect(encodeArguments(optionStructAbi, [someValue])).toEqual([ + new Fr(1n), + new Fr(1n), + new Fr(1n), + new Fr(2n), + new Fr((1n << 64n) - 3n), + ]); + }); + + it.each([undefined, null])('encodes %p as None', value => { + expect(encodeArguments(optionStructAbi, [value])).toEqual([Fr.ZERO, Fr.ZERO, Fr.ZERO, Fr.ZERO, Fr.ZERO]); + }); + + it('encodes the lowered ABI shape for Some', () => { + // eslint-disable-next-line camelcase + expect(encodeArguments(optionStructAbi, [{ _is_some: true, _value: someValue }])).toEqual([ + new Fr(1n), + new Fr(1n), + new Fr(1n), + new Fr(2n), + new Fr((1n << 64n) - 3n), + ]); + }); + + it('encodes the lowered ABI shape for None without requiring _value', () => { + // eslint-disable-next-line camelcase + expect(encodeArguments(optionStructAbi, [{ _is_some: false }])).toEqual([ + Fr.ZERO, + Fr.ZERO, + Fr.ZERO, + Fr.ZERO, + Fr.ZERO, + ]); + }); + }); + it('throws when passing string argument as field', () => { const testFunctionAbi: FunctionAbi = { name: 'constructor', diff --git a/yarn-project/stdlib/src/abi/encoder.ts b/yarn-project/stdlib/src/abi/encoder.ts index c4748c8e41f8..ecd0c127819b 100644 --- a/yarn-project/stdlib/src/abi/encoder.ts +++ b/yarn-project/stdlib/src/abi/encoder.ts @@ -1,7 +1,13 @@ import { Fr } from '@aztec/foundation/curves/bn254'; import type { AbiType, FunctionAbi } from './abi.js'; -import { isAddressStruct, isBoundedVecStruct, isFunctionSelectorStruct, isWrappedFieldStruct } from './utils.js'; +import { + isAddressStruct, + isBoundedVecStruct, + isFunctionSelectorStruct, + isOptionStruct, + isWrappedFieldStruct, +} from './utils.js'; /** * Encodes arguments for a function call. @@ -43,6 +49,32 @@ class ArgumentEncoder { * @param name - Name. */ private encodeArgument(abiType: AbiType, arg: any, name?: string) { + if (isOptionStruct(abiType)) { + const optionType = abiType as Extract; + const [isSomeField, valueField] = optionType.fields; + + if (arg === undefined || arg === null) { + this.encodeArgument(isSomeField.type, false, `${name}._is_some`); + this.#encodeDefaultValue(valueField.type); + return; + } + + if (typeof arg === 'object' && '_is_some' in arg) { + this.encodeArgument(isSomeField.type, arg._is_some, `${name}._is_some`); + + if (arg._is_some) { + this.encodeArgument(valueField.type, arg._value, `${name}._value`); + } else { + this.#encodeDefaultValue(valueField.type); + } + return; + } + + this.encodeArgument(isSomeField.type, true, `${name}._is_some`); + this.encodeArgument(valueField.type, arg, `${name}._value`); + return; + } + if (arg === undefined || arg == null) { throw new Error(`Undefined argument ${name ?? 'unnamed'} of type ${abiType.kind}`); } @@ -227,6 +259,14 @@ class ArgumentEncoder { this.encodeArgument(lenField.type, arg.length, 'len'); } } + + /** + * Appends the flattened zero value for an ABI type. + * Option::None still serializes the wrapped value, so we need to zero-fill its footprint. + */ + #encodeDefaultValue(abiType: AbiType) { + this.flattened.push(...new Array(ArgumentEncoder.typeSize(abiType)).fill(Fr.ZERO)); + } } /** diff --git a/yarn-project/stdlib/src/abi/utils.ts b/yarn-project/stdlib/src/abi/utils.ts index e9bc23565c0c..a1dea3316468 100644 --- a/yarn-project/stdlib/src/abi/utils.ts +++ b/yarn-project/stdlib/src/abi/utils.ts @@ -81,6 +81,31 @@ export function isBoundedVecStruct(abiType: AbiType) { ); } +/** + * Returns whether the ABI type is Noir's std::option::Option lowered to a struct. + * @param abiType - Type to check. + * @returns A boolean indicating whether the ABI type is an Option struct. + */ +export function isOptionStruct(abiType: AbiType) { + return ( + abiType.kind === 'struct' && + abiType.path === 'std::option::Option' && + abiType.fields.length === 2 && + abiType.fields[0].name === '_is_some' && + abiType.fields[1].name === '_value' + ); +} + +/** + * Returns whether `null` or `undefined` can be mapped to a valid ABI value for this type. + * + * @param abiType - Type to check. + * @returns A boolean indicating whether nullish values are valid shorthand for this ABI type. + */ +export function canBeMappedFromNullOrUndefined(abiType: AbiType) { + return isOptionStruct(abiType); +} + /** * Returns a bigint by parsing a serialized 2's complement signed int. * @param b - The signed int as a buffer 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);