Skip to content

Commit

Permalink
feat!: compile-time incorrect exec environment errors (#6442)
Browse files Browse the repository at this point in the history
Closes #5886.

This removes the `Context` struct and makes all state variables generic
over a new `Context` type parameter, with each state variable providing
implementations for `PrivateContext`, `PublicContext` or `()` (used to
marked `unconstarined` - more on this later).

The end result is that we get compile-time errors when calling functions
that are unavailable in the current context, reduced wrapping and
unwrapping, and no obscure `explicit trap hit in brilling
'self._is_some'` runtime errors, such as in
AztecProtocol/aztec-packages#3123.

## How

The implementation is prety much as described in #5886, except instead
of using traits we specialize the type directly for the contexts we
know.

```rust
struct MyStateVar<Context> { context: Context }

impl MyStateVar<&mut PrivateContext> { fn private_read() { } }
```

This works because there's only a couple of them, and users are not
expected to extend them.

The macros were altered so that instead of wrapping the context object
in `Context` and then passing that, we simply pass the raw object to the
`Storage::init` function. This means that `Storage` itself is now also
generic, resulting in some unfortunate new boilerplate in the struct
declaration.

All instances of `self.context.private.unwrap()` and friends were
removed: each function is now available under the corresponding `impl`
block that is specialized for the corresponding context. We could even
rename some since `read_public` and `read_private` is no longer
required: both impls can have `read` functions since they are
effectively methods for different types, so the shared name is a
non-issue.

## Oddities

This change revelead a number of small bugs in the codebase, in the form
of uncallable functions. These were undetected since no test called
them. I'll do a pass over the entire PR and leave comments where
relevant.

## Top-level unconstrained

This PR continues the formalization of what I've been calling 'top-level
unconstrained' (i.e. an unconstrained contract function) as a
fundamental Aztec.nr concept and third execution environment, alongside
private and public execution. So far we've been accessing oracles in
these unconstrained functions without much care (sometimes for
perfomance reasons - see
AztecProtocol/aztec-packages#5911), but the new
stricter compile-time checks force us to be a bit more careful.

In my mind, we are arriving at the following model:
- public execution: done by the sequencer, can be simulated locally with
old data, not unlike the evm
- private execution: able to produce valid private kernel proofs with
side effects collected in the context
- top-level unconstrained execution: local computation using both
private and old public data, with certain restrictions from private exec
lifted (e.g. unbounded loops), unable to produce any kind of proofs or
reason about state changes. only useful for computing values doing
arbitrary computation over both private and public state, with zero
validation and guarantees of correctness

Private execution requires a context object a it needs to collect side
effects, but public notably does not - it simply calls oracles and gets
them to do things. In this sense, the `PublicContext` type is acting as
a marker of the current execution environment in order to prevent
developers from accidentally doing things that are invalid in public,
which would otherwise result in either transpilation error or inability
to create public kernel proofs.

This means that we may want a third `UnconstrainedContext` to act as a
similar marker for this third type (where we can e.g. call `view_notes`,
read old public state, etc.). It currently doesn't exist: we simply have
`Context::none()`, and it is defined as the absense of one of the other
contexts. Because of this, I chose to temporarily use the unit type
(`()`) to mark this environment.

Note that in some cases the different execution environments share code
paths: `view_notes` is simply `get_notes` without any constraints, and
public storage reads are performed by calling the same oracles in both
public and unconstrained. I imagine small differences will arise in the
future, specially as work on the AVM continues.

---------

Co-authored-by: esau <152162806+sklppy88@users.noreply.github.com>
Co-authored-by: thunkar <gregojquiros@gmail.com>
  • Loading branch information
3 people authored and AztecBot committed May 22, 2024
1 parent 0c8bd45 commit 423690b
Show file tree
Hide file tree
Showing 15 changed files with 300 additions and 269 deletions.
67 changes: 22 additions & 45 deletions authwit/src/account.nr
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
use dep::aztec::context::{PrivateContext, PublicContext, Context};
use dep::aztec::context::{PrivateContext, PublicContext};
use dep::aztec::state_vars::{Map, PublicMutable};
use dep::aztec::protocol_types::{address::AztecAddress, abis::function_selector::FunctionSelector, hash::pedersen_hash};

use crate::entrypoint::{app::AppPayload, fee::FeePayload};
use crate::auth::{IS_VALID_SELECTOR, compute_outer_authwit_hash};

struct AccountActions {
struct AccountActions<Context> {
context: Context,
is_valid_impl: fn(&mut PrivateContext, Field) -> bool,
approved_action: Map<Field, PublicMutable<bool>>,
approved_action: Map<Field, PublicMutable<bool, Context>, Context>,
}

impl AccountActions {
impl<Context> AccountActions<Context> {
pub fn init(
context: Context,
approved_action_storage_slot: Field,
Expand All @@ -29,81 +29,58 @@ impl AccountActions {
)
}
}
}

pub fn private(
context: &mut PrivateContext,
approved_action_storage_slot: Field,
is_valid_impl: fn(&mut PrivateContext, Field) -> bool
) -> Self {
AccountActions::init(
Context::private(context),
approved_action_storage_slot,
is_valid_impl
)
}

pub fn public(
context: &mut PublicContext,
approved_action_storage_slot: Field,
is_valid_impl: fn(&mut PrivateContext, Field) -> bool
) -> Self {
AccountActions::init(
Context::public(context),
approved_action_storage_slot,
is_valid_impl
)
}

impl AccountActions<&mut PrivateContext> {
// docs:start:entrypoint
pub fn entrypoint(self, app_payload: AppPayload, fee_payload: FeePayload) {
let valid_fn = self.is_valid_impl;
let mut private_context = self.context.private.unwrap();

let fee_hash = fee_payload.hash();
assert(valid_fn(private_context, fee_hash));
fee_payload.execute_calls(private_context);
private_context.end_setup();
assert(valid_fn(self.context, fee_hash));
fee_payload.execute_calls(self.context);
self.context.end_setup();

let app_hash = app_payload.hash();
assert(valid_fn(private_context, app_hash));
app_payload.execute_calls(private_context);
assert(valid_fn(self.context, app_hash));
app_payload.execute_calls(self.context);
}
// docs:end:entrypoint

// docs:start:spend_private_authwit
pub fn spend_private_authwit(self, inner_hash: Field) -> Field {
let context = self.context.private.unwrap();
// The `inner_hash` is "siloed" with the `msg_sender` to ensure that only it can
// consume the message.
// This ensures that contracts cannot consume messages that are not intended for them.
let message_hash = compute_outer_authwit_hash(
context.msg_sender(),
context.chain_id(),
context.version(),
self.context.msg_sender(),
self.context.chain_id(),
self.context.version(),
inner_hash
);
let valid_fn = self.is_valid_impl;
assert(valid_fn(context, message_hash) == true, "Message not authorized by account");
context.push_new_nullifier(message_hash, 0);
assert(valid_fn(self.context, message_hash) == true, "Message not authorized by account");
self.context.push_new_nullifier(message_hash, 0);
IS_VALID_SELECTOR
}
// docs:end:spend_private_authwit
}

impl AccountActions<&mut PublicContext> {
// docs:start:spend_public_authwit
pub fn spend_public_authwit(self, inner_hash: Field) -> Field {
let context = self.context.public.unwrap();
// The `inner_hash` is "siloed" with the `msg_sender` to ensure that only it can
// consume the message.
// This ensures that contracts cannot consume messages that are not intended for them.
let message_hash = compute_outer_authwit_hash(
context.msg_sender(),
context.chain_id(),
context.version(),
self.context.msg_sender(),
self.context.chain_id(),
self.context.version(),
inner_hash
);
let is_valid = self.approved_action.at(message_hash).read();
assert(is_valid == true, "Message not authorized by account");
context.push_new_nullifier(message_hash, 0);
self.context.push_new_nullifier(message_hash, 0);
IS_VALID_SELECTOR
}
// docs:end:spend_public_authwit
Expand Down
5 changes: 1 addition & 4 deletions authwit/src/auth.nr
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,7 @@ use dep::aztec::protocol_types::{
};
use dep::aztec::{
prelude::Deserialize,
context::{
PrivateContext, PublicContext, Context, gas::GasOpts,
interface::{ContextInterface, PublicContextInterface}
},
context::{PrivateContext, PublicContext, gas::GasOpts, interface::{ContextInterface, PublicContextInterface}},
hash::hash_args_array
};

Expand Down
24 changes: 0 additions & 24 deletions aztec/src/context.nr
Original file line number Diff line number Diff line change
Expand Up @@ -20,27 +20,3 @@ use private_context::PackedReturns;
use public_context::PublicContext;
use public_context::FunctionReturns;
use avm_context::AvmContext;

struct Context {
private: Option<&mut PrivateContext>,
public: Option<&mut PublicContext>,
avm: Option<&mut AvmContext>,
}

impl Context {
pub fn private(context: &mut PrivateContext) -> Context {
Context { private: Option::some(context), public: Option::none(), avm: Option::none() }
}

pub fn public(context: &mut PublicContext) -> Context {
Context { public: Option::some(context), private: Option::none(), avm: Option::none() }
}

pub fn avm(context: &mut AvmContext) -> Context {
Context { avm: Option::some(context), public: Option::none(), private: Option::none() }
}

pub fn none() -> Context {
Context { public: Option::none(), private: Option::none(), avm: Option::none() }
}
}
7 changes: 3 additions & 4 deletions aztec/src/state_vars/map.nr
Original file line number Diff line number Diff line change
@@ -1,18 +1,17 @@
use crate::context::{PrivateContext, PublicContext, Context};
use dep::protocol_types::{hash::pedersen_hash, traits::ToField};
use crate::state_vars::storage::Storage;

// docs:start:map
struct Map<K, V> {
struct Map<K, V, Context> {
context: Context,
storage_slot: Field,
state_var_constructor: fn(Context, Field) -> V,
}
// docs:end:map

impl<K, T> Storage<T> for Map<K, T> {}
impl<K, T, Context> Storage<T> for Map<K, T, Context> {}

impl<K, V> Map<K, V> {
impl<K, V, Context> Map<K, V, Context> {
// docs:start:new
pub fn new(
context: Context,
Expand Down
40 changes: 21 additions & 19 deletions aztec/src/state_vars/private_immutable.nr
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use dep::protocol_types::{
constants::GENERATOR_INDEX__INITIALIZATION_NULLIFIER, hash::pedersen_hash
};

use crate::context::{PrivateContext, Context};
use crate::context::PrivateContext;
use crate::note::{
lifecycle::create_note, note_getter::{get_note, view_notes}, note_interface::NoteInterface,
note_viewer_options::NoteViewerOptions
Expand All @@ -12,19 +12,19 @@ use crate::oracle::notes::check_nullifier_exists;
use crate::state_vars::storage::Storage;

// docs:start:struct
struct PrivateImmutable<Note> {
context: Option<&mut PrivateContext>,
struct PrivateImmutable<Note, Context> {
context: Context,
storage_slot: Field,
}
// docs:end:struct

impl<T> Storage<T> for PrivateImmutable<T> {}
impl<T, Context> Storage<T> for PrivateImmutable<T, Context> {}

impl<Note> PrivateImmutable<Note> {
impl<Note, Context> PrivateImmutable<Note, Context> {
// docs:start:new
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: context.private, storage_slot }
Self { context, storage_slot }
}
// docs:end:new

Expand All @@ -40,39 +40,41 @@ impl<Note> PrivateImmutable<Note> {
GENERATOR_INDEX__INITIALIZATION_NULLIFIER
)
}
}

// docs:start:is_initialized
unconstrained pub fn is_initialized(self) -> bool {
let nullifier = self.compute_initialization_nullifier();
check_nullifier_exists(nullifier)
}
// docs:end:is_initialized

impl<Note> PrivateImmutable<Note, &mut PrivateContext> {
// docs:start:initialize
pub fn initialize<N>(
self,
note: &mut Note,
broadcast: bool,
ivpk_m: GrumpkinPoint
) where Note: NoteInterface<N> {
let context = self.context.unwrap();

// Nullify the storage slot.
let nullifier = self.compute_initialization_nullifier();
context.push_new_nullifier(nullifier, 0);
self.context.push_new_nullifier(nullifier, 0);

create_note(context, self.storage_slot, note, broadcast, ivpk_m);
create_note(self.context, self.storage_slot, note, broadcast, ivpk_m);
}
// docs:end:initialize

// docs:start:get_note
pub fn get_note<N>(self) -> Note where Note: NoteInterface<N> {
let context = self.context.unwrap();
let storage_slot = self.storage_slot;
get_note(context, storage_slot)
get_note(self.context, storage_slot)
}
// docs:end:get_note
}

impl<Note> PrivateImmutable<Note, ()> {
// docs:start:is_initialized
unconstrained pub fn is_initialized(self) -> bool {
let nullifier = self.compute_initialization_nullifier();
check_nullifier_exists(nullifier)
}
// docs:end:is_initialized

// view_note does not actually use the context, but it calls oracles that are only available in private
// docs:start:view_note
unconstrained pub fn view_note<N>(self) -> Note where Note: NoteInterface<N> {
let mut options = NoteViewerOptions::new();
Expand Down
52 changes: 27 additions & 25 deletions aztec/src/state_vars/private_mutable.nr
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use dep::protocol_types::{
grumpkin_point::GrumpkinPoint, hash::pedersen_hash
};

use crate::context::{PrivateContext, PublicContext, Context};
use crate::context::PrivateContext;
use crate::note::{
lifecycle::{create_note, destroy_note}, note_getter::{get_note, view_notes},
note_interface::NoteInterface, note_viewer_options::NoteViewerOptions
Expand All @@ -12,22 +12,26 @@ use crate::oracle::notes::check_nullifier_exists;
use crate::state_vars::storage::Storage;

// docs:start:struct
struct PrivateMutable<Note> {
context: Option<&mut PrivateContext>,
struct PrivateMutable<Note, Context> {
context: Context,
storage_slot: Field
}
// docs:end:struct

impl<T> Storage<T> for PrivateMutable<T> {}
impl<T, Context> Storage<T> for PrivateMutable<T, Context> {}

impl<Note> PrivateMutable<Note> {
impl<Note, Context> PrivateMutable<Note, Context> {
// docs:start:new
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: context.private, storage_slot }
Self { context, storage_slot }
}
// docs:end:new

pub fn to_unconstrained(self) -> PrivateMutable<Note, ()> {
PrivateMutable { context: (), storage_slot: self.storage_slot }
}

// The following computation is leaky, in that it doesn't hide the storage slot that has been initialized, nor does it hide the contract address of this contract.
// When this initialization nullifier is emitted, an observer could do a dictionary or rainbow attack to learn the preimage of this nullifier to deduce the storage slot and contract address.
// For some applications, leaking the details that a particular state variable of a particular contract has been initialized will be unacceptable.
Expand All @@ -42,28 +46,21 @@ impl<Note> PrivateMutable<Note> {
GENERATOR_INDEX__INITIALIZATION_NULLIFIER
)
}
}

// docs:start:is_initialized
unconstrained pub fn is_initialized(self) -> bool {
let nullifier = self.compute_initialization_nullifier();
check_nullifier_exists(nullifier)
}
// docs:end:is_initialized

impl<Note> PrivateMutable<Note, &mut PrivateContext> {
// docs:start:initialize
pub fn initialize<N>(
self,
note: &mut Note,
broadcast: bool,
ivpk_m: GrumpkinPoint
) where Note: NoteInterface<N> {
let context = self.context.unwrap();

// Nullify the storage slot.
let nullifier = self.compute_initialization_nullifier();
context.push_new_nullifier(nullifier, 0);
self.context.push_new_nullifier(nullifier, 0);

create_note(context, self.storage_slot, note, broadcast, ivpk_m);
create_note(self.context, self.storage_slot, note, broadcast, ivpk_m);
}
// docs:end:initialize

Expand All @@ -74,14 +71,13 @@ impl<Note> PrivateMutable<Note> {
broadcast: bool,
ivpk_m: GrumpkinPoint
) where Note: NoteInterface<N> {
let context = self.context.unwrap();
let prev_note: Note = get_note(context, self.storage_slot);
let prev_note: Note = get_note(self.context, self.storage_slot);

// Nullify previous note.
destroy_note(context, prev_note);
destroy_note(self.context, prev_note);

// Add replacement note.
create_note(context, self.storage_slot, new_note, broadcast, ivpk_m);
create_note(self.context, self.storage_slot, new_note, broadcast, ivpk_m);
}
// docs:end:replace

Expand All @@ -91,19 +87,25 @@ impl<Note> PrivateMutable<Note> {
broadcast: bool,
ivpk_m: GrumpkinPoint
) -> Note where Note: NoteInterface<N> {
let context = self.context.unwrap();
let mut note = get_note(context, self.storage_slot);
let mut note = get_note(self.context, self.storage_slot);

// Nullify current note to make sure it's reading the latest note.
destroy_note(context, note);
destroy_note(self.context, note);

// Add the same note again.
// Because a nonce is added to every note in the kernel, its nullifier will be different.
create_note(context, self.storage_slot, &mut note, broadcast, ivpk_m);
create_note(self.context, self.storage_slot, &mut note, broadcast, ivpk_m);

note
}
// docs:end:get_note
}

impl<Note> PrivateMutable<Note, ()> {
unconstrained pub fn is_initialized(self) -> bool {
let nullifier = self.compute_initialization_nullifier();
check_nullifier_exists(nullifier)
}

// docs:start:view_note
unconstrained pub fn view_note<N>(self) -> Note where Note: NoteInterface<N> {
Expand Down
Loading

0 comments on commit 423690b

Please sign in to comment.