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

Public kernel cbind errors #437

Merged
merged 4 commits into from
May 4, 2023
Merged

Public kernel cbind errors #437

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c80d491
cbinds for pub kernel + format public kernel tests
rahul-kothari May 2, 2023
84a8ee4
fix tests
rahul-kothari May 3, 2023
517d98f
fix per PR comments
rahul-kothari May 3, 2023
416557d
address PR comments
rahul-kothari May 4, 2023
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
13 changes: 12 additions & 1 deletion circuits/cpp/src/aztec3/circuits/abis/c_bind.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "c_bind.h"

#include "call_stack_item.hpp"
#include "function_data.hpp"
#include "function_leaf_preimage.hpp"
#include "kernel_circuit_public_inputs.hpp"
Expand All @@ -20,6 +21,7 @@
#include "aztec3/circuits/abis/function_leaf_preimage.hpp"
#include "aztec3/circuits/abis/new_contract_data.hpp"
#include "aztec3/circuits/abis/signed_tx_request.hpp"
#include "aztec3/circuits/abis/types.hpp"
#include <aztec3/circuits/hash.hpp>
#include <aztec3/constants.hpp>
#include <aztec3/utils/array.hpp>
Expand All @@ -34,12 +36,14 @@ namespace {

using aztec3::circuits::compute_constructor_hash;
using aztec3::circuits::compute_contract_address;
using aztec3::circuits::abis::CallStackItem;
using aztec3::circuits::abis::FunctionData;
using aztec3::circuits::abis::FunctionLeafPreimage;
using aztec3::circuits::abis::NewContractData;
using aztec3::circuits::abis::TxContext;
using aztec3::circuits::abis::TxRequest;
using NT = aztec3::utils::types::NativeTypes;
using aztec3::circuits::abis::PublicTypes;

// Cbind helper functions

Expand Down Expand Up @@ -128,7 +132,7 @@ WASM_EXPORT void abis__hash_tx_request(uint8_t const* tx_request_buf, uint8_t* o
{
TxRequest<NT> tx_request;
read(tx_request_buf, tx_request);
// TODO consider using write() and read() instead of
// TODO(dbanks12) consider using write() and read() instead of
// serialize to/from everywhere here and in test
NT::fr::serialize_to_buffer(tx_request.hash(), output);
}
Expand Down Expand Up @@ -345,6 +349,13 @@ WASM_EXPORT void abis__compute_contract_leaf(uint8_t const* contract_leaf_preima
NT::fr::serialize_to_buffer(to_write, output);
}

WASM_EXPORT void abis__compute_call_stack_item_hash(uint8_t const* call_stack_item_buf, uint8_t* output)
{
CallStackItem<NT, PublicTypes> call_stack_item;
read(call_stack_item_buf, call_stack_item);
NT::fr::serialize_to_buffer(call_stack_item.hash(), output);
}

/* Typescript test helpers that call as_string_output() to stress serialization.
* Each of these take an object buffer, and a string size pointer.
* They return a string pointer (to be bbfree'd) and write to the string size pointer. */
Expand Down
5 changes: 4 additions & 1 deletion circuits/cpp/src/aztec3/circuits/abis/c_bind.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,7 @@ WASM_EXPORT void abis__compute_contract_address(uint8_t const* deployer_address_
uint8_t* output);

WASM_EXPORT void abis__compute_contract_leaf(uint8_t const* contract_leaf_preimage_buf, uint8_t* output);
}

WASM_EXPORT void abis__compute_call_stack_item_hash(uint8_t const* call_stack_item_buf, uint8_t* output);

}
292 changes: 148 additions & 144 deletions circuits/cpp/src/aztec3/circuits/kernel/public/.test.cpp
View file
Open in desktop

Large diffs are not rendered by default.

27 changes: 16 additions & 11 deletions circuits/cpp/src/aztec3/circuits/kernel/public/c_bind.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ WASM_EXPORT size_t public_kernel__init_proving_key(uint8_t const** pk_buf)
WASM_EXPORT size_t public_kernel__init_verification_key(uint8_t const* pk_buf, uint8_t const** vk_buf)
{
std::vector<uint8_t> vk_vec(42, 0);
// TODO remove when proving key is used
// TODO(dbanks12): remove when proving key is used
(void)pk_buf; // unused

auto* raw_buf = (uint8_t*)malloc(vk_vec.size());
Expand All @@ -53,7 +53,9 @@ WASM_EXPORT size_t public_kernel__init_verification_key(uint8_t const* pk_buf, u
return vk_vec.size();
}

WASM_EXPORT size_t public_kernel__sim(uint8_t const* public_kernel_inputs_buf, uint8_t const** public_inputs_buf)
WASM_EXPORT uint8_t* public_kernel__sim(uint8_t const* public_kernel_inputs_buf,
size_t* public_kernel_public_inputs_size_out,
uint8_t const** public_kernel_public_inputs_buf)
{
DummyComposer composer = DummyComposer();

Expand All @@ -71,13 +73,15 @@ WASM_EXPORT size_t public_kernel__sim(uint8_t const* public_kernel_inputs_buf, u
// copy public inputs to output buffer
auto* raw_public_inputs_buf = (uint8_t*)malloc(public_inputs_vec.size());
memcpy(raw_public_inputs_buf, (void*)public_inputs_vec.data(), public_inputs_vec.size());
*public_inputs_buf = raw_public_inputs_buf;

return public_inputs_vec.size();
*public_kernel_public_inputs_buf = raw_public_inputs_buf;
*public_kernel_public_inputs_size_out = public_inputs_vec.size();
composer.log_failures_if_any("public_kernel__sim");
return composer.alloc_and_serialize_first_failure();
}

WASM_EXPORT size_t public_kernel_no_previous_kernel__sim(uint8_t const* public_kernel_inputs_buf,
uint8_t const** public_inputs_buf)
WASM_EXPORT uint8_t* public_kernel_no_previous_kernel__sim(uint8_t const* public_kernel_inputs_buf,
size_t* public_kernel_public_inputs_size_out,
uint8_t const** public_kernel_public_inputs_buf)
{
DummyComposer composer = DummyComposer();

Expand All @@ -93,9 +97,10 @@ WASM_EXPORT size_t public_kernel_no_previous_kernel__sim(uint8_t const* public_k
// copy public inputs to output buffer
auto* raw_public_inputs_buf = (uint8_t*)malloc(public_inputs_vec.size());
memcpy(raw_public_inputs_buf, (void*)public_inputs_vec.data(), public_inputs_vec.size());
*public_inputs_buf = raw_public_inputs_buf;

return public_inputs_vec.size();
*public_kernel_public_inputs_buf = raw_public_inputs_buf;
*public_kernel_public_inputs_size_out = public_inputs_vec.size();
composer.log_failures_if_any("public_kernel_no_previous_kernel__sim");
return composer.alloc_and_serialize_first_failure();
}

} // extern "C"
} // extern "C"
10 changes: 6 additions & 4 deletions circuits/cpp/src/aztec3/circuits/kernel/public/c_bind.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ extern "C" {

WASM_EXPORT size_t public_kernel__init_proving_key(uint8_t const** pk_buf);
WASM_EXPORT size_t public_kernel__init_verification_key(uint8_t const* pk_buf, uint8_t const** vk_buf);
WASM_EXPORT size_t public_kernel__sim(uint8_t const* public_kernel_inputs_buf,
uint8_t const** public_kernel_public_inputs_buf);
WASM_EXPORT size_t public_kernel_no_previous_kernel__sim(uint8_t const* public_kernel_inputs_buf,
uint8_t const** public_kernel_public_inputs_buf);
WASM_EXPORT uint8_t* public_kernel__sim(uint8_t const* public_kernel_inputs_buf,
size_t* public_kernel_public_inputs_size_out,
uint8_t const** public_kernel_public_inputs_buf);
WASM_EXPORT uint8_t* public_kernel_no_previous_kernel__sim(uint8_t const* public_kernel_inputs_buf,
size_t* public_kernel_public_inputs_size_out,
uint8_t const** public_kernel_public_inputs_buf);
}
6 changes: 5 additions & 1 deletion circuits/cpp/src/aztec3/circuits/kernel/public/common.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,11 @@ void validate_this_public_call_hash(DummyComposer& composer,

composer.do_assert(
popped_public_call_hash == calculated_this_public_call_hash,
"calculated public_call_hash does not match provided public_call_hash at the top of the call stack",
format("calculated public_call_hash (",
calculated_this_public_call_hash,
") does not match provided public_call_hash (",
popped_public_call_hash,
") at the top of the call stack"),
CircuitErrorCode::PUBLIC_KERNEL__CALCULATED_PRIVATE_CALL_HASH_AND_PROVIDED_PRIVATE_CALL_HASH_MISMATCH);
};
} // namespace aztec3::circuits::kernel::public_kernel
8 changes: 5 additions & 3 deletions circuits/cpp/src/aztec3/circuits/kernel/public/common.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,11 @@ void common_validate_call_stack(DummyComposer& composer, KernelInput const& publ
// Note: this assumes it's computationally infeasible to have `0` as a valid call_stack_item_hash.
// Assumes `hash == 0` means "this stack item is empty".
const auto calculated_hash = hash == 0 ? 0 : preimage.hash();
composer.do_assert(hash == calculated_hash,
format("public_call_stack[", i, "] = ", hash, "; does not reconcile"),
CircuitErrorCode::PUBLIC_KERNEL__PUBLIC_CALL_STACK_MISMATCH);
composer.do_assert(
hash == calculated_hash,
format(
"public_call_stack[", i, "] = ", hash, "; does not reconcile with calculatedHash = ", calculated_hash),
CircuitErrorCode::PUBLIC_KERNEL__PUBLIC_CALL_STACK_MISMATCH);

// here we validate the msg sender for each call on the stack
// we need to consider regular vs delegate calls
Expand Down
7 changes: 7 additions & 0 deletions yarn-project/circuits.js/src/abis/abis.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
TxRequest,
NewContractData,
FunctionLeafPreimage,
PublicCallStackItem,
} from '../index.js';
import { serializeToBuffer, serializeBufferArrayToVector } from '../utils/serialize.js';
import { AsyncWasmWrapper, WasmWrapper } from '@aztec/foundation/wasm';
Expand Down Expand Up @@ -147,3 +148,9 @@ export function computeContractLeaf(wasm: WasmWrapper, cd: NewContractData) {
const value = wasmSyncCall(wasm, 'abis__compute_contract_leaf', cd, 32);
return Fr.fromBuffer(value);
}

export function computeCallStackItemHash(wasm: WasmWrapper, callStackItem: PublicCallStackItem) {
wasm.call('pedersen__init');
const value = wasmSyncCall(wasm, 'abis__compute_call_stack_item_hash', callStackItem, 32);
return Fr.fromBuffer(value);
}
rahul-kothari marked this conversation as resolved.
Show resolved Hide resolved
9 changes: 5 additions & 4 deletions yarn-project/circuits.js/src/kernel/public_kernel.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { simulatePublicKernelCircuit, simulatePublicKernelCircuitNoPreviousKernel } from '../index.js';
import { Fr, simulatePublicKernelCircuit, simulatePublicKernelCircuitNoPreviousKernel } from '../index.js';
import { makePublicKernelInputsNoKernelInput, makePublicKernelInputsWithEmptyOutput } from '../tests/factories.js';

describe('kernel/public_kernel', () => {
it('simulates public kernel circuit with previous public kernel', async function () {
const input = makePublicKernelInputsWithEmptyOutput();
const input = await makePublicKernelInputsWithEmptyOutput();
// Fix validity
input.publicCallData.callStackItem.functionData.isConstructor = false;
input.publicCallData.callStackItem.functionData.isPrivate = false;
Expand All @@ -13,14 +13,15 @@ describe('kernel/public_kernel', () => {
});

it('simulates public kernel circuit with previous private kernel', async function () {
const input = makePublicKernelInputsWithEmptyOutput();
const input = await makePublicKernelInputsWithEmptyOutput();
input.previousKernel.publicInputs.isPrivateKernel = true;
input.previousKernel.publicInputs.end.publicCallCount = Fr.ZERO;
const result = await simulatePublicKernelCircuit(input);
expect(result).toBeDefined();
});

it('simulates public kernel circuit with no previous kernel', async function () {
const input = makePublicKernelInputsNoKernelInput();
const input = await makePublicKernelInputsNoKernelInput();
const result = await simulatePublicKernelCircuitNoPreviousKernel(input);
expect(result).toBeDefined();
});
Expand Down
80 changes: 40 additions & 40 deletions yarn-project/circuits.js/src/structs/kernel/__snapshots__/index.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -945,11 +945,11 @@ public_call:
call_stack_item:
contract_address: 0x1001
function_data: function_selector: 4098
is_private: 1
is_constructor: 1
is_private: 0
is_constructor: 0

public_inputs: call_context: msg_sender: 0x1011
storage_contract_address: 0x1012
storage_contract_address: 0x1001
portal_contract_address: 0x1313131313131313131313131313131313131313
is_delegate_call: 0
is_static_call: 0
Expand Down Expand Up @@ -980,7 +980,7 @@ current_value: 0x1514
storage_slot: 0x1514
current_value: 0x1515
]
public_call_stack: [ 0x1611 0x1612 0x1613 0x1614 ]
public_call_stack: [ 0x1f7ec2163f0e78b25257ca693b8606d5e6c940c36e2e77cd0aa1afbbf167e95a 0xbfa6e6e8892f68b806bca1fd20cf704dd5c449440b54ef2957a3d267342bda6 0x29618099103194faea4e2c1c2a77bbb165bfe89961c16369b797cfd38671a2ba 0x1403602a993517c978fd5ea6ea17c02280692bb4b84e38e1a92f8c6d1d7b858b ]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm on the fence about this change. On one hand, it's great to have "real" values for our factories. On the other, it makes it a lot more difficult to identify issues with the serialization.

I guess that troubleshooting serialization will become less important given @ludamad's upcoming changes, so we can keep going in this direction. WDYT?

new_l2_to_l1_msgs: [ 0x1711 0x1712 ]
historic_public_data_tree_root: 0x1811
prover_address: 0x1812
Expand All @@ -989,11 +989,11 @@ prover_address: 0x1812
public_call_stack_preimages:
[ contract_address: 0x1301
function_data: function_selector: 1302
is_private: 1
is_constructor: 1
is_private: 0
is_constructor: 0

public_inputs: call_context: msg_sender: 0x1311
storage_contract_address: 0x1312
public_inputs: call_context: msg_sender: 0x1001
storage_contract_address: 0x1301
portal_contract_address: 0x1313131313131313131313131313131313131313
is_delegate_call: 0
is_static_call: 0
Expand Down Expand Up @@ -1031,11 +1031,11 @@ prover_address: 0x1b12

contract_address: 0x1302
function_data: function_selector: 1303
is_private: 1
is_constructor: 1
is_private: 0
is_constructor: 0

public_inputs: call_context: msg_sender: 0x1312
storage_contract_address: 0x1313
public_inputs: call_context: msg_sender: 0x1001
storage_contract_address: 0x1302
portal_contract_address: 0x1414141414141414141414141414141414141414
is_delegate_call: 0
is_static_call: 0
Expand Down Expand Up @@ -1073,11 +1073,11 @@ prover_address: 0x1b13

contract_address: 0x1303
function_data: function_selector: 1304
is_private: 1
is_constructor: 1
is_private: 0
is_constructor: 0

public_inputs: call_context: msg_sender: 0x1313
storage_contract_address: 0x1314
public_inputs: call_context: msg_sender: 0x1001
storage_contract_address: 0x1303
portal_contract_address: 0x1515151515151515151515151515151515151515
is_delegate_call: 0
is_static_call: 0
Expand Down Expand Up @@ -1115,11 +1115,11 @@ prover_address: 0x1b14

contract_address: 0x1304
function_data: function_selector: 1305
is_private: 1
is_constructor: 1
is_private: 0
is_constructor: 0

public_inputs: call_context: msg_sender: 0x1314
storage_contract_address: 0x1315
public_inputs: call_context: msg_sender: 0x1001
storage_contract_address: 0x1304
portal_contract_address: 0x1616161616161616161616161616161616161616
is_delegate_call: 0
is_static_call: 0
Expand Down Expand Up @@ -1192,11 +1192,11 @@ public_call:
call_stack_item:
contract_address: 0x101
function_data: function_selector: 258
is_private: 1
is_constructor: 1
is_private: 0
is_constructor: 0

public_inputs: call_context: msg_sender: 0x111
storage_contract_address: 0x112
storage_contract_address: 0x101
portal_contract_address: 0x1313131313131313131313131313131313131313
is_delegate_call: 0
is_static_call: 0
Expand Down Expand Up @@ -1227,7 +1227,7 @@ current_value: 0x614
storage_slot: 0x614
current_value: 0x615
]
public_call_stack: [ 0x711 0x712 0x713 0x714 ]
public_call_stack: [ 0x2d514733a86bef4a33e2d599e01f3a73cbb7c17c316dfc5009838ce546587685 0x80562236b9b336749abfe2e71c4d2688ef96a2e2c09accce5e34fbb61c5416a 0x2569d412538dd4259fd18e60c166f3adbd0ab124dca7175a8448a7afdc1e7c40 0x1acab4b3623e71d9b08ecc27a170d06e38d1fca67ee03b3efe92365ebfb250f4 ]
new_l2_to_l1_msgs: [ 0x811 0x812 ]
historic_public_data_tree_root: 0x911
prover_address: 0x912
Expand All @@ -1236,11 +1236,11 @@ prover_address: 0x912
public_call_stack_preimages:
[ contract_address: 0x401
function_data: function_selector: 402
is_private: 1
is_constructor: 1
is_private: 0
is_constructor: 0

public_inputs: call_context: msg_sender: 0x411
storage_contract_address: 0x412
public_inputs: call_context: msg_sender: 0x101
storage_contract_address: 0x401
portal_contract_address: 0x1313131313131313131313131313131313131313
is_delegate_call: 0
is_static_call: 0
Expand Down Expand Up @@ -1278,11 +1278,11 @@ prover_address: 0xc12

contract_address: 0x402
function_data: function_selector: 403
is_private: 1
is_constructor: 1
is_private: 0
is_constructor: 0

public_inputs: call_context: msg_sender: 0x412
storage_contract_address: 0x413
public_inputs: call_context: msg_sender: 0x101
storage_contract_address: 0x402
portal_contract_address: 0x1414141414141414141414141414141414141414
is_delegate_call: 0
is_static_call: 0
Expand Down Expand Up @@ -1320,11 +1320,11 @@ prover_address: 0xc13

contract_address: 0x403
function_data: function_selector: 404
is_private: 1
is_constructor: 1
is_private: 0
is_constructor: 0

public_inputs: call_context: msg_sender: 0x413
storage_contract_address: 0x414
public_inputs: call_context: msg_sender: 0x101
storage_contract_address: 0x403
portal_contract_address: 0x1515151515151515151515151515151515151515
is_delegate_call: 0
is_static_call: 0
Expand Down Expand Up @@ -1362,11 +1362,11 @@ prover_address: 0xc14

contract_address: 0x404
function_data: function_selector: 405
is_private: 1
is_constructor: 1
is_private: 0
is_constructor: 0

public_inputs: call_context: msg_sender: 0x414
storage_contract_address: 0x415
public_inputs: call_context: msg_sender: 0x101
storage_contract_address: 0x404
portal_contract_address: 0x1616161616161616161616161616161616161616
is_delegate_call: 0
is_static_call: 0
Expand Down
Loading