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.

Sign up for GitHub

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

fix: Preserve public function call ordering in account entrypoint #2348

Merged
merged 1 commit into from
Sep 18, 2023
Merged

fix: Preserve public function call ordering in account entrypoint #2348

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
98 changes: 45 additions & 53 deletions yarn-project/aztec-nr/aztec/src/entrypoint.nr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,33 +7,45 @@ use crate::constants_gen::GENERATOR_INDEX__SIGNATURE_PAYLOAD;

use dep::std::hash;

global ACCOUNT_MAX_PRIVATE_CALLS: Field = 2;
global ACCOUNT_MAX_PUBLIC_CALLS: Field = 2;
global ACCOUNT_MAX_CALLS: Field = 4;
// 1 (ARGS_HASH) + 1 (FUNCTION_SELECTOR) + 1 (TARGET_ADDRESS)
global FUNCTION_CALL_SIZE: Field = 3;
// 1 (ARGS_HASH) + 1 (FUNCTION_SELECTOR) + 1 (TARGET_ADDRESS) + 1 (IS_PUBLIC)
global FUNCTION_CALL_SIZE: Field = 4;
// 3 * 32 + 1
global FUNCTION_CALL_SIZE_IN_BYTES: Field = 97;

struct FunctionCall {
args_hash: Field,
function_selector: Field,
target_address: Field,
is_public: bool,
}

impl FunctionCall {
fn serialize(self) -> [Field; FUNCTION_CALL_SIZE] {
[self.args_hash, self.function_selector, self.target_address]
[self.args_hash, self.function_selector, self.target_address, self.is_public as Field]
}

fn to_be_bytes(self) -> [u8; FUNCTION_CALL_SIZE_IN_BYTES] {
let mut bytes: [u8; FUNCTION_CALL_SIZE_IN_BYTES] = [0; FUNCTION_CALL_SIZE_IN_BYTES];
let args_hash_bytes = self.args_hash.to_be_bytes(32);
for i in 0..32 { bytes[i] = args_hash_bytes[i]; }
let function_selector_bytes = self.function_selector.to_be_bytes(32);
for i in 0..32 { bytes[i + 32] = function_selector_bytes[i]; }
let target_address_bytes = self.target_address.to_be_bytes(32);
for i in 0..32 { bytes[i + 64] = target_address_bytes[i]; }
bytes[96] = self.is_public as u8;
bytes
}
}

// FUNCTION_CALL_SIZE * (ACCOUNT_MAX_PUBLIC_CALLS + ACCOUNT_MAX_PRIVATE_CALLS) + 1
global ENTRYPOINT_PAYLOAD_SIZE: Field = 13;
global ENTRYPOINT_PAYLOAD_SIZE_IN_BYTES: Field = 416;
// FUNCTION_CALL_SIZE * ACCOUNT_MAX_CALLS + 1
global ENTRYPOINT_PAYLOAD_SIZE: Field = 17;
// FUNCTION_CALL_SIZE_IN_BYTES * ACCOUNT_MAX_CALLS + 32
global ENTRYPOINT_PAYLOAD_SIZE_IN_BYTES: Field = 420;

// docs:start:entrypoint-struct
struct EntrypointPayload {
flattened_args_hashes: [Field; ACCOUNT_MAX_CALLS],
flattened_selectors: [Field; ACCOUNT_MAX_CALLS],
flattened_targets: [Field; ACCOUNT_MAX_CALLS],
function_calls: [FunctionCall; ACCOUNT_MAX_CALLS],
nonce: Field,
}
// docs:end:entrypoint-struct
Expand All @@ -49,9 +61,9 @@ impl EntrypointPayload {
// Serializes the entrypoint struct
fn serialize(self) -> [Field; ENTRYPOINT_PAYLOAD_SIZE] {
let mut fields: BoundedVec<Field, ENTRYPOINT_PAYLOAD_SIZE> = BoundedVec::new(0);
fields.push_array(self.flattened_args_hashes);
fields.push_array(self.flattened_selectors);
fields.push_array(self.flattened_targets);
for call in self.function_calls {
fields.push_array(call.serialize());
}
fields.push(self.nonce);
fields.storage
}
Expand All @@ -60,57 +72,37 @@ impl EntrypointPayload {
fn to_be_bytes(self) -> [u8; ENTRYPOINT_PAYLOAD_SIZE_IN_BYTES] {
let mut bytes: [u8; ENTRYPOINT_PAYLOAD_SIZE_IN_BYTES] = [0; ENTRYPOINT_PAYLOAD_SIZE_IN_BYTES];

let args_len = self.flattened_args_hashes.len();
let selectors_len = self.flattened_selectors.len();
let targets_len = self.flattened_targets.len();

for i in 0..args_len {
let item_bytes = self.flattened_args_hashes[i].to_be_bytes(32);
for j in 0..32 {
bytes[i * 32 + j] = item_bytes[j];
}
}

for i in 0..selectors_len {
let item_bytes = self.flattened_selectors[i].to_be_bytes(32);
for j in 0..32 {
bytes[args_len * 32 + i * 32 + j] = item_bytes[j];
for i in 0..ACCOUNT_MAX_CALLS {
let item_bytes = self.function_calls[i].to_be_bytes();
for j in 0..FUNCTION_CALL_SIZE_IN_BYTES {
bytes[i * FUNCTION_CALL_SIZE_IN_BYTES + j] = item_bytes[j];
}
}

for i in 0..targets_len {
let item_bytes = self.flattened_targets[i].to_be_bytes(32);
for j in 0..32 {
bytes[(args_len + selectors_len) * 32 + i * 32 + j] = item_bytes[j];
}
let nonce_bytes = self.nonce.to_be_bytes(32);
let nonce_offset = FUNCTION_CALL_SIZE_IN_BYTES * ACCOUNT_MAX_CALLS;
for j in 0..32 {
bytes[nonce_offset + j] = nonce_bytes[j];
}

let item_bytes = self.nonce.to_be_bytes(32);
for j in 0..32 {
bytes[(args_len + selectors_len + targets_len) * 32 + j] = item_bytes[j];
}

bytes
}

// Executes all private and public calls
// docs:start:entrypoint-execute-calls
fn execute_calls(self, context: &mut PrivateContext) {
for i in 0..ACCOUNT_MAX_PRIVATE_CALLS {
let target_address = self.flattened_targets[i];
if target_address != 0 {
let function_selector = self.flattened_selectors[i];
let args_hash = self.flattened_args_hashes[i];
let _callStackItem = context.call_private_function_with_packed_args(target_address, function_selector, args_hash);
}
}
for i in ACCOUNT_MAX_PRIVATE_CALLS..ACCOUNT_MAX_CALLS {
let target_address = self.flattened_targets[i];
if target_address != 0 {
let function_selector = self.flattened_selectors[i];
let args_hash = self.flattened_args_hashes[i];
let _callStackItem = context.call_public_function_with_packed_args(target_address, function_selector, args_hash);
for call in self.function_calls {
if call.target_address != 0 {
if call.is_public {
context.call_public_function_with_packed_args(
call.target_address, call.function_selector, call.args_hash
);
} else {
let _result = context.call_private_function_with_packed_args(
call.target_address, call.function_selector, call.args_hash
);
}
}
}
}
// docs:end:entrypoint-execute-calls
Expand Down
53 changes: 30 additions & 23 deletions yarn-project/aztec.js/src/abis/ecdsa_account_contract.json
View file
Open in desktop

Large diffs are not rendered by default.

53 changes: 30 additions & 23 deletions yarn-project/aztec.js/src/abis/schnorr_account_contract.json
View file
Open in desktop

Large diffs are not rendered by default.

53 changes: 30 additions & 23 deletions yarn-project/aztec.js/src/abis/schnorr_single_key_account_contract.json
View file
Open in desktop

Large diffs are not rendered by default.

51 changes: 29 additions & 22 deletions yarn-project/aztec.js/src/account/defaults/default_entrypoint.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,32 +49,39 @@ export class DefaultAccountEntrypoint implements EntrypointInterface {
path: 'aztec::entrypoint::EntrypointPayload',
fields: [
{
name: 'flattened_args_hashes',
name: 'function_calls',
type: {
kind: 'array',
length: 4,
type: {
kind: 'field',
},
},
},
{
name: 'flattened_selectors',
type: {
kind: 'array',
length: 4,
type: {
kind: 'field',
},
},
},
{
name: 'flattened_targets',
type: {
kind: 'array',
length: 4,
type: {
kind: 'field',
kind: 'struct',
path: 'aztec::entrypoint::FunctionCall',
fields: [
{
name: 'args_hash',
type: {
kind: 'field',
},
},
{
name: 'function_selector',
type: {
kind: 'field',
},
},
{
name: 'target_address',
type: {
kind: 'field',
},
},
{
name: 'is_public',
type: {
kind: 'boolean',
},
},
],
},
},
},
Expand Down
71 changes: 40 additions & 31 deletions yarn-project/aztec.js/src/account/defaults/entrypoint_payload.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,30 @@ import { pedersenPlookupCompressWithHashIndex } from '@aztec/circuits.js/barrete
import { padArrayEnd } from '@aztec/foundation/collection';
import { FunctionCall, PackedArguments, emptyFunctionCall } from '@aztec/types';

import partition from 'lodash.partition';

// These must match the values defined in yarn-project/aztec-nr/aztec/src/entrypoint.nr
export const ACCOUNT_MAX_PRIVATE_CALLS = 2;
export const ACCOUNT_MAX_PUBLIC_CALLS = 2;
export const ACCOUNT_MAX_CALLS = 4;

/** Encoded payload for the account contract entrypoint */
export type EntrypointPayload = {
/** Encoded function call for account contract entrypoint */
export type EntrypointFunctionCall = {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is nice 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep, it was long due since Noir started supporting nested structs.

// eslint-disable-next-line camelcase
/** Arguments hash for the call */
args_hash: Fr;
// eslint-disable-next-line camelcase
/** Concatenated arguments for every call */
flattened_args_hashes: Fr[];
/** Selector of the function to call */
function_selector: Fr;
// eslint-disable-next-line camelcase
/** Concatenated selectors for every call */
flattened_selectors: Fr[];
/** Address of the contract to call */
target_address: Fr;
// eslint-disable-next-line camelcase
/** Whether the function is public or private */
is_public: boolean;
};

/** Encoded payload for the account contract entrypoint */
export type EntrypointPayload = {
// eslint-disable-next-line camelcase
/** Concatenated target addresses for every call */
flattened_targets: Fr[];
/** Encoded function calls to execute */
function_calls: EntrypointFunctionCall[];
/** A nonce for replay protection */
nonce: Fr;
};
Expand All @@ -33,28 +40,27 @@ export async function buildPayload(calls: FunctionCall[]): Promise<{
}> {
const nonce = Fr.random();

const [privateCalls, publicCalls] = partition(calls, call => call.functionData.isPrivate);

const paddedCalls = [
...padArrayEnd(privateCalls, emptyFunctionCall(), ACCOUNT_MAX_PRIVATE_CALLS),
...padArrayEnd(publicCalls, emptyFunctionCall(), ACCOUNT_MAX_PUBLIC_CALLS),
];

const packedArguments = [];
const wasm = await CircuitsWasm.get();

const paddedCalls = padArrayEnd(calls, emptyFunctionCall(), ACCOUNT_MAX_CALLS);
const packedArguments: PackedArguments[] = [];
for (const call of paddedCalls) {
packedArguments.push(await PackedArguments.fromArgs(call.args, wasm));
packedArguments.push(await PackedArguments.fromArgs(call.args));
}

const formattedCalls: EntrypointFunctionCall[] = paddedCalls.map((call, index) => ({
// eslint-disable-next-line camelcase
args_hash: packedArguments[index].hash,
// eslint-disable-next-line camelcase
function_selector: call.functionData.selector.toField(),
// eslint-disable-next-line camelcase
target_address: call.to.toField(),
// eslint-disable-next-line camelcase
is_public: !call.functionData.isPrivate,
}));

return {
payload: {
// eslint-disable-next-line camelcase
flattened_args_hashes: packedArguments.map(args => args.hash),
// eslint-disable-next-line camelcase
flattened_selectors: paddedCalls.map(call => call.functionData.selector.toField()),
// eslint-disable-next-line camelcase
flattened_targets: paddedCalls.map(call => call.to.toField()),
function_calls: formattedCalls,
nonce,
},
packedArguments,
Expand All @@ -73,9 +79,12 @@ export async function hashPayload(payload: EntrypointPayload) {
/** Flattens an entrypoint payload */
export function flattenPayload(payload: EntrypointPayload) {
return [
...payload.flattened_args_hashes,
...payload.flattened_selectors,
...payload.flattened_targets,
...payload.function_calls.flatMap(call => [
call.args_hash,
call.function_selector,
call.target_address,
new Fr(call.is_public),
]),
payload.nonce,
];
}
22 changes: 20 additions & 2 deletions yarn-project/end-to-end/src/e2e_nested_contract.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { AztecNodeService } from '@aztec/aztec-node';
import { AztecRPCServer } from '@aztec/aztec-rpc';
import { AztecAddress, Fr, Wallet } from '@aztec/aztec.js';
import { AztecAddress, BatchCall, Fr, Wallet } from '@aztec/aztec.js';
import { toBigIntBE } from '@aztec/foundation/bigint-buffer';
import { DebugLogger } from '@aztec/foundation/log';
import { toBigInt } from '@aztec/foundation/serialize';
import { ChildContract, ImportTestContract, ParentContract, TestContract } from '@aztec/noir-contracts/types';
import { AztecRPC } from '@aztec/types';
import { AztecRPC, L2BlockL2Logs } from '@aztec/types';

import { setup } from './fixtures/utils.js';

Expand Down Expand Up @@ -107,6 +108,23 @@ describe('e2e_nested_contract', () => {
.wait();
expect(await getChildStoredValue(childContract)).toEqual(84n);
}, 100_000);

// Regression for https://github.com/AztecProtocol/aztec-packages/issues/1645
// Executes a public call first and then a private call (which enqueues another public call)
// through the account contract, if the account entrypoint behaves properly, it will honor
// this order and not run the private call first which results in the public calls being inverted.
it('executes public calls in expected order', async () => {
const pubSetValueSelector = childContract.methods.pubSetValue.selector.toField();
const actions = [
childContract.methods.pubSetValue(20n).request(),
parentContract.methods.enqueueCallToChild(childContract.address, pubSetValueSelector, 40n).request(),
];

const tx = await new BatchCall(wallet, actions).send().wait();
const logs = L2BlockL2Logs.unrollLogs(await wallet.getUnencryptedLogs(tx.blockNumber!, 1)).map(toBigIntBE);
expect(logs).toEqual([20n, 40n]);
expect(await getChildStoredValue(childContract)).toEqual(40n);
});
});

describe('importer uses autogenerated test contract interface', () => {
Expand Down
5 changes: 3 additions & 2 deletions yarn-project/foundation/src/fields/fields.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ export class Fr {
*/
public readonly value: bigint;

constructor(value: bigint | number | Fr | AztecAddress) {
const isFr = (value: bigint | number | Fr | AztecAddress): value is Fr | AztecAddress => !!(value as Fr).toBigInt;
constructor(value: boolean | bigint | number | Fr | AztecAddress) {
const isFr = (value: boolean | bigint | number | Fr | AztecAddress): value is Fr | AztecAddress =>
!!(value as Fr).toBigInt;
this.value = isFr(value) ? value.toBigInt() : BigInt(value);
if (this.value > Fr.MAX_VALUE) {
throw new Error(`Fr out of range ${value}.`);
Expand Down