Skip to content

Commit

Permalink
feat(noir-contracts): Option<T> for get_notes (#1272)
Browse files Browse the repository at this point in the history
# Description

closes: #1020

Introduce `Option<T>` type to Noir.

Update `get_notes` logic:
- Remove the `is_real` member of some notes. `Option::is_some()` can now
be used instead.
- Remove the `is_dummy` method of the `NoteInterface`.
- When calling the `get_notes` oracle, a boolean `is_some` is injected
after the `nonce` to convey whether the note is a dummy or not.
- Change contract logic to `unwrap` `Option<Note>` types.

It's slightly more verbose in places, but it makes the notion of a dummy
note more enshrined (and actually aligns more closely to our inital
sketches of what utxo syntax might look like).

Thanks to Jake for the Option implementation!
  • Loading branch information
iAmMichaelConnor authored Aug 1, 2023
1 parent 072e13f commit 584b70f
Show file tree
Hide file tree
Showing 24 changed files with 411 additions and 239 deletions.
37 changes: 34 additions & 3 deletions yarn-project/acir-simulator/src/client/client_execution_context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ export class ClientTxExecutionContext {
/** The list of nullifiers created in this transaction. The commitment/note which is nullified
* might be pending or not (i.e., was generated in a previous transaction) */
private pendingNullifiers: Set<Fr> = new Set<Fr>(),

private log = createDebugLogger('aztec:simulator:client_execution_context'),
) {}

/**
Expand Down Expand Up @@ -146,8 +148,20 @@ export class ClientTxExecutionContext {
.join(', ')}`,
);

// TODO: notice, that if we don't have a note in our DB, we don't know how big the preimage needs to be, and so we don't actually know how many dummy notes to return, or big to make those dummy notes, or where to position `is_some` booleans to inform the noir program that _all_ the notes should be dummies.
// By a happy coincidence, a `0` field is interpreted as `is_none`, and since in this case (of an empty db) we'll return all zeros (paddedZeros), the noir program will treat the returned data as all dummies, but this is luck. Perhaps a preimage size should be conveyed by the get_notes noir oracle?
const preimageLength = notes?.[0]?.preimage.length ?? 0;
if (
!notes.every(({ preimage }) => {
return preimageLength === preimage.length;
})
)
throw new Error('Preimages for a particular note type should all be the same length');

// Combine pending and db preimages into a single flattened array.
const preimages = notes.flatMap(({ nonce, preimage }) => [nonce, ...preimage]);
const isSome = new Fr(1); // Boolean. Indicates whether the Noir Option<Note>::is_some();

const realNotePreimages = notes.flatMap(({ nonce, preimage }) => [nonce, isSome, ...preimage]);

// Add a partial witness for each note.
// It contains the note index for db notes. And flagged as transient for pending notes.
Expand All @@ -157,8 +171,25 @@ export class ClientTxExecutionContext {
);
});

const paddedZeros = Array(Math.max(0, returnSize - 2 - preimages.length)).fill(Fr.ZERO);
return [notes.length, contractAddress, ...preimages, ...paddedZeros].map(v => toACVMField(v));
const returnHeaderLength = 2; // is for the header values: `notes.length` and `contractAddress`.
const extraPreimageLength = 2; // is for the nonce and isSome fields.
const extendedPreimageLength = preimageLength + extraPreimageLength;
const numRealNotes = notes.length;
const numReturnNotes = Math.floor((returnSize - returnHeaderLength) / extendedPreimageLength);
const numDummyNotes = numReturnNotes - numRealNotes;

const dummyNotePreimage = Array(extendedPreimageLength).fill(Fr.ZERO);
const dummyNotePreimages = Array(numDummyNotes)
.fill(dummyNotePreimage)
.flatMap(note => note);

const paddedZeros = Array(
Math.max(0, returnSize - returnHeaderLength - realNotePreimages.length - dummyNotePreimages.length),
).fill(Fr.ZERO);

return [notes.length, contractAddress, ...realNotePreimages, ...dummyNotePreimages, ...paddedZeros].map(v =>
toACVMField(v),
);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ describe('Private Execution test suite', () => {

const buildNote = (amount: bigint, owner: AztecAddress, storageSlot = Fr.random()) => {
const nonce = new Fr(currentNoteIndex);
const preimage = [new Fr(amount), owner.toField(), Fr.random(), new Fr(1n)];
const preimage = [new Fr(amount), owner.toField(), Fr.random()];
return { contractAddress, storageSlot, index: currentNoteIndex++, nonce, nullifier: new Fr(0), preimage };
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ describe('Unconstrained Execution test suite', () => {
let owner: AztecAddress;

const buildNote = (amount: bigint, owner: AztecAddress) => {
return [new Fr(amount), owner, Fr.random(), new Fr(1n)];
return [new Fr(amount), owner, Fr.random()];
};

const calculateAddress = (privateKey: PrivateKey) => {
Expand Down Expand Up @@ -67,6 +67,7 @@ describe('Unconstrained Execution test suite', () => {
contractAddress,
storageSlot: Fr.random(),
nonce: Fr.random(),
isSome: new Fr(1),
preimage,
nullifier: Fr.random(),
index: BigInt(index),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,6 @@ impl EcdsaPublicKeyNote {
header: NoteHeader::empty(),
}
}

fn is_dummy(self) -> bool {
(self.x == [0;32]) & (self.y == [0;32]) & (self.owner == 0)
}
}

fn deserialise(preimage: [Field; ECDSA_PUBLIC_KEY_NOTE_LEN]) -> EcdsaPublicKeyNote {
Expand Down Expand Up @@ -116,10 +112,6 @@ fn dummy() -> EcdsaPublicKeyNote {
EcdsaPublicKeyNote::dummy()
}

fn is_dummy(note: EcdsaPublicKeyNote) -> bool {
note.is_dummy()
}

fn get_header(note: EcdsaPublicKeyNote) -> NoteHeader {
note.header
}
Expand All @@ -134,7 +126,6 @@ global EcdsaPublicKeyNoteInterface = NoteInterface {
compute_note_hash,
compute_nullifier,
dummy,
is_dummy,
get_header,
set_header,
};
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,6 @@ impl AddressNote {
header: NoteHeader::empty(),
}
}

fn is_dummy(self) -> bool {
(self.address == 0) & (self.owner == 0)
}
}

fn deserialise(preimage: [Field; ADDRESS_NOTE_LEN]) -> AddressNote {
Expand All @@ -78,10 +74,6 @@ fn dummy() -> AddressNote {
AddressNote::dummy()
}

fn is_dummy(note: AddressNote) -> bool {
note.is_dummy()
}

fn get_header(note: AddressNote) -> NoteHeader {
note.header
}
Expand All @@ -96,7 +88,6 @@ global AddressNoteInterface = NoteInterface {
compute_note_hash,
compute_nullifier,
dummy,
is_dummy,
get_header,
set_header,
};
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,15 @@ contract PendingCommitments {

let options = NoteGetterOptions::with_filter(get_2_notes, 0);
// get note inserted above
let got_notes = owner_balance.get_notes(&mut context, options);
let maybe_notes = owner_balance.get_notes(&mut context, options);

assert(note.value == got_notes[0].value);
assert(!got_notes[1].is_real);
let note0 = maybe_notes[0].unwrap();
assert(note.value == note0.value);
assert(maybe_notes[1].is_none());

context.return_values.push(got_notes[0].value);
context.return_values.push(note0.value);

owner_balance.remove(&mut context, got_notes[0]);
owner_balance.remove(&mut context, note0);

context.finish()
}
Expand All @@ -94,13 +95,12 @@ contract PendingCommitments {

let options = NoteGetterOptions::with_filter(get_2_notes, 0);
// get note (note inserted at bottom of function shouldn't exist yet)
let got_notes = owner_balance.get_notes(&mut context, options);
let maybe_notes = owner_balance.get_notes(&mut context, options);

assert(!got_notes[0].is_real);
assert(got_notes[0].value == 0);
assert(!got_notes[1].is_real);
assert(maybe_notes[0].is_none());
assert(maybe_notes[1].is_none());

context.return_values.push(got_notes[0].value);
context.return_values.push(0);

// Insert note and emit encrypted note preimage via oracle call
let mut note = ValueNote::new(amount, owner);
Expand Down Expand Up @@ -145,14 +145,13 @@ contract PendingCommitments {
let owner_balance = storage.balances.at(owner);

let options = NoteGetterOptions::with_filter(get_2_notes, 0);
let got_notes = owner_balance.get_notes(&mut context, options);
let note = owner_balance.get_notes(&mut context, options)[0].unwrap();

assert(expected_value == got_notes[0].value);
assert(!got_notes[1].is_real);
assert(expected_value == note.value);

context.return_values.push(got_notes[0].value);
context.return_values.push(expected_value);

owner_balance.remove(&mut context, got_notes[0]);
owner_balance.remove(&mut context, note);

context.finish()
}
Expand All @@ -171,10 +170,10 @@ contract PendingCommitments {
let owner_balance = storage.balances.at(owner);

let options = NoteGetterOptions::with_filter(get_2_notes, 0);
let got_notes = owner_balance.get_notes(&mut context, options);
let maybe_notes = owner_balance.get_notes(&mut context, options);

assert(!got_notes[0].is_real);
assert(!got_notes[1].is_real);
assert(maybe_notes[0].is_none());
assert(maybe_notes[1].is_none());

context.finish()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,6 @@ impl AddressNote {
header: NoteHeader::empty(),
}
}

fn is_dummy(self) -> bool {
self.address == 0
}
}

fn deserialise(preimage: [Field; ADDRESS_NOTE_LEN]) -> AddressNote {
Expand All @@ -75,10 +71,6 @@ fn dummy() -> AddressNote {
AddressNote::dummy()
}

fn is_dummy(note: AddressNote) -> bool {
note.is_dummy()
}

fn get_header(note: AddressNote) -> NoteHeader {
note.header
}
Expand All @@ -93,7 +85,6 @@ global AddressNoteInterface = NoteInterface {
compute_note_hash,
compute_nullifier,
dummy,
is_dummy,
get_header,
set_header,
};
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ contract PokeableToken {
use dep::value_note::{
balance_utils,
utils::{send_note, spend_notes},
value_note::{VALUE_NOTE_LEN, ValueNoteInterface},
value_note::{VALUE_NOTE_LEN, ValueNoteInterface, ValueNote},
filter::get_2_notes,
};
use dep::aztec::abi;
Expand Down Expand Up @@ -91,16 +91,22 @@ contract PokeableToken {
let sender_balance = storage.balances.at(sender.address);

let options = NoteGetterOptions::with_filter(get_2_notes, 0);
let notes = sender_balance.get_notes(&mut context, options);
let maybe_notes = sender_balance.get_notes(&mut context, options);

let note1 = notes[0];
let note2 = notes[1];
let note0 = maybe_notes[0].unwrap_or(ValueNote::dummy());
let note1 = maybe_notes[1].unwrap_or(ValueNote::dummy());

let note_sum = note1.value + note2.value;
let note_sum = note0.value + note1.value;

// Removes the 2 notes from the sender's set of notes.
sender_balance.remove(&mut context, note1);
sender_balance.remove(&mut context, note2);
if maybe_notes[0].is_some() {
assert(sender.address == note0.owner);
sender_balance.remove(&mut context, note0);
}
if maybe_notes[1].is_some() {
assert(sender.address == note1.owner);
sender_balance.remove(&mut context, note1);
}

// Create new note for the recipient.
let recipient_balance = storage.balances.at(recipient.address);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ impl Storage {
Storage {
sender: ImmutableSingleton::new(1, AddressNoteInterface),
recipient: ImmutableSingleton::new(2, AddressNoteInterface),
balances: Map::new(1, |s| Set::new(s, ValueNoteInterface)),
balances: Map::new(3, |s| Set::new(s, ValueNoteInterface)),
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,6 @@ impl PublicKeyNote {
header: NoteHeader::empty(),
}
}

fn is_dummy(self) -> bool {
(self.x == 0) & (self.y == 0) & (self.owner == 0)
}
}

fn deserialise(preimage: [Field; PUBLIC_KEY_NOTE_LEN]) -> PublicKeyNote {
Expand Down Expand Up @@ -84,10 +80,6 @@ fn dummy() -> PublicKeyNote {
PublicKeyNote::dummy()
}

fn is_dummy(note: PublicKeyNote) -> bool {
note.is_dummy()
}

fn get_header(note: PublicKeyNote) -> NoteHeader {
note.header
}
Expand All @@ -102,7 +94,6 @@ global PublicKeyNoteInterface = NoteInterface {
compute_note_hash,
compute_nullifier,
dummy,
is_dummy,
get_header,
set_header,
};
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,6 @@ impl ClaimNote {
}
}

fn is_dummy(self) -> bool {
self.value == 0
}

fn set_header(&mut self, header: NoteHeader) {
self.header = header;
}
Expand All @@ -86,10 +82,6 @@ fn dummy() -> ClaimNote {
ClaimNote::dummy()
}

fn is_dummy(note: ClaimNote) -> bool {
note.is_dummy()
}

fn get_header(note: ClaimNote) -> NoteHeader {
note.header
}
Expand All @@ -104,7 +96,6 @@ global ClaimNoteInterface = NoteInterface {
compute_note_hash,
compute_nullifier,
dummy,
is_dummy,
get_header,
set_header,
};
Loading

0 comments on commit 584b70f

Please sign in to comment.