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

refactor: storing &mut context in state vars #1926

Merged
merged 26 commits into from
Sep 4, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion yarn-project/aztec.js/src/abis/ecdsa_account_contract.json

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ mod storage;
contract Child {
use crate::storage::Storage;
use dep::aztec::oracle::logs::emit_unencrypted_log;
use dep::std::option::Option;

#[aztec(private)]
fn constructor() {}
Expand Down Expand Up @@ -37,7 +38,7 @@ contract Child {
// Sets `current_value` to `new_value`
#[aztec(public)]
fn pubSetValue(new_value: Field) -> Field {
let storage = Storage::init();
let storage = Storage::init(Option::none(), Option::some(&mut context));
storage.current_value.write(new_value);
let _hash = emit_unencrypted_log(new_value);

Expand All @@ -47,7 +48,7 @@ contract Child {
// Increments `current_value` by `new_value`
#[aztec(public)]
fn pubIncValue(new_value: Field) -> Field {
let storage = Storage::init();
let storage = Storage::init(Option::none(), Option::some(&mut context));
let old_value = storage.current_value.read();
storage.current_value.write(old_value + new_value);
let _hash = emit_unencrypted_log(new_value);
Expand All @@ -58,7 +59,7 @@ contract Child {
// Increments `current_value` by `new_value`. Can only be called from this contract.
#[aztec(public)]
fn pubIncValueInternal(new_value: Field) -> Field {
let storage = Storage::init();
let storage = Storage::init(Option::none(), Option::some(&mut context));
assert(inputs.call_context.msg_sender == inputs.call_context.storage_contract_address);
let old_value = storage.current_value.read();
storage.current_value.write(old_value + new_value);
Expand All @@ -72,14 +73,14 @@ contract Child {
let pubSetValueSelector = 0x5b0f91b0;
let _ret = context.call_public_function(context.this_address(), pubSetValueSelector, [10]);

let storage = Storage::init();
let storage = Storage::init(Option::none(), Option::some(&mut context));
storage.current_value.write(20);
let _hash = emit_unencrypted_log(20);
}

#[aztec(public)]
fn setValueTwiceWithNestedLast() {
let storage = Storage::init();
let storage = Storage::init(Option::none(), Option::some(&mut context));
storage.current_value.write(20);
let _hash = emit_unencrypted_log(20);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,25 @@
use dep::aztec::context::{PrivateContext, PublicContext};
use dep::aztec::state_vars::public_state::PublicState;
use dep::aztec::types::type_serialisation::field_serialisation::FieldSerialisationMethods;
use dep::aztec::types::type_serialisation::field_serialisation::FIELD_SERIALISED_LEN;
use dep::std::option::Option;

struct Storage {
current_value: PublicState<Field, FIELD_SERIALISED_LEN>,
}

impl Storage {
fn init() -> Self {
fn init(
private_context: Option<&mut PrivateContext>,
public_context: Option<&mut PublicContext>,
) -> Self {
Storage {
current_value: PublicState::new(1, FieldSerialisationMethods),
current_value: PublicState::new(
private_context,
public_context,
1,
FieldSerialisationMethods,
),
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,5 @@ impl AccountContractInterface {
AccountContractInterface { address }
}

fn send_rewards(_self: Self, _rewards: u8) {
}
}
fn send_rewards(_self: Self, _rewards: u8) {}
}
Original file line number Diff line number Diff line change
@@ -1,22 +1,13 @@
use dep::std::option::Option;
use dep::aztec::constants_gen::{MAX_READ_REQUESTS_PER_CALL, MAX_NOTES_PER_PAGE};
use dep::aztec::context::{
PrivateContext,
};
use dep::aztec::constants_gen::{MAX_NOTES_PER_PAGE, MAX_READ_REQUESTS_PER_CALL};
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did you run a code formatter or something? If so, how? We should get it into the CI so we ensure all our code gets formatted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used rustfmt but it was a semi-manual process because rustfmt fails on unconstrained methods and global vars. How I made it work is that I first renamed unconstrained get_balance to get_balance_unconstrained and removed the global vars and then reverted the changes. The formatting became too ugly so it was worth the hassle but this makes it impossible to use in CI without bigger modifications of rustfmt.

But this actually makes me curious whether to create noirfmt it would be sufficient to create some simple pre and post processors which would make and revert the changes I did manually... 🤔

Copy link
Collaborator

Choose a reason for hiding this comment

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

FWIW I found a tracking issue in Noir, which doesn't seem prioritised atm: noir-lang/noir#1580

As for using rustfmt, it's an interesting idea, but I'm worried that it'll keep diverging as we add more non-rust features to Noir. Also, removing noir-specific keywords is easy, but adding them back is not, given the file changes introduced by rustfmt. I think we should probably wait.

use dep::aztec::note::{
note_getter_options::NoteGetterOptions,
note_viewer_options::NoteViewerOptions,
note_getter_options::NoteGetterOptions, note_viewer_options::NoteViewerOptions,
};
use dep::aztec::state_vars::{
immutable_singleton::ImmutableSingleton,
map::Map,
public_state::PublicState,
set::Set,
immutable_singleton::ImmutableSingleton, map::Map, public_state::PublicState, set::Set,
singleton::Singleton,
};
use dep::aztec::types::type_serialisation::bool_serialisation::{
BOOL_SERIALISED_LEN,
};
use dep::aztec::types::type_serialisation::bool_serialisation::BOOL_SERIALISED_LEN;
use dep::std::option::Option;

use crate::types::{
card_note::{CardNote, CARD_NOTE_LEN},
Expand Down Expand Up @@ -50,122 +41,97 @@ fn get_current_queen(state_var: PublicState<Queen, QUEEN_SERIALISED_LEN>) -> Que
fn can_replace_queen(
state_var: PublicState<Queen, QUEEN_SERIALISED_LEN>,
new_queen: Queen,
) -> bool {
) -> bool {
let current_queen = get_current_queen(state_var);
new_queen.points > current_queen.points
}

// docs:start:state_vars-PublicStateWriteCustom
fn replace_queen(
state_var: PublicState<Queen, QUEEN_SERIALISED_LEN>,
new_queen: Queen,
) {
fn replace_queen(state_var: PublicState<Queen, QUEEN_SERIALISED_LEN>, new_queen: Queen) {
state_var.write(new_queen);
}
// docs:end:state_vars-PublicStateWriteCustom

// docs:start:state_vars-PublicStateReadWriteCustom
fn add_points_to_queen(
state_var: PublicState<Queen, QUEEN_SERIALISED_LEN>,
new_points: u8,
) {
fn add_points_to_queen(state_var: PublicState<Queen, QUEEN_SERIALISED_LEN>, new_points: u8) {
let mut queen = state_var.read();
queen.points += new_points;
state_var.write(queen);
}
// docs:end:state_vars-PublicStateReadWriteCustom

// docs:start:state_vars-SingletonInit
fn init_legendary_card(
context: &mut PrivateContext,
state_var: Singleton<CardNote, CARD_NOTE_LEN>,
card: &mut CardNote,
) {
state_var.initialise(context, card);
fn init_legendary_card(state_var: Singleton<CardNote, CARD_NOTE_LEN>, card: &mut CardNote) {
state_var.initialise(card);
}
// docs:end:state_vars-SingletonInit

// docs:start:state_vars-SingletonReplace
fn update_legendary_card(
context: &mut PrivateContext,
state_var: Singleton<CardNote, CARD_NOTE_LEN>,
card: &mut CardNote,
) {
state_var.replace(context, card);
fn update_legendary_card(state_var: Singleton<CardNote, CARD_NOTE_LEN>, card: &mut CardNote) {
state_var.replace(card);
}
// docs:end:state_vars-SingletonReplace

// docs:start:state_vars-SingletonGet
fn get_legendary_card(
context: &mut PrivateContext,
state_var: Singleton<CardNote, CARD_NOTE_LEN>,
) -> CardNote {
state_var.get_note(context)
fn get_legendary_card(state_var: Singleton<CardNote, CARD_NOTE_LEN>) -> CardNote {
state_var.get_note()
}
// docs:end:state_vars-SingletonGet

// docs:start:state_vars-ImmutableSingletonInit
fn init_game_rules(
context: &mut PrivateContext,
state_var: ImmutableSingleton<RulesNote, RULES_NOTE_LEN>,
rules: &mut RulesNote,
) {
state_var.initialise(context, rules);
state_var.initialise(rules);
}
// docs:end:state_vars-ImmutableSingletonInit

// docs:start:state_vars-ImmutableSingletonGet
fn is_valid_card(
context: &mut PrivateContext,
state_var: ImmutableSingleton<RulesNote, RULES_NOTE_LEN>,
card: CardNote,
) -> bool {
let rules = state_var.get_note(context);
fn is_valid_card(state_var: ImmutableSingleton<RulesNote, RULES_NOTE_LEN>, card: CardNote) -> bool {
let rules = state_var.get_note();
card.points >= rules.min_points & card.points <= rules.max_points
}
// docs:end:state_vars-ImmutableSingletonGet

// docs:start:state_vars-SetInsert
fn add_new_card(
context: &mut PrivateContext,
state_var: Set<CardNote, CARD_NOTE_LEN>,
card: &mut CardNote,
) {
state_var.insert(context, card);
fn add_new_card(state_var: Set<CardNote, CARD_NOTE_LEN>, card: &mut CardNote) {
state_var.insert(card);
}
// docs:end:state_vars-SetInsert

// docs:start:state_vars-SetRemove
fn remove_card(
context: &mut PrivateContext,
state_var: Set<CardNote, CARD_NOTE_LEN>,
card: CardNote,
) {
state_var.remove(context, card);
fn remove_card(state_var: Set<CardNote, CARD_NOTE_LEN>, card: CardNote) {
state_var.remove(card);
}
// docs:end:state_vars-SetRemove

// docs:start:state_vars-SetGet
fn get_cards<FILTER_PARAMS>(
context: &mut PrivateContext,
state_var: Set<CardNote, CARD_NOTE_LEN>,
options: NoteGetterOptions<CardNote, CARD_NOTE_LEN, FILTER_PARAMS>,
) -> [Option<CardNote>; MAX_READ_REQUESTS_PER_CALL] {
state_var.get_notes(context, options)
state_var.get_notes(options)
}
// docs:end:state_vars-SetGet

// docs:start:state_vars-SetView
unconstrained fn view_cards(
unconstrained fn view_cards(
state_var: Set<CardNote, CARD_NOTE_LEN>,
options: NoteViewerOptions<CardNote, CARD_NOTE_LEN>,
) -> [Option<CardNote>; MAX_NOTES_PER_PAGE] {
state_var.view_notes(options)
}
// docs:end:state_vars-SetView

unconstrained fn get_total_points(state_var: Set<CardNote, CARD_NOTE_LEN>, account: Field, offset: u32) -> u8 {
let options = NoteViewerOptions::new().select(2, account).set_offset(offset);
unconstrained fn get_total_points(
state_var: Set<CardNote, CARD_NOTE_LEN>,
account: Field,
offset: u32,
) -> u8 {
let options = NoteViewerOptions::new()
.select(2, account)
.set_offset(offset);
let mut total_points = 0;
let notes = view_cards(state_var, options);
for i in 0..notes.len() {
Expand All @@ -181,31 +147,28 @@ unconstrained fn get_total_points(state_var: Set<CardNote, CARD_NOTE_LEN>, accou

// docs:start:state_vars-SetContains
fn assert_contains_card<RERUEN_LEN, FILTER_PARAMS>(
context: &mut PrivateContext,
state_var: Set<CardNote, CARD_NOTE_LEN>,
card: CardNote,
) {
state_var.assert_contains_and_remove(context, card);
state_var.assert_contains_and_remove(card);
}
// docs:end:state_vars-SetContains

// docs:start:state_vars-MapAtSingletonInit
fn add_new_profile(
context: &mut PrivateContext,
state_var: Map<Singleton<ProfileNote, PROFILE_NOTE_LEN>>,
account: Field,
profile: &mut ProfileNote,
) {
state_var.at(account).initialise(context, profile);
state_var.at(account).initialise(profile);
}
// docs:end:state_vars-MapAtSingletonInit

// docs:start:state_vars-MapAtSingletonGet
fn get_profile(
context: &mut PrivateContext,
state_var: Map<Singleton<ProfileNote, PROFILE_NOTE_LEN>>,
account: Field,
) -> ProfileNote {
state_var.at(account).get_note(context)
state_var.at(account).get_note()
}
// docs:end:state_vars-MapAtSingletonGet
// docs:end:state_vars-MapAtSingletonGet
Loading