From bbb69314a9628612716111d4034740c2a58f00dc Mon Sep 17 00:00:00 2001 From: cheethas Date: Tue, 18 Apr 2023 14:43:26 +0000 Subject: [PATCH 1/9] fix: update function selector to be 4 bytes --- circuits/cpp/src/aztec3/circuits/abis/c_bind.test.cpp | 2 +- .../cpp/src/aztec3/circuits/abis/function_leaf_preimage.hpp | 3 ++- circuits/cpp/src/aztec3/circuits/hash.hpp | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/circuits/cpp/src/aztec3/circuits/abis/c_bind.test.cpp b/circuits/cpp/src/aztec3/circuits/abis/c_bind.test.cpp index 364c92bab2d0..1ee832d871e3 100644 --- a/circuits/cpp/src/aztec3/circuits/abis/c_bind.test.cpp +++ b/circuits/cpp/src/aztec3/circuits/abis/c_bind.test.cpp @@ -140,7 +140,7 @@ TEST(abi_tests, compute_function_leaf) { // Construct FunctionLeafPreimage with some randomized fields FunctionLeafPreimage preimage = FunctionLeafPreimage{ - .function_selector = NT::fr::random_element(), + .function_selector = engine.get_random_uint32(), .is_private = static_cast(engine.get_random_uint8() & 1), .vk_hash = NT::fr::random_element(), .acir_hash = NT::fr::random_element(), diff --git a/circuits/cpp/src/aztec3/circuits/abis/function_leaf_preimage.hpp b/circuits/cpp/src/aztec3/circuits/abis/function_leaf_preimage.hpp index 1963518c7730..04498db2a7fb 100644 --- a/circuits/cpp/src/aztec3/circuits/abis/function_leaf_preimage.hpp +++ b/circuits/cpp/src/aztec3/circuits/abis/function_leaf_preimage.hpp @@ -29,8 +29,9 @@ template struct FunctionLeafPreimage { typedef typename NCT::boolean boolean; typedef typename NCT::fr fr; + typedef typename NCT::uint32 uint32; - fr function_selector = 0; + uint32 function_selector = 0; boolean is_private = false; fr vk_hash = 0; fr acir_hash = 0; diff --git a/circuits/cpp/src/aztec3/circuits/hash.hpp b/circuits/cpp/src/aztec3/circuits/hash.hpp index c5a722c6d8b6..fcfde09f1745 100644 --- a/circuits/cpp/src/aztec3/circuits/hash.hpp +++ b/circuits/cpp/src/aztec3/circuits/hash.hpp @@ -126,7 +126,7 @@ typename NCT::fr root_from_sibling_path(typename NCT::fr const& leaf, */ template typename NCT::fr function_tree_root_from_siblings( - typename NCT::fr const& function_selector, + typename NCT::uint32 const& function_selector, typename NCT::boolean const& is_private, typename NCT::fr const& vk_hash, typename NCT::fr const& acir_hash, From eb524956c84298c3c1d277c28266361affdf8f49 Mon Sep 17 00:00:00 2001 From: cheethas Date: Tue, 18 Apr 2023 14:48:03 +0000 Subject: [PATCH 2/9] fix: update snapshot --- .../__snapshots__/function_leaf_preimage.test.ts.snap | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/yarn-project/circuits.js/src/structs/__snapshots__/function_leaf_preimage.test.ts.snap b/yarn-project/circuits.js/src/structs/__snapshots__/function_leaf_preimage.test.ts.snap index 9c0600d4b4dc..6b82b90ec0de 100644 --- a/yarn-project/circuits.js/src/structs/__snapshots__/function_leaf_preimage.test.ts.snap +++ b/yarn-project/circuits.js/src/structs/__snapshots__/function_leaf_preimage.test.ts.snap @@ -1,9 +1,9 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`basic FunctionLeafPreimage serialization serializes a trivial Function Leaf Preimage and prints it 1`] = ` -"function_selector: 0x7b01000000000000000000000000000000000000000000000000000000 -is_private: 0 +"function_selector: 123 +is_private: 1 vk_hash: 0x0 -acir_hash: 0x31000000082f1200f8e21100000000000000000000 +acir_hash: 0x0 " `; From a1c6ca027843e25fc5081cb827d0efb8f73aeb46 Mon Sep 17 00:00:00 2001 From: spypsy Date: Tue, 18 Apr 2023 16:13:15 +0000 Subject: [PATCH 3/9] use LeafPreimage class instead of Buffer --- .../aztec-rpc/src/contract_tree/contract_tree.ts | 10 +++++++--- yarn-project/circuits.js/src/abis/abis.ts | 16 +++++++++++----- yarn-project/circuits.js/src/structs/index.ts | 1 + 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/yarn-project/aztec-rpc/src/contract_tree/contract_tree.ts b/yarn-project/aztec-rpc/src/contract_tree/contract_tree.ts index 49d717a860fe..28a3aadf0593 100644 --- a/yarn-project/aztec-rpc/src/contract_tree/contract_tree.ts +++ b/yarn-project/aztec-rpc/src/contract_tree/contract_tree.ts @@ -3,6 +3,7 @@ import { CONTRACT_TREE_HEIGHT, FUNCTION_TREE_HEIGHT, FunctionData, + FunctionLeafPreimage, MembershipWitness, NewContractData, computeFunctionTree, @@ -50,10 +51,13 @@ async function generateFunctionLeaves(functions: ContractFunctionDao[], wasm: Ci const vkHash = await hashVKStr(f.verificationKey!, wasm); const acirHash = keccak(Buffer.from(f.bytecode, 'hex')); // TODO: selector is currently padded to 32 bytes in CBINDS, check this. - const fnLeaf = await computeFunctionLeaf( - wasm, - Buffer.concat([selector, Buffer.alloc(28, 0), Buffer.from([isPrivate ? 1 : 0]), vkHash, acirHash]), + const fnLeafPreimage = new FunctionLeafPreimage( + selector, + isPrivate, + Fr.fromBuffer(vkHash), + Fr.fromBuffer(acirHash), ); + const fnLeaf = await computeFunctionLeaf(wasm, fnLeafPreimage); result.push(fnLeaf); } return result; diff --git a/yarn-project/circuits.js/src/abis/abis.ts b/yarn-project/circuits.js/src/abis/abis.ts index e428aa69f108..caafc4d5c855 100644 --- a/yarn-project/circuits.js/src/abis/abis.ts +++ b/yarn-project/circuits.js/src/abis/abis.ts @@ -1,7 +1,13 @@ import { Buffer } from 'buffer'; import { AztecAddress, Fr } from '@aztec/foundation'; import { CircuitsWasm } from '../wasm/index.js'; -import { FunctionData, FUNCTION_SELECTOR_NUM_BYTES, TxRequest, NewContractData } from '../index.js'; +import { + FunctionData, + FUNCTION_SELECTOR_NUM_BYTES, + TxRequest, + NewContractData, + FunctionLeafPreimage, +} from '../index.js'; import { serializeToBuffer } from '../utils/serialize.js'; import { AsyncWasmWrapper, WasmWrapper } from '@aztec/foundation/wasm'; @@ -81,11 +87,11 @@ export async function hashVK(wasm: CircuitsWasm, vkBuf: Buffer) { return await wasmAsyncCall(wasm, 'abis__hash_vk', { toBuffer: () => vkBuf }, 32); } -export async function computeFunctionLeaf(wasm: CircuitsWasm, fnLeaf: Buffer) { - // Size must match circuits/cpp/src/aztec3/circuits/abis/function_leaf_preimage.hpp - if (fnLeaf.length !== 32 + 1 + 32 + 32) throw new Error(`Invalid length for function leaf`); +export async function computeFunctionLeaf(wasm: CircuitsWasm, fnLeaf: FunctionLeafPreimage) { + const fnLeafBuf = fnLeaf.toBuffer(); + if (fnLeafBuf.length !== 32 + 1 + 32 + 32) throw new Error(`Invalid length for function leaf`); wasm.call('pedersen__init'); - return Fr.fromBuffer(await wasmAsyncCall(wasm, 'abis__compute_function_leaf', { toBuffer: () => fnLeaf }, 32)); + return Fr.fromBuffer(await wasmAsyncCall(wasm, 'abis__compute_function_leaf', { toBuffer: () => fnLeafBuf }, 32)); } export async function computeFunctionTreeRoot(wasm: CircuitsWasm, fnLeafs: Fr[]) { diff --git a/yarn-project/circuits.js/src/structs/index.ts b/yarn-project/circuits.js/src/structs/index.ts index de6540632900..ab751bd425f5 100644 --- a/yarn-project/circuits.js/src/structs/index.ts +++ b/yarn-project/circuits.js/src/structs/index.ts @@ -11,4 +11,5 @@ export * from './shared.js'; export * from './tx.js'; export * from './verification_key.js'; export * from './private_call_stack_item.js'; +export * from './function_leaf_preimage.js'; export { Fr, Fq, AztecAddress, EthAddress } from '@aztec/foundation'; From 4f554396309091223bf279f0f569adc92c923b32 Mon Sep 17 00:00:00 2001 From: spypsy Date: Tue, 18 Apr 2023 16:39:58 +0000 Subject: [PATCH 4/9] use FunctionLeafPreimage class in tests + verifyBufferSize as static fn --- .../circuits.js/src/abis/__snapshots__/abis.test.ts.snap | 2 +- yarn-project/circuits.js/src/abis/abis.test.ts | 4 ++-- yarn-project/circuits.js/src/abis/abis.ts | 2 +- .../circuits.js/src/structs/function_leaf_preimage.ts | 4 ++++ 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/yarn-project/circuits.js/src/abis/__snapshots__/abis.test.ts.snap b/yarn-project/circuits.js/src/abis/__snapshots__/abis.test.ts.snap index 21707a772620..7b439b1ee61f 100644 --- a/yarn-project/circuits.js/src/abis/__snapshots__/abis.test.ts.snap +++ b/yarn-project/circuits.js/src/abis/__snapshots__/abis.test.ts.snap @@ -44,7 +44,7 @@ AztecAddress { exports[`abis wasm bindings computes a function leaf 1`] = ` Fr { - "value": 16255853943620434246084547000263971056928483888227499632732320436242372959484n, + "value": 8642913288836106416925248965145020288024020397040439357961472995481768818321n, } `; diff --git a/yarn-project/circuits.js/src/abis/abis.test.ts b/yarn-project/circuits.js/src/abis/abis.test.ts index 1cd0325d1e94..2ee18b902f33 100644 --- a/yarn-project/circuits.js/src/abis/abis.test.ts +++ b/yarn-project/circuits.js/src/abis/abis.test.ts @@ -1,4 +1,4 @@ -import { Fr, FunctionData, NewContractData } from '../index.js'; +import { Fr, FunctionData, FunctionLeafPreimage, NewContractData } from '../index.js'; import { makeEthAddress } from '../tests/factories.js'; import { makeAztecAddress, makeBytes, makeTxRequest, makeVerificationKey } from '../tests/factories.js'; import { CircuitsWasm } from '../wasm/circuits_wasm.js'; @@ -40,7 +40,7 @@ describe('abis wasm bindings', () => { }); it('computes a function leaf', async () => { - const leaf = Buffer.alloc(32 + 1 + 32 + 32, 0); + const leaf = new FunctionLeafPreimage(Buffer.from([0, 0, 0, 123]), true, Fr.ZERO, Fr.ZERO); const res = await computeFunctionLeaf(wasm, leaf); expect(res).toMatchSnapshot(); }); diff --git a/yarn-project/circuits.js/src/abis/abis.ts b/yarn-project/circuits.js/src/abis/abis.ts index caafc4d5c855..11a15f22e393 100644 --- a/yarn-project/circuits.js/src/abis/abis.ts +++ b/yarn-project/circuits.js/src/abis/abis.ts @@ -89,7 +89,7 @@ export async function hashVK(wasm: CircuitsWasm, vkBuf: Buffer) { export async function computeFunctionLeaf(wasm: CircuitsWasm, fnLeaf: FunctionLeafPreimage) { const fnLeafBuf = fnLeaf.toBuffer(); - if (fnLeafBuf.length !== 32 + 1 + 32 + 32) throw new Error(`Invalid length for function leaf`); + if (!FunctionLeafPreimage.verifyBufferSize(fnLeafBuf)) throw new Error(`Invalid length for function leaf`); wasm.call('pedersen__init'); return Fr.fromBuffer(await wasmAsyncCall(wasm, 'abis__compute_function_leaf', { toBuffer: () => fnLeafBuf }, 32)); } diff --git a/yarn-project/circuits.js/src/structs/function_leaf_preimage.ts b/yarn-project/circuits.js/src/structs/function_leaf_preimage.ts index 00898d772741..dc572de196e2 100644 --- a/yarn-project/circuits.js/src/structs/function_leaf_preimage.ts +++ b/yarn-project/circuits.js/src/structs/function_leaf_preimage.ts @@ -28,4 +28,8 @@ export class FunctionLeafPreimage { const reader = BufferReader.asReader(buffer); return new FunctionLeafPreimage(reader.readBytes(4), reader.readBoolean(), reader.readFr(), reader.readFr()); } + + static verifyBufferSize(buffer: Buffer): boolean { + return buffer.length === 4 + 1 + 32 + 32; + } } From 033a416da74d5f77ca663f543a7135095726d8a5 Mon Sep 17 00:00:00 2001 From: spypsy Date: Tue, 18 Apr 2023 18:15:01 +0000 Subject: [PATCH 5/9] update snapshot for updated cpp --- .../circuits.js/src/abis/__snapshots__/abis.test.ts.snap | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yarn-project/circuits.js/src/abis/__snapshots__/abis.test.ts.snap b/yarn-project/circuits.js/src/abis/__snapshots__/abis.test.ts.snap index 7b439b1ee61f..5ec7c15ebb82 100644 --- a/yarn-project/circuits.js/src/abis/__snapshots__/abis.test.ts.snap +++ b/yarn-project/circuits.js/src/abis/__snapshots__/abis.test.ts.snap @@ -44,7 +44,7 @@ AztecAddress { exports[`abis wasm bindings computes a function leaf 1`] = ` Fr { - "value": 8642913288836106416925248965145020288024020397040439357961472995481768818321n, + "value": 4561502337125734544623838483038987112287298230187358554089398390117787410340n, } `; From 6daf833eaf6efbb72c1e0f37737e79c8975d16a2 Mon Sep 17 00:00:00 2001 From: cheethas Date: Wed, 19 Apr 2023 12:01:52 +0000 Subject: [PATCH 6/9] clean: remove stale comment --- yarn-project/aztec-rpc/src/contract_tree/contract_tree.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/yarn-project/aztec-rpc/src/contract_tree/contract_tree.ts b/yarn-project/aztec-rpc/src/contract_tree/contract_tree.ts index 28a3aadf0593..c09bed156cc6 100644 --- a/yarn-project/aztec-rpc/src/contract_tree/contract_tree.ts +++ b/yarn-project/aztec-rpc/src/contract_tree/contract_tree.ts @@ -50,7 +50,6 @@ async function generateFunctionLeaves(functions: ContractFunctionDao[], wasm: Ci // All non-unconstrained functions have vks const vkHash = await hashVKStr(f.verificationKey!, wasm); const acirHash = keccak(Buffer.from(f.bytecode, 'hex')); - // TODO: selector is currently padded to 32 bytes in CBINDS, check this. const fnLeafPreimage = new FunctionLeafPreimage( selector, isPrivate, From 55e7ace5665c51fc71c19017facf2165c3fd38ff Mon Sep 17 00:00:00 2001 From: cheethas Date: Wed, 19 Apr 2023 12:21:08 +0000 Subject: [PATCH 7/9] fix: remove verify buffer bytes in favour of check --- yarn-project/circuits.js/src/abis/abis.ts | 1 - .../src/structs/function_leaf_preimage.ts | 17 +++++++++++------ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/yarn-project/circuits.js/src/abis/abis.ts b/yarn-project/circuits.js/src/abis/abis.ts index 11a15f22e393..7bad36c78198 100644 --- a/yarn-project/circuits.js/src/abis/abis.ts +++ b/yarn-project/circuits.js/src/abis/abis.ts @@ -89,7 +89,6 @@ export async function hashVK(wasm: CircuitsWasm, vkBuf: Buffer) { export async function computeFunctionLeaf(wasm: CircuitsWasm, fnLeaf: FunctionLeafPreimage) { const fnLeafBuf = fnLeaf.toBuffer(); - if (!FunctionLeafPreimage.verifyBufferSize(fnLeafBuf)) throw new Error(`Invalid length for function leaf`); wasm.call('pedersen__init'); return Fr.fromBuffer(await wasmAsyncCall(wasm, 'abis__compute_function_leaf', { toBuffer: () => fnLeafBuf }, 32)); } diff --git a/yarn-project/circuits.js/src/structs/function_leaf_preimage.ts b/yarn-project/circuits.js/src/structs/function_leaf_preimage.ts index dc572de196e2..905adc311a3b 100644 --- a/yarn-project/circuits.js/src/structs/function_leaf_preimage.ts +++ b/yarn-project/circuits.js/src/structs/function_leaf_preimage.ts @@ -6,9 +6,13 @@ import { serializeToBuffer } from '../utils/serialize.js'; * @see abis/function_leaf_preimage.hpp */ export class FunctionLeafPreimage { + readonly FUNCTION_SELECTOR_LENGTH = 4; + constructor(public functionSelector: Buffer, public isPrivate: boolean, public vkHash: Fr, public acirHash: Fr) { - if (functionSelector.byteLength !== 4) { - throw new Error(`Function selector must be 4 bytes long, got ${functionSelector.byteLength} bytes.`); + if (functionSelector.byteLength !== this.FUNCTION_SELECTOR_LENGTH) { + throw new Error( + `Function selector must be ${this.FUNCTION_SELECTOR_LENGTH} bytes long, got ${functionSelector.byteLength} bytes.`, + ); } } @@ -17,6 +21,11 @@ export class FunctionLeafPreimage { * @returns The buffer. */ toBuffer(): Buffer { + if (this.functionSelector.byteLength !== this.FUNCTION_SELECTOR_LENGTH) { + throw new Error( + `Function selector must be ${this.FUNCTION_SELECTOR_LENGTH} bytes long, got ${this.functionSelector.byteLength} bytes.`, + ); + } return serializeToBuffer(this.functionSelector, this.isPrivate, this.vkHash, this.acirHash); } @@ -28,8 +37,4 @@ export class FunctionLeafPreimage { const reader = BufferReader.asReader(buffer); return new FunctionLeafPreimage(reader.readBytes(4), reader.readBoolean(), reader.readFr(), reader.readFr()); } - - static verifyBufferSize(buffer: Buffer): boolean { - return buffer.length === 4 + 1 + 32 + 32; - } } From c3315d3633d7ff2e38044532c3e7b545ef695753 Mon Sep 17 00:00:00 2001 From: cheethas Date: Wed, 19 Apr 2023 12:24:15 +0000 Subject: [PATCH 8/9] refactor: use single method to perform length check refactor: use single method to perform length check refactor: use single method to perform length check --- yarn-project/circuits.js/src/abis/abis.test.ts | 16 ++++++++++++++++ .../src/structs/function_leaf_preimage.ts | 13 ++++++++----- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/yarn-project/circuits.js/src/abis/abis.test.ts b/yarn-project/circuits.js/src/abis/abis.test.ts index 2ee18b902f33..7498466746c3 100644 --- a/yarn-project/circuits.js/src/abis/abis.test.ts +++ b/yarn-project/circuits.js/src/abis/abis.test.ts @@ -45,6 +45,22 @@ describe('abis wasm bindings', () => { expect(res).toMatchSnapshot(); }); + it('compute function leaf should revert if buffer is over 4 bytes', () => { + expect(() => { + new FunctionLeafPreimage(Buffer.from([0, 0, 0, 0, 123]), true, Fr.ZERO, Fr.ZERO); + }).toThrow('Function selector must be 4 bytes long, got 5 bytes.'); + }); + + it('function leaf toBuffer should revert if buffer is over 4 bytes ', () => { + const initBuffer = Buffer.from([0, 0, 0, 123]); + const largerBuffer = Buffer.from([0, 0, 0, 0, 123]); + expect(() => { + const leaf = new FunctionLeafPreimage(initBuffer, true, Fr.ZERO, Fr.ZERO); + leaf.functionSelector = largerBuffer; + leaf.toBuffer(); + }).toThrow('Function selector must be 4 bytes long, got 5 bytes.'); + }); + it('computes function tree root', async () => { const res = await computeFunctionTreeRoot(wasm, [new Fr(0n), new Fr(0n), new Fr(0n), new Fr(0n)]); expect(res).toMatchSnapshot(); diff --git a/yarn-project/circuits.js/src/structs/function_leaf_preimage.ts b/yarn-project/circuits.js/src/structs/function_leaf_preimage.ts index 905adc311a3b..85d6caafcbe4 100644 --- a/yarn-project/circuits.js/src/structs/function_leaf_preimage.ts +++ b/yarn-project/circuits.js/src/structs/function_leaf_preimage.ts @@ -9,6 +9,13 @@ export class FunctionLeafPreimage { readonly FUNCTION_SELECTOR_LENGTH = 4; constructor(public functionSelector: Buffer, public isPrivate: boolean, public vkHash: Fr, public acirHash: Fr) { + this.assertFunctionSelectorLength(functionSelector); + } + + /** + * Assert the function selector buffer length matches `FUNCTION_SELECTOR_LENGTH` + */ + private assertFunctionSelectorLength(functionSelector: Buffer) { if (functionSelector.byteLength !== this.FUNCTION_SELECTOR_LENGTH) { throw new Error( `Function selector must be ${this.FUNCTION_SELECTOR_LENGTH} bytes long, got ${functionSelector.byteLength} bytes.`, @@ -21,11 +28,7 @@ export class FunctionLeafPreimage { * @returns The buffer. */ toBuffer(): Buffer { - if (this.functionSelector.byteLength !== this.FUNCTION_SELECTOR_LENGTH) { - throw new Error( - `Function selector must be ${this.FUNCTION_SELECTOR_LENGTH} bytes long, got ${this.functionSelector.byteLength} bytes.`, - ); - } + this.assertFunctionSelectorLength(this.functionSelector); return serializeToBuffer(this.functionSelector, this.isPrivate, this.vkHash, this.acirHash); } From d6350a3dd0dbc06e32880c685ef25fa0b3ec1920 Mon Sep 17 00:00:00 2001 From: cheethas Date: Wed, 19 Apr 2023 12:45:15 +0000 Subject: [PATCH 9/9] fix: just use fnleaf --- yarn-project/circuits.js/src/abis/abis.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/yarn-project/circuits.js/src/abis/abis.ts b/yarn-project/circuits.js/src/abis/abis.ts index 7bad36c78198..b3e0f652a9cc 100644 --- a/yarn-project/circuits.js/src/abis/abis.ts +++ b/yarn-project/circuits.js/src/abis/abis.ts @@ -88,9 +88,8 @@ export async function hashVK(wasm: CircuitsWasm, vkBuf: Buffer) { } export async function computeFunctionLeaf(wasm: CircuitsWasm, fnLeaf: FunctionLeafPreimage) { - const fnLeafBuf = fnLeaf.toBuffer(); wasm.call('pedersen__init'); - return Fr.fromBuffer(await wasmAsyncCall(wasm, 'abis__compute_function_leaf', { toBuffer: () => fnLeafBuf }, 32)); + return Fr.fromBuffer(await wasmAsyncCall(wasm, 'abis__compute_function_leaf', fnLeaf, 32)); } export async function computeFunctionTreeRoot(wasm: CircuitsWasm, fnLeafs: Fr[]) {