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): basic AVM-ACVM interoperability #5595

Merged
merged 3 commits into from
Apr 9, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion avm-transpiler/src/instructions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ 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
/// of generating an AVM bytecode from Brillig.
Expand Down
3 changes: 0 additions & 3 deletions avm-transpiler/src/transpile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,6 @@ pub fn brillig_to_avm(brillig: &Brillig) -> Vec<u8> {
BinaryIntOp::Xor => AvmOpcode::XOR,
BinaryIntOp::Shl => AvmOpcode::SHL,
BinaryIntOp::Shr => AvmOpcode::SHR,
_ => panic!(
"Transpiler doesn't know how to process {:?}", brillig_instr
),
};
avm_instrs.push(AvmInstruction {
opcode: avm_opcode,
Expand Down
1 change: 0 additions & 1 deletion avm-transpiler/src/transpile_contract.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use base64::Engine;
use log::info;
use regex::Regex;
use serde::{Deserialize, Serialize};

use acvm::acir::circuit::Program;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@ contract AvmTest {
// Libs
use dep::aztec::prelude::Map;
use dep::aztec::state_vars::PublicMutable;
use dep::aztec::history::nullifier_inclusion::prove_nullifier_inclusion;
use dep::aztec::protocol_types::{
address::{AztecAddress, EthAddress}, constants::L1_TO_L2_MESSAGE_LENGTH,
contract_instance::ContractInstance
contract_instance::ContractInstance, hash::silo_nullifier
};
use dep::aztec::context::gas::GasOpts;
use dep::aztec::oracle::get_contract_instance::{get_contract_instance_avm, get_contract_instance_internal_avm};
Expand Down Expand Up @@ -175,6 +176,58 @@ contract AvmTest {
dep::std::hash::pedersen_hash_with_separator(data, 20)
}

/************************************************************************
* ACVM interoperability
************************************************************************/
#[aztec(public)]
fn constant_field_acvm() -> pub Field {
123456
}

#[aztec(public-vm)]
fn constant_field_avm() -> pub Field {
123456
}

#[aztec(public)]
fn new_nullifier_acvm(nullifier: Field) -> pub Field {
context.push_new_nullifier(nullifier, 0);
}

#[aztec(public)]
fn assert_unsiloed_nullifier_acvm(nullifier: Field) {
// ACVM requires siloed nullifier.
let siloed_nullifier = silo_nullifier(context.this_address(), nullifier);
prove_nullifier_inclusion(siloed_nullifier, context);
}

#[aztec(public-vm)]
fn call_acvm_from_avm() -> pub Field {
let data_to_return: [Field; RETURN_VALUES_LENGTH] = context.call_public_function(
context.this_address(),
FunctionSelector::from_signature("constant_field_acvm()"),
[],
GasOpts::default()
);
data_to_return[0]
}

#[aztec(public)]
fn call_avm_from_acvm() -> pub Field {
let data_to_return: [Field; RETURN_VALUES_LENGTH] = context.call_public_function(
context.this_address(),
FunctionSelector::from_signature("constant_field_avm()"),
[],
GasOpts::default()
);
data_to_return[0]
}

#[aztec(public-vm)]
fn avm_to_acvm_call(selector: FunctionSelector, args: Field) {
context.call_public_function(context.this_address(), selector, [args], GasOpts::default());
}

/************************************************************************
* Contract instance
************************************************************************/
Expand Down
31 changes: 30 additions & 1 deletion yarn-project/end-to-end/src/e2e_avm_simulator.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { AztecAddress, Fr, TxStatus, type Wallet } from '@aztec/aztec.js';
import { AztecAddress, Fr, FunctionSelector, TxStatus, type Wallet } from '@aztec/aztec.js';
import { AvmInitializerTestContract, AvmTestContract } from '@aztec/noir-contracts.js';

import { jest } from '@jest/globals';
Expand Down Expand Up @@ -64,6 +64,35 @@ describe('e2e_avm_simulator', () => {
expect(tx.status).toEqual(TxStatus.MINED);
});
});

describe('ACVM interoperability', () => {
it('Can execute ACVM function among AVM functions', async () => {
expect(await avmContract.methods.constant_field_acvm().simulate()).toEqual([123456n, 0n, 0n, 0n]);
});

it('Can call AVM function from ACVM', async () => {
expect(await avmContract.methods.call_avm_from_acvm().simulate()).toEqual([123456n, 0n, 0n, 0n]);
});

it('Can call ACVM function from AVM', async () => {
expect(await avmContract.methods.call_acvm_from_avm().simulate()).toEqual([123456n, 0n, 0n, 0n]);
});

it('AVM sees settled nullifiers by ACVM', async () => {
const nullifier = new Fr(123456);
await avmContract.methods.new_nullifier(nullifier).send().wait();
await avmContract.methods.assert_unsiloed_nullifier_acvm(nullifier).send().wait();
});

it('AVM nested call to ACVM sees settled nullifiers', async () => {
const nullifier = new Fr(123456);
await avmContract.methods.new_nullifier(nullifier).send().wait();
await avmContract.methods
.avm_to_acvm_call(FunctionSelector.fromSignature('assert_unsiloed_nullifier_acvm(Field)'), nullifier)
.send()
.wait();
});
});
});

describe('AvmInitializerTestContract', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,8 +230,8 @@ export abstract class AbstractPhaseManager {
while (executionStack.length) {
const current = executionStack.pop()!;
const isExecutionRequest = !isPublicExecutionResult(current);

const sideEffectCounter = lastSideEffectCounter(tx) + 1;

const result = isExecutionRequest
? await this.publicExecutor.simulate(current, this.globalVariables, sideEffectCounter)
: current;
Expand Down
26 changes: 9 additions & 17 deletions yarn-project/simulator/src/avm/avm_execution_environment.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { FunctionSelector, type GlobalVariables } from '@aztec/circuits.js';
import { FunctionSelector, type GlobalVariables, type Header } from '@aztec/circuits.js';
import { computeVarArgsHash } from '@aztec/circuits.js/hash';
import { type AztecAddress } from '@aztec/foundation/aztec-address';
import { type EthAddress } from '@aztec/foundation/eth-address';
Expand All @@ -22,29 +22,18 @@ export class AvmContextInputs {
export class AvmExecutionEnvironment {
constructor(
public readonly address: AztecAddress,

public readonly storageAddress: AztecAddress,

public readonly origin: AztecAddress,

public readonly sender: AztecAddress,

public readonly portal: EthAddress,

public readonly feePerL1Gas: Fr,

public readonly feePerL2Gas: Fr,

public readonly feePerDaGas: Fr,

public readonly contractCallDepth: Fr,

public readonly header: Header,
public readonly globals: GlobalVariables,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we might eventually want opcodes to directly get certain entries from this header (different from HEADERMEMBER which gets historic info)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we can do all of it from DB at that point. This was the easy way out for now :)
BTW, when I discussed with Lasse, he thought that HEADERMEMBER only made sense w/the latest header, because all the rest you can do in private.


public readonly isStaticCall: boolean,

public readonly isDelegateCall: boolean,

public readonly calldata: Fr[],

// Function selector is temporary since eventually public contract bytecode will be one blob
Expand All @@ -59,20 +48,21 @@ export class AvmExecutionEnvironment {
}

public deriveEnvironmentForNestedCall(
address: AztecAddress,
targetAddress: AztecAddress,
calldata: Fr[],
temporaryFunctionSelector: FunctionSelector = FunctionSelector.empty(),
): AvmExecutionEnvironment {
return new AvmExecutionEnvironment(
address,
/*storageAddress=*/ address,
targetAddress,
/*storageAddress=*/ targetAddress,
this.origin,
this.sender,
this.address,
this.portal,
this.feePerL1Gas,
this.feePerL2Gas,
this.feePerDaGas,
this.contractCallDepth,
this.header,
this.globals,
this.isStaticCall,
this.isDelegateCall,
Expand All @@ -96,6 +86,7 @@ export class AvmExecutionEnvironment {
this.feePerL2Gas,
this.feePerDaGas,
this.contractCallDepth,
this.header,
this.globals,
/*isStaticCall=*/ true,
this.isDelegateCall,
Expand All @@ -119,6 +110,7 @@ export class AvmExecutionEnvironment {
this.feePerL2Gas,
this.feePerDaGas,
this.contractCallDepth,
this.header,
this.globals,
this.isStaticCall,
/*isDelegateCall=*/ true,
Expand Down