Skip to content
Open
Show file tree
Hide file tree
Changes from 10 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ The existence of minimum delays means that a private function that reads a publi

#### Declaration

Unlike other state variables, `DelayedPublicMutable` receives not only a type parameter for the underlying datatype, but also a `DELAY` type parameter with the value change delay as a number of seconds.
Unlike other state variables, `DelayedPublicMutable` receives not only a type parameter for the underlying datatype, but also a `DELAY` type parameter with the value change delay as a number of seconds. Delays must be greater than zero, both here and when scheduling a delay change: with no delay the value could change at any moment, leaving no window in which a private read is valid.

#include_code delayed_public_mutable_storage /noir-projects/labs/noir-contracts/contracts/app/auth_contract/src/main.nr rust

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ use crate::{context::{PrivateContext, PublicContext, UtilityContext}, state_vars
///
/// Additionally, a lower `expiration_timestamp` obviously causes transactions to expire earlier, resulting in
/// multiple issues. Among others, this can make large transactions that take long to prove be unfeasible, restrict
/// users with slow proving devices, and force large transaction fees to guarantee fast inclusion.
/// users with slow proving devices, and force large transaction fees to guarantee fast inclusion. At the limit, a zero
/// delay leaves no window at all in which a read is known to hold, making private reads impossible. Zero delays are
/// therefore rejected outright, both when declaring the state variable and when scheduling a delay change.
///
/// In practice, a delay of at least a couple hours is recommended. From a privacy point of view the optimal delay is
/// [`crate::protocol::constants::MAX_TX_LIFETIME`], which puts contracts in the same privacy set as those that do not
Expand Down Expand Up @@ -129,6 +131,7 @@ where
DelayedPublicMutableValues<T, InitialDelay>: Packable<N = M>,
{
fn new(context: Context, storage_slot: Field) -> Self {
std::static_assert(InitialDelay > 0, "InitialDelay must be greater than zero");
assert(storage_slot != 0, "Storage slot 0 not allowed. Storage slots must start from 1.");
Self { context, storage_slot }
}
Expand Down Expand Up @@ -211,6 +214,8 @@ where
/// [`get_current_delay`](DelayedPublicMutable::get_current_delay) automatically begins to return `new_delay`, and
/// [`schedule_value_change`](DelayedPublicMutable::schedule_value_change) begins using it.
///
/// `new_delay` must be greater than zero.
///
/// ## Multiple Scheduled Changes
///
/// Only a **single** delay can be scheduled to become the new delay at a given point in time. Any prior scheduled
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ where
pub fn unpack_delay_change<let INITIAL_DELAY: u64>(
packed: Field,
) -> ScheduledDelayChange<INITIAL_DELAY> {
std::static_assert(INITIAL_DELAY > 0, "INITIAL_DELAY must be greater than zero");

// This function expects to be called with just the first field of the packed representation, which contains sdc
// and svc timestamp_of_change. We'll discard the svc component.
let svc_timestamp_of_change = packed as u32;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use crate::{constants::{MAX_FIELD_VALUE, MAX_U32_VALUE}, traits::Packable};
use crate::delayed_public_mutable::{
delayed_public_mutable_values::DelayedPublicMutableValues,
scheduled_delay_change::ScheduledDelayChange, scheduled_value_change::ScheduledValueChange,
delayed_public_mutable_values::{DelayedPublicMutableValues, unpack_delay_change},
scheduled_delay_change::ScheduledDelayChange,
scheduled_value_change::ScheduledValueChange,
};

global TEST_INITIAL_DELAY: u64 = 13;
Expand Down Expand Up @@ -150,6 +151,14 @@ unconstrained fn schedule_change_accepts_delay_at_u32_max() {
assert_eq(sdc.post.unwrap(), max_u32);
}

// Every read of a stored delay goes through `unpack_delay_change`, so guarding it there covers both the aztec-nr and
// `aztec_sublib` state variables as well as the private kernel's contract update horizon. The `static_assert` failure
// surfaces as a test failure because each test is compiled on demand.
Comment thread
vezenovm marked this conversation as resolved.
Outdated
#[test(should_fail_with = "INITIAL_DELAY must be greater than zero")]
unconstrained fn unpacking_a_zero_initial_delay_fails() {
let _ = unpack_delay_change::<0_u64>(0);
}

#[test]
unconstrained fn packed_delayed_public_mutable_values_match_typescript() {
let pre_value = MockStruct { a: 1, b: 2 };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ mod test;
// is performed via `schedule_change` in order to satisfy ScheduleValueChange constraints: if e.g. we allowed for the
// delay to be decreased immediately then it'd be possible for the state variable to schedule a value change with a
// reduced delay, invalidating prior private reads.
// INITIAL_DELAY must be nonzero, since a zero delay leaves no window during which a private read is known to remain
// valid. DelayedPublicMutable rejects it when the state variable is declared, and `unpack_delay_change` guards every
// read of a stored delay, which also covers readers that do not go through a state variable.
pub struct ScheduledDelayChange<let INITIAL_DELAY: u64> {
// Both pre and post are stored in public storage, so by default they are zeroed. By wrapping them in an Option,
// they default to Option::none(), which we detect and replace with INITIAL_DELAY. The end result is that a
Expand Down Expand Up @@ -52,7 +55,12 @@ impl<let INITIAL_DELAY: u64> ScheduledDelayChange<INITIAL_DELAY> {
/// - when reducing the delay, the change will take effect after a delay equal to the difference between old and
/// new delay. For example, if reducing from 3 days to 1 day, the reduction will be scheduled to happen after 2
/// days.
///
/// The new delay must be strictly positive: a zero delay would let the value change at any moment, so there'd be no
/// window during which a private read is known to remain valid.
pub fn schedule_change(&mut self, new: u64, current_timestamp: u64) {
assert(new > 0, "Delay must be greater than zero");

let current = self.get_current(current_timestamp);

// When changing the delay value we must ensure that it is not possible to produce a value change with a delay
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,22 @@ unconstrained fn test_schedule_change_to_longer_delay_from_initial() {
assert_eq(delay_change.get_current(current_timestamp), new);
}

// A zero delay leaves no window during which a value is known not to change, so there is no effective minimum delay to
// report and the computation underflows instead. This is why `schedule_change` rejects zero.
#[test(should_fail_with = "attempt to subtract with overflow")]
unconstrained fn test_zero_delay_has_no_effective_minimum_delay() {
let delay_change = get_non_initial_delay_change(0, 0, 0);

let _ = delay_change.get_effective_minimum_delay_at(100);
}

Comment thread
vezenovm marked this conversation as resolved.
Outdated
#[test(should_fail_with = "Delay must be greater than zero")]
unconstrained fn test_schedule_change_to_zero_delay_fails() {
let mut delay_change = get_initial_delay_change();

delay_change.schedule_change(0, 50);
}

unconstrained fn assert_effective_minimum_delay_invariants<let INITIAL_DELAY: u64>(
delay_change: &mut ScheduledDelayChange<INITIAL_DELAY>,
anchor_block_timestamp: u64,
Expand Down Expand Up @@ -191,11 +207,11 @@ unconstrained fn assert_effective_minimum_delay_invariants<let INITIAL_DELAY: u6
change_schedule_timestamp + delay_change.get_current(change_schedule_timestamp);
assert(expected_earliest_value_change_timestamp <= value_change_timestamp);

// Finally, a delay reduction could be scheduled at the anchor block timestamp. We reduce the delay to zero, which
// means that at the delay timestamp of change there'll be no delay and a value change could be performed
// immediately then.
delay_change.schedule_change(0, anchor_block_timestamp + 0);
assert(expected_earliest_value_change_timestamp <= delay_change.timestamp_of_change);
// Finally, a delay reduction could be scheduled at the anchor block timestamp. We reduce the delay to 1 second (the
// smallest permitted delay), which means that at the delay timestamp of change a value change could be performed
// one second later.
delay_change.schedule_change(1, anchor_block_timestamp);
assert(expected_earliest_value_change_timestamp <= delay_change.timestamp_of_change + 1);
}

#[test]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,9 @@ mod test;
///
/// Additionally, a lower `expiration_timestamp` obviously causes transactions to expire earlier, resulting in
/// multiple issues. Among others, this can make large transactions that take long to prove be unfeasible, restrict
/// users with slow proving devices, and force large transaction fees to guarantee fast inclusion.
/// users with slow proving devices, and force large transaction fees to guarantee fast inclusion. At the limit, a zero
/// delay leaves no window at all in which a read is known to hold, making private reads impossible. Zero delays are
/// therefore rejected outright, both when declaring the state variable and when scheduling a delay change.
///
/// In practice, a delay of at least a couple hours is recommended. From a privacy point of view the optimal delay is
/// [`crate::protocol::constants::MAX_TX_LIFETIME`], which puts contracts in the same privacy set as those that do not
Expand Down Expand Up @@ -131,6 +133,7 @@ where
DelayedPublicMutableValues<T, InitialDelay>: Packable<N = M>,
{
fn new(context: Context, storage_slot: Field) -> Self {
std::static_assert(InitialDelay > 0, "InitialDelay must be greater than zero");
assert(storage_slot != 0, "Storage slot 0 not allowed. Storage slots must start from 1.");
Self { context, storage_slot }
}
Expand Down Expand Up @@ -213,6 +216,8 @@ where
/// [`get_current_delay`](DelayedPublicMutable::get_current_delay) automatically begins to return `new_delay`, and
/// [`schedule_value_change`](DelayedPublicMutable::schedule_value_change) begins using it.
///
/// `new_delay` must be greater than zero.
///
/// ## Multiple Scheduled Changes
///
/// Only a **single** delay can be scheduled to become the new delay at a given point in time. Any prior scheduled
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ unconstrained fn in_utility(
DelayedPublicMutable::new(context, storage_slot)
}

// The `static_assert` failure surfaces as a test failure because each test is compiled on demand.
#[test(should_fail_with = "InitialDelay must be greater than zero")]
unconstrained fn declaring_with_zero_initial_delay_fails() {
let _: DelayedPublicMutable<MockStruct, 0_u64, PublicContext> = DelayedPublicMutable::new(zeroed(), storage_slot);
}

#[test]
unconstrained fn get_current_value_in_public_initial() {
let env = TestEnvironment::new();
Expand Down
Loading