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): take sizeOffset in CALL #5763

Merged
merged 1 commit into from
Apr 16, 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
38 changes: 25 additions & 13 deletions avm-transpiler/src/transpile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use acvm::acir::brillig::Opcode as BrilligOpcode;
use acvm::acir::circuit::brillig::Brillig;

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

Expand Down Expand Up @@ -380,16 +380,16 @@ fn handle_external_call(
inputs: &Vec<ValueOrArray>,
opcode: AvmOpcode,
) {
if destinations.len() != 2 || inputs.len() != 4 {
if destinations.len() != 2 || inputs.len() != 5 {
panic!(
"Transpiler expects ForeignCall (Static)Call to have 2 destinations and 4 inputs, got {} and {}.
"Transpiler expects ForeignCall (Static)Call to have 2 destinations and 5 inputs, got {} and {}.
Make sure your call instructions's input/return arrays have static length (`[Field; <size>]`)!",
destinations.len(),
inputs.len()
);
}
let gas_offset_maybe = inputs[0];
let gas_offset = match gas_offset_maybe {
let gas = inputs[0];
let gas_offset = match gas {
ValueOrArray::HeapArray(HeapArray { pointer, size }) => {
assert!(size == 3, "Call instruction's gas input should be a HeapArray of size 3 (`[l1Gas, l2Gas, daGas]`)");
pointer.0 as u32
Expand All @@ -401,13 +401,16 @@ fn handle_external_call(
ValueOrArray::MemoryAddress(offset) => offset.to_usize() as u32,
_ => panic!("Call instruction's target address input should be a basic MemoryAddress",),
};
let args_offset_maybe = inputs[2];
let (args_offset, args_size) = match args_offset_maybe {
ValueOrArray::HeapArray(HeapArray { pointer, size }) => (pointer.0 as u32, size as u32),
ValueOrArray::HeapVector(_) => panic!("Call instruction's args must be a HeapArray, not a HeapVector. Make sure you are explicitly defining its size (`[arg0, arg1, ... argN]`)!"),
_ => panic!("Call instruction's args input should be a HeapArray input"),
// The args are a slice, and this is represented as a (Field, HeapVector).
// The field is the length (memory address) and the HeapVector has the data and length again.
// This is an ACIR internal representation detail that leaks to the SSA.
// Observe that below, we use `inputs[3]` and therefore skip the length field.
let args = inputs[3];
let (args_offset, args_size_offset) = match args {
ValueOrArray::HeapVector(HeapVector { pointer, size }) => (pointer.0 as u32, size.0 as u32),
_ => panic!("Call instruction's args input should be a HeapVector input"),
};
let temporary_function_selector_offset = match &inputs[3] {
let temporary_function_selector_offset = match &inputs[4] {
ValueOrArray::MemoryAddress(offset) => offset.to_usize() as u32,
_ => panic!(
"Call instruction's temporary function selector input should be a basic MemoryAddress",
Expand All @@ -426,14 +429,23 @@ fn handle_external_call(
};
avm_instrs.push(AvmInstruction {
opcode: opcode,
indirect: Some(0b01101), // (left to right) selector direct, ret offset INDIRECT, args offset INDIRECT, address offset direct, gas offset INDIRECT
// (left to right)
// * selector direct
// * ret offset INDIRECT
// * arg size offset direct
// * args offset INDIRECT
// * address offset direct
// * gas offset INDIRECT
indirect: Some(0b010101),
operands: vec![
AvmOperand::U32 { value: gas_offset },
AvmOperand::U32 {
value: address_offset,
},
AvmOperand::U32 { value: args_offset },
AvmOperand::U32 { value: args_size },
AvmOperand::U32 {
value: args_size_offset,
},
AvmOperand::U32 { value: ret_offset },
AvmOperand::U32 { value: ret_size },
AvmOperand::U32 {
Expand Down
6 changes: 3 additions & 3 deletions docs/docs/protocol-specs/public-vm/gen/_instruction-set.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -1700,7 +1700,7 @@ Call into another contract
- **gasOffset**: offset to three words containing `{l1GasLeft, l2GasLeft, daGasLeft}`: amount of gas to provide to the callee
- **addrOffset**: address of the contract to call
- **argsOffset**: memory offset to args (will become the callee's calldata)
- **argsSize**: number of words to pass via callee's calldata
- **argsSizeOffset**: memory offset for the number of words to pass via callee's calldata
- **retOffset**: destination memory offset specifying where to store the data returned from the callee
- **retSize**: number of words to copy from data returned by callee
- **successOffset**: destination memory offset specifying where to store the call's success (0: failure, 1: success)
Expand Down Expand Up @@ -1748,7 +1748,7 @@ Call into another contract, disallowing World State and Accrued Substate modific
- **gasOffset**: offset to three words containing `{l1GasLeft, l2GasLeft, daGasLeft}`: amount of gas to provide to the callee
- **addrOffset**: address of the contract to call
- **argsOffset**: memory offset to args (will become the callee's calldata)
- **argsSize**: number of words to pass via callee's calldata
- **argsSizeOffset**: memory offset for the number of words to pass via callee's calldata
- **retOffset**: destination memory offset specifying where to store the data returned from the callee
- **retSize**: number of words to copy from data returned by callee
- **successOffset**: destination memory offset specifying where to store the call's success (0: failure, 1: success)
Expand Down Expand Up @@ -1793,7 +1793,7 @@ Call into another contract, but keep the caller's `sender` and `storageAddress`
- **gasOffset**: offset to three words containing `{l1GasLeft, l2GasLeft, daGasLeft}`: amount of gas to provide to the callee
- **addrOffset**: address of the contract to call
- **argsOffset**: memory offset to args (will become the callee's calldata)
- **argsSize**: number of words to pass via callee's calldata
- **argsSizeOffset**: memory offset for the number of words to pass via callee's calldata
- **retOffset**: destination memory offset specifying where to store the data returned from the callee
- **retSize**: number of words to copy from data returned by callee
- **successOffset**: destination memory offset specifying where to store the call's success (0: failure, 1: success)
Expand Down
7 changes: 3 additions & 4 deletions docs/src/preprocess/InstructionSet/InstructionSet.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,9 @@ const CALL_INSTRUCTION_ARGS = [
description: "memory offset to args (will become the callee's calldata)",
},
{
name: "argsSize",
description: "number of words to pass via callee's calldata",
mode: "immediate",
type: "u32",
name: "argsSizeOffset",
description:
"memory offset for the number of words to pass via callee's calldata",
},
{
name: "retOffset",
Expand Down
16 changes: 8 additions & 8 deletions noir-projects/aztec-nr/aztec/src/context/avm_context.nr
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ impl AvmContext {
call(
gas_for_call(gas),
contract_address,
args,
args.as_slice(),
temporary_function_selector
)
}
Expand All @@ -78,7 +78,7 @@ impl AvmContext {
call_static(
gas_for_call(gas),
contract_address,
args,
args.as_slice(),
temporary_function_selector
)
}
Expand Down Expand Up @@ -157,7 +157,7 @@ impl PublicContextInterface for AvmContext {
let results = call(
gas_for_call(gas_opts),
contract_address,
args,
args.as_slice(),
temporary_function_selector.to_field()
);
let data_to_return: [Field; RETURNS_COUNT] = results.0;
Expand All @@ -177,7 +177,7 @@ impl PublicContextInterface for AvmContext {
let (data_to_return, success): ([Field; RETURNS_COUNT], u8) = call_static(
gas_for_call(gas_opts),
contract_address,
args,
args.as_slice(),
temporary_function_selector.to_field()
);

Expand Down Expand Up @@ -308,20 +308,20 @@ fn l1_to_l2_msg_exists(msg_hash: Field, msg_leaf_index: Field) -> u8 {}
fn send_l2_to_l1_msg(recipient: EthAddress, content: Field) {}

#[oracle(avmOpcodeCall)]
fn call<ARGS_COUNT, RET_SIZE>(
fn call<RET_SIZE>(
gas: [Field; 3], // gas allocation: [l1_gas, l2_gas, da_gas]
address: AztecAddress,
args: [Field; ARGS_COUNT],
args: [Field],
// TODO(5110): consider passing in calldata directly
temporary_function_selector: Field
) -> ([Field; RET_SIZE], u8) {}
// ^ return data ^ success

#[oracle(avmOpcodeStaticCall)]
fn call_static<ARGS_COUNT, RET_SIZE>(
fn call_static<RET_SIZE>(
gas: [Field; 3], // gas allocation: [l1_gas, l2_gas, da_gas]
address: AztecAddress,
args: [Field; ARGS_COUNT],
args: [Field],
// TODO(5110): consider passing in calldata directly
temporary_function_selector: Field
) -> ([Field; RET_SIZE], u8) {}
Expand Down
22 changes: 14 additions & 8 deletions yarn-project/simulator/src/avm/opcodes/external_calls.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { mock } from 'jest-mock-extended';
import { type CommitmentsDB, type PublicContractsDB, type PublicStateDB } from '../../index.js';
import { markBytecodeAsAvm } from '../../public/transitional_adaptors.js';
import { type AvmContext } from '../avm_context.js';
import { Field, Uint8 } from '../avm_memory_types.js';
import { Field, Uint8, Uint32 } from '../avm_memory_types.js';
import { adjustCalldataIndex, initContext } from '../fixtures/index.js';
import { HostStorage } from '../journal/host_storage.js';
import { AvmPersistableStateManager } from '../journal/journal.js';
Expand Down Expand Up @@ -36,7 +36,7 @@ describe('External Calls', () => {
...Buffer.from('12345678', 'hex'), // gasOffset
...Buffer.from('a2345678', 'hex'), // addrOffset
...Buffer.from('b2345678', 'hex'), // argsOffset
...Buffer.from('c2345678', 'hex'), // argsSize
...Buffer.from('c2345678', 'hex'), // argsSizeOffset
...Buffer.from('d2345678', 'hex'), // retOffset
...Buffer.from('e2345678', 'hex'), // retSize
...Buffer.from('f2345678', 'hex'), // successOffset
Expand All @@ -47,7 +47,7 @@ describe('External Calls', () => {
/*gasOffset=*/ 0x12345678,
/*addrOffset=*/ 0xa2345678,
/*argsOffset=*/ 0xb2345678,
/*argsSize=*/ 0xc2345678,
/*argsSizeOffset=*/ 0xc2345678,
/*retOffset=*/ 0xd2345678,
/*retSize=*/ 0xe2345678,
/*successOffset=*/ 0xf2345678,
Expand All @@ -68,6 +68,7 @@ describe('External Calls', () => {
const argsOffset = 4;
const args = [new Field(1n), new Field(2n), new Field(3n)];
const argsSize = args.length;
const argsSizeOffset = 20;
const retOffset = 8;
const retSize = 2;
const successOffset = 7;
Expand All @@ -93,6 +94,7 @@ describe('External Calls', () => {
context.machineState.memory.set(1, new Field(l2Gas));
context.machineState.memory.set(2, new Field(daGas));
context.machineState.memory.set(3, new Field(addr));
context.machineState.memory.set(argsSizeOffset, new Uint32(argsSize));
context.machineState.memory.setSlice(4, args);
jest
.spyOn(context.persistableState.hostStorage.contractsDb, 'getBytecode')
Expand All @@ -103,7 +105,7 @@ describe('External Calls', () => {
gasOffset,
addrOffset,
argsOffset,
argsSize,
argsSizeOffset,
retOffset,
retSize,
successOffset,
Expand Down Expand Up @@ -145,6 +147,7 @@ describe('External Calls', () => {
const argsOffset = 4;
const args = [new Field(1n), new Field(2n), new Field(3n)];
const argsSize = args.length;
const argsSizeOffset = 20;
const retOffset = 8;
const retSize = 2;
const successOffset = 7;
Expand All @@ -153,6 +156,7 @@ describe('External Calls', () => {
context.machineState.memory.set(1, new Field(l2Gas));
context.machineState.memory.set(2, new Field(daGas));
context.machineState.memory.set(3, new Field(addr));
context.machineState.memory.set(argsSizeOffset, new Uint32(argsSize));
context.machineState.memory.setSlice(4, args);

jest
Expand All @@ -164,7 +168,7 @@ describe('External Calls', () => {
gasOffset,
addrOffset,
argsOffset,
argsSize,
argsSizeOffset,
retOffset,
retSize,
successOffset,
Expand All @@ -183,7 +187,7 @@ describe('External Calls', () => {
...Buffer.from('12345678', 'hex'), // gasOffset
...Buffer.from('a2345678', 'hex'), // addrOffset
...Buffer.from('b2345678', 'hex'), // argsOffset
...Buffer.from('c2345678', 'hex'), // argsSize
...Buffer.from('c2345678', 'hex'), // argsSizeOffset
...Buffer.from('d2345678', 'hex'), // retOffset
...Buffer.from('e2345678', 'hex'), // retSize
...Buffer.from('f2345678', 'hex'), // successOffset
Expand All @@ -194,7 +198,7 @@ describe('External Calls', () => {
/*gasOffset=*/ 0x12345678,
/*addrOffset=*/ 0xa2345678,
/*argsOffset=*/ 0xb2345678,
/*argsSize=*/ 0xc2345678,
/*argsSizeOffset=*/ 0xc2345678,
/*retOffset=*/ 0xd2345678,
/*retSize=*/ 0xe2345678,
/*successOffset=*/ 0xf2345678,
Expand All @@ -214,12 +218,14 @@ describe('External Calls', () => {
const args = [new Field(1n), new Field(2n), new Field(3n)];

const argsSize = args.length;
const argsSizeOffset = 20;
const retOffset = 8;
const retSize = 2;
const successOffset = 7;

context.machineState.memory.set(0, gas);
context.machineState.memory.set(1, addr);
context.machineState.memory.set(argsSizeOffset, new Uint32(argsSize));
context.machineState.memory.setSlice(2, args);

const otherContextInstructions: Instruction[] = [
Expand All @@ -243,7 +249,7 @@ describe('External Calls', () => {
gasOffset,
addrOffset,
argsOffset,
argsSize,
argsSizeOffset,
retOffset,
retSize,
successOffset,
Expand Down
13 changes: 8 additions & 5 deletions yarn-project/simulator/src/avm/opcodes/external_calls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ abstract class ExternalCall extends Instruction {
private gasOffset: number /* Unused due to no formal gas implementation at this moment */,
private addrOffset: number,
private argsOffset: number,
private argsSize: number,
private argsSizeOffset: number,
private retOffset: number,
private retSize: number,
private successOffset: number,
Expand All @@ -50,20 +50,23 @@ abstract class ExternalCall extends Instruction {

public async execute(context: AvmContext) {
const memory = context.machineState.memory.track(this.type);
const [gasOffset, addrOffset, argsOffset, retOffset, successOffset] = Addressing.fromWire(this.indirect).resolve(
[this.gasOffset, this.addrOffset, this.argsOffset, this.retOffset, this.successOffset],
const [gasOffset, addrOffset, argsOffset, argsSizeOffset, retOffset, successOffset] = Addressing.fromWire(
this.indirect,
).resolve(
[this.gasOffset, this.addrOffset, this.argsOffset, this.argsSizeOffset, this.retOffset, this.successOffset],
memory,
);

const callAddress = memory.getAs<Field>(addrOffset);
const calldata = memory.getSlice(argsOffset, this.argsSize).map(f => f.toFr());
const calldataSize = memory.get(argsSizeOffset).toNumber();
const calldata = memory.getSlice(argsOffset, calldataSize).map(f => f.toFr());
const l1Gas = memory.get(gasOffset).toNumber();
const l2Gas = memory.getAs<Field>(gasOffset + 1).toNumber();
const daGas = memory.getAs<Field>(gasOffset + 2).toNumber();
const functionSelector = memory.getAs<Field>(this.temporaryFunctionSelectorOffset).toFr();

const allocatedGas = { l1Gas, l2Gas, daGas };
const memoryOperations = { reads: this.argsSize + 5, writes: 1 + this.retSize, indirect: this.indirect };
const memoryOperations = { reads: calldataSize + 6, writes: 1 + this.retSize, indirect: this.indirect };
const totalGas = sumGas(this.gasCost(memoryOperations), allocatedGas);
context.machineState.consumeGas(totalGas);

Expand Down
Loading