Skip to content

feat: Private Kernel Recursion#6278

Merged
PhilWindle merged 34 commits into
masterfrom
pw/complete-kernel-recursion
May 9, 2024
Merged

feat: Private Kernel Recursion#6278
PhilWindle merged 34 commits into
masterfrom
pw/complete-kernel-recursion

Conversation

@PhilWindle

@PhilWindle PhilWindle commented May 8, 2024

Copy link
Copy Markdown
Collaborator

This PR introduces recursive verification to the private kernel circuits. Both app circuit and previous kernel circuit proofs are verified. This closes #5978

The changes can be largely categorised as:

  1. PXE modifications to pass proofs and verification keys from the output of a proving process as inputs to the next simulation/proving process.
  2. Serialisation of PrivateCircuitPublicInputs and PrivateKernelCircuitPublicInputs structs to fields.
  3. Aggregation of proofs using Noir's verify_proof api.

Additional task create here to prevent the specification of pub on arguments to private functions.

@PhilWindle PhilWindle marked this pull request as ready for review May 8, 2024 16:54
@github-actions

github-actions Bot commented May 8, 2024

Copy link
Copy Markdown
Contributor

Changes to circuit sizes

Generated at commit: fd831c6fccff00a6b54b67f727192b29b1c4af6d, compared to commit: ef9cdde09d6cdd8a5deb0217fea1e828477f0c03

🧾 Summary (100% most significant diffs)

Program ACIR opcodes (+/-) % Circuit size (+/-) %
private_kernel_inner +870 ❌ +0.61% +773,123 ❌ +198.55%
private_kernel_init +419 ❌ +0.34% +288,694 ❌ +101.67%
private_kernel_tail +451 ❌ +0.17% +475,397 ❌ +37.85%
private_kernel_tail_to_public +451 ❌ +0.09% +475,394 ❌ +26.62%

Full diff report 👇
Program ACIR opcodes (+/-) % Circuit size (+/-) %
private_kernel_inner 143,777 (+870) +0.61% 1,162,509 (+773,123) +198.55%
private_kernel_init 123,312 (+419) +0.34% 572,633 (+288,694) +101.67%
private_kernel_tail 265,679 (+451) +0.17% 1,731,369 (+475,397) +37.85%
private_kernel_tail_to_public 494,152 (+451) +0.09% 2,260,944 (+475,394) +26.62%

@LeilaWang LeilaWang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Some suggestions. Looks great to me overall! 🚀

global PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH: u64 = CALL_CONTEXT_LENGTH + 2 + (READ_REQUEST_LENGTH * MAX_NULLIFIER_READ_REQUESTS_PER_CALL) + (READ_REQUEST_LENGTH * MAX_NULLIFIER_NON_EXISTENT_READ_REQUESTS_PER_CALL) + (CONTRACT_STORAGE_UPDATE_REQUEST_LENGTH * MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_CALL) + (CONTRACT_STORAGE_READ_LENGTH * MAX_PUBLIC_DATA_READS_PER_CALL) + MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL + (NOTE_HASH_LENGTH * MAX_NEW_NOTE_HASHES_PER_CALL) + (NULLIFIER_LENGTH * MAX_NEW_NULLIFIERS_PER_CALL) + (L2_TO_L1_MESSAGE_LENGTH * MAX_NEW_L2_TO_L1_MSGS_PER_CALL) + 2 + (SIDE_EFFECT_LENGTH * MAX_UNENCRYPTED_LOGS_PER_CALL) + 1 + HEADER_LENGTH + GLOBAL_VARIABLES_LENGTH + AZTEC_ADDRESS_LENGTH + /* revert_code */ 1 + 2 * GAS_LENGTH + /* transaction_fee */ 1;
global PRIVATE_CALL_STACK_ITEM_LENGTH: u64 = AZTEC_ADDRESS_LENGTH + FUNCTION_DATA_LENGTH + PRIVATE_CIRCUIT_PUBLIC_INPUTS_LENGTH;

global READ_REQUEST_CONTEXT_SERIALIZED_LEN = 3;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks like this is copied from read_request.nr. Should we replace the constant in read_request.nr with this? I renamed the one in that file to SCOPED_READ_REQUEST_SERIALIZED_LEN. But maybe we should call it SCOPED_READ_REQUEST_LENGTH so that it's more consistent with other SCOPED_XXX_LENGTH.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done!

global CALL_REQUEST_LENGTH = 1 + AZTEC_ADDRESS_LENGTH + CALLER_CONTEXT_LENGTH + 2;
global PRIVATE_ACCUMULATED_DATA_LENGTH = (SCOPED_NOTE_HASH_LENGTH * MAX_NEW_NOTE_HASHES_PER_TX) + (SCOPED_NULLIFIER_LENGTH * MAX_NEW_NULLIFIERS_PER_TX) + (MAX_NEW_L2_TO_L1_MSGS_PER_TX * SCOPED_L2_TO_L1_MESSAGE_LENGTH) + (SIDE_EFFECT_LENGTH * MAX_ENCRYPTED_LOGS_PER_TX) + (SIDE_EFFECT_LENGTH * MAX_UNENCRYPTED_LOGS_PER_TX) + 2 + (CALL_REQUEST_LENGTH * MAX_PRIVATE_CALL_STACK_LENGTH_PER_TX) + (CALL_REQUEST_LENGTH * MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX);
global PRIVATE_KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 1 + VALIDATION_REQUESTS_LENGTH + PRIVATE_ACCUMULATED_DATA_LENGTH + COMBINED_CONSTANT_DATA_LENGTH + CALL_REQUEST_LENGTH;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Although these constants are not used outside of noir. But can still run yarn remake-constants in circuits.js to update other constant files so that they won't appear in another irrelevant pr.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Wasn't aware of that. Have pushed the updated files.

await fs.writeFile(proofFileName, proof.buffer);
await fs.writeFile(verificationKeyPath, verificationKey);

const result = await verifyProof(this.bbBinaryPath, proofFileName, verificationKeyPath!, logFunction);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shall we catch the error and always delete the directory?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes

@PhilWindle PhilWindle enabled auto-merge (squash) May 9, 2024 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Proving Phase 2]: Update private kernel circuits to support recursive verification

2 participants