diff --git a/docs/docs/migration_notes.md b/docs/docs/migration_notes.md index 6db903b05c21..bbfb931f1728 100644 --- a/docs/docs/migration_notes.md +++ b/docs/docs/migration_notes.md @@ -12,6 +12,22 @@ Aztec is in full-speed development. Literally every version breaks compatibility `get_token` and `get_portal_address` functions got merged into a single `get_config` function that returns a struct containing both the token and portal addresses. +### [Aztec.nr] `SharedMutable` can store size of packed length larger than 1 + +`SharedMutable` has been modified such that now it can store type `T` which packs to a length larger than 1. +This is a breaking change because now `SharedMutable` requires `T` to implement `Packable` trait instead of `ToField` and `FromField` traits. + +To implement the `Packable` trait for your type you can use the derive macro: + +```diff ++ use std::meta::derive; + ++ #[derive(Packable)] +pub struct YourType { + ... +} +``` + ### [Aztec.nr] Introduction of `WithHash` `WithHash` is a struct that allows for efficient reading of value `T` from public storage in private. diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable.nr b/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable.nr index 907a4c5e2002..e480aa390e8c 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable.nr @@ -1,20 +1,19 @@ -use dep::protocol_types::{ - address::AztecAddress, - hash::{poseidon2_hash, poseidon2_hash_with_separator}, - traits::{FromField, Packable, ToField}, - utils::arrays::array_concat, -}; - -use crate::context::{PrivateContext, PublicContext, UnconstrainedContext}; -use crate::oracle::storage::storage_read; -use crate::state_vars::{ - shared_mutable::{ - scheduled_delay_change::ScheduledDelayChange, scheduled_value_change::ScheduledValueChange, +use dep::protocol_types::traits::Packable; + +use crate::{ + context::{PrivateContext, PublicContext, UnconstrainedContext}, + state_vars::{ + shared_mutable::{ + scheduled_delay_change::ScheduledDelayChange, + scheduled_value_change::ScheduledValueChange, + shared_mutable_values::SharedMutableValues, + }, + storage::Storage, }, - storage::Storage, + utils::with_hash::WithHash, }; -use dep::std::mem::zeroed; +pub(crate) mod shared_mutable_values; pub(crate) mod scheduled_delay_change; pub(crate) mod scheduled_value_change; mod test; @@ -24,21 +23,11 @@ pub struct SharedMutable { storage_slot: Field, } -// Separators separating storage slot of different values within the same state variable -global VALUE_CHANGE_SEPARATOR: u32 = 0; -global DELAY_CHANGE_SEPARATOR: u32 = 1; -global HASH_SEPARATOR: u32 = 2; - -// This will make the Aztec macros require that T implements the Serialize trait, and allocate N storage slots to -// this state variable. This is incorrect, since what we actually store is: -// - a ScheduledValueChange, which requires 1 + 2 * M storage slots, where M is the serialization length of T -// - a ScheduledDelayChange, which requires another storage slot -// -// 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 +// This will make the Aztec macros require that T implements the Packable and Eq traits, and allocate `M` storage +// slots to this state variable. +impl Storage for SharedMutable where - T: Packable, + WithHash, _>: Packable, { fn get_storage_slot(self) -> Field { self.storage_slot @@ -56,39 +45,27 @@ where // future, so that they can guarantee the value will not have possibly changed by then (because of the delay). // The delay for changing a value is initially equal to INITIAL_DELAY, but can be changed by calling // `schedule_delay_change`. -impl SharedMutable +impl SharedMutable where - T: ToField + FromField + Eq, + T: Packable + Eq, { pub fn new(context: Context, storage_slot: Field) -> Self { assert(storage_slot != 0, "Storage slot 0 not allowed. Storage slots must start from 1."); Self { context, storage_slot } } - // Since we can't rely on the native storage allocation scheme, we hash the storage slot to get a unique location in - // which we can safely store as much data as we need. - // See https://github.com/AztecProtocol/aztec-packages/issues/5492 and - // https://github.com/AztecProtocol/aztec-packages/issues/5736 - // We store three things in public storage: - // - a ScheduledValueChange - // - a ScheduledDelaChange - // - the hash of both of these (via `hash_scheduled_data`) fn get_value_change_storage_slot(self) -> Field { - poseidon2_hash_with_separator([self.storage_slot], VALUE_CHANGE_SEPARATOR) + SharedMutableValues::::get_value_change_storage_slot(self.storage_slot) } fn get_delay_change_storage_slot(self) -> Field { - poseidon2_hash_with_separator([self.storage_slot], DELAY_CHANGE_SEPARATOR) - } - - fn get_hash_storage_slot(self) -> Field { - poseidon2_hash_with_separator([self.storage_slot], HASH_SEPARATOR) + SharedMutableValues::::get_delay_change_storage_slot(self.storage_slot) } } -impl SharedMutable +impl SharedMutable where - T: ToField + FromField + Eq, + T: Packable + Eq, { pub fn schedule_value_change(self, new_value: T) { @@ -147,24 +124,22 @@ where value_change: ScheduledValueChange, delay_change: ScheduledDelayChange, ) { - // Whenever we write to public storage, we write both the value change and delay change as well as the hash of - // them both. This guarantees that the hash is always kept up to date. - // While this makes for more costly writes, it also makes private proofs much simpler because they only need to - // produce a historical proof for the hash, which results in a single inclusion proof (as opposed to 4 in the - // best case scenario in which T is a single field). Private shared mutable reads are assumed to be much more - // frequent than public writes, so this tradeoff makes sense. - self.context.storage_write(self.get_value_change_storage_slot(), value_change); - self.context.storage_write(self.get_delay_change_storage_slot(), delay_change); - self.context.storage_write( - self.get_hash_storage_slot(), - SharedMutable::hash_scheduled_data(value_change, delay_change), - ); + // Whenever we write to public storage, we write both the value change and delay change to storage at once. + // We do so by wrapping them in a single struct (`SharedMutableValues`). Then we wrap the resulting struct in + // `WithHash`. + // Wrapping in `WithHash` makes for more costly writes but it also makes private proofs much simpler because + // they only need to produce a historical proof for the hash, which results in a single inclusion proof (as + // opposed to 4 in the best case scenario in which T is a single field). Private shared mutable reads are + // assumed to be much more frequent than public writes, so this tradeoff makes sense. + let values = WithHash::new(SharedMutableValues::new(value_change, delay_change)); + + self.context.storage_write(self.storage_slot, values); } } -impl SharedMutable +impl SharedMutable where - T: ToField + FromField + Eq, + T: Packable + Eq, { pub fn get_current_value(self) -> T { // When reading the current value in private we construct a historical state proof for the public value. @@ -200,83 +175,24 @@ where let historical_block_number = header.global_variables.block_number as u32; - // We could simply produce historical inclusion proofs for both the ScheduledValueChange and - // ScheduledDelayChange, but that'd require one full sibling path per storage slot (since due to kernel siloing - // the storage is not contiguous), and in the best case in which T is a single field that'd be 4 slots. - // Instead, we get an oracle to provide us the correct values for both the value and delay changes, and instead - // prove inclusion of their hash, which is both a much smaller proof (a single slot), and also independent of - // the size of T. - /// Safety: The hints are checked to be a preimage of a hash obtained from constrained context. - let (value_change_hint, delay_change_hint) = unsafe { - get_public_storage_hints(address, self.storage_slot, historical_block_number) - }; - - // Ideally the following would be simply public_storage::read_historical, but we can't implement that yet. - let hash = header.public_storage_historical_read(self.get_hash_storage_slot(), address); - - if hash != 0 { - assert_eq( - hash, - SharedMutable::hash_scheduled_data(value_change_hint, delay_change_hint), - "Hint values do not match hash", - ); - } else { - // The hash slot can only hold a zero if it is uninitialized, meaning no value or delay change was ever - // scheduled. Therefore, the hints must then correspond to uninitialized scheduled changes. - assert_eq( - value_change_hint, - ScheduledValueChange::unpack(zeroed()), - "Non-zero value change for zero hash", - ); - assert_eq( - delay_change_hint, - ScheduledDelayChange::unpack(zeroed()), - "Non-zero delay change for zero hash", - ); - }; - - (value_change_hint, delay_change_hint, historical_block_number) - } + let values: SharedMutableValues = + WithHash::historical_public_storage_read(header, address, self.storage_slot); - fn hash_scheduled_data( - value_change: ScheduledValueChange, - delay_change: ScheduledDelayChange, - ) -> Field { - let concatenated: [Field; 4] = array_concat(value_change.pack(), delay_change.pack()); - poseidon2_hash(concatenated) + (values.svc, values.sdc, historical_block_number) } } -impl SharedMutable +impl SharedMutable where - T: ToField + FromField + Eq, + T: Packable + Eq, { pub unconstrained fn get_current_value(self) -> T { - let block_number = self.context.block_number() as u32; - self.read_value_change().get_current_at(block_number) - } + let value_change: ScheduledValueChange = WithHash::unconstrained_public_storage_read( + self.context, + self.get_value_change_storage_slot(), + ); - unconstrained fn read_value_change(self) -> ScheduledValueChange { - self.context.storage_read(self.get_value_change_storage_slot()) + let block_number = self.context.block_number() as u32; + value_change.get_current_at(block_number) } } - -unconstrained fn get_public_storage_hints( - address: AztecAddress, - storage_slot: Field, - block_number: u32, -) -> (ScheduledValueChange, ScheduledDelayChange) -where - T: ToField + FromField + Eq, -{ - // This function cannot be part of the &mut PrivateContext impl because that'd mean that by passing `self` we'd also - // be passing a mutable reference to an unconstrained function, which is not allowed. We therefore create a dummy - // state variable here so that we can access the methods to compute storage slots. This will all be removed in the - // future once we do proper storage slot allocation (#5492). - let dummy: SharedMutable = SharedMutable::new((), storage_slot); - - ( - storage_read(address, dummy.get_value_change_storage_slot(), block_number), - storage_read(address, dummy.get_delay_change_storage_slot(), block_number), - ) -} diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/scheduled_delay_change.nr b/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/scheduled_delay_change.nr index d325dbd0a0f0..8ffd57d09e4e 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/scheduled_delay_change.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/scheduled_delay_change.nr @@ -3,6 +3,8 @@ use std::cmp::min; mod test; +pub(crate) global SCHEDULED_DELAY_CHANGE_PCKD_LEN: u32 = 1; + // This data structure is used by SharedMutable to store the minimum delay with which a ScheduledValueChange object can // schedule a change. // This delay is initially equal to INITIAL_DELAY, and can be safely mutated to any other value over time. This mutation @@ -125,8 +127,8 @@ impl ScheduledDelayChange { } } -impl Packable<1> for ScheduledDelayChange { - fn pack(self) -> [Field; 1] { +impl Packable for ScheduledDelayChange { + fn pack(self) -> [Field; SCHEDULED_DELAY_CHANGE_PCKD_LEN] { // We pack all three u32 values into a single U128, which is made up of two u64 limbs. // Low limb: [ pre_inner: u32 | post_inner: u32 ] // High limb: [ empty | pre_is_some: u8 | post_is_some: u8 | block_of_change: u32 ] @@ -142,7 +144,7 @@ impl Packable<1> for ScheduledDelayChange [packed.to_integer()] } - fn unpack(input: [Field; 1]) -> Self { + fn unpack(input: [Field; SCHEDULED_DELAY_CHANGE_PCKD_LEN]) -> Self { let packed = U128::from_integer(input[0]); // We use division and modulo to clear the bits that correspond to other values when unpacking. diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/scheduled_value_change.nr b/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/scheduled_value_change.nr index eeb55c46e760..17db009b9d60 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/scheduled_value_change.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/scheduled_value_change.nr @@ -1,4 +1,5 @@ -use dep::protocol_types::traits::{FromField, Packable, ToField}; +use crate::utils::array; +use dep::protocol_types::traits::Packable; use std::cmp::min; mod test; @@ -133,19 +134,29 @@ impl ScheduledValueChange { } } -impl Packable<3> for ScheduledValueChange +// We store 2 Ts and one extra field for the block of change. +impl Packable<2 * N + 1> for ScheduledValueChange where - T: ToField + FromField, + T: Packable, { - fn pack(self) -> [Field; 3] { - [self.pre.to_field(), self.post.to_field(), self.block_of_change.to_field()] + fn pack(self) -> [Field; 2 * N + 1] { + let mut packed = [0; 2 * N + 1]; + let pre_packed = self.pre.pack(); + let post_packed = self.post.pack(); + + for i in 0..N { + packed[i] = pre_packed[i]; + packed[N + i] = post_packed[i]; + } + packed[2 * N] = self.block_of_change.to_field(); + packed } - fn unpack(input: [Field; 3]) -> Self { + fn unpack(input: [Field; 2 * N + 1]) -> Self { Self { - pre: FromField::from_field(input[0]), - post: FromField::from_field(input[1]), - block_of_change: FromField::from_field(input[2]), + pre: T::unpack(array::subarray(input, 0)), + post: T::unpack(array::subarray(input, N)), + block_of_change: input[2 * N] as u32, } } } diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/shared_mutable_values.nr b/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/shared_mutable_values.nr new file mode 100644 index 000000000000..1eb1fd5be3d3 --- /dev/null +++ b/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/shared_mutable_values.nr @@ -0,0 +1,55 @@ +use crate::state_vars::shared_mutable::{ + scheduled_delay_change::{SCHEDULED_DELAY_CHANGE_PCKD_LEN, ScheduledDelayChange}, + scheduled_value_change::ScheduledValueChange, +}; +use crate::utils::array; +use dep::protocol_types::{traits::Packable, utils::arrays::array_concat}; +use std::meta::derive; + +/// SharedMutableValues is just a wrapper around ScheduledValueChange and ScheduledDelayChange that then allows us +/// to wrap both of these values in WithHash. WithHash allows for efficient read of values in private. +/// +/// Note that the WithHash optimization does not work in public (due to there being no unconstrained). But we also want +/// to be able to read the values efficiently in public and we want to be able to read each value separately. For that +/// reason we expose `get_delay_change_storage_slot` and `get_value_change_storage_slot` which point to the correct +/// location in the storage. This is "hacky" as we pack and store the values together but there is no way around it. +#[derive(Eq)] +pub(crate) struct SharedMutableValues { + svc: ScheduledValueChange, + sdc: ScheduledDelayChange, +} + +impl SharedMutableValues { + pub(crate) fn new( + svc: ScheduledValueChange, + sdc: ScheduledDelayChange, + ) -> Self { + SharedMutableValues { svc, sdc } + } + + pub fn get_delay_change_storage_slot(shared_mutable_storage_slot: Field) -> Field { + shared_mutable_storage_slot + } + + pub fn get_value_change_storage_slot(shared_mutable_storage_slot: Field) -> Field { + shared_mutable_storage_slot + (SCHEDULED_DELAY_CHANGE_PCKD_LEN as Field) + } +} + +// We pack to `2 * N + 1 + SCHEDULED_DELAY_CHANGE_PCKD_LEN` fields because ScheduledValueChange contains T twice +// + 1 field for block_of_change + SCHEDULED_DELAY_CHANGE_PCKD_LEN fields for ScheduledDelayChange +impl Packable<2 * N + 1 + SCHEDULED_DELAY_CHANGE_PCKD_LEN> for SharedMutableValues +where + T: Packable, +{ + fn pack(self) -> [Field; 2 * N + 1 + SCHEDULED_DELAY_CHANGE_PCKD_LEN] { + array_concat(self.sdc.pack(), self.svc.pack()) + } + + fn unpack(fields: [Field; 2 * N + 1 + SCHEDULED_DELAY_CHANGE_PCKD_LEN]) -> Self { + SharedMutableValues { + sdc: Packable::unpack(array::subarray(fields, 0)), + svc: Packable::unpack(array::subarray(fields, SCHEDULED_DELAY_CHANGE_PCKD_LEN)), + } + } +} diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/test.nr b/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/test.nr index 8983b35e44ae..d4d5a8f143d4 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/test.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/test.nr @@ -1,15 +1,12 @@ use crate::{ context::{PrivateContext, PublicContext, UnconstrainedContext}, - state_vars::shared_mutable::{ - scheduled_delay_change::ScheduledDelayChange, scheduled_value_change::ScheduledValueChange, - SharedMutable, - }, - test::helpers::test_environment::TestEnvironment, + state_vars::shared_mutable::SharedMutable, + test::{helpers::test_environment::TestEnvironment, mocks::mock_struct::MockStruct}, }; -use dep::std::{mem::zeroed, test::OracleMock}; +use dep::std::mem::zeroed; -global new_value: Field = 17; +global new_value: MockStruct = MockStruct { a: 17, b: 42 }; global new_delay: u32 = 20; @@ -23,20 +20,20 @@ unconstrained fn setup() -> TestEnvironment { unconstrained fn in_public( env: TestEnvironment, -) -> SharedMutable { +) -> SharedMutable { SharedMutable::new(&mut env.public(), storage_slot) } unconstrained fn in_private( env: &mut TestEnvironment, historical_block_number: u32, -) -> SharedMutable { +) -> SharedMutable { SharedMutable::new(&mut env.private_at(historical_block_number), storage_slot) } unconstrained fn in_unconstrained( env: TestEnvironment, -) -> SharedMutable { +) -> SharedMutable { SharedMutable::new(env.unkonstrained(), storage_slot) } @@ -197,7 +194,7 @@ unconstrained fn test_get_current_value_in_private_before_change() { let schedule_block_number = env.block_number(); let private_state_var = in_private(&mut env, schedule_block_number); - assert_eq(private_state_var.get_current_value(), 0); + assert_eq(private_state_var.get_current_value(), MockStruct::empty()); assert_eq(private_state_var.context.max_block_number.unwrap(), block_of_change - 1); } @@ -214,7 +211,7 @@ unconstrained fn test_get_current_value_in_private_immediately_before_change() { // Note that this transaction would never be valid since the max block number is the same as the historical block // used to built the proof, i.e. in the past. - assert_eq(private_state_var.get_current_value(), 0); + assert_eq(private_state_var.get_current_value(), MockStruct::empty()); assert_eq(private_state_var.context.max_block_number.unwrap(), block_of_change - 1); } @@ -279,91 +276,6 @@ unconstrained fn test_get_current_value_in_private_with_non_initial_delay() { ); } -#[test(should_fail_with = "Hint values do not match hash")] -unconstrained fn test_get_current_value_in_private_bad_value_hints() { - let mut env = setup(); - - let public_state_var = in_public(env); - public_state_var.schedule_value_change(new_value); - - let schedule_block_number = env.block_number(); - let private_state_var = in_private(&mut env, schedule_block_number); - - let mocked: ScheduledValueChange = - ScheduledValueChange::new(0, new_value + 1, schedule_block_number); - let _ = OracleMock::mock("storageRead") - .with_params(( - env.contract_address().to_field(), private_state_var.get_value_change_storage_slot(), - schedule_block_number, 3, - )) - .returns(mocked.pack()) - .times(1); - - let _ = private_state_var.get_current_value(); -} - -#[test(should_fail_with = "Hint values do not match hash")] -unconstrained fn test_get_current_value_in_private_bad_delay_hints() { - let mut env = setup(); - - let public_state_var = in_public(env); - public_state_var.schedule_value_change(new_value); - - let schedule_block_number = env.block_number(); - let private_state_var = in_private(&mut env, schedule_block_number); - - let mocked: ScheduledDelayChange = - ScheduledDelayChange::new(Option::none(), Option::some(42), schedule_block_number); - let _ = OracleMock::mock("storageRead") - .with_params(( - env.contract_address().to_field(), private_state_var.get_delay_change_storage_slot(), - schedule_block_number, 1, - )) - .returns(mocked.pack()) - .times(1); - - let _ = private_state_var.get_current_value(); -} - -#[test(should_fail_with = "Non-zero value change for zero hash")] -unconstrained fn test_get_current_value_in_private_bad_zero_hash_value_hints() { - let mut env = setup(); - - let historical_block_number = env.block_number(); - let state_var = in_private(&mut env, historical_block_number); - - let mocked: ScheduledValueChange = ScheduledValueChange::new(0, new_value, 0); - let _ = OracleMock::mock("storageRead") - .with_params(( - env.contract_address().to_field(), state_var.get_value_change_storage_slot(), - historical_block_number, 3, - )) - .returns(mocked.pack()) - .times(1); - - let _ = state_var.get_current_value(); -} - -#[test(should_fail_with = "Non-zero delay change for zero hash")] -unconstrained fn test_get_current_value_in_private_bad_zero_hash_delay_hints() { - let mut env = setup(); - - let historical_block_number = env.block_number(); - let state_var = in_private(&mut env, historical_block_number); - - let mocked: ScheduledDelayChange = - ScheduledDelayChange::new(Option::none(), Option::some(new_delay), 0); - let _ = OracleMock::mock("storageRead") - .with_params(( - env.contract_address().to_field(), state_var.get_delay_change_storage_slot(), - historical_block_number, 1, - )) - .returns(mocked.pack()) - .times(1); - - let _ = state_var.get_current_value(); -} - #[test] unconstrained fn test_get_current_value_in_unconstrained_initial() { let env = setup(); diff --git a/noir-projects/aztec-nr/aztec/src/test/mocks/mock_struct.nr b/noir-projects/aztec-nr/aztec/src/test/mocks/mock_struct.nr index eb252267fc36..e9dd02c811d8 100644 --- a/noir-projects/aztec-nr/aztec/src/test/mocks/mock_struct.nr +++ b/noir-projects/aztec-nr/aztec/src/test/mocks/mock_struct.nr @@ -1,4 +1,4 @@ -use dep::protocol_types::traits::{Deserialize, Packable, Serialize}; +use dep::protocol_types::traits::{Deserialize, Empty, Packable, Serialize}; pub struct MockStruct { pub a: Field, @@ -17,6 +17,12 @@ impl Eq for MockStruct { } } +impl Empty for MockStruct { + fn empty() -> Self { + Self { a: 0, b: 0 } + } +} + impl Serialize<2> for MockStruct { fn serialize(self) -> [Field; 2] { [self.a, self.b] 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 a8461de2020d..1103419ac3d3 100644 --- a/noir-projects/noir-contracts/contracts/test_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/test_contract/src/main.nr @@ -14,7 +14,7 @@ pub contract Test { }; use dep::aztec::prelude::{ AztecAddress, EthAddress, FunctionSelector, Map, NoteGetterOptions, NoteViewerOptions, - PrivateImmutable, PrivateSet, PublicImmutable, + PrivateImmutable, PrivateSet, PublicImmutable, SharedMutable, }; use dep::aztec::protocol_types::{ @@ -61,7 +61,7 @@ pub contract Test { value4: Field, } - #[derive(Packable)] + #[derive(Eq, Packable)] struct ExampleStruct { value0: Field, value1: Field, @@ -70,11 +70,14 @@ pub contract Test { value4: Field, } + global SHARED_MUTABLE_INITIAL_DELAY: u32 = 2; + // This struct is used to test the storage slot allocation mechanism - if modified the test_storage_slot_allocation // test function must also be updated accordingly. #[storage] struct Storage { example_constant: PrivateImmutable, + example_struct_in_shared_mutable: SharedMutable, example_set: PrivateSet, example_struct: PrivateImmutable, example_struct_in_public_immutable: PublicImmutable, diff --git a/noir-projects/noir-contracts/contracts/test_contract/src/test.nr b/noir-projects/noir-contracts/contracts/test_contract/src/test.nr index 2318510695f4..9a299c74d71d 100644 --- a/noir-projects/noir-contracts/contracts/test_contract/src/test.nr +++ b/noir-projects/noir-contracts/contracts/test_contract/src/test.nr @@ -23,6 +23,7 @@ unconstrained fn test_storage_slot_allocation() { // #[storage] // struct Storage { // example_constant: PrivateImmutable, + // example_struct_in_shared_mutable: SharedMutable, // example_set: PrivateSet, // example_struct: PrivateImmutable, // example_struct_in_public_immutable: PublicImmutable, @@ -41,6 +42,9 @@ unconstrained fn test_storage_slot_allocation() { // single slot. expected_slot += 1; + assert_eq(Test::storage_layout().example_struct_in_shared_mutable.slot, expected_slot); + expected_slot += get_shared_mutable_num_slots(); + assert_eq(Test::storage_layout().example_set.slot, expected_slot); // example_set also held a note, so it should have only allocated a single slot. expected_slot += 1; @@ -51,13 +55,17 @@ unconstrained fn test_storage_slot_allocation() { assert_eq(Test::storage_layout().example_struct_in_public_immutable.slot, expected_slot); // example_struct_in_public_immutable should allocate 6 slots because ExampleStruct occupies 5 slots - // and `PublicImmutable` wraps the struct in a `WithHash` that adds an additional slot. expected_slot += 6; assert_eq(Test::storage_layout().example_struct_in_map.slot, expected_slot); - // example_struct_in_map should allocate a single slot because it's a map, regardless of whatever it holds. The Map - // type is going to deal with its own dynamic allocation based on keys - expected_slot += 1; +} + +fn get_shared_mutable_num_slots() -> Field { + let scheduled_delay_packed_len = 1; // There is 1 ScheduledDelayChange stored in storage and it fits into 1 slot. + let example_struct_packed_len = 5; // ExampleStruct packs to 5 slots. + // There are 2 example structs stored in the packed representation of ScheduledValueChange. + let scheduled_value_change_packed_len = (example_struct_packed_len * 2 + 1); + let with_hash_additional_slots = 1; // Wrapping the values in WithHash adds 1 slot. - assert_eq(Test::storage_layout().another_example_struct.slot, expected_slot); + scheduled_delay_packed_len + scheduled_value_change_packed_len + with_hash_additional_slots } diff --git a/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/types/roles.nr b/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/types/roles.nr index 46b784f6899d..0a075b07292a 100644 --- a/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/types/roles.nr +++ b/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/types/roles.nr @@ -1,4 +1,4 @@ -use dep::aztec::protocol_types::traits::{Deserialize, FromField, Serialize, ToField}; +use dep::aztec::protocol_types::traits::{Deserialize, Packable, Serialize}; global ADMIN_FLAG: u64 = 1; global MINTER_FLAG: u64 = 2; @@ -10,19 +10,8 @@ pub struct UserFlags { is_blacklisted: bool, } -impl FromField for UserFlags { - fn from_field(value: Field) -> UserFlags { - let value: u64 = value as u64; - let is_admin = value & ADMIN_FLAG == ADMIN_FLAG; - let is_minter = value & MINTER_FLAG == MINTER_FLAG; - let is_blacklisted = value & BLACKLIST_FLAG == BLACKLIST_FLAG; - - Self { is_admin, is_minter, is_blacklisted } - } -} - -impl ToField for UserFlags { - fn to_field(self) -> Field { +impl Packable<1> for UserFlags { + fn pack(self) -> [Field; 1] { let mut value: u64 = 0; if self.is_admin { @@ -37,7 +26,16 @@ impl ToField for UserFlags { value = value | BLACKLIST_FLAG; } - value.to_field() + [value.to_field()] + } + + fn unpack(fields: [Field; 1]) -> Self { + let value: u64 = fields[0] as u64; + let is_admin = value & ADMIN_FLAG == ADMIN_FLAG; + let is_minter = value & MINTER_FLAG == MINTER_FLAG; + let is_blacklisted = value & BLACKLIST_FLAG == BLACKLIST_FLAG; + + Self { is_admin, is_minter, is_blacklisted } } } @@ -75,9 +73,9 @@ impl Deserialize<3> for UserFlags { mod test { use crate::types::roles::UserFlags; - fn assert_to_from_field(is_minter: bool, is_admin: bool, is_blacklisted: bool) { + fn assert_packable(is_minter: bool, is_admin: bool, is_blacklisted: bool) { let flags = UserFlags { is_minter, is_admin, is_blacklisted }; - let converted = UserFlags::from_field(flags.to_field()); + let converted = UserFlags::unpack(flags.pack()); assert_eq(converted.is_minter, is_minter); assert_eq(converted.is_admin, is_admin); @@ -86,16 +84,16 @@ mod test { #[test] fn test_to_from_field() { - assert_to_from_field(false, false, false); - assert_to_from_field(false, false, true); + assert_packable(false, false, false); + assert_packable(false, false, true); - assert_to_from_field(false, true, false); - assert_to_from_field(false, true, true); + assert_packable(false, true, false); + assert_packable(false, true, true); - assert_to_from_field(true, false, false); - assert_to_from_field(true, false, true); + assert_packable(true, false, false); + assert_packable(true, false, true); - assert_to_from_field(true, true, false); - assert_to_from_field(true, true, true); + assert_packable(true, true, false); + assert_packable(true, true, true); } }