Skip to content

Commit

Permalink
chore: enable skipped ordering tests since AVM properly updates side-…
Browse files Browse the repository at this point in the history
…effect counter for nested calls
  • Loading branch information
dbanks12 committed Jun 17, 2024
1 parent fcbd44b commit c9ff021
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 32 deletions.
1 change: 0 additions & 1 deletion noir-projects/aztec-nr/aztec/src/oracle/public_call.nr
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use dep::protocol_types::{abis::function_selector::FunctionSelector, address::AztecAddress};
// TODO(6052): get new side effect counter from this call
#[oracle(callPublicFunction)]
unconstrained fn call_public_function_oracle<RETURNS_COUNT>(
_contract_address: AztecAddress,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,6 @@ contract Child {
}

#[aztec(public)]
// TODO(6052): The logs emitted are currently in the wrong order as we don't update
// counters for nested public calls
fn set_value_with_two_nested_calls() {
Child::at(context.this_address()).set_value_twice_with_nested_first().call(&mut context);
Child::at(context.this_address()).set_value_twice_with_nested_last().call(&mut context);
Expand Down
50 changes: 21 additions & 29 deletions yarn-project/end-to-end/src/e2e_ordering.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,42 +92,34 @@ describe('e2e_ordering', () => {
const expectedOrders = {
set_value_twice_with_nested_first: [nestedValue, directValue] as bigint[], // eslint-disable-line camelcase
set_value_twice_with_nested_last: [directValue, nestedValue] as bigint[], // eslint-disable-line camelcase
// TODO(6052)
// set_value_with_two_nested_calls: [nestedValue, directValue, directValue, nestedValue, directValue] as bigint[], // eslint-disable-line camelcase
set_value_with_two_nested_calls: [nestedValue, directValue, directValue, nestedValue, directValue] as bigint[], // eslint-disable-line camelcase
} as const;

// TODO(6052): Once resolved, add 'set_value_with_nested_calls'
it.each(['set_value_twice_with_nested_first', 'set_value_twice_with_nested_last'] as const)(
'orders public state updates in %s (and ensures final state value is correct)',
async method => {
const expectedOrder = expectedOrders[method];
it.each([
'set_value_twice_with_nested_first',
'set_value_twice_with_nested_last',
'set_value_with_two_nested_calls',
] as const)('orders public state updates in %s (and ensures final state value is correct)', async method => {
const expectedOrder = expectedOrders[method];

await child.methods[method]().send().wait();
await child.methods[method]().send().wait();

const value = await pxe.getPublicStorageAt(child.address, new Fr(1));
expect(value.value).toBe(expectedOrder[expectedOrder.length - 1]); // final state should match last value set
},
);
const value = await pxe.getPublicStorageAt(child.address, new Fr(1));
expect(value.value).toBe(expectedOrder[expectedOrder.length - 1]); // final state should match last value set
});

// TODO(#838): Public kernel outputs logs in wrong order!
// Full explanation:
// Emitting logs twice (first in a nested call, then directly) leads
// to a misordering of them by the public kernel because it sees them
// in reverse order. More info in this thread: https://discourse.aztec.network/t/identifying-the-ordering-of-state-access-across-contract-calls/382/12#transition-counters-for-private-calls-2
// The below only works due to a hack which sorts the logs in ts
// See tail_phase_manager.ts
// TODO(6052): Once resolved, add 'set_value_with_two_nested_calls'
it.each(['set_value_twice_with_nested_first', 'set_value_twice_with_nested_last'] as const)(
'orders unencrypted logs in %s',
async method => {
const expectedOrder = expectedOrders[method];
it.each([
'set_value_twice_with_nested_first',
'set_value_twice_with_nested_last',
'set_value_with_two_nested_calls',
] as const)('orders unencrypted logs in %s', async method => {
const expectedOrder = expectedOrders[method];

await child.methods[method]().send().wait();
await child.methods[method]().send().wait();

// Logs are emitted in the expected order
await expectLogsFromLastBlockToBe(expectedOrder);
},
);
// Logs are emitted in the expected order
await expectLogsFromLastBlockToBe(expectedOrder);
});
});
});
});

0 comments on commit c9ff021

Please sign in to comment.