From 04121b6ab60c4847c25c3fdddf627f026647b410 Mon Sep 17 00:00:00 2001 From: benesjan Date: Thu, 5 Sep 2024 08:13:44 +0000 Subject: [PATCH 1/8] feat: addressing Nico's router comments --- noir-projects/aztec-nr/aztec/src/note/note_getter/mod.nr | 2 +- .../aztec/src/utils/{collapse.nr => collapse_array.nr} | 0 noir-projects/aztec-nr/aztec/src/utils/mod.nr | 4 +++- noir-projects/aztec-nr/aztec/src/utils/test.nr | 2 +- 4 files changed, 5 insertions(+), 3 deletions(-) rename noir-projects/aztec-nr/aztec/src/utils/{collapse.nr => collapse_array.nr} (100%) diff --git a/noir-projects/aztec-nr/aztec/src/note/note_getter/mod.nr b/noir-projects/aztec-nr/aztec/src/note/note_getter/mod.nr index 7638072fac22..4fa9276f1b64 100644 --- a/noir-projects/aztec-nr/aztec/src/note/note_getter/mod.nr +++ b/noir-projects/aztec-nr/aztec/src/note/note_getter/mod.nr @@ -125,7 +125,7 @@ fn constrain_get_notes_internal = BoundedVec::new(); // We have now collapsed the sparse array of Options into a BoundedVec. This is a more ergonomic type and also diff --git a/noir-projects/aztec-nr/aztec/src/utils/collapse.nr b/noir-projects/aztec-nr/aztec/src/utils/collapse_array.nr similarity index 100% rename from noir-projects/aztec-nr/aztec/src/utils/collapse.nr rename to noir-projects/aztec-nr/aztec/src/utils/collapse_array.nr diff --git a/noir-projects/aztec-nr/aztec/src/utils/mod.nr b/noir-projects/aztec-nr/aztec/src/utils/mod.nr index e670a6403bfe..6412e14c7f7e 100644 --- a/noir-projects/aztec-nr/aztec/src/utils/mod.nr +++ b/noir-projects/aztec-nr/aztec/src/utils/mod.nr @@ -1,4 +1,6 @@ -mod collapse; +mod collapse_array; mod comparison; mod point; mod test; + +use crate::utils::collapse_array::{collapse_array, verify_collapse_hints}; diff --git a/noir-projects/aztec-nr/aztec/src/utils/test.nr b/noir-projects/aztec-nr/aztec/src/utils/test.nr index 85b827c792d7..4eadb4e2123e 100644 --- a/noir-projects/aztec-nr/aztec/src/utils/test.nr +++ b/noir-projects/aztec-nr/aztec/src/utils/test.nr @@ -1,4 +1,4 @@ -use super::collapse::{collapse_array, verify_collapse_hints}; +use super::collapse_array::{collapse_array, verify_collapse_hints}; #[test] fn collapse_empty_array() { From fdb15aa76a844090c7ef9559af7b642d2abd01a1 Mon Sep 17 00:00:00 2001 From: benesjan Date: Thu, 5 Sep 2024 08:23:23 +0000 Subject: [PATCH 2/8] WIP --- docs/docs/aztec/glossary/call_types.md | 7 +++---- .../codealong/contract_tutorials/crowdfunding_contract.md | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/docs/docs/aztec/glossary/call_types.md b/docs/docs/aztec/glossary/call_types.md index 48303f44b81c..34ab2be456ed 100644 --- a/docs/docs/aztec/glossary/call_types.md +++ b/docs/docs/aztec/glossary/call_types.md @@ -118,9 +118,8 @@ A common pattern is to enqueue public calls to check some validity condition on #include_code enqueueing /noir-projects/noir-contracts/contracts/router_contract/src/main.nr rust -Note that this reveals what public function is being called on what contract. -For this reason we've created a canonical router contract which implements some of the checks commonly performed. -This conceals what contract performed the public call as the `context.msg_sender()` in the public function is the router itself (since the router's private function enqueued the public call). +Note that this reveals what public function is being called on what contract, and perhaps more importantly which contract enqueued the call during private execution. +For this reason we've created a canonical router contract which implements some of the checks commonly performed: this conceals the calling contract, as the `context.msg_sender()` in the public function will be the router itself (since it is the router that enqueues the public call). An example of how a deadline can be checked using the router contract follows: @@ -131,7 +130,7 @@ This is what the implementation of the check timestamp functionality looks like: #include_code check_timestamp /noir-projects/noir-contracts/contracts/router_contract/src/main.nr rust Even with the router contract achieving good privacy is hard. -This is especially the case when the value being checked is unique and stored in the contract's public storage. +For example, if the value being checked against is unique and stored in the contract's public storage, it's then simple to find private transactions that are using that value in the enqueued public reads, and therefore link them to this contract. For this reason it is encouraged to try to avoid public function calls and instead privately read [Shared State](../../reference/developer_references/smart_contract_reference/storage/shared_state.md) when possible. ### Public Execution diff --git a/docs/docs/tutorials/codealong/contract_tutorials/crowdfunding_contract.md b/docs/docs/tutorials/codealong/contract_tutorials/crowdfunding_contract.md index e198b68d4161..5cfb2dba062e 100644 --- a/docs/docs/tutorials/codealong/contract_tutorials/crowdfunding_contract.md +++ b/docs/docs/tutorials/codealong/contract_tutorials/crowdfunding_contract.md @@ -135,9 +135,9 @@ We read the deadline from public storage in private and use the router contract #include_code call-check-deadline /noir-projects/noir-contracts/contracts/crowdfunding_contract/src/main.nr rust -We do the check via the router contract to conceal which contract is performing the check (This is achieved by calling a private function on the router contract which then enqueues a call to a public function on the router contract. This then results in the msg_sender in the public call being the router contract.) +We perform this check via the router contract to not reveal which contract is performing the check - this is achieved by calling a private function on the router contract which then enqueues a call to a public function on the router contract. The result is that `msg_sender` in the public call will then be the router contract. Note that the privacy here is dependent upon what deadline value is chosen by the Crowdfunding contract deployer. -If it's unique to this contract, then we are leaking a privacy. +If it's unique to this contract, then there'll be a privacy leak regardless, as third parties will be able to observe a deadline check against the Crowdfunding deadline, and therefore infer that the associated transaction is interacting with it. Now conclude adding all dependencies to the `Crowdfunding` contract: From 75a39f203dbd9d29a59271dc4bff8878adb48e02 Mon Sep 17 00:00:00 2001 From: benesjan Date: Thu, 5 Sep 2024 08:52:07 +0000 Subject: [PATCH 3/8] assert_comparison --> compare --- .../aztec/src/note/note_getter/mod.nr | 9 +- .../aztec/src/note/note_getter_options.nr | 2 +- .../aztec-nr/aztec/src/utils/comparison.nr | 114 ++++++------------ .../contracts/router_contract/src/main.nr | 6 +- 4 files changed, 47 insertions(+), 84 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/note/note_getter/mod.nr b/noir-projects/aztec-nr/aztec/src/note/note_getter/mod.nr index 4fa9276f1b64..235619416713 100644 --- a/noir-projects/aztec-nr/aztec/src/note/note_getter/mod.nr +++ b/noir-projects/aztec-nr/aztec/src/note/note_getter/mod.nr @@ -7,7 +7,7 @@ use crate::note::{ utils::compute_note_hash_for_read_request }; use crate::oracle; -use crate::utils::comparison::assert_comparison; +use crate::utils::comparison::compare; mod test; @@ -51,11 +51,8 @@ fn check_note_fields( let select = selects.get_unchecked(i).unwrap_unchecked(); let value_field = extract_property_value_from_selector(serialized_note, select.property_selector); - assert_comparison( - value_field, - select.comparator, - select.value.to_field(), - "Mismatch return note field." + assert( + compare(value_field, select.comparator, select.value.to_field()), "Mismatch return note field." ); } } diff --git a/noir-projects/aztec-nr/aztec/src/note/note_getter_options.nr b/noir-projects/aztec-nr/aztec/src/note/note_getter_options.nr index d4317f9edf00..2414ca7b10d3 100644 --- a/noir-projects/aztec-nr/aztec/src/note/note_getter_options.nr +++ b/noir-projects/aztec-nr/aztec/src/note/note_getter_options.nr @@ -86,7 +86,7 @@ impl NoteGetterOpt // This method adds a `Select` criterion to the options. // It takes a property_selector indicating which field to select, // a value representing the specific value to match in that field, and - // a comparator (For possible values of comparators, please see the Comparator enum above) + // a comparator (For possible values of comparators, please see the Comparator enum from `utils::comparison`) pub fn select( &mut self, property_selector: PropertySelector, diff --git a/noir-projects/aztec-nr/aztec/src/utils/comparison.nr b/noir-projects/aztec-nr/aztec/src/utils/comparison.nr index e5a91883775b..2621bfd58750 100644 --- a/noir-projects/aztec-nr/aztec/src/utils/comparison.nr +++ b/noir-projects/aztec-nr/aztec/src/utils/comparison.nr @@ -16,137 +16,103 @@ global Comparator = ComparatorEnum { GTE: 6, }; -pub fn assert_comparison(lhs: Field, operation: u8, rhs: Field, error_msg: str) { +pub fn compare(lhs: Field, operation: u8, rhs: Field) -> bool { // Values are computed ahead of time because circuits evaluate all branches let is_equal = lhs == rhs; let is_lt = lhs.lt(rhs); if (operation == Comparator.EQ) { - assert(is_equal, error_msg); + is_equal } else if (operation == Comparator.NEQ) { - assert(!is_equal, error_msg); + !is_equal } else if (operation == Comparator.LT) { - assert(is_lt, error_msg); + is_lt } else if (operation == Comparator.LTE) { - assert(is_lt | is_equal, error_msg); + is_lt | is_equal } else if (operation == Comparator.GT) { - assert(!is_lt & !is_equal, error_msg); + !is_lt & !is_equal } else if (operation == Comparator.GTE) { - assert(!is_lt, error_msg); + !is_lt + } else { + assert(false, "Invalid operation"); + false // Noir would complain without boolean value here } } mod test { - use super::assert_comparison; + use super::compare; use super::Comparator; #[test] - fn test_assert_comparison_happy_path() { + fn test_compare() { let lhs = 10; let rhs = 10; - assert_comparison(lhs, Comparator.EQ, rhs, "Expected lhs to be equal to rhs"); + let result = compare(lhs, Comparator.EQ, rhs); + assert(result, "Expected lhs to be equal to rhs"); let lhs = 10; let rhs = 11; - assert_comparison(lhs, Comparator.NEQ, rhs, "Expected lhs to be not equal to rhs"); + let result = compare(lhs, Comparator.NEQ, rhs); + assert(result, "Expected lhs to be not equal to rhs"); let lhs = 10; let rhs = 11; - assert_comparison(lhs, Comparator.LT, rhs, "Expected lhs to be less than rhs"); + let result = compare(lhs, Comparator.LT, rhs); + assert(result, "Expected lhs to be less than rhs"); let lhs = 10; let rhs = 10; - assert_comparison( - lhs, - Comparator.LTE, - rhs, - "Expected lhs to be less than or equal to rhs" - ); + let result = compare(lhs, Comparator.LTE, rhs); + assert(result, "Expected lhs to be less than or equal to rhs"); let lhs = 11; let rhs = 10; - assert_comparison(lhs, Comparator.GT, rhs, "Expected lhs to be greater than rhs"); + let result = compare(lhs, Comparator.GT, rhs); + assert(result, "Expected lhs to be greater than rhs"); let lhs = 10; let rhs = 10; - assert_comparison( - lhs, - Comparator.GTE, - rhs, - "Expected lhs to be greater than or equal to rhs" - ); + let result = compare(lhs, Comparator.GTE, rhs); + assert(result, "Expected lhs to be greater than or equal to rhs"); let lhs = 11; let rhs = 10; - assert_comparison( - lhs, - Comparator.GTE, - rhs, - "Expected lhs to be greater than or equal to rhs" - ); - } + let result = compare(lhs, Comparator.GTE, rhs); + assert(result, "Expected lhs to be greater than or equal to rhs"); - #[test(should_fail_with="Expected lhs to be equal to rhs")] - fn test_assert_comparison_fail_eq() { let lhs = 10; let rhs = 11; - assert_comparison(lhs, Comparator.EQ, rhs, "Expected lhs to be equal to rhs"); - } + let result = compare(lhs, Comparator.EQ, rhs); + assert(!result, "Expected lhs to be not equal to rhs"); - #[test(should_fail_with="Expected lhs to be not equal to rhs")] - fn test_assert_comparison_fail_neq() { let lhs = 10; let rhs = 10; - assert_comparison(lhs, Comparator.NEQ, rhs, "Expected lhs to be not equal to rhs"); - } + let result = compare(lhs, Comparator.NEQ, rhs); + assert(!result, "Expected lhs to not be not equal to rhs"); - #[test(should_fail_with="Expected lhs to be less than rhs")] - fn test_assert_comparison_fail_lt() { let lhs = 11; let rhs = 10; - assert_comparison(lhs, Comparator.LT, rhs, "Expected lhs to be less than rhs"); - } + let result = compare(lhs, Comparator.LT, rhs); + assert(!result, "Expected lhs to not be less than rhs"); - #[test(should_fail_with="Expected lhs to be less than or equal to rhs")] - fn test_assert_comparison_fail_lte() { let lhs = 11; let rhs = 10; - assert_comparison( - lhs, - Comparator.LTE, - rhs, - "Expected lhs to be less than or equal to rhs" - ); - } + let result = compare(lhs, Comparator.LTE, rhs); + assert(!result, "Expected lhs to not be less than or equal to rhs"); - #[test(should_fail_with="Expected lhs to be greater than rhs")] - fn test_assert_comparison_fail_gt() { let lhs = 10; let rhs = 10; - assert_comparison(lhs, Comparator.GT, rhs, "Expected lhs to be greater than rhs"); - } + let result = compare(lhs, Comparator.GT, rhs); + assert(!result, "Expected lhs to not be greater than rhs"); - #[test(should_fail_with="Expected lhs to be greater than or equal to rhs")] - fn test_assert_comparison_fail_gte() { let lhs = 10; let rhs = 11; - assert_comparison( - lhs, - Comparator.GTE, - rhs, - "Expected lhs to be greater than or equal to rhs" - ); - } + let result = compare(lhs, Comparator.GTE, rhs); + assert(!result, "Expected lhs to not be greater than or equal to rhs"); - #[test(should_fail_with="Expected lhs to be greater than or equal to rhs")] - fn test_assert_comparison_fail_gte_2() { let lhs = 10; let rhs = 11; - assert_comparison( - lhs, - Comparator.GTE, - rhs, - "Expected lhs to be greater than or equal to rhs" - ); + let result = compare(lhs, Comparator.GTE, rhs); + assert(!result, "Expected lhs to not be greater than or equal to rhs"); } } diff --git a/noir-projects/noir-contracts/contracts/router_contract/src/main.nr b/noir-projects/noir-contracts/contracts/router_contract/src/main.nr index 484a701b7bb8..8df522b20026 100644 --- a/noir-projects/noir-contracts/contracts/router_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/router_contract/src/main.nr @@ -4,7 +4,7 @@ mod test; /// call. This is achieved by having a private function on this contract that enques the public call and hence /// the `msg_sender` in the public call is the address of this contract. contract Router { - use aztec::utils::comparison::assert_comparison; + use aztec::utils::comparison::compare; // docs:start:check_timestamp /// Enqueues a public call that asserts that `lhs` (left side of the comparison) timestamp satisfies @@ -20,7 +20,7 @@ contract Router { fn _check_timestamp(lhs: u64, operation: u8) { let lhs_field = lhs as Field; let rhs = context.timestamp() as Field; - assert_comparison(lhs_field, operation, rhs, "Timestamp mismatch."); + assert(compare(lhs_field, operation, rhs), "Timestamp mismatch."); } // docs:end:check_timestamp @@ -38,6 +38,6 @@ contract Router { #[aztec(view)] fn _check_block_number(lhs: Field, operation: u8) { let rhs = context.block_number(); - assert_comparison(lhs, operation, rhs, "Block number mismatch."); + assert(compare(lhs, operation, rhs), "Block number mismatch."); } } From 59b7607d0a0a40a2fe11143d3f1fb3306db221f2 Mon Sep 17 00:00:00 2001 From: benesjan Date: Thu, 5 Sep 2024 09:26:23 +0000 Subject: [PATCH 4/8] Better code clarity --- .../src/core/libraries/ConstantsGen.sol | 2 +- .../app_subscription_contract/src/main.nr | 8 ++--- .../crowdfunding_contract/src/main.nr | 2 +- .../contracts/router_contract/src/main.nr | 29 +++++++++---------- .../contracts/router_contract/src/test.nr | 4 +-- .../crates/types/src/constants.nr | 2 +- yarn-project/circuits.js/src/constants.gen.ts | 2 +- 7 files changed, 24 insertions(+), 25 deletions(-) diff --git a/l1-contracts/src/core/libraries/ConstantsGen.sol b/l1-contracts/src/core/libraries/ConstantsGen.sol index 49f48e82f062..c8bcd862df50 100644 --- a/l1-contracts/src/core/libraries/ConstantsGen.sol +++ b/l1-contracts/src/core/libraries/ConstantsGen.sol @@ -144,7 +144,7 @@ library Constants { uint256 internal constant FEE_JUICE_ADDRESS = 10248142274714515101077825679585135641434041564851038865006795089686437446849; uint256 internal constant ROUTER_ADDRESS = - 8135649085127523915405560812661632604783066942985338123941332115593181690668; + 7268799613082469933251235702514160327341161584122631177360064643484764773587; uint256 internal constant AZTEC_ADDRESS_LENGTH = 1; uint256 internal constant GAS_FEES_LENGTH = 2; uint256 internal constant GAS_LENGTH = 2; diff --git a/noir-projects/noir-contracts/contracts/app_subscription_contract/src/main.nr b/noir-projects/noir-contracts/contracts/app_subscription_contract/src/main.nr index 73fd1f257bb8..81da7a9aad7b 100644 --- a/noir-projects/noir-contracts/contracts/app_subscription_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/app_subscription_contract/src/main.nr @@ -54,7 +54,7 @@ contract AppSubscription { // We check that the note is not expired. We do that via the router contract to conceal which contract // is performing the check. - Router::at(ROUTER_ADDRESS).check_block_number(note.expiry_block_number, Comparator.GT).call(&mut context); + Router::at(ROUTER_ADDRESS).check_block_number(Comparator.LT, note.expiry_block_number).call(&mut context); payload.execute_calls(&mut context, storage.target_address.read_private()); } @@ -86,11 +86,11 @@ contract AppSubscription { nonce ).call(&mut context); - // Assert that the `expiry_block_number - SUBSCRIPTION_DURATION_IN_BLOCKS < current_block_number`. + // Assert that the `current_block_number > expiry_block_number - SUBSCRIPTION_DURATION_IN_BLOCKS`. // --> We do that via the router contract to conceal which contract is performing the check. Router::at(ROUTER_ADDRESS).check_block_number( - expiry_block_number - SUBSCRIPTION_DURATION_IN_BLOCKS, - Comparator.LT + Comparator.GT, + expiry_block_number - SUBSCRIPTION_DURATION_IN_BLOCKS ).call(&mut context); let subscriber_keys = get_current_public_keys(&mut context, subscriber); diff --git a/noir-projects/noir-contracts/contracts/crowdfunding_contract/src/main.nr b/noir-projects/noir-contracts/contracts/crowdfunding_contract/src/main.nr index 2d5a06b9e8b1..6b98a8ac6419 100644 --- a/noir-projects/noir-contracts/contracts/crowdfunding_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/crowdfunding_contract/src/main.nr @@ -60,7 +60,7 @@ contract Crowdfunding { // is performing the check. // docs:start:call-check-deadline let deadline = storage.deadline.read_private(); - Router::at(ROUTER_ADDRESS).check_timestamp(deadline, Comparator.GT).call(&mut context); + Router::at(ROUTER_ADDRESS).check_timestamp(Comparator.LT, deadline).call(&mut context); // docs:end:call-check-deadline // docs:start:do-transfer diff --git a/noir-projects/noir-contracts/contracts/router_contract/src/main.nr b/noir-projects/noir-contracts/contracts/router_contract/src/main.nr index 8df522b20026..da84a82e3c06 100644 --- a/noir-projects/noir-contracts/contracts/router_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/router_contract/src/main.nr @@ -7,37 +7,36 @@ contract Router { use aztec::utils::comparison::compare; // docs:start:check_timestamp - /// Enqueues a public call that asserts that `lhs` (left side of the comparison) timestamp satisfies - /// the `operation` with respect to the current timestamp. + /// Asserts that the current timestamp in the enqueued public call satisfies the `operation` with respect + /// to the `value. #[aztec(private)] - fn check_timestamp(lhs: u64, operation: u8) { - Router::at(context.this_address())._check_timestamp(lhs, operation).enqueue_view(&mut context); + fn check_timestamp(operation: u8, value: u64) { + Router::at(context.this_address())._check_timestamp(operation, value).enqueue_view(&mut context); } #[aztec(public)] #[aztec(internal)] #[aztec(view)] - fn _check_timestamp(lhs: u64, operation: u8) { - let lhs_field = lhs as Field; - let rhs = context.timestamp() as Field; - assert(compare(lhs_field, operation, rhs), "Timestamp mismatch."); + fn _check_timestamp(operation: u8, value: u64) { + let lhs_field = context.timestamp() as Field; + let rhs_field = value as Field; + assert(compare(lhs_field, operation, rhs_field), "Timestamp mismatch."); } // docs:end:check_timestamp - /// Enqueues a public call that asserts that `lhs` (left side of the comparison) block number satisfies - /// the `operation` with respect to the current block number. + /// Asserts that the current block number in the enqueued public call satisfies the `operation` with respect + /// to the `value. #[aztec(private)] - fn check_block_number(lhs: Field, operation: u8) { + fn check_block_number(operation: u8, value: Field) { // docs:start:enqueueing - Router::at(context.this_address())._check_block_number(lhs, operation).enqueue_view(&mut context); + Router::at(context.this_address())._check_block_number(operation, value).enqueue_view(&mut context); // docs:end:enqueueing } #[aztec(public)] #[aztec(internal)] #[aztec(view)] - fn _check_block_number(lhs: Field, operation: u8) { - let rhs = context.block_number(); - assert(compare(lhs, operation, rhs), "Block number mismatch."); + fn _check_block_number(operation: u8, value: Field) { + assert(compare(context.block_number(), operation, value), "Block number mismatch."); } } diff --git a/noir-projects/noir-contracts/contracts/router_contract/src/test.nr b/noir-projects/noir-contracts/contracts/router_contract/src/test.nr index 992d719425ef..91bc74c543d8 100644 --- a/noir-projects/noir-contracts/contracts/router_contract/src/test.nr +++ b/noir-projects/noir-contracts/contracts/router_contract/src/test.nr @@ -17,10 +17,10 @@ unconstrained fn test_check_block_number() { assert(current_block_number == 10, "Expected block number to be 10"); // We test just one success case and 1 failure case in this test as the rest is tested in the comparator unit tests - let call_1 = router.check_block_number(11, Comparator.GT); + let call_1 = router.check_block_number(Comparator.LT, 11); env.call_private_void(call_1); - let call_2 = router.check_block_number(5, Comparator.GT); + let call_2 = router.check_block_number(Comparator.LT, 5); env.assert_private_call_fails(call_2); } diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr b/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr index 3077964c3891..9db6d4271f7f 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr @@ -196,7 +196,7 @@ global CANONICAL_AUTH_REGISTRY_ADDRESS = AztecAddress::from_field(0x24877c50868f global DEPLOYER_CONTRACT_ADDRESS = AztecAddress::from_field(0x2ab1a2bd6d07d8d61ea56d85861446349e52c6b7c0612b702cb1e6db6ad0b089); global REGISTERER_CONTRACT_ADDRESS = AztecAddress::from_field(0x05d15342d76e46e5be07d3cda0d753158431cdc5e39d29ce4e8fe1f5c070564a); global FEE_JUICE_ADDRESS = AztecAddress::from_field(0x16a83e3395bc921a2441db55dce24f0e0932636901a2e676fa68b9b2b9a644c1); -global ROUTER_ADDRESS = AztecAddress::from_field(0x11fc9d3c438ea027f3d52cb7cf844fa4bb197520205c7366b8887a624f6a7b2c); +global ROUTER_ADDRESS = AztecAddress::from_field(0x1011feaa54609098a884322267ec754c637b280c15aa79c3be9f1394e2b29cd3); // LENGTH OF STRUCTS SERIALIZED TO FIELDS global AZTEC_ADDRESS_LENGTH = 1; diff --git a/yarn-project/circuits.js/src/constants.gen.ts b/yarn-project/circuits.js/src/constants.gen.ts index df0e0d0f9a1e..74916b3e08f7 100644 --- a/yarn-project/circuits.js/src/constants.gen.ts +++ b/yarn-project/circuits.js/src/constants.gen.ts @@ -126,7 +126,7 @@ export const DEPLOYER_CONTRACT_ADDRESS = 193109947607833303683371634801986023939 export const REGISTERER_CONTRACT_ADDRESS = 2631409926445785927331173506476539962589925110142857699603561302478860342858n; export const FEE_JUICE_ADDRESS = 10248142274714515101077825679585135641434041564851038865006795089686437446849n; -export const ROUTER_ADDRESS = 8135649085127523915405560812661632604783066942985338123941332115593181690668n; +export const ROUTER_ADDRESS = 7268799613082469933251235702514160327341161584122631177360064643484764773587n; export const AZTEC_ADDRESS_LENGTH = 1; export const GAS_FEES_LENGTH = 2; export const GAS_LENGTH = 2; From 4b491f6759ec2c60409f92e654bfb468651423ee Mon Sep 17 00:00:00 2001 From: benesjan Date: Thu, 5 Sep 2024 09:48:55 +0000 Subject: [PATCH 5/8] helper lib --- cspell.json | 2 ++ docs/docs/aztec/glossary/call_types.md | 10 +++++++--- .../app_subscription_contract/src/main.nr | 15 ++++++++------- .../contracts/crowdfunding_contract/src/main.nr | 9 ++++----- .../contracts/router_contract/src/main.nr | 3 +-- .../contracts/router_contract/src/utils.nr | 15 +++++++++++++++ 6 files changed, 37 insertions(+), 17 deletions(-) create mode 100644 noir-projects/noir-contracts/contracts/router_contract/src/utils.nr diff --git a/cspell.json b/cspell.json index dd51854aaf15..b6110189f084 100644 --- a/cspell.json +++ b/cspell.json @@ -217,6 +217,7 @@ "REALDIR", "realpath", "rebundle", + "reentrancy", "repr", "Reserialize", "retag", @@ -278,6 +279,7 @@ "Validium", "vals", "viem", + "Vyper", "wasms", "webassembly", "WITGEN", diff --git a/docs/docs/aztec/glossary/call_types.md b/docs/docs/aztec/glossary/call_types.md index 34ab2be456ed..db30eed8d034 100644 --- a/docs/docs/aztec/glossary/call_types.md +++ b/docs/docs/aztec/glossary/call_types.md @@ -116,7 +116,7 @@ It is also possible to create public functions that can _only_ be invoked by pri A common pattern is to enqueue public calls to check some validity condition on public state, e.g. that a deadline has not expired or that some public value is set. -#include_code enqueueing /noir-projects/noir-contracts/contracts/router_contract/src/main.nr rust +#include_code enqueueing /noir-projects/noir-contracts/contracts/router_contract/src/utils.nr rust Note that this reveals what public function is being called on what contract, and perhaps more importantly which contract enqueued the call during private execution. For this reason we've created a canonical router contract which implements some of the checks commonly performed: this conceals the calling contract, as the `context.msg_sender()` in the public function will be the router itself (since it is the router that enqueues the public call). @@ -125,6 +125,10 @@ An example of how a deadline can be checked using the router contract follows: #include_code call-check-deadline /noir-projects/noir-contracts/contracts/crowdfunding_contract/src/main.nr rust +`privately_check_timestamp` and `privately_check_block_number` are helper functions around the call to the router contract: + +#include_code helper_router_functions /noir-projects/noir-contracts/contracts/router_contract/src/utils.nr rust + This is what the implementation of the check timestamp functionality looks like: #include_code check_timestamp /noir-projects/noir-contracts/contracts/router_contract/src/main.nr rust @@ -173,11 +177,11 @@ No correctness is guaranteed on the result of `simulate`! Correct execution is e #### `prove` -This creates and returns a transaction request, which includes proof of correct private execution and side-efects. The request is not broadcast however, and no gas is spent. It is typically used in testing contexts to inspect transaction parameters or to check for execution failure. +This creates and returns a transaction request, which includes proof of correct private execution and side-effects. The request is not broadcast however, and no gas is spent. It is typically used in testing contexts to inspect transaction parameters or to check for execution failure. #include_code local-tx-fails /yarn-project/end-to-end/src/guides/dapp_testing.test.ts typescript -Like most Ethereum libraries, `prove` also simulates public execution to try to detect runtime errors that would only occur once the transaction is picked up by the sequencer. This makes `prove` very useful in testing environments, but users shuld be wary of both false positives and negatives in production environments, particularly if the node's data is stale. Public simulation can be skipped by setting the `skipPublicSimulation` flag. +Like most Ethereum libraries, `prove` also simulates public execution to try to detect runtime errors that would only occur once the transaction is picked up by the sequencer. This makes `prove` very useful in testing environments, but users should be wary of both false positives and negatives in production environments, particularly if the node's data is stale. Public simulation can be skipped by setting the `skipPublicSimulation` flag. #### `send` diff --git a/noir-projects/noir-contracts/contracts/app_subscription_contract/src/main.nr b/noir-projects/noir-contracts/contracts/app_subscription_contract/src/main.nr index 81da7a9aad7b..7be2b0de5144 100644 --- a/noir-projects/noir-contracts/contracts/app_subscription_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/app_subscription_contract/src/main.nr @@ -7,12 +7,12 @@ contract AppSubscription { use aztec::{ prelude::{AztecAddress, Map, PrivateMutable, SharedImmutable}, encrypted_logs::encrypted_note_emission::{encode_and_encrypt_note, encode_and_encrypt_note_with_keys}, - keys::getters::get_current_public_keys, - protocol_types::constants::{MAX_FIELD_VALUE, ROUTER_ADDRESS}, utils::comparison::Comparator + keys::getters::get_current_public_keys, protocol_types::constants::MAX_FIELD_VALUE, + utils::comparison::Comparator }; use authwit::auth::assert_current_call_valid_authwit; use token::Token; - use router::Router; + use router::utils::privately_check_block_number; #[aztec(storage)] struct Storage { @@ -54,7 +54,7 @@ contract AppSubscription { // We check that the note is not expired. We do that via the router contract to conceal which contract // is performing the check. - Router::at(ROUTER_ADDRESS).check_block_number(Comparator.LT, note.expiry_block_number).call(&mut context); + privately_check_block_number(Comparator.LT, note.expiry_block_number, &mut context); payload.execute_calls(&mut context, storage.target_address.read_private()); } @@ -88,10 +88,11 @@ contract AppSubscription { // Assert that the `current_block_number > expiry_block_number - SUBSCRIPTION_DURATION_IN_BLOCKS`. // --> We do that via the router contract to conceal which contract is performing the check. - Router::at(ROUTER_ADDRESS).check_block_number( + privately_check_block_number( Comparator.GT, - expiry_block_number - SUBSCRIPTION_DURATION_IN_BLOCKS - ).call(&mut context); + expiry_block_number - SUBSCRIPTION_DURATION_IN_BLOCKS, + &mut context + ); let subscriber_keys = get_current_public_keys(&mut context, subscriber); let msg_sender_ovpk_m = get_current_public_keys(&mut context, context.msg_sender()).ovpk_m; diff --git a/noir-projects/noir-contracts/contracts/crowdfunding_contract/src/main.nr b/noir-projects/noir-contracts/contracts/crowdfunding_contract/src/main.nr index 6b98a8ac6419..4e0feeceb05b 100644 --- a/noir-projects/noir-contracts/contracts/crowdfunding_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/crowdfunding_contract/src/main.nr @@ -4,17 +4,16 @@ contract Crowdfunding { // docs:start:all-deps use dep::aztec::{ - protocol_types::{address::AztecAddress, constants::ROUTER_ADDRESS}, encrypted_logs::encrypted_note_emission::encode_and_encrypt_note_with_keys, - keys::getters::get_current_public_keys, state_vars::{PrivateSet, SharedImmutable}, - note::note_getter_options::Comparator + keys::getters::get_current_public_keys, prelude::{AztecAddress, PrivateSet, SharedImmutable}, + utils::comparison::Comparator }; use dep::aztec::unencrypted_logs::unencrypted_event_emission::encode_event; // docs:start:import_valuenote use dep::value_note::value_note::ValueNote; // docs:end:import_valuenote use dep::token::Token; - use router::Router; + use router::utils::privately_check_timestamp; // docs:end:all-deps #[aztec(event)] @@ -60,7 +59,7 @@ contract Crowdfunding { // is performing the check. // docs:start:call-check-deadline let deadline = storage.deadline.read_private(); - Router::at(ROUTER_ADDRESS).check_timestamp(Comparator.LT, deadline).call(&mut context); + privately_check_timestamp(Comparator.LT, deadline, &mut context); // docs:end:call-check-deadline // docs:start:do-transfer diff --git a/noir-projects/noir-contracts/contracts/router_contract/src/main.nr b/noir-projects/noir-contracts/contracts/router_contract/src/main.nr index da84a82e3c06..fd614c18c540 100644 --- a/noir-projects/noir-contracts/contracts/router_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/router_contract/src/main.nr @@ -1,4 +1,5 @@ mod test; +mod utils; /// The purpose of this contract is to perform a check in public without revealing what contract enqued the public /// call. This is achieved by having a private function on this contract that enques the public call and hence @@ -28,9 +29,7 @@ contract Router { /// to the `value. #[aztec(private)] fn check_block_number(operation: u8, value: Field) { - // docs:start:enqueueing Router::at(context.this_address())._check_block_number(operation, value).enqueue_view(&mut context); - // docs:end:enqueueing } #[aztec(public)] diff --git a/noir-projects/noir-contracts/contracts/router_contract/src/utils.nr b/noir-projects/noir-contracts/contracts/router_contract/src/utils.nr new file mode 100644 index 000000000000..643483d1656c --- /dev/null +++ b/noir-projects/noir-contracts/contracts/router_contract/src/utils.nr @@ -0,0 +1,15 @@ +use aztec::protocol_types::constants::ROUTER_ADDRESS; +use aztec::context::private_context::PrivateContext; +use crate::Router; + +// docs:start:helper_router_functions +pub fn privately_check_timestamp(operation: u8, value: u64, context: &mut PrivateContext) { + Router::at(ROUTER_ADDRESS).check_timestamp(operation, value).call(context); +} + +pub fn privately_check_block_number(operation: u8, value: Field, context: &mut PrivateContext) { + // docs:start:enqueueing + Router::at(ROUTER_ADDRESS).check_block_number(operation, value).call(context); + // docs:end:enqueueing +} +// docs:end:helper_router_functions \ No newline at end of file From 0aa1046c07af5a0939b48c86ffcf8d2105c18de1 Mon Sep 17 00:00:00 2001 From: benesjan Date: Thu, 5 Sep 2024 10:12:37 +0000 Subject: [PATCH 6/8] improved docs --- docs/docs/aztec/glossary/call_types.md | 9 +++++++++ .../contract_tutorials/crowdfunding_contract.md | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/docs/docs/aztec/glossary/call_types.md b/docs/docs/aztec/glossary/call_types.md index db30eed8d034..efd977d48cb0 100644 --- a/docs/docs/aztec/glossary/call_types.md +++ b/docs/docs/aztec/glossary/call_types.md @@ -133,6 +133,15 @@ This is what the implementation of the check timestamp functionality looks like: #include_code check_timestamp /noir-projects/noir-contracts/contracts/router_contract/src/main.nr rust +:::note +Note that the router contract is not currently part of the [aztec-nr repository](https://github.com/AztecProtocol/aztec-nr). +To add it as a dependency point to the aztec-packages repository instead: +```toml +[dependencies] +aztec = { git = "https://github.com/AztecProtocol/aztec-packages/", tag = "#include_aztec_version", directory = "noir-projects/noir-contracts/contracts/router_contract/src" } +``` +::: + Even with the router contract achieving good privacy is hard. For example, if the value being checked against is unique and stored in the contract's public storage, it's then simple to find private transactions that are using that value in the enqueued public reads, and therefore link them to this contract. For this reason it is encouraged to try to avoid public function calls and instead privately read [Shared State](../../reference/developer_references/smart_contract_reference/storage/shared_state.md) when possible. diff --git a/docs/docs/tutorials/codealong/contract_tutorials/crowdfunding_contract.md b/docs/docs/tutorials/codealong/contract_tutorials/crowdfunding_contract.md index 5cfb2dba062e..9995740f5eec 100644 --- a/docs/docs/tutorials/codealong/contract_tutorials/crowdfunding_contract.md +++ b/docs/docs/tutorials/codealong/contract_tutorials/crowdfunding_contract.md @@ -93,7 +93,7 @@ aztec = { git="https://github.com/AztecProtocol/aztec-packages/", tag="#include_ A word about versions: - Choose the aztec packages version to match your aztec sandbox version -- Check that your `compiler_version` in Nargo.toml is satisified by your aztec compiler - `aztec-nargo -V` +- Check that your `compiler_version` in Nargo.toml is satisfied by your aztec compiler - `aztec-nargo -V` Inside the Crowdfunding contract definition, use the dependency that defines the address type `AztecAddress` (same syntax as Rust) From 4ed01efe23ec41d6e4f1549ae1459db6c6f425b2 Mon Sep 17 00:00:00 2001 From: benesjan Date: Thu, 5 Sep 2024 15:07:51 +0000 Subject: [PATCH 7/8] inline docs --- .../noir-contracts/contracts/router_contract/src/utils.nr | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/noir-projects/noir-contracts/contracts/router_contract/src/utils.nr b/noir-projects/noir-contracts/contracts/router_contract/src/utils.nr index 643483d1656c..1258985565a8 100644 --- a/noir-projects/noir-contracts/contracts/router_contract/src/utils.nr +++ b/noir-projects/noir-contracts/contracts/router_contract/src/utils.nr @@ -3,10 +3,16 @@ use aztec::context::private_context::PrivateContext; use crate::Router; // docs:start:helper_router_functions +/// Asserts that the current timestamp in the enqueued public call enqueued by `check_timestamp` satisfies +/// the `operation` with respect to the `value. Preserves privacy by performing the check via the router contract. +/// This conceals an address of the calling contract by setting `context.msg_sender` to the router contract address. pub fn privately_check_timestamp(operation: u8, value: u64, context: &mut PrivateContext) { Router::at(ROUTER_ADDRESS).check_timestamp(operation, value).call(context); } +/// Asserts that the current block number in the enqueued public call enqueued by `check_block_number` satisfies +/// the `operation` with respect to the `value. Preserves privacy by performing the check via the router contract. +/// This conceals an address of the calling contract by setting `context.msg_sender` to the router contract address. pub fn privately_check_block_number(operation: u8, value: Field, context: &mut PrivateContext) { // docs:start:enqueueing Router::at(ROUTER_ADDRESS).check_block_number(operation, value).call(context); From 54de1b0ada3f0b9a3f4c2f97a911641fb538e55a Mon Sep 17 00:00:00 2001 From: benesjan Date: Thu, 5 Sep 2024 15:08:05 +0000 Subject: [PATCH 8/8] less verbose tests --- .../aztec-nr/aztec/src/utils/comparison.nr | 42 +++++++------------ 1 file changed, 14 insertions(+), 28 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/utils/comparison.nr b/noir-projects/aztec-nr/aztec/src/utils/comparison.nr index 2621bfd58750..3f44be53a406 100644 --- a/noir-projects/aztec-nr/aztec/src/utils/comparison.nr +++ b/noir-projects/aztec-nr/aztec/src/utils/comparison.nr @@ -47,72 +47,58 @@ mod test { fn test_compare() { let lhs = 10; let rhs = 10; - let result = compare(lhs, Comparator.EQ, rhs); - assert(result, "Expected lhs to be equal to rhs"); + assert(compare(lhs, Comparator.EQ, rhs), "Expected lhs to be equal to rhs"); let lhs = 10; let rhs = 11; - let result = compare(lhs, Comparator.NEQ, rhs); - assert(result, "Expected lhs to be not equal to rhs"); + assert(compare(lhs, Comparator.NEQ, rhs), "Expected lhs to be not equal to rhs"); let lhs = 10; let rhs = 11; - let result = compare(lhs, Comparator.LT, rhs); - assert(result, "Expected lhs to be less than rhs"); + assert(compare(lhs, Comparator.LT, rhs), "Expected lhs to be less than rhs"); let lhs = 10; let rhs = 10; - let result = compare(lhs, Comparator.LTE, rhs); - assert(result, "Expected lhs to be less than or equal to rhs"); + assert(compare(lhs, Comparator.LTE, rhs), "Expected lhs to be less than or equal to rhs"); let lhs = 11; let rhs = 10; - let result = compare(lhs, Comparator.GT, rhs); - assert(result, "Expected lhs to be greater than rhs"); + assert(compare(lhs, Comparator.GT, rhs), "Expected lhs to be greater than rhs"); let lhs = 10; let rhs = 10; - let result = compare(lhs, Comparator.GTE, rhs); - assert(result, "Expected lhs to be greater than or equal to rhs"); + assert(compare(lhs, Comparator.GTE, rhs), "Expected lhs to be greater than or equal to rhs"); let lhs = 11; let rhs = 10; - let result = compare(lhs, Comparator.GTE, rhs); - assert(result, "Expected lhs to be greater than or equal to rhs"); + assert(compare(lhs, Comparator.GTE, rhs), "Expected lhs to be greater than or equal to rhs"); let lhs = 10; let rhs = 11; - let result = compare(lhs, Comparator.EQ, rhs); - assert(!result, "Expected lhs to be not equal to rhs"); + assert(!compare(lhs, Comparator.EQ, rhs), "Expected lhs to be not equal to rhs"); let lhs = 10; let rhs = 10; - let result = compare(lhs, Comparator.NEQ, rhs); - assert(!result, "Expected lhs to not be not equal to rhs"); + assert(!compare(lhs, Comparator.NEQ, rhs), "Expected lhs to not be not equal to rhs"); let lhs = 11; let rhs = 10; - let result = compare(lhs, Comparator.LT, rhs); - assert(!result, "Expected lhs to not be less than rhs"); + assert(!compare(lhs, Comparator.LT, rhs), "Expected lhs to not be less than rhs"); let lhs = 11; let rhs = 10; - let result = compare(lhs, Comparator.LTE, rhs); - assert(!result, "Expected lhs to not be less than or equal to rhs"); + assert(!compare(lhs, Comparator.LTE, rhs), "Expected lhs to not be less than or equal to rhs"); let lhs = 10; let rhs = 10; - let result = compare(lhs, Comparator.GT, rhs); - assert(!result, "Expected lhs to not be greater than rhs"); + assert(!compare(lhs, Comparator.GT, rhs), "Expected lhs to not be greater than rhs"); let lhs = 10; let rhs = 11; - let result = compare(lhs, Comparator.GTE, rhs); - assert(!result, "Expected lhs to not be greater than or equal to rhs"); + assert(!compare(lhs, Comparator.GTE, rhs), "Expected lhs to not be greater than or equal to rhs"); let lhs = 10; let rhs = 11; - let result = compare(lhs, Comparator.GTE, rhs); - assert(!result, "Expected lhs to not be greater than or equal to rhs"); + assert(!compare(lhs, Comparator.GTE, rhs), "Expected lhs to not be greater than or equal to rhs"); } }