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

feat: Encode static error strings in the ABI #9552

Merged
merged 26 commits into from
Nov 4, 2024
Merged

feat: Encode static error strings in the ABI #9552

merged 26 commits into from
Nov 4, 2024

Conversation

sirasistant
Copy link
Collaborator

@sirasistant sirasistant commented Oct 29, 2024 •
edited
Loading

Avoids embedding revert string in circuits. Instead, static string errors get a specific selector, and they are encoded in the ABI. We use noir_abi to resolve those messages in the error case.

@github-actions GitHub Actions
Copy link
Contributor

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

Changes to public function bytecode sizes

Generated at commit: 947597260c6e84446b2a5fbf6b952c400d2dfe88, compared to commit: f47cc17046001fd86db0324e800847d634aefc03

🧾 Summary (100% most significant diffs)

Program Bytecode size in bytes (+/-) %
Test::assert_public_global_vars +88 ❌ +29.33%
Test::dummy_public_call +18 ❌ +14.17%
DocsExample::update_leader +46 ❌ +13.86%
AvmTest::assert_timestamp +18 ❌ +13.33%
AvmTest::assert_nullifier_exists +18 ❌ +13.24%
AvmTest::emit_nullifier_and_check +18 ❌ +12.86%
AvmTest::assert_same +18 ❌ +12.86%
TokenBridge::_assert_token_is_same +74 ❌ +12.82%
InclusionProofs::push_nullifier_public +18 ❌ +12.41%
AvmTest::set_storage_single +32 ❌ +11.94%
AvmTest::test_get_contract_instance_matches +46 ❌ +11.65%
AvmTest::test_get_contract_instance +46 ❌ +11.50%
TokenBlacklist::total_supply +74 ❌ +11.25%
Token::total_supply +74 ❌ +11.25%
AvmTest::set_storage_list +32 ❌ +11.11%
NFT::public_get_symbol +60 ❌ +10.83%
TokenBridge::get_token +60 ❌ +10.83%
Token::get_admin +60 ❌ +10.83%
NFT::public_get_name +60 ❌ +10.73%
NFT::get_admin +60 ❌ +10.73%
Token::public_get_name +60 ❌ +10.73%
Token::public_get_symbol +60 ❌ +10.73%
InclusionProofs::test_nullifier_inclusion_from_public +18 ❌ +10.53%
Token::public_get_decimals +60 ❌ +10.45%
TokenBridge::get_portal_address_public +60 ❌ +10.15%
StaticChild::pub_get_value +18 ❌ +9.78%
AvmTest::assertion_failure +18 ❌ +9.73%
Uniswap::_assert_token_is_same +98 ❌ +9.72%
AvmTest::check_selector +18 ❌ +9.14%
AvmTest::assert_calldata_copy +42 ❌ +8.81%
Child::pub_set_value +32 ❌ +8.79%
StaticChild::pub_set_value +32 ❌ +8.79%
AvmInitializerTest::read_storage_immutable +46 ❌ +8.73%
AuthWitTest::consume_public +46 ❌ +8.10%
AvmTest::add_u128 +18 ❌ +8.07%
AvmTest::u128_from_integer_overflow +60 ❌ +8.05%
DocsExample::get_shared_immutable_constrained_public +46 ❌ +8.04%
TokenBlacklist::_reduce_total_supply +80 ❌ +7.79%
Token::_reduce_total_supply +80 ❌ +7.75%
Lending::get_assets +60 ❌ +7.10%
EasyPrivateVoting::end_vote +52 ❌ +6.93%
NFT::set_admin +52 ❌ +6.92%
Token::set_admin +52 ❌ +6.92%
AvmTest::read_storage_single +32 ❌ +6.57%
TokenBridge::_call_mint_on_token +60 ❌ +6.48%
Parent::pub_entry_point +32 ❌ +5.95%
StaticParent::public_call +32 ❌ +5.95%
FeeJuice::set_portal +52 ❌ +5.13%
Router::_check_block_number +46 ❌ +5.01%
Router::_check_timestamp +46 ❌ +4.92%
AvmTest::read_storage_list +32 ❌ +4.88%
StaticChild::pub_illegal_inc_value +38 ❌ +4.68%
AvmTest::nested_call_to_add_with_gas +42 ❌ +4.67%
Child::pub_inc_value_internal +38 ❌ +4.65%
StaticParent::public_get_value_from_child +42 ❌ +4.61%
AvmTest::nested_static_call_to_add +42 ❌ +4.54%
AvmTest::nested_call_to_assert_same +42 ❌ +4.54%
AvmTest::nested_call_to_add +42 ❌ +4.54%
FPC::pay_refund +88 ❌ +4.51%
DocsExample::get_shared_immutable_constrained_public_multiple +32 ❌ +4.31%
Router::public_dispatch +104 ❌ +4.09%
DocsExample::get_shared_immutable_constrained_public_indirect +38 ❌ +3.90%
FPC::prepare_fee +74 ❌ +3.88%
AvmTest::new_note_hash +4 ❌ +3.81%
AvmTest::new_nullifier +4 ❌ +3.81%
Test::emit_nullifier_public +4 ❌ +3.81%
AvmTest::get_timestamp +4 ❌ +3.77%
AvmTest::set_opcode_u8 +4 ❌ +3.77%
AvmTest::get_da_gas_left +4 ❌ +3.77%
AvmTest::get_transaction_fee +4 ❌ +3.77%
AvmTest::get_l2_gas_left +4 ❌ +3.77%
AvmTest::get_sender +4 ❌ +3.77%
AvmTest::get_address +4 ❌ +3.77%
AvmTest::get_fee_per_l2_gas +4 ❌ +3.77%
AvmTest::get_version +4 ❌ +3.77%
AvmTest::get_block_number +4 ❌ +3.77%
AvmTest::get_chain_id +4 ❌ +3.77%
AvmTest::get_fee_per_da_gas +4 ❌ +3.77%
AvmTest::nullifier_collision +4 ❌ +3.67%
AvmTest::set_opcode_u32 +4 ❌ +3.64%
FPC::pay_refund_with_shielded_rebate +60 ❌ +3.59%
AvmTest::send_l2_to_l1_msg +4 ❌ +3.54%
Test::create_l2_to_l1_message_arbitrary_recipient_public +4 ❌ +3.54%
AvmTest::set_opcode_u64 +4 ❌ +3.51%
AvmTest::set_read_storage_single +24 ❌ +3.46%
DocsExample::spend_public_authwit +4 ❌ +3.45%
DocsExample::initialize_shared_immutable +30 ❌ +3.31%
DocsExample::initialize_public_immutable +30 ❌ +3.31%
AvmTest::add_args_return +4 ❌ +3.28%
AvmTest::set_opcode_small_field +4 ❌ +3.28%
Lending::withdraw_public +60 ❌ +3.28%
Lending::borrow_public +60 ❌ +3.28%
AvmTest::nullifier_exists +4 ❌ +3.23%
AvmTest::note_hash_exists +4 ❌ +3.20%
AvmTest::l1_to_l2_msg_exists +4 ❌ +3.20%
AvmTest::modulo2 +4 ❌ +3.13%
Child::pub_inc_value +24 ❌ +3.06%
StaticChild::pub_inc_value +24 ❌ +3.06%
Child::set_value_with_two_nested_calls +32 ❌ +3.00%
Parent::public_static_call +32 ❌ +2.91%
StaticParent::public_static_call +32 ❌ +2.91%
AvmTest::set_opcode_big_field +4 ❌ +2.90%
AvmTest::set_opcode_really_big_field +4 ❌ +2.90%
Child::set_value_twice_with_nested_last +34 ❌ +2.88%
Child::set_value_twice_with_nested_first +34 ❌ +2.88%
AvmTest::pedersen_commit +34 ❌ +2.86%
Test::is_time_equal +4 ❌ +2.84%
TokenBlacklist::balance_of_public +56 ❌ +2.71%
Token::balance_of_public +56 ❌ +2.71%
Child::pub_get_value +4 ❌ +2.63%
Lending::repay_public +60 ❌ +2.54%
Parent::pub_entry_point_twice +24 ❌ +2.52%
Lending::deposit_public +60 ❌ +2.42%
Crowdfunding::_publish_donation_receipts +32 ❌ +2.35%
FeeJuice::check_balance +56 ❌ +2.35%
AuthWitTest::public_dispatch +44 ❌ +2.34%
AvmTest::get_function_selector +4 ❌ +2.27%
NFT::_finish_transfer_to_public +42 ❌ +2.23%
NFT::is_minter +42 ❌ +2.12%
Token::is_minter +42 ❌ +2.12%
FeeJuice::balance_of_public +42 ❌ +2.07%
NFT::_store_payload_in_transient_storage_unsafe +28 ❌ +2.04%
Token::_store_payload_in_transient_storage_unsafe +28 ❌ +2.04%
Lending::get_asset +42 ❌ +1.94%
EasyPrivateVoting::add_to_tally_public +48 ❌ +1.92%
Auth::get_authorized_delay +42 ❌ +1.78%
NFT::owner_of +44 ❌ +1.60%
Token::set_minter +34 ❌ +1.57%
NFT::set_minter +34 ❌ +1.57%
NFT::transfer_in_public +56 ❌ +1.53%
NFT::mint +50 ❌ +1.51%
AvmInitializerTest::public_dispatch +48 ❌ +1.46%
Auth::get_scheduled_authorized +28 ❌ +1.46%
PriceFeed::set_price +28 ❌ +1.46%
StatefulTest::get_public_value +28 ❌ +1.45%
AuthRegistry::is_reject_all +28 ❌ +1.44%
PriceFeed::get_price +28 ❌ +1.43%
Auth::get_authorized +28 ❌ +1.39%
AvmInitializerTest::constructor +34 ❌ +1.35%
Token::burn_public +62 ❌ +1.34%
Auth::constructor +40 ❌ +1.31%
Token::mint_private_old +44 ❌ +1.29%
ImportTest::public_dispatch +20 ❌ +1.28%
TokenBlacklist::_increase_public_balance +36 ❌ +1.28%
Token::_increase_public_balance +36 ❌ +1.28%
StaticParent::public_nested_static_call +30 ❌ +1.26%
TokenBlacklist::get_roles +34 ❌ +1.24%
Token::transfer_public +58 ❌ +1.22%
Token::shield +62 ❌ +1.20%
Lending::_deposit +36 ❌ +1.20%
AuthRegistry::_set_authorized +24 ❌ +1.18%
FeeJuice::_increase_public_balance +30 ❌ +1.17%
Lending::get_position +60 ❌ +1.16%
AvmTest::return_oracle +4 ❌ +1.14%
AuthRegistry::is_consumable +24 ❌ +1.14%
EasyPrivateVoting::public_dispatch +68 ❌ +1.13%
TokenBlacklist::transfer_public +70 ❌ +1.12%
StatefulTest::public_constructor +42 ❌ +1.10%
AvmTest::u128_addition_overflow +14 ❌ +1.07%
StaticChild::public_dispatch +32 ❌ +1.06%
Lending::update_accumulator +60 ❌ +1.05%
Token::mint_public +36 ❌ +1.03%
TokenBlacklist::burn_public +60 ❌ +1.03%
Lending::_repay +62 ❌ +1.01%
NFT::constructor +48 ❌ +1.01%
AvmTest::revert_oracle +4 ❌ +1.00%
AvmTest::to_radix_le +4 ❌ +0.99%
TokenBlacklist::shield +60 ❌ +0.94%
StatefulTest::increment_public_value +20 ❌ +0.94%
AvmTest::variable_base_msm +4 ❌ +0.89%
Token::constructor +44 ❌ +0.86%
AvmTest::nested_static_call_to_set_storage +4 ❌ +0.86%
FPC::constructor +26 ❌ +0.86%
Uniswap::constructor +26 ❌ +0.86%
ImportTest::pub_call_public_fn +4 ❌ +0.86%
AvmTest::create_same_nullifier_in_nested_call +4 ❌ +0.85%
AvmTest::keccak_f1600 +4 ❌ +0.83%
AvmTest::create_different_nullifier_in_nested_call +4 ❌ +0.83%
Lending::init +26 ❌ +0.82%
EasyPrivateVoting::constructor +26 ❌ +0.81%
TokenBridge::constructor +26 ❌ +0.81%
Claim::constructor +26 ❌ +0.80%
Spam::public_spam +22 ❌ +0.79%
InclusionProofs::constructor +20 ❌ +0.77%
AuthRegistry::set_reject_all +14 ❌ +0.76%
TokenBlacklist::mint_public +34 ❌ +0.74%
AvmTest::read_storage_map +14 ❌ +0.73%
AvmTest::set_storage_map +14 ❌ +0.72%
AvmTest::elliptic_curve_add +4 ❌ +0.71%
Benchmarking::broadcast +14 ❌ +0.71%
TokenBlacklist::mint_private +28 ❌ +0.66%
Crowdfunding::init +22 ❌ +0.63%
FeeJuice::public_dispatch +38 ❌ +0.61%
AppSubscription::constructor +22 ❌ +0.60%
PrivateFPC::constructor +18 ❌ +0.57%
AvmTest::elliptic_curve_add_and_double +4 ❌ +0.57%
AuthRegistry::set_authorized +10 ❌ +0.50%
Parent::public_nested_static_call +16 ❌ +0.48%
Lending::_borrow +42 ❌ +0.47%
AvmTest::poseidon2_hash +8 ❌ +0.46%
AvmTest::get_args_hash +8 ❌ +0.45%
Benchmarking::increment_balance +16 ❌ +0.43%
FPC::public_dispatch +38 ❌ +0.40%
AvmTest::returndata_copy_oracle +8 ❌ +0.40%
Lending::_withdraw +34 ❌ +0.37%
Uniswap::_approve_bridge_and_exit_input_asset_to_L1 +22 ❌ +0.34%
InclusionProofs::public_dispatch +14 ❌ +0.34%
AvmTest::debug_logging +4 ❌ +0.31%
AuthRegistry::consume +10 ❌ +0.31%
StatefulTest::increment_public_value_no_init_check +6 ❌ +0.29%
Test::emit_unencrypted +4 ❌ +0.24%
Auth::public_dispatch +20 ❌ +0.19%
Claim::public_dispatch +6 ❌ +0.14%
Token::complete_refund +6 ❌ +0.10%
Child::public_dispatch +4 ❌ +0.07%
Spam::public_dispatch +2 ❌ +0.05%
AppSubscription::public_dispatch +2 ❌ +0.04%
PrivateFPC::public_dispatch -2 ✅ -0.05%
StatefulTest::public_dispatch -4 ✅ -0.05%
AvmTest::add_storage_map -2 ✅ -0.09%
TokenBlacklist::public_dispatch -38 ✅ -0.15%
PriceFeed::public_dispatch -6 ✅ -0.15%
Lending::public_dispatch -52 ✅ -0.18%
AuthRegistry::public_dispatch -16 ✅ -0.19%
CardGame::on_game_joined -10 ✅ -0.20%
Token::public_dispatch -90 ✅ -0.24%
NFT::public_dispatch -90 ✅ -0.34%
Crowdfunding::public_dispatch -26 ✅ -0.42%
Benchmarking::public_dispatch -24 ✅ -0.42%
TestLog::emit_unencrypted_events -8 ✅ -0.43%
NFT::_finalize_transfer_to_private_unsafe -38 ✅ -0.58%
AvmTest::emit_unencrypted_log -18 ✅ -0.60%
Token::_finalize_mint_to_private_unsafe -46 ✅ -0.64%
Token::_finalize_transfer_to_private_unsafe -50 ✅ -0.67%
TokenBlacklist::update_roles -54 ✅ -0.70%
TokenBlacklist::constructor -62 ✅ -0.78%
NFT::finalize_transfer_to_private -52 ✅ -0.80%
CardGame::on_card_played -50 ✅ -0.83%
Token::finalize_mint_to_private -60 ✅ -0.84%
Auth::set_authorized -52 ✅ -0.85%
Token::finalize_transfer_to_private -64 ✅ -0.87%
Auth::set_authorized_delay -52 ✅ -0.87%
AvmTest::pedersen_hash -26 ✅ -0.88%
AvmTest::pedersen_hash_with_index -26 ✅ -0.88%
Parent::public_dispatch -84 ✅ -0.98%
StaticParent::public_dispatch -90 ✅ -1.22%
Uniswap::public_dispatch -318 ✅ -1.25%
Test::create_l2_to_l1_message_public -26 ✅ -1.26%
CardGame::on_cards_claimed -100 ✅ -1.34%
AvmTest::public_dispatch -874 ✅ -1.44%
Uniswap::swap_public -236 ✅ -1.48%
TokenBridge::exit_to_l1_public -132 ✅ -1.58%
TokenBridge::public_dispatch -370 ✅ -1.61%
AvmTest::keccak_hash -46 ✅ -1.65%
TestLog::public_dispatch -56 ✅ -1.69%
AvmTest::bulk_testing -472 ✅ -1.82%
CardGame::public_dispatch -350 ✅ -2.10%
Test::public_dispatch -438 ✅ -2.17%
TokenBridge::claim_public -276 ✅ -2.17%
AvmTest::sha256_hash -86 ✅ -2.17%
Test::consume_message_from_arbitrary_sender_public -222 ✅ -2.71%
CardGame::start_game -198 ✅ -2.76%
Test::consume_mint_public_message -310 ✅ -2.84%
Full diff report 👇
Program Bytecode size in bytes (+/-) %
Test::assert_public_global_vars 388 (+88) +29.33%
Test::dummy_public_call 145 (+18) +14.17%
DocsExample::update_leader 378 (+46) +13.86%
AvmTest::assert_timestamp 153 (+18) +13.33%
AvmTest::assert_nullifier_exists 154 (+18) +13.24%
AvmTest::emit_nullifier_and_check 158 (+18) +12.86%
AvmTest::assert_same 158 (+18) +12.86%
TokenBridge::_assert_token_is_same 651 (+74) +12.82%
InclusionProofs::push_nullifier_public 163 (+18) +12.41%
AvmTest::set_storage_single 300 (+32) +11.94%
AvmTest::test_get_contract_instance_matches 441 (+46) +11.65%
AvmTest::test_get_contract_instance 446 (+46) +11.50%
TokenBlacklist::total_supply 732 (+74) +11.25%
Token::total_supply 732 (+74) +11.25%
AvmTest::set_storage_list 320 (+32) +11.11%
NFT::public_get_symbol 614 (+60) +10.83%
TokenBridge::get_token 614 (+60) +10.83%
Token::get_admin 614 (+60) +10.83%
NFT::public_get_name 619 (+60) +10.73%
NFT::get_admin 619 (+60) +10.73%
Token::public_get_name 619 (+60) +10.73%
Token::public_get_symbol 619 (+60) +10.73%
InclusionProofs::test_nullifier_inclusion_from_public 189 (+18) +10.53%
Token::public_get_decimals 634 (+60) +10.45%
TokenBridge::get_portal_address_public 651 (+60) +10.15%
StaticChild::pub_get_value 202 (+18) +9.78%
AvmTest::assertion_failure 203 (+18) +9.73%
Uniswap::_assert_token_is_same 1,106 (+98) +9.72%
AvmTest::check_selector 215 (+18) +9.14%
AvmTest::assert_calldata_copy 519 (+42) +8.81%
Child::pub_set_value 396 (+32) +8.79%
StaticChild::pub_set_value 396 (+32) +8.79%
AvmInitializerTest::read_storage_immutable 573 (+46) +8.73%
AuthWitTest::consume_public 614 (+46) +8.10%
AvmTest::add_u128 241 (+18) +8.07%
AvmTest::u128_from_integer_overflow 805 (+60) +8.05%
DocsExample::get_shared_immutable_constrained_public 618 (+46) +8.04%
TokenBlacklist::_reduce_total_supply 1,107 (+80) +7.79%
Token::_reduce_total_supply 1,112 (+80) +7.75%
Lending::get_assets 905 (+60) +7.10%
EasyPrivateVoting::end_vote 802 (+52) +6.93%
NFT::set_admin 803 (+52) +6.92%
Token::set_admin 803 (+52) +6.92%
AvmTest::read_storage_single 519 (+32) +6.57%
TokenBridge::_call_mint_on_token 986 (+60) +6.48%
Parent::pub_entry_point 570 (+32) +5.95%
StaticParent::public_call 570 (+32) +5.95%
FeeJuice::set_portal 1,066 (+52) +5.13%
Router::_check_block_number 964 (+46) +5.01%
Router::_check_timestamp 981 (+46) +4.92%
AvmTest::read_storage_list 688 (+32) +4.88%
StaticChild::pub_illegal_inc_value 850 (+38) +4.68%
AvmTest::nested_call_to_add_with_gas 942 (+42) +4.67%
Child::pub_inc_value_internal 855 (+38) +4.65%
StaticParent::public_get_value_from_child 953 (+42) +4.61%
AvmTest::nested_static_call_to_add 967 (+42) +4.54%
AvmTest::nested_call_to_assert_same 967 (+42) +4.54%
AvmTest::nested_call_to_add 967 (+42) +4.54%
FPC::pay_refund 2,038 (+88) +4.51%
DocsExample::get_shared_immutable_constrained_public_multiple 775 (+32) +4.31%
Router::public_dispatch 2,645 (+104) +4.09%
DocsExample::get_shared_immutable_constrained_public_indirect 1,013 (+38) +3.90%
FPC::prepare_fee 1,983 (+74) +3.88%
AvmTest::new_note_hash 109 (+4) +3.81%
AvmTest::new_nullifier 109 (+4) +3.81%
Test::emit_nullifier_public 109 (+4) +3.81%
AvmTest::get_timestamp 110 (+4) +3.77%
AvmTest::set_opcode_u8 110 (+4) +3.77%
AvmTest::get_da_gas_left 110 (+4) +3.77%
AvmTest::get_transaction_fee 110 (+4) +3.77%
AvmTest::get_l2_gas_left 110 (+4) +3.77%
AvmTest::get_sender 110 (+4) +3.77%
AvmTest::get_address 110 (+4) +3.77%
AvmTest::get_fee_per_l2_gas 110 (+4) +3.77%
AvmTest::get_version 110 (+4) +3.77%
AvmTest::get_block_number 110 (+4) +3.77%
AvmTest::get_chain_id 110 (+4) +3.77%
AvmTest::get_fee_per_da_gas 110 (+4) +3.77%
AvmTest::nullifier_collision 113 (+4) +3.67%
AvmTest::set_opcode_u32 114 (+4) +3.64%
FPC::pay_refund_with_shielded_rebate 1,732 (+60) +3.59%
AvmTest::send_l2_to_l1_msg 117 (+4) +3.54%
Test::create_l2_to_l1_message_arbitrary_recipient_public 117 (+4) +3.54%
AvmTest::set_opcode_u64 118 (+4) +3.51%
AvmTest::set_read_storage_single 718 (+24) +3.46%
DocsExample::spend_public_authwit 120 (+4) +3.45%
DocsExample::initialize_shared_immutable 935 (+30) +3.31%
DocsExample::initialize_public_immutable 935 (+30) +3.31%
AvmTest::add_args_return 126 (+4) +3.28%
AvmTest::set_opcode_small_field 126 (+4) +3.28%
Lending::withdraw_public 1,891 (+60) +3.28%
Lending::borrow_public 1,891 (+60) +3.28%
AvmTest::nullifier_exists 128 (+4) +3.23%
AvmTest::note_hash_exists 129 (+4) +3.20%
AvmTest::l1_to_l2_msg_exists 129 (+4) +3.20%
AvmTest::modulo2 132 (+4) +3.13%
Child::pub_inc_value 809 (+24) +3.06%
StaticChild::pub_inc_value 809 (+24) +3.06%
Child::set_value_with_two_nested_calls 1,100 (+32) +3.00%
Parent::public_static_call 1,130 (+32) +2.91%
StaticParent::public_static_call 1,130 (+32) +2.91%
AvmTest::set_opcode_big_field 142 (+4) +2.90%
AvmTest::set_opcode_really_big_field 142 (+4) +2.90%
Child::set_value_twice_with_nested_last 1,216 (+34) +2.88%
Child::set_value_twice_with_nested_first 1,216 (+34) +2.88%
AvmTest::pedersen_commit 1,223 (+34) +2.86%
Test::is_time_equal 145 (+4) +2.84%
TokenBlacklist::balance_of_public 2,122 (+56) +2.71%
Token::balance_of_public 2,122 (+56) +2.71%
Child::pub_get_value 156 (+4) +2.63%
Lending::repay_public 2,426 (+60) +2.54%
Parent::pub_entry_point_twice 976 (+24) +2.52%
Lending::deposit_public 2,543 (+60) +2.42%
Crowdfunding::_publish_donation_receipts 1,391 (+32) +2.35%
FeeJuice::check_balance 2,441 (+56) +2.35%
AuthWitTest::public_dispatch 1,928 (+44) +2.34%
AvmTest::get_function_selector 180 (+4) +2.27%
NFT::_finish_transfer_to_public 1,929 (+42) +2.23%
NFT::is_minter 2,024 (+42) +2.12%
Token::is_minter 2,024 (+42) +2.12%
FeeJuice::balance_of_public 2,068 (+42) +2.07%
NFT::_store_payload_in_transient_storage_unsafe 1,403 (+28) +2.04%
Token::_store_payload_in_transient_storage_unsafe 1,403 (+28) +2.04%
Lending::get_asset 2,209 (+42) +1.94%
EasyPrivateVoting::add_to_tally_public 2,547 (+48) +1.92%
Auth::get_authorized_delay 2,398 (+42) +1.78%
NFT::owner_of 2,796 (+44) +1.60%
Token::set_minter 2,200 (+34) +1.57%
NFT::set_minter 2,205 (+34) +1.57%
NFT::transfer_in_public 3,714 (+56) +1.53%
NFT::mint 3,364 (+50) +1.51%
AvmInitializerTest::public_dispatch 3,326 (+48) +1.46%
Auth::get_scheduled_authorized 1,941 (+28) +1.46%
PriceFeed::set_price 1,946 (+28) +1.46%
StatefulTest::get_public_value 1,960 (+28) +1.45%
AuthRegistry::is_reject_all 1,970 (+28) +1.44%
PriceFeed::get_price 1,988 (+28) +1.43%
Auth::get_authorized 2,041 (+28) +1.39%
AvmInitializerTest::constructor 2,545 (+34) +1.35%
Token::burn_public 4,689 (+62) +1.34%
Auth::constructor 3,097 (+40) +1.31%
Token::mint_private_old 3,454 (+44) +1.29%
ImportTest::public_dispatch 1,577 (+20) +1.28%
TokenBlacklist::_increase_public_balance 2,855 (+36) +1.28%
Token::_increase_public_balance 2,855 (+36) +1.28%
StaticParent::public_nested_static_call 2,413 (+30) +1.26%
TokenBlacklist::get_roles 2,773 (+34) +1.24%
Token::transfer_public 4,797 (+58) +1.22%
Token::shield 5,222 (+62) +1.20%
Lending::_deposit 3,041 (+36) +1.20%
AuthRegistry::_set_authorized 2,054 (+24) +1.18%
FeeJuice::_increase_public_balance 2,586 (+30) +1.17%
Lending::get_position 5,238 (+60) +1.16%
AvmTest::return_oracle 354 (+4) +1.14%
AuthRegistry::is_consumable 2,137 (+24) +1.14%
EasyPrivateVoting::public_dispatch 6,101 (+68) +1.13%
TokenBlacklist::transfer_public 6,311 (+70) +1.12%
StatefulTest::public_constructor 3,860 (+42) +1.10%
AvmTest::u128_addition_overflow 1,325 (+14) +1.07%
StaticChild::public_dispatch 3,046 (+32) +1.06%
Lending::update_accumulator 5,764 (+60) +1.05%
Token::mint_public 3,525 (+36) +1.03%
TokenBlacklist::burn_public 5,878 (+60) +1.03%
Lending::_repay 6,179 (+62) +1.01%
NFT::constructor 4,787 (+48) +1.01%
AvmTest::revert_oracle 404 (+4) +1.00%
AvmTest::to_radix_le 409 (+4) +0.99%
TokenBlacklist::shield 6,416 (+60) +0.94%
StatefulTest::increment_public_value 2,147 (+20) +0.94%
AvmTest::variable_base_msm 453 (+4) +0.89%
Token::constructor 5,143 (+44) +0.86%
AvmTest::nested_static_call_to_set_storage 470 (+4) +0.86%
FPC::constructor 3,056 (+26) +0.86%
Uniswap::constructor 3,056 (+26) +0.86%
ImportTest::pub_call_public_fn 471 (+4) +0.86%
AvmTest::create_same_nullifier_in_nested_call 476 (+4) +0.85%
AvmTest::keccak_f1600 486 (+4) +0.83%
AvmTest::create_different_nullifier_in_nested_call 486 (+4) +0.83%
Lending::init 3,192 (+26) +0.82%
EasyPrivateVoting::constructor 3,223 (+26) +0.81%
TokenBridge::constructor 3,232 (+26) +0.81%
Claim::constructor 3,269 (+26) +0.80%
Spam::public_spam 2,796 (+22) +0.79%
InclusionProofs::constructor 2,621 (+20) +0.77%
AuthRegistry::set_reject_all 1,845 (+14) +0.76%
TokenBlacklist::mint_public 4,611 (+34) +0.74%
AvmTest::read_storage_map 1,929 (+14) +0.73%
AvmTest::set_storage_map 1,956 (+14) +0.72%
AvmTest::elliptic_curve_add 568 (+4) +0.71%
Benchmarking::broadcast 1,994 (+14) +0.71%
TokenBlacklist::mint_private 4,294 (+28) +0.66%
Crowdfunding::init 3,535 (+22) +0.63%
FeeJuice::public_dispatch 6,265 (+38) +0.61%
AppSubscription::constructor 3,690 (+22) +0.60%
PrivateFPC::constructor 3,152 (+18) +0.57%
AvmTest::elliptic_curve_add_and_double 701 (+4) +0.57%
AuthRegistry::set_authorized 2,007 (+10) +0.50%
Parent::public_nested_static_call 3,364 (+16) +0.48%
Lending::_borrow 9,009 (+42) +0.47%
AvmTest::poseidon2_hash 1,734 (+8) +0.46%
AvmTest::get_args_hash 1,804 (+8) +0.45%
Benchmarking::increment_balance 3,713 (+16) +0.43%
FPC::public_dispatch 9,472 (+38) +0.40%
AvmTest::returndata_copy_oracle 2,011 (+8) +0.40%
Lending::_withdraw 9,273 (+34) +0.37%
Uniswap::_approve_bridge_and_exit_input_asset_to_L1 6,451 (+22) +0.34%
InclusionProofs::public_dispatch 4,182 (+14) +0.34%
AvmTest::debug_logging 1,280 (+4) +0.31%
AuthRegistry::consume 3,222 (+10) +0.31%
StatefulTest::increment_public_value_no_init_check 2,098 (+6) +0.29%
Test::emit_unencrypted 1,702 (+4) +0.24%
Auth::public_dispatch 10,802 (+20) +0.19%
Claim::public_dispatch 4,240 (+6) +0.14%
Token::complete_refund 6,057 (+6) +0.10%
Child::public_dispatch 5,994 (+4) +0.07%
Spam::public_dispatch 3,778 (+2) +0.05%
AppSubscription::public_dispatch 5,030 (+2) +0.04%
PrivateFPC::public_dispatch 4,118 (-2) -0.05%
StatefulTest::public_dispatch 8,128 (-4) -0.05%
AvmTest::add_storage_map 2,316 (-2) -0.09%
TokenBlacklist::public_dispatch 25,334 (-38) -0.15%
PriceFeed::public_dispatch 3,986 (-6) -0.15%
Lending::public_dispatch 28,431 (-52) -0.18%
AuthRegistry::public_dispatch 8,396 (-16) -0.19%
CardGame::on_game_joined 5,046 (-10) -0.20%
Token::public_dispatch 36,991 (-90) -0.24%
NFT::public_dispatch 26,393 (-90) -0.34%
Crowdfunding::public_dispatch 6,184 (-26) -0.42%
Benchmarking::public_dispatch 5,697 (-24) -0.42%
TestLog::emit_unencrypted_events 1,853 (-8) -0.43%
NFT::_finalize_transfer_to_private_unsafe 6,495 (-38) -0.58%
AvmTest::emit_unencrypted_log 2,995 (-18) -0.60%
Token::_finalize_mint_to_private_unsafe 7,165 (-46) -0.64%
Token::_finalize_transfer_to_private_unsafe 7,375 (-50) -0.67%
TokenBlacklist::update_roles 7,677 (-54) -0.70%
TokenBlacklist::constructor 7,916 (-62) -0.78%
NFT::finalize_transfer_to_private 6,448 (-52) -0.80%
CardGame::on_card_played 5,947 (-50) -0.83%
Token::finalize_mint_to_private 7,118 (-60) -0.84%
Auth::set_authorized 6,032 (-52) -0.85%
Token::finalize_transfer_to_private 7,328 (-64) -0.87%
Auth::set_authorized_delay 5,946 (-52) -0.87%
AvmTest::pedersen_hash 2,923 (-26) -0.88%
AvmTest::pedersen_hash_with_index 2,923 (-26) -0.88%
Parent::public_dispatch 8,505 (-84) -0.98%
StaticParent::public_dispatch 7,292 (-90) -1.22%
Uniswap::public_dispatch 25,135 (-318) -1.25%
Test::create_l2_to_l1_message_public 2,030 (-26) -1.26%
CardGame::on_cards_claimed 7,338 (-100) -1.34%
AvmTest::public_dispatch 59,688 (-874) -1.44%
Uniswap::swap_public 15,701 (-236) -1.48%
TokenBridge::exit_to_l1_public 8,214 (-132) -1.58%
TokenBridge::public_dispatch 22,672 (-370) -1.61%
AvmTest::keccak_hash 2,737 (-46) -1.65%
TestLog::public_dispatch 3,258 (-56) -1.69%
AvmTest::bulk_testing 25,497 (-472) -1.82%
CardGame::public_dispatch 16,326 (-350) -2.10%
Test::public_dispatch 19,778 (-438) -2.17%
TokenBridge::claim_public 12,458 (-276) -2.17%
AvmTest::sha256_hash 3,878 (-86) -2.17%
Test::consume_message_from_arbitrary_sender_public 7,955 (-222) -2.71%
CardGame::start_game 6,982 (-198) -2.76%
Test::consume_mint_public_message 10,606 (-310) -2.84%

@sirasistant
Copy link
Collaborator Author

I'm going to investigate the bytecode size regressions

@fcarreiro
Copy link
Contributor

I'm going to investigate the bytecode size regressions

It could just be a wrong comparison, I'd be surprised if this changes anything.

You can execute

RUST_LOG=info ./bootstrap.sh 2>&1 | grep public_dispatch

in noir-contracts and compare

@sirasistant
Copy link
Collaborator Author

I think I know what's happening. This makes bigger any assert with static message due to codegening the constant. For example, the token contract has:
59 array checks
74 overflow checks
2 range checks
38 user asserts

@sirasistant sirasistant changed the title feat: Encode static error strings in the ABI (WIP) feat: Encode static error strings in the ABI Oct 31, 2024
@sirasistant
Copy link
Collaborator Author

It's ready for review now

TomAFrench
TomAFrench approved these changes Nov 1, 2024
View reviewed changes
Copy link
Member

@TomAFrench TomAFrench left a comment

Choose a reason for hiding this comment

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

Some nits but otherwise LGTM

noir-projects/noir-protocol-circuits/bootstrap.sh Outdated Show resolved Hide resolved
noir/noir-repo/acvm-repo/acvm/src/pwg/mod.rs Outdated Show resolved Hide resolved
noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs Show resolved Hide resolved
noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs Show resolved Hide resolved
noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs Show resolved Hide resolved
@sirasistant sirasistant merged commit 1a41d42 into master Nov 4, 2024
68 checks passed
@sirasistant sirasistant deleted the arv/brillig_string_errors_abi branch November 4, 2024 09:51
@AztecBot AztecBot mentioned this pull request Nov 4, 2024
Merged
TomAFrench added a commit that referenced this pull request Nov 4, 2024
@TomAFrench
Merge branch 'master' into sync-noir
856f3c3 
* master: (81 commits)
  feat: Encode static error strings in the ABI (#9552)
  chore: redo typo PR by donatik27 (#9693)
  chore: update install instructions for foundry to display version of rust to install (#9653)
  chore: disable bench upload until #9692
  fix: earthly-ci in bench-e2e (#9689)
  chore: redo typo PR by cypherpepe (#9687)
  chore: redo typo PR by youyyytrok (#9686)
  chore: redo typo PR by mdqst (#9685)
  chore: redo typo PR by mdqst (#9684)
  feat: adding tags to encrypted logs (#9566)
  fix: enable gerousia e2e test (#9677)
  git subrepo push --branch=master noir-projects/aztec-nr
  git_subrepo.sh: Fix parent in .gitrepo file. [skip ci]
  chore: replace relative paths to noir-protocol-circuits
  git subrepo push --branch=master barretenberg
  chore: redo typo PR by dsarfed (#9667)
  fix: bench e2e jobs (#9662)
  fix: Fix random for Mac users  (#9670)
  feat: Graph methods for circuit analysis (part 1) (#7948)
  feat: Faster random sampling (#9655)
  ...
TomAFrench added a commit that referenced this pull request Nov 4, 2024
@TomAFrench
Merge branch 'master' into sync-noir
5254d40 
* master: (83 commits)
  fix: update solhint (#9399)
  chore: redo typo PR by leopardracer (#9705)
  feat: Encode static error strings in the ABI (#9552)
  chore: redo typo PR by donatik27 (#9693)
  chore: update install instructions for foundry to display version of rust to install (#9653)
  chore: disable bench upload until #9692
  fix: earthly-ci in bench-e2e (#9689)
  chore: redo typo PR by cypherpepe (#9687)
  chore: redo typo PR by youyyytrok (#9686)
  chore: redo typo PR by mdqst (#9685)
  chore: redo typo PR by mdqst (#9684)
  feat: adding tags to encrypted logs (#9566)
  fix: enable gerousia e2e test (#9677)
  git subrepo push --branch=master noir-projects/aztec-nr
  git_subrepo.sh: Fix parent in .gitrepo file. [skip ci]
  chore: replace relative paths to noir-protocol-circuits
  git subrepo push --branch=master barretenberg
  chore: redo typo PR by dsarfed (#9667)
  fix: bench e2e jobs (#9662)
  fix: Fix random for Mac users  (#9670)
  ...
@ludamad ludamad mentioned this pull request Nov 4, 2024
Closed
@sirasistant sirasistant mentioned this pull request Nov 5, 2024
Merged
sirasistant added a commit that referenced this pull request Nov 5, 2024
@sirasistant
fix: Include revertdata in Avm simulation errors for failures in nonr…
5f85f5c 
…evertible (#9744)

Fixes #9727 errors after #9552
Maddiaa0 pushed a commit that referenced this pull request Nov 6, 2024
@sirasistant @Maddiaa0
fix: Include revertdata in Avm simulation errors for failures in nonr…
3f52f71 
…evertible (#9744)

Fixes #9727 errors after #9552
@AztecBot AztecBot mentioned this pull request Nov 19, 2024
Merged
This was referenced Nov 19, 2024
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TomAFrench TomAFrench TomAFrench approved these changes

@fcarreiro fcarreiro Awaiting requested review from fcarreiro fcarreiro is a code owner

@dbanks12 dbanks12 Awaiting requested review from dbanks12 dbanks12 is a code owner

@vezenovm vezenovm Awaiting requested review from vezenovm vezenovm is a code owner

@guipublic guipublic Awaiting requested review from guipublic guipublic is a code owner

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

Successfully merging this pull request may close these issues.
3 participants
@sirasistant @fcarreiro @TomAFrench