Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: prepare ScheduledValueChange for mutable delays. #6085

Merged
merged 6 commits into from
Apr 30, 2024
Merged

chore: prepare ScheduledValueChange for mutable delays. #6085

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e2a0956
Remove delay from ScheduledValueChange
nventuro Apr 26, 2024
ecb94f3
Remove immutable delays from scheduled value change
nventuro Apr 29, 2024
d04fe04
Merge branch 'master' into nv/shared-mutable-infra
nventuro Apr 29, 2024
841fb05
Address review comments
nventuro Apr 30, 2024
ab67189
Merge branch 'master' into nv/shared-mutable-infra
nventuro Apr 30, 2024
8b0c826
Merge branch 'master' into nv/shared-mutable-infra
nventuro Apr 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 72 additions & 54 deletions noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/scheduled_value_change.nr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,18 @@ use dep::protocol_types::traits::{Serialize, Deserialize, FromField, ToField};

// This data structure is used by SharedMutable to represent a value that changes from `pre` to `post` at some block
// called the `block_of_change`. The value can only be made to change by scheduling a change event at some future block
// of change after some delay measured in blocks has elapsed. This means that at any given block number we know both the
// current value and the smallest block number at which the value might change - this is called the 'block horizon'.
//
// The delay being a type parameter instead of a struct field is an implementation detail, and is due to a number of
// reasons:
// - we want to serialize and deserialize this object in order to store it in public storage, but we don't want to
// include the delay there because it is immutable
// - because of how aztec-nr state variables are declared, having a type with some immutable property is better
// expressed via types, since they are always constructed with the same `::new(context, storage_slot)` function.
struct ScheduledValueChange<T, DELAY> {
// of change after some minimum delay measured in blocks has elapsed. This means that at any given block number we know
// both the current value and the smallest block number at which the value might change - this is called the
// 'block horizon'.
struct ScheduledValueChange<T> {
pre: T,
post: T,
block_of_change: u32,
// The _dummy variable forces DELAY to be interpreted as a numberic value. This is a workaround to
// https://github.com/noir-lang/noir/issues/4633. Remove once resolved.
_dummy: [Field; DELAY],
}

impl<T, DELAY> ScheduledValueChange<T, DELAY> {
impl<T> ScheduledValueChange<T> {
pub fn new(pre: T, post: T, block_of_change: u32) -> Self {
Self { pre, post, block_of_change, _dummy: [0; DELAY] }
Self { pre, post, block_of_change }
}

/// Returns the value stored in the data structure at a given block. This function can be called both in public
Expand Down Expand Up @@ -54,10 +45,12 @@ impl<T, DELAY> ScheduledValueChange<T, DELAY> {
/// Returns the largest block number at which the value returned by `get_current_at` is known to remain the current
/// value. This value is only meaningful in private when constructing a proof at some `historical_block_number`,
/// since due to its asynchronous nature private execution cannot know about any later scheduled changes.
/// The caller of this function must know how quickly the value can change due to a scheduled change in the form of
/// `minimum_delay`. If the delay itself is immutable, then this is just its duration.
/// The value returned by `get_current_at` in private when called with a historical block number is only safe to use
/// if the transaction's `max_block_number` property is set to a value lower or equal to the block horizon computed
/// using the same historical block number.
pub fn get_block_horizon(self, historical_block_number: u32) -> u32 {
pub fn get_block_horizon(self, historical_block_number: u32, minimum_delay: u32) -> u32 {
// The block horizon is the very last block in which the current value is known. Any block past the horizon
// (i.e. with a block number larger than the block horizon) may have a different current value. Reading the
// current value in private typically requires constraining the maximum valid block number to be equal to the
Expand All @@ -68,77 +61,87 @@ impl<T, DELAY> ScheduledValueChange<T, DELAY> {
// change is scheduled. This did not happen at the historical block number (or else it would not be
// greater or equal to the block of change), and therefore could only happen after the historical block
// number. The earliest would be the immediate next block, and so the smallest possible next block of change
// equals `historical_block_number + 1 + DELAY`. Our block horizon is simply the previous block to that one.
// equals `historical_block_number + 1 + minimum_delay`. Our block horizon is simply the previous block to
// that one.
//
// block of historical
// change block block horizon
// =======|=============N===================H===========>
// ^ ^
// ---------------------
// delay
// minimum delay

historical_block_number + DELAY
historical_block_number + minimum_delay
} else {
// If the block of change has not yet been mined however, then there are two possible scenarios.
// a) It could be so far into the future that the block horizon is actually determined by the delay,
// because a new change could be scheduled and take place _before_ the currently scheduled one. This is
// similar to the scenario where the block of change is in the past: the time horizon is the block
// prior to the earliest one in which a new block of change might land.
// a) It could be so far into the future that the block horizon is actually determined by the minimum
// delay, because a new change could be scheduled and take place _before_ the currently scheduled one.
// This is similar to the scenario where the block of change is in the past: the time horizon is the
// block prior to the earliest one in which a new block of change might land.
//
// historical
// block block horizon block of change
// =====N=================================H=================|=========>
// ^ ^
// | |
// -----------------------------------
// delay
// minimum delay
//
// b) It could be fewer than `delay` blocks away from the historical block number, in which case it would
// become the limiting factor for the time horizon, which would be the block right before the block of
// change (since by definition the value changes at the block of change).
// b) It could be fewer than `minimum_delay` blocks away from the historical block number, in which case
// the block of change would become the limiting factor for the time horizon, which would equal the
// block right before the block of change (since by definition the value changes at the block of
// change).
//
// historical block horizon
// block block of change if not scheduled
// =======N=============|===================H=================>
// ^ ^ ^
// | actual horizon |
// -----------------------------------
// delay
// minimum delay
//
// Note that the current implementation does not allow the caller to set the block of change to an arbitrary
// value, and therefore scenario a) is not currently possible. However implementing #5501 would allow for
// this to happen.

// Because historical_block_number < self.block_of_change, then block_of_change > 0 and we can safely
// subtract 1.
min(self.block_of_change - 1, historical_block_number + DELAY)
min(
self.block_of_change - 1,
historical_block_number + minimum_delay
)
}
}

/// Mutates a scheduled value change by scheduling a change at the current block number. This function is only
/// meaningful when called in public with the current block number.
pub fn schedule_change(&mut self, new_value: T, current_block_number: u32) {
pub fn schedule_change(
&mut self,
new_value: T,
current_block_number: u32,
delay: u32,
nventuro marked this conversation as resolved.
Show resolved Hide resolved
block_of_change: u32
) {
self.pre = self.get_current_at(current_block_number);
self.post = new_value;
// TODO: make this configurable
// https://github.com/AztecProtocol/aztec-packages/issues/5501
self.block_of_change = current_block_number + DELAY;

assert(block_of_change >= current_block_number + delay);
nventuro marked this conversation as resolved.
Show resolved Hide resolved
self.block_of_change = block_of_change;
}
}

impl<T, DELAY> Serialize<3> for ScheduledValueChange<T, DELAY> {
impl<T> Serialize<3> for ScheduledValueChange<T> {
fn serialize(self) -> [Field; 3] where T: ToField {
[self.pre.to_field(), self.post.to_field(), self.block_of_change.to_field()]
}
}

impl<T, DELAY> Deserialize<3> for ScheduledValueChange<T, DELAY> {
impl<T> Deserialize<3> for ScheduledValueChange<T> {
fn deserialize(input: [Field; 3]) -> Self where T: FromField {
Self {
pre: FromField::from_field(input[0]),
post: FromField::from_field(input[1]),
block_of_change: FromField::from_field(input[2]),
_dummy: [0; DELAY]
block_of_change: FromField::from_field(input[2]),
}
}
}
Expand All @@ -157,15 +160,15 @@ fn test_min() {
mod test {
use crate::state_vars::shared_mutable::scheduled_value_change::ScheduledValueChange;

global TEST_DELAY = 200;
global TEST_DELAY: u32 = 200;

#[test]
fn test_get_current_at() {
let pre = 1;
let post = 2;
let block_of_change = 50;

let value: ScheduledValueChange<Field, TEST_DELAY> = ScheduledValueChange::new(pre, post, block_of_change);
let value: ScheduledValueChange<Field> = ScheduledValueChange::new(pre, post, block_of_change);

assert_eq(value.get_current_at(0), pre);
assert_eq(value.get_current_at(block_of_change - 1), pre);
Expand All @@ -179,13 +182,13 @@ mod test {
let post = 2;
let block_of_change = 50;

let value: ScheduledValueChange<Field, TEST_DELAY> = ScheduledValueChange::new(pre, post, block_of_change);
let value: ScheduledValueChange<Field> = ScheduledValueChange::new(pre, post, block_of_change);

assert_eq(value.get_scheduled(), (post, block_of_change));
}

fn assert_block_horizon_invariants(
value: &mut ScheduledValueChange<Field, TEST_DELAY>,
value: &mut ScheduledValueChange<Field>,
historical_block_number: u32,
block_horizon: u32
) {
Expand All @@ -198,7 +201,12 @@ mod test {
// changing at the previously determined block_horizon.

let new = value.pre + value.post; // Make sure it's different to both pre and post
value.schedule_change(new, historical_block_number + 1);
value.schedule_change(
new,
historical_block_number + 1,
TEST_DELAY,
historical_block_number + 1 + TEST_DELAY
);

assert(value.block_of_change > block_horizon);
assert_eq(current_at_historical, value.get_current_at(block_horizon));
Expand All @@ -209,9 +217,9 @@ mod test {
let historical_block_number = 100;
let block_of_change = 50;

let mut value: ScheduledValueChange<Field, TEST_DELAY> = ScheduledValueChange::new(1, 2, block_of_change);
let mut value: ScheduledValueChange<Field> = ScheduledValueChange::new(1, 2, block_of_change);

let block_horizon = value.get_block_horizon(historical_block_number);
let block_horizon = value.get_block_horizon(historical_block_number, TEST_DELAY);
assert_eq(block_horizon, historical_block_number + TEST_DELAY);

assert_block_horizon_invariants(&mut value, historical_block_number, block_horizon);
Expand All @@ -222,9 +230,9 @@ mod test {
let historical_block_number = 100;
let block_of_change = 100;

let mut value: ScheduledValueChange<Field, TEST_DELAY> = ScheduledValueChange::new(1, 2, block_of_change);
let mut value: ScheduledValueChange<Field> = ScheduledValueChange::new(1, 2, block_of_change);

let block_horizon = value.get_block_horizon(historical_block_number);
let block_horizon = value.get_block_horizon(historical_block_number, TEST_DELAY);
assert_eq(block_horizon, historical_block_number + TEST_DELAY);

assert_block_horizon_invariants(&mut value, historical_block_number, block_horizon);
Expand All @@ -235,12 +243,12 @@ mod test {
let historical_block_number = 100;
let block_of_change = 120;

let mut value: ScheduledValueChange<Field, TEST_DELAY> = ScheduledValueChange::new(1, 2, block_of_change);
let mut value: ScheduledValueChange<Field> = ScheduledValueChange::new(1, 2, block_of_change);

// Note that this is the only scenario in which the block of change informs the block horizon.
// This may result in privacy leaks when interacting with applications that have a scheduled change
// in the near future.
let block_horizon = value.get_block_horizon(historical_block_number);
let block_horizon = value.get_block_horizon(historical_block_number, TEST_DELAY);
assert_eq(block_horizon, block_of_change - 1);

assert_block_horizon_invariants(&mut value, historical_block_number, block_horizon);
Expand All @@ -251,9 +259,9 @@ mod test {
let historical_block_number = 100;
let block_of_change = 500;

let mut value: ScheduledValueChange<Field, TEST_DELAY> = ScheduledValueChange::new(1, 2, block_of_change);
let mut value: ScheduledValueChange<Field> = ScheduledValueChange::new(1, 2, block_of_change);

let block_horizon = value.get_block_horizon(historical_block_number);
let block_horizon = value.get_block_horizon(historical_block_number, TEST_DELAY);
assert_eq(block_horizon, historical_block_number + TEST_DELAY);

assert_block_horizon_invariants(&mut value, historical_block_number, block_horizon);
Expand All @@ -265,11 +273,16 @@ mod test {
let post = 2;
let block_of_change = 500;

let mut value: ScheduledValueChange<Field, TEST_DELAY> = ScheduledValueChange::new(pre, post, block_of_change);
let mut value: ScheduledValueChange<Field> = ScheduledValueChange::new(pre, post, block_of_change);

let new = 42;
let current_block_number = block_of_change - 50;
value.schedule_change(new, current_block_number);
value.schedule_change(
new,
current_block_number,
TEST_DELAY,
current_block_number + TEST_DELAY
);

// Because we re-schedule before the last scheduled change takes effect, the old `post` value is lost.
assert_eq(value.pre, pre);
Expand All @@ -283,11 +296,16 @@ mod test {
let post = 2;
let block_of_change = 500;

let mut value: ScheduledValueChange<Field, TEST_DELAY> = ScheduledValueChange::new(pre, post, block_of_change);
let mut value: ScheduledValueChange<Field> = ScheduledValueChange::new(pre, post, block_of_change);

let new = 42;
let current_block_number = block_of_change + 50;
value.schedule_change(new, current_block_number);
value.schedule_change(
new,
current_block_number,
TEST_DELAY,
current_block_number + TEST_DELAY
);

assert_eq(value.pre, post);
assert_eq(value.post, new);
Expand Down
24 changes: 16 additions & 8 deletions noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/shared_mutable.nr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ use crate::state_vars::{storage::Storage, shared_mutable::scheduled_value_change
struct SharedMutable<T, DELAY> {
context: Context,
storage_slot: Field,
// The _dummy variable forces DELAY to be interpreted as a numberic value. This is a workaround to
nventuro marked this conversation as resolved.
Show resolved Hide resolved
// https://github.com/noir-lang/noir/issues/4633. Remove once resolved.
_dummy: [Field; DELAY],
}

impl<T, DELAY> Storage<T> for SharedMutable<T, DELAY> {}
Expand All @@ -24,35 +27,40 @@ impl<T, DELAY> Storage<T> for SharedMutable<T, DELAY> {}
impl<T, DELAY> SharedMutable<T, DELAY> {
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 }
Self { context, storage_slot, _dummy: [0; DELAY] }
}

pub fn schedule_value_change(self, new_value: T) {
let context = self.context.public.unwrap();
let mut scheduled_value_change: ScheduledValueChange<T, DELAY> = public_storage::read(self.get_derived_storage_slot());
let mut scheduled_value_change: ScheduledValueChange<T> = public_storage::read(self.get_derived_storage_slot());

scheduled_value_change.schedule_change(new_value, context.block_number() as u32);
let block_number = context.block_number() as u32;
// TODO: make this configurable
// https://github.com/AztecProtocol/aztec-packages/issues/5501
let block_of_change = block_number + DELAY;

scheduled_value_change.schedule_change(new_value, block_number, DELAY, block_of_change);

public_storage::write(self.get_derived_storage_slot(), scheduled_value_change);
}

pub fn get_current_value_in_public(self) -> T {
let scheduled_value_change: ScheduledValueChange<T, DELAY> = public_storage::read(self.get_derived_storage_slot());
let scheduled_value_change: ScheduledValueChange<T> = public_storage::read(self.get_derived_storage_slot());

let block_number = self.context.public.unwrap().block_number() as u32;
scheduled_value_change.get_current_at(block_number)
}

pub fn get_scheduled_value_in_public(self) -> (T, u32) {
let scheduled_value_change: ScheduledValueChange<T, DELAY> = public_storage::read(self.get_derived_storage_slot());
let scheduled_value_change: ScheduledValueChange<T> = public_storage::read(self.get_derived_storage_slot());
scheduled_value_change.get_scheduled()
}

pub fn get_current_value_in_private(self) -> T where T: FromField {
let mut context = self.context.private.unwrap();

let (scheduled_value_change, historical_block_number) = self.historical_read_from_public_storage(*context);
let block_horizon = scheduled_value_change.get_block_horizon(historical_block_number);
let block_horizon = scheduled_value_change.get_block_horizon(historical_block_number, 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.
Expand All @@ -63,7 +71,7 @@ impl<T, DELAY> SharedMutable<T, DELAY> {
fn historical_read_from_public_storage(
self,
context: PrivateContext
) -> (ScheduledValueChange<T, DELAY>, u32) where T: FromField {
) -> (ScheduledValueChange<T>, u32) where T: FromField {
let derived_slot = self.get_derived_storage_slot();

// Ideally the following would be simply public_storage::read_historical, but we can't implement that yet.
Expand All @@ -76,7 +84,7 @@ impl<T, DELAY> SharedMutable<T, DELAY> {
);
}

let scheduled_value: ScheduledValueChange<T, DELAY> = ScheduledValueChange::deserialize(raw_fields);
let scheduled_value: ScheduledValueChange<T> = ScheduledValueChange::deserialize(raw_fields);
let historical_block_number = context.historical_header.global_variables.block_number as u32;

(scheduled_value, historical_block_number)
Expand Down
Loading
Loading