diff --git a/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/node_store/cached_content_addressed_tree_store.hpp b/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/node_store/cached_content_addressed_tree_store.hpp index 01888967c45b..8093d2884049 100644 --- a/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/node_store/cached_content_addressed_tree_store.hpp +++ b/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/node_store/cached_content_addressed_tree_store.hpp @@ -867,6 +867,10 @@ void ContentAddressedCachedTreeStore::advance_finalized_block(con ReadTransactionPtr readTx = create_read_transaction(); get_meta(uncommittedMeta); get_meta(committedMeta, *readTx, false); + // do nothing if the block is already finalized + if (committedMeta.finalizedBlockHeight >= blockNumber) { + return; + } if (!dataStore_->read_block_data(blockNumber, blockPayload, *readTx)) { throw std::runtime_error(format("Unable to advance finalized block: ", blockNumber, @@ -874,10 +878,6 @@ void ContentAddressedCachedTreeStore::advance_finalized_block(con forkConstantData_.name_)); } } - // do nothing if the block is already finalized - if (committedMeta.finalizedBlockHeight >= blockNumber) { - return; - } // can currently only finalize up to the unfinalized block height if (committedMeta.finalizedBlockHeight > committedMeta.unfinalizedBlockHeight) { diff --git a/noir-projects/aztec-nr/aztec/src/context/private_context.nr b/noir-projects/aztec-nr/aztec/src/context/private_context.nr index 4f278b5fa62c..d0a1d1ea8547 100644 --- a/noir-projects/aztec-nr/aztec/src/context/private_context.nr +++ b/noir-projects/aztec-nr/aztec/src/context/private_context.nr @@ -607,7 +607,7 @@ impl PrivateContext { /// network as a whole. For example, if a contract interaction sets include-by to some publicly-known value (e.g. /// the time when a contract upgrades), then the wallet might wish to set an even lower one to avoid revealing that /// this tx is interacting with said contract. Ideally, all wallets should standardize on an approach in order to - /// provide users with a large anonymity set -- although the exact approach + /// provide users with a large privacy set -- although the exact approach /// will need to be discussed. Wallets that deviate from a standard might accidentally reveal which wallet each /// transaction originates from. /// diff --git a/noir-projects/aztec-nr/aztec/src/history/nullifier.nr b/noir-projects/aztec-nr/aztec/src/history/nullifier.nr index 866e4fe9a1cb..329a6c1b33bb 100644 --- a/noir-projects/aztec-nr/aztec/src/history/nullifier.nr +++ b/noir-projects/aztec-nr/aztec/src/history/nullifier.nr @@ -26,7 +26,7 @@ mod test; /// /// ## Cost /// -/// This function performs a full merkle tree inclusion proof, which is in the order of 4k gates. +/// This function performs a single merkle tree inclusion proof, which is in the order of 4k gates. /// /// If you don't need to assert existence at a _specific_ past block, consider using /// [`PrivateContext::assert_nullifier_exists`](crate::context::PrivateContext::assert_nullifier_exists) instead, which @@ -82,7 +82,7 @@ pub fn assert_nullifier_existed_by(block_header: BlockHeader, siloed_nullifier: /// /// ## Cost /// -/// This function performs a full merkle tree inclusion proof, which is in the order of 4k gates. +/// This function performs a single merkle tree inclusion proof, which is in the order of 4k gates. pub fn assert_nullifier_did_not_exist_by(block_header: BlockHeader, siloed_nullifier: Field) { // 1) Get the membership witness of a low nullifier of the nullifier. // Safety: The witness is only used as a "magical value" that makes the proof below pass. Hence it's safe. diff --git a/noir-projects/aztec-nr/aztec/src/macros/notes.nr b/noir-projects/aztec-nr/aztec/src/macros/notes.nr index 7c51db3a046d..9943804900e7 100644 --- a/noir-projects/aztec-nr/aztec/src/macros/notes.nr +++ b/noir-projects/aztec-nr/aztec/src/macros/notes.nr @@ -280,7 +280,7 @@ pub comptime fn custom_note(s: TypeDefinition) -> Quoted { } } -/// Asserts that the given note implements the `Packable` trait. +/// Asserts that the given note implements the [`Packable`](crate::protocol::traits::Packable) trait. /// /// We require that notes have the `Packable` trait implemented because it is used when emitting a note in a log or as /// an offchain message. diff --git a/noir-projects/aztec-nr/aztec/src/oracle/block_header.nr b/noir-projects/aztec-nr/aztec/src/oracle/block_header.nr index d1f7b81243e2..b213b73e4a7b 100644 --- a/noir-projects/aztec-nr/aztec/src/oracle/block_header.nr +++ b/noir-projects/aztec-nr/aztec/src/oracle/block_header.nr @@ -75,11 +75,107 @@ fn constrain_get_block_header_at_internal( } mod test { + use crate::protocol::traits::Hash; use crate::test::helpers::test_environment::TestEnvironment; - use super::{constrain_get_block_header_at_internal, get_block_header_at_internal}; + use super::{constrain_get_block_header_at_internal, get_block_header_at, get_block_header_at_internal}; - #[test(should_fail_with = "Proving membership of a block in archive failed")] - unconstrained fn fetching_header_with_mismatched_block_number_should_fail() { + #[test] + unconstrained fn fetching_earliest_block_header_succeeds() { + let env = TestEnvironment::new(); + + env.mine_block(); + env.mine_block(); + env.mine_block(); + env.mine_block(); + + env.private_context(|context| { + let anchor_block_header = context.anchor_block_header; + + let header = get_block_header_at_internal(1); + constrain_get_block_header_at_internal(header, 1, anchor_block_header); + + assert_eq(header.block_number(), 1); + }); + } + + #[test] + unconstrained fn fetching_past_block_header_succeeds() { + let env = TestEnvironment::new(); + + env.mine_block(); + env.mine_block(); + env.mine_block(); + env.mine_block(); + + env.private_context(|context| { + let anchor_block_header = context.anchor_block_header; + let target_block_number = anchor_block_header.block_number() - 2; + + let header = get_block_header_at_internal(target_block_number); + constrain_get_block_header_at_internal(header, target_block_number, anchor_block_header); + + assert_eq(header.block_number(), target_block_number); + }); + } + + #[test] + unconstrained fn fetching_header_immediately_before_anchor_succeeds() { + let env = TestEnvironment::new(); + + env.mine_block(); + env.mine_block(); + env.mine_block(); + env.mine_block(); + + // Block N-1 is the boundary case: last_archive covers exactly up to block N-1. + env.private_context(|context| { + let anchor_block_header = context.anchor_block_header; + let target_block_number = anchor_block_header.block_number() - 1; + + let header = get_block_header_at_internal(target_block_number); + constrain_get_block_header_at_internal(header, target_block_number, anchor_block_header); + + assert_eq(header.block_number(), target_block_number); + }); + } + + #[test] + unconstrained fn fetching_anchor_block_header_works() { + let env = TestEnvironment::new(); + + env.mine_block(); + env.mine_block(); + env.mine_block(); + env.mine_block(); + + env.private_context(|context| { + let anchor_block_number = context.anchor_block_header.block_number(); + + let header = get_block_header_at(anchor_block_number, *context); + + assert_eq(header.block_number(), anchor_block_number); + assert_eq(header.hash(), context.anchor_block_header.hash()); + }); + } + + #[test(should_fail_with = "Last archive block number is smaller than the block number")] + unconstrained fn fetching_future_block_header_fails() { + let env = TestEnvironment::new(); + + env.mine_block(); + env.mine_block(); + env.mine_block(); + env.mine_block(); + + env.private_context(|context| { + let anchor_block_number = context.anchor_block_header.block_number(); + + let _header = get_block_header_at(anchor_block_number + 1, *context); + }); + } + + #[test(should_fail_with = "Block number provided is not the same as the block number from the header hint")] + unconstrained fn fetching_header_with_mismatched_block_number_fails() { let env = TestEnvironment::new(); env.mine_block(); diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/delayed_public_mutable.nr b/noir-projects/aztec-nr/aztec/src/state_vars/delayed_public_mutable.nr index 78cd64900a7b..43bc948327a1 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/delayed_public_mutable.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/delayed_public_mutable.nr @@ -12,26 +12,120 @@ use crate::{context::{PrivateContext, PublicContext, UtilityContext}, state_vars mod test; -/// Public mutable values with private read access. +/// Mutable public values with private read access. +/// +/// This is an advanced public state variable, with no native Solidity equivalent. +/// +/// Like [`PublicMutable`](crate::state_vars::PublicMutable) it represents a public value of type `T` that can be +/// written to repeatedly, but with a key improvement: the current value can also be **read from a private contract +/// function**. +/// +/// This comes at the cost of extra restrictions on the state variable: writes do not come into effect immediately, +/// they must be **scheduled** to take place after some minimum delay. Reading from the state variable will therefore +/// return the previous value until some time passes, which is why this is a _delayed_ mutable variable. +/// +/// It is these delays that enable the capacity for reads from private contract functions, as they provide guarantees +/// regarding how long can some historical state observed at the anchor block be known to not change. +/// +/// Delays can be modified during the lifetime of the contract. +/// +/// ## Access Patterns +/// +/// The current value stored in a `DelayedPublicMutable` can be read from public contract functions, and writes can be +/// scheduled to happen in the future. +/// +/// Public contract functions can also schedule changes to the write delay, as well as inspect any already scheduled +/// value or delay changes. +/// +/// Private contract functions can read the **current** value of the state variable, but not past or scheduled values. +/// They cannot read the current delay, and they cannot schedule any kind of value change. +/// +/// ## Privacy +/// +/// The value stored in `DelayedPublicMutable` is fully public, as are all scheduled value and delay changes. +/// +/// Reads from a private contract function are almost fully private: the only observable effect is that they set the +/// transaction's `expiration_timestamp` property, possibly reducing the privacy set. See +/// [`PrivateContext::set_expiration_timestamp`](crate::context::PrivateContext::set_expiration_timestamp). +/// +/// ## Use Cases +/// +/// These are mostly an extension of [`PublicMutable`](crate::state_vars::PublicMutable)'s, given that what this state +/// variable essentially achieves is to provide private reads to it. For example, it can be used for global contract +/// configuration (such as fees, access control, etc.) that users will need to access during private interactions. +/// +/// The key consideration is whether the enforced minimum delay on writes prevents using this state variable. In some +/// scenarios this restriction is incompatible with requirements (such as a token's total supply, which must always be +/// up to date), while in others the enhanced privacy might make the tradeoff acceptable (such as when dealing with +/// contract pauses or access control revocation, where a delay of some hours could be acceptable). +/// +/// Note that, just like in [`PublicMutable`](crate::state_vars::PublicMutable), the fact that the values are public +/// does not necessarily mean the actions that update these values must themselves be wholly public. To learn more, +/// see the notes there regarding usage of [`only_self`](crate::macros::functions::only_self). +/// +/// ## Choosing Delays +/// +/// A short delay reduces the most obvious downside of `DelayedPublicMutable`, and so it is natural to wish to make it +/// be as low as possible. It is therefore important to understand the tradeoffs involved in delay selection. +/// +/// A shorter delay will result in a lower `expiration_timestamp` property of transactions that privately read the +/// state variable, reducing its privacy set. If the delay is smaller than that of any other contract, then this +/// privacy leak might be large enough to uniquely identify those transactions that interact with the contract - fully +/// defeating the purpose of `DelayedPublicMutable`. +/// +/// 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. +/// +/// 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 +/// use `DelayedPublicMutable` at all. +/// +/// ## Examples +/// +/// Declaring a `DelayedPublicMutable` in the contract's [`storage`](crate::macros::storage::storage) struct +/// requires specifying the type `T` that is stored in the variable, along with the initial delay used when scheduling +/// value changes: +/// +/// ```noir +/// global PAUSE_CONTRACT_INITIAL_DELAY_S: u64 = 6 * 60 * 60; // 6 hours +/// global CHANGE_AUTHORIZATION_INITIAL_DELAY_S: u64 = 24 * 60 * 60; // 24 hours +/// +/// #[storage] +/// struct Storage { +/// paused: DelayedPublicMutable, +/// user_authorization: Map, C>, +/// } +/// ``` +/// +/// Note that this initial delay can be altered during the contract's lifetime via +/// [`DelayedPublicMutable::schedule_delay_change`]. +/// +/// ## Requirements +/// +/// The type `T` stored in the `DelayedPublicMutable` must implement the `Eq` and +/// [`Packable`](crate::protocol::traits::Packable) traits. +/// +/// ## Implementation Details +/// +/// This state variable stores more information in public storage than +/// [`PublicMutable`](crate::state_vars::PublicMutable), as it needs to keep track of the current and scheduled change +/// information for both the value and the delay - see +/// [`crate::protocol::delayed_public_mutable::DelayedPublicMutableValues`]. +/// +/// It also stores a hash of this entire configuration so that private reads can be performed in a single historical +/// public storage read - see [`crate::utils::WithHash`]. +/// +/// This results in a total of `N * 2 + 2` storage slots used, where `N` is the packing length of the stored type `T`. +/// This makes it quite important to ensure `T`'s implementation of [`Packable`](crate::protocol::traits::Packable) is +/// space-efficient. pub struct DelayedPublicMutable { context: Context, storage_slot: Field, } -// DelayedPublicMutable 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. DelayedPublicMutable 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 seconds. Reads in private are only valid as long as they are included in a block with a -// timestamp 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 InitialDelay, but can be changed by -// calling `schedule_delay_change`. -// -// This implementation requires that T implements the Packable and Eq traits, and allocates `M + 1` storage slots to -// this state variable. +// We allocate `M + 1` slots because we're going to store a `WithHash>`, +// and `WithHash` increases the packing length by one. impl StateVariable for DelayedPublicMutable where DelayedPublicMutableValues: Packable, @@ -50,15 +144,47 @@ impl DelayedPublicMutable + /// t1`). The result is that the current value continues to be `A` all the way until `t2`, at which point it + /// changes to `C`. + /// + /// This also means that it is possible to **cancel** a scheduled change by calling `schedule_value_change` with + /// the current value. + /// + /// ## Examples + /// + /// A public setter that authorizes a user: + /// ```noir + /// #[external("public")] + /// fn authorize_user(user: AztecAddress) { + /// assert_eq(self.storage.admin.read(), self.msg_sender(), "caller is not admin"); + /// self.storage.user_authorization.at(user).schedule_value_change(true); + /// } + /// ``` + /// + /// ## Cost + /// + /// The `SSTORE` AVM opcode is invoked `2 * N + 2` times, where `N` is `T`'s packed length. pub fn schedule_value_change(self, new_value: T) where T: Packable, { - let _value_change = self.schedule_and_return_value_change(new_value); + let _ = self.schedule_and_get_value_change(new_value); } - pub fn schedule_and_return_value_change(self, new_value: T) -> ScheduledValueChange + /// Schedules a write to the current value, returning the scheduled entry. + pub fn schedule_and_get_value_change(self, new_value: T) -> ScheduledValueChange where T: Packable, { @@ -77,6 +203,57 @@ where value_change } + /// Schedules a write to the current delay. + /// + /// This works just like [`schedule_value_change`](DelayedPublicMutable::schedule_value_change), except instead of + /// changing the value in the state variable, it changes the delay that will govern future invocations of that + /// function. + /// + /// The current delay does not immediately change. Once the current delay passes, + /// [`get_current_delay`](DelayedPublicMutable::get_current_delay) automatically begins to return `new_delay`, and + /// [`schedule_value_change`](DelayedPublicMutable::schedule_value_change) begins using it. + /// + /// ## Multiple Scheduled Changes + /// + /// Only a **single** delay can be scheduled to become the new delay at a given point in time. Any prior scheduled + /// changes which have not yet become current are **replaced** with the new one and discarded. + /// + /// To illustrate this, consider a scenario at `t0` with a current delay `A`. A delay change to `B` is scheduled to + /// occur at `t1`. At some point _before_ `t1`, a second delay change to `C` is scheduled to occur at `t2` (`t2 > + /// t1`). The result is that the current delay continues to be `A` all the way until `t2`, at which point it + /// changes to `C`. + /// + /// ## Delays When Changing Delays + /// + /// A delay change cannot always be immediate: if it were, then it'd be possible to break `DelayedPublicMutable`'s + /// invariants by setting the delay to a very low or zero value and then scheduling a value change, resulting in a + /// new value becoming the current one earlier than was predictable based on the prior delay. This would prohibit + /// private reads, which is the reason for existence of this state variable. + /// + /// Instead, delay changes are themselves scheduled and delay so that the property mentioned above is preserved. + /// This results in delay increases and decreases being asymmetrical. + /// + /// If the delay is being decreased, then this requires a delay equal to the difference between the current and new + /// delay, so that a scheduled value change that occurred as the new delay came into effect would be scheduled for + /// the same timestamp as if no delay change had occurred. + /// + /// If the delay is being increased, then the new delay becomes effective immediately, as new value changes would + /// be scheduled for a timestamp that is further than the current delay. + /// + /// ## Examples + /// + /// A public setter that sets the pause delay: + /// ```noir + /// #[public] + /// fn set_pause_delay(delay: u64) { + /// assert_eq(self.storage.admin.read(), self.msg_sender(), "caller is not admin"); + /// self.storage.paused.schedule_delay_change(delay); + /// } + /// ``` + /// + /// ## Cost + /// + /// The `SSTORE` AVM opcode is invoked `2 * N + 2` times, where `N` is `T`'s packed length. pub fn schedule_delay_change(self, new_delay: u64) where T: Packable, @@ -87,9 +264,39 @@ where delay_change.schedule_change(new_delay, current_timestamp); + // We can't just update the `ScheduledDelayChange`, we need to update the entire storage because we need to + // also recompute and write the hash. + // We _could_ just read everything, update the hash and `ScheduledDelayChange` but not overwrite the + // `ScheduledValueChange`, resulting in fewer storage writes, but that would require careful handling of + // storage slots and `WithHash`'s internal layout, which is not worth doing at this point. self.write(self.read_value_change(), delay_change); } + /// Returns the current value. + /// + /// If [`schedule_value_change`](DelayedPublicMutable::schedule_value_change) has never been called, then this + /// returns the default empty public storage value, which is all zeroes - equivalent to `let t = + /// T::unpack(std::mem::zeroed());`. + /// + /// It is not possible to detect if a `DelayedPublicMutable` has ever been initialized or not other than by testing + /// for the zero sentinel value. For a more robust solution, store an `Option` in the `DelayedPublicMutable`. + /// + /// Use [`get_scheduled_value`](DelayedPublicMutable::get_scheduled_value) to instead get the last value that was + /// scheduled to become the current one (which will equal the current value if the delay has already passed). + /// + /// ## Examples + /// + /// A public getter that returns a user's authorization status: + /// ```noir + /// #[external("public")] + /// fn is_authorized(user: AztecAddress) -> bool { + /// self.storage.user_authorization.at(user).get_current_value() + /// } + /// ``` + /// + /// ## Cost + /// + /// The `SLOAD` AVM opcode is invoked `2 * N + 1` times, where `N` is `T`'s packed length. pub fn get_current_value(self) -> T where T: Packable, @@ -100,6 +307,30 @@ where value_change.get_current_at(current_timestamp) } + /// Returns the current delay. + /// + /// This is the delay that would be used by [`schedule_value_change`](DelayedPublicMutable::schedule_value_change) + /// if it were called in the current transaction. + /// + /// If [`schedule_delay_change`](DelayedPublicMutable::schedule_delay_change) has never been called, then this + /// returns the `InitialDelay` used in the [`storage`](crate::macros::storage::storage) struct. + /// + /// Use [`get_scheduled_delay`](DelayedPublicMutable::get_scheduled_delay) to instead get the last delay that was + /// scheduled to become the current one (which will equal the current delay if the delay has already passed). + /// + /// ## Examples + /// + /// A public getter that returns the pause delay: + /// ```noir + /// #[external("public")] + /// fn get_pause_delay() -> u64 { + /// self.storage.paused.get_current_delay() + /// } + /// ``` + /// + /// ## Cost + /// + /// The `SLOAD` AVM opcode is invoked a single time, regardless of `T`. pub fn get_current_delay(self) -> u64 where T: Packable, @@ -108,6 +339,7 @@ where self.read_delay_change().get_current(current_timestamp) } + /// Returns the last scheduled value and timestamp of change. pub fn get_scheduled_value(self) -> (T, u64) where T: Packable, @@ -115,6 +347,7 @@ where self.read_value_change().get_scheduled() } + /// Returns the last scheduled delay and timestamp of change. pub fn get_scheduled_delay(self) -> (u64, u64) where T: Packable, @@ -166,34 +399,80 @@ impl DelayedPublicMutable` in the + /// `DelayedPublicMutable`. + /// + /// ## Privacy + /// + /// This function does leak some privacy, though in a subtle way. Understanding this is key to understanding how + /// to use `DelayedPublicMutable` in a privacy-preserving way. + /// + /// Private reads are based on a historical public storage read at the anchor block (i.e. + /// [`crate::history::storage::public_storage_historical_read`]). `DelayedPublicMutable` is able to provide + /// guarantees about values read in the past remaining the state variable's current value into the future due to + /// the existence of delays when scheduling writes. It then sets the `expiration_timestamp` property of the + /// current transaction (see + /// [`PrivateContext::set_expiration_timestamp`](crate::context::PrivateContext::set_expiration_timestamp)) to + /// ensure that the transaction can only be included in a block **prior** to the state variable's value changing. + /// In other words, it knows some facts about the near future up until some time horizon, and then makes sure + /// that it doesn't act on this knowledge past said moment. + /// + /// Because the `expiration_timestamp` property is part of the transaction's public information, any mutation to + /// this value could result in transaction fingerprinting. Note that multiple contracts may set this value during + /// a transaction: it is the smallest (most restrictive) timestamp that will be used. + /// + /// If the state variable **does not** have any value changes scheduled, then the timestamp will be set to that + /// of the anchor block plus the current delay. If multiple contracts use the same delay for their + /// `DelayedPublicMutable` state variables, then these will all be in the same privacy set. /// - /// Because of this reason, if two mutable values are often privately read together it can be convenient to group - /// them in a single type `T`. Note however that this will result in them sharing the mutation delay: + /// If the state variable **does** have a value change scheduled, then the timestamp will be set to equal the + /// time at which the current value will change, i.e. the one + /// [`get_scheduled_value`](DelayedPublicMutable::get_scheduled_value) returns - which is public information. + /// This results in an unavoidable privacy leak of any transactions in which a contract privately reads a + /// `DelayedPublicMutable` that will change soon. /// + /// Transactions that do not read from a `DelayedPublicMutable` are part of a privacy set in which the + /// `expiration_timestamp` is set to their anchor block plus + /// [`crate::protocol::constants::MAX_TX_LIFETIME`], making this the most privacy-preserving delay. The less + /// frequent said value changes are, the more private the contract is. Wallets can also then choose to further + /// lower this timestamp to make it less obvious that their transactions are interacting with this + /// soon-to-change variable. + /// + /// ## Examples + /// + /// A private action that requires authorization: /// ```noir - /// // Bad: reading both `paused` and `fee` will require two historical public storage reads - /// #[storage] - /// struct Storage { - /// paused: DelayedPublicMutable, - /// fee: DelayedPublicMutable, - /// } + /// #[external("private")] + /// fn do_action() { + /// assert( + /// self.storage.user_authorization.at(self.msg_sender()).get_current_value(), + /// "caller is not authorized" + /// ); /// - /// // Good: both `paused` and `fee` are retrieved in a single historical public storage read - /// #[derive(Packable)] - /// struct Config { - /// paused: bool, - /// fee: Field, + /// // do the action /// } + /// ``` /// - /// #[storage] - /// struct Storage { - /// config: DelayedPublicMutable, + /// A private action that can be paused: + /// ```noir + /// #[external("private")] + /// fn do_action() { + /// assert(!self.storage.paused.get_current_value(), "contract is paused"); + /// + /// // do the action /// } /// ``` + /// + /// ## Cost + /// + /// This function performs a single merkle tree inclusion proof, which is in the order of 4k gates. pub fn get_current_value(self) -> T where T: Packable, diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/public_immutable.nr b/noir-projects/aztec-nr/aztec/src/state_vars/public_immutable.nr index 2db7c2f325e4..0581f9c0d721 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/public_immutable.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/public_immutable.nr @@ -28,10 +28,9 @@ mod test; /// A value stored in a `PublicImmutable` can be read and initialized from public contract functions. /// /// Unlike [`PublicMutable`](crate::state_vars::PublicMutable) it is **also** possible to read a `PublicImmutable` from -/// a -/// private contract function, though it is not possible to initialize one. A common pattern is to have these functions -/// [enqueue a public self calls](crate::contract_self::ContractSelfPrivate::enqueue) in which the initialization -/// operation is performed. +/// a private contract function, though it is not possible to initialize one. A common pattern is to have these +/// functions [enqueue a public self calls](crate::contract_self::ContractSelfPrivate::enqueue) in which the +/// initialization operation is performed. /// /// For a mutable (with restrictions) variant which also can be read from private functions see /// [`DelayedPublicMutable`](crate::state_vars::DelayedPublicMutable). @@ -54,26 +53,26 @@ mod test; /// /// `PublicImmutable`'s main limitation is the immutability, which in many cases leads to /// [`DelayedPublicMutable`](crate::state_vars::DelayedPublicMutable) being used instead. But in those cases where -/// fixed -/// values are not a problem, this is a fine choice for storage. +/// fixed values are not a problem, this is a fine choice for storage. /// /// ## Examples /// -/// Declaring a `PublicImmutable` in the the contract's [`storage`](crate::macros::storage::storage) struct requires +/// Declaring a `PublicImmutable` in the contract's [`storage`](crate::macros::storage::storage) struct requires /// specifying the type `T` that is stored in the variable: /// /// ```noir /// #[storage] -/// struct Storage { -/// decimals: PublicImmutable, +/// struct Storage { +/// decimals: PublicImmutable, /// -/// account_types: Map, Context>, +/// account_types: Map, C>, /// } /// ``` /// /// ## Requirements /// -/// The type `T` stored in the `PublicImmutable` must implement the `Packable` trait. +/// The type `T` stored in the `PublicImmutable` must implement the `Eq` and +/// [`Packable`](crate::protocol::traits::Packable) traits. /// /// ## Implementation Details /// @@ -130,7 +129,7 @@ impl PublicImmutable { /// #[external("public")] /// #[initializer] /// fn initialize(decimals: u8) { - /// self.storage.decimals.iniitalize(decimals); + /// self.storage.decimals.initialize(decimals); /// } /// ``` /// @@ -139,7 +138,7 @@ impl PublicImmutable { /// // Can only be called once per account /// #[external("public")] /// fn set_account_type(account_type: AccountType) { - /// self.storage.account_types.at(self.msg_sender()).iniitalize(account_type); + /// self.storage.account_types.at(self.msg_sender()).initialize(account_type); /// } /// ``` /// @@ -223,7 +222,7 @@ impl PublicImmutable { /// #[external("public")] /// fn set_account_type_if_not_set(account_type: AccountType) { /// if !self.storage.account_types.at(self.msg_sender()).is_initialized() { - /// self.storage.account_types.at(self.msg_sender()).iniitalize(account_type); + /// self.storage.account_types.at(self.msg_sender()).initialize(account_type); /// } /// } /// ``` @@ -300,7 +299,7 @@ impl PublicImmutable { /// #[storage] /// struct Storage { /// decimals: PublicImmutable, - /// symbol: PubicImmutable, + /// symbol: PublicImmutable, /// } /// /// // Good: both `decimals` and `symbol` are retrieved in a single historical public storage read diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/public_mutable.nr b/noir-projects/aztec-nr/aztec/src/state_vars/public_mutable.nr index c7f53d505d9d..92d61704fa49 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/public_mutable.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/public_mutable.nr @@ -35,7 +35,7 @@ use crate::state_vars::StateVariable; /// This is suitable for any kind of global state that needs to be accessible by everyone. For example, a token may /// have a public total supply, or a voting contract may have public vote tallies. /// -/// Note that contracts having public values does not necessarily mean the the actions that update these values must +/// Note that contracts having public values does not necessarily mean the actions that update these values must /// themselves be wholly public. For example, the token could allow for private minting and burning, and casting a vote /// could be kept private: these private functions would enqueue a public function that writes to the `PublicMutable`. /// @@ -50,22 +50,23 @@ use crate::state_vars::StateVariable; /// /// ## Examples /// -/// Declaring a `PublicMutable` in the the contract's [`storage`](crate::macros::storage::storage) struct requires +/// Declaring a `PublicMutable` in the contract's [`storage`](crate::macros::storage::storage) struct requires /// specifying the type `T` that is stored in the variable: /// /// ```noir /// #[storage] -/// struct Storage { -/// total_supply: PublicMutable, -/// public_balances: Map, Context>, +/// struct Storage { +/// total_supply: PublicMutable, +/// public_balances: Map, C>, /// -/// vote_tallies: Map, Context>, +/// vote_tallies: Map, C>, /// } /// ``` /// /// ## Requirements /// -/// The type `T` stored in the `PublicMutable` must implement the `Packable` trait. +/// The type `T` stored in the `PublicMutable` must implement the [`Packable`](crate::protocol::traits::Packable) +/// trait. /// /// ## Implementation Details /// @@ -162,7 +163,7 @@ impl PublicMutable { /// } /// ``` /// - /// An [`only_self`](crate::macros::functions::only_self) helper that updates public state trigered by a private + /// An [`only_self`](crate::macros::functions::only_self) helper that updates public state triggered by a private /// function: /// ```noir /// #[external("private")] @@ -175,8 +176,8 @@ impl PublicMutable { /// #[external("public")] /// #[only_self] /// fn _tally_vote(election_id: ElectionId, votes: u128) { - /// let current = self.storage.vote_tallies.read(); - /// self.storage.vote_tallies.write(current + votes); + /// let current = self.storage.vote_tallies.at(election_id).read(); + /// self.storage.vote_tallies.at(election_id).write(current + votes); /// } /// ``` /// diff --git a/noir-projects/noir-contracts/contracts/protocol/aztec_sublib/src/state_vars/delayed_public_mutable.nr b/noir-projects/noir-contracts/contracts/protocol/aztec_sublib/src/state_vars/delayed_public_mutable.nr index f12ae2e959ec..a72ccc13fefb 100644 --- a/noir-projects/noir-contracts/contracts/protocol/aztec_sublib/src/state_vars/delayed_public_mutable.nr +++ b/noir-projects/noir-contracts/contracts/protocol/aztec_sublib/src/state_vars/delayed_public_mutable.nr @@ -8,22 +8,122 @@ use crate::protocol::{ traits::Packable, }; -use crate::{ - context::{PrivateContext, PublicContext, UtilityContext}, - state_vars::StateVariable, - utils::WithHash, -}; - -/// Public mutable values with private read access. +use crate::{context::{PrivateContext, PublicContext, UtilityContext}, state_vars::StateVariable, utils::WithHash}; + +/// Mutable public values with private read access. +/// +/// This is an advanced public state variable, with no native Solidity equivalent. +/// +/// Like [`PublicMutable`](crate::state_vars::PublicMutable) it represents a public value of type `T` that can be +/// written to repeatedly, but with a key improvement: the current value can also be **read from a private contract +/// function**. +/// +/// This comes at the cost of extra restrictions on the state variable: writes do not come into effect immediately, +/// they must be **scheduled** to take place after some minimum delay. Reading from the state variable will therefore +/// return the previous value until some time passes, which is why this is a _delayed_ mutable variable. +/// +/// It is these delays that enable the capacity for reads from private contract functions, as they provide guarantees +/// regarding how long can some historical state observed at the anchor block be known to not change. +/// +/// Delays can be modified during the lifetime of the contract. +/// +/// ## Access Patterns +/// +/// The current value stored in a `DelayedPublicMutable` can be read from public contract functions, and writes can be +/// scheduled to happen in the future. +/// +/// Public contract functions can also schedule changes to the write delay, as well as inspect any already scheduled +/// value or delay changes. +/// +/// Private contract functions can read the **current** value of the state variable, but not past or scheduled values. +/// They cannot read the current delay, and they cannot schedule any kind of value change. +/// +/// ## Privacy +/// +/// The value stored in `DelayedPublicMutable` is fully public, as are all scheduled value and delay changes. +/// +/// Reads from a private contract function are almost fully private: the only observable effect is that they set the +/// transaction's `expiration_timestamp` property, possibly reducing the privacy set. See +/// [`PrivateContext::set_expiration_timestamp`](crate::context::PrivateContext::set_expiration_timestamp). +/// +/// ## Use Cases +/// +/// These are mostly an extension of [`PublicMutable`](crate::state_vars::PublicMutable)'s, given that what this state +/// variable essentially achieves is to provide private reads to it. For example, it can be used for global contract +/// configuration (such as fees, access control, etc.) that users will need to access during private interactions. +/// +/// The key consideration is whether the enforced minimum delay on writes prevents using this state variable. In some +/// scenarios this restriction is incompatible with requirements (such as a token's total supply, which must always be +/// up to date), while in others the enhanced privacy might make the tradeoff acceptable (such as when dealing with +/// contract pauses or access control revocation, where a delay of some hours could be acceptable). +/// +/// Note that, just like in [`PublicMutable`](crate::state_vars::PublicMutable), the fact that the values are public +/// does not necessarily mean the actions that update these values must themselves be wholly public. To learn more, +/// see the notes there regarding usage of [`only_self`](crate::macros::functions::only_self). +/// +/// ## Choosing Delays +/// +/// A short delay reduces the most obvious downside of `DelayedPublicMutable`, and so it is natural to wish to make it +/// be as low as possible. It is therefore important to understand the tradeoffs involved in delay selection. +/// +/// A shorter delay will result in a lower `expiration_timestamp` property of transactions that privately read the +/// state variable, reducing its privacy set. If the delay is smaller than that of any other contract, then this +/// privacy leak might be large enough to uniquely identify those transactions that interact with the contract - fully +/// defeating the purpose of `DelayedPublicMutable`. +/// +/// 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. +/// +/// 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 +/// use `DelayedPublicMutable` at all. +/// +/// ## Examples +/// +/// Declaring a `DelayedPublicMutable` in the contract's [`storage`](crate::macros::storage::storage) struct +/// requires specifying the type `T` that is stored in the variable, along with the initial delay used when scheduling +/// value changes: +/// +/// ```noir +/// global PAUSE_CONTRACT_INITIAL_DELAY_S: u64 = 6 * 60 * 60; // 6 hours +/// global CHANGE_AUTHORIZATION_INITIAL_DELAY_S: u64 = 24 * 60 * 60; // 24 hours +/// +/// #[storage] +/// struct Storage { +/// paused: DelayedPublicMutable, +/// user_authorization: Map, C>, +/// } +/// ``` +/// +/// Note that this initial delay can be altered during the contract's lifetime via +/// [`DelayedPublicMutable::schedule_delay_change`]. +/// +/// ## Requirements +/// +/// The type `T` stored in the `DelayedPublicMutable` must implement the `Eq` and +/// [`Packable`](crate::protocol::traits::Packable) traits. +/// +/// ## Implementation Details +/// +/// This state variable stores more information in public storage than +/// [`PublicMutable`](crate::state_vars::PublicMutable), as it needs to keep track of the current and scheduled change +/// information for both the value and the delay - see +/// [`crate::protocol::delayed_public_mutable::DelayedPublicMutableValues`]. +/// +/// It also stores a hash of this entire configuration so that private reads can be performed in a single historical +/// public storage read - see [`crate::utils::WithHash`]. +/// +/// This results in a total of `N * 2 + 2` storage slots used, where `N` is the packing length of the stored type `T`. +/// This makes it quite important to ensure `T`'s implementation of [`Packable`](crate::protocol::traits::Packable) is +/// space-efficient. pub struct DelayedPublicMutable { context: Context, storage_slot: Field, } -// DelayedPublicMutable stores a value of type T that is: -// - publicly known (i.e. unencrypted) -// - mutable in public -// - readable in private with no contention +// We allocate `M + 1` slots because we're going to store a `WithHash>`, +// and `WithHash` increases the packing length by one. impl StateVariable for DelayedPublicMutable where DelayedPublicMutableValues: Packable, @@ -42,15 +142,47 @@ impl DelayedPublicMutable + /// t1`). The result is that the current value continues to be `A` all the way until `t2`, at which point it + /// changes to `C`. + /// + /// This also means that it is possible to **cancel** a scheduled change by calling `schedule_value_change` with + /// the current value. + /// + /// ## Examples + /// + /// A public setter that authorizes a user: + /// ```noir + /// #[external("public")] + /// fn authorize_user(user: AztecAddress) { + /// assert_eq(self.storage.admin.read(), self.msg_sender(), "caller is not admin"); + /// self.storage.user_authorization.at(user).schedule_value_change(true); + /// } + /// ``` + /// + /// ## Cost + /// + /// The `SSTORE` AVM opcode is invoked `2 * N + 2` times, where `N` is `T`'s packed length. pub fn schedule_value_change(self, new_value: T) where T: Packable, { - let _value_change = self.schedule_and_return_value_change(new_value); + let _ = self.schedule_and_get_value_change(new_value); } - pub fn schedule_and_return_value_change(self, new_value: T) -> ScheduledValueChange + /// Schedules a write to the current value, returning the scheduled entry. + pub fn schedule_and_get_value_change(self, new_value: T) -> ScheduledValueChange where T: Packable, { @@ -62,18 +194,64 @@ where // TODO: make this configurable https://github.com/AztecProtocol/aztec-packages/issues/5501 let timestamp_of_change = current_timestamp + current_delay; - value_change.schedule_change( - new_value, - current_timestamp, - current_delay, - timestamp_of_change, - ); + value_change.schedule_change(new_value, current_timestamp, current_delay, timestamp_of_change); self.write(value_change, delay_change); value_change } + /// Schedules a write to the current delay. + /// + /// This works just like [`schedule_value_change`](DelayedPublicMutable::schedule_value_change), except instead of + /// changing the value in the state variable, it changes the delay that will govern future invocations of that + /// function. + /// + /// The current delay does not immediately change. Once the current delay passes, + /// [`get_current_delay`](DelayedPublicMutable::get_current_delay) automatically begins to return `new_delay`, and + /// [`schedule_value_change`](DelayedPublicMutable::schedule_value_change) begins using it. + /// + /// ## Multiple Scheduled Changes + /// + /// Only a **single** delay can be scheduled to become the new delay at a given point in time. Any prior scheduled + /// changes which have not yet become current are **replaced** with the new one and discarded. + /// + /// To illustrate this, consider a scenario at `t0` with a current delay `A`. A delay change to `B` is scheduled to + /// occur at `t1`. At some point _before_ `t1`, a second delay change to `C` is scheduled to occur at `t2` (`t2 > + /// t1`). The result is that the current delay continues to be `A` all the way until `t2`, at which point it + /// changes to `C`. + /// + /// ## Delays When Changing Delays + /// + /// A delay change cannot always be immediate: if it were, then it'd be possible to break `DelayedPublicMutable`'s + /// invariants by setting the delay to a very low or zero value and then scheduling a value change, resulting in a + /// new value becoming the current one earlier than was predictable based on the prior delay. This would prohibit + /// private reads, which is the reason for existence of this state variable. + /// + /// Instead, delay changes are themselves scheduled and delay so that the property mentioned above is preserved. + /// This results in delay increases and decreases being asymmetrical. + /// + /// If the delay is being decreased, then this requires a delay equal to the difference between the current and new + /// delay, so that a scheduled value change that occurred as the new delay came into effect would be scheduled for + /// the same timestamp as if no delay change had occurred. + /// + /// If the delay is being increased, then the new delay becomes effective immediately, as new value changes would + /// be scheduled for a timestamp that is further than the current delay. + /// + /// ## Examples + /// + /// A public setter that sets the pause delay: + /// ```noir + /// #[public] + /// fn set_pause_delay(delay: u64) { + /// assert_eq(self.storage.admin.read(), self.msg_sender(), "caller is not admin"); + /// self.storage.paused.schedule_delay_change(delay); + /// } + /// ``` + /// + /// ## Cost + /// + /// The `SSTORE` AVM opcode is invoked `2 * N + 2` times, where `N` is `T`'s packed length. pub fn schedule_delay_change(self, new_delay: u64) where T: Packable, @@ -84,9 +262,39 @@ where delay_change.schedule_change(new_delay, current_timestamp); + // We can't just update the `ScheduledDelayChange`, we need to update the entire storage because we need to + // also recompute and write the hash. + // We _could_ just read everything, update the hash and `ScheduledDelayChange` but not overwrite the + // `ScheduledValueChange`, resulting in fewer storage writes, but that would require careful handling of + // storage slots and `WithHash`'s internal layout, which is not worth doing at this point. self.write(self.read_value_change(), delay_change); } + /// Returns the current value. + /// + /// If [`schedule_value_change`](DelayedPublicMutable::schedule_value_change) has never been called, then this + /// returns the default empty public storage value, which is all zeroes - equivalent to `let t = + /// T::unpack(std::mem::zeroed());`. + /// + /// It is not possible to detect if a `DelayedPublicMutable` has ever been initialized or not other than by testing + /// for the zero sentinel value. For a more robust solution, store an `Option` in the `DelayedPublicMutable`. + /// + /// Use [`get_scheduled_value`](DelayedPublicMutable::get_scheduled_value) to instead get the last value that was + /// scheduled to become the current one (which will equal the current value if the delay has already passed). + /// + /// ## Examples + /// + /// A public getter that returns a user's authorization status: + /// ```noir + /// #[external("public")] + /// fn is_authorized(user: AztecAddress) -> bool { + /// self.storage.user_authorization.at(user).get_current_value() + /// } + /// ``` + /// + /// ## Cost + /// + /// The `SLOAD` AVM opcode is invoked `2 * N + 1` times, where `N` is `T`'s packed length. pub fn get_current_value(self) -> T where T: Packable, @@ -97,6 +305,30 @@ where value_change.get_current_at(current_timestamp) } + /// Returns the current delay. + /// + /// This is the delay that would be used by [`schedule_value_change`](DelayedPublicMutable::schedule_value_change) + /// if it were called in the current transaction. + /// + /// If [`schedule_delay_change`](DelayedPublicMutable::schedule_delay_change) has never been called, then this + /// returns the `InitialDelay` used in the [`storage`](crate::macros::storage::storage) struct. + /// + /// Use [`get_scheduled_delay`](DelayedPublicMutable::get_scheduled_delay) to instead get the last delay that was + /// scheduled to become the current one (which will equal the current delay if the delay has already passed). + /// + /// ## Examples + /// + /// A public getter that returns the pause delay: + /// ```noir + /// #[external("public")] + /// fn get_pause_delay() -> u64 { + /// self.storage.paused.get_current_delay() + /// } + /// ``` + /// + /// ## Cost + /// + /// The `SLOAD` AVM opcode is invoked a single time, regardless of `T`. pub fn get_current_delay(self) -> u64 where T: Packable, @@ -105,6 +337,7 @@ where self.read_delay_change().get_current(current_timestamp) } + /// Returns the last scheduled value and timestamp of change. pub fn get_scheduled_value(self) -> (T, u64) where T: Packable, @@ -112,6 +345,7 @@ where self.read_value_change().get_scheduled() } + /// Returns the last scheduled delay and timestamp of change. pub fn get_scheduled_delay(self) -> (u64, u64) where T: Packable, @@ -123,6 +357,8 @@ where where T: Packable, { + // We don't read ScheduledValueChange directly by having it implement Packable because ScheduledValueChange and + // ScheduledDelayChange are packed together (sdc and svc.timestamp_of_change are stored in the same slot). let packed = self.context.storage_read(self.storage_slot); unpack_value_change::::N>(packed) } @@ -131,18 +367,26 @@ where where T: Packable, { + // Since all ScheduledDelayChange member are packed into a single field, we can read a single storage slot here + // and skip the ones that correspond to ScheduledValueChange members. We are abusing the fact that the field + // containing the ScheduledDelayChange data is the first one in the storage layout - otherwise we'd need to + // offset the storage slot to get the position where it'd land. We don't read ScheduledDelayChange directly by + // having it implement Packable because ScheduledValueChange and ScheduledDelayChange are packed together (sdc + // and svc.timestamp_of_change are stored in the same slot). let packed = self.context.storage_read(self.storage_slot); unpack_delay_change::(packed) } - fn write( - self, - value_change: ScheduledValueChange, - delay_change: ScheduledDelayChange, - ) + fn write(self, value_change: ScheduledValueChange, delay_change: ScheduledDelayChange) where T: Packable, { + // Whenever we write to public storage, we write both the value change and delay change to storage at once. We + // do so by wrapping them in a single struct (`DelayedPublicMutableValues`). Then we wrap the resulting struct + // in `WithHash`. Wrapping in `WithHash` makes for more costly writes but it also makes private proofs much + // simpler because they only need to produce a historical proof for the hash, which results in a single + // inclusion proof (as opposed to 4 in the best case scenario in which T is a single field). Private delayed + // public mutable reads are assumed to be much more frequent than public writes, so this tradeoff makes sense. let values = WithHash::new(DelayedPublicMutableValues::new(value_change, delay_change)); self.context.storage_write(self.storage_slot, values); @@ -153,25 +397,107 @@ impl DelayedPublicMutable` in the `DelayedPublicMutable`. + /// + /// ## Privacy + /// + /// This function does leak some privacy, though in a subtle way. Understanding this is key to understanding how to + /// use `DelayedPublicMutable` in a privacy-preserving way. + /// + /// Private reads are based on a historical public storage read at the anchor block (i.e. + /// [`crate::history::storage::public_storage_historical_read`]). `DelayedPublicMutable` is able to provide + /// guarantees about values read in the past remaining the state variable's current value into the future due to + /// the existence of delays when scheduling writes. It then sets the `expiration_timestamp` property of the current + /// transaction (see + /// [`PrivateContext::set_expiration_timestamp`](crate::context::PrivateContext::set_expiration_timestamp)) to + /// ensure that the transaction can only be included in a block **prior** to the state variable's value changing. + /// In other words, it knows some facts about the near future up until some time horizon, and then makes sure that + /// it doesn't act on this knowledge past said moment. + /// + /// Because the `expiration_timestamp` property is part of the transaction's public information, any mutation to + /// this value could result in transaction fingerprinting. Note that multiple contracts may set this value during a + /// transaction: it is the smallest (most restrictive) timestamp that will be used. + /// + /// If the state variable **does not** have any value changes scheduled, then the timestamp will be set to that of + /// the anchor block plus the current delay. If multiple contracts use the same delay for their + /// `DelayedPublicMutable` state variables, then these will all be in the same privacy set. + /// + /// If the state variable **does** have a value change scheduled, then the timestamp will be set to equal the time + /// at which the current value will change, i.e. the one + /// [`get_scheduled_value`](DelayedPublicMutable::get_scheduled_value) returns - which is public information. This + /// results in an unavoidable privacy leak of any transactions in which a contract privately reads a + /// `DelayedPublicMutable` that will change soon. + /// + /// Transactions that do not read from a `DelayedPublicMutable` are part of a privacy set in which the + /// `expiration_timestamp` is set to their anchor block plus [`crate::protocol::constants::MAX_TX_LIFETIME`], + /// making this the most privacy-preserving delay. The less frequent said value changes are, the more private the + /// contract is. Wallets can also then choose to further lower this timestamp to make it less obvious that their + /// transactions are interacting with this soon-to-change variable. + /// + /// ## Examples + /// + /// A private action that requires authorization: + /// ```noir + /// #[external("private")] + /// fn do_action() { + /// assert( + /// self.storage.user_authorization.at(self.msg_sender()).get_current_value(), + /// "caller is not authorized" + /// ); + /// + /// // do the action + /// } + /// ``` + /// + /// A private action that can be paused: + /// ```noir + /// #[external("private")] + /// fn do_action() { + /// assert(!self.storage.paused.get_current_value(), "contract is paused"); + /// + /// // do the action + /// } + /// ``` + /// + /// ## Cost + /// + /// This function performs a single merkle tree inclusion proof, which is in the order of 4k gates. pub fn get_current_value(self) -> T where T: Packable, { // 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 timestamp as this proof + // will only be valid for the time 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, anchor_timestamp) = self.anchor_read_from_public_storage(); + // We use the effective minimum delay as opposed to the current delay at the anchor block's timestamp as this + // one also takes into consideration any scheduled delay changes. For example, consider a scenario in which at + // timestamp `x` the current delay was 86400 seconds (1 day). We may naively think that the earliest we could + // change the value would be at timestamp `x + 86400` by scheduling immediately after the anchor block's + // timestamp, i.e. at timestamp `x + 1`. But if there was a delay change scheduled for timestamp `y` to reduce + // the delay to 43200 seconds (12 hours), then if a value change was scheduled at timestamp `y` it would go + // into effect at timestamp `y + 43200`, 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(anchor_timestamp); let time_horizon = value_change.get_time_horizon(anchor_timestamp, effective_minimum_delay); - // We prevent this transaction from being included in any timestamp after the time horizon. + // We prevent this transaction from being included in any timestamp after the time horizon, ensuring that the + // historical public value matches the current one, since it can only change after the horizon. self.context.set_expiration_timestamp(time_horizon); value_change.get_current_at(anchor_timestamp) } - fn anchor_read_from_public_storage( - self, - ) -> (ScheduledValueChange, ScheduledDelayChange, u64) + fn anchor_read_from_public_storage(self) -> (ScheduledValueChange, ScheduledDelayChange, u64) where T: Packable, { diff --git a/noir-projects/noir-contracts/contracts/protocol/contract_instance_registry_contract/src/main.nr b/noir-projects/noir-contracts/contracts/protocol/contract_instance_registry_contract/src/main.nr index 75de07726984..eb48b949d35a 100644 --- a/noir-projects/noir-contracts/contracts/protocol/contract_instance_registry_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/protocol/contract_instance_registry_contract/src/main.nr @@ -210,7 +210,7 @@ pub contract ContractInstanceRegistry { let scheduled_value_update = storage .updated_class_ids .at(address) - .schedule_and_return_value_change(new_contract_class_id); + .schedule_and_get_value_change(new_contract_class_id); let (prev_contract_class_id, timestamp_of_change) = scheduled_value_update.get_previous(); let event = ContractInstanceUpdated { diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/lib.nr b/noir-projects/noir-protocol-circuits/crates/types/src/lib.nr index 33fb026d8ae8..577acdf1ce09 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/lib.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/lib.nr @@ -1,3 +1,5 @@ +//! Types and constants related to the Aztec protocol. + // Traits need to be first for the #[derive(...)] macro to work. pub mod traits; diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/meta/mod.nr b/noir-projects/noir-protocol-circuits/crates/types/src/meta/mod.nr index 95929e3f8389..4ca42ab7ed99 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/meta/mod.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/meta/mod.nr @@ -97,7 +97,7 @@ comptime fn get_where_trait_clause(s: TypeDefinition, trait_name: Quoted) -> Quo } } -/// Generates a `Packable` trait implementation for a given struct `s`. +/// Generates a [`Packable`](crate::traits::Packable) trait implementation for a given struct `s`. /// /// # Arguments /// * `s` - The struct type definition to generate the implementation for diff --git a/spartan/environments/staging-public.env b/spartan/environments/staging-public.env index fd3c5b52df47..fa894ca74041 100644 --- a/spartan/environments/staging-public.env +++ b/spartan/environments/staging-public.env @@ -31,6 +31,7 @@ SEQ_MAX_TX_PER_CHECKPOINT=7 # 0.1 TPS # Build checkpoint even if block is empty. SEQ_BUILD_CHECKPOINT_IF_EMPTY=true SEQ_BLOCK_DURATION_MS=6000 +SEQ_L1_PUBLISHING_TIME_ALLOWANCE_IN_SLOT=36 CREATE_ROLLUP_CONTRACTS=false P2P_TX_POOL_DELETE_TXS_AFTER_REORG=true diff --git a/spartan/scripts/deploy_network.sh b/spartan/scripts/deploy_network.sh index 9a5300e83796..2973265afd68 100755 --- a/spartan/scripts/deploy_network.sh +++ b/spartan/scripts/deploy_network.sh @@ -107,11 +107,7 @@ PROVER_FAILED_PROOF_STORE=${PROVER_FAILED_PROOF_STORE:-} SEQ_MIN_TX_PER_BLOCK=${SEQ_MIN_TX_PER_BLOCK:-1} SEQ_MAX_TX_PER_BLOCK=${SEQ_MAX_TX_PER_BLOCK:-null} SEQ_MAX_TX_PER_CHECKPOINT=${SEQ_MAX_TX_PER_CHECKPOINT:-8} -<<<<<<< HEAD -SEQ_PER_BLOCK_ALLOCATION_MULTIPLIER=${SEQ_PER_BLOCK_ALLOCATION_MULTIPLIER:-2} -======= SEQ_PER_BLOCK_ALLOCATION_MULTIPLIER=${SEQ_PER_BLOCK_ALLOCATION_MULTIPLIER:-} ->>>>>>> origin/v4 SEQ_BLOCK_DURATION_MS=${SEQ_BLOCK_DURATION_MS:-} SEQ_L1_PUBLISHING_TIME_ALLOWANCE_IN_SLOT=${SEQ_L1_PUBLISHING_TIME_ALLOWANCE_IN_SLOT:-} SEQ_BUILD_CHECKPOINT_IF_EMPTY=${SEQ_BUILD_CHECKPOINT_IF_EMPTY:-} diff --git a/yarn-project/archiver/src/modules/data_store_updater.ts b/yarn-project/archiver/src/modules/data_store_updater.ts index 6bf2975b3c3f..40875ecec4e6 100644 --- a/yarn-project/archiver/src/modules/data_store_updater.ts +++ b/yarn-project/archiver/src/modules/data_store_updater.ts @@ -446,7 +446,7 @@ export class ArchiverDataStoreUpdater { if (validFnCount > 0) { this.log.verbose(`Storing ${validFnCount} functions for contract class ${contractClassId.toString()}`); } - return await this.store.addFunctions(contractClassId, validPrivateFns, validUtilityFns); + await this.store.addFunctions(contractClassId, validPrivateFns, validUtilityFns); } return true; } diff --git a/yarn-project/aztec-node/src/aztec-node/server.ts b/yarn-project/aztec-node/src/aztec-node/server.ts index cb373bc7d042..30de30bf7966 100644 --- a/yarn-project/aztec-node/src/aztec-node/server.ts +++ b/yarn-project/aztec-node/src/aztec-node/server.ts @@ -1038,7 +1038,11 @@ export class AztecNodeService implements AztecNode, AztecNodeAdmin, Traceable { referenceBlock: BlockParameter, blockHash: BlockHash, ): Promise | undefined> { - const committedDb = await this.getWorldState(referenceBlock); + // The Noir circuit checks the archive membership proof against `anchor_block_header.last_archive.root`, + // which is the archive tree root BEFORE the anchor block was added (i.e. the state after block N-1). + // So we need the world state at block N-1, not block N, to produce a sibling path matching that root. + const referenceBlockNumber = await this.resolveBlockNumber(referenceBlock); + const committedDb = await this.getWorldState(BlockNumber(referenceBlockNumber - 1)); const [pathAndIndex] = await committedDb.findSiblingPaths(MerkleTreeId.ARCHIVE, [blockHash]); return pathAndIndex === undefined ? undefined @@ -1660,6 +1664,25 @@ export class AztecNodeService implements AztecNode, AztecNodeAdmin, Traceable { return snapshot; } + /** Resolves a block parameter to a block number. */ + protected async resolveBlockNumber(block: BlockParameter): Promise { + if (block === 'latest') { + return BlockNumber(await this.blockSource.getBlockNumber()); + } + if (BlockHash.isBlockHash(block)) { + const initialBlockHash = await this.#getInitialHeaderHash(); + if (block.equals(initialBlockHash)) { + return BlockNumber.ZERO; + } + const header = await this.blockSource.getBlockHeaderByHash(block); + if (!header) { + throw new Error(`Block hash ${block.toString()} not found.`); + } + return header.getBlockNumber(); + } + return block as BlockNumber; + } + /** * Ensure we fully sync the world state * @returns A promise that fulfils once the world state is synced diff --git a/yarn-project/stdlib/src/p2p/checkpoint_proposal.ts b/yarn-project/stdlib/src/p2p/checkpoint_proposal.ts index ae84e24204d7..aa837388211c 100644 --- a/yarn-project/stdlib/src/p2p/checkpoint_proposal.ts +++ b/yarn-project/stdlib/src/p2p/checkpoint_proposal.ts @@ -178,29 +178,32 @@ export class CheckpointProposal extends Gossipable { blockNumber: lastBlockInfo?.blockHeader?.globalVariables.blockNumber ?? BlockNumber(0), dutyType: DutyType.CHECKPOINT_PROPOSAL, }; - const checkpointSignature = await payloadSigner(checkpointHash, checkpointContext); - if (!lastBlockInfo) { - return new CheckpointProposal(checkpointHeader, archiveRoot, feeAssetPriceModifier, checkpointSignature); + if (lastBlockInfo) { + // Sign block proposal before signing checkpoint proposal to ensure HA protection + const lastBlockProposal = await BlockProposal.createProposalFromSigner( + lastBlockInfo.blockHeader, + lastBlockInfo.indexWithinCheckpoint, + checkpointHeader.inHash, + archiveRoot, + lastBlockInfo.txHashes, + lastBlockInfo.txs, + payloadSigner, + ); + + const checkpointSignature = await payloadSigner(checkpointHash, checkpointContext); + + return new CheckpointProposal(checkpointHeader, archiveRoot, feeAssetPriceModifier, checkpointSignature, { + blockHeader: lastBlockInfo.blockHeader, + indexWithinCheckpoint: lastBlockInfo.indexWithinCheckpoint, + txHashes: lastBlockInfo.txHashes, + signature: lastBlockProposal.signature, + signedTxs: lastBlockProposal.signedTxs, + }); } - const lastBlockProposal = await BlockProposal.createProposalFromSigner( - lastBlockInfo.blockHeader, - lastBlockInfo.indexWithinCheckpoint, - checkpointHeader.inHash, - archiveRoot, - lastBlockInfo.txHashes, - lastBlockInfo.txs, - payloadSigner, - ); - - return new CheckpointProposal(checkpointHeader, archiveRoot, feeAssetPriceModifier, checkpointSignature, { - blockHeader: lastBlockInfo.blockHeader, - indexWithinCheckpoint: lastBlockInfo.indexWithinCheckpoint, - txHashes: lastBlockInfo.txHashes, - signature: lastBlockProposal.signature, - signedTxs: lastBlockProposal.signedTxs, - }); + const checkpointSignature = await payloadSigner(checkpointHash, checkpointContext); + return new CheckpointProposal(checkpointHeader, archiveRoot, feeAssetPriceModifier, checkpointSignature); } /** diff --git a/yarn-project/validator-client/src/block_proposal_handler.ts b/yarn-project/validator-client/src/block_proposal_handler.ts index 43c890bdafa8..1582c74b334c 100644 --- a/yarn-project/validator-client/src/block_proposal_handler.ts +++ b/yarn-project/validator-client/src/block_proposal_handler.ts @@ -487,7 +487,9 @@ export class BlockProposalHandler { } private getReexecuteFailureReason(err: any): BlockProposalValidationFailureReason { - if (err instanceof ReExInitialStateMismatchError) { + if (err instanceof TransactionsNotAvailableError) { + return 'txs_not_available'; + } else if (err instanceof ReExInitialStateMismatchError) { return 'initial_state_mismatch'; } else if (err instanceof ReExStateMismatchError) { return 'state_mismatch'; diff --git a/yarn-project/world-state/src/synchronizer/server_world_state_synchronizer.ts b/yarn-project/world-state/src/synchronizer/server_world_state_synchronizer.ts index 511579f3d2e4..37d55fce1eb4 100644 --- a/yarn-project/world-state/src/synchronizer/server_world_state_synchronizer.ts +++ b/yarn-project/world-state/src/synchronizer/server_world_state_synchronizer.ts @@ -388,6 +388,18 @@ export class ServerWorldStateSynchronizer private async handleChainFinalized(blockNumber: BlockNumber) { this.log.verbose(`Finalized chain is now at block ${blockNumber}`); + // If the finalized block number is older than the oldest available block in world state, + // skip entirely. The finalized block number can jump backwards (e.g. when the finalization + // heuristic changes) and try to read block data that has already been pruned. When this + // happens, there is nothing useful to do — the native world state is already finalized + // past this point and pruning has already happened. + const currentSummary = await this.merkleTreeDb.getStatusSummary(); + if (blockNumber < currentSummary.oldestHistoricalBlock || blockNumber < 1) { + this.log.trace( + `Finalized block ${blockNumber} is older than the oldest available block ${currentSummary.oldestHistoricalBlock}. Skipping.`, + ); + return; + } const summary = await this.merkleTreeDb.setFinalized(blockNumber); if (this.historyToKeep === undefined) { return; @@ -421,6 +433,12 @@ export class ServerWorldStateSynchronizer } // Find the block at the start of the checkpoint and remove blocks up to this one const newHistoricBlock = historicCheckpoint.checkpoint.blocks[0]; + if (newHistoricBlock.number <= currentSummary.oldestHistoricalBlock) { + this.log.debug( + `Historic block ${newHistoricBlock.number} is not newer than oldest available ${currentSummary.oldestHistoricalBlock}. Skipping prune.`, + ); + return; + } this.log.verbose(`Pruning historic blocks to ${newHistoricBlock.number}`); const status = await this.merkleTreeDb.removeHistoricalBlocks(BlockNumber(newHistoricBlock.number)); this.log.debug(`World state summary `, status.summary); diff --git a/yarn-project/world-state/src/test/integration.test.ts b/yarn-project/world-state/src/test/integration.test.ts index 4f75871da279..fd1c096460dc 100644 --- a/yarn-project/world-state/src/test/integration.test.ts +++ b/yarn-project/world-state/src/test/integration.test.ts @@ -252,6 +252,44 @@ describe('world-state integration', () => { await awaitSync(5, 4); await expectSynchedToBlock(5, 4); }); + + it('does not throw when finalized block jumps backwards past pruned blocks', async () => { + // Create 20 blocks and sync them all + await archiver.createBlocks(MAX_CHECKPOINT_COUNT); + await synchronizer.start(); + await awaitSync(MAX_CHECKPOINT_COUNT); + await expectSynchedToBlock(MAX_CHECKPOINT_COUNT); + + // Manually finalize to block 15 and prune historical blocks up to block 10 + // to simulate world-state having pruned old data. + await db.setFinalized(BlockNumber(15)); + await db.removeHistoricalBlocks(BlockNumber(10)); + + const summary = await db.getStatusSummary(); + log.info( + `After manual finalize+prune: oldest=${summary.oldestHistoricalBlock}, finalized=${summary.finalizedBlockNumber}`, + ); + expect(summary.oldestHistoricalBlock).toBe(10); + expect(summary.finalizedBlockNumber).toBe(15); + + // Now simulate the scenario from PR #21597: finalized block jumps backwards + // to a block M that is older than oldestHistoricalBlock. + // This should NOT throw — the clamping logic should handle it. + const backwardsFinalized = BlockNumber(5); + log.info( + `Sending chain-finalized for block ${backwardsFinalized} (below oldest ${summary.oldestHistoricalBlock})`, + ); + await expect( + synchronizer.handleBlockStreamEvent({ + type: 'chain-finalized', + block: { number: backwardsFinalized, hash: '' }, + }), + ).resolves.not.toThrow(); + + // Finalized block should remain at 15 (unchanged by the backwards event) + const afterSummary = await db.getStatusSummary(); + expect(afterSummary.finalizedBlockNumber).toBe(15); + }); }); });