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 66f1400917e..7710c73f6a3 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 @@ -171,3 +171,9 @@ impl Deserialize<1> for ScheduledDelayChange { } } } + +impl Eq for ScheduledDelayChange { + fn eq(self, other: Self) -> bool { + (self.pre == other.pre) & (self.post == other.post) & (self.block_of_change == other.block_of_change) + } +} diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/scheduled_delay_change/test.nr b/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/scheduled_delay_change/test.nr index 5e4a6d9bcf9..c569510e2cd 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/scheduled_delay_change/test.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/scheduled_delay_change/test.nr @@ -7,6 +7,7 @@ fn assert_equal_after_conversion(original: ScheduledDelayChange = ScheduledDelayChange::deserialize((original).serialize()); + assert_eq(original, converted); // This also tests the Eq impl assert_eq(original.pre, converted.pre); assert_eq(original.post, converted.post); assert_eq(original.block_of_change, converted.block_of_change); 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 8ca92f61693..d0d84ad94c0 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 @@ -152,3 +152,9 @@ impl Deserialize<3> for ScheduledValueChange { } } } + +impl Eq for ScheduledValueChange { + fn eq(self, other: Self) -> bool where T: Eq { + (self.pre == other.pre) & (self.post == other.post) & (self.block_of_change == other.block_of_change) + } +} diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/scheduled_value_change/test.nr b/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/scheduled_value_change/test.nr index 8a48ee013be..5dab89c9701 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/scheduled_value_change/test.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/scheduled_value_change/test.nr @@ -11,6 +11,7 @@ fn test_serde() { let original = ScheduledValueChange::new(pre, post, block_of_change); let converted = ScheduledValueChange::deserialize((original).serialize()); + assert_eq(original, converted); // This also tests the Eq impl assert_eq(original.pre, converted.pre); assert_eq(original.post, converted.post); assert_eq(original.block_of_change, converted.block_of_change); 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 197d1402986..9823b25d8c9 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,10 +1,15 @@ -use dep::protocol_types::{hash::pedersen_hash, traits::FromField}; +use dep::protocol_types::{ + hash::{pedersen_hash, poseidon2_hash}, header::Header, address::AztecAddress, + traits::{FromField, ToField} +}; use crate::context::{PrivateContext, PublicContext}; use crate::state_vars::{ storage::Storage, shared_mutable::{scheduled_value_change::ScheduledValueChange, scheduled_delay_change::ScheduledDelayChange} }; +use crate::oracle::storage::storage_read; +use dep::std::unsafe::zeroed; mod test; @@ -18,10 +23,23 @@ struct SharedMutable { // - 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 +// 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 {} +// TODO: extract into a utils module once we can do arithmetic on generics, i.e. https://github.com/noir-lang/noir/issues/4784 +fn concat_arrays(arr_n: [Field; N], arr_m: [Field; M]) -> [Field; O] { + assert_eq(N + M, O); + let mut out: [Field; O] = [0; O]; + for i in 0..N { + out[i] = arr_n[i]; + } + for i in 0..M { + out[N+i] = arr_m[i]; + } + out +} + // SharedMutable stores a value of type T that is: // - publicly known (i.e. unencrypted) // - mutable in public @@ -29,9 +47,9 @@ impl Storage for SharedMutable SharedMutable { pub fn new(context: Context, storage_slot: Field) -> Self { @@ -39,10 +57,24 @@ impl SharedMutable { Self { context, storage_slot } } + fn hash_scheduled_data( + value_change: ScheduledValueChange, + delay_change: ScheduledDelayChange + ) -> Field where T: ToField { + // TODO(#5491 and https://github.com/noir-lang/noir/issues/4784): update this so that we don't need to rely on + // ScheduledValueChange serializing to 3 and ScheduledDelayChange serializing to 1 + let concatenated: [Field; 4] = concat_arrays(value_change.serialize(), delay_change.serialize()); + poseidon2_hash(concatenated) + } + // 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 + // 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 { pedersen_hash([self.storage_slot, 0], 0) } @@ -50,10 +82,53 @@ impl SharedMutable { fn get_delay_change_storage_slot(self) -> Field { pedersen_hash([self.storage_slot, 1], 0) } + + fn get_hash_storage_slot(self) -> Field { + pedersen_hash([self.storage_slot, 2], 0) + } + + // It may seem odd that we take a header and address instead of reading from e.g. a PrivateContext, but this lets us + // reuse this function in SharedMutablePrivateGetter. + fn historical_read_from_public_storage( + self, + header: Header, + address: AztecAddress + ) -> (ScheduledValueChange, ScheduledDelayChange, u32) where T: FromField + ToField + Eq { + 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) = 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); + + // @todo This is written strangely to bypass a formatting issue with the if that is breaking ci. + let (a, b, c) = if hash != 0 { + let a = SharedMutable::hash_scheduled_data(value_change_hint, delay_change_hint); + (a, value_change_hint, delay_change_hint) + } 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. + let b = ScheduledValueChange::deserialize(zeroed()); + let c = ScheduledDelayChange::deserialize(zeroed()); + (hash, b, c) + }; + + assert_eq(hash, a, "Hint values do not match hash"); + assert_eq(value_change_hint, b, "Non-zero value change for zero hash"); + assert_eq(delay_change_hint, c, "Non-zero delay change for zero hash"); + + (value_change_hint, delay_change_hint, historical_block_number) + } } impl SharedMutable { - pub fn schedule_value_change(self, new_value: T) { + pub fn schedule_value_change(self, new_value: T) where T: ToField { let mut value_change = self.read_value_change(); let delay_change = self.read_delay_change(); @@ -65,17 +140,17 @@ impl SharedMutable { 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(value_change); + self.write(value_change, delay_change); } - pub fn schedule_delay_change(self, new_delay: u32) { + pub fn schedule_delay_change(self, new_delay: u32) where T: ToField { 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_delay_change(delay_change); + self.write(self.read_value_change(), delay_change); } pub fn get_current_value_in_public(self) -> T { @@ -104,64 +179,64 @@ impl SharedMutable { self.context.storage_read(self.get_delay_change_storage_slot()) } - fn write_value_change(self, value_change: ScheduledValueChange) { + fn write( + self, + value_change: ScheduledValueChange, + delay_change: ScheduledDelayChange + ) where T: ToField { + // 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); - } - - fn write_delay_change(self, delay_change: ScheduledDelayChange) { 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 { - pub fn get_current_value_in_private(self) -> T where T: FromField { + pub fn get_current_value_in_private(self) -> T where T: FromField + ToField + Eq { // 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(*self.context); + let (value_change, delay_change, historical_block_number) = self.historical_read_from_public_storage(self.context.get_header(), self.context.this_address()); // 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. + // 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 + // 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 + // 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, - context: PrivateContext - ) -> (ScheduledValueChange, ScheduledDelayChange, u32) where T: FromField { - let header = context.get_header(); - // Ideally the following would be simply public_storage::read_historical, but we can't implement that yet. - let value_change_slot = self.get_value_change_storage_slot(); - let mut raw_value_change_fields = [0; 3]; - for i in 0..3 { - raw_value_change_fields[i] = header.public_storage_historical_read( - value_change_slot + i as Field, - context.this_address() - ); - } - - // Ideally the following would be simply public_storage::read_historical, but we can't implement that yet. - let delay_change_slot = self.get_delay_change_storage_slot(); - let raw_delay_change_fields = [header.public_storage_historical_read(delay_change_slot, context.this_address())]; - - let value_change = ScheduledValueChange::deserialize(raw_value_change_fields); - let delay_change = ScheduledDelayChange::deserialize(raw_delay_change_fields); - - let historical_block_number = context.historical_header.global_variables.block_number as u32; - - (value_change, delay_change, historical_block_number) - } +unconstrained fn get_public_storage_hints( + address: AztecAddress, + storage_slot: Field, + block_number: u32 +) -> (ScheduledValueChange, ScheduledDelayChange) { + // 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::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/shared_mutable_private_getter.nr b/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/shared_mutable_private_getter.nr index 29ef525192c..78c7cc66bb3 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/shared_mutable_private_getter.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/shared_mutable_private_getter.nr @@ -1,9 +1,15 @@ -use dep::protocol_types::{hash::pedersen_hash, traits::FromField, address::AztecAddress, header::Header}; +use dep::protocol_types::{ + hash::{pedersen_hash, poseidon2_hash}, traits::{FromField, ToField}, address::AztecAddress, + header::Header +}; use crate::context::PrivateContext; use crate::state_vars::{ storage::Storage, - shared_mutable::{scheduled_delay_change::ScheduledDelayChange, scheduled_value_change::ScheduledValueChange} + shared_mutable::{ + shared_mutable::SharedMutable, scheduled_delay_change::ScheduledDelayChange, + scheduled_value_change::ScheduledValueChange +} }; struct SharedMutablePrivateGetter { @@ -30,8 +36,12 @@ impl SharedMutablePrivateGetter { Self { context, other_contract_address, storage_slot, _dummy: [0; INITIAL_DELAY] } } - pub fn get_value_in_private(self, header: Header) -> T where T: FromField { - let (value_change, delay_change, historical_block_number) = self.historical_read_from_public_storage(header); + pub fn get_value_in_private(self, header: Header) -> T where T: FromField + ToField + Eq { + // We create a dummy SharedMutable state variable so that we can reuse its historical_read_from_public_storage + // method, greatly reducing code duplication. + let dummy: SharedMutable = SharedMutable::new((), self.storage_slot); + let (value_change, delay_change, historical_block_number) = dummy.historical_read_from_public_storage(header, self.other_contract_address); + 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); @@ -44,36 +54,4 @@ impl SharedMutablePrivateGetter { value_change.get_current_at(historical_block_number) } - - fn historical_read_from_public_storage( - self, - header: Header - ) -> (ScheduledValueChange, ScheduledDelayChange, u32) where T: FromField { - let value_change_slot = self.get_value_change_storage_slot(); - let mut raw_value_change_fields = [0; 3]; - for i in 0..3 { - raw_value_change_fields[i] = header.public_storage_historical_read( - value_change_slot + i as Field, - self.other_contract_address - ); - } - - let delay_change_slot = self.get_delay_change_storage_slot(); - let raw_delay_change_fields = [header.public_storage_historical_read(delay_change_slot, self.other_contract_address)]; - - let value_change = ScheduledValueChange::deserialize(raw_value_change_fields); - let delay_change = ScheduledDelayChange::deserialize(raw_delay_change_fields); - - let historical_block_number = header.global_variables.block_number as u32; - - (value_change, delay_change, historical_block_number) - } - - fn get_value_change_storage_slot(self) -> Field { - pedersen_hash([self.storage_slot, 0], 0) - } - - fn get_delay_change_storage_slot(self) -> Field { - pedersen_hash([self.storage_slot, 1], 0) - } } 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 b193640a54b..abf0e28940b 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,18 +1,22 @@ use crate::{ - context::{PublicContext, PrivateContext}, state_vars::shared_mutable::shared_mutable::SharedMutable, + context::{PublicContext, PrivateContext}, + state_vars::shared_mutable::{ + shared_mutable::SharedMutable, scheduled_value_change::ScheduledValueChange, + scheduled_delay_change::ScheduledDelayChange +}, test::helpers::test_environment::TestEnvironment }; use dep::protocol_types::address::AztecAddress; +use dep::std::{test::OracleMock, unsafe::zeroed}; -global new_value = 57; +global new_value = 17; -global pre_delay = 20; -global post_delay = 15; +global new_delay = 20; -global storage_slot = 57; +global storage_slot = 47; -global TEST_INITIAL_DELAY: u32 = 30; +global TEST_INITIAL_DELAY: u32 = 32; fn setup() -> TestEnvironment { TestEnvironment::new() @@ -29,335 +33,291 @@ fn in_private( SharedMutable::new(&mut env.private_at(historical_block_number), storage_slot) } -// #[test] -// fn test_get_current_value_in_public_initial() { -// let env = setup(); -// let state_var = in_public(env); +#[test] +fn test_get_current_value_in_public_initial() { + let env = setup(); + let state_var = in_public(env); -// // 0 is the default empty value for a Field -// assert_eq(state_var.get_current_value_in_public(), 0); -// } + assert_eq(state_var.get_current_value_in_public(), zeroed()); +} + +#[test] +fn test_get_scheduled_value_in_public() { + let mut env = setup(); + let state_var = in_public(env); + + state_var.schedule_value_change(new_value); + + let (scheduled, block_of_change) = state_var.get_scheduled_value_in_public(); + assert_eq(scheduled, new_value); + assert_eq(block_of_change, env.block_number() + TEST_INITIAL_DELAY); +} + +#[test] +fn test_get_current_value_in_public_before_scheduled_change() { + let mut env = setup(); + let state_var = in_public(env); + + state_var.schedule_value_change(new_value); + + let (_, block_of_change) = state_var.get_scheduled_value_in_public(); + + let original_value = zeroed(); + + // The current value has not changed + assert_eq(state_var.get_current_value_in_public(), 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); +} + +#[test] +fn test_get_current_value_in_public_at_scheduled_change() { + let mut env = setup(); + let state_var = in_public(env); + + state_var.schedule_value_change(new_value); -// #[test] -// fn test_get_current_value_in_public_before_scheduled_change() { -// let mut env = setup(); -// let state_var = in_public(env); + let (_, block_of_change) = state_var.get_scheduled_value_in_public(); -// state_var.schedule_value_change(new_value); + env.advance_block_to(block_of_change); + assert_eq(state_var.get_current_value_in_public(), new_value); +} -// let (_, block_of_change) = state_var.get_scheduled_value_in_public(); +#[test] +fn test_get_current_value_in_public_after_scheduled_change() { + let mut env = setup(); + let state_var = in_public(env); -// let original_value = 0; + state_var.schedule_value_change(new_value); -// // The current value has not changed -// assert_eq(state_var.get_current_value_in_public(), original_value); + let (_, block_of_change) = state_var.get_scheduled_value_in_public(); -// // 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); -// } + env.advance_block_to(block_of_change + 10); + assert_eq(state_var.get_current_value_in_public(), new_value); +} -// #[test] -// fn test_get_current_value_in_public_at_scheduled_change() { -// let mut env = setup(); -// let state_var = in_public(env); +#[test] +fn test_get_current_delay_in_public_initial() { + let env = setup(); + let state_var = in_public(env); -// state_var.schedule_value_change(new_value); + assert_eq(state_var.get_current_delay_in_public(), TEST_INITIAL_DELAY); +} -// let (_, block_of_change) = state_var.get_scheduled_value_in_public(); +#[test] +fn test_get_scheduled_delay_in_public() { + let mut env = setup(); + let state_var = in_public(env); -// env.advance_block_to(block_of_change); -// assert_eq(state_var.get_current_value_in_public(), new_value); -// } + state_var.schedule_delay_change(new_delay); -// #[test] -// fn test_get_current_value_in_public_after_scheduled_change() { -// let mut env = setup(); -// let state_var = in_public(env); + let (scheduled, block_of_change) = state_var.get_scheduled_delay_in_public(); + 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); +} -// state_var.schedule_value_change(new_value); +#[test] +fn test_get_current_delay_in_public_before_scheduled_change() { + let mut env = setup(); + let state_var = in_public(env); -// let (_, block_of_change) = state_var.get_scheduled_value_in_public(); + state_var.schedule_delay_change(new_delay); -// env.advance_block_to(block_of_change + 10); -// assert_eq(state_var.get_current_value_in_public(), new_value); -// } + let (_, block_of_change) = state_var.get_scheduled_delay_in_public(); -// #[test] -// fn test_get_current_value_in_private_before_change() { -// let mut env = setup(); + let original_delay = TEST_INITIAL_DELAY; + + // The current delay has not changed + assert_eq(state_var.get_current_delay_in_public(), 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); +} -// let public_state_var = in_public(env); -// public_state_var.schedule_value_change(new_value); +#[test] +fn test_get_current_delay_in_public_at_scheduled_change() { + let mut env = setup(); + let state_var = in_public(env); -// let (_, block_of_change) = public_state_var.get_scheduled_value_in_public(); + state_var.schedule_delay_change(new_delay); -// let schedule_block_number = env.block_number(); + let (_, block_of_change) = state_var.get_scheduled_delay_in_public(); -// 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.context.max_block_number.unwrap(), block_of_change - 1); -// } + env.advance_block_to(block_of_change); + assert_eq(state_var.get_current_delay_in_public(), new_delay); +} -// #[test] -// fn test_get_current_value_in_private_immediately_before_change() { -// let mut env = setup(); +#[test] +fn test_get_current_delay_in_public_after_scheduled_change() { + let mut env = setup(); + let state_var = in_public(env); -// let public_state_var = in_public(env); -// public_state_var.schedule_value_change(new_value); + state_var.schedule_delay_change(new_delay); -// let (_, block_of_change) = public_state_var.get_scheduled_value_in_public(); + let (_, block_of_change) = state_var.get_scheduled_delay_in_public(); -// let private_state_var = in_private(&mut env, block_of_change - 1); + env.advance_block_to(block_of_change + 10); + assert_eq(state_var.get_current_delay_in_public(), new_delay); +} -// assert_eq(private_state_var.get_current_value_in_private(), 0); -// assert_eq(private_state_var.context.max_block_number.unwrap(), block_of_change - 1); -// } +#[test] +fn test_get_current_value_in_private_initial() { + let mut env = setup(); -// #[test] -// fn test_get_current_value_in_private_at_change() { -// let mut env = setup(); + let historical_block_number = env.block_number(); + let state_var = in_private(&mut env, historical_block_number); -// let public_state_var = in_public(env); -// public_state_var.schedule_value_change(new_value); + assert_eq(state_var.get_current_value_in_private(), zeroed()); + assert_eq(state_var.context.max_block_number.unwrap(), historical_block_number + TEST_INITIAL_DELAY); +} -// let (_, block_of_change) = public_state_var.get_scheduled_value_in_public(); +#[test] +fn test_get_current_value_in_private_before_change() { + let mut env = setup(); -// 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.context.max_block_number.unwrap(), historical_block_number + TEST_INITIAL_DELAY -// ); -// } + let public_state_var = in_public(env); + public_state_var.schedule_value_change(new_value); -// #[test] -// fn test_get_current_value_in_private_after_change() { -// let mut env = setup(); + let (_, block_of_change) = public_state_var.get_scheduled_value_in_public(); -// let public_state_var = in_public(env); -// public_state_var.schedule_value_change(new_value); + let schedule_block_number = env.block_number(); -// let (_, block_of_change) = public_state_var.get_scheduled_value_in_public(); + 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.context.max_block_number.unwrap(), block_of_change - 1); +} -// 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.context.max_block_number.unwrap(), historical_block_number + TEST_INITIAL_DELAY -// ); -// } +#[test] +fn test_get_current_value_in_private_immediately_before_change() { + let mut env = setup(); -// #[test] -// fn test_get_current_delay_in_public() { -// let (state_var, block_number) = setup(); + let public_state_var = in_public(env); + public_state_var.schedule_value_change(new_value); -// // Uninitialized -// mock_delay_change_read_uninitialized(state_var); -// assert_eq(state_var.get_current_delay_in_public(), TEST_INITIAL_DELAY as u32); + let (_, block_of_change) = public_state_var.get_scheduled_value_in_public(); -// // Change in the future, current value is pre -// mock_delay_change_read(state_var, pre_delay, post_delay, block_number + 1); -// assert_eq(state_var.get_current_delay_in_public(), pre_delay as u32); + let private_state_var = in_private(&mut env, block_of_change - 1); -// // Change in the current block, current value is post -// mock_delay_change_read(state_var, pre_delay, post_delay, block_number); -// assert_eq(state_var.get_current_delay_in_public(), post_delay as u32); + // 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.context.max_block_number.unwrap(), block_of_change - 1); +} -// // Change in the past, current value is post -// mock_delay_change_read(state_var, pre_delay, post_delay, block_number - 1); -// assert_eq(state_var.get_current_delay_in_public(), post_delay as u32); -// } +#[test] +fn test_get_current_value_in_private_at_change() { + let mut env = setup(); -// #[test] -// fn test_get_scheduled_delay_in_public_before_change() { -// let (state_var, block_number) = setup(); + let public_state_var = in_public(env); + public_state_var.schedule_value_change(new_value); -// // Uninitialized -// mock_delay_change_read_uninitialized(state_var); -// assert_eq(state_var.get_scheduled_delay_in_public(), (TEST_INITIAL_DELAY as u32, 0)); + let (_, block_of_change) = public_state_var.get_scheduled_value_in_public(); -// // Change in the future, scheduled is post (always is) -// mock_delay_change_read(state_var, pre_delay, post_delay, block_number + 1); -// assert_eq(state_var.get_scheduled_delay_in_public(), (post_delay as u32, (block_number + 1) as u32)); + 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.context.max_block_number.unwrap(), historical_block_number + TEST_INITIAL_DELAY + ); +} -// // Change in the current block, scheduled is post (always is) -// mock_delay_change_read(state_var, pre_delay, post_delay, block_number); -// assert_eq(state_var.get_scheduled_delay_in_public(), (post_delay as u32, block_number as u32)); +#[test] +fn test_get_current_value_in_private_after_change() { + let mut env = setup(); -// // Change in the past, scheduled is post (always is) -// mock_delay_change_read(state_var, pre_delay, post_delay, block_number - 1); -// assert_eq(state_var.get_scheduled_delay_in_public(), (post_delay as u32, (block_number - 1) as u32)); -// } + let public_state_var = in_public(env); + public_state_var.schedule_value_change(new_value); -// #[test] -// fn test_schedule_value_change_no_delay() { -// let (state_var, block_number) = setup(); + let (_, block_of_change) = public_state_var.get_scheduled_value_in_public(); -// // Last value change was in the past -// mock_value_change_read(state_var, pre_value, post_value, 0); - -// // Current delay is 0 -// mock_delay_change_read(state_var, 0, 0, block_number); - -// let write_mock = mock_value_change_write(); + 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.context.max_block_number.unwrap(), historical_block_number + TEST_INITIAL_DELAY + ); +} -// state_var.schedule_value_change(new_value); - -// // The new value has a block of change equal to the current block, i.e. it is the current value -// assert_value_change_write(state_var, write_mock, post_value, new_value, block_number); -// } - -// #[test] -// fn test_schedule_value_change_before_change_no_scheduled_delay() { -// let (state_var, block_number) = setup(); - -// // Value change in the future, delay change in the past -// mock_value_and_delay_read(state_var, block_number + 1, block_number - 1); -// let write_mock = mock_value_change_write(); - -// state_var.schedule_value_change(new_value); - -// // The new scheduled value change replaces the old one, post delay (current) is used -// assert_value_change_write( -// state_var, -// write_mock, -// pre_value, -// new_value, -// block_number + post_delay -// ); -// } - -// #[test] -// fn test_schedule_value_change_before_change_scheduled_delay() { -// let (state_var, block_number) = setup(); - -// // Value change in the future, delay change in the future -// mock_value_and_delay_read(state_var, block_number + 1, block_number + 1); - -// let write_mock = mock_value_change_write(); - -// state_var.schedule_value_change(new_value); - -// // The new scheduled value change replaces the old one, pre delay (current, not scheduled) is used -// assert_value_change_write( -// state_var, -// write_mock, -// pre_value, -// new_value, -// block_number + pre_delay -// ); -// } - -// #[test] -// fn test_schedule_value_change_after_change_no_scheduled_delay() { -// let (state_var, block_number) = setup(); - -// // Value change in the past, delay change in the past -// mock_value_and_delay_read(state_var, block_number - 1, block_number - 1); -// let write_mock = mock_value_change_write(); - -// state_var.schedule_value_change(new_value); - -// // The previous post value becomes the pre value, post delay (current) is used -// assert_value_change_write( -// state_var, -// write_mock, -// post_value, -// new_value, -// block_number + post_delay -// ); -// } - -// #[test] -// fn test_schedule_value_change_after_change_scheduled_delay() { -// let (state_var, block_number) = setup(); - -// // Value change in the past, delay change in the future -// mock_value_and_delay_read(state_var, block_number - 1, block_number + 1); - -// let write_mock = mock_value_change_write(); - -// state_var.schedule_value_change(new_value); - -// // The previous post value becomes the pre value, pre delay (current, not scheduled) is used -// assert_value_change_write( -// state_var, -// write_mock, -// post_value, -// new_value, -// block_number + pre_delay -// ); -// } - -// #[test] -// fn test_schedule_delay_increase_before_change() { -// let (state_var, block_number) = setup(); - -// // Delay change in future, current delay is pre -// mock_delay_change_read(state_var, pre_delay, post_delay, block_number + 1); -// let write_mock = mock_delay_change_write(); - -// let new_delay = pre_delay + 1; -// state_var.schedule_delay_change(new_delay as u32); - -// // The previous scheduled change is lost, change is immediate (due to increase) -// assert_delay_change_write(state_var, write_mock, pre_delay, new_delay, block_number); -// } - -// #[test] -// fn test_schedule_delay_reduction_before_change() { -// let (state_var, block_number) = setup(); - -// // Delay change in future, current delay is pre -// mock_delay_change_read(state_var, pre_delay, post_delay, block_number + 1); -// let write_mock = mock_delay_change_write(); - -// let new_delay = pre_delay - 1; -// state_var.schedule_delay_change(new_delay as u32); - -// // The previous scheduled change is lost, change delay equals difference (due to reduction) -// assert_delay_change_write( -// state_var, -// write_mock, -// pre_delay, -// new_delay, -// block_number + pre_delay - new_delay -// ); -// } - -// #[test] -// fn test_schedule_delay_increase_after_change() { -// let (state_var, block_number) = setup(); - -// // Delay change in the past, current delay is post -// mock_delay_change_read(state_var, pre_delay, post_delay, block_number - 1); -// let write_mock = mock_delay_change_write(); - -// let new_delay = post_delay + 1; -// state_var.schedule_delay_change(new_delay as u32); - -// // The current value becomes pre, change is immediate (due to increase) -// assert_delay_change_write(state_var, write_mock, post_delay, new_delay, block_number); -// } - -// #[test] -// fn test_schedule_delay_reduction_after_change() { -// let (state_var, block_number) = setup(); - -// // Delay change in the past, current delay is post -// mock_delay_change_read(state_var, pre_delay, post_delay, block_number - 1); -// let write_mock = mock_delay_change_write(); - -// let new_delay = post_delay - 1; -// state_var.schedule_delay_change(new_delay as u32); - -// // The current value becomes pre, change delay equals difference (due to reduction) -// assert_delay_change_write( -// state_var, -// write_mock, -// post_delay, -// new_delay, -// block_number + post_delay - new_delay -// ); -// } \ No newline at end of file +#[test] +fn test_get_current_value_in_private_with_non_initial_delay() { + let mut env = setup(); + + let public_state_var = in_public(env); + 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 historical_block_number = if value_block_of_change > delay_block_of_change { + value_block_of_change + } else { + delay_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.context.max_block_number.unwrap(), historical_block_number + new_delay); +} + +#[test(should_fail_with="Hint values do not match hash")] +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((private_state_var.get_value_change_storage_slot(), 3)).returns(mocked.serialize()).times(1); + + let _ = private_state_var.get_current_value_in_private(); +} + +#[test(should_fail_with="Hint values do not match hash")] +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((private_state_var.get_delay_change_storage_slot(), 1)).returns(mocked.serialize()).times(1); + + let _ = private_state_var.get_current_value_in_private(); +} + +#[test(should_fail_with="Non-zero value change for zero hash")] +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((state_var.get_value_change_storage_slot(), 3)).returns(mocked.serialize()).times(1); + + let _ = state_var.get_current_value_in_private(); +} + +#[test(should_fail_with="Non-zero delay change for zero hash")] +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((state_var.get_delay_change_storage_slot(), 1)).returns(mocked.serialize()).times(1); + + let _ = state_var.get_current_value_in_private(); +} diff --git a/noir-projects/noir-contracts/contracts/key_registry_contract/src/main.nr b/noir-projects/noir-contracts/contracts/key_registry_contract/src/main.nr index aef32ca6133..f51631c1f1c 100644 --- a/noir-projects/noir-contracts/contracts/key_registry_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/key_registry_contract/src/main.nr @@ -41,8 +41,15 @@ contract KeyRegistry { npk_m_y_registry.schedule_value_change(new_npk_m.y); } + // We need to have two separate register functions because a single one would produce too many storage writes, since + // each SharedMutable.schedule_value_change call results in 5 writes (pre, post, block_of_change, delay and hash), + // totaling 40 writes, while the kernels only accept up to 32 writes. + // Once SharedMutable accepts multi-field values, we can have a single state variable hold all keys, and that way + // also have a single block of change, hash, and delay. + // TODO (#5491): make this be a single function with a single schedule call. + #[aztec(public)] - fn register(address: AztecAddress, partial_address: PartialAddress, keys: PublicKeys) { + fn register_npk_and_ivpk(address: AztecAddress, partial_address: PartialAddress, keys: PublicKeys) { let computed_address = AztecAddress::compute(keys.hash(), partial_address); assert(computed_address.eq(address), "Computed address does not match supplied address"); @@ -51,15 +58,24 @@ contract KeyRegistry { let npk_m_y_registry = storage.npk_m_y_registry.at(address); let ivpk_m_x_registry = storage.ivpk_m_x_registry.at(address); let ivpk_m_y_registry = storage.ivpk_m_y_registry.at(address); - let ovpk_m_x_registry = storage.ovpk_m_x_registry.at(address); - let ovpk_m_y_registry = storage.ovpk_m_y_registry.at(address); - let tpk_m_x_registry = storage.tpk_m_x_registry.at(address); - let tpk_m_y_registry = storage.tpk_m_y_registry.at(address); npk_m_x_registry.schedule_value_change(keys.npk_m.x); npk_m_y_registry.schedule_value_change(keys.npk_m.y); ivpk_m_x_registry.schedule_value_change(keys.ivpk_m.x); ivpk_m_y_registry.schedule_value_change(keys.ivpk_m.y); + } + + #[aztec(public)] + fn register_ovpk_and_tpk(address: AztecAddress, partial_address: PartialAddress, keys: PublicKeys) { + let computed_address = AztecAddress::compute(keys.hash(), partial_address); + + assert(computed_address.eq(address), "Computed address does not match supplied address"); + + let ovpk_m_x_registry = storage.ovpk_m_x_registry.at(address); + let ovpk_m_y_registry = storage.ovpk_m_y_registry.at(address); + let tpk_m_x_registry = storage.tpk_m_x_registry.at(address); + let tpk_m_y_registry = storage.tpk_m_y_registry.at(address); + ovpk_m_x_registry.schedule_value_change(keys.ovpk_m.x); ovpk_m_y_registry.schedule_value_change(keys.ovpk_m.y); tpk_m_x_registry.schedule_value_change(keys.tpk_m.x); 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 4321a2a7f28..bbb93b0efa4 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 @@ -41,6 +41,12 @@ impl ToField for UserFlags { } } +impl Eq for UserFlags { + fn eq(self, other: Self) -> bool { + (self.is_admin == other.is_admin) & (self.is_minter == other.is_minter) & (self.is_blacklisted == other.is_blacklisted) + } +} + // We implement this as it is used when serializing the state variable into return values // This is very inefficient if used to store the state variable. // We are currently "abusing" that the `to_field` is called in the `scheduled_value_change` diff --git a/yarn-project/end-to-end/src/e2e_key_registry.test.ts b/yarn-project/end-to-end/src/e2e_key_registry.test.ts index d667eef74e3..a9d3a3c3d58 100644 --- a/yarn-project/end-to-end/src/e2e_key_registry.test.ts +++ b/yarn-project/end-to-end/src/e2e_key_registry.test.ts @@ -58,7 +58,7 @@ describe('Key Registry', () => { await expect( keyRegistry .withWallet(wallets[0]) - .methods.register( + .methods.register_npk_and_ivpk( account, account.partialAddress, // TODO(#6337): Make calling `toNoirStruct()` unnecessary @@ -108,7 +108,18 @@ describe('Key Registry', () => { it('registers', async () => { await keyRegistry .withWallet(wallets[0]) - .methods.register( + .methods.register_npk_and_ivpk( + account, + account.partialAddress, + // TODO(#6337): Make calling `toNoirStruct()` unnecessary + account.publicKeys.toNoirStruct(), + ) + .send() + .wait(); + + await keyRegistry + .withWallet(wallets[0]) + .methods.register_ovpk_and_tpk( account, account.partialAddress, // TODO(#6337): Make calling `toNoirStruct()` unnecessary diff --git a/yarn-project/simulator/src/client/private_execution.test.ts b/yarn-project/simulator/src/client/private_execution.test.ts index e5e1e22ac18..8b439279a91 100644 --- a/yarn-project/simulator/src/client/private_execution.test.ts +++ b/yarn-project/simulator/src/client/private_execution.test.ts @@ -2,6 +2,7 @@ import { type AztecNode, EncryptedNoteFunctionL2Logs, type L1ToL2Message, + type L2BlockNumber, Note, PackedValues, PublicDataWitness, @@ -262,6 +263,12 @@ describe('Private Execution test suite', () => { ), ); + node = mock(); + // eslint-disable-next-line @typescript-eslint/no-unused-vars + node.getPublicStorageAt.mockImplementation((address: Fr, storageSlot: Fr, blockNumber: L2BlockNumber) => { + return Promise.resolve(Fr.ZERO); + }); + acirSimulator = new AcirSimulator(oracle, node); });