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

chore: fix array access issue - nuclear edition #4814

Closed
wants to merge 40 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
d630b44
git subrepo pull --branch=master --force noir
sirasistant Feb 14, 2024
518ec41
wip: restricting bit sizes
sirasistant Feb 15, 2024
636c316
fix: more fixes for restricted bit sizes
sirasistant Feb 15, 2024
f92dcdd
more u128
sirasistant Feb 16, 2024
c7986a3
get them all compiling
Maddiaa0 Feb 21, 2024
ab832ff
hack build
Maddiaa0 Feb 21, 2024
5c5156f
fix: noir stdlib error message
Maddiaa0 Feb 21, 2024
aecdd3a
Merge branch 'master' into md/02-21-get_them_all_compiling
Maddiaa0 Feb 21, 2024
1002bb9
fix: update gas rebate contract
Maddiaa0 Feb 21, 2024
ec6ce1d
Merge branch 'master' into md/02-21-get_them_all_compiling
Maddiaa0 Feb 26, 2024
38181cd
fix: merge
Maddiaa0 Feb 26, 2024
9eeb153
exp: blacklist return field
Maddiaa0 Feb 26, 2024
ea0d0c6
fix: underflow error messages
Maddiaa0 Feb 26, 2024
9d4fe7e
fix
Maddiaa0 Feb 26, 2024
e9db646
fix: update lending simulator check
Maddiaa0 Feb 27, 2024
7af5de5
fmt
Maddiaa0 Feb 27, 2024
621f593
fix
Maddiaa0 Feb 27, 2024
f2fe7fc
fix
Maddiaa0 Feb 27, 2024
e92208f
Merge branch 'master' into md/02-21-get_them_all_compiling
Maddiaa0 Feb 27, 2024
1d26069
fix: error message
Maddiaa0 Feb 27, 2024
c4e0c5e
temp
Maddiaa0 Feb 14, 2024
543e093
fix: get e2e hashing working
Maddiaa0 Feb 14, 2024
800a774
sweep
Maddiaa0 Feb 14, 2024
1bd7952
sweep 2
Maddiaa0 Feb 14, 2024
2522293
feat: avm storage
Maddiaa0 Feb 16, 2024
94466de
fix: rebase
Maddiaa0 Feb 27, 2024
fdda301
Merge branch 'master' into md/02-16-feat_avm_storage
Maddiaa0 Feb 27, 2024
d16dd58
fix: merge
Maddiaa0 Feb 27, 2024
eded899
fixes: test and tag check alignment
Maddiaa0 Feb 27, 2024
025dc2a
align addressing
Maddiaa0 Feb 27, 2024
d633b8b
chore: working with bitsize hack
Maddiaa0 Feb 27, 2024
3a327dd
fix: dont meddle with ssa
Maddiaa0 Feb 27, 2024
a932153
Merge branch 'master' into md/02-16-feat_avm_storage
Maddiaa0 Feb 27, 2024
2ae18c2
nuclear u32 swap to solve issue
Maddiaa0 Feb 27, 2024
2636555
rm debug file
Maddiaa0 Feb 27, 2024
fe52ba6
fix: external call test
Maddiaa0 Feb 27, 2024
0acb758
u32ify protocol circuits
Maddiaa0 Feb 27, 2024
ff30070
fix: dangling u64
Maddiaa0 Feb 27, 2024
ef9a36c
fix: update noir tests
Maddiaa0 Feb 27, 2024
800b9e3
fix: merge issue
Maddiaa0 Feb 27, 2024
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
1 change: 1 addition & 0 deletions avm-transpiler/src/instructions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use crate::opcodes::AvmOpcode;
pub const ALL_DIRECT: u8 = 0b00000000;
pub const ZEROTH_OPERAND_INDIRECT: u8 = 0b00000001;
pub const FIRST_OPERAND_INDIRECT: u8 = 0b00000010;
pub const SECOND_OPERAND_INDIRECT: u8 = 0b00000100;
pub const ZEROTH_FIRST_OPERANDS_INDIRECT: u8 = 0b00000011;

/// A simple representation of an AVM instruction for the purpose
Expand Down
90 changes: 87 additions & 3 deletions avm-transpiler/src/transpile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use acvm::brillig_vm::brillig::{

use crate::instructions::{
AvmInstruction, AvmOperand, AvmTypeTag, ALL_DIRECT, FIRST_OPERAND_INDIRECT,
ZEROTH_OPERAND_INDIRECT,
SECOND_OPERAND_INDIRECT, ZEROTH_OPERAND_INDIRECT,
};
use crate::opcodes::AvmOpcode;
use crate::utils::{dbg_print_avm_program, dbg_print_brillig_program};
Expand Down Expand Up @@ -250,7 +250,9 @@ fn handle_foreign_call(
}
"poseidon" => {
handle_single_field_hash_instruction(avm_instrs, function, destinations, inputs)
}
},
"storageWrite" => emit_storage_write(avm_instrs, destinations, inputs),
"storageRead" => emit_storage_read(avm_instrs, destinations, inputs),
_ => handle_getter_instruction(avm_instrs, function, destinations, inputs),
}
}
Expand All @@ -270,7 +272,7 @@ fn handle_emit_note_hash_or_nullifier(
"EMITNOTEHASH"
};

if destinations.len() != 0 || inputs.len() != 1 {
if !destinations.is_empty() || inputs.len() != 1 {
panic!(
"Transpiler expects ForeignCall::{} to have 0 destinations and 1 input, got {} and {}",
function_name,
Expand Down Expand Up @@ -299,6 +301,88 @@ fn handle_emit_note_hash_or_nullifier(
});
}

/// Emit a storage write opcode
/// The current implementation writes an array of values into storage ( contiguous slots in memory )
fn emit_storage_write(
avm_instrs: &mut Vec<AvmInstruction>,
destinations: &Vec<ValueOrArray>,
inputs: &Vec<ValueOrArray>)
{
// For the foreign calls we want to handle, we do not want inputs, as they are getters
assert!(inputs.len() == 2);
assert!(destinations.len() == 1); // TODO: we want this to be empty - change aztec nr?

let slot_offset_maybe = inputs[0];
let slot_offset = match slot_offset_maybe {
ValueOrArray::MemoryAddress(slot_offset) => slot_offset.0,
_ => panic!("ForeignCall address destination should be a single value"),
};

let src_offset_maybe = inputs[1];
let (src_offset, src_size) = match src_offset_maybe {
ValueOrArray::HeapArray(HeapArray { pointer, size }) => (pointer.0, size),
_ => panic!("Storage write address inputs should be an array of values"),
};

avm_instrs.push(AvmInstruction {
opcode: AvmOpcode::SSTORE,
indirect: Some(ZEROTH_OPERAND_INDIRECT),
operands: vec![
AvmOperand::U32 {
value: src_offset as u32,
},
AvmOperand::U32 {
value: src_size as u32,
},
AvmOperand::U32 {
value: slot_offset as u32,
},
],
..Default::default()
})
}

/// Emit a storage read opcode
/// The current implementation reads an array of values from storage ( contiguous slots in memory )
fn emit_storage_read(
avm_instrs: &mut Vec<AvmInstruction>,
destinations: &Vec<ValueOrArray>,
inputs: &Vec<ValueOrArray>
) {
// For the foreign calls we want to handle, we do not want inputs, as they are getters
assert!(inputs.len() == 2); // output, len - but we dont use this len - its for the oracle
assert!(destinations.len() == 1);

let slot_offset_maybe = inputs[0];
let slot_offset = match slot_offset_maybe {
ValueOrArray::MemoryAddress(slot_offset) => slot_offset.0,
_ => panic!("ForeignCall address destination should be a single value"),
};

let dest_offset_maybe = destinations[0];
let (dest_offset, src_size) = match dest_offset_maybe {
ValueOrArray::HeapArray(HeapArray { pointer, size }) => (pointer.0, size),
_ => panic!("Storage write address inputs should be an array of values"),
};

avm_instrs.push(AvmInstruction {
opcode: AvmOpcode::SLOAD,
indirect: Some(SECOND_OPERAND_INDIRECT),
operands: vec![
AvmOperand::U32 {
value: slot_offset as u32,
},
AvmOperand::U32 {
value: src_size as u32,
},
AvmOperand::U32 {
value: dest_offset as u32,
},
],
..Default::default()
})
}

/// Handle an AVM NULLIFIEREXISTS instruction
/// (a nullifierExists brillig foreign call was encountered)
/// Adds the new instruction to the avm instructions list.
Expand Down
1 change: 1 addition & 0 deletions l1-contracts/src/core/libraries/ConstantsGen.sol
Original file line number Diff line number Diff line change
Expand Up @@ -123,4 +123,5 @@ library Constants {
uint256 internal constant CONTRACT_DATA_NUM_BYTES_PER_BASE_ROLLUP_UNPADDED = 52;
uint256 internal constant L2_TO_L1_MSGS_NUM_BYTES_PER_BASE_ROLLUP = 64;
uint256 internal constant LOGS_HASHES_NUM_BYTES_PER_BASE_ROLLUP = 64;
uint256 internal constant APPEND_ONLY_TREE_SNAPSHOT_LENGTH = 2;
}
11 changes: 8 additions & 3 deletions noir-projects/aztec-nr/aztec/src/context.nr
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,23 @@ use avm::AVMContext;
struct Context {
private: Option<&mut PrivateContext>,
public: Option<&mut PublicContext>,
public_vm: Option<&mut AVMContext>,
}

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

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

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

pub fn none() -> Context {
Context { public: Option::none(), private: Option::none() }
Context { public: Option::none(), private: Option::none(), public_vm: Option::none() }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ struct L1ToL2MessageGetterData {
leaf_index: Field
}

pub fn l1_to_l2_message_getter_len() -> u64 {
pub fn l1_to_l2_message_getter_len() -> u32 {
L1_TO_L2_MESSAGE_LENGTH + 1 + L1_TO_L2_MSG_TREE_HEIGHT
}

pub fn make_l1_to_l2_message_getter_data<N>(
fields: [Field; N],
start: u64,
start: u32,
secret: Field
) -> L1ToL2MessageGetterData {
L1ToL2MessageGetterData {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ impl<Note, N, FILTER_ARGS> NoteGetterOptions<Note, N, FILTER_ARGS> {
pub fn with_filter(
filter: fn([Option<Note>; MAX_READ_REQUESTS_PER_CALL], FILTER_ARGS) -> [Option<Note>; MAX_READ_REQUESTS_PER_CALL],
filter_args: FILTER_ARGS
) -> Self where Note: NoteInterface<N> {
) -> Self where Note: NoteInterface<N> {
NoteGetterOptions {
selects: BoundedVec::new(),
sorts: BoundedVec::new(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use dep::protocol_types::{
utils::arr_copy_slice
};

global LEAF_PREIMAGE_LENGTH: u64 = 4;
global LEAF_PREIMAGE_LENGTH: u32 = 4;
global PUBLIC_DATA_WITNESS: Field = 45;

struct PublicDataWitness {
Expand All @@ -23,7 +23,7 @@ unconstrained pub fn get_public_data_witness(block_number: u32, leaf_slot: Field
let fields = get_public_data_witness_oracle(block_number, leaf_slot);
PublicDataWitness {
index: fields[0],
leaf_preimage: PublicDataTreeLeafPreimage { slot: fields[1], value: fields[2], next_index: fields[3] as u64, next_slot: fields[4] },
leaf_preimage: PublicDataTreeLeafPreimage { slot: fields[1], value: fields[2], next_index: fields[3] as u32, next_slot: fields[4] },
path: arr_copy_slice(fields, [0; PUBLIC_DATA_TREE_HEIGHT], 1 + LEAF_PREIMAGE_LENGTH)
}
}
10 changes: 5 additions & 5 deletions noir-projects/aztec-nr/aztec/src/oracle/notes.nr
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,14 @@ unconstrained pub fn get_notes<Note, N, M, S, NS>(
status,
placeholder_fields
);
let num_notes = fields[0] as u64;
let num_notes = fields[0] as u32;
let contract_address = AztecAddress::from_field(fields[1]);
for i in 0..placeholder_opt_notes.len() {
if i < num_notes {
if i as u32 < num_notes {
// lengths named as per typescript.
let return_header_length: u64 = 2; // num_notes & contract_address.
let extra_preimage_length: u64 = 2; // nonce & is_transient.
let read_offset: u64 = return_header_length + i * (N + extra_preimage_length);
let return_header_length: u32 = 2; // num_notes & contract_address.
let extra_preimage_length: u32 = 2; // nonce & is_transient.
let read_offset: u32 = return_header_length + i * (N + extra_preimage_length);
let nonce = fields[read_offset];
let is_transient = fields[read_offset + 1] as bool;
let header = NoteHeader { contract_address, nonce, storage_slot, is_transient };
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,29 @@
contract AvmTest {
// Libs
use dep::aztec::protocol_types::address::{AztecAddress, EthAddress};
use dep::aztec::state_vars::PublicMutable;

// avm lib
use dep::aztec::avm::hash::{keccak256, poseidon, sha256};

#[aztec(private)]
fn constructor() {}

// Public-vm macro will prefix avm to the function name for transpilation
struct Storage {
owner: PublicMutable<AztecAddress>
}

#[aztec(public-vm)]
fn setAdmin() {
storage.owner.write(context.sender());
}

#[aztec(public-vm)]
fn setAndRead() -> pub AztecAddress {
storage.owner.write(context.sender());
storage.owner.read()
}

#[aztec(public-vm)]
fn addArgsReturn(argA: Field, argB: Field) -> pub Field {
argA + argB
Expand Down Expand Up @@ -40,7 +55,6 @@ contract AvmTest {
// fn setOpcodeUint128() -> pub u128 {
// 1 << 120 as u128
// }

// Field should fit in 128 bits
// ACIR only supports fields of up to 126 bits!
// Same with internal fields for unconstrained functions, apprently.
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
array.len() should return a u64
array.len() should return a u32
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ use dep::types::{
struct PrivateKernelTailCircuitPrivateInputs {
previous_kernel: PrivateKernelInnerData,
sorted_new_note_hashes: [SideEffect; MAX_NEW_NOTE_HASHES_PER_TX],
sorted_new_note_hashes_indexes: [u64; MAX_NEW_NOTE_HASHES_PER_TX],
read_commitment_hints: [u64; MAX_READ_REQUESTS_PER_TX],
sorted_new_note_hashes_indexes: [u32; MAX_NEW_NOTE_HASHES_PER_TX],
read_commitment_hints: [u32; MAX_READ_REQUESTS_PER_TX],
sorted_new_nullifiers: [SideEffectLinkedToNoteHash; MAX_NEW_NULLIFIERS_PER_TX],
sorted_new_nullifiers_indexes: [u64; MAX_NEW_NULLIFIERS_PER_TX],
nullifier_commitment_hints: [u64; MAX_NEW_NULLIFIERS_PER_TX],
sorted_new_nullifiers_indexes: [u32; MAX_NEW_NULLIFIERS_PER_TX],
nullifier_commitment_hints: [u32; MAX_NEW_NULLIFIERS_PER_TX],
master_nullifier_secret_keys: [GrumpkinPrivateKey; MAX_NULLIFIER_KEY_VALIDATION_REQUESTS_PER_TX],
}

Expand Down Expand Up @@ -64,7 +64,7 @@ impl PrivateKernelTailCircuitPrivateInputs {
// match reads to commitments from the previous call(s)
for rr_idx in 0..MAX_READ_REQUESTS_PER_TX {
let read_request = read_requests.get_unchecked(rr_idx);
let read_commitment_hint = self.read_commitment_hints[rr_idx] as u64;
let read_commitment_hint = self.read_commitment_hints[rr_idx] as u32;

if (read_request.value != 0) {
let hash = new_note_hashes.get_unchecked(read_commitment_hint);
Expand All @@ -79,7 +79,7 @@ impl PrivateKernelTailCircuitPrivateInputs {
public_inputs.end.read_requests = BoundedVec::new();
}

fn assert_sorted_counters<T, N>(original: [T; N], sorted: [T; N], indexes: [u64; N]) where T: Eq + Ordered + Empty {
fn assert_sorted_counters<T, N>(original: [T; N], sorted: [T; N], indexes: [u32; N]) where T: Eq + Ordered + Empty {
let mut prev_was_empty = false;

for i in 0..N {
Expand Down Expand Up @@ -126,15 +126,15 @@ impl PrivateKernelTailCircuitPrivateInputs {
let nullifier = new_nullifiers[n_idx];
// TODO - should not be able to squash the first nullifier.
let nullified_note_hash = nullifier.note_hash;
let hint_pos = self.nullifier_commitment_hints[n_idx] as u64;
let hint_pos = self.nullifier_commitment_hints[n_idx] as u32;

// Nullified_commitment of value `0` implies non-transient (persistable)
// nullifier in which case no attempt will be made to match it to a hash.
// Non-empty nullified_note_hash implies transient nullifier which MUST be matched to a hash below!
// 0-valued nullified_note_hash is empty and will be ignored
if nullified_note_hash != 0 {
assert(
hint_pos < MAX_NEW_NOTE_HASHES_PER_TX as u64, "New nullifier is transient but hint is invalid"
hint_pos < MAX_NEW_NOTE_HASHES_PER_TX as u32, "New nullifier is transient but hint is invalid"
);
let hash = new_note_hashes[hint_pos];
assert_eq(nullified_note_hash, hash.value, "Hinted hash does not match");
Expand All @@ -145,7 +145,7 @@ impl PrivateKernelTailCircuitPrivateInputs {
// squash both the nullifier and the hash
// (set to 0 here and then rearrange array after loop)
new_note_hashes[hint_pos] = SideEffect::empty();
new_nullifiers[n_idx as u64] = SideEffectLinkedToNoteHash::empty();
new_nullifiers[n_idx as u32] = SideEffectLinkedToNoteHash::empty();
}
// non-transient (persistable) nullifiers are just kept in new_nullifiers array and forwarded
// to public inputs (used later by base rollup circuit)
Expand Down Expand Up @@ -240,8 +240,8 @@ mod tests {

struct PrivateKernelOrderingInputsBuilder {
previous_kernel: PreviousKernelDataBuilder,
read_commitment_hints: [u64; MAX_READ_REQUESTS_PER_TX],
nullifier_commitment_hints: [u64; MAX_NEW_NULLIFIERS_PER_TX],
read_commitment_hints: [u32; MAX_READ_REQUESTS_PER_TX],
nullifier_commitment_hints: [u32; MAX_NEW_NULLIFIERS_PER_TX],
}

impl PrivateKernelOrderingInputsBuilder {
Expand Down Expand Up @@ -272,32 +272,32 @@ mod tests {
compute_unique_siloed_note_hashes(first_nullifier.value, commitments)
}

pub fn append_transient_commitments(&mut self, num_commitments: u64) {
pub fn append_transient_commitments(&mut self, num_commitments: u32) {
// All new note hashes aggregated in the previous kernel are transient commitments.
self.previous_kernel.append_new_note_hashes(num_commitments);
}

pub fn add_transient_read(&mut self, commitment_index: u64) {
pub fn add_transient_read(&mut self, commitment_index: u32) {
let read_request_index = self.previous_kernel.add_read_request_for_transient_commitment(commitment_index);
self.read_commitment_hints[read_request_index] = commitment_index;
}

pub fn append_nullifiers(&mut self, num_nullifiers: u64) {
pub fn append_nullifiers(&mut self, num_nullifiers: u32) {
self.previous_kernel.append_new_nullifiers_from_private(num_nullifiers);
}

pub fn nullify_transient_commitment(&mut self, nullifier_index: Field, commitment_index: u64) {
pub fn nullify_transient_commitment(&mut self, nullifier_index: Field, commitment_index: u32) {
self.previous_kernel.end.new_nullifiers.storage[nullifier_index].note_hash = self.previous_kernel.end.new_note_hashes.get(commitment_index).value;
self.nullifier_commitment_hints[nullifier_index] = commitment_index;
}

fn sort_sideffects<T, N>(original: [T; N]) -> ([T; N], [u64; N]) where T: Ordered + Eq + Empty {
fn sort_sideffects<T, N>(original: [T; N]) -> ([T; N], [u32; N]) where T: Ordered + Eq + Empty {
let mut indexes = [0; N];
for i in 0..N {
indexes[i] = i;
}
let sorted_indexes = indexes.sort_via(
|a_index: u64, b_index: u64| {
|a_index: u32, b_index: u32| {
let a = original[a_index];
let b = original[b_index];
if is_empty(b) {
Expand All @@ -309,7 +309,7 @@ mod tests {
}
}
);
let sorted_sideffects = sorted_indexes.map(|i: u64| original[i]);
let sorted_sideffects = sorted_indexes.map(|i: u32| original[i]);
let mut reverse_map = [0; N];
for i in 0..N {
reverse_map[sorted_indexes[i]] = i;
Expand Down
Loading
Loading