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(avm): hashing to simulator #4527

Merged
merged 10 commits into from
Feb 20, 2024
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
6 changes: 3 additions & 3 deletions avm-transpiler/src/opcodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,10 +150,10 @@ impl AvmOpcode {
// World State
AvmOpcode::SLOAD => "SLOAD", // Public Storage
AvmOpcode::SSTORE => "SSTORE", // Public Storage
AvmOpcode::NOTEHASHEXISTS => "NOTEHASHEXISTS", // Notes & Nullifiers
AvmOpcode::EMITNOTEHASH => "EMITNOTEHASH", // Notes & Nullifiers
AvmOpcode::NOTEHASHEXISTS => "NOTEHASHEXISTS", // Notes & Nullifiers
Copy link
Member Author

Choose a reason for hiding this comment

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

cargo fmt changes

AvmOpcode::EMITNOTEHASH => "EMITNOTEHASH", // Notes & Nullifiers
AvmOpcode::NULLIFIEREXISTS => "NULLIFIEREXISTS", // Notes & Nullifiers
AvmOpcode::EMITNULLIFIER => "EMITNULLIFIER", // Notes & Nullifiers
AvmOpcode::EMITNULLIFIER => "EMITNULLIFIER", // Notes & Nullifiers
AvmOpcode::READL1TOL2MSG => "READL1TOL2MSG", // Messages
AvmOpcode::HEADERMEMBER => "HEADERMEMBER", // Archive tree & Headers

Expand Down
203 changes: 190 additions & 13 deletions avm-transpiler/src/transpile.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use acvm::acir::brillig::Opcode as BrilligOpcode;
use acvm::acir::circuit::brillig::Brillig;

use acvm::brillig_vm::brillig::{BinaryFieldOp, BinaryIntOp, MemoryAddress, Value, ValueOrArray};
use acvm::brillig_vm::brillig::{
BinaryFieldOp, BinaryIntOp, BlackBoxOp, HeapArray, MemoryAddress, Value, ValueOrArray,
};

use crate::instructions::{
AvmInstruction, AvmOperand, AvmTypeTag, ALL_DIRECT, FIRST_OPERAND_INDIRECT,
Expand Down Expand Up @@ -93,7 +95,6 @@ pub fn brillig_to_avm(brillig: &Brillig) -> Vec<u8> {
value: destination.to_usize() as u32,
},
],
..Default::default()
});
}
BrilligOpcode::CalldataCopy { destination_address, size, offset } => {
Expand Down Expand Up @@ -200,9 +201,13 @@ pub fn brillig_to_avm(brillig: &Brillig) -> Vec<u8> {
..Default::default()
});
},
BrilligOpcode::Cast { destination, source, bit_size } => {
avm_instrs.push(emit_cast(source.to_usize() as u32, destination.to_usize() as u32, tag_from_bit_size(*bit_size)));
}
Comment on lines +204 to +206
Copy link
Collaborator

Choose a reason for hiding this comment

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

Didn't support cast before?

Copy link
Member Author

Choose a reason for hiding this comment

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

nope

BrilligOpcode::ForeignCall { function, destinations, inputs, destination_value_types:_, input_value_types:_ } => {
handle_foreign_call(&mut avm_instrs, function, destinations, inputs);
},
BrilligOpcode::BlackBox(operation) => handle_black_box_function(&mut avm_instrs, operation),
_ => panic!(
"Transpiler doesn't know how to process {:?} brillig instruction",
brillig_instr
Expand All @@ -228,9 +233,148 @@ fn handle_foreign_call(
function: &String,
destinations: &Vec<ValueOrArray>,
inputs: &Vec<ValueOrArray>,
) {
match function.as_str() {
"keccak256" | "sha256" => {
emit_2_field_hash_instruction(avm_instrs, function, destinations, inputs)
}
"poseidon" => {
emit_single_field_hash_instruction(avm_instrs, function, destinations, inputs)
}
_ => handle_getter_instruction(avm_instrs, function, destinations, inputs),
}
}

/// Two field hash instructions represent instruction's that's outputs are larger than a field element
///
/// This includes:
/// - keccak
/// - sha256
///
/// In the future the output of these may expand / contract depending on what is most efficient for the circuit
/// to reason about. In order to decrease user friction we will use two field outputs.
fn emit_2_field_hash_instruction(
avm_instrs: &mut Vec<AvmInstruction>,
function: &String,
destinations: &[ValueOrArray],
inputs: &[ValueOrArray],
) {
// handle field returns differently
let hash_offset_maybe = inputs[0];
println!("hash_offset_maybe: {:?}", hash_offset_maybe);
let (hash_offset, hash_size) = match hash_offset_maybe {
ValueOrArray::HeapArray(HeapArray { pointer, size }) => (pointer.0, size),
_ => panic!("Keccak | Sha256 address inputs destination should be a single value"),
};

assert!(destinations.len() == 1);
let dest_offset_maybe = destinations[0];
let dest_offset = match dest_offset_maybe {
ValueOrArray::HeapArray(HeapArray { pointer, size }) => {
assert!(size == 2);
pointer.0
}
_ => panic!("Keccak | Poseidon address destination should be a single value"),
};

let opcode = match function.as_str() {
"keccak256" => AvmOpcode::KECCAK,
"sha256" => AvmOpcode::SHA256,
_ => panic!(
"Transpiler doesn't know how to process ForeignCall function {:?}",
function
),
};

avm_instrs.push(AvmInstruction {
opcode,
indirect: Some(3), // 11 - addressing mode, indirect for input and output
operands: vec![
AvmOperand::U32 {
value: dest_offset as u32,
},
AvmOperand::U32 {
value: hash_offset as u32,
},
AvmOperand::U32 {
value: hash_size as u32,
},
],
..Default::default()
});
}

/// A single field hash instruction includes hash functions that emit a single field element
/// directly onto the stack.
///
/// This includes (snark friendly functions):
/// - poseidon2
///
/// Pedersen is not implemented this way as the black box function representation has the correct api.
/// As the Poseidon BBF only deals with a single permutation, it is not quite suitable for our current avm
/// representation.
fn emit_single_field_hash_instruction(
avm_instrs: &mut Vec<AvmInstruction>,
function: &String,
destinations: &[ValueOrArray],
inputs: &[ValueOrArray],
) {
// handle field returns differently
let hash_offset_maybe = inputs[0];
let (hash_offset, hash_size) = match hash_offset_maybe {
ValueOrArray::HeapArray(HeapArray { pointer, size }) => (pointer.0, size),
_ => panic!("Poseidon address inputs destination should be a single value"),
};

assert!(destinations.len() == 1);
let dest_offset_maybe = destinations[0];
let dest_offset = match dest_offset_maybe {
ValueOrArray::MemoryAddress(dest_offset) => dest_offset.0,
_ => panic!("Poseidon address destination should be a single value"),
};

let opcode = match function.as_str() {
"poseidon" => AvmOpcode::POSEIDON,
_ => panic!(
"Transpiler doesn't know how to process ForeignCall function {:?}",
function
),
};

avm_instrs.push(AvmInstruction {
opcode,
indirect: Some(1),
operands: vec![
AvmOperand::U32 {
value: dest_offset as u32,
},
AvmOperand::U32 {
value: hash_offset as u32,
},
AvmOperand::U32 {
value: hash_size as u32,
},
],
..Default::default()
});
}

/// Getter Instructions are instructions that take NO inputs, and return information
/// from the current execution context.
///
/// This includes:
/// - Global variables
/// - Caller
/// - storage address
/// - ...
fn handle_getter_instruction(
avm_instrs: &mut Vec<AvmInstruction>,
function: &String,
destinations: &Vec<ValueOrArray>,
inputs: &Vec<ValueOrArray>,
) {
Comment on lines +370 to 375
Copy link
Collaborator

Choose a reason for hiding this comment

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

comment, especially explaining what a getter is

// For the foreign calls we want to handle, we do not want inputs, as they are getters
assert!(inputs.len() == 0);
assert!(inputs.is_empty());
assert!(destinations.len() == 1);
let dest_offset_maybe = destinations[0];
let dest_offset = match dest_offset_maybe {
Expand All @@ -257,15 +401,14 @@ fn handle_foreign_call(
function
),
};

avm_instrs.push(AvmInstruction {
opcode,
indirect: Some(ALL_DIRECT),
operands: vec![AvmOperand::U32 {
value: dest_offset as u32,
}],
..Default::default()
});
})
}

/// Handles Brillig's CONST opcode.
Expand Down Expand Up @@ -316,7 +459,7 @@ fn emit_set(tag: AvmTypeTag, dest: u32, value: u128) -> AvmInstruction {
AvmTypeTag::UINT64 => AvmOperand::U64 {
value: value as u64,
},
AvmTypeTag::UINT128 => AvmOperand::U128 { value: value },
AvmTypeTag::UINT128 => AvmOperand::U128 { value },
_ => panic!("Invalid type tag {:?} for set", tag),
},
// dest offset
Expand All @@ -342,7 +485,7 @@ fn emit_cast(source: u32, destination: u32, dst_tag: AvmTypeTag) -> AvmInstructi
fn emit_mov(indirect: Option<u8>, source: u32, dest: u32) -> AvmInstruction {
AvmInstruction {
opcode: AvmOpcode::MOV,
indirect: indirect,
indirect,
operands: vec![
AvmOperand::U32 { value: source },
AvmOperand::U32 { value: dest },
Expand All @@ -351,6 +494,44 @@ fn emit_mov(indirect: Option<u8>, source: u32, dest: u32) -> AvmInstruction {
}
}

/// Black box functions, for the meantime only covers pedersen operations as the blackbox function api suits our current needs.
/// (array goes in -> field element comes out)
fn handle_black_box_function(avm_instrs: &mut Vec<AvmInstruction>, operation: &BlackBoxOp) {
match operation {
BlackBoxOp::PedersenHash {
inputs,
domain_separator: _,
Comment on lines +499 to +503
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as other functions (comment, rename?)

Copy link
Collaborator

Choose a reason for hiding this comment

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

This one is clearer though just by name. Simple short comment is probably fine.

output,
} => {
let hash_offset = inputs.pointer.0;
let hash_size = inputs.size.0;

let dest_offset = output.0;

avm_instrs.push(AvmInstruction {
opcode: AvmOpcode::PEDERSEN,
indirect: Some(1),
operands: vec![
AvmOperand::U32 {
value: dest_offset as u32,
},
AvmOperand::U32 {
value: hash_offset as u32,
},
AvmOperand::U32 {
value: hash_size as u32,
},
],
..Default::default()
});
}
_ => panic!(
"Transpiler doesn't know how to process BlackBoxOp {:?}",
operation
),
}
}

/// Compute an array that maps each Brillig pc to an AVM pc.
/// This must be done before transpiling to properly transpile jump destinations.
/// This is necessary for two reasons:
Expand All @@ -367,12 +548,7 @@ fn map_brillig_pcs_to_avm_pcs(initial_offset: usize, brillig: &Brillig) -> Vec<u
pc_map[0] = initial_offset;
for i in 0..brillig.bytecode.len() - 1 {
let num_avm_instrs_for_this_brillig_instr = match &brillig.bytecode[i] {
BrilligOpcode::Load { .. } => 2,
BrilligOpcode::Store { .. } => 2,
Comment on lines -370 to -371
Copy link
Collaborator

Choose a reason for hiding this comment

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

Was this a bug where these were left as 2 👀 oof that would've been a time killer debugging that

Copy link
Member Author

Choose a reason for hiding this comment

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

probably took an hour to find lmao, the avm got stuck in a loop and i couldnt work out why it was reverting

BrilligOpcode::Const { bit_size, .. } => match bit_size {
254 => 2, // Field.
_ => 1,
},
BrilligOpcode::Const { bit_size: 254, .. } => 2,
_ => 1,
};
// next Brillig pc will map to an AVM pc offset by the
Expand All @@ -384,6 +560,7 @@ fn map_brillig_pcs_to_avm_pcs(initial_offset: usize, brillig: &Brillig) -> Vec<u

fn tag_from_bit_size(bit_size: u32) -> AvmTypeTag {
match bit_size {
1 => AvmTypeTag::UINT8, // temp workaround
Copy link
Member Author

Choose a reason for hiding this comment

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

brillig did not like me not having this

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is fine. I do worry with temporary workarounds like this that some Noir code just won't work right in the AVM and we'll spend time debugging only to realize this.

Link this comment to @sirasistant's avm wishlist that mentions u1 support

8 => AvmTypeTag::UINT8,
16 => AvmTypeTag::UINT16,
32 => AvmTypeTag::UINT32,
Expand Down
1 change: 1 addition & 0 deletions noir-projects/aztec-nr/aztec/src/avm.nr
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
mod context;
mod hash;
8 changes: 8 additions & 0 deletions noir-projects/aztec-nr/aztec/src/avm/hash.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#[oracle(keccak256)]
pub fn keccak256<N>(input: [Field; N]) -> [Field; 2] {}

#[oracle(poseidon)]
pub fn poseidon<N>(input: [Field; N]) -> Field {}

#[oracle(sha256)]
pub fn sha256<N>(input: [Field; N]) -> [Field; 2] {}
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
//

contract AvmTest {
// Libs
use dep::aztec::protocol_types::address::{AztecAddress, EthAddress};

// avm lib
use dep::aztec::avm::context::AvmContext;
use dep::aztec::avm::{
context::AvmContext,
Copy link
Member Author

@Maddiaa0 Maddiaa0 Feb 14, 2024

Choose a reason for hiding this comment

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

Special opcodes for these using the oracles, this way we dont have to deal with the current refactor of these ( for now )- where just the permutation is being put in a gadget

in hash.nr

hash::{
keccak256,
poseidon,
sha256
}
};

#[aztec(private)]
fn constructor() {}
Expand Down Expand Up @@ -51,9 +56,32 @@ contract AvmTest {
200 as Field
}

/************************************************************************
* AvmContext functions
************************************************************************/
// /************************************************************************
// * Hashing functions
// ************************************************************************/
#[aztec(public-vm)]
fn keccak_hash(data: [Field; 3]) -> pub [Field; 2] {
keccak256(data)
}

#[aztec(public-vm)]
fn poseidon_hash(data: [Field; 3]) -> pub Field {
poseidon(data)
}

#[aztec(public-vm)]
fn sha256_hash(data: [Field; 3]) -> pub [Field; 2] {
sha256(data)
}

#[aztec(public-vm)]
fn pedersen_hash(data: [Field; 3]) -> pub Field {
dep::std::hash::pedersen_hash(data)
}

// /************************************************************************
// * AvmContext functions
// ************************************************************************/
#[aztec(public-vm)]
fn getAddress() -> pub AztecAddress {
context.address()
Expand Down
Loading
Loading