From 2643a45789c6d76b61bcc8996dfea496be111ce7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Fri, 22 Nov 2024 23:06:11 +0000 Subject: [PATCH] Rename SharedMutable methods --- docs/docs/migration_notes.md | 14 + .../storage/shared_state.md | 12 +- .../aztec/src/state_vars/shared_mutable.nr | 277 +++++++++++++++++- .../shared_mutable/shared_mutable.nr | 276 ----------------- .../src/state_vars/shared_mutable/test.nr | 86 +++--- .../contracts/auth_contract/src/main.nr | 10 +- .../token_blacklist_contract/src/main.nr | 30 +- 7 files changed, 356 insertions(+), 349 deletions(-) delete mode 100644 noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/shared_mutable.nr diff --git a/docs/docs/migration_notes.md b/docs/docs/migration_notes.md index cf3a5b89368..4b258efb087 100644 --- a/docs/docs/migration_notes.md +++ b/docs/docs/migration_notes.md @@ -8,6 +8,20 @@ Aztec is in full-speed development. Literally every version breaks compatibility ## TBD +### [aztec.nr] SharedMutable renamings + +`SharedMutable` getters (`get_current_value_in_public`, etc.) were renamed by dropping the `_in` suffix, since only one of these versions is ever available depending on the current context. + +```diff +// In private +- let value = storage.my_var.get_current_value_in_private(); ++ let value = storage.my_var.get_current_value(); + +// In public +- let value = storage.my_var.get_current_value_in_public(); ++ let value = storage.my_var.get_current_value(); +``` + ### [aztec.js] Random addresses are now valid The `AztecAddress.random()` function now returns valid addresses, i.e. addresses that can receive encrypted messages and therefore have notes be sent to them. `AztecAddress.isValid()` was also added to check for validity of an address. diff --git a/docs/docs/reference/developer_references/smart_contract_reference/storage/shared_state.md b/docs/docs/reference/developer_references/smart_contract_reference/storage/shared_state.md index 6ba216f1283..f7c08262556 100644 --- a/docs/docs/reference/developer_references/smart_contract_reference/storage/shared_state.md +++ b/docs/docs/reference/developer_references/smart_contract_reference/storage/shared_state.md @@ -80,21 +80,17 @@ If one wishes to schedule a value change from private, simply enqueue a public c A `SharedMutable`'s storage **must** only be mutated via `schedule_value_change`. Attempting to override this by manually accessing the underlying storage slots breaks all properties of the data structure, rendering it useless. ::: -### `get_current_value_in_public` +### `get_current_value` -Returns the current value in a public execution context. Once a value change is scheduled via `schedule_value_change` and a number of blocks equal to the delay passes, this automatically returns the new value. +Returns the current value in a public, private or unconstrained execution context. Once a value change is scheduled via `schedule_value_change` and a number of blocks equal to the delay passes, this automatically returns the new value. #include_code shared_mutable_get_current_public /noir-projects/noir-contracts/contracts/auth_contract/src/main.nr rust -### `get_current_value_in_private` - -Returns the current value in a private execution context. Once a value change is scheduled via `schedule_value_change` and a number of blocks equal to the delay passes, this automatically returns the new value. - -Calling this function will set the `max_block_number` property of the transaction request, introducing a new validity condition to the entire transaction: it cannot be included in any block with a block number larger than `max_block_number`. This could [potentially leak some privacy](#privacy-considerations). +Calling this function in a private execution context will set the `max_block_number` property of the transaction request, introducing a new validity condition to the entire transaction: it cannot be included in any block with a block number larger than `max_block_number`. This could [potentially leak some privacy](#privacy-considerations). #include_code shared_mutable_get_current_private /noir-projects/noir-contracts/contracts/auth_contract/src/main.nr rust -### `get_scheduled_value_in_public` +### `get_scheduled_value` Returns the last scheduled value change, along with the block number at which the scheduled value becomes the current value. This may either be a pending change, if the block number is in the future, or the last executed scheduled change if the block number is in the past (in which case there are no pending changes). 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 8b8cde20948..a56f06c7f66 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,5 +1,278 @@ -pub mod shared_mutable; +use dep::protocol_types::{ + address::AztecAddress, + hash::{poseidon2_hash, poseidon2_hash_with_separator}, + traits::{Deserialize, FromField, Serialize, 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, + }, + storage::Storage, +}; +use dep::std::mem::zeroed; + pub(crate) mod scheduled_delay_change; pub(crate) mod scheduled_value_change; +mod test; + +pub struct SharedMutable { + context: Context, + 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 +where + T: Serialize + Deserialize, +{} + +// SharedMutable stores a value of type T that is: +// - publicly known (i.e. unencrypted) +// - mutable in public +// - readable in private with no contention (i.e. multiple parties can all read the same value without blocking one +// another nor needing to coordinate) +// This is famously a hard problem to solve. SharedMutable makes it work by introducing a delay to public mutation: +// the value is not changed immediately but rather a value change is scheduled to happen in the future after some delay +// measured in blocks. Reads in private are only valid as long as they are included in a block not too far into the +// 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 +where + T: ToField + FromField + 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) + } + + 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) + } +} + +impl SharedMutable +where + T: ToField + FromField + Eq, +{ + + pub fn schedule_value_change(self, new_value: T) { + let mut value_change = self.read_value_change(); + let delay_change = self.read_delay_change(); + + let block_number = self.context.block_number() as u32; + let current_delay = delay_change.get_current(block_number); + + // TODO: make this configurable + // https://github.com/AztecProtocol/aztec-packages/issues/5501 + let block_of_change = block_number + current_delay; + value_change.schedule_change(new_value, block_number, current_delay, block_of_change); + + self.write(value_change, delay_change); + } + + pub fn schedule_delay_change(self, new_delay: u32) { + let mut delay_change = self.read_delay_change(); + + let block_number = self.context.block_number() as u32; + + delay_change.schedule_change(new_delay, block_number); + + self.write(self.read_value_change(), delay_change); + } + + pub fn get_current_value(self) -> T { + let block_number = self.context.block_number() as u32; + self.read_value_change().get_current_at(block_number) + } + + pub fn get_current_delay(self) -> u32 { + let block_number = self.context.block_number() as u32; + self.read_delay_change().get_current(block_number) + } + + pub fn get_scheduled_value(self) -> (T, u32) { + self.read_value_change().get_scheduled() + } + + pub fn get_scheduled_delay(self) -> (u32, u32) { + self.read_delay_change().get_scheduled() + } + + fn read_value_change(self) -> ScheduledValueChange { + self.context.storage_read(self.get_value_change_storage_slot()) + } + + fn read_delay_change(self) -> ScheduledDelayChange { + self.context.storage_read(self.get_delay_change_storage_slot()) + } + + fn write( + self, + 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), + ); + } +} + +impl SharedMutable +where + T: ToField + FromField + 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. + // However, since this value might change, we must constrain the maximum transaction block number as this proof + // will only be valid for however many blocks we can ensure the value will not change, which will depend on the + // current delay and any scheduled delay changes. + let (value_change, delay_change, historical_block_number) = + self.historical_read_from_public_storage(); + + // We use the effective minimum delay as opposed to the current delay at the historical block as this one also + // takes into consideration any scheduled delay changes. + // For example, consider a scenario in which at block 200 the current delay was 50. We may naively think that + // the earliest we could change the value would be at block 251 by scheduling immediately after the historical + // block, i.e. at block 201. But if there was a delay change scheduled for block 210 to reduce the delay to 20 + // blocks, then if a value change was scheduled at block 210 it would go into effect at block 230, which is + // earlier than what we'd expect if we only considered the current delay. + let effective_minimum_delay = + delay_change.get_effective_minimum_delay_at(historical_block_number); + let block_horizon = + value_change.get_block_horizon(historical_block_number, effective_minimum_delay); + + // We prevent this transaction from being included in any block after the block horizon, ensuring that the + // historical public value matches the current one, since it can only change after the horizon. + self.context.set_tx_max_block_number(block_horizon); + value_change.get_current_at(historical_block_number) + } + + fn historical_read_from_public_storage( + self, + ) -> (ScheduledValueChange, ScheduledDelayChange, u32) { + let header = self.context.get_header(); + let address = self.context.this_address(); + + 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. + 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::deserialize(zeroed()), + "Non-zero value change for zero hash", + ); + assert_eq( + delay_change_hint, + ScheduledDelayChange::deserialize(zeroed()), + "Non-zero delay change for zero hash", + ); + }; + + (value_change_hint, delay_change_hint, historical_block_number) + } + + fn hash_scheduled_data( + value_change: ScheduledValueChange, + delay_change: ScheduledDelayChange, + ) -> Field { + let concatenated: [Field; 4] = + array_concat(value_change.serialize(), delay_change.serialize()); + poseidon2_hash(concatenated) + } +} + +impl SharedMutable +where + T: ToField + FromField + 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) + } + + unconstrained fn read_value_change(self) -> ScheduledValueChange { + self.context.storage_read(self.get_value_change_storage_slot()) + } +} + +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); -pub use shared_mutable::SharedMutable; + ( + 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/shared_mutable.nr b/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/shared_mutable.nr deleted file mode 100644 index 0d1d76d984a..00000000000 --- a/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/shared_mutable.nr +++ /dev/null @@ -1,276 +0,0 @@ -use dep::protocol_types::{ - address::AztecAddress, - hash::{poseidon2_hash, poseidon2_hash_with_separator}, - traits::{Deserialize, FromField, Serialize, 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, - }, - storage::Storage, -}; -use dep::std::mem::zeroed; - -mod test; - -pub struct SharedMutable { - context: Context, - 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 -where - T: Serialize + Deserialize, -{} - -// SharedMutable stores a value of type T that is: -// - publicly known (i.e. unencrypted) -// - mutable in public -// - readable in private with no contention (i.e. multiple parties can all read the same value without blocking one -// another nor needing to coordinate) -// This is famously a hard problem to solve. SharedMutable makes it work by introducing a delay to public mutation: -// the value is not changed immediately but rather a value change is scheduled to happen in the future after some delay -// measured in blocks. Reads in private are only valid as long as they are included in a block not too far into the -// 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 -where - T: ToField + FromField + 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) - } - - 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) - } -} - -impl SharedMutable -where - T: ToField + FromField + Eq, -{ - - pub fn schedule_value_change(self, new_value: T) { - let mut value_change = self.read_value_change(); - let delay_change = self.read_delay_change(); - - let block_number = self.context.block_number() as u32; - let current_delay = delay_change.get_current(block_number); - - // TODO: make this configurable - // https://github.com/AztecProtocol/aztec-packages/issues/5501 - let block_of_change = block_number + current_delay; - value_change.schedule_change(new_value, block_number, current_delay, block_of_change); - - self.write(value_change, delay_change); - } - - pub fn schedule_delay_change(self, new_delay: u32) { - let mut delay_change = self.read_delay_change(); - - let block_number = self.context.block_number() as u32; - - delay_change.schedule_change(new_delay, block_number); - - self.write(self.read_value_change(), delay_change); - } - - pub fn get_current_value_in_public(self) -> T { - let block_number = self.context.block_number() as u32; - self.read_value_change().get_current_at(block_number) - } - - pub fn get_current_delay_in_public(self) -> u32 { - let block_number = self.context.block_number() as u32; - self.read_delay_change().get_current(block_number) - } - - pub fn get_scheduled_value_in_public(self) -> (T, u32) { - self.read_value_change().get_scheduled() - } - - pub fn get_scheduled_delay_in_public(self) -> (u32, u32) { - self.read_delay_change().get_scheduled() - } - - fn read_value_change(self) -> ScheduledValueChange { - self.context.storage_read(self.get_value_change_storage_slot()) - } - - fn read_delay_change(self) -> ScheduledDelayChange { - self.context.storage_read(self.get_delay_change_storage_slot()) - } - - fn write( - self, - 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), - ); - } -} - -impl SharedMutable -where - T: ToField + FromField + Eq, -{ - pub fn get_current_value_in_private(self) -> T { - // When reading the current value in private we construct a historical state proof for the public value. - // However, since this value might change, we must constrain the maximum transaction block number as this proof - // will only be valid for however many blocks we can ensure the value will not change, which will depend on the - // current delay and any scheduled delay changes. - let (value_change, delay_change, historical_block_number) = - self.historical_read_from_public_storage(); - - // We use the effective minimum delay as opposed to the current delay at the historical block as this one also - // takes into consideration any scheduled delay changes. - // For example, consider a scenario in which at block 200 the current delay was 50. We may naively think that - // the earliest we could change the value would be at block 251 by scheduling immediately after the historical - // block, i.e. at block 201. But if there was a delay change scheduled for block 210 to reduce the delay to 20 - // blocks, then if a value change was scheduled at block 210 it would go into effect at block 230, which is - // earlier than what we'd expect if we only considered the current delay. - let effective_minimum_delay = - delay_change.get_effective_minimum_delay_at(historical_block_number); - let block_horizon = - value_change.get_block_horizon(historical_block_number, effective_minimum_delay); - - // We prevent this transaction from being included in any block after the block horizon, ensuring that the - // historical public value matches the current one, since it can only change after the horizon. - self.context.set_tx_max_block_number(block_horizon); - value_change.get_current_at(historical_block_number) - } - - fn historical_read_from_public_storage( - self, - ) -> (ScheduledValueChange, ScheduledDelayChange, u32) { - let header = self.context.get_header(); - let address = self.context.this_address(); - - 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. - 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::deserialize(zeroed()), - "Non-zero value change for zero hash", - ); - assert_eq( - delay_change_hint, - ScheduledDelayChange::deserialize(zeroed()), - "Non-zero delay change for zero hash", - ); - }; - - (value_change_hint, delay_change_hint, historical_block_number) - } - - fn hash_scheduled_data( - value_change: ScheduledValueChange, - delay_change: ScheduledDelayChange, - ) -> Field { - let concatenated: [Field; 4] = - array_concat(value_change.serialize(), delay_change.serialize()); - poseidon2_hash(concatenated) - } -} - -impl SharedMutable -where - T: ToField + FromField + Eq, -{ - pub unconstrained fn get_current_value_in_unconstrained(self) -> T { - let block_number = self.context.block_number() as u32; - self.read_value_change().get_current_at(block_number) - } - - unconstrained fn read_value_change(self) -> ScheduledValueChange { - self.context.storage_read(self.get_value_change_storage_slot()) - } -} - -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/test.nr b/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/test.nr index cdf5414ec83..de2d1f2c0bb 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 @@ -2,7 +2,7 @@ use crate::{ context::{PrivateContext, PublicContext, UnconstrainedContext}, state_vars::shared_mutable::{ scheduled_delay_change::ScheduledDelayChange, scheduled_value_change::ScheduledValueChange, - shared_mutable::SharedMutable, + SharedMutable, }, test::helpers::test_environment::TestEnvironment, }; @@ -45,7 +45,7 @@ unconstrained fn test_get_current_value_in_public_initial() { let env = setup(); let state_var = in_public(env); - assert_eq(state_var.get_current_value_in_public(), zeroed()); + assert_eq(state_var.get_current_value(), zeroed()); } #[test] @@ -55,7 +55,7 @@ unconstrained fn test_get_scheduled_value_in_public() { state_var.schedule_value_change(new_value); - let (scheduled, block_of_change) = state_var.get_scheduled_value_in_public(); + let (scheduled, block_of_change) = state_var.get_scheduled_value(); assert_eq(scheduled, new_value); assert_eq(block_of_change, env.block_number() + TEST_INITIAL_DELAY); } @@ -67,16 +67,16 @@ unconstrained fn test_get_current_value_in_public_before_scheduled_change() { state_var.schedule_value_change(new_value); - let (_, block_of_change) = state_var.get_scheduled_value_in_public(); + let (_, block_of_change) = state_var.get_scheduled_value(); let original_value = zeroed(); // The current value has not changed - assert_eq(state_var.get_current_value_in_public(), original_value); + assert_eq(state_var.get_current_value(), original_value); // The current value still does not change right before the block of change env.advance_block_to(block_of_change - 1); - assert_eq(state_var.get_current_value_in_public(), original_value); + assert_eq(state_var.get_current_value(), original_value); } #[test] @@ -86,10 +86,10 @@ unconstrained fn test_get_current_value_in_public_at_scheduled_change() { state_var.schedule_value_change(new_value); - let (_, block_of_change) = state_var.get_scheduled_value_in_public(); + let (_, block_of_change) = state_var.get_scheduled_value(); env.advance_block_to(block_of_change); - assert_eq(state_var.get_current_value_in_public(), new_value); + assert_eq(state_var.get_current_value(), new_value); } #[test] @@ -99,10 +99,10 @@ unconstrained fn test_get_current_value_in_public_after_scheduled_change() { state_var.schedule_value_change(new_value); - let (_, block_of_change) = state_var.get_scheduled_value_in_public(); + let (_, block_of_change) = state_var.get_scheduled_value(); env.advance_block_to(block_of_change + 10); - assert_eq(state_var.get_current_value_in_public(), new_value); + assert_eq(state_var.get_current_value(), new_value); } #[test] @@ -110,7 +110,7 @@ unconstrained fn test_get_current_delay_in_public_initial() { let env = setup(); let state_var = in_public(env); - assert_eq(state_var.get_current_delay_in_public(), TEST_INITIAL_DELAY); + assert_eq(state_var.get_current_delay(), TEST_INITIAL_DELAY); } #[test] @@ -120,7 +120,7 @@ unconstrained fn test_get_scheduled_delay_in_public() { state_var.schedule_delay_change(new_delay); - let (scheduled, block_of_change) = state_var.get_scheduled_delay_in_public(); + let (scheduled, block_of_change) = state_var.get_scheduled_delay(); assert_eq(scheduled, new_delay); // The new delay is smaller, therefore we need to wait for the difference between current and new assert_eq(block_of_change, env.block_number() + TEST_INITIAL_DELAY - new_delay); @@ -133,16 +133,16 @@ unconstrained fn test_get_current_delay_in_public_before_scheduled_change() { state_var.schedule_delay_change(new_delay); - let (_, block_of_change) = state_var.get_scheduled_delay_in_public(); + let (_, block_of_change) = state_var.get_scheduled_delay(); let original_delay = TEST_INITIAL_DELAY; // The current delay has not changed - assert_eq(state_var.get_current_delay_in_public(), original_delay); + assert_eq(state_var.get_current_delay(), original_delay); // The current delay still does not change right before the block of change env.advance_block_to(block_of_change - 1); - assert_eq(state_var.get_current_delay_in_public(), original_delay); + assert_eq(state_var.get_current_delay(), original_delay); } #[test] @@ -152,10 +152,10 @@ unconstrained fn test_get_current_delay_in_public_at_scheduled_change() { state_var.schedule_delay_change(new_delay); - let (_, block_of_change) = state_var.get_scheduled_delay_in_public(); + let (_, block_of_change) = state_var.get_scheduled_delay(); env.advance_block_to(block_of_change); - assert_eq(state_var.get_current_delay_in_public(), new_delay); + assert_eq(state_var.get_current_delay(), new_delay); } #[test] @@ -165,10 +165,10 @@ unconstrained fn test_get_current_delay_in_public_after_scheduled_change() { state_var.schedule_delay_change(new_delay); - let (_, block_of_change) = state_var.get_scheduled_delay_in_public(); + let (_, block_of_change) = state_var.get_scheduled_delay(); env.advance_block_to(block_of_change + 10); - assert_eq(state_var.get_current_delay_in_public(), new_delay); + assert_eq(state_var.get_current_delay(), new_delay); } #[test] @@ -178,7 +178,7 @@ unconstrained fn test_get_current_value_in_private_initial() { let historical_block_number = env.block_number(); let state_var = in_private(&mut env, historical_block_number); - assert_eq(state_var.get_current_value_in_private(), zeroed()); + assert_eq(state_var.get_current_value(), zeroed()); assert_eq( state_var.context.max_block_number.unwrap(), historical_block_number + TEST_INITIAL_DELAY, @@ -192,12 +192,12 @@ unconstrained fn test_get_current_value_in_private_before_change() { let public_state_var = in_public(env); public_state_var.schedule_value_change(new_value); - let (_, block_of_change) = public_state_var.get_scheduled_value_in_public(); + let (_, block_of_change) = public_state_var.get_scheduled_value(); 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_in_private(), 0); + assert_eq(private_state_var.get_current_value(), 0); assert_eq(private_state_var.context.max_block_number.unwrap(), block_of_change - 1); } @@ -208,13 +208,13 @@ unconstrained fn test_get_current_value_in_private_immediately_before_change() { let public_state_var = in_public(env); public_state_var.schedule_value_change(new_value); - let (_, block_of_change) = public_state_var.get_scheduled_value_in_public(); + let (_, block_of_change) = public_state_var.get_scheduled_value(); let private_state_var = in_private(&mut env, block_of_change - 1); // 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_in_private(), 0); + assert_eq(private_state_var.get_current_value(), 0); assert_eq(private_state_var.context.max_block_number.unwrap(), block_of_change - 1); } @@ -225,11 +225,11 @@ unconstrained fn test_get_current_value_in_private_at_change() { let public_state_var = in_public(env); public_state_var.schedule_value_change(new_value); - let (_, block_of_change) = public_state_var.get_scheduled_value_in_public(); + let (_, block_of_change) = public_state_var.get_scheduled_value(); let historical_block_number = block_of_change; let private_state_var = in_private(&mut env, historical_block_number); - assert_eq(private_state_var.get_current_value_in_private(), new_value); + assert_eq(private_state_var.get_current_value(), new_value); assert_eq( private_state_var.context.max_block_number.unwrap(), historical_block_number + TEST_INITIAL_DELAY, @@ -243,11 +243,11 @@ unconstrained fn test_get_current_value_in_private_after_change() { let public_state_var = in_public(env); public_state_var.schedule_value_change(new_value); - let (_, block_of_change) = public_state_var.get_scheduled_value_in_public(); + let (_, block_of_change) = public_state_var.get_scheduled_value(); let historical_block_number = block_of_change + 10; let private_state_var = in_private(&mut env, historical_block_number); - assert_eq(private_state_var.get_current_value_in_private(), new_value); + assert_eq(private_state_var.get_current_value(), new_value); assert_eq( private_state_var.context.max_block_number.unwrap(), historical_block_number + TEST_INITIAL_DELAY, @@ -262,8 +262,8 @@ unconstrained fn test_get_current_value_in_private_with_non_initial_delay() { public_state_var.schedule_value_change(new_value); public_state_var.schedule_delay_change(new_delay); - let (_, value_block_of_change) = public_state_var.get_scheduled_value_in_public(); - let (_, delay_block_of_change) = public_state_var.get_scheduled_delay_in_public(); + let (_, value_block_of_change) = public_state_var.get_scheduled_value(); + let (_, delay_block_of_change) = public_state_var.get_scheduled_delay(); let historical_block_number = if value_block_of_change > delay_block_of_change { value_block_of_change @@ -272,7 +272,7 @@ unconstrained fn test_get_current_value_in_private_with_non_initial_delay() { }; let private_state_var = in_private(&mut env, historical_block_number); - assert_eq(private_state_var.get_current_value_in_private(), new_value); + assert_eq(private_state_var.get_current_value(), new_value); assert_eq( private_state_var.context.max_block_number.unwrap(), historical_block_number + new_delay, @@ -299,7 +299,7 @@ unconstrained fn test_get_current_value_in_private_bad_value_hints() { .returns(mocked.serialize()) .times(1); - let _ = private_state_var.get_current_value_in_private(); + let _ = private_state_var.get_current_value(); } #[test(should_fail_with = "Hint values do not match hash")] @@ -322,7 +322,7 @@ unconstrained fn test_get_current_value_in_private_bad_delay_hints() { .returns(mocked.serialize()) .times(1); - let _ = private_state_var.get_current_value_in_private(); + let _ = private_state_var.get_current_value(); } #[test(should_fail_with = "Non-zero value change for zero hash")] @@ -341,7 +341,7 @@ unconstrained fn test_get_current_value_in_private_bad_zero_hash_value_hints() { .returns(mocked.serialize()) .times(1); - let _ = state_var.get_current_value_in_private(); + let _ = state_var.get_current_value(); } #[test(should_fail_with = "Non-zero delay change for zero hash")] @@ -361,7 +361,7 @@ unconstrained fn test_get_current_value_in_private_bad_zero_hash_delay_hints() { .returns(mocked.serialize()) .times(1); - let _ = state_var.get_current_value_in_private(); + let _ = state_var.get_current_value(); } #[test] @@ -369,7 +369,7 @@ unconstrained fn test_get_current_value_in_unconstrained_initial() { let env = setup(); let state_var = in_unconstrained(env); - assert_eq(state_var.get_current_value_in_unconstrained(), zeroed()); + assert_eq(state_var.get_current_value(), zeroed()); } #[test] @@ -379,20 +379,20 @@ unconstrained fn test_get_current_value_in_unconstrained_before_scheduled_change state_var_public.schedule_value_change(new_value); - let (_, block_of_change) = state_var_public.get_scheduled_value_in_public(); + let (_, block_of_change) = state_var_public.get_scheduled_value(); let original_value = zeroed(); let mut state_var_unconstrained = in_unconstrained(env); // The current value has not changed - assert_eq(state_var_unconstrained.get_current_value_in_unconstrained(), original_value); + assert_eq(state_var_unconstrained.get_current_value(), original_value); // The current value still does not change right before the block of change env.advance_block_to(block_of_change - 1); state_var_unconstrained = in_unconstrained(env); - assert_eq(state_var_unconstrained.get_current_value_in_unconstrained(), original_value); + assert_eq(state_var_unconstrained.get_current_value(), original_value); } #[test] @@ -402,12 +402,12 @@ unconstrained fn test_get_current_value_in_unconstrained_at_scheduled_change() { state_var_public.schedule_value_change(new_value); - let (_, block_of_change) = state_var_public.get_scheduled_value_in_public(); + let (_, block_of_change) = state_var_public.get_scheduled_value(); env.advance_block_to(block_of_change); let state_var_unconstrained = in_unconstrained(env); - assert_eq(state_var_unconstrained.get_current_value_in_unconstrained(), new_value); + assert_eq(state_var_unconstrained.get_current_value(), new_value); } #[test] @@ -417,9 +417,9 @@ unconstrained fn test_get_current_value_in_unconstrained_after_scheduled_change( state_var_public.schedule_value_change(new_value); - let (_, block_of_change) = state_var_public.get_scheduled_value_in_public(); + let (_, block_of_change) = state_var_public.get_scheduled_value(); env.advance_block_to(block_of_change + 10); let state_var_unconstrained = in_unconstrained(env); - assert_eq(state_var_unconstrained.get_current_value_in_unconstrained(), new_value); + assert_eq(state_var_unconstrained.get_current_value(), new_value); } diff --git a/noir-projects/noir-contracts/contracts/auth_contract/src/main.nr b/noir-projects/noir-contracts/contracts/auth_contract/src/main.nr index e14deb2bf2e..b28debc8aad 100644 --- a/noir-projects/noir-contracts/contracts/auth_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/auth_contract/src/main.nr @@ -44,7 +44,7 @@ contract Auth { #[view] fn get_authorized() -> AztecAddress { // docs:start:shared_mutable_get_current_public - storage.authorized.get_current_value_in_public() + storage.authorized.get_current_value() // docs:end:shared_mutable_get_current_public } // docs:end:public_getter @@ -54,7 +54,7 @@ contract Auth { fn get_scheduled_authorized() -> AztecAddress { // docs:start:shared_mutable_get_scheduled_public let (scheduled_value, _block_of_change): (AztecAddress, u32) = - storage.authorized.get_scheduled_value_in_public(); + storage.authorized.get_scheduled_value(); // docs:end:shared_mutable_get_scheduled_public scheduled_value } @@ -62,7 +62,7 @@ contract Auth { #[public] #[view] fn get_authorized_delay() -> pub u32 { - storage.authorized.get_current_delay_in_public() + storage.authorized.get_current_delay() } #[public] @@ -76,7 +76,7 @@ contract Auth { // circuit will reject this tx if included in a block past the block horizon, which is as far as the circuit can // guarantee the value will not change from some historical value (due to CHANGE_AUTHORIZED_DELAY_BLOCKS). // docs:start:shared_mutable_get_current_private - let authorized = storage.authorized.get_current_value_in_private(); + let authorized = storage.authorized.get_current_value(); // docs:end:shared_mutable_get_current_private assert_eq(authorized, context.msg_sender(), "caller is not authorized"); } @@ -84,6 +84,6 @@ contract Auth { #[private] #[view] fn get_authorized_in_private() -> AztecAddress { - storage.authorized.get_current_value_in_private() + storage.authorized.get_current_value() } } diff --git a/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/main.nr b/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/main.nr index bdb3b8bd0a9..4588e7382e7 100644 --- a/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/main.nr @@ -66,12 +66,12 @@ contract TokenBlacklist { #[public] #[view] fn get_roles(user: AztecAddress) -> UserFlags { - storage.roles.at(user).get_current_value_in_public() + storage.roles.at(user).get_current_value() } #[public] fn update_roles(user: AztecAddress, roles: UserFlags) { - let caller_roles = storage.roles.at(context.msg_sender()).get_current_value_in_public(); + let caller_roles = storage.roles.at(context.msg_sender()).get_current_value(); assert(caller_roles.is_admin, "caller is not admin"); storage.roles.at(user).schedule_value_change(roles); @@ -79,10 +79,10 @@ contract TokenBlacklist { #[public] fn mint_public(to: AztecAddress, amount: Field) { - let to_roles = storage.roles.at(to).get_current_value_in_public(); + let to_roles = storage.roles.at(to).get_current_value(); assert(!to_roles.is_blacklisted, "Blacklisted: Recipient"); - let caller_roles = storage.roles.at(context.msg_sender()).get_current_value_in_public(); + let caller_roles = storage.roles.at(context.msg_sender()).get_current_value(); assert(caller_roles.is_minter, "caller is not minter"); let amount = U128::from_integer(amount); @@ -95,7 +95,7 @@ contract TokenBlacklist { #[public] fn mint_private(amount: Field, secret_hash: Field) { - let caller_roles = storage.roles.at(context.msg_sender()).get_current_value_in_public(); + let caller_roles = storage.roles.at(context.msg_sender()).get_current_value(); assert(caller_roles.is_minter, "caller is not minter"); let pending_shields = storage.pending_shields; @@ -108,7 +108,7 @@ contract TokenBlacklist { #[public] fn shield(from: AztecAddress, amount: Field, secret_hash: Field, nonce: Field) { - let from_roles = storage.roles.at(from).get_current_value_in_public(); + let from_roles = storage.roles.at(from).get_current_value(); assert(!from_roles.is_blacklisted, "Blacklisted: Sender"); if (!from.eq(context.msg_sender())) { @@ -130,9 +130,9 @@ contract TokenBlacklist { #[public] fn transfer_public(from: AztecAddress, to: AztecAddress, amount: Field, nonce: Field) { - let from_roles = storage.roles.at(from).get_current_value_in_public(); + let from_roles = storage.roles.at(from).get_current_value(); assert(!from_roles.is_blacklisted, "Blacklisted: Sender"); - let to_roles = storage.roles.at(to).get_current_value_in_public(); + let to_roles = storage.roles.at(to).get_current_value(); assert(!to_roles.is_blacklisted, "Blacklisted: Recipient"); if (!from.eq(context.msg_sender())) { @@ -151,7 +151,7 @@ contract TokenBlacklist { #[public] fn burn_public(from: AztecAddress, amount: Field, nonce: Field) { - let from_roles = storage.roles.at(from).get_current_value_in_public(); + let from_roles = storage.roles.at(from).get_current_value(); assert(!from_roles.is_blacklisted, "Blacklisted: Sender"); if (!from.eq(context.msg_sender())) { @@ -170,7 +170,7 @@ contract TokenBlacklist { #[private] fn redeem_shield(to: AztecAddress, amount: Field, secret: Field) { - let to_roles = storage.roles.at(to).get_current_value_in_private(); + let to_roles = storage.roles.at(to).get_current_value(); assert(!to_roles.is_blacklisted, "Blacklisted: Recipient"); let secret_hash = compute_secret_hash(secret); @@ -202,9 +202,9 @@ contract TokenBlacklist { #[private] fn unshield(from: AztecAddress, to: AztecAddress, amount: Field, nonce: Field) { - let from_roles = storage.roles.at(from).get_current_value_in_private(); + let from_roles = storage.roles.at(from).get_current_value(); assert(!from_roles.is_blacklisted, "Blacklisted: Sender"); - let to_roles = storage.roles.at(to).get_current_value_in_private(); + let to_roles = storage.roles.at(to).get_current_value(); assert(!to_roles.is_blacklisted, "Blacklisted: Recipient"); if (!from.eq(context.msg_sender())) { @@ -228,9 +228,9 @@ contract TokenBlacklist { // docs:start:transfer_private #[private] fn transfer(from: AztecAddress, to: AztecAddress, amount: Field, nonce: Field) { - let from_roles = storage.roles.at(from).get_current_value_in_private(); + let from_roles = storage.roles.at(from).get_current_value(); assert(!from_roles.is_blacklisted, "Blacklisted: Sender"); - let to_roles = storage.roles.at(to).get_current_value_in_private(); + let to_roles = storage.roles.at(to).get_current_value(); assert(!to_roles.is_blacklisted, "Blacklisted: Recipient"); if (!from.eq(context.msg_sender())) { @@ -258,7 +258,7 @@ contract TokenBlacklist { #[private] fn burn(from: AztecAddress, amount: Field, nonce: Field) { - let from_roles = storage.roles.at(from).get_current_value_in_private(); + let from_roles = storage.roles.at(from).get_current_value(); assert(!from_roles.is_blacklisted, "Blacklisted: Sender"); if (!from.eq(context.msg_sender())) {