diff --git a/.noir-sync-commit b/.noir-sync-commit index 8bf1f2ff4b0..53551d01353 100644 --- a/.noir-sync-commit +++ b/.noir-sync-commit @@ -1 +1 @@ -598230d9427cf988fc6da8fe9e1eb2b7c00a2fa6 +d8e549aad6bae0f96621c57b58a301693463f732 diff --git a/noir-projects/aztec-nr/aztec/src/event/event_interface.nr b/noir-projects/aztec-nr/aztec/src/event/event_interface.nr index 08498f1373d..1c76a038de4 100644 --- a/noir-projects/aztec-nr/aztec/src/event/event_interface.nr +++ b/noir-projects/aztec-nr/aztec/src/event/event_interface.nr @@ -1,8 +1,9 @@ use dep::protocol_types::abis::event_selector::EventSelector; +use dep::protocol_types::traits::{Deserialize, Serialize}; -pub trait EventInterface { +pub trait EventInterface: Serialize { fn private_to_be_bytes(self, randomness: Field) -> [u8; N * 32 + 64]; fn to_be_bytes(self) -> [u8; N * 32 + 32]; fn get_event_type_id() -> EventSelector; - fn emit(self, _emit: fn[Env](Self) -> ()); + fn emit(self, emit: fn[Env](Self) -> ()); } diff --git a/noir-projects/aztec-nr/aztec/src/note/note_interface.nr b/noir-projects/aztec-nr/aztec/src/note/note_interface.nr index b2c0f5b8d17..8566d7673a3 100644 --- a/noir-projects/aztec-nr/aztec/src/note/note_interface.nr +++ b/noir-projects/aztec-nr/aztec/src/note/note_interface.nr @@ -1,11 +1,16 @@ use crate::context::PrivateContext; use crate::note::note_header::NoteHeader; +use dep::protocol_types::traits::{Deserialize, Empty, Serialize}; pub trait NoteProperties { fn properties() -> T; } -pub trait PartialNote { +pub trait PartialNote +where + S: Empty, + F: Empty, +{ fn setup_payload() -> S; fn finalization_payload() -> F; diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/map.nr b/noir-projects/aztec-nr/aztec/src/state_vars/map.nr index f695a78a49d..5e9febc29cb 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/map.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/map.nr @@ -1,5 +1,8 @@ use crate::state_vars::storage::Storage; -use dep::protocol_types::{storage::map::derive_storage_slot_in_map, traits::ToField}; +use dep::protocol_types::{ + storage::map::derive_storage_slot_in_map, + traits::{Deserialize, Serialize, ToField}, +}; // docs:start:map pub struct Map { @@ -9,7 +12,10 @@ pub struct Map { } // docs:end:map -impl Storage for Map {} +impl Storage for Map +where + T: Serialize + Deserialize, +{} impl Map { // docs:start:new 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 81bb6e42e6f..89d827f5951 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 @@ -1,5 +1,7 @@ use dep::protocol_types::{ - constants::GENERATOR_INDEX__INITIALIZATION_NULLIFIER, hash::poseidon2_hash_with_separator, + constants::GENERATOR_INDEX__INITIALIZATION_NULLIFIER, + hash::poseidon2_hash_with_separator, + traits::{Deserialize, Serialize}, }; use crate::context::{PrivateContext, UnconstrainedContext}; @@ -20,7 +22,10 @@ pub struct PrivateImmutable { } // docs:end:struct -impl Storage for PrivateImmutable {} +impl Storage for PrivateImmutable +where + T: Serialize + Deserialize, +{} impl PrivateImmutable { // docs:start:new diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/private_mutable.nr b/noir-projects/aztec-nr/aztec/src/state_vars/private_mutable.nr index 7b144d1302a..f721a30a7c2 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/private_mutable.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/private_mutable.nr @@ -1,5 +1,7 @@ use dep::protocol_types::{ - constants::GENERATOR_INDEX__INITIALIZATION_NULLIFIER, hash::poseidon2_hash_with_separator, + constants::GENERATOR_INDEX__INITIALIZATION_NULLIFIER, + hash::poseidon2_hash_with_separator, + traits::{Deserialize, Serialize}, }; use crate::context::{PrivateContext, UnconstrainedContext}; @@ -22,7 +24,10 @@ pub struct PrivateMutable { mod test; -impl Storage for PrivateMutable {} +impl Storage for PrivateMutable +where + T: Serialize + Deserialize, +{} impl PrivateMutable { // docs:start:new diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/private_set.nr b/noir-projects/aztec-nr/aztec/src/state_vars/private_set.nr index b63638fc33f..7e94bdd8b0c 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/private_set.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/private_set.nr @@ -11,7 +11,9 @@ use crate::note::{ }; use crate::state_vars::storage::Storage; use dep::protocol_types::{ - abis::read_request::ReadRequest, constants::MAX_NOTE_HASH_READ_REQUESTS_PER_CALL, + abis::read_request::ReadRequest, + constants::MAX_NOTE_HASH_READ_REQUESTS_PER_CALL, + traits::{Deserialize, Serialize}, }; // docs:start:struct @@ -21,7 +23,10 @@ pub struct PrivateSet { } // docs:end:struct -impl Storage for PrivateSet {} +impl Storage for PrivateSet +where + T: Serialize + Deserialize, +{} impl PrivateSet { // docs:start:new 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 1e9afe1de55..ae971bc9c35 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 @@ -12,7 +12,10 @@ pub struct PublicImmutable { } // docs:end:public_immutable_struct -impl Storage for PublicImmutable {} +impl Storage for PublicImmutable +where + T: Serialize + Deserialize, +{} impl PublicImmutable { // docs:start:public_immutable_struct_new diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/public_mutable.nr b/noir-projects/aztec-nr/aztec/src/state_vars/public_mutable.nr index d2b965d3377..0f69d4cd528 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/public_mutable.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/public_mutable.nr @@ -9,7 +9,10 @@ pub struct PublicMutable { } // docs:end:public_mutable_struct -impl Storage for PublicMutable {} +impl Storage for PublicMutable +where + T: Serialize + Deserialize, +{} impl PublicMutable { // docs:start:public_mutable_struct_new diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/shared_immutable.nr b/noir-projects/aztec-nr/aztec/src/state_vars/shared_immutable.nr index 00add0afb26..52eab0990c4 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/shared_immutable.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/shared_immutable.nr @@ -13,7 +13,10 @@ pub struct SharedImmutable { storage_slot: Field, } -impl Storage for SharedImmutable {} +impl Storage for SharedImmutable +where + T: Serialize + Deserialize, +{} impl SharedImmutable { pub fn new( diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/shared_mutable.nr b/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/shared_mutable.nr index 0a499c40c70..0d1d76d984a 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/shared_mutable.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/shared_mutable.nr @@ -1,7 +1,7 @@ use dep::protocol_types::{ address::AztecAddress, hash::{poseidon2_hash, poseidon2_hash_with_separator}, - traits::{FromField, ToField}, + traits::{Deserialize, FromField, Serialize, ToField}, utils::arrays::array_concat, }; @@ -34,7 +34,10 @@ global HASH_SEPARATOR: u32 = 2; // // TODO https://github.com/AztecProtocol/aztec-packages/issues/5736: change the storage allocation scheme so that we // can actually use it here -impl Storage for SharedMutable {} +impl Storage for SharedMutable +where + T: Serialize + Deserialize, +{} // SharedMutable stores a value of type T that is: // - publicly known (i.e. unencrypted) diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/storage.nr b/noir-projects/aztec-nr/aztec/src/state_vars/storage.nr index c8b7b37c229..be7d986eaf5 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/storage.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/storage.nr @@ -1,6 +1,6 @@ use dep::protocol_types::traits::{Deserialize, Serialize}; -pub trait Storage +pub trait Storage where T: Serialize + Deserialize, { diff --git a/noir-projects/aztec-nr/aztec/src/unencrypted_logs/unencrypted_event_emission.nr b/noir-projects/aztec-nr/aztec/src/unencrypted_logs/unencrypted_event_emission.nr index a5a8995ec63..2636bbc0ed5 100644 --- a/noir-projects/aztec-nr/aztec/src/unencrypted_logs/unencrypted_event_emission.nr +++ b/noir-projects/aztec-nr/aztec/src/unencrypted_logs/unencrypted_event_emission.nr @@ -1,9 +1,8 @@ use crate::{context::PublicContext, event::event_interface::EventInterface}; -use dep::protocol_types::traits::Serialize; fn emit(context: &mut PublicContext, event: Event) where - Event: EventInterface + Serialize, + Event: EventInterface, { let selector = Event::get_event_type_id(); @@ -24,7 +23,7 @@ pub fn encode_event( context: &mut PublicContext, ) -> fn[(&mut PublicContext,)](Event) -> () where - Event: EventInterface + Serialize, + Event: EventInterface, { |e: Event| { emit(context, e); } } diff --git a/noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr b/noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr index 4f7402b954c..5d39516e793 100644 --- a/noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr @@ -212,7 +212,7 @@ contract AvmTest { #[public] fn to_radix_le(input: Field) -> [u8; 10] { - input.to_le_radix( /*base=*/ 2) + input.to_le_radix(/*base=*/ 2) } // Helper functions to demonstrate an internal call stack in error messages @@ -292,7 +292,7 @@ contract AvmTest { #[public] fn pedersen_hash_with_index(data: [Field; 10]) -> Field { - std::hash::pedersen_hash_with_separator(data, /*index=*/ 20) + std::hash::pedersen_hash_with_separator(data, /*index=*/ 20) } /************************************************************************ @@ -425,11 +425,11 @@ contract AvmTest { #[public] fn emit_unencrypted_log() { - context.emit_unencrypted_log( /*message=*/ [10, 20, 30]); - context.emit_unencrypted_log( /*message=*/ "Hello, world!"); + context.emit_unencrypted_log(/*message=*/ [10, 20, 30]); + context.emit_unencrypted_log(/*message=*/ "Hello, world!"); let s: CompressedString<2, 44> = CompressedString::from_string("A long time ago, in a galaxy far far away..."); - context.emit_unencrypted_log( /*message=*/ s); + context.emit_unencrypted_log(/*message=*/ s); } #[public] diff --git a/noir-projects/noir-contracts/contracts/contract_instance_deployer_contract/src/main.nr b/noir-projects/noir-contracts/contracts/contract_instance_deployer_contract/src/main.nr index 7a990b386dc..4ce03ebb6e1 100644 --- a/noir-projects/noir-contracts/contracts/contract_instance_deployer_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/contract_instance_deployer_contract/src/main.nr @@ -19,6 +19,7 @@ contract ContractInstanceDeployer { use std::meta::derive; #[event] + #[derive(Serialize)] struct ContractInstanceDeployed { DEPLOYER_CONTRACT_INSTANCE_DEPLOYED_MAGIC_VALUE: Field, address: AztecAddress, diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/test/access_control.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/test/access_control.nr index fef5a236372..42373942d78 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/test/access_control.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/test/access_control.nr @@ -5,7 +5,7 @@ use crate::test::utils; unconstrained fn access_control() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, nft_contract_address, owner, recipient) = - utils::setup( /* with_account_contracts */ false); + utils::setup(/* with_account_contracts */ false); // Set a new admin NFT::at(nft_contract_address).set_admin(recipient).call(&mut env.public()); diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/test/minting.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/test/minting.nr index cc9d12e24dd..5302e0e397d 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/test/minting.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/test/minting.nr @@ -4,7 +4,7 @@ use crate::test::utils; #[test] unconstrained fn mint_success() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough - let (env, nft_contract_address, owner, _) = utils::setup( /* with_account_contracts */ false); + let (env, nft_contract_address, owner, _) = utils::setup(/* with_account_contracts */ false); let token_id = 10000; NFT::at(nft_contract_address).mint(owner, token_id).call(&mut env.public()); @@ -16,7 +16,7 @@ unconstrained fn mint_success() { unconstrained fn mint_failures() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, nft_contract_address, owner, recipient) = - utils::setup( /* with_account_contracts */ false); + utils::setup(/* with_account_contracts */ false); // MINTING AS A NON-MINTER let token_id = 10000; diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_in_private.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_in_private.nr index 63022cc57a0..b6b07566bc9 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_in_private.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_in_private.nr @@ -8,7 +8,7 @@ use aztec::oracle::random::random; unconstrained fn transfer_in_private() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, nft_contract_address, sender, recipient, token_id) = - utils::setup_mint_and_transfer_to_private( /* with_account_contracts */ false); + utils::setup_mint_and_transfer_to_private(/* with_account_contracts */ false); // Transfer the NFT to the recipient NFT::at(nft_contract_address).transfer_in_private(sender, recipient, token_id, 0).call( @@ -23,7 +23,7 @@ unconstrained fn transfer_in_private() { unconstrained fn transfer_in_private_to_self() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, nft_contract_address, owner, _, token_id) = - utils::setup_mint_and_transfer_to_private( /* with_account_contracts */ false); + utils::setup_mint_and_transfer_to_private(/* with_account_contracts */ false); // Transfer the NFT back to the owner NFT::at(nft_contract_address).transfer_in_private(owner, owner, token_id, 0).call( @@ -38,7 +38,7 @@ unconstrained fn transfer_in_private_to_self() { unconstrained fn transfer_in_private_to_non_deployed_account() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, nft_contract_address, sender, _, token_id) = - utils::setup_mint_and_transfer_to_private( /* with_account_contracts */ false); + utils::setup_mint_and_transfer_to_private(/* with_account_contracts */ false); let not_deployed = cheatcodes::create_account(); // Transfer the NFT to the recipient @@ -54,7 +54,7 @@ unconstrained fn transfer_in_private_to_non_deployed_account() { unconstrained fn transfer_in_private_on_behalf_of_other() { // Setup with account contracts. Slower since we actually deploy them, but needed for authwits. let (env, nft_contract_address, sender, recipient, token_id) = - utils::setup_mint_and_transfer_to_private( /* with_account_contracts */ true); + utils::setup_mint_and_transfer_to_private(/* with_account_contracts */ true); // Transfer the NFT to the recipient let transfer_in_private_call_interface = @@ -78,7 +78,7 @@ unconstrained fn transfer_in_private_on_behalf_of_other() { unconstrained fn transfer_in_private_failure_not_an_owner() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, nft_contract_address, owner, not_owner, token_id) = - utils::setup_mint_and_transfer_to_private( /* with_account_contracts */ false); + utils::setup_mint_and_transfer_to_private(/* with_account_contracts */ false); // Try transferring the NFT from not_owner env.impersonate(not_owner); NFT::at(nft_contract_address).transfer_in_private(not_owner, owner, token_id, 0).call( @@ -92,7 +92,7 @@ unconstrained fn transfer_in_private_failure_on_behalf_of_self_non_zero_nonce() // The nonce check is in the beginning so we don't need to waste time on minting the NFT and transferring // it to private. let (env, nft_contract_address, sender, recipient) = - utils::setup( /* with_account_contracts */ false); + utils::setup(/* with_account_contracts */ false); // We set random value for the token_id as the nonce check is before we use the value. let token_id = random(); @@ -109,7 +109,7 @@ unconstrained fn transfer_in_private_failure_on_behalf_of_other_without_approval // The authwit check is in the beginning so we don't need to waste time on minting the NFT and transferring // it to private. let (env, nft_contract_address, sender, recipient) = - utils::setup( /* with_account_contracts */ true); + utils::setup(/* with_account_contracts */ true); // We set random value for the token_id as the nonce check is before we use the value. let token_id = random(); @@ -128,7 +128,7 @@ unconstrained fn transfer_in_private_failure_on_behalf_of_other_wrong_caller() { // The authwit check is in the beginning so we don't need to waste time on minting the NFT and transferring // it to private. let (env, nft_contract_address, sender, recipient) = - utils::setup( /* with_account_contracts */ true); + utils::setup(/* with_account_contracts */ true); // We set random value for the token_id as the nonce check is before we use the value. let token_id = random(); diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_in_public.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_in_public.nr index d64c5180461..f02d84202a7 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_in_public.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_in_public.nr @@ -7,7 +7,7 @@ use dep::aztec::oracle::random::random; unconstrained fn transfer_in_public() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, nft_contract_address, sender, recipient, token_id) = - utils::setup_and_mint( /* with_account_contracts */ false); + utils::setup_and_mint(/* with_account_contracts */ false); // Transfer the NFT NFT::at(nft_contract_address).transfer_in_public(sender, recipient, token_id, 0).call( @@ -21,7 +21,7 @@ unconstrained fn transfer_in_public() { unconstrained fn transfer_in_public_to_self() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, nft_contract_address, user, _, token_id) = - utils::setup_and_mint( /* with_account_contracts */ false); + utils::setup_and_mint(/* with_account_contracts */ false); // Transfer the NFT NFT::at(nft_contract_address).transfer_in_public(user, user, token_id, 0).call(&mut env.public()); @@ -34,7 +34,7 @@ unconstrained fn transfer_in_public_to_self() { unconstrained fn transfer_in_public_on_behalf_of_other() { // Setup with account contracts. Slower since we actually deploy them, but needed for authwits. let (env, nft_contract_address, sender, recipient, token_id) = - utils::setup_and_mint( /* with_account_contracts */ true); + utils::setup_and_mint(/* with_account_contracts */ true); let transfer_in_public_from_call_interface = NFT::at(nft_contract_address).transfer_in_public(sender, recipient, token_id, 1); @@ -58,7 +58,7 @@ unconstrained fn transfer_in_public_failure_on_behalf_of_self_non_zero_nonce() { // The authwit check is in the beginning so we don't need to waste time on minting the NFT and transferring // it to private.. let (env, nft_contract_address, sender, recipient) = - utils::setup( /* with_account_contracts */ false); + utils::setup(/* with_account_contracts */ false); // We set random value for the token_id as the nonce check is before we use the value. let token_id = random(); @@ -73,7 +73,7 @@ unconstrained fn transfer_in_public_failure_on_behalf_of_self_non_zero_nonce() { unconstrained fn transfer_in_public_non_existent_nft() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, nft_contract_address, sender, recipient) = - utils::setup( /* with_account_contracts */ false); + utils::setup(/* with_account_contracts */ false); // Try to transfer the NFT let token_id = 612; @@ -86,7 +86,7 @@ unconstrained fn transfer_in_public_non_existent_nft() { unconstrained fn transfer_in_public_failure_on_behalf_of_other_without_approval() { // Setup with account contracts. Slower since we actually deploy them, but needed for authwits. let (env, nft_contract_address, sender, recipient, token_id) = - utils::setup_and_mint( /* with_account_contracts */ true); + utils::setup_and_mint(/* with_account_contracts */ true); // Impersonate recipient to perform the call env.impersonate(recipient); @@ -100,7 +100,7 @@ unconstrained fn transfer_in_public_failure_on_behalf_of_other_without_approval( unconstrained fn transfer_in_public_failure_on_behalf_of_other_wrong_caller() { // Setup with account contracts. Slower since we actually deploy them, but needed for authwits. let (env, nft_contract_address, sender, recipient, token_id) = - utils::setup_and_mint( /* with_account_contracts */ true); + utils::setup_and_mint(/* with_account_contracts */ true); let transfer_in_public_from_call_interface = NFT::at(nft_contract_address).transfer_in_public(sender, recipient, token_id, 1); authwit_cheatcodes::add_public_authwit_from_call_interface( diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr index e70b66e8ab6..21e109704a7 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr @@ -17,7 +17,7 @@ unconstrained fn transfer_to_private_internal_orchestration() { // in this test we just call it and check the outcome. // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, nft_contract_address, user, _, token_id) = - utils::setup_mint_and_transfer_to_private( /* with_account_contracts */ false); + utils::setup_mint_and_transfer_to_private(/* with_account_contracts */ false); // User should have the note in their private nfts utils::assert_owns_private_nft(nft_contract_address, user, token_id); @@ -32,7 +32,7 @@ unconstrained fn transfer_to_private_internal_orchestration() { unconstrained fn transfer_to_private_external_orchestration() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, nft_contract_address, _, recipient, token_id) = - utils::setup_and_mint( /* with_account_contracts */ false); + utils::setup_and_mint(/* with_account_contracts */ false); let note_randomness = random(); @@ -77,7 +77,7 @@ unconstrained fn transfer_to_private_external_orchestration() { unconstrained fn transfer_to_private_transfer_not_prepared() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, nft_contract_address, _, _, token_id) = - utils::setup_and_mint( /* with_account_contracts */ false); + utils::setup_and_mint(/* with_account_contracts */ false); // Transfer was not prepared so we can use random value for the hiding point slot let hiding_point_slot = random(); @@ -92,7 +92,7 @@ unconstrained fn transfer_to_private_transfer_not_prepared() { unconstrained fn transfer_to_private_failure_not_an_owner() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, nft_contract_address, _, not_owner, token_id) = - utils::setup_and_mint( /* with_account_contracts */ false); + utils::setup_and_mint(/* with_account_contracts */ false); // (For this specific test we could set a random value for the commitment and not do the call to `prepare...` // as the NFT owner check is before we use the value but that would made the test less robust against changes diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_public.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_public.nr index 0dbec15b255..83f3690a5dd 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_public.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_public.nr @@ -7,7 +7,7 @@ use dep::aztec::oracle::random::random; unconstrained fn transfer_to_public() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, nft_contract_address, sender, recipient, token_id) = - utils::setup_mint_and_transfer_to_private( /* with_account_contracts */ false); + utils::setup_mint_and_transfer_to_private(/* with_account_contracts */ false); NFT::at(nft_contract_address).transfer_to_public(sender, recipient, token_id, 0).call( &mut env.private(), @@ -21,7 +21,7 @@ unconstrained fn transfer_to_public() { unconstrained fn transfer_to_public_to_self() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, nft_contract_address, user, _, token_id) = - utils::setup_mint_and_transfer_to_private( /* with_account_contracts */ false); + utils::setup_mint_and_transfer_to_private(/* with_account_contracts */ false); NFT::at(nft_contract_address).transfer_to_public(user, user, token_id, 0).call( &mut env.private(), @@ -34,7 +34,7 @@ unconstrained fn transfer_to_public_to_self() { #[test] unconstrained fn transfer_to_public_on_behalf_of_other() { let (env, nft_contract_address, sender, recipient, token_id) = - utils::setup_mint_and_transfer_to_private( /* with_account_contracts */ true); + utils::setup_mint_and_transfer_to_private(/* with_account_contracts */ true); let transfer_to_public_call_interface = NFT::at(nft_contract_address).transfer_to_public(sender, recipient, token_id, 0); @@ -56,7 +56,7 @@ unconstrained fn transfer_to_public_on_behalf_of_other() { unconstrained fn transfer_to_public_failure_not_an_owner() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, nft_contract_address, _, not_owner, token_id) = - utils::setup_mint_and_transfer_to_private( /* with_account_contracts */ false); + utils::setup_mint_and_transfer_to_private(/* with_account_contracts */ false); env.impersonate(not_owner); NFT::at(nft_contract_address).transfer_to_public(not_owner, not_owner, token_id, 0).call( @@ -68,7 +68,7 @@ unconstrained fn transfer_to_public_failure_not_an_owner() { unconstrained fn transfer_to_public_failure_on_behalf_of_self_non_zero_nonce() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, nft_contract_address, user, _, token_id) = - utils::setup_mint_and_transfer_to_private( /* with_account_contracts */ false); + utils::setup_mint_and_transfer_to_private(/* with_account_contracts */ false); NFT::at(nft_contract_address).transfer_to_public(user, user, token_id, random()).call( &mut env.private(), @@ -78,7 +78,7 @@ unconstrained fn transfer_to_public_failure_on_behalf_of_self_non_zero_nonce() { #[test(should_fail_with = "Authorization not found for message hash")] unconstrained fn transfer_to_public_failure_on_behalf_of_other_invalid_designated_caller() { let (env, nft_contract_address, sender, recipient, token_id) = - utils::setup_mint_and_transfer_to_private( /* with_account_contracts */ true); + utils::setup_mint_and_transfer_to_private(/* with_account_contracts */ true); let transfer_to_public_call_interface = NFT::at(nft_contract_address).transfer_to_public(sender, recipient, token_id, 0); @@ -96,7 +96,7 @@ unconstrained fn transfer_to_public_failure_on_behalf_of_other_invalid_designate #[test(should_fail_with = "Authorization not found for message hash")] unconstrained fn transfer_to_public_failure_on_behalf_of_other_no_approval() { let (env, nft_contract_address, sender, recipient, token_id) = - utils::setup_mint_and_transfer_to_private( /* with_account_contracts */ true); + utils::setup_mint_and_transfer_to_private(/* with_account_contracts */ true); // Impersonate recipient env.impersonate(recipient); diff --git a/noir-projects/noir-contracts/contracts/test_contract/src/main.nr b/noir-projects/noir-contracts/contracts/test_contract/src/main.nr index c19a1a63ecd..82e254ef37b 100644 --- a/noir-projects/noir-contracts/contracts/test_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/test_contract/src/main.nr @@ -368,9 +368,9 @@ contract Test { #[public] fn emit_unencrypted(value: Field) { // docs:start:emit_unencrypted - context.emit_unencrypted_log( /*message=*/ value); - context.emit_unencrypted_log( /*message=*/ [10, 20, 30]); - context.emit_unencrypted_log( /*message=*/ "Hello, world!"); + context.emit_unencrypted_log(/*message=*/ value); + context.emit_unencrypted_log(/*message=*/ [10, 20, 30]); + context.emit_unencrypted_log(/*message=*/ "Hello, world!"); // docs:end:emit_unencrypted } diff --git a/noir-projects/noir-contracts/contracts/test_contract/src/test_note.nr b/noir-projects/noir-contracts/contracts/test_contract/src/test_note.nr index 7f938fa42cd..477a58bea0c 100644 --- a/noir-projects/noir-contracts/contracts/test_contract/src/test_note.nr +++ b/noir-projects/noir-contracts/contracts/test_contract/src/test_note.nr @@ -2,6 +2,7 @@ use dep::aztec::{ context::PrivateContext, macros::notes::note, note::{note_header::NoteHeader, note_interface::NullifiableNote}, + protocol_types::{address::AztecAddress, traits::{Deserialize, Serialize}}, }; // A note which stores a field and is expected to be passed around using the `addNote` function. @@ -9,6 +10,7 @@ use dep::aztec::{ // serialized_note attack on it. This note has been developed purely for testing purposes so that it can easily be // manually added to PXE. Do not use for real applications. #[note] +#[derive(Serialize, Deserialize)] pub struct TestNote { value: Field, } diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/main.nr b/noir-projects/noir-contracts/contracts/token_contract/src/main.nr index 2fc3f7f06b9..be8359aab8c 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/main.nr @@ -64,8 +64,8 @@ contract Token { // an overall small circuit regardless. global RECURSIVE_TRANSFER_CALL_MAX_NOTES: u32 = 8; - #[event] #[derive(Serialize)] + #[event] struct Transfer { from: AztecAddress, to: AztecAddress, diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/test/access_control.nr b/noir-projects/noir-contracts/contracts/token_contract/src/test/access_control.nr index 9cb547b237d..aafe621f498 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/test/access_control.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/test/access_control.nr @@ -5,7 +5,7 @@ use crate::Token; unconstrained fn access_control() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, owner, recipient) = - utils::setup( /* with_account_contracts */ false); + utils::setup(/* with_account_contracts */ false); // Set a new admin Token::at(token_contract_address).set_admin(recipient).call(&mut env.public()); diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/test/burn.nr b/noir-projects/noir-contracts/contracts/token_contract/src/test/burn.nr index 3fbf1d73c64..daa91a0d26c 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/test/burn.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/test/burn.nr @@ -6,7 +6,7 @@ use dep::aztec::oracle::random::random; #[test] unconstrained fn burn_public_success() { let (env, token_contract_address, owner, recipient, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ false); + utils::setup_and_mint(/* with_account_contracts */ false); let burn_amount = mint_amount / 10; // Burn less than balance @@ -17,7 +17,7 @@ unconstrained fn burn_public_success() { #[test] unconstrained fn burn_public_on_behalf_of_other() { let (env, token_contract_address, owner, recipient, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ true); + utils::setup_and_mint(/* with_account_contracts */ true); let burn_amount = mint_amount / 10; // Burn on behalf of other @@ -38,7 +38,7 @@ unconstrained fn burn_public_on_behalf_of_other() { #[test(should_fail_with = "attempt to subtract with underflow")] unconstrained fn burn_public_failure_more_than_balance() { let (env, token_contract_address, owner, _, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ false); + utils::setup_and_mint(/* with_account_contracts */ false); // Burn more than balance let burn_amount = mint_amount * 10; @@ -49,7 +49,7 @@ unconstrained fn burn_public_failure_more_than_balance() { #[test(should_fail_with = "invalid nonce")] unconstrained fn burn_public_failure_on_behalf_of_self_non_zero_nonce() { let (env, token_contract_address, owner, _, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ false); + utils::setup_and_mint(/* with_account_contracts */ false); // Burn on behalf of self with non-zero nonce let burn_amount = mint_amount / 10; @@ -62,7 +62,7 @@ unconstrained fn burn_public_failure_on_behalf_of_self_non_zero_nonce() { #[test(should_fail_with = "unauthorized")] unconstrained fn burn_public_failure_on_behalf_of_other_without_approval() { let (env, token_contract_address, owner, recipient, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ true); + utils::setup_and_mint(/* with_account_contracts */ true); // Burn on behalf of other without approval let burn_amount = mint_amount / 10; @@ -76,7 +76,7 @@ unconstrained fn burn_public_failure_on_behalf_of_other_without_approval() { #[test(should_fail_with = "unauthorized")] unconstrained fn burn_public_failure_on_behalf_of_other_wrong_caller() { let (env, token_contract_address, owner, recipient, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ true); + utils::setup_and_mint(/* with_account_contracts */ true); // Burn on behalf of other, wrong designated caller let burn_amount = mint_amount / 10; @@ -91,7 +91,7 @@ unconstrained fn burn_public_failure_on_behalf_of_other_wrong_caller() { #[test] unconstrained fn burn_private_on_behalf_of_self() { let (env, token_contract_address, owner, _, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ false); + utils::setup_and_mint(/* with_account_contracts */ false); let burn_amount = mint_amount / 10; // Burn less than balance @@ -102,7 +102,7 @@ unconstrained fn burn_private_on_behalf_of_self() { #[test] unconstrained fn burn_private_on_behalf_of_other() { let (env, token_contract_address, owner, recipient, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ true); + utils::setup_and_mint(/* with_account_contracts */ true); let burn_amount = mint_amount / 10; // Burn on behalf of other @@ -122,7 +122,7 @@ unconstrained fn burn_private_on_behalf_of_other() { #[test(should_fail_with = "Balance too low")] unconstrained fn burn_private_failure_more_than_balance() { let (env, token_contract_address, owner, _, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ false); + utils::setup_and_mint(/* with_account_contracts */ false); // Burn more than balance let burn_amount = mint_amount * 10; @@ -132,7 +132,7 @@ unconstrained fn burn_private_failure_more_than_balance() { #[test(should_fail_with = "invalid nonce")] unconstrained fn burn_private_failure_on_behalf_of_self_non_zero_nonce() { let (env, token_contract_address, owner, _, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ false); + utils::setup_and_mint(/* with_account_contracts */ false); // Burn more than balance let burn_amount = mint_amount / 10; @@ -142,7 +142,7 @@ unconstrained fn burn_private_failure_on_behalf_of_self_non_zero_nonce() { #[test(should_fail_with = "Balance too low")] unconstrained fn burn_private_failure_on_behalf_of_other_more_than_balance() { let (env, token_contract_address, owner, recipient, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ true); + utils::setup_and_mint(/* with_account_contracts */ true); // Burn more than balance let burn_amount = mint_amount * 10; @@ -161,7 +161,7 @@ unconstrained fn burn_private_failure_on_behalf_of_other_more_than_balance() { #[test(should_fail_with = "Authorization not found for message hash")] unconstrained fn burn_private_failure_on_behalf_of_other_without_approval() { let (env, token_contract_address, owner, recipient, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ true); + utils::setup_and_mint(/* with_account_contracts */ true); // Burn more than balance let burn_amount = mint_amount / 10; @@ -175,7 +175,7 @@ unconstrained fn burn_private_failure_on_behalf_of_other_without_approval() { #[test(should_fail_with = "Authorization not found for message hash")] unconstrained fn burn_private_failure_on_behalf_of_other_wrong_designated_caller() { let (env, token_contract_address, owner, recipient, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ true); + utils::setup_and_mint(/* with_account_contracts */ true); // Burn more than balance let burn_amount = mint_amount / 10; diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/test/minting.nr b/noir-projects/noir-contracts/contracts/token_contract/src/test/minting.nr index fb894336bd5..6d2b21207ae 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/test/minting.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/test/minting.nr @@ -5,7 +5,7 @@ use dep::aztec::{hash::compute_secret_hash, oracle::random::random, test::helper #[test] unconstrained fn mint_public_success() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough - let (env, token_contract_address, owner, _) = utils::setup( /* with_account_contracts */ false); + let (env, token_contract_address, owner, _) = utils::setup(/* with_account_contracts */ false); let mint_amount = 10000; Token::at(token_contract_address).mint_public(owner, mint_amount).call(&mut env.public()); @@ -20,7 +20,7 @@ unconstrained fn mint_public_success() { unconstrained fn mint_public_failures() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, owner, recipient) = - utils::setup( /* with_account_contracts */ false); + utils::setup(/* with_account_contracts */ false); // As non-minter let mint_amount = 10000; @@ -60,7 +60,7 @@ unconstrained fn mint_public_failures() { #[test] unconstrained fn mint_private_success() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough - let (env, token_contract_address, owner, _) = utils::setup( /* with_account_contracts */ false); + let (env, token_contract_address, owner, _) = utils::setup(/* with_account_contracts */ false); let mint_amount = 10000; // Mint some tokens let secret = random(); @@ -91,7 +91,7 @@ unconstrained fn mint_private_success() { unconstrained fn mint_private_failure_double_spend() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, owner, recipient) = - utils::setup( /* with_account_contracts */ false); + utils::setup(/* with_account_contracts */ false); let mint_amount = 10000; // Mint some tokens let secret = random(); @@ -127,7 +127,7 @@ unconstrained fn mint_private_failure_double_spend() { unconstrained fn mint_private_failure_non_minter() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, _, recipient) = - utils::setup( /* with_account_contracts */ false); + utils::setup(/* with_account_contracts */ false); let mint_amount = 10000; // Try to mint some tokens impersonating recipient env.impersonate(recipient); @@ -140,7 +140,7 @@ unconstrained fn mint_private_failure_non_minter() { #[test(should_fail_with = "call to assert_max_bit_size")] unconstrained fn mint_private_failure_overflow() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough - let (env, token_contract_address, _, _) = utils::setup( /* with_account_contracts */ false); + let (env, token_contract_address, _, _) = utils::setup(/* with_account_contracts */ false); // Overflow recipient let mint_amount = 2.pow_32(128); @@ -152,7 +152,7 @@ unconstrained fn mint_private_failure_overflow() { #[test(should_fail_with = "attempt to add with overflow")] unconstrained fn mint_private_failure_overflow_recipient() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough - let (env, token_contract_address, owner, _) = utils::setup( /* with_account_contracts */ false); + let (env, token_contract_address, owner, _) = utils::setup(/* with_account_contracts */ false); let mint_amount = 10000; // Mint some tokens let secret = random(); @@ -186,7 +186,7 @@ unconstrained fn mint_private_failure_overflow_recipient() { unconstrained fn mint_private_failure_overflow_total_supply() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, owner, recipient) = - utils::setup( /* with_account_contracts */ false); + utils::setup(/* with_account_contracts */ false); let mint_amount = 10000; // Mint some tokens let secret_owner = random(); diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/test/reading_constants.nr b/noir-projects/noir-contracts/contracts/token_contract/src/test/reading_constants.nr index 6081e27ae4c..af3583f5f25 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/test/reading_constants.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/test/reading_constants.nr @@ -7,7 +7,7 @@ use dep::aztec::test::helpers::cheatcodes; #[test] unconstrained fn check_decimals_private() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough - let (env, token_contract_address, _, _) = utils::setup( /* with_account_contracts */ false); + let (env, token_contract_address, _, _) = utils::setup(/* with_account_contracts */ false); // Check decimals let result = Token::at(token_contract_address).private_get_decimals().view(&mut env.private()); @@ -18,7 +18,7 @@ unconstrained fn check_decimals_private() { #[test] unconstrained fn check_decimals_public() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough - let (env, token_contract_address, _, _) = utils::setup( /* with_account_contracts */ false); + let (env, token_contract_address, _, _) = utils::setup(/* with_account_contracts */ false); // Check decimals let result = Token::at(token_contract_address).public_get_decimals().view(&mut env.public()); diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/test/shielding.nr b/noir-projects/noir-contracts/contracts/token_contract/src/test/shielding.nr index ed49a1807d4..a4b5832e579 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/test/shielding.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/test/shielding.nr @@ -7,7 +7,7 @@ use dep::aztec::{hash::compute_secret_hash, oracle::random::random}; unconstrained fn shielding_on_behalf_of_self() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, owner, _, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ false); + utils::setup_and_mint(/* with_account_contracts */ false); let secret = random(); let secret_hash = compute_secret_hash(secret); // Shield tokens @@ -36,7 +36,7 @@ unconstrained fn shielding_on_behalf_of_self() { #[test] unconstrained fn shielding_on_behalf_of_other() { let (env, token_contract_address, owner, recipient, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ true); + utils::setup_and_mint(/* with_account_contracts */ true); let secret = random(); let secret_hash = compute_secret_hash(secret); @@ -77,7 +77,7 @@ unconstrained fn shielding_on_behalf_of_other() { unconstrained fn shielding_failure_on_behalf_of_self_more_than_balance() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, owner, _, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ true); + utils::setup_and_mint(/* with_account_contracts */ true); let secret = random(); let secret_hash = compute_secret_hash(secret); // Shield tokens @@ -95,7 +95,7 @@ unconstrained fn shielding_failure_on_behalf_of_self_more_than_balance() { unconstrained fn shielding_failure_on_behalf_of_self_invalid_nonce() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, owner, _, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ true); + utils::setup_and_mint(/* with_account_contracts */ true); let secret = random(); let secret_hash = compute_secret_hash(secret); // Shield tokens @@ -113,7 +113,7 @@ unconstrained fn shielding_failure_on_behalf_of_self_invalid_nonce() { unconstrained fn shielding_failure_on_behalf_of_other_more_than_balance() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, owner, recipient, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ true); + utils::setup_and_mint(/* with_account_contracts */ true); let secret = random(); let secret_hash = compute_secret_hash(secret); // Shield tokens on behalf of owner @@ -139,7 +139,7 @@ unconstrained fn shielding_failure_on_behalf_of_other_more_than_balance() { unconstrained fn shielding_failure_on_behalf_of_other_wrong_caller() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, owner, recipient, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ true); + utils::setup_and_mint(/* with_account_contracts */ true); let secret = random(); let secret_hash = compute_secret_hash(secret); // Shield tokens on behalf of owner @@ -161,7 +161,7 @@ unconstrained fn shielding_failure_on_behalf_of_other_wrong_caller() { unconstrained fn shielding_failure_on_behalf_of_other_without_approval() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, owner, recipient, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ true); + utils::setup_and_mint(/* with_account_contracts */ true); let secret = random(); let secret_hash = compute_secret_hash(secret); // Shield tokens on behalf of owner diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/test/transfer_private.nr b/noir-projects/noir-contracts/contracts/token_contract/src/test/transfer_private.nr index 8e606030d1c..66d6d13bd0e 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/test/transfer_private.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/test/transfer_private.nr @@ -7,7 +7,7 @@ use dep::aztec::test::helpers::cheatcodes; unconstrained fn transfer_private() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, owner, recipient, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ false); + utils::setup_and_mint(/* with_account_contracts */ false); // docs:start:txe_test_transfer_private // Transfer tokens @@ -23,7 +23,7 @@ unconstrained fn transfer_private() { unconstrained fn transfer_private_to_self() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, owner, _, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ false); + utils::setup_and_mint(/* with_account_contracts */ false); // Transfer tokens let transfer_amount = 1000; Token::at(token_contract_address).transfer(owner, transfer_amount).call(&mut env.private()); @@ -36,7 +36,7 @@ unconstrained fn transfer_private_to_self() { unconstrained fn transfer_private_to_non_deployed_account() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, owner, _, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ false); + utils::setup_and_mint(/* with_account_contracts */ false); let not_deployed = cheatcodes::create_account(); // Transfer tokens let transfer_amount = 1000; @@ -57,7 +57,7 @@ unconstrained fn transfer_private_to_non_deployed_account() { unconstrained fn transfer_private_on_behalf_of_other() { // Setup with account contracts. Slower since we actually deploy them, but needed for authwits. let (env, token_contract_address, owner, recipient, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ true); + utils::setup_and_mint(/* with_account_contracts */ true); // Add authwit // docs:start:private_authwit let transfer_amount = 1000; @@ -82,7 +82,7 @@ unconstrained fn transfer_private_on_behalf_of_other() { unconstrained fn transfer_private_failure_more_than_balance() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, _, recipient, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ false); + utils::setup_and_mint(/* with_account_contracts */ false); // Transfer tokens let transfer_amount = mint_amount + 1; Token::at(token_contract_address).transfer(recipient, transfer_amount).call(&mut env.private()); @@ -93,7 +93,7 @@ unconstrained fn transfer_private_failure_more_than_balance() { unconstrained fn transfer_private_failure_on_behalf_of_self_non_zero_nonce() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, owner, recipient, _) = - utils::setup_and_mint( /* with_account_contracts */ false); + utils::setup_and_mint(/* with_account_contracts */ false); // Add authwit let transfer_amount = 1000; let transfer_private_from_call_interface = @@ -112,7 +112,7 @@ unconstrained fn transfer_private_failure_on_behalf_of_self_non_zero_nonce() { unconstrained fn transfer_private_failure_on_behalf_of_more_than_balance() { // Setup with account contracts. Slower since we actually deploy them, but needed for authwits. let (env, token_contract_address, owner, recipient, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ true); + utils::setup_and_mint(/* with_account_contracts */ true); // Add authwit let transfer_amount = mint_amount + 1; let transfer_private_from_call_interface = @@ -132,7 +132,7 @@ unconstrained fn transfer_private_failure_on_behalf_of_more_than_balance() { unconstrained fn transfer_private_failure_on_behalf_of_other_without_approval() { // Setup with account contracts. Slower since we actually deploy them, but needed for authwits. let (env, token_contract_address, owner, recipient, _) = - utils::setup_and_mint( /* with_account_contracts */ true); + utils::setup_and_mint(/* with_account_contracts */ true); // Add authwit let transfer_amount = 1000; let transfer_private_from_call_interface = @@ -147,7 +147,7 @@ unconstrained fn transfer_private_failure_on_behalf_of_other_without_approval() unconstrained fn transfer_private_failure_on_behalf_of_other_wrong_caller() { // Setup with account contracts. Slower since we actually deploy them, but needed for authwits. let (env, token_contract_address, owner, recipient, _) = - utils::setup_and_mint( /* with_account_contracts */ true); + utils::setup_and_mint(/* with_account_contracts */ true); // Add authwit let transfer_amount = 1000; let transfer_private_from_call_interface = diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/test/transfer_public.nr b/noir-projects/noir-contracts/contracts/token_contract/src/test/transfer_public.nr index 829fd1a31a6..72a6eb3f029 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/test/transfer_public.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/test/transfer_public.nr @@ -7,7 +7,7 @@ use dep::aztec::oracle::random::random; unconstrained fn public_transfer() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, owner, recipient, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ false); + utils::setup_and_mint(/* with_account_contracts */ false); // Transfer tokens let transfer_amount = mint_amount / 10; Token::at(token_contract_address).transfer_public(owner, recipient, transfer_amount, 0).call( @@ -23,7 +23,7 @@ unconstrained fn public_transfer() { unconstrained fn public_transfer_to_self() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, owner, _, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ false); + utils::setup_and_mint(/* with_account_contracts */ false); // Transfer tokens let transfer_amount = mint_amount / 10; // docs:start:call_public @@ -39,7 +39,7 @@ unconstrained fn public_transfer_to_self() { unconstrained fn public_transfer_on_behalf_of_other() { // Setup with account contracts. Slower since we actually deploy them, but needed for authwits. let (env, token_contract_address, owner, recipient, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ true); + utils::setup_and_mint(/* with_account_contracts */ true); let transfer_amount = mint_amount / 10; let public_transfer_from_call_interface = Token::at(token_contract_address).transfer_public(owner, recipient, transfer_amount, 1); @@ -61,7 +61,7 @@ unconstrained fn public_transfer_on_behalf_of_other() { unconstrained fn public_transfer_failure_more_than_balance() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, owner, recipient, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ false); + utils::setup_and_mint(/* with_account_contracts */ false); // Transfer tokens let transfer_amount = mint_amount + 1; let public_transfer_call_interface = @@ -74,7 +74,7 @@ unconstrained fn public_transfer_failure_more_than_balance() { unconstrained fn public_transfer_failure_on_behalf_of_self_non_zero_nonce() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, owner, recipient, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ false); + utils::setup_and_mint(/* with_account_contracts */ false); // Transfer tokens let transfer_amount = mint_amount / 10; let public_transfer_call_interface = Token::at(token_contract_address).transfer_public( @@ -96,7 +96,7 @@ unconstrained fn public_transfer_failure_on_behalf_of_self_non_zero_nonce() { unconstrained fn public_transfer_failure_on_behalf_of_other_without_approval() { // Setup with account contracts. Slower since we actually deploy them, but needed for authwits. let (env, token_contract_address, owner, recipient, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ true); + utils::setup_and_mint(/* with_account_contracts */ true); let transfer_amount = mint_amount / 10; let public_transfer_from_call_interface = Token::at(token_contract_address).transfer_public(owner, recipient, transfer_amount, 1); @@ -110,7 +110,7 @@ unconstrained fn public_transfer_failure_on_behalf_of_other_without_approval() { unconstrained fn public_transfer_failure_on_behalf_of_other_more_than_balance() { // Setup with account contracts. Slower since we actually deploy them, but needed for authwits. let (env, token_contract_address, owner, recipient, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ true); + utils::setup_and_mint(/* with_account_contracts */ true); let transfer_amount = mint_amount + 1; // docs:start:public_authwit let public_transfer_from_call_interface = @@ -131,7 +131,7 @@ unconstrained fn public_transfer_failure_on_behalf_of_other_more_than_balance() unconstrained fn public_transfer_failure_on_behalf_of_other_wrong_caller() { // Setup with account contracts. Slower since we actually deploy them, but needed for authwits. let (env, token_contract_address, owner, recipient, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ true); + utils::setup_and_mint(/* with_account_contracts */ true); let transfer_amount = mint_amount / 10; let public_transfer_from_call_interface = Token::at(token_contract_address).transfer_public(owner, recipient, transfer_amount, 1); diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/test/unshielding.nr b/noir-projects/noir-contracts/contracts/token_contract/src/test/unshielding.nr index 60350ca624b..ffe3e55b89c 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/test/unshielding.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/test/unshielding.nr @@ -7,7 +7,7 @@ use dep::aztec::oracle::random::random; unconstrained fn unshield_on_behalf_of_self() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, owner, _, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ false); + utils::setup_and_mint(/* with_account_contracts */ false); let unshield_amount = mint_amount / 10; Token::at(token_contract_address).unshield(owner, owner, unshield_amount, 0).call( @@ -20,7 +20,7 @@ unconstrained fn unshield_on_behalf_of_self() { #[test] unconstrained fn unshield_on_behalf_of_other() { let (env, token_contract_address, owner, recipient, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ true); + utils::setup_and_mint(/* with_account_contracts */ true); let unshield_amount = mint_amount / 10; let unshield_call_interface = @@ -42,7 +42,7 @@ unconstrained fn unshield_on_behalf_of_other() { unconstrained fn unshield_failure_more_than_balance() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, owner, _, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ false); + utils::setup_and_mint(/* with_account_contracts */ false); let unshield_amount = mint_amount + 1; Token::at(token_contract_address).unshield(owner, owner, unshield_amount, 0).call( @@ -54,7 +54,7 @@ unconstrained fn unshield_failure_more_than_balance() { unconstrained fn unshield_failure_on_behalf_of_self_non_zero_nonce() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, owner, _, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ false); + utils::setup_and_mint(/* with_account_contracts */ false); let unshield_amount = mint_amount + 1; Token::at(token_contract_address).unshield(owner, owner, unshield_amount, random()).call( @@ -65,7 +65,7 @@ unconstrained fn unshield_failure_on_behalf_of_self_non_zero_nonce() { #[test(should_fail_with = "Balance too low")] unconstrained fn unshield_failure_on_behalf_of_other_more_than_balance() { let (env, token_contract_address, owner, recipient, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ true); + utils::setup_and_mint(/* with_account_contracts */ true); let unshield_amount = mint_amount + 1; let unshield_call_interface = @@ -84,7 +84,7 @@ unconstrained fn unshield_failure_on_behalf_of_other_more_than_balance() { #[test(should_fail_with = "Authorization not found for message hash")] unconstrained fn unshield_failure_on_behalf_of_other_invalid_designated_caller() { let (env, token_contract_address, owner, recipient, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ true); + utils::setup_and_mint(/* with_account_contracts */ true); let unshield_amount = mint_amount + 1; let unshield_call_interface = @@ -103,7 +103,7 @@ unconstrained fn unshield_failure_on_behalf_of_other_invalid_designated_caller() #[test(should_fail_with = "Authorization not found for message hash")] unconstrained fn unshield_failure_on_behalf_of_other_no_approval() { let (env, token_contract_address, owner, recipient, mint_amount) = - utils::setup_and_mint( /* with_account_contracts */ true); + utils::setup_and_mint(/* with_account_contracts */ true); let unshield_amount = mint_amount + 1; let unshield_call_interface = diff --git a/noir-projects/noir-protocol-circuits/crates/reset-kernel-lib/src/reset/key_validation_hint.nr b/noir-projects/noir-protocol-circuits/crates/reset-kernel-lib/src/reset/key_validation_hint.nr index dd4e3a7bedd..6f67737014b 100644 --- a/noir-projects/noir-protocol-circuits/crates/reset-kernel-lib/src/reset/key_validation_hint.nr +++ b/noir-projects/noir-protocol-circuits/crates/reset-kernel-lib/src/reset/key_validation_hint.nr @@ -1,6 +1,8 @@ +// TODO: For some reason this file requires `IndexedTreeLeafPreimage` but it's unclear why. use dep::types::{ abis::validation_requests::ScopedKeyValidationRequestAndGenerator, - hash::poseidon2_hash_with_separator, scalar::Scalar, traits::Empty, + hash::poseidon2_hash_with_separator, merkle_tree::IndexedTreeLeafPreimage, scalar::Scalar, + traits::Empty, }; use std::embedded_curve_ops::fixed_base_scalar_mul as derive_public_key; diff --git a/noir-projects/scripts/check.sh b/noir-projects/scripts/check.sh new file mode 100755 index 00000000000..d644d966ef6 --- /dev/null +++ b/noir-projects/scripts/check.sh @@ -0,0 +1,9 @@ +#!/usr/bin/env bash +set -eu + +cd $(dirname "$0")/../ + +nargo check --program-dir ./aztec-nr --silence-warnings +nargo check --program-dir ./noir-contracts --silence-warnings +nargo check --program-dir ./noir-protocol-circuits --silence-warnings +nargo check --program-dir ./mock-protocol-circuits --silence-warnings diff --git a/noir-projects/scripts/format.sh b/noir-projects/scripts/format.sh new file mode 100755 index 00000000000..e3eb57c7ecf --- /dev/null +++ b/noir-projects/scripts/format.sh @@ -0,0 +1,9 @@ +#!/usr/bin/env bash +set -eu + +cd $(dirname "$0")/../ + +nargo fmt --program-dir ./aztec-nr +nargo fmt --program-dir ./noir-contracts +nargo fmt --program-dir ./noir-protocol-circuits +nargo fmt --program-dir ./mock-protocol-circuits diff --git a/noir/bb-version b/noir/bb-version index a60476bfe1c..7e750b4ebf3 100644 --- a/noir/bb-version +++ b/noir/bb-version @@ -1 +1 @@ -0.58.0 +0.60.0 diff --git a/noir/noir-repo/Cargo.lock b/noir/noir-repo/Cargo.lock index 3bfffca46e5..dabac0a7570 100644 --- a/noir/noir-repo/Cargo.lock +++ b/noir/noir-repo/Cargo.lock @@ -2762,12 +2762,15 @@ name = "noir_profiler" version = "0.36.0" dependencies = [ "acir", + "bn254_blackbox_solver", "clap", "color-eyre", "const_format", "fm", + "fxhash", "im", "inferno", + "nargo", "noirc_abi", "noirc_artifacts", "noirc_driver", diff --git a/noir/noir-repo/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs b/noir/noir-repo/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs index e06286d179e..fa51caf5155 100644 --- a/noir/noir-repo/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs +++ b/noir/noir-repo/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs @@ -1,3 +1,5 @@ +use std::collections::BTreeSet; + use crate::native_types::Witness; use crate::{AcirField, BlackBoxFunc}; @@ -389,6 +391,16 @@ impl BlackBoxFuncCall { BlackBoxFuncCall::BigIntToLeBytes { outputs, .. } => outputs.to_vec(), } } + + pub fn get_input_witnesses(&self) -> BTreeSet { + let mut result = BTreeSet::new(); + for input in self.get_inputs_vec() { + if let ConstantOrWitnessEnum::Witness(w) = input.input() { + result.insert(w); + } + } + result + } } const ABBREVIATION_LIMIT: usize = 5; diff --git a/noir/noir-repo/acvm-repo/acvm/src/compiler/optimizers/merge_expressions.rs b/noir/noir-repo/acvm-repo/acvm/src/compiler/optimizers/merge_expressions.rs new file mode 100644 index 00000000000..ddf86f60f77 --- /dev/null +++ b/noir/noir-repo/acvm-repo/acvm/src/compiler/optimizers/merge_expressions.rs @@ -0,0 +1,202 @@ +use std::collections::{BTreeMap, BTreeSet, HashMap}; + +use acir::{ + circuit::{brillig::BrilligInputs, directives::Directive, opcodes::BlockId, Circuit, Opcode}, + native_types::{Expression, Witness}, + AcirField, +}; + +pub(crate) struct MergeExpressionsOptimizer { + resolved_blocks: HashMap>, +} + +impl MergeExpressionsOptimizer { + pub(crate) fn new() -> Self { + MergeExpressionsOptimizer { resolved_blocks: HashMap::new() } + } + /// This pass analyzes the circuit and identifies intermediate variables that are + /// only used in two gates. It then merges the gate that produces the + /// intermediate variable into the second one that uses it + /// Note: This pass is only relevant for backends that can handle unlimited width + pub(crate) fn eliminate_intermediate_variable( + &mut self, + circuit: &Circuit, + acir_opcode_positions: Vec, + ) -> (Vec>, Vec) { + // Keep track, for each witness, of the gates that use it + let circuit_inputs = circuit.circuit_arguments(); + self.resolved_blocks = HashMap::new(); + let mut used_witness: BTreeMap> = BTreeMap::new(); + for (i, opcode) in circuit.opcodes.iter().enumerate() { + let witnesses = self.witness_inputs(opcode); + if let Opcode::MemoryInit { block_id, .. } = opcode { + self.resolved_blocks.insert(*block_id, witnesses.clone()); + } + for w in witnesses { + // We do not simplify circuit inputs + if !circuit_inputs.contains(&w) { + used_witness.entry(w).or_default().insert(i); + } + } + } + + let mut modified_gates: HashMap> = HashMap::new(); + let mut new_circuit = Vec::new(); + let mut new_acir_opcode_positions = Vec::new(); + // For each opcode, try to get a target opcode to merge with + for (i, opcode) in circuit.opcodes.iter().enumerate() { + if !matches!(opcode, Opcode::AssertZero(_)) { + new_circuit.push(opcode.clone()); + new_acir_opcode_positions.push(acir_opcode_positions[i]); + continue; + } + let opcode = modified_gates.get(&i).unwrap_or(opcode).clone(); + let mut to_keep = true; + let input_witnesses = self.witness_inputs(&opcode); + for w in input_witnesses.clone() { + let empty_gates = BTreeSet::new(); + let gates_using_w = used_witness.get(&w).unwrap_or(&empty_gates); + // We only consider witness which are used in exactly two arithmetic gates + if gates_using_w.len() == 2 { + let gates_using_w: Vec<_> = gates_using_w.iter().collect(); + let mut b = *gates_using_w[1]; + if b == i { + b = *gates_using_w[0]; + } else { + // sanity check + assert!(i == *gates_using_w[0]); + } + let second_gate = modified_gates.get(&b).unwrap_or(&circuit.opcodes[b]).clone(); + if let (Opcode::AssertZero(expr_define), Opcode::AssertZero(expr_use)) = + (opcode.clone(), second_gate) + { + if let Some(expr) = Self::merge(&expr_use, &expr_define, w) { + // sanity check + assert!(i < b); + modified_gates.insert(b, Opcode::AssertZero(expr)); + to_keep = false; + // Update the 'used_witness' map to account for the merge. + for w2 in Self::expr_wit(&expr_define) { + if !circuit_inputs.contains(&w2) { + let mut v = used_witness[&w2].clone(); + v.insert(b); + v.remove(&i); + used_witness.insert(w2, v); + } + } + // We need to stop here and continue with the next opcode + // because the merge invalidate the current opcode + break; + } + } + } + } + + if to_keep { + if modified_gates.contains_key(&i) { + new_circuit.push(modified_gates[&i].clone()); + } else { + new_circuit.push(opcode.clone()); + } + new_acir_opcode_positions.push(acir_opcode_positions[i]); + } + } + (new_circuit, new_acir_opcode_positions) + } + + fn expr_wit(expr: &Expression) -> BTreeSet { + let mut result = BTreeSet::new(); + result.extend(expr.mul_terms.iter().flat_map(|i| vec![i.1, i.2])); + result.extend(expr.linear_combinations.iter().map(|i| i.1)); + result + } + + fn brillig_input_wit(&self, input: &BrilligInputs) -> BTreeSet { + let mut result = BTreeSet::new(); + match input { + BrilligInputs::Single(expr) => { + result.extend(Self::expr_wit(expr)); + } + BrilligInputs::Array(exprs) => { + for expr in exprs { + result.extend(Self::expr_wit(expr)); + } + } + BrilligInputs::MemoryArray(block_id) => { + let witnesses = self.resolved_blocks.get(block_id).expect("Unknown block id"); + result.extend(witnesses); + } + } + result + } + + // Returns the input witnesses used by the opcode + fn witness_inputs(&self, opcode: &Opcode) -> BTreeSet { + let mut witnesses = BTreeSet::new(); + match opcode { + Opcode::AssertZero(expr) => Self::expr_wit(expr), + Opcode::BlackBoxFuncCall(bb_func) => bb_func.get_input_witnesses(), + Opcode::Directive(Directive::ToLeRadix { a, .. }) => Self::expr_wit(a), + Opcode::MemoryOp { block_id: _, op, predicate } => { + //index et value, et predicate + let mut witnesses = BTreeSet::new(); + witnesses.extend(Self::expr_wit(&op.index)); + witnesses.extend(Self::expr_wit(&op.value)); + if let Some(p) = predicate { + witnesses.extend(Self::expr_wit(p)); + } + witnesses + } + + Opcode::MemoryInit { block_id: _, init, block_type: _ } => { + init.iter().cloned().collect() + } + Opcode::BrilligCall { inputs, .. } => { + for i in inputs { + witnesses.extend(self.brillig_input_wit(i)); + } + witnesses + } + Opcode::Call { id: _, inputs, outputs: _, predicate } => { + for i in inputs { + witnesses.insert(*i); + } + if let Some(p) = predicate { + witnesses.extend(Self::expr_wit(p)); + } + witnesses + } + } + } + + // Merge 'expr' into 'target' via Gaussian elimination on 'w' + // Returns None if the expressions cannot be merged + fn merge( + target: &Expression, + expr: &Expression, + w: Witness, + ) -> Option> { + // Check that the witness is not part of multiplication terms + for m in &target.mul_terms { + if m.1 == w || m.2 == w { + return None; + } + } + for m in &expr.mul_terms { + if m.1 == w || m.2 == w { + return None; + } + } + + for k in &target.linear_combinations { + if k.1 == w { + for i in &expr.linear_combinations { + if i.1 == w { + return Some(target.add_mul(-(k.0 / i.0), expr)); + } + } + } + } + None + } +} diff --git a/noir/noir-repo/acvm-repo/acvm/src/compiler/optimizers/mod.rs b/noir/noir-repo/acvm-repo/acvm/src/compiler/optimizers/mod.rs index e20ad97a108..1947a80dc35 100644 --- a/noir/noir-repo/acvm-repo/acvm/src/compiler/optimizers/mod.rs +++ b/noir/noir-repo/acvm-repo/acvm/src/compiler/optimizers/mod.rs @@ -5,10 +5,12 @@ use acir::{ // mod constant_backpropagation; mod general; +mod merge_expressions; mod redundant_range; mod unused_memory; pub(crate) use general::GeneralOptimizer; +pub(crate) use merge_expressions::MergeExpressionsOptimizer; pub(crate) use redundant_range::RangeOptimizer; use tracing::info; diff --git a/noir/noir-repo/acvm-repo/acvm/src/compiler/transformers/mod.rs b/noir/noir-repo/acvm-repo/acvm/src/compiler/transformers/mod.rs index 4e29681cbed..b11d054a57b 100644 --- a/noir/noir-repo/acvm-repo/acvm/src/compiler/transformers/mod.rs +++ b/noir/noir-repo/acvm-repo/acvm/src/compiler/transformers/mod.rs @@ -10,7 +10,9 @@ mod csat; pub(crate) use csat::CSatTransformer; pub use csat::MIN_EXPRESSION_WIDTH; -use super::{transform_assert_messages, AcirTransformationMap}; +use super::{ + optimizers::MergeExpressionsOptimizer, transform_assert_messages, AcirTransformationMap, +}; /// Applies [`ProofSystemCompiler`][crate::ProofSystemCompiler] specific optimizations to a [`Circuit`]. pub fn transform( @@ -166,6 +168,16 @@ pub(super) fn transform_internal( // The transformer does not add new public inputs ..acir }; - + let mut merge_optimizer = MergeExpressionsOptimizer::new(); + let (opcodes, new_acir_opcode_positions) = + merge_optimizer.eliminate_intermediate_variable(&acir, new_acir_opcode_positions); + // n.b. we do not update current_witness_index after the eliminate_intermediate_variable pass, the real index could be less. + let acir = Circuit { + current_witness_index, + expression_width, + opcodes, + // The optimizer does not add new public inputs + ..acir + }; (acir, new_acir_opcode_positions) } diff --git a/noir/noir-repo/acvm-repo/acvm/src/pwg/brillig.rs b/noir/noir-repo/acvm-repo/acvm/src/pwg/brillig.rs index 5ec3224dbaa..dffa45dbd7a 100644 --- a/noir/noir-repo/acvm-repo/acvm/src/pwg/brillig.rs +++ b/noir/noir-repo/acvm-repo/acvm/src/pwg/brillig.rs @@ -12,7 +12,7 @@ use acir::{ AcirField, }; use acvm_blackbox_solver::BlackBoxFunctionSolver; -use brillig_vm::{FailureReason, MemoryValue, VMStatus, VM}; +use brillig_vm::{BrilligProfilingSamples, FailureReason, MemoryValue, VMStatus, VM}; use serde::{Deserialize, Serialize}; use crate::{pwg::OpcodeNotSolvable, OpcodeResolutionError}; @@ -58,6 +58,7 @@ impl<'b, B: BlackBoxFunctionSolver, F: AcirField> BrilligSolver<'b, F, B> { /// Constructs a solver for a Brillig block given the bytecode and initial /// witness. + #[allow(clippy::too_many_arguments)] pub(crate) fn new_call( initial_witness: &WitnessMap, memory: &HashMap>, @@ -66,9 +67,16 @@ impl<'b, B: BlackBoxFunctionSolver, F: AcirField> BrilligSolver<'b, F, B> { bb_solver: &'b B, acir_index: usize, brillig_function_id: BrilligFunctionId, + profiling_active: bool, ) -> Result> { - let vm = - Self::setup_brillig_vm(initial_witness, memory, inputs, brillig_bytecode, bb_solver)?; + let vm = Self::setup_brillig_vm( + initial_witness, + memory, + inputs, + brillig_bytecode, + bb_solver, + profiling_active, + )?; Ok(Self { vm, acir_index, function_id: brillig_function_id }) } @@ -78,6 +86,7 @@ impl<'b, B: BlackBoxFunctionSolver, F: AcirField> BrilligSolver<'b, F, B> { inputs: &[BrilligInputs], brillig_bytecode: &'b [BrilligOpcode], bb_solver: &'b B, + profiling_active: bool, ) -> Result, OpcodeResolutionError> { // Set input values let mut calldata: Vec = Vec::new(); @@ -125,7 +134,7 @@ impl<'b, B: BlackBoxFunctionSolver, F: AcirField> BrilligSolver<'b, F, B> { // Instantiate a Brillig VM given the solved calldata // along with the Brillig bytecode. - let vm = VM::new(calldata, brillig_bytecode, vec![], bb_solver); + let vm = VM::new(calldata, brillig_bytecode, vec![], bb_solver, profiling_active); Ok(vm) } @@ -203,6 +212,25 @@ impl<'b, B: BlackBoxFunctionSolver, F: AcirField> BrilligSolver<'b, F, B> { self, witness: &mut WitnessMap, outputs: &[BrilligOutputs], + ) -> Result<(), OpcodeResolutionError> { + assert!(!self.vm.is_profiling_active(), "Expected VM profiling to not be active"); + self.finalize_inner(witness, outputs) + } + + pub(crate) fn finalize_with_profiling( + mut self, + witness: &mut WitnessMap, + outputs: &[BrilligOutputs], + ) -> Result> { + assert!(self.vm.is_profiling_active(), "Expected VM profiling to be active"); + self.finalize_inner(witness, outputs)?; + Ok(self.vm.take_profiling_samples()) + } + + fn finalize_inner( + &self, + witness: &mut WitnessMap, + outputs: &[BrilligOutputs], ) -> Result<(), OpcodeResolutionError> { // Finish the Brillig execution by writing the outputs to the witness map let vm_status = self.vm.get_status(); diff --git a/noir/noir-repo/acvm-repo/acvm/src/pwg/mod.rs b/noir/noir-repo/acvm-repo/acvm/src/pwg/mod.rs index c73893ceea6..fa3498da613 100644 --- a/noir/noir-repo/acvm-repo/acvm/src/pwg/mod.rs +++ b/noir/noir-repo/acvm-repo/acvm/src/pwg/mod.rs @@ -168,6 +168,14 @@ impl From for OpcodeResolutionError { } } +pub type ProfilingSamples = Vec; + +#[derive(Default)] +pub struct ProfilingSample { + pub call_stack: Vec, + pub brillig_function_id: Option, +} + pub struct ACVM<'a, F, B: BlackBoxFunctionSolver> { status: ACVMStatus, @@ -198,6 +206,10 @@ pub struct ACVM<'a, F, B: BlackBoxFunctionSolver> { unconstrained_functions: &'a [BrilligBytecode], assertion_payloads: &'a [(OpcodeLocation, AssertionPayload)], + + profiling_active: bool, + + profiling_samples: ProfilingSamples, } impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { @@ -222,9 +234,16 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { acir_call_results: Vec::default(), unconstrained_functions, assertion_payloads, + profiling_active: false, + profiling_samples: Vec::new(), } } + // Enable profiling + pub fn with_profiler(&mut self, profiling_active: bool) { + self.profiling_active = profiling_active; + } + /// Returns a reference to the current state of the ACVM's [`WitnessMap`]. /// /// Once execution has completed, the witness map can be extracted using [`ACVM::finalize`] @@ -246,6 +265,10 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { self.instruction_pointer } + pub fn take_profiling_samples(&mut self) -> ProfilingSamples { + std::mem::take(&mut self.profiling_samples) + } + /// Finalize the ACVM execution, returning the resulting [`WitnessMap`]. pub fn finalize(self) -> WitnessMap { if self.status != ACVMStatus::Solved { @@ -503,6 +526,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { self.backend, self.instruction_pointer, *id, + self.profiling_active, )?, }; @@ -519,7 +543,28 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { } BrilligSolverStatus::Finished => { // Write execution outputs - solver.finalize(&mut self.witness_map, outputs)?; + if self.profiling_active { + let profiling_info = + solver.finalize_with_profiling(&mut self.witness_map, outputs)?; + profiling_info.into_iter().for_each(|sample| { + let mapped = + sample.call_stack.into_iter().map(|loc| OpcodeLocation::Brillig { + acir_index: self.instruction_pointer, + brillig_index: loc, + }); + self.profiling_samples.push(ProfilingSample { + call_stack: std::iter::once(OpcodeLocation::Acir( + self.instruction_pointer, + )) + .chain(mapped) + .collect(), + brillig_function_id: Some(*id), + }); + }); + } else { + solver.finalize(&mut self.witness_map, outputs)?; + } + Ok(None) } } @@ -575,6 +620,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { self.backend, self.instruction_pointer, *id, + self.profiling_active, ); match solver { Ok(solver) => StepResult::IntoBrillig(solver), diff --git a/noir/noir-repo/acvm-repo/brillig_vm/src/lib.rs b/noir/noir-repo/acvm-repo/brillig_vm/src/lib.rs index 062298d5324..89d72c4614b 100644 --- a/noir/noir-repo/acvm-repo/brillig_vm/src/lib.rs +++ b/noir/noir-repo/acvm-repo/brillig_vm/src/lib.rs @@ -63,6 +63,15 @@ pub enum VMStatus { }, } +// A sample for each opcode that was executed. +pub type BrilligProfilingSamples = Vec; + +#[derive(Debug, PartialEq, Eq, Clone)] +pub struct BrilligProfilingSample { + // The call stack when processing a given opcode. + pub call_stack: Vec, +} + #[derive(Debug, PartialEq, Eq, Clone)] /// VM encapsulates the state of the Brillig VM during execution. pub struct VM<'a, F, B: BlackBoxFunctionSolver> { @@ -88,6 +97,10 @@ pub struct VM<'a, F, B: BlackBoxFunctionSolver> { black_box_solver: &'a B, // The solver for big integers bigint_solver: BrilligBigintSolver, + // Flag that determines whether we want to profile VM. + profiling_active: bool, + // Samples for profiling the VM execution. + profiling_samples: BrilligProfilingSamples, } impl<'a, F: AcirField, B: BlackBoxFunctionSolver> VM<'a, F, B> { @@ -97,6 +110,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> VM<'a, F, B> { bytecode: &'a [Opcode], foreign_call_results: Vec>, black_box_solver: &'a B, + profiling_active: bool, ) -> Self { Self { calldata, @@ -109,9 +123,19 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> VM<'a, F, B> { call_stack: Vec::new(), black_box_solver, bigint_solver: Default::default(), + profiling_active, + profiling_samples: Vec::with_capacity(bytecode.len()), } } + pub fn is_profiling_active(&self) -> bool { + self.profiling_active + } + + pub fn take_profiling_samples(&mut self) -> BrilligProfilingSamples { + std::mem::take(&mut self.profiling_samples) + } + /// Updates the current status of the VM. /// Returns the given status. fn status(&mut self, status: VMStatus) -> VMStatus { @@ -196,6 +220,15 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> VM<'a, F, B> { /// Process a single opcode and modify the program counter. pub fn process_opcode(&mut self) -> VMStatus { + if self.profiling_active { + let call_stack: Vec = self.get_call_stack(); + self.profiling_samples.push(BrilligProfilingSample { call_stack }); + } + + self.process_opcode_internal() + } + + fn process_opcode_internal(&mut self) -> VMStatus { let opcode = &self.bytecode[self.program_counter]; match opcode { Opcode::BinaryFieldOp { op, lhs, rhs, destination: result } => { @@ -814,7 +847,7 @@ mod tests { }]; // Start VM - let mut vm = VM::new(calldata, &opcodes, vec![], &StubbedBlackBoxSolver); + let mut vm = VM::new(calldata, &opcodes, vec![], &StubbedBlackBoxSolver, false); let status = vm.process_opcode(); assert_eq!(status, VMStatus::Finished { return_data_offset: 0, return_data_size: 0 }); @@ -864,7 +897,7 @@ mod tests { Opcode::JumpIf { condition: destination, location: 6 }, ]; - let mut vm = VM::new(calldata, &opcodes, vec![], &StubbedBlackBoxSolver); + let mut vm = VM::new(calldata, &opcodes, vec![], &StubbedBlackBoxSolver, false); let status = vm.process_opcode(); assert_eq!(status, VMStatus::InProgress); @@ -932,7 +965,7 @@ mod tests { }, ]; - let mut vm = VM::new(calldata, &opcodes, vec![], &StubbedBlackBoxSolver); + let mut vm = VM::new(calldata, &opcodes, vec![], &StubbedBlackBoxSolver, false); let status = vm.process_opcode(); assert_eq!(status, VMStatus::InProgress); @@ -995,7 +1028,7 @@ mod tests { }, Opcode::Stop { return_data_offset: 1, return_data_size: 1 }, ]; - let mut vm = VM::new(calldata, opcodes, vec![], &StubbedBlackBoxSolver); + let mut vm = VM::new(calldata, opcodes, vec![], &StubbedBlackBoxSolver, false); let status = vm.process_opcode(); assert_eq!(status, VMStatus::InProgress); @@ -1046,7 +1079,7 @@ mod tests { }, Opcode::Stop { return_data_offset: 1, return_data_size: 1 }, ]; - let mut vm = VM::new(calldata, opcodes, vec![], &StubbedBlackBoxSolver); + let mut vm = VM::new(calldata, opcodes, vec![], &StubbedBlackBoxSolver, false); let status = vm.process_opcode(); assert_eq!(status, VMStatus::InProgress); @@ -1092,7 +1125,7 @@ mod tests { }, Opcode::Mov { destination: MemoryAddress::direct(2), source: MemoryAddress::direct(0) }, ]; - let mut vm = VM::new(calldata, opcodes, vec![], &StubbedBlackBoxSolver); + let mut vm = VM::new(calldata, opcodes, vec![], &StubbedBlackBoxSolver, false); let status = vm.process_opcode(); assert_eq!(status, VMStatus::InProgress); @@ -1157,7 +1190,7 @@ mod tests { condition: MemoryAddress::direct(1), }, ]; - let mut vm = VM::new(calldata, opcodes, vec![], &StubbedBlackBoxSolver); + let mut vm = VM::new(calldata, opcodes, vec![], &StubbedBlackBoxSolver, false); let status = vm.process_opcode(); assert_eq!(status, VMStatus::InProgress); @@ -1253,7 +1286,7 @@ mod tests { .chain(cast_opcodes) .chain([equal_opcode, not_equal_opcode, less_than_opcode, less_than_equal_opcode]) .collect(); - let mut vm = VM::new(calldata, &opcodes, vec![], &StubbedBlackBoxSolver); + let mut vm = VM::new(calldata, &opcodes, vec![], &StubbedBlackBoxSolver, false); // Calldata copy let status = vm.process_opcode(); @@ -1381,7 +1414,7 @@ mod tests { value: FieldElement::from(27_usize), }, ]; - let mut vm = VM::new(vec![], opcodes, vec![], &StubbedBlackBoxSolver); + let mut vm = VM::new(vec![], opcodes, vec![], &StubbedBlackBoxSolver, false); let status = vm.process_opcode(); assert_eq!(status, VMStatus::InProgress); @@ -1605,7 +1638,7 @@ mod tests { calldata: Vec, opcodes: &[Opcode], ) -> VM<'_, F, StubbedBlackBoxSolver> { - let mut vm = VM::new(calldata, opcodes, vec![], &StubbedBlackBoxSolver); + let mut vm = VM::new(calldata, opcodes, vec![], &StubbedBlackBoxSolver, false); brillig_execute(&mut vm); assert_eq!(vm.call_stack, vec![]); vm @@ -2284,7 +2317,7 @@ mod tests { }, ]; - let mut vm = VM::new(calldata, &opcodes, vec![], &StubbedBlackBoxSolver); + let mut vm = VM::new(calldata, &opcodes, vec![], &StubbedBlackBoxSolver, false); vm.process_opcode(); vm.process_opcode(); diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir.rs index 1f8260236f8..0c6097b8f6d 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir.rs @@ -233,7 +233,8 @@ pub(crate) mod tests { calldata: Vec, bytecode: &[BrilligOpcode], ) -> (VM<'_, FieldElement, DummyBlackBoxSolver>, usize, usize) { - let mut vm = VM::new(calldata, bytecode, vec![], &DummyBlackBoxSolver); + let profiling_active = false; + let mut vm = VM::new(calldata, bytecode, vec![], &DummyBlackBoxSolver, profiling_active); let status = vm.process_opcodes(); if let VMStatus::Finished { return_data_offset, return_data_size } = status { @@ -311,6 +312,7 @@ pub(crate) mod tests { &bytecode, vec![ForeignCallResult { values: vec![ForeignCallParam::Array(number_sequence)] }], &DummyBlackBoxSolver, + false, ); let status = vm.process_opcodes(); assert_eq!(status, VMStatus::Finished { return_data_offset: 0, return_data_size: 0 }); diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index db08b906185..811b728a9d5 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -2195,7 +2195,8 @@ fn execute_brillig( // We pass a stubbed solver here as a concrete solver implies a field choice which conflicts with this function // being generic. let solver = acvm::blackbox_solver::StubbedBlackBoxSolver; - let mut vm = VM::new(calldata, code, Vec::new(), &solver); + let profiling_active = false; + let mut vm = VM::new(calldata, code, Vec::new(), &solver, profiling_active); // Run the Brillig VM on these inputs, bytecode, etc! let vm_status = vm.process_opcodes(); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs index 641e2d1d57e..9f56b72f4e9 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs @@ -1041,11 +1041,14 @@ impl<'context> Elaborator<'context> { self.file = trait_impl.file_id; self.local_module = trait_impl.module_id; - self.check_parent_traits_are_implemented(&trait_impl); - - self.generics = trait_impl.resolved_generics; + self.generics = trait_impl.resolved_generics.clone(); self.current_trait_impl = trait_impl.impl_id; + self.add_trait_impl_assumed_trait_implementations(trait_impl.impl_id); + self.check_trait_impl_where_clause_matches_trait_where_clause(&trait_impl); + self.check_parent_traits_are_implemented(&trait_impl); + self.remove_trait_impl_assumed_trait_implementations(trait_impl.impl_id); + for (module, function, _) in &trait_impl.methods.functions { self.local_module = *module; let errors = check_trait_impl_method_matches_declaration(self.interner, *function); @@ -1059,6 +1062,95 @@ impl<'context> Elaborator<'context> { self.generics.clear(); } + fn add_trait_impl_assumed_trait_implementations(&mut self, impl_id: Option) { + if let Some(impl_id) = impl_id { + if let Some(trait_implementation) = self.interner.try_get_trait_implementation(impl_id) + { + for trait_constrain in &trait_implementation.borrow().where_clause { + let trait_bound = &trait_constrain.trait_bound; + self.interner.add_assumed_trait_implementation( + trait_constrain.typ.clone(), + trait_bound.trait_id, + trait_bound.trait_generics.clone(), + ); + } + } + } + } + + fn remove_trait_impl_assumed_trait_implementations(&mut self, impl_id: Option) { + if let Some(impl_id) = impl_id { + if let Some(trait_implementation) = self.interner.try_get_trait_implementation(impl_id) + { + for trait_constrain in &trait_implementation.borrow().where_clause { + self.interner.remove_assumed_trait_implementations_for_trait( + trait_constrain.trait_bound.trait_id, + ); + } + } + } + } + + fn check_trait_impl_where_clause_matches_trait_where_clause( + &mut self, + trait_impl: &UnresolvedTraitImpl, + ) { + let Some(trait_id) = trait_impl.trait_id else { + return; + }; + + let Some(the_trait) = self.interner.try_get_trait(trait_id) else { + return; + }; + + if the_trait.where_clause.is_empty() { + return; + } + + let impl_trait = the_trait.name.to_string(); + let the_trait_file = the_trait.location.file; + + let mut bindings = TypeBindings::new(); + bind_ordered_generics( + &the_trait.generics, + &trait_impl.resolved_trait_generics, + &mut bindings, + ); + + // Check that each of the trait's where clause constraints is satisfied + for trait_constraint in the_trait.where_clause.clone() { + let Some(trait_constraint_trait) = + self.interner.try_get_trait(trait_constraint.trait_bound.trait_id) + else { + continue; + }; + + let trait_constraint_type = trait_constraint.typ.substitute(&bindings); + let trait_bound = &trait_constraint.trait_bound; + + if self + .interner + .try_lookup_trait_implementation( + &trait_constraint_type, + trait_bound.trait_id, + &trait_bound.trait_generics.ordered, + &trait_bound.trait_generics.named, + ) + .is_err() + { + let missing_trait = + format!("{}{}", trait_constraint_trait.name, trait_bound.trait_generics); + self.push_err(ResolverError::TraitNotImplemented { + impl_trait: impl_trait.clone(), + missing_trait, + type_missing_trait: trait_constraint_type.to_string(), + span: trait_impl.object_type.span, + missing_trait_location: Location::new(trait_bound.span, the_trait_file), + }); + } + } + } + fn check_parent_traits_are_implemented(&mut self, trait_impl: &UnresolvedTraitImpl) { let Some(trait_id) = trait_impl.trait_id else { return; @@ -1182,7 +1274,7 @@ impl<'context> Elaborator<'context> { trait_id, trait_generics, file: trait_impl.file_id, - where_clause: where_clause.clone(), + where_clause, methods, }); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/traits.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/traits.rs index e877682972c..ae278616e03 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/traits.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/traits.rs @@ -32,6 +32,9 @@ impl<'context> Elaborator<'context> { &resolved_generics, ); + let where_clause = + this.resolve_trait_constraints(&unresolved_trait.trait_def.where_clause); + // Each associated type in this trait is also an implicit generic for associated_type in &this.interner.get_trait(*trait_id).associated_types { this.generics.push(associated_type.clone()); @@ -48,6 +51,7 @@ impl<'context> Elaborator<'context> { this.interner.update_trait(*trait_id, |trait_def| { trait_def.set_methods(methods); trait_def.set_trait_bounds(resolved_trait_bounds); + trait_def.set_where_clause(where_clause); }); }); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir_def/traits.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir_def/traits.rs index 534805c2dad..6fd3c4f7a24 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir_def/traits.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir_def/traits.rs @@ -75,6 +75,8 @@ pub struct Trait { /// The resolved trait bounds (for example in `trait Foo: Bar + Baz`, this would be `Bar + Baz`) pub trait_bounds: Vec, + + pub where_clause: Vec, } #[derive(Debug)] @@ -154,6 +156,10 @@ impl Trait { self.trait_bounds = trait_bounds; } + pub fn set_where_clause(&mut self, where_clause: Vec) { + self.where_clause = where_clause; + } + pub fn find_method(&self, name: &str) -> Option { for (idx, method) in self.methods.iter().enumerate() { if &method.name == name { diff --git a/noir/noir-repo/compiler/noirc_frontend/src/node_interner.rs b/noir/noir-repo/compiler/noirc_frontend/src/node_interner.rs index ca7e0c6aa59..5fe88ed4e23 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/node_interner.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/node_interner.rs @@ -735,6 +735,7 @@ impl NodeInterner { method_ids: unresolved_trait.method_ids.clone(), associated_types, trait_bounds: Vec::new(), + where_clause: Vec::new(), }; self.traits.insert(type_id, new_trait); @@ -1874,8 +1875,33 @@ impl NodeInterner { /// Removes all TraitImplKind::Assumed from the list of known impls for the given trait pub fn remove_assumed_trait_implementations_for_trait(&mut self, trait_id: TraitId) { + self.remove_assumed_trait_implementations_for_trait_and_parents(trait_id, trait_id); + } + + fn remove_assumed_trait_implementations_for_trait_and_parents( + &mut self, + trait_id: TraitId, + starting_trait_id: TraitId, + ) { let entries = self.trait_implementation_map.entry(trait_id).or_default(); entries.retain(|(_, kind)| matches!(kind, TraitImplKind::Normal(_))); + + // Also remove assumed implementations for the parent traits, if any + if let Some(trait_bounds) = + self.try_get_trait(trait_id).map(|the_trait| the_trait.trait_bounds.clone()) + { + for parent_trait_bound in trait_bounds { + // Avoid looping forever in case there are cycles + if parent_trait_bound.trait_id == starting_trait_id { + continue; + } + + self.remove_assumed_trait_implementations_for_trait_and_parents( + parent_trait_bound.trait_id, + starting_trait_id, + ); + } + } } /// Tags the given identifier with the selected trait_impl so that monomorphization diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests.rs index ce4ad4d1bb9..e06881127fd 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests.rs @@ -1,7 +1,9 @@ #![cfg(test)] +mod aliases; mod bound_checks; mod imports; +mod metaprogramming; mod name_shadowing; mod references; mod traits; @@ -2969,9 +2971,7 @@ fn uses_self_type_in_trait_where_clause() { } } - struct Bar { - - } + struct Bar {} impl Foo for Bar { @@ -2983,12 +2983,17 @@ fn uses_self_type_in_trait_where_clause() { "#; let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); + assert_eq!(errors.len(), 2); + + let CompilationError::ResolverError(ResolverError::TraitNotImplemented { .. }) = &errors[0].0 + else { + panic!("Expected a trait not implemented error, got {:?}", errors[0].0); + }; let CompilationError::TypeError(TypeCheckError::UnresolvedMethodCall { method_name, .. }) = - &errors[0].0 + &errors[1].0 else { - panic!("Expected an unresolved method call error, got {:?}", errors[0].0); + panic!("Expected an unresolved method call error, got {:?}", errors[1].0); }; assert_eq!(method_name, "trait_func"); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests/aliases.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests/aliases.rs new file mode 100644 index 00000000000..2ca460ca635 --- /dev/null +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests/aliases.rs @@ -0,0 +1,52 @@ +use super::assert_no_errors; + +#[test] +fn allows_usage_of_type_alias_as_argument_type() { + let src = r#" + type Foo = Field; + + fn accepts_a_foo(x: Foo) { + assert_eq(x, 42); + } + + fn main() { + accepts_a_foo(42); + } + "#; + assert_no_errors(src); +} + +#[test] +fn allows_usage_of_type_alias_as_return_type() { + let src = r#" + type Foo = Field; + + fn returns_a_foo() -> Foo { + 42 + } + + fn main() { + let _ = returns_a_foo(); + } + "#; + assert_no_errors(src); +} + +// This is a regression test for https://github.com/noir-lang/noir/issues/6347 +#[test] +#[should_panic = r#"ResolverError(Expected { span: Span(Span { start: ByteIndex(95), end: ByteIndex(98) }), expected: "type", got: "type alias" }"#] +fn allows_destructuring_a_type_alias_of_a_struct() { + let src = r#" + struct Foo { + inner: Field + } + + type Bar = Foo; + + fn main() { + let Bar { inner } = Foo { inner: 42 }; + assert_eq(inner, 42); + } + "#; + assert_no_errors(src); +} diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests/metaprogramming.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests/metaprogramming.rs index ec52310b3d6..a5241cc26ed 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests/metaprogramming.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests/metaprogramming.rs @@ -1,6 +1,9 @@ -use crate::hir::def_collector::dc_crate::CompilationError; +use crate::hir::{ + def_collector::dc_crate::CompilationError, resolution::errors::ResolverError, + type_check::TypeCheckError, +}; -use super::get_program_errors; +use super::{assert_no_errors, get_program_errors}; // Regression for #5388 #[test] @@ -74,3 +77,22 @@ fn unquoted_integer_as_integer_token() { assert_no_errors(src); } + +#[test] +fn allows_references_to_structs_generated_by_macros() { + let src = r#" + comptime fn make_new_struct(_s: StructDefinition) -> Quoted { + quote { struct Bar {} } + } + + #[make_new_struct] + struct Foo {} + + fn main() { + let _ = Foo {}; + let _ = Bar {}; + } + "#; + + assert_no_errors(src); +} diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests/traits.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests/traits.rs index ee84cc0e890..88138ecde4d 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests/traits.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests/traits.rs @@ -148,3 +148,116 @@ fn trait_inheritance_missing_parent_implementation() { assert_eq!(typ, "Struct"); assert_eq!(impl_trait, "Bar"); } + +#[test] +fn errors_on_unknown_type_in_trait_where_clause() { + let src = r#" + pub trait Foo where T: Unknown {} + + fn main() { + } + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); +} + +#[test] +fn does_not_error_if_impl_trait_constraint_is_satisfied_for_concrete_type() { + let src = r#" + pub trait Greeter { + fn greet(self); + } + + pub trait Foo + where + T: Greeter, + { + fn greet(object: U) + where + U: Greeter, + { + object.greet(); + } + } + + pub struct SomeGreeter; + impl Greeter for SomeGreeter { + fn greet(self) {} + } + + pub struct Bar; + + impl Foo for Bar {} + + fn main() {} + "#; + assert_no_errors(src); +} + +#[test] +fn does_not_error_if_impl_trait_constraint_is_satisfied_for_type_variable() { + let src = r#" + pub trait Greeter { + fn greet(self); + } + + pub trait Foo where T: Greeter { + fn greet(object: T) { + object.greet(); + } + } + + pub struct Bar; + + impl Foo for Bar where T: Greeter { + } + + fn main() { + } + "#; + assert_no_errors(src); +} +#[test] +fn errors_if_impl_trait_constraint_is_not_satisfied() { + let src = r#" + pub trait Greeter { + fn greet(self); + } + + pub trait Foo + where + T: Greeter, + { + fn greet(object: U) + where + U: Greeter, + { + object.greet(); + } + } + + pub struct SomeGreeter; + + pub struct Bar; + + impl Foo for Bar {} + + fn main() {} + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + let CompilationError::ResolverError(ResolverError::TraitNotImplemented { + impl_trait, + missing_trait: the_trait, + type_missing_trait: typ, + .. + }) = &errors[0].0 + else { + panic!("Expected a TraitNotImplemented error, got {:?}", &errors[0].0); + }; + + assert_eq!(the_trait, "Greeter"); + assert_eq!(typ, "SomeGreeter"); + assert_eq!(impl_trait, "Foo"); +} diff --git a/noir/noir-repo/docs/versioned_docs/version-v0.36.0/how_to/_category_.json b/noir/noir-repo/docs/versioned_docs/version-v0.36.0/how_to/_category_.json index 360a8ad77f0..0b4f367ce53 100644 --- a/noir/noir-repo/docs/versioned_docs/version-v0.36.0/how_to/_category_.json +++ b/noir/noir-repo/docs/versioned_docs/version-v0.36.0/how_to/_category_.json @@ -1,6 +1,5 @@ { - "label": "Modules, Packages and Crates", - "position": 2, + "position": 1, "collapsible": true, "collapsed": true } diff --git a/noir/noir-repo/docs/versioned_docs/version-v0.36.0/noir/concepts/data_types/_category_.json b/noir/noir-repo/docs/versioned_docs/version-v0.36.0/noir/concepts/data_types/_category_.json index 5b6a20a609a..5d694210bbf 100644 --- a/noir/noir-repo/docs/versioned_docs/version-v0.36.0/noir/concepts/data_types/_category_.json +++ b/noir/noir-repo/docs/versioned_docs/version-v0.36.0/noir/concepts/data_types/_category_.json @@ -1,5 +1,5 @@ { - "position": 4, + "position": 0, "collapsible": true, "collapsed": true } diff --git a/noir/noir-repo/docs/versioned_docs/version-v0.36.0/noir/standard_library/cryptographic_primitives/_category_.json b/noir/noir-repo/docs/versioned_docs/version-v0.36.0/noir/standard_library/cryptographic_primitives/_category_.json index 7da08f8a8c5..5d694210bbf 100644 --- a/noir/noir-repo/docs/versioned_docs/version-v0.36.0/noir/standard_library/cryptographic_primitives/_category_.json +++ b/noir/noir-repo/docs/versioned_docs/version-v0.36.0/noir/standard_library/cryptographic_primitives/_category_.json @@ -1,6 +1,5 @@ { - "label": "Concepts", "position": 0, "collapsible": true, "collapsed": true -} \ No newline at end of file +} diff --git a/noir/noir-repo/noir_stdlib/src/hash/sha256.nr b/noir/noir-repo/noir_stdlib/src/hash/sha256.nr index b5d2624d3e7..a3ac0b9e5da 100644 --- a/noir/noir-repo/noir_stdlib/src/hash/sha256.nr +++ b/noir/noir-repo/noir_stdlib/src/hash/sha256.nr @@ -12,6 +12,20 @@ global MSG_SIZE_PTR = 56; // Size of the message block when packed as 4-byte integer array. global INT_BLOCK_SIZE = 16; +// A `u32` integer consists of 4 bytes. +global INT_SIZE = 4; + +// Index of the integer in the `INT_BLOCK` where the length is written. +global INT_SIZE_PTR = MSG_SIZE_PTR / INT_SIZE; + +// Magic numbers for bit shifting. +// Works with actual bit shifting as well as the compiler turns them into * and / +// but circuit execution appears to be 10% faster this way. +global TWO_POW_8 = 256; +global TWO_POW_16 = TWO_POW_8 * 256; +global TWO_POW_24 = TWO_POW_16 * 256; +global TWO_POW_32 = TWO_POW_24 as u64 * 256; + // Index of a byte in a 64 byte block; ie. 0..=63 type BLOCK_BYTE_PTR = u32; @@ -19,8 +33,8 @@ type BLOCK_BYTE_PTR = u32; type INT_BLOCK = [u32; INT_BLOCK_SIZE]; // A message block is a slice of the original message of a fixed size, -// potentially padded with zeroes. -type MSG_BLOCK = [u8; BLOCK_SIZE]; +// potentially padded with zeros, with neighbouring 4 bytes packed into integers. +type MSG_BLOCK = INT_BLOCK; // The hash is 32 bytes. type HASH = [u8; 32]; @@ -50,16 +64,19 @@ pub fn digest(msg: [u8; N]) -> HASH { pub fn sha256_var(msg: [u8; N], message_size: u64) -> HASH { let message_size = message_size as u32; let num_blocks = N / BLOCK_SIZE; - let mut msg_block: MSG_BLOCK = [0; BLOCK_SIZE]; + let mut msg_block: MSG_BLOCK = [0; INT_BLOCK_SIZE]; + // Intermediate hash, starting with the canonical initial value let mut h: STATE = [ 1779033703, 3144134277, 1013904242, 2773480762, 1359893119, 2600822924, 528734635, 1541459225, - ]; // Intermediate hash, starting with the canonical initial value - let mut msg_byte_ptr = 0; // Pointer into msg_block + ]; + // Pointer into msg_block on a 64 byte scale + let mut msg_byte_ptr = 0; for i in 0..num_blocks { let msg_start = BLOCK_SIZE * i; let (new_msg_block, new_msg_byte_ptr) = unsafe { build_msg_block(msg, message_size, msg_start) }; + if msg_start < message_size { msg_block = new_msg_block; } @@ -77,7 +94,7 @@ pub fn sha256_var(msg: [u8; N], message_size: u64) -> HASH { // If the block is filled, compress it. // An un-filled block is handled after this loop. if (msg_start < message_size) & (msg_byte_ptr == BLOCK_SIZE) { - h = sha256_compression(msg_u8_to_u32(msg_block), h); + h = sha256_compression(msg_block, h); } } @@ -114,49 +131,39 @@ pub fn sha256_var(msg: [u8; N], message_size: u64) -> HASH { // Pad the rest such that we have a [u32; 2] block at the end representing the length // of the message, and a block of 1 0 ... 0 following the message (i.e. [1 << 7, 0, ..., 0]). // Here we rely on the fact that everything beyond the available input is set to 0. - msg_block[msg_byte_ptr] = 1 << 7; - let last_block = msg_block; + msg_block = update_block_item( + msg_block, + msg_byte_ptr, + |msg_item| set_item_byte_then_zeros(msg_item, msg_byte_ptr, 1 << 7), + ); msg_byte_ptr = msg_byte_ptr + 1; + let last_block = msg_block; // If we don't have room to write the size, compress the block and reset it. if msg_byte_ptr > MSG_SIZE_PTR { - h = sha256_compression(msg_u8_to_u32(msg_block), h); + h = sha256_compression(msg_block, h); // `attach_len_to_msg_block` will zero out everything after the `msg_byte_ptr`. msg_byte_ptr = 0; } msg_block = unsafe { attach_len_to_msg_block(msg_block, msg_byte_ptr, message_size) }; - if !crate::runtime::is_unconstrained() { + if !is_unconstrained() { verify_msg_len(msg_block, last_block, msg_byte_ptr, message_size); } hash_final_block(msg_block, h) } -// Convert 64-byte array to array of 16 u32s -fn msg_u8_to_u32(msg: MSG_BLOCK) -> INT_BLOCK { - let mut msg32: INT_BLOCK = [0; INT_BLOCK_SIZE]; - - for i in 0..INT_BLOCK_SIZE { - let mut msg_field: Field = 0; - for j in 0..4 { - msg_field = msg_field * 256 + msg[64 - 4 * (i + 1) + j] as Field; - } - msg32[15 - i] = msg_field as u32; - } - - msg32 -} - // Take `BLOCK_SIZE` number of bytes from `msg` starting at `msg_start`. -// Returns the block and the length that has been copied rather than padded with zeroes. +// Returns the block and the length that has been copied rather than padded with zeros. unconstrained fn build_msg_block( msg: [u8; N], message_size: u32, msg_start: u32, ) -> (MSG_BLOCK, BLOCK_BYTE_PTR) { - let mut msg_block: MSG_BLOCK = [0; BLOCK_SIZE]; + let mut msg_block: MSG_BLOCK = [0; INT_BLOCK_SIZE]; + // We insert `BLOCK_SIZE` bytes (or up to the end of the message) let block_input = if msg_start + BLOCK_SIZE > message_size { if message_size < msg_start { @@ -169,29 +176,73 @@ unconstrained fn build_msg_block( } else { BLOCK_SIZE }; - for k in 0..block_input { - msg_block[k] = msg[msg_start + k]; + + // Figure out the number of items in the int array that we have to pack. + // e.g. if the input is [0,1,2,3,4,5] then we need to pack it as 2 items: [0123, 4500] + let mut int_input = block_input / INT_SIZE; + if block_input % INT_SIZE != 0 { + int_input = int_input + 1; + }; + + for i in 0..int_input { + let mut msg_item: u32 = 0; + // Always construct the integer as 4 bytes, even if it means going beyond the input. + for j in 0..INT_SIZE { + let k = i * INT_SIZE + j; + let msg_byte = if k < block_input { + msg[msg_start + k] + } else { + 0 + }; + msg_item = lshift8(msg_item, 1) + msg_byte as u32; + } + msg_block[i] = msg_item; } + + // Returning the index as if it was a 64 byte array. + // We have to project it down to 16 items and bit shifting to get a byte back if we need it. (msg_block, block_input) } // Verify the block we are compressing was appropriately constructed by `build_msg_block` // and matches the input data. Returns the index of the first unset item. +// If `message_size` is less than `msg_start` then this is called with the old non-empty block; +// in that case we can skip verification, ie. no need to check that everything is zero. fn verify_msg_block( msg: [u8; N], message_size: u32, msg_block: MSG_BLOCK, msg_start: u32, ) -> BLOCK_BYTE_PTR { - let mut msg_byte_ptr: u32 = 0; // Message byte pointer + let mut msg_byte_ptr = 0; let mut msg_end = msg_start + BLOCK_SIZE; if msg_end > N { msg_end = N; } + // We might have to go beyond the input to pad the fields. + if msg_end % INT_SIZE != 0 { + msg_end = msg_end + INT_SIZE - msg_end % INT_SIZE; + } - for k in msg_start..msg_end { - if k < message_size { - assert_eq(msg_block[msg_byte_ptr], msg[k]); + // Reconstructed packed item. + let mut msg_item: u32 = 0; + + // Inclusive at the end so that we can compare the last item. + let mut i: u32 = 0; + for k in msg_start..=msg_end { + if k % INT_SIZE == 0 { + // If we consumed some input we can compare against the block. + if (msg_start < message_size) & (k > msg_start) { + assert_eq(msg_block[i], msg_item as u32); + i = i + 1; + msg_item = 0; + } + } + // Shift the accumulator + msg_item = lshift8(msg_item, 1); + // If we have input to consume, add it at the rightmost position. + if k < message_size & k < msg_end { + msg_item = msg_item + msg[k] as u32; msg_byte_ptr = msg_byte_ptr + 1; } } @@ -199,69 +250,261 @@ fn verify_msg_block( msg_byte_ptr } -// Verify the block we are compressing was appropriately padded with zeroes by `build_msg_block`. +// Verify the block we are compressing was appropriately padded with zeros by `build_msg_block`. // This is only relevant for the last, potentially partially filled block. fn verify_msg_block_padding(msg_block: MSG_BLOCK, msg_byte_ptr: BLOCK_BYTE_PTR) { + // Check all the way to the end of the block. + verify_msg_block_zeros(msg_block, msg_byte_ptr, INT_BLOCK_SIZE); +} + +// Verify that a region of ints in the message block are (partially) zeroed, +// up to an (exclusive) maximum which can either be the end of the block +// or just where the size is to be written. +fn verify_msg_block_zeros( + msg_block: MSG_BLOCK, + mut msg_byte_ptr: BLOCK_BYTE_PTR, + max_int_byte_ptr: u32, +) { // This variable is used to get around the compiler under-constrained check giving a warning. // We want to check against a constant zero, but if it does not come from the circuit inputs // or return values the compiler check will issue a warning. let zero = msg_block[0] - msg_block[0]; - for i in 0..BLOCK_SIZE { - if i >= msg_byte_ptr { + // First integer which is supposed to be (partially) zero. + let mut int_byte_ptr = msg_byte_ptr / INT_SIZE; + + // Check partial zeros. + let modulo = msg_byte_ptr % INT_SIZE; + if modulo != 0 { + let zeros = INT_SIZE - modulo; + let mask = if zeros == 3 { + TWO_POW_24 + } else if zeros == 2 { + TWO_POW_16 + } else { + TWO_POW_8 + }; + assert_eq(msg_block[int_byte_ptr] % mask, zero); + int_byte_ptr = int_byte_ptr + 1; + } + + // Check the rest of the items. + for i in 0..max_int_byte_ptr { + if i >= int_byte_ptr { assert_eq(msg_block[i], zero); } } } +// Verify that up to the byte pointer the two blocks are equal. +// At the byte pointer the new block can be partially zeroed. +fn verify_msg_block_equals_last( + msg_block: MSG_BLOCK, + last_block: MSG_BLOCK, + mut msg_byte_ptr: BLOCK_BYTE_PTR, +) { + // msg_byte_ptr is the position at which they are no longer have to be the same. + // First integer which is supposed to be (partially) zero contains that pointer. + let mut int_byte_ptr = msg_byte_ptr / INT_SIZE; + + // Check partial zeros. + let modulo = msg_byte_ptr % INT_SIZE; + if modulo != 0 { + // Reconstruct the partially zero item from the last block. + let last_field = last_block[int_byte_ptr]; + let mut msg_item: u32 = 0; + // Reset to where they are still equal. + msg_byte_ptr = msg_byte_ptr - modulo; + for i in 0..INT_SIZE { + msg_item = lshift8(msg_item, 1); + if i < modulo { + msg_item = msg_item + get_item_byte(last_field, msg_byte_ptr) as u32; + msg_byte_ptr = msg_byte_ptr + 1; + } + } + assert_eq(msg_block[int_byte_ptr], msg_item); + } + + for i in 0..INT_SIZE_PTR { + if i < int_byte_ptr { + assert_eq(msg_block[i], last_block[i]); + } + } +} + +// Apply a function on the block item which the pointer indicates. +fn update_block_item( + mut msg_block: MSG_BLOCK, + msg_byte_ptr: BLOCK_BYTE_PTR, + f: fn[Env](u32) -> u32, +) -> MSG_BLOCK { + let i = msg_byte_ptr / INT_SIZE; + msg_block[i] = f(msg_block[i]); + msg_block +} + +// Set the rightmost `zeros` number of bytes to 0. +fn set_item_zeros(item: u32, zeros: u8) -> u32 { + lshift8(rshift8(item, zeros), zeros) +} + +// Replace one byte in the item with a value, and set everything after it to zero. +fn set_item_byte_then_zeros(msg_item: u32, msg_byte_ptr: BLOCK_BYTE_PTR, msg_byte: u8) -> u32 { + let zeros = INT_SIZE - msg_byte_ptr % INT_SIZE; + let zeroed_item = set_item_zeros(msg_item, zeros as u8); + let new_item = byte_into_item(msg_byte, msg_byte_ptr); + zeroed_item + new_item +} + +// Get a byte of a message item according to its overall position in the `BLOCK_SIZE` space. +fn get_item_byte(mut msg_item: u32, msg_byte_ptr: BLOCK_BYTE_PTR) -> u8 { + // How many times do we have to shift to the right to get to the position we want? + let max_shifts = INT_SIZE - 1; + let shifts = max_shifts - msg_byte_ptr % INT_SIZE; + msg_item = rshift8(msg_item, shifts as u8); + // At this point the byte we want is in the rightmost position. + msg_item as u8 +} + +// Project a byte into a position in a field based on the overall block pointer. +// For example putting 1 into pointer 5 would be 100, because overall we would +// have [____, 0100] with indexes [0123,4567]. +fn byte_into_item(msg_byte: u8, msg_byte_ptr: BLOCK_BYTE_PTR) -> u32 { + let mut msg_item = msg_byte as u32; + // How many times do we have to shift to the left to get to the position we want? + let max_shifts = INT_SIZE - 1; + let shifts = max_shifts - msg_byte_ptr % INT_SIZE; + lshift8(msg_item, shifts as u8) +} + +// Construct a field out of 4 bytes. +fn make_item(b0: u8, b1: u8, b2: u8, b3: u8) -> u32 { + let mut item = b0 as u32; + item = lshift8(item, 1) + b1 as u32; + item = lshift8(item, 1) + b2 as u32; + item = lshift8(item, 1) + b3 as u32; + item +} + +// Shift by 8 bits to the left between 0 and 4 times. +// Checks `is_unconstrained()` to just use a bitshift if we're running in an unconstrained context, +// otherwise multiplies by 256. +fn lshift8(item: u32, shifts: u8) -> u32 { + if is_unconstrained() { + if item == 0 { + 0 + } else { + // Brillig wouldn't shift 0<<4 without overflow. + item << (8 * shifts) + } + } else { + // We can do a for loop up to INT_SIZE or an if-else. + if shifts == 0 { + item + } else if shifts == 1 { + item * TWO_POW_8 + } else if shifts == 2 { + item * TWO_POW_16 + } else if shifts == 3 { + item * TWO_POW_24 + } else { + // Doesn't make sense, but it's most likely called on 0 anyway. + 0 + } + } +} + +// Shift by 8 bits to the right between 0 and 4 times. +// Checks `is_unconstrained()` to just use a bitshift if we're running in an unconstrained context, +// otherwise divides by 256. +fn rshift8(item: u32, shifts: u8) -> u32 { + if is_unconstrained() { + item >> (8 * shifts) + } else { + // Division wouldn't work on `Field`. + if shifts == 0 { + item + } else if shifts == 1 { + item / TWO_POW_8 + } else if shifts == 2 { + item / TWO_POW_16 + } else if shifts == 3 { + item / TWO_POW_24 + } else { + 0 + } + } +} + // Zero out all bytes between the end of the message and where the length is appended, // then write the length into the last 8 bytes of the block. unconstrained fn attach_len_to_msg_block( mut msg_block: MSG_BLOCK, - msg_byte_ptr: BLOCK_BYTE_PTR, + mut msg_byte_ptr: BLOCK_BYTE_PTR, message_size: u32, ) -> MSG_BLOCK { // We assume that `msg_byte_ptr` is less than 57 because if not then it is reset to zero before calling this function. - // In any case, fill blocks up with zeros until the last 64 (i.e. until msg_byte_ptr = 56). - for i in msg_byte_ptr..MSG_SIZE_PTR { + // In any case, fill blocks up with zeros until the last 64 bits (i.e. until msg_byte_ptr = 56). + // There can be one item which has to be partially zeroed. + let modulo = msg_byte_ptr % INT_SIZE; + if modulo != 0 { + // Index of the block in which we find the item we need to partially zero. + let i = msg_byte_ptr / INT_SIZE; + let zeros = INT_SIZE - modulo; + msg_block[i] = set_item_zeros(msg_block[i], zeros as u8); + msg_byte_ptr = msg_byte_ptr + zeros; + } + + // The rest can be zeroed without bit shifting anything. + for i in (msg_byte_ptr / INT_SIZE)..INT_SIZE_PTR { msg_block[i] = 0; } + // Set the last two 4 byte ints as the first/second half of the 8 bytes of the length. let len = 8 * message_size; let len_bytes: [u8; 8] = (len as Field).to_be_bytes(); - for i in 0..8 { - msg_block[MSG_SIZE_PTR + i] = len_bytes[i]; + for i in 0..=1 { + let shift = i * 4; + msg_block[INT_SIZE_PTR + i] = make_item( + len_bytes[shift], + len_bytes[shift + 1], + len_bytes[shift + 2], + len_bytes[shift + 3], + ); } msg_block } -// Verify that the message length was correctly written by `attach_len_to_msg_block`. +// Verify that the message length was correctly written by `attach_len_to_msg_block`, +// and that everything between the byte pointer and the size pointer was zeroed, +// and that everything before the byte pointer was untouched. fn verify_msg_len( msg_block: MSG_BLOCK, last_block: MSG_BLOCK, msg_byte_ptr: BLOCK_BYTE_PTR, message_size: u32, ) { - for i in 0..MSG_SIZE_PTR { - let predicate = (i < msg_byte_ptr) as u8; - let expected_byte = predicate * last_block[i]; - assert_eq(msg_block[i], expected_byte); - } + // Check zeros up to the size pointer. + verify_msg_block_zeros(msg_block, msg_byte_ptr, INT_SIZE_PTR); + + // Check that up to the pointer we match the last block. + verify_msg_block_equals_last(msg_block, last_block, msg_byte_ptr); // We verify the message length was inserted correctly by reversing the byte decomposition. - let len = 8 * message_size; - let mut reconstructed_len: Field = 0; - for i in MSG_SIZE_PTR..BLOCK_SIZE { - reconstructed_len = 256 * reconstructed_len + msg_block[i] as Field; + let mut reconstructed_len: u64 = 0; + for i in INT_SIZE_PTR..INT_BLOCK_SIZE { + reconstructed_len = reconstructed_len * TWO_POW_32; + reconstructed_len = reconstructed_len + msg_block[i] as u64; } - assert_eq(reconstructed_len, len as Field); + let len = 8 * message_size as u64; + assert_eq(reconstructed_len, len); } // Perform the final compression, then transform the `STATE` into `HASH`. fn hash_final_block(msg_block: MSG_BLOCK, mut state: STATE) -> HASH { let mut out_h: HASH = [0; 32]; // Digest as sequence of bytes // Hash final padded block - state = sha256_compression(msg_u8_to_u32(msg_block), state); + state = sha256_compression(msg_block, state); // Return final hash as byte array for j in 0..8 { @@ -275,6 +518,11 @@ fn hash_final_block(msg_block: MSG_BLOCK, mut state: STATE) -> HASH { } mod tests { + use super::{ + attach_len_to_msg_block, build_msg_block, byte_into_item, get_item_byte, lshift8, make_item, + set_item_byte_then_zeros, set_item_zeros, + }; + use super::{INT_BLOCK, INT_BLOCK_SIZE, MSG_BLOCK}; use super::sha256_var; #[test] @@ -510,4 +758,79 @@ mod tests { assert_eq(var_length_hash_575, fixed_length_hash); assert_eq(var_length_hash_576, fixed_length_hash); } + + #[test] + fn test_get_item_byte() { + let fld = make_item(10, 20, 30, 40); + assert_eq(fld, 0x0a141e28); + assert_eq(get_item_byte(fld, 0), 10); + assert_eq(get_item_byte(fld, 4), 10); + assert_eq(get_item_byte(fld, 6), 30); + } + + #[test] + fn test_byte_into_item() { + let fld = make_item(0, 20, 0, 0); + assert_eq(byte_into_item(20, 1), fld); + assert_eq(byte_into_item(20, 5), fld); + } + + #[test] + fn test_set_item_zeros() { + let fld0 = make_item(10, 20, 30, 40); + let fld1 = make_item(10, 0, 0, 0); + assert_eq(set_item_zeros(fld0, 3), fld1); + assert_eq(set_item_zeros(fld0, 4), 0); + assert_eq(set_item_zeros(0, 4), 0); + } + + #[test] + fn test_set_item_byte_then_zeros() { + let fld0 = make_item(10, 20, 30, 40); + let fld1 = make_item(10, 50, 0, 0); + assert_eq(set_item_byte_then_zeros(fld0, 1, 50), fld1); + } + + #[test] + fn test_build_msg_block_start_0() { + let input = [ + 102, 114, 111, 109, 58, 114, 117, 110, 110, 105, 101, 114, 46, 108, 101, 97, 103, 117, + 101, 115, 46, 48, + ]; + assert_eq(input.len(), 22); + let (msg_block, msg_byte_ptr) = unsafe { build_msg_block(input, input.len(), 0) }; + assert_eq(msg_byte_ptr, input.len()); + assert_eq(msg_block[0], make_item(input[0], input[1], input[2], input[3])); + assert_eq(msg_block[1], make_item(input[4], input[5], input[6], input[7])); + assert_eq(msg_block[5], make_item(input[20], input[21], 0, 0)); + assert_eq(msg_block[6], 0); + } + + #[test] + fn test_build_msg_block_start_1() { + let input = [ + 102, 114, 111, 109, 58, 114, 117, 110, 110, 105, 101, 114, 46, 108, 101, 97, 103, 117, + 101, 115, 46, 48, 106, 64, 105, 99, 108, 111, 117, 100, 46, 99, 111, 109, 13, 10, 99, + 111, 110, 116, 101, 110, 116, 45, 116, 121, 112, 101, 58, 116, 101, 120, 116, 47, 112, + 108, 97, 105, 110, 59, 32, 99, 104, 97, 114, 115, 101, 116, + ]; + assert_eq(input.len(), 68); + let (msg_block, msg_byte_ptr) = unsafe { build_msg_block(input, input.len(), 64) }; + assert_eq(msg_byte_ptr, 4); + assert_eq(msg_block[0], make_item(input[64], input[65], input[66], input[67])); + assert_eq(msg_block[1], 0); + } + + #[test] + fn test_attach_len_to_msg_block() { + let input: INT_BLOCK = [ + 2152555847, 1397309779, 1936618851, 1262052426, 1936876331, 1985297723, 543702374, + 1919905082, 1131376244, 1701737517, 1417244773, 978151789, 1697470053, 1920166255, + 1849316213, 1651139939, + ]; + let msg_block = unsafe { attach_len_to_msg_block(input, 1, 448) }; + assert_eq(msg_block[0], ((1 << 7) as u32) * 256 * 256 * 256); + assert_eq(msg_block[1], 0); + assert_eq(msg_block[15], 3584); + } } diff --git a/noir/noir-repo/scripts/bump-bb.sh b/noir/noir-repo/scripts/bump-bb.sh index fb1a2b1d31e..661a8eb9d43 100755 --- a/noir/noir-repo/scripts/bump-bb.sh +++ b/noir/noir-repo/scripts/bump-bb.sh @@ -5,7 +5,7 @@ BB_VERSION=$1 sed -i.bak "s/^VERSION=.*/VERSION=\"$BB_VERSION\"/" ./scripts/install_bb.sh && rm ./scripts/install_bb.sh.bak tmp=$(mktemp) -INTEGRATION_TESTS_PACKAGE_JSON=./compiler/integration_tests/package.json +INTEGRATION_TESTS_PACKAGE_JSON=./compiler/integration-tests/package.json jq --arg v $BB_VERSION '.dependencies."@aztec/bb.js" = $v' $INTEGRATION_TESTS_PACKAGE_JSON > $tmp && mv $tmp $INTEGRATION_TESTS_PACKAGE_JSON YARN_ENABLE_IMMUTABLE_INSTALLS=false yarn install diff --git a/noir/noir-repo/scripts/install_bb.sh b/noir/noir-repo/scripts/install_bb.sh index 596f0a54ba4..64baf78c182 100755 --- a/noir/noir-repo/scripts/install_bb.sh +++ b/noir/noir-repo/scripts/install_bb.sh @@ -1,6 +1,6 @@ #!/bin/bash -VERSION="0.58.0" +VERSION="0.60.0" BBUP_PATH=~/.bb/bbup diff --git a/noir/noir-repo/test_programs/compile_success_empty/arithmetic_generics/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/arithmetic_generics/src/main.nr index f599d2879ee..ae50748c8e7 100644 --- a/noir/noir-repo/test_programs/compile_success_empty/arithmetic_generics/src/main.nr +++ b/noir/noir-repo/test_programs/compile_success_empty/arithmetic_generics/src/main.nr @@ -77,25 +77,25 @@ fn equiv_trans( x: Equiv, y: Equiv, ) -> Equiv, Equiv), V, (Equiv, Equiv)> { - Equiv { to_: |z| { y.to(x.to(z)) }, fro_: |z| { x.fro(y.fro(z)) } } + Equiv { to_: |z| y.to(x.to(z)), fro_: |z| x.fro(y.fro(z)) } } fn mul_one_r() -> Equiv, (), W, ()> { - Equiv { to_: |_x| { W {} }, fro_: |_x| { W {} } } + Equiv { to_: |_x| W {}, fro_: |_x| W {} } } fn add_equiv_r( _: Equiv, EN, W, EM>, ) -> Equiv, (), W, ()> { - Equiv { to_: |_x| { W {} }, fro_: |_x| { W {} } } + Equiv { to_: |_x| W {}, fro_: |_x| W {} } } fn mul_comm() -> Equiv, (), W, ()> { - Equiv { to_: |_x| { W {} }, fro_: |_x| { W {} } } + Equiv { to_: |_x| W {}, fro_: |_x| W {} } } fn mul_add() -> Equiv, (), W, ()> { - Equiv { to_: |_x| { W {} }, fro_: |_x| { W {} } } + Equiv { to_: |_x| W {}, fro_: |_x| W {} } } // (N + 1) * N == N * N + N diff --git a/noir/noir-repo/test_programs/compile_success_empty/comptime_closures/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/comptime_closures/src/main.nr index 132c6df6c91..e8a0366a304 100644 --- a/noir/noir-repo/test_programs/compile_success_empty/comptime_closures/src/main.nr +++ b/noir/noir-repo/test_programs/compile_success_empty/comptime_closures/src/main.nr @@ -6,7 +6,7 @@ fn main() { fn closure_test(mut x: Field) { let one = 1; - let add1 = |z| { (|| { *z += one; })() }; + let add1 = |z| (|| { *z += one; })(); let two = 2; let add2 = |z| { *z = *z + two; }; diff --git a/noir/noir-repo/test_programs/compile_success_empty/inner_outer_cl/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/inner_outer_cl/src/main.nr index 57890cd6b67..17cbabec48e 100644 --- a/noir/noir-repo/test_programs/compile_success_empty/inner_outer_cl/src/main.nr +++ b/noir/noir-repo/test_programs/compile_success_empty/inner_outer_cl/src/main.nr @@ -2,7 +2,7 @@ fn main() { let z1 = 0; let z2 = 1; let cl_outer = |x| { - let cl_inner = |y| { x + y + z2 }; + let cl_inner = |y| x + y + z2; cl_inner(1) + z1 }; let result = cl_outer(1); diff --git a/noir/noir-repo/test_programs/compile_success_empty/use_callers_scope/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/use_callers_scope/src/main.nr index bc93bc5187e..26429997f0e 100644 --- a/noir/noir-repo/test_programs/compile_success_empty/use_callers_scope/src/main.nr +++ b/noir/noir-repo/test_programs/compile_success_empty/use_callers_scope/src/main.nr @@ -19,7 +19,7 @@ mod bar { // Ensure closures can still access Bar even // though `map` separates them from `fn_attr`. - let _ = &[1, 2, 3].map(|_| { quote { Bar }.as_type() }); + let _ = &[1, 2, 3].map(|_| quote { Bar }.as_type()); } // use_callers_scope should also work nested diff --git a/noir/noir-repo/test_programs/execution_success/higher_order_functions/src/main.nr b/noir/noir-repo/test_programs/execution_success/higher_order_functions/src/main.nr index 6d094946f6c..1f9598f8591 100644 --- a/noir/noir-repo/test_programs/execution_success/higher_order_functions/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/higher_order_functions/src/main.nr @@ -5,7 +5,7 @@ fn main(w: Field) -> pub Field { assert(twice(|x| x * 2, 5) == 20); assert((|x, y| x + y + 1)(2, 3) == 6); // nested lambdas - assert((|a, b| { a + (|c| c + 2)(b) })(0, 1) == 3); + assert((|a, b| a + (|c| c + 2)(b))(0, 1) == 3); // Closures: let a = 42; let g = || a; diff --git a/noir/noir-repo/tooling/nargo/src/ops/execute.rs b/noir/noir-repo/tooling/nargo/src/ops/execute.rs index eb03bdf01c1..09ef554d2aa 100644 --- a/noir/noir-repo/tooling/nargo/src/ops/execute.rs +++ b/noir/noir-repo/tooling/nargo/src/ops/execute.rs @@ -3,7 +3,9 @@ use acvm::acir::circuit::{ OpcodeLocation, Program, ResolvedAssertionPayload, ResolvedOpcodeLocation, }; use acvm::acir::native_types::WitnessStack; -use acvm::pwg::{ACVMStatus, ErrorLocation, OpcodeNotSolvable, OpcodeResolutionError, ACVM}; +use acvm::pwg::{ + ACVMStatus, ErrorLocation, OpcodeNotSolvable, OpcodeResolutionError, ProfilingSamples, ACVM, +}; use acvm::{acir::circuit::Circuit, acir::native_types::WitnessMap}; use acvm::{AcirField, BlackBoxFunctionSolver}; @@ -32,6 +34,10 @@ struct ProgramExecutor<'a, F, B: BlackBoxFunctionSolver, E: ForeignCallExecut // This is used to fetch the function we want to execute // and to resolve call stack locations across many function calls. current_function_index: usize, + + // Flag that states whether we want to profile the VM. Profiling can add extra + // execution costs so we want to make sure we only trigger it explicitly. + profiling_active: bool, } impl<'a, F: AcirField, B: BlackBoxFunctionSolver, E: ForeignCallExecutor> @@ -42,6 +48,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver, E: ForeignCallExecutor> unconstrained_functions: &'a [BrilligBytecode], blackbox_solver: &'a B, foreign_call_executor: &'a mut E, + profiling_active: bool, ) -> Self { ProgramExecutor { functions, @@ -51,6 +58,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver, E: ForeignCallExecutor> foreign_call_executor, call_stack: Vec::default(), current_function_index: 0, + profiling_active, } } @@ -62,7 +70,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver, E: ForeignCallExecutor> fn execute_circuit( &mut self, initial_witness: WitnessMap, - ) -> Result, NargoError> { + ) -> Result<(WitnessMap, ProfilingSamples), NargoError> { let circuit = &self.functions[self.current_function_index]; let mut acvm = ACVM::new( self.blackbox_solver, @@ -71,6 +79,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver, E: ForeignCallExecutor> self.unconstrained_functions, &circuit.assert_messages, ); + acvm.with_profiler(self.profiling_active); loop { let solver_status = acvm.solve(); @@ -155,7 +164,8 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver, E: ForeignCallExecutor> // Execute the ACIR call let acir_to_call = &self.functions[call_info.id.as_usize()]; let initial_witness = call_info.initial_witness; - let call_solved_witness = self.execute_circuit(initial_witness)?; + // TODO: Profiling among multiple circuits is not supported + let (call_solved_witness, _) = self.execute_circuit(initial_witness)?; // Set tracking index back to the parent function after ACIR call execution self.current_function_index = acir_function_caller; @@ -184,25 +194,67 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver, E: ForeignCallExecutor> // included in a failure case. self.call_stack.clear(); - Ok(acvm.finalize()) + let profiling_samples = acvm.take_profiling_samples(); + Ok((acvm.finalize(), profiling_samples)) } } -#[tracing::instrument(level = "trace", skip_all)] pub fn execute_program, E: ForeignCallExecutor>( program: &Program, initial_witness: WitnessMap, blackbox_solver: &B, foreign_call_executor: &mut E, ) -> Result, NargoError> { + let profiling_active = false; + let (witness_stack, profiling_samples) = execute_program_inner( + program, + initial_witness, + blackbox_solver, + foreign_call_executor, + profiling_active, + )?; + assert!(profiling_samples.is_empty(), "Expected no profiling samples"); + + Ok(witness_stack) +} + +pub fn execute_program_with_profiling< + F: AcirField, + B: BlackBoxFunctionSolver, + E: ForeignCallExecutor, +>( + program: &Program, + initial_witness: WitnessMap, + blackbox_solver: &B, + foreign_call_executor: &mut E, +) -> Result<(WitnessStack, ProfilingSamples), NargoError> { + let profiling_active = true; + execute_program_inner( + program, + initial_witness, + blackbox_solver, + foreign_call_executor, + profiling_active, + ) +} + +#[tracing::instrument(level = "trace", skip_all)] +fn execute_program_inner, E: ForeignCallExecutor>( + program: &Program, + initial_witness: WitnessMap, + blackbox_solver: &B, + foreign_call_executor: &mut E, + profiling_active: bool, +) -> Result<(WitnessStack, ProfilingSamples), NargoError> { let mut executor = ProgramExecutor::new( &program.functions, &program.unconstrained_functions, blackbox_solver, foreign_call_executor, + profiling_active, ); - let main_witness = executor.execute_circuit(initial_witness)?; + let (main_witness, profiling_samples) = executor.execute_circuit(initial_witness)?; executor.witness_stack.push(0, main_witness); - Ok(executor.finalize()) + Ok((executor.finalize(), profiling_samples)) } diff --git a/noir/noir-repo/tooling/nargo/src/ops/mod.rs b/noir/noir-repo/tooling/nargo/src/ops/mod.rs index cada2f0e915..16680dab980 100644 --- a/noir/noir-repo/tooling/nargo/src/ops/mod.rs +++ b/noir/noir-repo/tooling/nargo/src/ops/mod.rs @@ -2,7 +2,7 @@ pub use self::compile::{ collect_errors, compile_contract, compile_program, compile_program_with_debug_instrumenter, compile_workspace, report_errors, }; -pub use self::execute::execute_program; +pub use self::execute::{execute_program, execute_program_with_profiling}; pub use self::foreign_calls::{DefaultForeignCallExecutor, ForeignCall, ForeignCallExecutor}; pub use self::optimize::{optimize_contract, optimize_program}; pub use self::transform::{transform_contract, transform_program}; diff --git a/noir/noir-repo/tooling/nargo_cli/tests/stdlib-tests.rs b/noir/noir-repo/tooling/nargo_cli/tests/stdlib-tests.rs index d27a2961a62..46a241b7e0b 100644 --- a/noir/noir-repo/tooling/nargo_cli/tests/stdlib-tests.rs +++ b/noir/noir-repo/tooling/nargo_cli/tests/stdlib-tests.rs @@ -76,7 +76,7 @@ fn run_stdlib_tests() { &bn254_blackbox_solver::Bn254BlackBoxSolver, &mut context, &test_function, - false, + true, None, Some(dummy_package.root_dir.clone()), Some(dummy_package.name.to_string()), diff --git a/noir/noir-repo/tooling/nargo_fmt/src/chunks.rs b/noir/noir-repo/tooling/nargo_fmt/src/chunks.rs index 073e03ea74a..673cf020aed 100644 --- a/noir/noir-repo/tooling/nargo_fmt/src/chunks.rs +++ b/noir/noir-repo/tooling/nargo_fmt/src/chunks.rs @@ -466,7 +466,11 @@ pub(crate) enum GroupKind { /// fit in the current line and it's not a block, instead of splitting that expression /// somewhere that's probably undesired, we'll "turn it" into a block /// (write the "{" and "}" delimiters) and write the lambda body in the next line. - LambdaBody { is_block: bool }, + LambdaBody { + block_statement_count: Option, + has_comments: bool, + lambda_has_return_type: bool, + }, /// A method call. /// We track all this information to see, if we end up needing to format this call /// in multiple lines, if we can write everything up to the left parentheses (inclusive) @@ -762,14 +766,14 @@ impl<'a> Formatter<'a> { // If a lambda body doesn't fit in the current line and it's not a block, // we can turn it into a block and write it in the next line, so its contents fit. - if let GroupKind::LambdaBody { is_block: false } = group.kind { + if let GroupKind::LambdaBody { block_statement_count: None, .. } = group.kind { // Try to format it again in the next line, but we don't want to recurse // infinitely so we change the group kind. group.kind = GroupKind::Regular; self.write("{"); self.trim_spaces(); self.increase_indentation(); - self.write_line(); + self.write_line_without_skipping_whitespace_and_comments(); self.write_indentation(); self.format_chunk_group_impl(group); @@ -803,7 +807,7 @@ impl<'a> Formatter<'a> { // ) let comma_trimmed = self.trim_comma(); self.decrease_indentation(); - self.write_line(); + self.write_line_without_skipping_whitespace_and_comments(); self.write_indentation(); self.write("}"); if comma_trimmed { @@ -816,6 +820,24 @@ impl<'a> Formatter<'a> { return; } + // At this point we determined we are going to write this group in a single line. + // If the current group is a lambda body that is a block with a single statement, like this: + // + // |x| { 1 + 2 } + // + // given that we determined the block fits the current line, if we remove the surrounding + // `{ .. }` it will still fit the current line, and reduce some noise from the code + // (this is what rustfmt seems to do too). + if let GroupKind::LambdaBody { + block_statement_count: Some(1), + has_comments: false, + lambda_has_return_type: false, + } = group.kind + { + self.format_lambda_body_removing_braces(group); + return; + } + self.format_chunk_group_in_one_line(group); } @@ -827,10 +849,10 @@ impl<'a> Formatter<'a> { } Chunk::TrailingComment(text_chunk) | Chunk::LeadingComment(text_chunk) => { self.write(&text_chunk.string); - self.write(" "); + self.write_space_without_skipping_whitespace_and_comments(); } Chunk::Group(chunks) => self.format_chunk_group_impl(chunks), - Chunk::SpaceOrLine => self.write(" "), + Chunk::SpaceOrLine => self.write_space_without_skipping_whitespace_and_comments(), Chunk::IncreaseIndentation => self.increase_indentation(), Chunk::DecreaseIndentation => self.decrease_indentation(), Chunk::PushIndentation => self.push_indentation(), @@ -893,9 +915,16 @@ impl<'a> Formatter<'a> { self.write_indentation(); } Chunk::LeadingComment(text_chunk) => { + let ends_with_newline = text_chunk.string.ends_with('\n'); self.write_chunk_lines(text_chunk.string.trim()); - self.write_line_without_skipping_whitespace_and_comments(); - self.write_indentation(); + + // Respect whether the leading comment had a newline before what comes next or not + if ends_with_newline { + self.write_line_without_skipping_whitespace_and_comments(); + self.write_indentation(); + } else { + self.write_space_without_skipping_whitespace_and_comments(); + } } Chunk::Group(mut group) => { if chunks.force_multiline_on_children_with_same_tag_if_multiline @@ -939,6 +968,31 @@ impl<'a> Formatter<'a> { } } + fn format_lambda_body_removing_braces(&mut self, group: ChunkGroup) { + // Write to an intermediate string so we can remove the braces if needed. + let text_chunk = self.chunk_formatter().chunk(|formatter| { + formatter.format_chunk_group_in_one_line(group); + }); + let string = text_chunk.string; + + // Don't remove the braces if the lambda's body is a Semi expression. + if string.ends_with("; }") || string.ends_with("; },") { + self.write(&string); + return; + } + + let string = string.strip_prefix("{ ").unwrap(); + + // The lambda might have a trailing comma if it's inside an arguments list + if let Some(string) = string.strip_suffix(" },") { + self.write(string); + self.write(","); + } else { + let string = string.strip_suffix(" }").unwrap(); + self.write(string); + } + } + /// Appends the string to the current buffer line by line, with some pre-checks. fn write_chunk_lines(&mut self, string: &str) { for (index, line) in string.lines().enumerate() { diff --git a/noir/noir-repo/tooling/nargo_fmt/src/formatter/comments_and_whitespace.rs b/noir/noir-repo/tooling/nargo_fmt/src/formatter/comments_and_whitespace.rs index e5f15bc397e..4aba0481e24 100644 --- a/noir/noir-repo/tooling/nargo_fmt/src/formatter/comments_and_whitespace.rs +++ b/noir/noir-repo/tooling/nargo_fmt/src/formatter/comments_and_whitespace.rs @@ -301,8 +301,8 @@ mod tests { let src = "global x = [ /* hello */ 1 , 2 ] ;"; let expected = "global x = [ - /* hello */ - 1, 2, + /* hello */ 1, + 2, ]; "; assert_format_with_max_width(src, expected, 20); diff --git a/noir/noir-repo/tooling/nargo_fmt/src/formatter/expression.rs b/noir/noir-repo/tooling/nargo_fmt/src/formatter/expression.rs index 6f7ebf56348..ebfa3ae78fb 100644 --- a/noir/noir-repo/tooling/nargo_fmt/src/formatter/expression.rs +++ b/noir/noir-repo/tooling/nargo_fmt/src/formatter/expression.rs @@ -198,6 +198,8 @@ impl<'a, 'b> ChunkFormatter<'a, 'b> { fn format_lambda(&mut self, lambda: Lambda) -> FormattedLambda { let mut group = ChunkGroup::new(); + let lambda_has_return_type = lambda.return_type.typ != UnresolvedTypeData::Unspecified; + let params_and_return_type_chunk = self.chunk(|formatter| { formatter.write_token(Token::Pipe); for (index, (pattern, typ)) in lambda.parameters.into_iter().enumerate() { @@ -218,7 +220,7 @@ impl<'a, 'b> ChunkFormatter<'a, 'b> { } formatter.write_token(Token::Pipe); formatter.write_space(); - if lambda.return_type.typ != UnresolvedTypeData::Unspecified { + if lambda_has_return_type { formatter.write_token(Token::Arrow); formatter.write_space(); formatter.format_type(lambda.return_type); @@ -230,16 +232,27 @@ impl<'a, 'b> ChunkFormatter<'a, 'b> { group.text(params_and_return_type_chunk); - let body_is_block = matches!(lambda.body.kind, ExpressionKind::Block(..)); + let block_statement_count = if let ExpressionKind::Block(block) = &lambda.body.kind { + Some(block.statements.len()) + } else { + None + }; let mut body_group = ChunkGroup::new(); - body_group.kind = GroupKind::LambdaBody { is_block: body_is_block }; + let comments_count_before_body = self.written_comments_count; self.format_expression(lambda.body, &mut body_group); + + body_group.kind = GroupKind::LambdaBody { + block_statement_count, + has_comments: self.written_comments_count > comments_count_before_body, + lambda_has_return_type, + }; + group.group(body_group); let first_line_width = params_and_return_type_chunk_width - + (if body_is_block { + + (if block_statement_count.is_some() { // 1 because we already have `|param1, param2, ..., paramN| ` (including the space) // so all that's left is a `{`. 1 @@ -433,23 +446,22 @@ impl<'a, 'b> ChunkFormatter<'a, 'b> { { let mut comments_chunk = self.skip_comments_and_whitespace_chunk(); - // If the comment is not empty but doesn't have newlines, it's surely `/* comment */`. - // We format that with spaces surrounding it so it looks, for example, like `Foo { /* comment */ field ..`. - if !comments_chunk.string.trim().is_empty() && !comments_chunk.has_newlines { - // Note: there's no space after `{}` because space will be produced by format_items_separated_by_comma - comments_chunk.string = if surround_with_spaces { - format!(" {}", comments_chunk.string.trim()) - } else { - format!(" {} ", comments_chunk.string.trim()) - }; - group.text(comments_chunk); - + // Handle leading block vs. line comments a bit differently. + if comments_chunk.string.trim().starts_with("/*") { group.increase_indentation(); if surround_with_spaces { group.space_or_line(); } else { group.line(); } + + // Note: there's no space before `{}` because it was just produced + comments_chunk.string = if surround_with_spaces { + comments_chunk.string.trim().to_string() + } else { + format!("{} ", comments_chunk.string.trim()) + }; + group.leading_comment(comments_chunk); } else { group.increase_indentation(); if surround_with_spaces { @@ -466,7 +478,24 @@ impl<'a, 'b> ChunkFormatter<'a, 'b> { group.text_attached_to_last_group(self.chunk(|formatter| { formatter.write_comma(); })); - group.trailing_comment(self.skip_comments_and_whitespace_chunk()); + let newlines_count_before_comment = self.following_newlines_count(); + group.text(self.chunk(|formatter| { + formatter.skip_whitespace(); + })); + if let Token::BlockComment(..) = &self.token { + // We let block comments be part of the item that's going to be formatted + } else { + // Line comments can be trailing or leading, depending on whether there are newlines before them + let comments_and_whitespace_chunk = self.skip_comments_and_whitespace_chunk(); + if !comments_and_whitespace_chunk.string.trim().is_empty() { + if newlines_count_before_comment > 0 { + group.line(); + group.leading_comment(comments_and_whitespace_chunk); + } else { + group.trailing_comment(comments_and_whitespace_chunk); + } + } + } group.space_or_line(); } format_item(self, expr, group); @@ -1313,6 +1342,56 @@ global y = 1; assert_format_with_config(src, expected, config); } + #[test] + fn format_short_array_with_block_comment_before_elements() { + let src = "global x = [ /* one */ 1, /* two */ 2 ] ;"; + let expected = "global x = [/* one */ 1, /* two */ 2];\n"; + + assert_format(src, expected); + } + + #[test] + fn format_long_array_with_block_comment_before_elements() { + let src = "global x = [ /* one */ 1, /* two */ 123456789012345, 3, 4 ] ;"; + let expected = "global x = [ + /* one */ 1, + /* two */ 123456789012345, + 3, + 4, +]; +"; + + let config = + Config { short_array_element_width_threshold: 5, max_width: 30, ..Config::default() }; + assert_format_with_config(src, expected, config); + } + + #[test] + fn format_long_array_with_line_comment_before_elements() { + let src = "global x = [ + // one + 1, + // two + 123456789012345, + 3, + 4, +]; +"; + let expected = "global x = [ + // one + 1, + // two + 123456789012345, + 3, + 4, +]; +"; + + let config = + Config { short_array_element_width_threshold: 5, max_width: 30, ..Config::default() }; + assert_format_with_config(src, expected, config); + } + #[test] fn format_cast() { let src = "global x = 1 as u8 ;"; @@ -1980,12 +2059,44 @@ global y = 1; } #[test] - fn format_lambda_with_block() { + fn format_lambda_with_block_simplifies() { let src = "global x = | | { 1 } ;"; - let expected = "global x = || { 1 };\n"; + let expected = "global x = || 1;\n"; assert_format(src, expected); } + #[test] + fn format_lambda_with_block_does_not_simplify_if_it_ends_with_semicolon() { + let src = "global x = | | { 1; } ;"; + let expected = "global x = || { 1; };\n"; + assert_format(src, expected); + } + + #[test] + fn format_lambda_with_block_does_not_simplify_if_it_has_return_type() { + let src = "global x = | | -> i32 { 1 } ;"; + let expected = "global x = || -> i32 { 1 };\n"; + assert_format(src, expected); + } + + #[test] + fn format_lambda_with_simplifies_block_with_quote() { + let src = "global x = | | { quote { 1 } } ;"; + let expected = "global x = || quote { 1 };\n"; + assert_format(src, expected); + } + + #[test] + fn format_lambda_with_block_simplifies_inside_arguments_list() { + let src = "global x = some_call(this_is_a_long_argument, | | { 1 });"; + let expected = "global x = some_call( + this_is_a_long_argument, + || 1, +); +"; + assert_format_with_max_width(src, expected, 20); + } + #[test] fn format_lambda_with_block_multiple_statements() { let src = "global x = | a, b | { 1; 2 } ;"; diff --git a/noir/noir-repo/tooling/nargo_fmt/src/formatter/statement.rs b/noir/noir-repo/tooling/nargo_fmt/src/formatter/statement.rs index b0692d2d631..28107294909 100644 --- a/noir/noir-repo/tooling/nargo_fmt/src/formatter/statement.rs +++ b/noir/noir-repo/tooling/nargo_fmt/src/formatter/statement.rs @@ -568,10 +568,10 @@ mod tests { #[test] fn format_two_for_separated_by_multiple_lines() { - let src = " fn foo() { for x in array { 1 } - + let src = " fn foo() { for x in array { 1 } + for x in array { 1 } - + } "; let expected = "fn foo() { for x in array { diff --git a/noir/noir-repo/tooling/nargo_fmt/src/formatter/use_tree.rs b/noir/noir-repo/tooling/nargo_fmt/src/formatter/use_tree.rs index bc8dc3fcabb..98d63ef6611 100644 --- a/noir/noir-repo/tooling/nargo_fmt/src/formatter/use_tree.rs +++ b/noir/noir-repo/tooling/nargo_fmt/src/formatter/use_tree.rs @@ -211,7 +211,7 @@ mod tests { #[test] fn format_use_list_one_item_with_comments() { let src = " use foo::{ /* do not remove me */ bar, };"; - let expected = "use foo::{ /* do not remove me */ bar};\n"; + let expected = "use foo::{/* do not remove me */ bar};\n"; assert_format(src, expected); } diff --git a/noir/noir-repo/tooling/nargo_fmt/src/formatter/use_tree_merge.rs b/noir/noir-repo/tooling/nargo_fmt/src/formatter/use_tree_merge.rs index e24b7b8cbf6..834280ddba3 100644 --- a/noir/noir-repo/tooling/nargo_fmt/src/formatter/use_tree_merge.rs +++ b/noir/noir-repo/tooling/nargo_fmt/src/formatter/use_tree_merge.rs @@ -397,7 +397,7 @@ mod tests { #[test] fn format_use_list_one_item_with_comments() { let src = " use foo::{ /* do not remove me */ bar, };"; - let expected = "use foo::{ /* do not remove me */ bar};\n"; + let expected = "use foo::{/* do not remove me */ bar};\n"; assert_format(src, expected); } diff --git a/noir/noir-repo/tooling/nargo_fmt/tests/expected/contract.nr b/noir/noir-repo/tooling/nargo_fmt/tests/expected/contract.nr index ad53e61c911..86f8d56b62d 100644 --- a/noir/noir-repo/tooling/nargo_fmt/tests/expected/contract.nr +++ b/noir/noir-repo/tooling/nargo_fmt/tests/expected/contract.nr @@ -34,7 +34,7 @@ contract Benchmarking { notes: Map::new( context, 1, - |context, slot| { PrivateSet::new(context, slot, ValueNoteMethods) }, + |context, slot| PrivateSet::new(context, slot, ValueNoteMethods), ), balances: Map::new( context, diff --git a/noir/noir-repo/tooling/nargo_fmt/tests/expected/tuple.nr b/noir/noir-repo/tooling/nargo_fmt/tests/expected/tuple.nr index d4b8b239815..29f32f83e55 100644 --- a/noir/noir-repo/tooling/nargo_fmt/tests/expected/tuple.nr +++ b/noir/noir-repo/tooling/nargo_fmt/tests/expected/tuple.nr @@ -4,13 +4,13 @@ fn main() { // hello 1, ); - ( /*hello*/ 1,); + (/*hello*/ 1,); (1/*hello*/,); (1,); - ( /*test*/ 1,); - ( /*a*/ 1/*b*/,); - ( /*a*/ 1/*b*/, /*c*/ 2/*d*/, /*c*/ 2/*d*/); - ( /*a*/ 1/*b*/, /*c*/ 2/*d*/, /*c*/ 2/*d*/, /*e*/ 3/*f*/); + (/*test*/ 1,); + (/*a*/ 1/*b*/,); + (/*a*/ 1/*b*/, /*c*/ 2/*d*/, /*c*/ 2/*d*/); + (/*a*/ 1/*b*/, /*c*/ 2/*d*/, /*c*/ 2/*d*/, /*e*/ 3/*f*/); (1 /*1*/, 2 /* 2*/); @@ -28,7 +28,7 @@ fn main() { 2, ); - ( /*1*/ 1, /*2*/ 2); + (/*1*/ 1, /*2*/ 2); ( ( @@ -39,14 +39,8 @@ fn main() { ), ); ( - /*a*/ - 1 - /*b*/, - /*c*/ - 2/*d*/, - /*c*/ - 2/*d*/, - /*e*/ - 3,/*f*/ + /*a*/ 1 + /*b*/, /*c*/ + 2/*d*/, /*c*/ 2/*d*/, /*e*/ 3,/*f*/ ); } diff --git a/noir/noir-repo/tooling/profiler/Cargo.toml b/noir/noir-repo/tooling/profiler/Cargo.toml index 136775d5831..c76b4c73e65 100644 --- a/noir/noir-repo/tooling/profiler/Cargo.toml +++ b/noir/noir-repo/tooling/profiler/Cargo.toml @@ -18,8 +18,10 @@ name = "noir-profiler" path = "src/main.rs" [dependencies] +bn254_blackbox_solver.workspace = true color-eyre.workspace = true clap.workspace = true +fxhash.workspace = true noirc_artifacts.workspace = true const_format.workspace = true serde.workspace = true @@ -28,7 +30,9 @@ fm.workspace = true inferno = "0.11.19" im.workspace = true acir.workspace = true +nargo.workspace = true noirc_errors.workspace = true +noirc_abi.workspace = true # Logs tracing-subscriber.workspace = true diff --git a/noir/noir-repo/tooling/profiler/src/cli/execution_flamegraph_cmd.rs b/noir/noir-repo/tooling/profiler/src/cli/execution_flamegraph_cmd.rs new file mode 100644 index 00000000000..a0b3d6a3128 --- /dev/null +++ b/noir/noir-repo/tooling/profiler/src/cli/execution_flamegraph_cmd.rs @@ -0,0 +1,96 @@ +use std::path::{Path, PathBuf}; + +use acir::circuit::OpcodeLocation; +use acir::FieldElement; +use clap::Args; +use color_eyre::eyre::{self, Context}; + +use crate::flamegraph::{FlamegraphGenerator, InfernoFlamegraphGenerator, Sample}; +use crate::fs::{read_inputs_from_file, read_program_from_file}; +use crate::opcode_formatter::AcirOrBrilligOpcode; +use bn254_blackbox_solver::Bn254BlackBoxSolver; +use nargo::ops::DefaultForeignCallExecutor; +use noirc_abi::input_parser::Format; +use noirc_artifacts::debug::DebugArtifact; + +#[derive(Debug, Clone, Args)] +pub(crate) struct ExecutionFlamegraphCommand { + /// The path to the artifact JSON file + #[clap(long, short)] + artifact_path: PathBuf, + + /// The path to the Prover.toml file + #[clap(long, short)] + prover_toml_path: PathBuf, + + /// The output folder for the flamegraph svg files + #[clap(long, short)] + output: PathBuf, +} + +pub(crate) fn run(args: ExecutionFlamegraphCommand) -> eyre::Result<()> { + run_with_generator( + &args.artifact_path, + &args.prover_toml_path, + &InfernoFlamegraphGenerator { count_name: "samples".to_string() }, + &args.output, + ) +} + +fn run_with_generator( + artifact_path: &Path, + prover_toml_path: &Path, + flamegraph_generator: &impl FlamegraphGenerator, + output_path: &Path, +) -> eyre::Result<()> { + let program = + read_program_from_file(artifact_path).context("Error reading program from file")?; + + let (inputs_map, _) = read_inputs_from_file(prover_toml_path, Format::Toml, &program.abi)?; + + let initial_witness = program.abi.encode(&inputs_map, None)?; + + println!("Executing"); + let (_, profiling_samples) = nargo::ops::execute_program_with_profiling( + &program.bytecode, + initial_witness, + &Bn254BlackBoxSolver, + &mut DefaultForeignCallExecutor::new(true, None, None, None), + )?; + println!("Executed"); + + let profiling_samples: Vec> = profiling_samples + .into_iter() + .map(|sample| { + let call_stack = sample.call_stack; + let brillig_function_id = sample.brillig_function_id; + let last_entry = call_stack.last(); + let opcode = brillig_function_id + .and_then(|id| program.bytecode.unconstrained_functions.get(id.0 as usize)) + .and_then(|func| { + if let Some(OpcodeLocation::Brillig { brillig_index, .. }) = last_entry { + func.bytecode.get(*brillig_index) + } else { + None + } + }) + .map(|opcode| AcirOrBrilligOpcode::Brillig(opcode.clone())); + Sample { opcode, call_stack, count: 1, brillig_function_id } + }) + .collect(); + + let debug_artifact: DebugArtifact = program.into(); + + println!("Generating flamegraph with {} samples", profiling_samples.len()); + + flamegraph_generator.generate_flamegraph( + profiling_samples, + &debug_artifact.debug_symbols[0], + &debug_artifact, + artifact_path.to_str().unwrap(), + "main", + &Path::new(&output_path).join(Path::new(&format!("{}.svg", "main"))), + )?; + + Ok(()) +} diff --git a/noir/noir-repo/tooling/profiler/src/cli/gates_flamegraph_cmd.rs b/noir/noir-repo/tooling/profiler/src/cli/gates_flamegraph_cmd.rs index d5fefc4ecda..20cc1b747c3 100644 --- a/noir/noir-repo/tooling/profiler/src/cli/gates_flamegraph_cmd.rs +++ b/noir/noir-repo/tooling/profiler/src/cli/gates_flamegraph_cmd.rs @@ -84,7 +84,7 @@ fn run_with_provider( .zip(bytecode.opcodes) .enumerate() .map(|(index, (gates, opcode))| Sample { - opcode: AcirOrBrilligOpcode::Acir(opcode), + opcode: Some(AcirOrBrilligOpcode::Acir(opcode)), call_stack: vec![OpcodeLocation::Acir(index)], count: gates, brillig_function_id: None, diff --git a/noir/noir-repo/tooling/profiler/src/cli/mod.rs b/noir/noir-repo/tooling/profiler/src/cli/mod.rs index 7cfc6ed7c9e..80c6bceb3ce 100644 --- a/noir/noir-repo/tooling/profiler/src/cli/mod.rs +++ b/noir/noir-repo/tooling/profiler/src/cli/mod.rs @@ -2,6 +2,7 @@ use clap::{Parser, Subcommand}; use color_eyre::eyre; use const_format::formatcp; +mod execution_flamegraph_cmd; mod gates_flamegraph_cmd; mod opcodes_flamegraph_cmd; @@ -19,15 +20,17 @@ struct ProfilerCli { #[non_exhaustive] #[derive(Subcommand, Clone, Debug)] enum ProfilerCommand { - GatesFlamegraph(gates_flamegraph_cmd::GatesFlamegraphCommand), - OpcodesFlamegraph(opcodes_flamegraph_cmd::OpcodesFlamegraphCommand), + Gates(gates_flamegraph_cmd::GatesFlamegraphCommand), + Opcodes(opcodes_flamegraph_cmd::OpcodesFlamegraphCommand), + ExecutionOpcodes(execution_flamegraph_cmd::ExecutionFlamegraphCommand), } pub(crate) fn start_cli() -> eyre::Result<()> { let ProfilerCli { command } = ProfilerCli::parse(); match command { - ProfilerCommand::GatesFlamegraph(args) => gates_flamegraph_cmd::run(args), - ProfilerCommand::OpcodesFlamegraph(args) => opcodes_flamegraph_cmd::run(args), + ProfilerCommand::Gates(args) => gates_flamegraph_cmd::run(args), + ProfilerCommand::Opcodes(args) => opcodes_flamegraph_cmd::run(args), + ProfilerCommand::ExecutionOpcodes(args) => execution_flamegraph_cmd::run(args), } } diff --git a/noir/noir-repo/tooling/profiler/src/cli/opcodes_flamegraph_cmd.rs b/noir/noir-repo/tooling/profiler/src/cli/opcodes_flamegraph_cmd.rs index 863d45b96d1..bb3df86c339 100644 --- a/noir/noir-repo/tooling/profiler/src/cli/opcodes_flamegraph_cmd.rs +++ b/noir/noir-repo/tooling/profiler/src/cli/opcodes_flamegraph_cmd.rs @@ -60,7 +60,7 @@ fn run_with_generator( .iter() .enumerate() .map(|(index, opcode)| Sample { - opcode: AcirOrBrilligOpcode::Acir(opcode.clone()), + opcode: Some(AcirOrBrilligOpcode::Acir(opcode.clone())), call_stack: vec![OpcodeLocation::Acir(index)], count: 1, brillig_function_id: None, @@ -97,7 +97,7 @@ fn run_with_generator( .into_iter() .enumerate() .map(|(brillig_index, opcode)| Sample { - opcode: AcirOrBrilligOpcode::Brillig(opcode), + opcode: Some(AcirOrBrilligOpcode::Brillig(opcode)), call_stack: vec![OpcodeLocation::Brillig { acir_index: acir_opcode_index, brillig_index, diff --git a/noir/noir-repo/tooling/profiler/src/flamegraph.rs b/noir/noir-repo/tooling/profiler/src/flamegraph.rs index 488079de50a..a72168a20af 100644 --- a/noir/noir-repo/tooling/profiler/src/flamegraph.rs +++ b/noir/noir-repo/tooling/profiler/src/flamegraph.rs @@ -6,6 +6,7 @@ use acir::circuit::OpcodeLocation; use acir::AcirField; use color_eyre::eyre::{self}; use fm::codespan_files::Files; +use fxhash::FxHashMap as HashMap; use inferno::flamegraph::{from_lines, Options, TextTruncateDirection}; use noirc_errors::debug_info::DebugInfo; use noirc_errors::reporter::line_and_column_from_span; @@ -17,7 +18,7 @@ use super::opcode_formatter::format_opcode; #[derive(Debug)] pub(crate) struct Sample { - pub(crate) opcode: AcirOrBrilligOpcode, + pub(crate) opcode: Option>, pub(crate) call_stack: Vec, pub(crate) count: usize, pub(crate) brillig_function_id: Option, @@ -88,41 +89,61 @@ fn generate_folded_sorted_lines<'files, F: AcirField>( // Create a nested hashmap with the stack items, folding the gates for all the callsites that are equal let mut folded_stack_items = BTreeMap::new(); - samples.into_iter().for_each(|sample| { - let mut location_names: Vec = sample - .call_stack - .into_iter() - .flat_map(|opcode_location| { - debug_symbols.opcode_location(&opcode_location).unwrap_or_else(|| { - if let (Some(brillig_function_id), Some(brillig_location)) = - (sample.brillig_function_id, opcode_location.to_brillig_location()) - { - let brillig_locations = - debug_symbols.brillig_locations.get(&brillig_function_id); - if let Some(brillig_locations) = brillig_locations { - brillig_locations.get(&brillig_location).cloned().unwrap_or_default() - } else { - vec![] - } - } else { - vec![] - } + let mut resolution_cache: HashMap> = HashMap::default(); + for sample in samples { + let mut location_names = Vec::with_capacity(sample.call_stack.len()); + for opcode_location in sample.call_stack { + let callsite_labels = resolution_cache + .entry(opcode_location) + .or_insert_with(|| { + find_callsite_labels( + debug_symbols, + &opcode_location, + sample.brillig_function_id, + files, + ) }) - }) - .map(|location| location_to_callsite_label(location, files)) - .collect(); + .clone(); - if location_names.is_empty() { - location_names.push("unknown".to_string()); + location_names.extend(callsite_labels); } - location_names.push(format_opcode(&sample.opcode)); + if let Some(opcode) = &sample.opcode { + location_names.push(format_opcode(opcode)); + } add_locations_to_folded_stack_items(&mut folded_stack_items, location_names, sample.count); - }); + } to_folded_sorted_lines(&folded_stack_items, Default::default()) } +fn find_callsite_labels<'files>( + debug_symbols: &DebugInfo, + opcode_location: &OpcodeLocation, + brillig_function_id: Option, + files: &'files impl Files<'files, FileId = fm::FileId>, +) -> Vec { + let source_locations = debug_symbols.opcode_location(opcode_location).unwrap_or_else(|| { + if let (Some(brillig_function_id), Some(brillig_location)) = + (brillig_function_id, opcode_location.to_brillig_location()) + { + let brillig_locations = debug_symbols.brillig_locations.get(&brillig_function_id); + if let Some(brillig_locations) = brillig_locations { + brillig_locations.get(&brillig_location).cloned().unwrap_or_default() + } else { + vec![] + } + } else { + vec![] + } + }); + let callsite_labels: Vec<_> = source_locations + .into_iter() + .map(|location| location_to_callsite_label(location, files)) + .collect(); + callsite_labels +} + fn location_to_callsite_label<'files>( location: Location, files: &'files impl Files<'files, FileId = fm::FileId>, @@ -300,23 +321,27 @@ mod tests { let samples: Vec> = vec![ Sample { - opcode: AcirOrBrilligOpcode::Acir(AcirOpcode::AssertZero(Expression::default())), + opcode: Some(AcirOrBrilligOpcode::Acir(AcirOpcode::AssertZero( + Expression::default(), + ))), call_stack: vec![OpcodeLocation::Acir(0)], count: 10, brillig_function_id: None, }, Sample { - opcode: AcirOrBrilligOpcode::Acir(AcirOpcode::AssertZero(Expression::default())), + opcode: Some(AcirOrBrilligOpcode::Acir(AcirOpcode::AssertZero( + Expression::default(), + ))), call_stack: vec![OpcodeLocation::Acir(1)], count: 20, brillig_function_id: None, }, Sample { - opcode: AcirOrBrilligOpcode::Acir(AcirOpcode::MemoryInit { + opcode: Some(AcirOrBrilligOpcode::Acir(AcirOpcode::MemoryInit { block_id: BlockId(0), init: vec![], block_type: acir::circuit::opcodes::BlockType::Memory, - }), + })), call_stack: vec![OpcodeLocation::Acir(2)], count: 30, brillig_function_id: None, diff --git a/noir/noir-repo/tooling/profiler/src/fs.rs b/noir/noir-repo/tooling/profiler/src/fs.rs index e8eec2cbb14..43848504a7f 100644 --- a/noir/noir-repo/tooling/profiler/src/fs.rs +++ b/noir/noir-repo/tooling/profiler/src/fs.rs @@ -1,6 +1,10 @@ -use std::path::Path; +use std::{collections::BTreeMap, path::Path}; use color_eyre::eyre; +use noirc_abi::{ + input_parser::{Format, InputValue}, + Abi, InputMap, MAIN_RETURN_NAME, +}; use noirc_artifacts::program::ProgramArtifact; pub(crate) fn read_program_from_file>( @@ -13,3 +17,26 @@ pub(crate) fn read_program_from_file>( Ok(program) } + +/// Returns the circuit's parameters and its return value, if one exists. +/// # Examples +/// +/// ```ignore +/// let (input_map, return_value): (InputMap, Option) = +/// read_inputs_from_file(path, "Verifier", Format::Toml, &abi)?; +/// ``` +pub(crate) fn read_inputs_from_file( + file_path: &Path, + format: Format, + abi: &Abi, +) -> eyre::Result<(InputMap, Option)> { + if abi.is_empty() { + return Ok((BTreeMap::new(), None)); + } + + let input_string = std::fs::read_to_string(file_path)?; + let mut input_map = format.parse(&input_string, abi)?; + let return_value = input_map.remove(MAIN_RETURN_NAME); + + Ok((input_map, return_value)) +}