Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat!: compile-time incorrect exec environment errors #6442

Merged
merged 22 commits into from
May 21, 2024
Merged
Show file tree
Hide file tree
Changes from 6 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
6 changes: 4 additions & 2 deletions docs/docs/aztec/aztec/glossary/call_types.md
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,11 @@ This is the same function that was called by privately enqueuing a call to it! P

### Top-level Unconstrained

Contract functions with the `unconstrained` Noir keyword are a special type of function still under development, and their semantics will likely change in the near future. They are used to perform state queries from an off-chain client, and are never included in any transaction. No guarantees are made on the correctness of the result since they rely exclusively on unconstrained oracle calls.
Contract functions with the `unconstrained` Noir keyword are a special type of function still under development, and their semantics will likely change in the near future. They are used to perform state queries from an off-chain client (from both private and public state!), and are never included in any transaction. No guarantees are made on the correctness of the result since the entire execution is unconstrained and heavily reliant on oracle calls.

A reasonable mental model for them is that of a `view` Solidity function that is never called in any transaction, and is only ever invoked via `eth_call`. Note that in these the caller assumes that the node is acting honestly by exectuing the true contract bytecode with correct blockchain state, the same way the Aztec version assumes the oracles are returning legitimate data.
Any programming language could be used to construct these queries, since all they do is perform arbitrary computation on data that is either publicly available from any node, or locally available from PXE. Top-level unconstrained functions exist because they let developers the rest of the contract code directly by being part of the same Noir contract, as opposed to having to rely on e.g. manual computation of storage slots, struct layout and padding, etc.
nventuro marked this conversation as resolved.
Show resolved Hide resolved

A reasonable mental model for them is that of a `view` Solidity function that can never called in any transaction, and is only ever invoked via `eth_call`. Note that in these the caller assumes that the node is acting honestly by exectuing the true contract bytecode with correct blockchain state, the same way the Aztec version assumes the oracles are returning legitimate data.
nventuro marked this conversation as resolved.
Show resolved Hide resolved

### aztec.js

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ You can't read public storage in private domain. But nevertheless reading public
1. You pass the data as a parameter to your private method and later assert in public that the data is correct. E.g.:

```rust
struct Storage {
token: PublicMutable<Field>,
struct Storage<Context> {
token: PublicMutable<Field, Context>,
}

contract Bridge {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ To learn more about how storage works in Aztec, read [the concepts](/guides/guid

```rust
#[aztec(storage)]
struct Storage {
struct Storage<Context> {
// public state variables
// private state variables
}
Expand Down
56 changes: 56 additions & 0 deletions docs/docs/migration_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,62 @@ The function `debug_log_array_with_prefix` has been removed. Use `debug_log_form

## 0.39.0

### [Aztec.nr] State variable rework

Aztec.nr state variables have been reworked so that calling private functions in public and vice versa is detected as an error during compilation instead of at runtime. This affects users in a number of ways:

#### New compile time errors

It used to be that calling a state variable method only available in public from a private function resulted in obscure runtime errors in the form of a failed `_is_some` assertion.

Incorrect usage of the state variable methods now results in compile time errors. For example, given the following function:

```rust
#[aztec(public)]
fn get_decimals() -> pub u8 {
storage.decimals.read_private()
}
```

The compiler will now error out with
```
Expected type SharedImmutable<_, &mut PrivateContext>, found type SharedImmutable<u8, &mut PublicContext>
```

The key component is the second generic parameter: the compiler expects a `PrivateContext` (becuse `read_private` is only available during private execution), but a `PublicContext` is being used instead (because of the `#[aztec(public)]` attribute).

#### Generic parameters in `Storage`

The `Storage` struct (the one marked with `#[aztec(storage)]`) should now be generic over a `Context` type, which matches the new generic parameter of all Aztec.nr libraries. This parameter is always the last generic parameter.

This means that there's now slightly more boilerplate when declaring this struct:

```diff
#[aztec(storage)]
- struct Storage {
+ struct Storage<Context> {
- nonce_for_burn_approval: PublicMutable<Field>,
+ nonce_for_burn_approval: PublicMutable<Field, Context>,
- portal_address: SharedImmutable<EthAddress>,
+ portal_address: SharedImmutable<EthAddress, Context>,
- approved_action: Map<Field, PublicMutable<bool>>,
+ approved_action: Map<Field, PublicMutable<bool, Context>, Context>,
}
```

Note that `Map` also has a generic `Context` parameter, so for each `Map` an extra `Context` must be added:

```diff
- game_decks: Map<Field, Map<AztecAddress, Deck>>,
+ game_decks: Map<Field, Map<AztecAddress, Deck<Context>, Context>, Context>,
```

This is an unfortunate side-effect, and we're looking into ways to improve this.

#### Removal of `Context`

The `Context` type no longer exists. End users typically didn't use it, but if imported it needs to be deleted.

### [Aztec.nr] Mutable delays in `SharedMutable`

The type signature for `SharedMutable` changed from `SharedMutable<T, DELAY>` to `SharedMutable<T, INITIAL_DELAY>`. The behavior is the same as before, except the delay can now be changed after deployment by calling `schedule_delay_change`.
Expand Down
12 changes: 6 additions & 6 deletions docs/docs/reference/reference/sandbox_reference/cheat_codes.md
Original file line number Diff line number Diff line change
Expand Up @@ -463,8 +463,8 @@ The baseSlot is specified in the Aztec.nr contract.

```rust
#[aztec(storage)]
struct Storage {
balances: Map<AztecAddress, PublicMutable<Field>>,
struct Storage<Context> {
balances: Map<AztecAddress, PublicMutable<Field, Context>, Context>,
}

contract Token {
Expand Down Expand Up @@ -494,8 +494,8 @@ Note: One Field element occupies a storage slot. Hence, structs with multiple fi

```rust
#[aztec(storage)]
struct Storage {
balances: Map<AztecAddress, PublicMutable<Field>>,
struct Storage<Context> {
balances: Map<AztecAddress, PublicMutable<Field, Context>, Context>,
}

contract Token {
Expand Down Expand Up @@ -526,9 +526,9 @@ Note: One Field element occupies a storage slot. Hence, structs with multiple fi
#### Example
```rust
#[aztec(storage)]
struct Storage {
struct Storage<Context> {
...
pending_shields: Set<TransparentNote, TRANSPARENT_NOTE_LEN>,
pending_shields: PrivateSet<TransparentNote, TRANSPARENT_NOTE_LEN, Context>,
}

contract Token {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ To learn more about storage slots, read [this explainer](/guides/guides/smart_co

You control this storage in Aztec using a struct annotated with `#[aztec(storage)]`. This struct serves as the housing unit for all your smart contract's state variables - the data it needs to keep track of and maintain.

These state variables come in two forms: public and private. Public variables are visible to anyone, and private variables remain hidden within the contract.
These state variables come in two forms: [public](./public_state.md) and [private](./private_state.md). Public variables are visible to anyone, and private variables remain hidden within the contract. A state variable with both public and private components is said to be [shared](./shared_state.md).

Aztec.nr has a few abstractions to help define the type of data your contract holds. These include PrivateMutable, PrivateImmutable, PublicMutable, PrivateSet, and SharedImmutable.

Expand All @@ -22,22 +22,32 @@ On this and the following pages in this section, you’ll learn:
- Practical implications of Storage in real smart contracts
In an Aztec.nr contract, storage is to be defined as a single struct, that contains both public and private state variables.

## Public and private state variables
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section didn't really seem to add anything here: the concepts are barely mentioned, whereas we have both high and low level explanations in other places. I think we can just remove this part and rely on the other sections to cover this in detail. I added links to help users find those.

## The `Context` parameter

Public state variables can be read by anyone, while private state variables can only be read by their owner (or people whom the owner has shared the decrypted data or note viewing key with).
Aztec contracts have three different modes of execution: [private](../../../../aztec/aztec/glossary/call_types.md#private-execution), [public](../../../../aztec/aztec/glossary/call_types.md#public-execution) and [top-level unconstrained](../../../../aztec/aztec/glossary/call_types.md#top-level-unconstrained). How storage is accessed depends on the execution mode: for example, `PublicImmutable` can be read in all execution modes but only initialized in public, while `PrivateMutable` is entirely unavailable in public.

Public state follows the Ethereum style account model, where each contract has its own key-value datastore. Private state follows a UTXO model, where note contents (/aztec/aztec/concepts/state_model/index.md) and [private/public execution](/aztec/aztec/concepts/smart_contracts/communication/public_private_calls.md)) for more background.
Aztec.nr prevents developers from calling functions unavailable in the current execution mode via the `context` variable that is injected into all contract functions. Its type indicates the current execution mode:
- `&mut PrivateContext` for private execution
- `&mut PublicContext` for public execution
- `()` for unconstrained

## Storage struct
All state variables are generic over this `Context` type, and expose different methods in each execution mode. In the example above, `PublicImmutable`'s `initialize` function is only available with a public execution context, and so the following code results in a compilation error:

```rust
#[aztec(storage)]
struct Storage {
// public state variables
// private state variables
struct Storage<Context> {
variable: PublicImmutable<Field, Context>,
}

#[aztec(private)]
fn some_private_function() {
storage.variable.initialize(0);
// ^ ERROR: Expected type PublicImmutable<_, &mut PublicContext>, found type PublicImmutable<Field, &mut PrivateContext>
}
```

This does mean however that the storage struct must be generic over a `Context` type parameter, which must in turn be passed to all state variables (`PublicImmutable`, `Map`, etc.).

## Map

A `map` is a state variable that "maps" a key to a value. It can be used with private or public storage variables.
Expand Down
69 changes: 23 additions & 46 deletions noir-projects/aztec-nr/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
)
}
Comment on lines -33 to -55
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was entirely unnecessary - all these do is call the init function with the correct type. I made init generic and replaced all of the private and public calls with init.


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 All @@ -113,4 +90,4 @@ impl AccountActions {
self.approved_action.at(message_hash).write(true);
}
// docs:end:approve_public_authwit
}
}
2 changes: 1 addition & 1 deletion noir-projects/aztec-nr/authwit/src/auth.nr
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use dep::aztec::protocol_types::{
use dep::aztec::{
prelude::Deserialize,
context::{
PrivateContext, PublicContext, Context, gas::GasOpts,
PrivateContext, PublicContext, gas::GasOpts,
interface::{ContextInterface, PublicContextInterface}
},
hash::hash_args_array
Expand Down
24 changes: 0 additions & 24 deletions noir-projects/aztec-nr/aztec/src/context.nr
Original file line number Diff line number Diff line change
Expand Up @@ -16,27 +16,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 noir-projects/aztec-nr/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
Loading