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): storage #4673

Merged
merged 50 commits into from
Mar 6, 2024
Merged
Show file tree
Hide file tree
Changes from 42 commits
Commits
Show all changes
50 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
7525ee8
Merge branch 'master' into md/02-16-feat_avm_storage
Maddiaa0 Mar 4, 2024
4997ce4
exp
Maddiaa0 Mar 4, 2024
1ad0307
merge 2: electric boogaloo
Maddiaa0 Mar 4, 2024
d534d2d
fmt
Maddiaa0 Mar 4, 2024
85d7ce8
fix: allow memory mem tags to be less than or equal
Maddiaa0 Mar 4, 2024
1bde64b
chore: remove noir logs
Maddiaa0 Mar 4, 2024
ccd689f
chore: cleanup ts comments
Maddiaa0 Mar 4, 2024
4916061
Merge branch 'master' into md/02-16-feat_avm_storage
Maddiaa0 Mar 4, 2024
b90675d
Merge branch 'master' into md/02-16-feat_avm_storage
Maddiaa0 Mar 6, 2024
84670d6
tidy
Maddiaa0 Mar 6, 2024
332f333
sweep
Maddiaa0 Mar 6, 2024
957d0bd
tag name update
Maddiaa0 Mar 6, 2024
d0386d4
fix: lint, tag not used
Maddiaa0 Mar 6, 2024
51f8570
death and taxes
Maddiaa0 Mar 6, 2024
3499714
fmt
Maddiaa0 Mar 6, 2024
78de9b7
fix test
Maddiaa0 Mar 6, 2024
6cdd047
fix: test
Maddiaa0 Mar 6, 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 = ZEROTH_OPERAND_INDIRECT | FIRST_OPERAND_INDIRECT;

/// A simple representation of an AVM instruction for the purpose
Expand Down
92 changes: 88 additions & 4 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 @@ -257,8 +257,10 @@ fn handle_foreign_call(
"avmOpcodePoseidon" => {
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),
fcarreiro marked this conversation as resolved.
Show resolved Hide resolved
// Getters.
_ if inputs.len() == 0 && destinations.len() == 1 => {
_ if inputs.is_empty() && destinations.len() == 1 => {
handle_getter_instruction(avm_instrs, function, destinations, inputs)
}
// Anything else.
Expand Down Expand Up @@ -361,7 +363,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 @@ -390,6 +392,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
fcarreiro marked this conversation as resolved.
Show resolved Hide resolved
assert!(inputs.len() == 2);
assert!(destinations.len() == 1); // TODO: we want this to be empty - change aztec nr?
fcarreiro marked this conversation as resolved.
Show resolved Hide resolved

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 Expand Up @@ -483,7 +567,7 @@ fn handle_send_l2_to_l1_msg(
destinations: &Vec<ValueOrArray>,
inputs: &Vec<ValueOrArray>,
) {
if destinations.len() != 0 || inputs.len() != 2 {
if !destinations.is_empty() || inputs.len() != 2 {
panic!(
"Transpiler expects ForeignCall::SENDL2TOL1MSG to have 0 destinations and 2 inputs, got {} and {}",
destinations.len(),
Expand Down
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 @@ -14,18 +14,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
@@ -1,5 +1,6 @@
contract AvmTest {
// Libs
use dep::aztec::state_vars::PublicMutable;
use dep::aztec::protocol_types::{address::{AztecAddress, EthAddress}, constants::L1_TO_L2_MESSAGE_LENGTH};
use dep::compressed_string::CompressedString;

Expand All @@ -9,7 +10,21 @@ contract AvmTest {
#[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 @@ -41,7 +56,6 @@ contract AvmTest {
// fn setOpcodeUint128() -> pub u128 {
// 1 << 120 as u128
// }

fcarreiro marked this conversation as resolved.
Show resolved Hide resolved
// 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
8 changes: 7 additions & 1 deletion noir/noir-repo/aztec_macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -727,8 +727,14 @@ fn transform_function(
/// Transform a function to work with AVM bytecode
fn transform_vm_function(
func: &mut NoirFunction,
_storage_defined: bool,
storage_defined: bool,
) -> Result<(), AztecMacroError> {
// Create access to storage
if storage_defined {
let storage = abstract_storage("public_vm", true);
func.def.body.0.insert(0, storage);
}

// Push Avm context creation to the beginning of the function
let create_context = create_avm_context()?;
func.def.body.0.insert(0, create_context);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use num_bigint::BigUint;
/// The Brillig VM does not apply a limit to the memory address space,
/// As a convention, we take use 64 bits. This means that we assume that
/// memory has 2^64 memory slots.
pub(crate) const BRILLIG_MEMORY_ADDRESSING_BIT_SIZE: u32 = 32;
pub(crate) const BRILLIG_MEMORY_ADDRESSING_BIT_SIZE: u32 = 64;

// Registers reserved in runtime for special purposes.
pub(crate) enum ReservedRegisters {
Expand Down Expand Up @@ -562,6 +562,7 @@ impl BrilligContext {
bit_size: u32,
) {
self.debug_show.const_instruction(result, constant);

self.push_opcode(BrilligOpcode::Const { destination: result, value: constant, bit_size });
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[package]
name = "should_fail_with_mismatch"
type = "bin"
authors = [""]
[dependencies]
6 changes: 6 additions & 0 deletions yarn-project/simulator/src/avm/avm_memory_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,12 @@ export class TaggedMemory {
}
}

public checkTagLessThanEqual(tag: TypeTag, offset: number) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider having checkIsValidMemoryOffsetTag() or something like that, which we can then just use everywhere we need to check for mem.

Copy link
Member Author

Choose a reason for hiding this comment

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

in replace of this or in addition?

if (this.getTag(offset) > tag) {
throw TagCheckError.forOffset(offset, TypeTag[this.getTag(offset)], TypeTag[tag]);
}
}

public static checkIsIntegralTag(tag: TypeTag) {
if (![TypeTag.UINT8, TypeTag.UINT16, TypeTag.UINT32, TypeTag.UINT64, TypeTag.UINT128].includes(tag)) {
throw TagCheckError.forTag(TypeTag[tag], 'integral');
Expand Down
62 changes: 62 additions & 0 deletions yarn-project/simulator/src/avm/avm_simulator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,68 @@ describe('AVM simulator', () => {
});
});

describe('Storage accesses', () => {
it('Should set a single value in storage', async () => {
// We want to set
const calldata: Fr[] = [];
// We are setting the owner
const slot = 1n;
const sender = AztecAddress.fromField(new Fr(1));
const address = AztecAddress.fromField(new Fr(420));

// Get contract function artifact
const artifact = AvmTestContractArtifact.functions.find(f => f.name === 'avm_setAdmin')!;
fcarreiro marked this conversation as resolved.
Show resolved Hide resolved

// Decode bytecode into instructions
const bytecode = Buffer.from(artifact.bytecode, 'base64');

const context = initContext({
env: initExecutionEnvironment({ calldata, sender, address, storageAddress: address }),
fcarreiro marked this conversation as resolved.
Show resolved Hide resolved
});
jest
.spyOn(context.persistableState.hostStorage.contractsDb, 'getBytecode')
.mockReturnValue(Promise.resolve(bytecode));

const simulator = new AvmSimulator(context);
const results = await simulator.execute();
expect(results.reverted).toBe(false);

// Contract 420 - Storage slot 1 should contain the value 1
const worldState = context.persistableState.flush();

const storageSlot = worldState.currentStorageValue.get(address.toBigInt())!;
const adminSlotValue = storageSlot.get(slot)!;
fcarreiro marked this conversation as resolved.
Show resolved Hide resolved
expect(adminSlotValue).toEqual(sender.toField());
fcarreiro marked this conversation as resolved.
Show resolved Hide resolved
});

it('Should read a value from storage', async () => {
const calldata: Fr[] = [];
// We are setting the owner
const sender = AztecAddress.fromField(new Fr(1));
const address = AztecAddress.fromField(new Fr(420));

// Get contract function artifact
const artifact = AvmTestContractArtifact.functions.find(f => f.name === 'avm_setAndRead')!;
fcarreiro marked this conversation as resolved.
Show resolved Hide resolved

// Decode bytecode into instructions
const bytecode = Buffer.from(artifact.bytecode, 'base64');

const context = initContext({
env: initExecutionEnvironment({ calldata, sender, address, storageAddress: address }),
fcarreiro marked this conversation as resolved.
Show resolved Hide resolved
});
jest
.spyOn(context.persistableState.hostStorage.contractsDb, 'getBytecode')
.mockReturnValue(Promise.resolve(bytecode));

const simulator = new AvmSimulator(context);
const results = await simulator.execute();
expect(results.reverted).toBe(false);

const returnData = results.output;
expect(returnData[0]).toEqual(sender.toField());
fcarreiro marked this conversation as resolved.
Show resolved Hide resolved
});
});

describe('Test env getters from noir contract', () => {
const testEnvGetter = async (valueName: string, value: any, functionName: string, globalVar: boolean = false) => {
// Execute
Expand Down
10 changes: 5 additions & 5 deletions yarn-project/simulator/src/avm/journal/journal.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ describe('journal', () => {
expect(cacheMissResult).toEqual(storedValue);

// Write to storage
journal.writeStorage(contractAddress, key, cachedValue);
journal.writeStorage(contractAddress, key, [cachedValue]);

// Get the storage value
const cachedResult = await journal.readStorage(contractAddress, key);
Expand Down Expand Up @@ -157,7 +157,7 @@ describe('journal', () => {
const index = new Fr(42);
const indexT1 = new Fr(24);

journal.writeStorage(contractAddress, key, value);
journal.writeStorage(contractAddress, key, [value]);
await journal.readStorage(contractAddress, key);
journal.writeNoteHash(commitment);
journal.writeLog(new Fr(log.address), new Fr(log.selector), log.data);
Expand All @@ -167,7 +167,7 @@ describe('journal', () => {
await journal.checkL1ToL2MessageExists(commitment, index);

const childJournal = new AvmPersistableStateManager(journal.hostStorage, journal);
childJournal.writeStorage(contractAddress, key, valueT1);
childJournal.writeStorage(contractAddress, key, [valueT1]);
await childJournal.readStorage(contractAddress, key);
childJournal.writeNoteHash(commitmentT1);
childJournal.writeLog(new Fr(logT1.address), new Fr(logT1.selector), logT1.data);
Expand Down Expand Up @@ -246,7 +246,7 @@ describe('journal', () => {
const index = new Fr(42);
const indexT1 = new Fr(24);

journal.writeStorage(contractAddress, key, value);
journal.writeStorage(contractAddress, key, [value]);
await journal.readStorage(contractAddress, key);
journal.writeNoteHash(commitment);
await journal.writeNullifier(contractAddress, commitment);
Expand All @@ -256,7 +256,7 @@ describe('journal', () => {
journal.writeL1Message(recipient, commitment);

const childJournal = new AvmPersistableStateManager(journal.hostStorage, journal);
childJournal.writeStorage(contractAddress, key, valueT1);
childJournal.writeStorage(contractAddress, key, [valueT1]);
await childJournal.readStorage(contractAddress, key);
childJournal.writeNoteHash(commitmentT1);
await childJournal.writeNullifier(contractAddress, commitmentT1);
Expand Down
6 changes: 3 additions & 3 deletions yarn-project/simulator/src/avm/journal/journal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,11 @@ export class AvmPersistableStateManager {
* @param slot - the slot in the contract's storage being written to
* @param value - the value being written to the slot
*/
public writeStorage(storageAddress: Fr, slot: Fr, value: Fr) {
public writeStorage(storageAddress: Fr, slot: Fr, values: /*temporarily an array*/ Fr[]) {
// Cache storage writes for later reference/reads
this.publicStorage.write(storageAddress, slot, value);
this.publicStorage.write(storageAddress, slot, values);
// Trace all storage writes (even reverted ones)
this.trace.tracePublicStorageWrite(storageAddress, slot, value);
this.trace.tracePublicStorageWrite(storageAddress, slot, values);
}

/**
Expand Down
Loading
Loading