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

Jump to bottom

chore: reproduce AVM ecadd bug #9019

Merged
merged 2 commits into from
Oct 8, 2024
Merged

chore: reproduce AVM ecadd bug #9019

merged 2 commits into from
Oct 8, 2024

Conversation

sirasistant
Copy link
Contributor

Please read contributing guidelines and remove this line.

@sirasistant sirasistant mentioned this pull request Oct 4, 2024
Open
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 4, 2024 •
edited
Loading

Changes to public function bytecode sizes

Generated at commit: 912b40bb1897f34cca8133059119a51978f514bb, compared to commit: f9fc73a40f5b9d11ad92a6cee3e29d3fcc80425e

🧾 Summary (100% most significant diffs)

Program Bytecode size in bytes (+/-) %
AvmTest::elliptic_curve_add_and_double +9 ❌ +5.26%
CardGame::on_card_played +185 ❌ +3.23%
DocsExample::get_shared_immutable_constrained_public_indirect +11 ❌ +2.61%
Child::set_value_with_two_nested_calls +14 ❌ +1.90%
AvmTest::nested_call_to_add_with_gas +14 ❌ +1.67%
AvmTest::public_dispatch +1,460 ❌ +1.58%
Uniswap::_assert_token_is_same +7 ❌ +1.50%
CardGame::public_dispatch +334 ❌ +1.20%
Token::public_dispatch +312 ❌ +0.46%
NFT::public_dispatch +167 ❌ +0.42%
Lending::_deposit +10 ❌ +0.40%
Uniswap::public_dispatch +120 ❌ +0.36%
InclusionProofs::public_dispatch +20 ❌ +0.35%
FPC::public_dispatch +41 ❌ +0.35%
Auth::public_dispatch +86 ❌ +0.29%
StatefulTest::public_dispatch +36 ❌ +0.29%
StatefulTest::public_constructor +13 ❌ +0.29%
EasyPrivateVoting::public_dispatch +26 ❌ +0.28%
AvmInitializerTest::public_dispatch +10 ❌ +0.27%
Child::public_dispatch +14 ❌ +0.25%
TokenBlacklist::public_dispatch +332 ❌ +0.25%
Lending::_repay +23 ❌ +0.25%
TokenBlacklist::constructor +33 ❌ +0.22%
AuthRegistry::consume +18 ❌ +0.22%
AvmTest::set_storage_map +6 ❌ +0.21%
AvmTest::add_storage_map +9 ❌ +0.21%
AuthRegistry::set_authorized +6 ❌ +0.21%
AuthRegistry::_set_authorized +6 ❌ +0.20%
NFT::mint +12 ❌ +0.20%
AuthRegistry::is_consumable +6 ❌ +0.20%
TokenBlacklist::transfer_public +30 ❌ +0.19%
AvmTest::poseidon2_hash +3 ❌ +0.19%
TokenBlacklist::mint_public +18 ❌ +0.19%
TokenBlacklist::update_roles +27 ❌ +0.19%
Token::transfer_public +18 ❌ +0.18%
AvmTest::get_args_hash +3 ❌ +0.18%
NFT::owner_of +6 ❌ +0.18%
Auth::set_authorized_delay +18 ❌ +0.18%
AuthRegistry::public_dispatch +42 ❌ +0.18%
FeeJuice::_increase_public_balance +6 ❌ +0.18%
TokenBlacklist::get_roles +6 ❌ +0.18%
Auth::set_authorized +18 ❌ +0.18%
TokenBlacklist::_increase_public_balance +6 ❌ +0.18%
Token::_increase_public_balance +6 ❌ +0.18%
EasyPrivateVoting::add_to_tally_public +6 ❌ +0.18%
Spam::public_spam +6 ❌ +0.18%
TokenBlacklist::burn_public +18 ❌ +0.17%
AuthRegistry::set_reject_all +3 ❌ +0.17%
Lending::_withdraw +23 ❌ +0.17%
Token::mint_public +9 ❌ +0.17%
NFT::_finish_transfer_to_public +3 ❌ +0.17%
PriceFeed::set_price +3 ❌ +0.17%
AvmTest::read_storage_map +3 ❌ +0.17%
StatefulTest::get_public_value +3 ❌ +0.16%
AuthRegistry::is_reject_all +3 ❌ +0.16%
Token::burn_public +12 ❌ +0.16%
PriceFeed::get_price +3 ❌ +0.16%
NFT::transfer_in_public +9 ❌ +0.16%
Benchmarking::broadcast +3 ❌ +0.16%
InclusionProofs::constructor +6 ❌ +0.16%
NFT::is_minter +3 ❌ +0.16%
Token::is_minter +3 ❌ +0.16%
Auth::get_scheduled_authorized +3 ❌ +0.16%
Crowdfunding::public_dispatch +13 ❌ +0.16%
FeeJuice::balance_of_public +3 ❌ +0.16%
StatefulTest::increment_public_value_no_init_check +3 ❌ +0.15%
TokenBlacklist::balance_of_public +3 ❌ +0.15%
Token::balance_of_public +3 ❌ +0.15%
TokenBlacklist::shield +18 ❌ +0.15%
Benchmarking::increment_balance +6 ❌ +0.15%
StatefulTest::increment_public_value +3 ❌ +0.15%
Auth::get_authorized +3 ❌ +0.15%
NFT::set_minter +3 ❌ +0.15%
Token::set_minter +3 ❌ +0.15%
PrivateFPC::constructor +6 ❌ +0.15%
Lending::get_asset +3 ❌ +0.14%
FPC::constructor +6 ❌ +0.14%
Uniswap::constructor +6 ❌ +0.14%
Auth::constructor +6 ❌ +0.14%
TokenBridge::constructor +6 ❌ +0.14%
TokenBridge::public_dispatch +45 ❌ +0.14%
FeeJuice::check_balance +3 ❌ +0.14%
Auth::get_authorized_delay +3 ❌ +0.14%
EasyPrivateVoting::constructor +6 ❌ +0.14%
Token::shield +12 ❌ +0.13%
Claim::constructor +6 ❌ +0.13%
Spam::public_dispatch +6 ❌ +0.13%
Benchmarking::public_dispatch +9 ❌ +0.13%
Uniswap::_approve_bridge_and_exit_input_asset_to_L1 +12 ❌ +0.13%
NFT::constructor +9 ❌ +0.12%
Token::assert_minter_and_mint +3 ❌ +0.12%
PriceFeed::public_dispatch +6 ❌ +0.12%
FeeJuice::public_dispatch +12 ❌ +0.12%
DocsExample::public_dispatch +7 ❌ +0.12%
Lending::get_position +9 ❌ +0.12%
Crowdfunding::init +6 ❌ +0.12%
Token::constructor +9 ❌ +0.11%
Lending::public_dispatch +90 ❌ +0.11%
PrivateFPC::public_dispatch +6 ❌ +0.11%
Lending::init +3 ❌ +0.11%
Claim::public_dispatch +6 ❌ +0.11%
Lending::_borrow +16 ❌ +0.10%
AvmInitializerTest::constructor +3 ❌ +0.10%
TokenBlacklist::mint_private +6 ❌ +0.10%
AppSubscription::constructor +6 ❌ +0.10%
AvmTest::sha256_hash +3 ❌ +0.08%
AvmTest::bulk_testing +24 ❌ +0.08%
AppSubscription::public_dispatch +6 ❌ +0.07%
Token::mint_private +3 ❌ +0.07%
Parent::public_nested_static_call +3 ❌ +0.07%
CardGame::on_game_joined +3 ❌ +0.06%
NFT::finalize_transfer_to_private +3 ❌ +0.05%
CardGame::start_game +3 ❌ +0.04%
CardGame::on_cards_claimed +3 ❌ +0.04%
Parent::public_dispatch +3 ❌ +0.03%
Lending::update_accumulator +3 ❌ +0.02%
Test::public_dispatch -21 ✅ -0.06%
Test::consume_message_from_arbitrary_sender_public -6 ✅ -0.07%
TokenBridge::claim_public -15 ✅ -0.10%
Uniswap::swap_public -17 ✅ -0.11%
TokenBridge::exit_to_l1_public -9 ✅ -0.11%
Test::consume_mint_public_message -15 ✅ -0.11%
Full diff report 👇
Program Bytecode size in bytes (+/-) %
AvmTest::elliptic_curve_add_and_double 180 (+9) +5.26%
CardGame::on_card_played 5,908 (+185) +3.23%
DocsExample::get_shared_immutable_constrained_public_indirect 432 (+11) +2.61%
Child::set_value_with_two_nested_calls 752 (+14) +1.90%
AvmTest::nested_call_to_add_with_gas 852 (+14) +1.67%
AvmTest::public_dispatch 93,820 (+1,460) +1.58%
Uniswap::_assert_token_is_same 474 (+7) +1.50%
CardGame::public_dispatch 28,156 (+334) +1.20%
Token::public_dispatch 67,534 (+312) +0.46%
NFT::public_dispatch 40,137 (+167) +0.42%
Lending::_deposit 2,504 (+10) +0.40%
Uniswap::public_dispatch 33,645 (+120) +0.36%
InclusionProofs::public_dispatch 5,686 (+20) +0.35%
FPC::public_dispatch 11,779 (+41) +0.35%
Auth::public_dispatch 29,936 (+86) +0.29%
StatefulTest::public_dispatch 12,552 (+36) +0.29%
StatefulTest::public_constructor 4,571 (+13) +0.29%
EasyPrivateVoting::public_dispatch 9,233 (+26) +0.28%
AvmInitializerTest::public_dispatch 3,692 (+10) +0.27%
Child::public_dispatch 5,508 (+14) +0.25%
TokenBlacklist::public_dispatch 131,265 (+332) +0.25%
Lending::_repay 9,387 (+23) +0.25%
TokenBlacklist::constructor 14,778 (+33) +0.22%
AuthRegistry::consume 8,258 (+18) +0.22%
AvmTest::set_storage_map 2,804 (+6) +0.21%
AvmTest::add_storage_map 4,212 (+9) +0.21%
AuthRegistry::set_authorized 2,927 (+6) +0.21%
AuthRegistry::_set_authorized 2,954 (+6) +0.20%
NFT::mint 5,948 (+12) +0.20%
AuthRegistry::is_consumable 3,041 (+6) +0.20%
TokenBlacklist::transfer_public 15,630 (+30) +0.19%
AvmTest::poseidon2_hash 1,566 (+3) +0.19%
TokenBlacklist::mint_public 9,611 (+18) +0.19%
TokenBlacklist::update_roles 14,608 (+27) +0.19%
Token::transfer_public 9,784 (+18) +0.18%
AvmTest::get_args_hash 1,632 (+3) +0.18%
NFT::owner_of 3,284 (+6) +0.18%
Auth::set_authorized_delay 9,918 (+18) +0.18%
AuthRegistry::public_dispatch 23,251 (+42) +0.18%
FeeJuice::_increase_public_balance 3,342 (+6) +0.18%
TokenBlacklist::get_roles 3,353 (+6) +0.18%
Auth::set_authorized 10,114 (+18) +0.18%
TokenBlacklist::_increase_public_balance 3,387 (+6) +0.18%
Token::_increase_public_balance 3,387 (+6) +0.18%
EasyPrivateVoting::add_to_tally_public 3,390 (+6) +0.18%
Spam::public_spam 3,396 (+6) +0.18%
TokenBlacklist::burn_public 10,333 (+18) +0.17%
AuthRegistry::set_reject_all 1,729 (+3) +0.17%
Lending::_withdraw 13,368 (+23) +0.17%
Token::mint_public 5,256 (+9) +0.17%
NFT::_finish_transfer_to_public 1,789 (+3) +0.17%
PriceFeed::set_price 1,813 (+3) +0.17%
AvmTest::read_storage_map 1,817 (+3) +0.17%
StatefulTest::get_public_value 1,828 (+3) +0.16%
AuthRegistry::is_reject_all 1,838 (+3) +0.16%
Token::burn_public 7,363 (+12) +0.16%
PriceFeed::get_price 1,859 (+3) +0.16%
NFT::transfer_in_public 5,606 (+9) +0.16%
Benchmarking::broadcast 1,878 (+3) +0.16%
InclusionProofs::constructor 3,771 (+6) +0.16%
NFT::is_minter 1,888 (+3) +0.16%
Token::is_minter 1,888 (+3) +0.16%
Auth::get_scheduled_authorized 1,893 (+3) +0.16%
Crowdfunding::public_dispatch 8,219 (+13) +0.16%
FeeJuice::balance_of_public 1,920 (+3) +0.16%
StatefulTest::increment_public_value_no_init_check 1,947 (+3) +0.15%
TokenBlacklist::balance_of_public 1,970 (+3) +0.15%
Token::balance_of_public 1,970 (+3) +0.15%
TokenBlacklist::shield 11,902 (+18) +0.15%
Benchmarking::increment_balance 3,984 (+6) +0.15%
StatefulTest::increment_public_value 1,992 (+3) +0.15%
Auth::get_authorized 1,994 (+3) +0.15%
NFT::set_minter 2,024 (+3) +0.15%
Token::set_minter 2,024 (+3) +0.15%
PrivateFPC::constructor 4,100 (+6) +0.15%
Lending::get_asset 2,078 (+3) +0.14%
FPC::constructor 4,169 (+6) +0.14%
Uniswap::constructor 4,169 (+6) +0.14%
Auth::constructor 4,191 (+6) +0.14%
TokenBridge::constructor 4,238 (+6) +0.14%
TokenBridge::public_dispatch 32,279 (+45) +0.14%
FeeJuice::check_balance 2,179 (+3) +0.14%
Auth::get_authorized_delay 2,219 (+3) +0.14%
EasyPrivateVoting::constructor 4,446 (+6) +0.14%
Token::shield 8,932 (+12) +0.13%
Claim::constructor 4,561 (+6) +0.13%
Spam::public_dispatch 4,562 (+6) +0.13%
Benchmarking::public_dispatch 7,024 (+9) +0.13%
Uniswap::_approve_bridge_and_exit_input_asset_to_L1 9,442 (+12) +0.13%
NFT::constructor 7,355 (+9) +0.12%
Token::assert_minter_and_mint 2,458 (+3) +0.12%
PriceFeed::public_dispatch 4,935 (+6) +0.12%
FeeJuice::public_dispatch 9,875 (+12) +0.12%
DocsExample::public_dispatch 5,807 (+7) +0.12%
Lending::get_position 7,620 (+9) +0.12%
Crowdfunding::init 5,135 (+6) +0.12%
Token::constructor 7,844 (+9) +0.11%
Lending::public_dispatch 78,617 (+90) +0.11%
PrivateFPC::public_dispatch 5,250 (+6) +0.11%
Lending::init 2,768 (+3) +0.11%
Claim::public_dispatch 5,711 (+6) +0.11%
Lending::_borrow 15,348 (+16) +0.10%
AvmInitializerTest::constructor 2,880 (+3) +0.10%
TokenBlacklist::mint_private 5,866 (+6) +0.10%
AppSubscription::constructor 6,077 (+6) +0.10%
AvmTest::sha256_hash 3,549 (+3) +0.08%
AvmTest::bulk_testing 31,988 (+24) +0.08%
AppSubscription::public_dispatch 8,046 (+6) +0.07%
Token::mint_private 4,387 (+3) +0.07%
Parent::public_nested_static_call 4,503 (+3) +0.07%
CardGame::on_game_joined 4,767 (+3) +0.06%
NFT::finalize_transfer_to_private 6,180 (+3) +0.05%
CardGame::start_game 7,285 (+3) +0.04%
CardGame::on_cards_claimed 7,475 (+3) +0.04%
Parent::public_dispatch 11,279 (+3) +0.03%
Lending::update_accumulator 12,821 (+3) +0.02%
Test::public_dispatch 32,778 (-21) -0.06%
Test::consume_message_from_arbitrary_sender_public 8,632 (-6) -0.07%
TokenBridge::claim_public 14,799 (-15) -0.10%
Uniswap::swap_public 15,342 (-17) -0.11%
TokenBridge::exit_to_l1_public 8,032 (-9) -0.11%
Test::consume_mint_public_message 13,363 (-15) -0.11%

@IlyasRidhuan
fix(avm): guard against inf (#9081)
fc95fa1 
This is a bit of a hack for now. The bb wasm interface represents
infinity in different ways - none of which utilise the boolean
`isInfinite` flag. Since aztec-nr shouldn't run into this issue, we
solve it in an avm-specific way.

Specifically, we just guard the opcodes to treat infinities separately -
without calling the bb wasm interface

Closes #9020
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 8, 2024

Changes to circuit sizes

Generated at commit: 912b40bb1897f34cca8133059119a51978f514bb, compared to commit: f9fc73a40f5b9d11ad92a6cee3e29d3fcc80425e

🧾 Summary (100% most significant diffs)

Program ACIR opcodes (+/-) % Circuit size (+/-) %
public_kernel_merge_simulated 0 ➖ 0.00% +3,130 ❌ +313000.00%
public_kernel_inner_simulated 0 ➖ 0.00% +2,475 ❌ +247500.00%
private_kernel_tail_to_public_simulated 0 ➖ 0.00% +3,130 ❌ +78250.00%
public_kernel_tail_simulated 0 ➖ 0.00% +667 ❌ +66700.00%
private_kernel_init_simulated 0 ➖ 0.00% +1,973 ❌ +49325.00%
private_kernel_inner_simulated 0 ➖ 0.00% +1,973 ❌ +49325.00%
private_kernel_reset_simulated 0 ➖ 0.00% +1,973 ❌ +49325.00%
private_kernel_reset_simulated_4_4_4_4_4_4_4_4_1 0 ➖ 0.00% +1,973 ❌ +49325.00%
private_kernel_empty_simulated 0 ➖ 0.00% +667 ❌ +16675.00%
private_kernel_tail_simulated 0 ➖ 0.00% +667 ❌ +16675.00%
rollup_base_simulated 0 ➖ 0.00% +33 ❌ +3300.00%
empty_nested 0 ➖ +∞% +4 ❌ +100.00%
empty_nested_simulated 0 ➖ 0.00% +4 ❌ +100.00%
parity_base -3,282 ✅ -61.11% +21,130 ❌ +65.46%
private_kernel_tail +3 ❌ +0.06% +2,550 ❌ +28.26%
private_kernel_empty 0 ➖ 0.00% +661 ❌ +19.06%
private_kernel_tail_to_public +5 ❌ +0.02% +5,427 ❌ +13.74%
private_kernel_inner +19 ❌ +0.04% +4,926 ❌ +9.49%
rollup_block_root_empty -32 ✅ -25.60% +73 ❌ +2.51%
private_kernel_reset_4_4_4_4_4_4_4_4_1 +4 ❌ +0.01% +1,090 ❌ +1.46%
public_kernel_merge 0 ➖ 0.00% +14,254 ❌ +1.29%
rollup_block_root -2,220 ✅ -53.03% +31,627 ❌ +1.11%
parity_root -3,282 ✅ -60.79% +41,777 ❌ +1.11%
rollup_merge -2,188 ✅ -59.60% +20,436 ❌ +1.08%
private_kernel_init +8 ❌ +0.03% +327 ❌ +1.03%
private_kernel_reset +4 ❌ +0.00% +4,295 ❌ +0.92%
public_kernel_tail +750 ❌ +0.29% +11,477 ❌ +0.51%
public_kernel_inner 0 ➖ 0.00% +1,728 ❌ +0.33%
rollup_block_merge -22,675 ✅ -54.59% -30,581 ✅ -1.54%
rollup_root -22,675 ✅ -54.61% -30,597 ✅ -1.54%
rollup_base -248,861 ✅ -37.04% -210,708 ✅ -5.98%
Full diff report 👇
Program ACIR opcodes (+/-) % Circuit size (+/-) %
public_kernel_merge_simulated 1 (0) 0.00% 3,131 (+3,130) +313000.00%
public_kernel_inner_simulated 1 (0) 0.00% 2,476 (+2,475) +247500.00%
private_kernel_tail_to_public_simulated 1 (0) 0.00% 3,134 (+3,130) +78250.00%
public_kernel_tail_simulated 1 (0) 0.00% 668 (+667) +66700.00%
private_kernel_init_simulated 1 (0) 0.00% 1,977 (+1,973) +49325.00%
private_kernel_inner_simulated 1 (0) 0.00% 1,977 (+1,973) +49325.00%
private_kernel_reset_simulated 1 (0) 0.00% 1,977 (+1,973) +49325.00%
private_kernel_reset_simulated_4_4_4_4_4_4_4_4_1 1 (0) 0.00% 1,977 (+1,973) +49325.00%
private_kernel_empty_simulated 1 (0) 0.00% 671 (+667) +16675.00%
private_kernel_tail_simulated 1 (0) 0.00% 671 (+667) +16675.00%
rollup_base_simulated 1 (0) 0.00% 34 (+33) +3300.00%
empty_nested 0 (0) +∞% 8 (+4) +100.00%
empty_nested_simulated 1 (0) 0.00% 8 (+4) +100.00%
parity_base 2,089 (-3,282) -61.11% 53,408 (+21,130) +65.46%
private_kernel_tail 4,745 (+3) +0.06% 11,573 (+2,550) +28.26%
private_kernel_empty 670 (0) 0.00% 4,129 (+661) +19.06%
private_kernel_tail_to_public 29,831 (+5) +0.02% 44,919 (+5,427) +13.74%
private_kernel_inner 43,951 (+19) +0.04% 56,831 (+4,926) +9.49%
rollup_block_root_empty 93 (-32) -25.60% 2,980 (+73) +2.51%
private_kernel_reset_4_4_4_4_4_4_4_4_1 34,899 (+4) +0.01% 75,549 (+1,090) +1.46%
public_kernel_merge 53,488 (0) 0.00% 1,117,793 (+14,254) +1.29%
rollup_block_root 1,966 (-2,220) -53.03% 2,868,779 (+31,627) +1.11%
parity_root 2,117 (-3,282) -60.79% 3,816,888 (+41,777) +1.11%
rollup_merge 1,483 (-2,188) -59.60% 1,916,503 (+20,436) +1.08%
private_kernel_init 24,901 (+8) +0.03% 32,162 (+327) +1.03%
private_kernel_reset 91,935 (+4) +0.00% 472,006 (+4,295) +0.92%
public_kernel_tail 259,172 (+750) +0.29% 2,281,716 (+11,477) +0.51%
public_kernel_inner 268,756 (0) 0.00% 518,289 (+1,728) +0.33%
rollup_block_merge 18,860 (-22,675) -54.59% 1,952,775 (-30,581) -1.54%
rollup_root 18,844 (-22,675) -54.61% 1,952,743 (-30,597) -1.54%
rollup_base 423,026 (-248,861) -37.04% 3,315,266 (-210,708) -5.98%

@IlyasRidhuan IlyasRidhuan merged commit 757ccef into master Oct 8, 2024
50 checks passed
@IlyasRidhuan IlyasRidhuan deleted the arv/bug_ecadd_avm branch October 8, 2024 21:02
@AztecBot AztecBot mentioned this pull request Oct 8, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@IlyasRidhuan IlyasRidhuan IlyasRidhuan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sirasistant @IlyasRidhuan