Skip to content

Commit

Permalink
chore(avm-simulator): remove AvmContext::raw_* external calls (#5869)
Browse files Browse the repository at this point in the history 
I introduced raw and "old interface" methods in the AvmContext when I was less familiar with Aztec-nr. I now think we only should have the one supporting the old interface behaviour. Once we successfully transition, we can change those as much as we want.
  • Loading branch information
@fcarreiro
fcarreiro authored Apr 19, 2024
1 parent c999dae commit 0c9d0b4
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 150 deletions.
30 changes: 0 additions & 30 deletions noir-projects/aztec-nr/aztec/src/context/avm_context.nr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,36 +50,6 @@ impl AvmContext {
pub fn l1_to_l2_msg_exists(self, msg_hash: Field, msg_leaf_index: Field) -> bool {
l1_to_l2_msg_exists(msg_hash, msg_leaf_index) == 1
}

fn call_public_function_raw<RET_COUNT>(
self: &mut Self,
gas: GasOpts,
contract_address: AztecAddress,
temporary_function_selector: Field,
args: [Field]
) -> ([Field; RET_COUNT], u8) {
call(
gas_for_call(gas),
contract_address,
args,
temporary_function_selector
)
}

fn static_call_public_function_raw<RET_COUNT>(
self: &mut Self,
gas: GasOpts,
contract_address: AztecAddress,
temporary_function_selector: Field,
args: [Field]
) -> ([Field; RET_COUNT], u8) {
call_static(
gas_for_call(gas),
contract_address,
args,
temporary_function_selector
)
}
}

impl PublicContextInterface for AvmContext {
Expand Down
75 changes: 3 additions & 72 deletions noir-projects/noir-contracts/contracts/avm_nested_calls_test_contract/src/main.nr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,54 +32,16 @@ contract AvmNestedCallsTest {
context.push_new_nullifier(nullifier, 0);
}

// Directly call the external call opcode to initiate a nested call to the add function
// External call opcode to initiate a nested call to the add function with user-specified gas
#[aztec(public-vm)]
fn raw_nested_call_to_add(arg_a: Field, arg_b: Field) -> pub Field {
let selector = FunctionSelector::from_signature("add_args_return(Field,Field)").to_field();

// Nested call
let results = context.call_public_function_raw(
GasOpts::default(),
context.this_address(),
selector,
[arg_a, arg_b].as_slice()
);
let data_to_return: [Field; 1] = results.0;
// this explicit size is necessary to ensure that ret_size is compile-time
// (ensure the data_to_return is in a HeapArray not a HeapVector)
let success: u8 = results.1;

assert(success == 1, "Call failed");

let add_result = data_to_return[0];
add_result
}

// Directly call the external call opcode to initiate a nested call to the add function with user-specified gas
#[aztec(public-vm)]
fn raw_nested_call_to_add_with_gas(
fn nested_call_to_add_with_gas(
arg_a: Field,
arg_b: Field,
l1_gas: Field,
l2_gas: Field,
da_gas: Field
) -> pub Field {
let selector = FunctionSelector::from_signature("add_args_return(Field,Field)").to_field();

// Nested call
let results = context.call_public_function_raw(
GasOpts::new(l1_gas, l2_gas, da_gas),
context.this_address(),
selector,
[arg_a, arg_b].as_slice()
);
let data_to_return: [Field; 1] = results.0;
// this explicit size is necessary to ensure that ret_size is compile-time
// (ensure the data_to_return is in a HeapArray not a HeapVector)
let _success: u8 = results.1;

let add_result = data_to_return[0];
add_result
AvmNestedCallsTest::at(context.this_address()).add_args_return(arg_a, arg_b).call(&mut context, GasOpts::new(l1_gas, l2_gas, da_gas))
}

// Use the `call_public_function` wrapper to initiate a nested call to the add function
Expand All @@ -88,37 +50,6 @@ contract AvmNestedCallsTest {
AvmNestedCallsTest::at(context.this_address()).add_args_return(arg_a, arg_b).call(&mut context, GasOpts::default())
}

// Directly call_static the external call opcode to initiate a nested call to the add function
#[aztec(public-vm)]
fn raw_nested_static_call_to_add(arg_a: Field, arg_b: Field) -> pub (Field, u8) {
let selector = FunctionSelector::from_signature("add_args_return(Field,Field)").to_field();

let (result_data, success): ([Field; 1], u8) = context.static_call_public_function_raw(
GasOpts::default(),
context.this_address(),
selector,
[arg_a, arg_b].as_slice()
);

(result_data[0], success)
}

// Directly call_static `set_storage_single`. Should fail since it's accessing storage.
#[aztec(public-vm)]
fn raw_nested_static_call_to_set_storage() -> pub u8 {
let selector = FunctionSelector::from_signature("set_storage_single(Field)").to_field();
let calldata: [Field; 1] = [20];

let (_data_to_return, success): ([Field; 0], u8) = context.static_call_public_function_raw(
GasOpts::default(),
context.this_address(),
selector,
calldata.as_slice()
);

success
}

// Indirectly call_static the external call opcode to initiate a nested call to the add function
#[aztec(public-vm)]
fn nested_static_call_to_add(arg_a: Field, arg_b: Field) -> pub Field {
Expand Down
4 changes: 4 additions & 0 deletions yarn-project/end-to-end/src/e2e_avm_simulator.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ describe('e2e_avm_simulator', () => {
.send()
.wait();
});

describe('Authwit', () => {
it('Works if authwit provided', async () => {
const recipient = AztecAddress.random();
Expand Down Expand Up @@ -167,6 +168,7 @@ describe('e2e_avm_simulator', () => {
avmContract.methods.create_same_nullifier_in_nested_call(avmContract.address, nullifier).send().wait(),
).rejects.toThrow();
});

it('Should be able to emit different unsiloed nullifiers from the same contract', async () => {
const nullifier = new Fr(1);
const tx = await avmContract.methods
Expand All @@ -175,6 +177,7 @@ describe('e2e_avm_simulator', () => {
.wait();
expect(tx.status).toEqual(TxStatus.MINED);
});

// TODO(4293): this should work! Fails in public kernel because both nullifiers are incorrectly being siloed by same address
it.skip('Should be able to emit the same unsiloed nullifier from two different contracts', async () => {
const nullifier = new Fr(1);
Expand All @@ -184,6 +187,7 @@ describe('e2e_avm_simulator', () => {
.wait();
expect(tx.status).toEqual(TxStatus.MINED);
});

it('Should be able to emit different unsiloed nullifiers from two different contracts', async () => {
const nullifier = new Fr(1);
const tx = await avmContract.methods
Expand Down
51 changes: 3 additions & 48 deletions yarn-project/simulator/src/avm/avm_simulator.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -787,27 +787,11 @@ describe('AVM simulator: transpiled Noir contracts', () => {
});

describe('Nested external calls', () => {
it(`Nested call succeeds`, async () => {
const calldata: Fr[] = [new Fr(1), new Fr(2)];
const callBytecode = getAvmNestedCallsTestContractBytecode('raw_nested_call_to_add');
const addBytecode = getAvmNestedCallsTestContractBytecode('add_args_return');
const context = initContext({ env: initExecutionEnvironment({ calldata }) });
jest
.spyOn(context.persistableState.hostStorage.contractsDb, 'getBytecode')
.mockReturnValue(Promise.resolve(addBytecode));

const results = await new AvmSimulator(context).executeBytecode(callBytecode);

expect(results.revertReason).toBeUndefined();
expect(results.reverted).toBe(false);
expect(results.output).toEqual([new Fr(3)]);
});

// TODO(https://github.com/AztecProtocol/aztec-packages/issues/5625): gas not plumbed through correctly in nested calls.
// it(`Nested call with not enough gas`, async () => {
// const gas = [/*l1=*/ 10000, /*l2=*/ 20, /*da=*/ 10000].map(g => new Fr(g));
// const calldata: Fr[] = [new Fr(1), new Fr(2), ...gas];
// const callBytecode = getAvmNestedCallsTestContractBytecode('raw_nested_call_to_add_with_gas');
// const callBytecode = getAvmNestedCallsTestContractBytecode('nested_call_to_add_with_gas');
// const addBytecode = getAvmNestedCallsTestContractBytecode('add_args_return');
// const context = initContext({ env: initExecutionEnvironment({ calldata }) });
// jest
Expand All @@ -822,7 +806,7 @@ describe('AVM simulator: transpiled Noir contracts', () => {
// expect(results.output).toEqual([new Fr(0)]);
// });

it(`Nested call through the old interface`, async () => {
it(`Nested call`, async () => {
const calldata: Fr[] = [new Fr(1), new Fr(2)];
const callBytecode = getAvmNestedCallsTestContractBytecode('nested_call_to_add');
const addBytecode = getAvmNestedCallsTestContractBytecode('add_args_return');
Expand All @@ -838,35 +822,6 @@ describe('AVM simulator: transpiled Noir contracts', () => {
});

it(`Nested static call`, async () => {
const calldata: Fr[] = [new Fr(1), new Fr(2)];
const callBytecode = getAvmNestedCallsTestContractBytecode('raw_nested_static_call_to_add');
const addBytecode = getAvmNestedCallsTestContractBytecode('add_args_return');
const context = initContext({ env: initExecutionEnvironment({ calldata }) });
jest
.spyOn(context.persistableState.hostStorage.contractsDb, 'getBytecode')
.mockReturnValue(Promise.resolve(addBytecode));

const results = await new AvmSimulator(context).executeBytecode(callBytecode);

expect(results.reverted).toBe(false);
expect(results.output).toEqual([/*result=*/ new Fr(3), /*success=*/ new Fr(1)]);
});

it(`Nested static call which modifies storage`, async () => {
const callBytecode = getAvmNestedCallsTestContractBytecode('raw_nested_static_call_to_set_storage');
const nestedBytecode = getAvmNestedCallsTestContractBytecode('set_storage_single');
const context = initContext();
jest
.spyOn(context.persistableState.hostStorage.contractsDb, 'getBytecode')
.mockReturnValue(Promise.resolve(nestedBytecode));

const results = await new AvmSimulator(context).executeBytecode(callBytecode);

expect(results.reverted).toBe(false); // The outer call should not revert.
expect(results.output).toEqual([new Fr(0)]); // The inner call should have reverted.
});

it(`Nested static call (old interface)`, async () => {
const calldata: Fr[] = [new Fr(1), new Fr(2)];
const callBytecode = getAvmNestedCallsTestContractBytecode('nested_static_call_to_add');
const addBytecode = getAvmNestedCallsTestContractBytecode('add_args_return');
Expand All @@ -881,7 +836,7 @@ describe('AVM simulator: transpiled Noir contracts', () => {
expect(results.output).toEqual([/*result=*/ new Fr(3)]);
});

it(`Nested static call which modifies storage (old interface)`, async () => {
it(`Nested static call which modifies storage`, async () => {
const callBytecode = getAvmNestedCallsTestContractBytecode('nested_static_call_to_set_storage');
const nestedBytecode = getAvmNestedCallsTestContractBytecode('set_storage_single');
const context = initContext();
Expand Down

0 comments on commit 0c9d0b4

Please sign in to comment.