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.

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

feat: send G_0 in proof to reduce tube size #9766

Merged
merged 11 commits into from
Nov 8, 2024
Merged

Conversation

lucasxia01
Copy link
Contributor

@lucasxia01 lucasxia01 commented Nov 5, 2024

Removes the G^0 MSM computation from the recursive verifier and instead includes it in the proof.

Adds test to ensure that IPA recursive verifier is a fixed circuit no matter the ECCVM size.

For the command: FLOW=prove_then_verify_tube ./run_acir_tests.sh fold_basic, which has 6 circuits:
Tube gates before constification and before MSM removal: 7104756
Tube gates after: 4172057

For the ClientTubeBase test with 8 circuits, we see:
Tube before: 10047313
Tube gates after: 4172057

@lucasxia01 lucasxia01 self-assigned this Nov 5, 2024
@lucasxia01 lucasxia01 changed the title feat: send G_0 in proof feat: send G_0 in proof to reduce tube size Nov 6, 2024
const auto poly_length = static_cast<uint32_t>(poly_length_var.get_value());
info("IPA poly length: ", poly_length);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

will delete

// TODO(https://github.com/AztecProtocol/barretenberg/issues/1142): Handle this return value correctly.
const typename PCS::VerifierAccumulator batched_opening_accumulator =
PCS::reduce_verify(key->pcs_verification_key, batch_opening_claim, transcript);

info("pre-finalized size after IPA opening: ", builder->num_gates);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

will undo these measurement prints

Copy link
Contributor

@ledwards2225 ledwards2225 left a comment

Choose a reason for hiding this comment

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

LGTM, couple minor comments

@@ -409,6 +413,8 @@ template <typename Curve_> class IPA {
// Compute G₀
Commitment G_zero = bb::scalar_multiplication::pippenger_without_endomorphism_basis_points<Curve>(
s_poly, {&G_vec_local[0], /*size*/ poly_length}, vk->pippenger_runtime_state);
Commitment G_zero_sent = transcript->template receive_from_prover<Commitment>("IPA:G_0");
ASSERT(G_zero == G_zero_sent && "G_0 should be equal to G_0 sent in transcript.");
Copy link
Contributor

Choose a reason for hiding this comment

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

interesting, haven't seen this syntax before

UltraFlavor::VerificationKey verification_key_1(pk_1);
UltraFlavor::VerificationKey verification_key_2(pk_2);
bool broke(false);
auto check_eq = [&broke](auto& p1, auto& p2) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose its useful to have all of this logic here for debugging purposes but just FYI I believe the operators are in place so you can also just check equality of verification keys directly (just sometimes have to manually exclude the commitment keys depending on how they were constructed)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah ok, yeah I just wanted to have the debugging logic

Copy link
Contributor

github-actions bot commented Nov 8, 2024

Changes to public function bytecode sizes

Generated at commit: 4b28ea51acfb9924f10d90d6517b68c2c72ed8fb, compared to commit: 2096dc236c627cfd802ca05e0c9cb0ea6c441458

🧾 Summary (100% most significant diffs)

Program Bytecode size in bytes (+/-) %
Router::public_dispatch +62 ❌ +2.40%
ImportTest::public_dispatch +26 ❌ +1.68%
Parent::public_dispatch +124 ❌ +1.48%
TestLog::public_dispatch +39 ❌ +1.21%
AppSubscription::public_dispatch +57 ❌ +1.15%
AuthWitTest::public_dispatch +21 ❌ +1.10%
StaticParent::public_dispatch +72 ❌ +1.00%
PriceFeed::public_dispatch +39 ❌ +0.99%
FPC::public_dispatch +76 ❌ +0.81%
Token::public_dispatch +287 ❌ +0.78%
AvmTest::public_dispatch +407 ❌ +0.68%
AvmTest::returndata_copy_oracle +13 ❌ +0.65%
NFT::public_dispatch +160 ❌ +0.61%
CardGame::public_dispatch +92 ❌ +0.57%
TokenBlacklist::public_dispatch +137 ❌ +0.54%
StaticChild::public_dispatch +13 ❌ +0.43%
AuthRegistry::public_dispatch +33 ❌ +0.39%
Uniswap::public_dispatch +89 ❌ +0.36%
Spam::public_dispatch +13 ❌ +0.35%
Lending::public_dispatch +92 ❌ +0.32%
PrivateFPC::public_dispatch +13 ❌ +0.32%
InclusionProofs::public_dispatch +13 ❌ +0.31%
Claim::public_dispatch +13 ❌ +0.31%
EasyPrivateVoting::public_dispatch +16 ❌ +0.26%
Child::public_dispatch +13 ❌ +0.22%
Token::complete_refund +13 ❌ +0.22%
TokenBridge::public_dispatch +47 ❌ +0.21%
NFT::finalize_transfer_to_private +13 ❌ +0.20%
NFT::_finalize_transfer_to_private_unsafe +13 ❌ +0.20%
Auth::public_dispatch +20 ❌ +0.19%
Token::finalize_mint_to_private +13 ❌ +0.18%
Token::_finalize_mint_to_private_unsafe +13 ❌ +0.18%
Token::finalize_transfer_to_private +13 ❌ +0.18%
Token::_finalize_transfer_to_private_unsafe +13 ❌ +0.18%
Benchmarking::public_dispatch +10 ❌ +0.18%
Test::public_dispatch +27 ❌ +0.14%
StatefulTest::public_dispatch +6 ❌ +0.07%
DocsExample::public_dispatch +4 ❌ +0.07%
FeeJuice::public_dispatch +4 ❌ +0.06%
Crowdfunding::public_dispatch -28 ✅ -0.45%

Full diff report 👇
Program Bytecode size in bytes (+/-) %
Router::public_dispatch 2,645 (+62) +2.40%
ImportTest::public_dispatch 1,577 (+26) +1.68%
Parent::public_dispatch 8,505 (+124) +1.48%
TestLog::public_dispatch 3,258 (+39) +1.21%
AppSubscription::public_dispatch 5,030 (+57) +1.15%
AuthWitTest::public_dispatch 1,928 (+21) +1.10%
StaticParent::public_dispatch 7,292 (+72) +1.00%
PriceFeed::public_dispatch 3,986 (+39) +0.99%
FPC::public_dispatch 9,472 (+76) +0.81%
Token::public_dispatch 36,991 (+287) +0.78%
AvmTest::public_dispatch 60,539 (+407) +0.68%
AvmTest::returndata_copy_oracle 2,011 (+13) +0.65%
NFT::public_dispatch 26,393 (+160) +0.61%
CardGame::public_dispatch 16,326 (+92) +0.57%
TokenBlacklist::public_dispatch 25,334 (+137) +0.54%
StaticChild::public_dispatch 3,046 (+13) +0.43%
AuthRegistry::public_dispatch 8,396 (+33) +0.39%
Uniswap::public_dispatch 25,135 (+89) +0.36%
Spam::public_dispatch 3,778 (+13) +0.35%
Lending::public_dispatch 28,431 (+92) +0.32%
PrivateFPC::public_dispatch 4,118 (+13) +0.32%
InclusionProofs::public_dispatch 4,182 (+13) +0.31%
Claim::public_dispatch 4,240 (+13) +0.31%
EasyPrivateVoting::public_dispatch 6,101 (+16) +0.26%
Child::public_dispatch 5,994 (+13) +0.22%
Token::complete_refund 6,057 (+13) +0.22%
TokenBridge::public_dispatch 22,672 (+47) +0.21%
NFT::finalize_transfer_to_private 6,448 (+13) +0.20%
NFT::_finalize_transfer_to_private_unsafe 6,495 (+13) +0.20%
Auth::public_dispatch 10,802 (+20) +0.19%
Token::finalize_mint_to_private 7,118 (+13) +0.18%
Token::_finalize_mint_to_private_unsafe 7,165 (+13) +0.18%
Token::finalize_transfer_to_private 7,328 (+13) +0.18%
Token::_finalize_transfer_to_private_unsafe 7,375 (+13) +0.18%
Benchmarking::public_dispatch 5,697 (+10) +0.18%
Test::public_dispatch 19,778 (+27) +0.14%
StatefulTest::public_dispatch 8,128 (+6) +0.07%
DocsExample::public_dispatch 5,842 (+4) +0.07%
FeeJuice::public_dispatch 6,265 (+4) +0.06%
Crowdfunding::public_dispatch 6,184 (-28) -0.45%

@lucasxia01 lucasxia01 enabled auto-merge (squash) November 8, 2024 17:15
@lucasxia01 lucasxia01 merged commit 9bc5a2f into master Nov 8, 2024
47 checks passed
@lucasxia01 lucasxia01 deleted the lx/update-ipa-proofs branch November 8, 2024 17:19
ludamad pushed a commit that referenced this pull request Nov 11, 2024
Removes the G^0 MSM computation from the recursive verifier and instead
includes it in the proof.

Adds test to ensure that IPA recursive verifier is a fixed circuit no
matter the ECCVM size.

For the command: `FLOW=prove_then_verify_tube ./run_acir_tests.sh
fold_basic`, which has 6 circuits:
Tube gates before constification and before MSM removal: 7104756
Tube gates after: 4172057

For the ClientTubeBase test with 8 circuits, we see:
Tube before: 10047313
Tube gates after: 4172057
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.

3 participants