-
Notifications
You must be signed in to change notification settings - Fork 611
feat(avm): protocol contractg mutations #19586
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,7 @@ | |
| #include "barretenberg/avm_fuzzer/mutations/basic_types/uint64_t.hpp" | ||
| #include "barretenberg/avm_fuzzer/mutations/configuration.hpp" | ||
| #include "barretenberg/avm_fuzzer/mutations/fuzzer_data.hpp" | ||
| #include "barretenberg/avm_fuzzer/mutations/protocol_contracts.hpp" | ||
| #include "barretenberg/avm_fuzzer/mutations/tx_data.hpp" | ||
| #include "barretenberg/avm_fuzzer/mutations/tx_types/gas.hpp" | ||
| #include "barretenberg/common/log.hpp" | ||
|
|
@@ -49,6 +50,22 @@ void setup_fuzzer_state(FuzzerWorldStateManager& ws_mgr, FuzzerContractDB& contr | |
| contract_db.add_contract_instance(contract_address, contract_instance); | ||
| } | ||
|
|
||
| // For protocol contracts, also add instances keyed by canonical address (1-11). | ||
| // This is needed because protocol contracts are looked up by canonical address, | ||
| // but the derived address in protocol_contracts.derived_addresses maps to the actual instance. | ||
| for (size_t i = 0; i < tx_data.protocol_contracts.derived_addresses.size(); ++i) { | ||
| const auto& derived_address = tx_data.protocol_contracts.derived_addresses[i]; | ||
| if (!derived_address.is_zero()) { | ||
| // Canonical address is index + 1 (addresses 1-11 map to indices 0-10) | ||
| AztecAddress canonical_address(static_cast<uint256_t>(i + 1)); | ||
| // Find the instance for this derived address and also add it by canonical address | ||
| auto maybe_instance = contract_db.get_contract_instance(derived_address); | ||
| if (maybe_instance.has_value()) { | ||
| contract_db.add_contract_instance(canonical_address, maybe_instance.value()); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Register contract addresses in the world state | ||
| for (const auto& addr : tx_data.contract_addresses) { | ||
| ws_mgr.register_contract_address(addr); | ||
|
|
@@ -85,8 +102,13 @@ SimulatorResult fuzz_tx(FuzzerWorldStateManager& ws_mgr, FuzzerContractDB& contr | |
|
|
||
| try { | ||
| ws_mgr.checkpoint(); | ||
| cpp_result = cpp_simulator.simulate( | ||
| ws_mgr, contract_db, tx_data.tx, tx_data.global_variables, tx_data.public_data_writes, tx_data.note_hashes); | ||
| cpp_result = cpp_simulator.simulate(ws_mgr, | ||
| contract_db, | ||
| tx_data.tx, | ||
| tx_data.global_variables, | ||
| tx_data.public_data_writes, | ||
| tx_data.note_hashes, | ||
| tx_data.protocol_contracts); | ||
| fuzz_info("CppSimulator completed without exception"); | ||
| fuzz_info("CppSimulator result: ", cpp_result); | ||
| ws_mgr.revert(); | ||
|
|
@@ -102,8 +124,13 @@ SimulatorResult fuzz_tx(FuzzerWorldStateManager& ws_mgr, FuzzerContractDB& contr | |
| } | ||
|
|
||
| ws_mgr.checkpoint(); | ||
| auto js_result = js_simulator->simulate( | ||
| ws_mgr, contract_db, tx_data.tx, tx_data.global_variables, tx_data.public_data_writes, tx_data.note_hashes); | ||
| auto js_result = js_simulator->simulate(ws_mgr, | ||
| contract_db, | ||
| tx_data.tx, | ||
| tx_data.global_variables, | ||
| tx_data.public_data_writes, | ||
| tx_data.note_hashes, | ||
| tx_data.protocol_contracts); | ||
|
|
||
| // If the results do not match | ||
| if (!compare_simulator_results(cpp_result, js_result)) { | ||
|
|
@@ -126,7 +153,7 @@ SimulatorResult fuzz_tx(FuzzerWorldStateManager& ws_mgr, FuzzerContractDB& contr | |
| /// @throws An exception if simulation results differ or check_circuit fails | ||
| int fuzz_prover(FuzzerWorldStateManager& ws_mgr, FuzzerContractDB& contract_db, FuzzerTxData& tx_data) | ||
| { | ||
| ProtocolContracts protocol_contracts{}; | ||
| ProtocolContracts& protocol_contracts = tx_data.protocol_contracts; | ||
| WorldState& ws = ws_mgr.get_world_state(); | ||
| WorldStateRevision ws_rev = ws_mgr.get_current_revision(); | ||
| AvmSimulationHelper helper; | ||
|
|
@@ -369,14 +396,21 @@ size_t mutate_tx_data(FuzzerContext& context, | |
| // This is just mutating the gas values and timestamp | ||
| mutate_uint64_t(tx_data.global_variables.timestamp, rng, BASIC_UINT64_T_MUTATION_CONFIGURATION); | ||
| mutate_gas_fees(tx_data.global_variables.gas_fees, rng); | ||
| // This must be less than or equal to the tx max fees per gas | ||
| tx_data.global_variables.gas_fees.fee_per_da_gas = std::min( | ||
| tx_data.global_variables.gas_fees.fee_per_da_gas, tx_data.tx.gas_settings.max_fees_per_gas.fee_per_da_gas); | ||
| tx_data.global_variables.gas_fees.fee_per_l2_gas = std::min( | ||
| tx_data.global_variables.gas_fees.fee_per_l2_gas, tx_data.tx.gas_settings.max_fees_per_gas.fee_per_l2_gas); | ||
| break; | ||
| // case TxDataMutationType::ProtocolContractsMutation: | ||
| // break; | ||
| case FuzzerTxDataMutationType::ProtocolContractsMutation: | ||
| mutate_protocol_contracts(tx_data.protocol_contracts, tx_data.tx, tx_data.contract_addresses, rng); | ||
| break; | ||
| } | ||
|
|
||
| // Clear any protocol contract derived addresses that reference addresses no longer in the contract set. | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is pretty lazy, but i was running into some issues with calling stale contract addresses. I'll investigate a cleaner way to ensure we dont need a clean up step after mutations. |
||
| // This can happen when mutations (e.g., ContractClassMutation, ContractInstanceMutation) change contract addresses. | ||
| // Must run AFTER all mutations since some mutations modify contract_addresses. | ||
| std::unordered_set<AztecAddress> valid_addresses(tx_data.contract_addresses.begin(), | ||
| tx_data.contract_addresses.end()); | ||
| for (auto& derived_address : tx_data.protocol_contracts.derived_addresses) { | ||
| if (!derived_address.is_zero() && !valid_addresses.contains(derived_address)) { | ||
| derived_address = AztecAddress(0); | ||
| } | ||
| } | ||
|
|
||
| // todo: do we need to ensure this or are should we able to process 0 enqueued calls? | ||
|
|
@@ -393,6 +427,13 @@ size_t mutate_tx_data(FuzzerContext& context, | |
| .calldata = calldata }); | ||
| } | ||
|
|
||
| // Ensure global gas_fees <= max_fees_per_gas (required for compute_effective_gas_fees) | ||
| // This must run after ANY mutation since TxMutation can reduce max_fees_per_gas | ||
| tx_data.global_variables.gas_fees.fee_per_da_gas = std::min( | ||
| tx_data.global_variables.gas_fees.fee_per_da_gas, tx_data.tx.gas_settings.max_fees_per_gas.fee_per_da_gas); | ||
| tx_data.global_variables.gas_fees.fee_per_l2_gas = std::min( | ||
| tx_data.global_variables.gas_fees.fee_per_l2_gas, tx_data.tx.gas_settings.max_fees_per_gas.fee_per_l2_gas); | ||
|
|
||
| // Compute effective gas fees matching TS computeEffectiveGasFees | ||
| // This must be done after any mutation that could affect gas settings or global variables | ||
| tx_data.tx.effective_gas_fees = | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| #include "barretenberg/avm_fuzzer/mutations/protocol_contracts.hpp" | ||
|
|
||
| #include "barretenberg/vm2/common/aztec_constants.hpp" | ||
| #include "barretenberg/vm2/common/aztec_types.hpp" | ||
|
|
||
| namespace bb::avm2::fuzzer { | ||
|
|
||
| namespace { | ||
|
|
||
| void update_enqueued_calls_for_protocol_contract(Tx& tx, | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is a bit of a code-smell, but we need to update the addresses used in the enqueued calls whenever we modify the protocol contracts and we would have potentially invalidated addresses that are referenced by enqueued calls |
||
| const AztecAddress& prev_address, | ||
| const AztecAddress& new_address) | ||
| { | ||
| // Update setup_enqueued_calls | ||
| for (auto& call : tx.setup_enqueued_calls) { | ||
| if (call.request.contract_address == prev_address) { | ||
| call.request.contract_address = new_address; | ||
| } | ||
| } | ||
| // Update app_logic_enqueued_calls | ||
| for (auto& call : tx.app_logic_enqueued_calls) { | ||
| if (call.request.contract_address == prev_address) { | ||
| call.request.contract_address = new_address; | ||
| } | ||
| } | ||
| // Update teardown_enqueued_call | ||
| if (tx.teardown_enqueued_call.has_value() && tx.teardown_enqueued_call->request.contract_address == prev_address) { | ||
| tx.teardown_enqueued_call->request.contract_address = new_address; | ||
| } | ||
| } | ||
|
|
||
| } // namespace | ||
|
|
||
| void mutate_protocol_contracts(ProtocolContracts& protocol_contracts, | ||
| Tx& tx, | ||
| const std::vector<AztecAddress>& contract_addresses, | ||
| std::mt19937_64& rng) | ||
| { | ||
| if (contract_addresses.empty()) { | ||
| return; | ||
| } | ||
|
|
||
| auto choice = PROTOCOL_CONTRACTS_MUTATION_CONFIGURATION.select(rng); | ||
| switch (choice) { | ||
| case ProtocolContractsMutationOptions::Mutate: { | ||
| // Pick a random index and add a protocol contract | ||
| auto protocol_contract_index = std::uniform_int_distribution<size_t>(0, MAX_PROTOCOL_CONTRACTS - 1)(rng); | ||
| auto contract_address_index = std::uniform_int_distribution<size_t>(0, contract_addresses.size() - 1)(rng); | ||
|
|
||
| AztecAddress derived_address = contract_addresses[contract_address_index]; | ||
| protocol_contracts.derived_addresses[protocol_contract_index] = derived_address; | ||
|
|
||
| // The index of the protocol contracts array maps to canonical address. | ||
| // Add 1 because canonical addresses are 1-indexed | ||
| AztecAddress canonical_address(static_cast<uint256_t>(protocol_contract_index + 1)); | ||
|
|
||
| // todo(ilyas): there should be a more efficient way to do this | ||
| // Update any enqueued calls that reference the derived address to now use the canonical address | ||
| update_enqueued_calls_for_protocol_contract( | ||
| tx, /*prev_address=*/derived_address, /*new_address=*/canonical_address); | ||
| break; | ||
| } | ||
| case ProtocolContractsMutationOptions::Remove: { | ||
| // Pick an index, and zero it out, removing the protocol contract. It may already be zero but the fuzzer should | ||
| // figure out better mutations | ||
| size_t idx = std::uniform_int_distribution<size_t>(0, MAX_PROTOCOL_CONTRACTS - 1)(rng); | ||
| AztecAddress canonical_address(static_cast<uint256_t>(idx + 1)); | ||
| AztecAddress derived_address = protocol_contracts.derived_addresses[idx]; | ||
| // Update any enqueued calls that reference the canonical address to use the derived address | ||
| update_enqueued_calls_for_protocol_contract( | ||
| tx, /*prev_address=*/canonical_address, /*new_address=*/derived_address); | ||
| // Set the derived address to zero | ||
| protocol_contracts.derived_addresses[idx] = AztecAddress(0); | ||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| } // namespace bb::avm2::fuzzer | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| #pragma once | ||
|
|
||
| #include "barretenberg/avm_fuzzer/common/weighted_selection.hpp" | ||
| #include "barretenberg/vm2/common/avm_io.hpp" | ||
| #include "barretenberg/vm2/common/aztec_types.hpp" | ||
|
|
||
| #include <random> | ||
|
|
||
| namespace bb::avm2::fuzzer { | ||
|
|
||
| enum class ProtocolContractsMutationOptions : uint8_t { | ||
| Mutate, | ||
| Remove, | ||
| }; | ||
|
|
||
| using ProtocolContractsMutationConfig = WeightedSelectionConfig<ProtocolContractsMutationOptions, 2>; | ||
|
|
||
| constexpr ProtocolContractsMutationConfig PROTOCOL_CONTRACTS_MUTATION_CONFIGURATION = ProtocolContractsMutationConfig({ | ||
| { ProtocolContractsMutationOptions::Mutate, 3 }, | ||
| { ProtocolContractsMutationOptions::Remove, 1 }, | ||
| }); | ||
|
|
||
| void mutate_protocol_contracts(bb::avm2::ProtocolContracts& protocol_contracts, | ||
| bb::avm2::Tx& tx, | ||
| const std::vector<AztecAddress>& contract_addresses, | ||
| std::mt19937_64& rng); | ||
|
|
||
| } // namespace bb::avm2::fuzzer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is so that when we look up the protocol contracts we can retrieve the instance