From b786d92e19a21dde849eb0766dd7b54f3d7bd8fb Mon Sep 17 00:00:00 2001 From: Thomas Letan Date: Fri, 29 Mar 2024 21:37:53 +0100 Subject: [PATCH] EVM Kernel: Fix reimbursement to the sequencer pool address The failsafe storage mechanism implemented recently is quite subtle (and error-prone). During the production of a block, `/evm/world_state` is copied to `/tmp/evm/world_state` and *every access to the durable storage is prefixed by `/tmp`*. This is why the configuration is passed over in the various block validation function. This logic was not used for the sequencer pool address, which is read in place at `/evm/sequencer_pool_address` (that is, `/tmp/evm/sequencer_pool_address`). We fix this bug by reading the sequencer pool address once, before starting the block production, and passing the argument over to use site. --- etherlink/kernel_evm/kernel/src/apply.rs | 6 +- etherlink/kernel_evm/kernel/src/block.rs | 33 ++++++++- etherlink/kernel_evm/kernel/src/fees.rs | 19 ++++-- etherlink/kernel_evm/kernel/src/lib.rs | 14 +++- etherlink/tezt/lib/configuration.ml | 20 ++++-- etherlink/tezt/lib/configuration.mli | 1 + etherlink/tezt/lib/durable_storage_path.ml | 2 + etherlink/tezt/lib/durable_storage_path.mli | 3 + etherlink/tezt/tests/evm_sequencer.ml | 74 ++++++++++++++++++++- 9 files changed, 154 insertions(+), 18 deletions(-) diff --git a/etherlink/kernel_evm/kernel/src/apply.rs b/etherlink/kernel_evm/kernel/src/apply.rs index 8fb73e7a96b9..8db0d290c83d 100644 --- a/etherlink/kernel_evm/kernel/src/apply.rs +++ b/etherlink/kernel_evm/kernel/src/apply.rs @@ -549,6 +549,7 @@ pub fn handle_transaction_result( transaction_result: TransactionResult, pay_fees: bool, ticketer: &Option, + sequencer_pool_address: Option, ) -> Result { let TransactionResult { caller, @@ -572,7 +573,7 @@ pub fn handle_transaction_result( } if pay_fees { - fee_updates.apply(host, evm_account_storage, caller)?; + fee_updates.apply(host, evm_account_storage, caller, sequencer_pool_address)?; } let object_info = make_object_info(transaction, caller, index, &fee_updates)?; @@ -608,6 +609,7 @@ pub fn apply_transaction( allocated_ticks: u64, retriable: bool, ticketer: &Option, + sequencer_pool_address: Option, ) -> Result, anyhow::Error> { let apply_result = match &transaction.content { TransactionContent::Ethereum(tx) => apply_ethereum_transaction_common( @@ -649,6 +651,7 @@ pub fn apply_transaction( tx_result, true, ticketer, + sequencer_pool_address, )?; Ok(ExecutionResult::Valid(execution_result)) } @@ -666,6 +669,7 @@ pub fn apply_transaction( tx_result, false, ticketer, + sequencer_pool_address, )?; Ok(ExecutionResult::Retriable(execution_result)) } diff --git a/etherlink/kernel_evm/kernel/src/block.rs b/etherlink/kernel_evm/kernel/src/block.rs index 58ae77d18cd5..587d5e7264ea 100644 --- a/etherlink/kernel_evm/kernel/src/block.rs +++ b/etherlink/kernel_evm/kernel/src/block.rs @@ -27,7 +27,7 @@ use block_in_progress::BlockInProgress; use evm_execution::account_storage::{init_account_storage, EthereumAccountStorage}; use evm_execution::precompiles; use evm_execution::precompiles::PrecompileBTreeMap; -use primitive_types::{H256, U256}; +use primitive_types::{H160, H256, U256}; use tezos_crypto_rs::hash::ContractKt1Hash; use tezos_ethereum::block::BlockFees; use tezos_evm_logging::{log, Level::*}; @@ -71,6 +71,7 @@ fn compute( accounts_index: &mut IndexableStorage, is_first_block_of_reboot: bool, ticketer: &Option, + sequencer_pool_address: Option, ) -> Result { log!( host, @@ -105,6 +106,7 @@ fn compute( allocated_ticks, retriable, ticketer, + sequencer_pool_address, )? { ExecutionResult::Valid(ExecutionInfo { receipt_info, @@ -233,6 +235,7 @@ fn compute_bip( tick_counter: &mut TickCounter, first_block_of_reboot: &mut bool, ticketer: &Option, + sequencer_pool_address: Option, ) -> anyhow::Result { let result = compute( host, @@ -244,6 +247,7 @@ fn compute_bip( accounts_index, *first_block_of_reboot, ticketer, + sequencer_pool_address, )?; match result { ComputationResult::RebootNeeded => { @@ -328,6 +332,7 @@ pub fn produce( chain_id: U256, block_fees: BlockFees, config: &mut Configuration, + sequencer_pool_address: Option, ) -> Result { let kernel_upgrade = upgrade::read_kernel_upgrade(host)?; @@ -383,6 +388,7 @@ pub fn produce( &mut tick_counter, &mut first_block_of_reboot, &config.tezos_contracts.ticketer, + sequencer_pool_address, ) { Ok(ComputationResult::Finished) => promote_block( &mut safe_host, @@ -406,6 +412,8 @@ pub fn produce( } } + // Using `safe_host.host` allows to escape from the failsafe storage, which is necessary + // because the sequencer pool address is located outside of `/evm/world_state`. upgrade::possible_sequencer_upgrade(safe_host.host)?; // Execute stored blueprints @@ -461,6 +469,7 @@ pub fn produce( &mut tick_counter, &mut first_block_of_reboot, &config.tezos_contracts.ticketer, + sequencer_pool_address, ) { Ok(ComputationResult::Finished) => { promote_block(&mut safe_host, &outbox_queue, false, processed_blueprint)? @@ -718,6 +727,7 @@ mod tests { DUMMY_CHAIN_ID, dummy_block_fees(), &mut Configuration::default(), + None, ) .expect("The block production failed."); } @@ -764,6 +774,7 @@ mod tests { DUMMY_CHAIN_ID, dummy_block_fees(), &mut Configuration::default(), + None, ) .expect("The block production failed."); @@ -807,6 +818,7 @@ mod tests { DUMMY_CHAIN_ID, dummy_block_fees(), &mut Configuration::default(), + None, ) .expect("The block production failed."); @@ -853,6 +865,7 @@ mod tests { DUMMY_CHAIN_ID, dummy_block_fees(), &mut Configuration::default(), + None, ) .expect("The block production failed."); let receipt = read_transaction_receipt(&mut host, &tx_hash) @@ -932,6 +945,7 @@ mod tests { DUMMY_CHAIN_ID, dummy_block_fees(), &mut Configuration::default(), + None, ) .expect("The block production failed."); @@ -989,6 +1003,7 @@ mod tests { DUMMY_CHAIN_ID, dummy_block_fees, &mut Configuration::default(), + None, ) .expect("The block production failed."); let receipt0 = read_transaction_receipt(&mut host, &tx_hash_0) @@ -1052,6 +1067,7 @@ mod tests { DUMMY_CHAIN_ID, dummy_block_fees(), &mut Configuration::default(), + None, ) .expect("The block production failed."); @@ -1111,6 +1127,7 @@ mod tests { DUMMY_CHAIN_ID, dummy_block_fees(), &mut Configuration::default(), + None, ) .expect("The block production failed."); @@ -1145,6 +1162,7 @@ mod tests { DUMMY_CHAIN_ID, dummy_block_fees(), &mut Configuration::default(), + None, ) .expect("The block production failed."); @@ -1156,6 +1174,7 @@ mod tests { DUMMY_CHAIN_ID, dummy_block_fees(), &mut Configuration::default(), + None, ) .expect("The block production failed."); @@ -1207,6 +1226,7 @@ mod tests { DUMMY_CHAIN_ID, dummy_block_fees(), &mut Configuration::default(), + None, ) .expect("The block production failed."); @@ -1297,6 +1317,7 @@ mod tests { &mut accounts_index, true, &None, + None, ) .expect("Should have computed block"); @@ -1365,6 +1386,7 @@ mod tests { DUMMY_CHAIN_ID, dummy_block_fees(), &mut Configuration::default(), + None, ) .expect("The block production failed."); assert!( @@ -1410,6 +1432,7 @@ mod tests { DUMMY_CHAIN_ID, dummy_block_fees(), &mut Configuration::default(), + None, ) .expect("Empty block should have been produced"); check_current_block_number(&mut host, 0); @@ -1422,6 +1445,7 @@ mod tests { DUMMY_CHAIN_ID, dummy_block_fees(), &mut Configuration::default(), + None, ) .expect("Empty block should have been produced"); check_current_block_number(&mut host, 1); @@ -1434,6 +1458,7 @@ mod tests { DUMMY_CHAIN_ID, dummy_block_fees(), &mut Configuration::default(), + None, ) .expect("Empty block should have been produced"); check_current_block_number(&mut host, 2); @@ -1564,6 +1589,7 @@ mod tests { DUMMY_CHAIN_ID, dummy_block_fees(), &mut Configuration::default(), + None, ) .expect("Should have produced"); @@ -1637,6 +1663,7 @@ mod tests { DUMMY_CHAIN_ID, dummy_block_fees(), &mut Configuration::default(), + None, ) .expect("Should have produced"); @@ -1730,6 +1757,7 @@ mod tests { DUMMY_CHAIN_ID, block_fees, &mut Configuration::default(), + None, ) .expect("The block production failed."); @@ -1795,6 +1823,7 @@ mod tests { DUMMY_CHAIN_ID, dummy_block_fees(), &mut Configuration::default(), + None, ) .expect("Should have produced"); @@ -1814,6 +1843,7 @@ mod tests { DUMMY_CHAIN_ID, dummy_block_fees(), &mut Configuration::default(), + None, ) .expect("Should have produced"); @@ -1899,6 +1929,7 @@ mod tests { DUMMY_CHAIN_ID, dummy_block_fees(), &mut Configuration::default(), + None, ) .expect("The block production failed."); diff --git a/etherlink/kernel_evm/kernel/src/fees.rs b/etherlink/kernel_evm/kernel/src/fees.rs index 53b8d3acb08e..6564d3bafc26 100644 --- a/etherlink/kernel_evm/kernel/src/fees.rs +++ b/etherlink/kernel_evm/kernel/src/fees.rs @@ -16,7 +16,6 @@ //! Additionally, we charge a _data-availability_ fee, for each tx posted through L1. use crate::inbox::TransactionContent; -use crate::storage::read_sequencer_pool_address; use evm_execution::account_storage::{account_path, EthereumAccountStorage}; use evm_execution::handler::ExecutionOutcome; @@ -150,6 +149,7 @@ impl FeeUpdates { host: &mut impl Runtime, accounts: &mut EthereumAccountStorage, caller: H160, + sequencer_pool_address: Option, ) -> Result<(), anyhow::Error> { tezos_evm_logging::log!( host, @@ -166,7 +166,7 @@ impl FeeUpdates { )); } - let sequencer = match read_sequencer_pool_address(host) { + let sequencer = match sequencer_pool_address { None => { let burned_fee = self .burn_amount @@ -287,7 +287,6 @@ fn gas_as_u64(gas_for_fees: U256) -> Result { #[cfg(test)] mod tests { use super::*; - use crate::storage::store_sequencer_pool_address; use evm::{ExitReason, ExitSucceed}; use evm_execution::{ account_storage::{account_path, EthereumAccountStorage}, @@ -365,7 +364,8 @@ mod tests { }; // Act - let result = fee_updates.apply(&mut host, &mut evm_account_storage, address); + let result = + fee_updates.apply(&mut host, &mut evm_account_storage, address, None); // Assert assert!(result.is_ok()); @@ -384,7 +384,6 @@ mod tests { let mut host = MockHost::default(); let sequencer_address = address_from_str("0123456789ABCDEF0123456789ABCDEF01234567"); - store_sequencer_pool_address(&mut host, sequencer_address).unwrap(); let mut evm_account_storage = evm_execution::account_storage::init_account_storage().unwrap(); @@ -413,7 +412,12 @@ mod tests { }; // Act - let result = fee_updates.apply(&mut host, &mut evm_account_storage, address); + let result = fee_updates.apply( + &mut host, + &mut evm_account_storage, + address, + Some(sequencer_address), + ); // Assert assert!(result.is_ok()); @@ -451,7 +455,8 @@ mod tests { }; // Act - let result = fee_updates.apply(&mut host, &mut evm_account_storage, address); + let result = + fee_updates.apply(&mut host, &mut evm_account_storage, address, None); // Assert assert!(result.is_err()); diff --git a/etherlink/kernel_evm/kernel/src/lib.rs b/etherlink/kernel_evm/kernel/src/lib.rs index a51aed4397b9..fb0c84dfaa8f 100644 --- a/etherlink/kernel_evm/kernel/src/lib.rs +++ b/etherlink/kernel_evm/kernel/src/lib.rs @@ -10,6 +10,7 @@ use crate::error::Error; use crate::error::UpgradeProcessError::Fallback; use crate::migration::storage_migration; use crate::stage_one::fetch; +use crate::storage::read_sequencer_pool_address; use anyhow::Context; use delayed_inbox::DelayedInbox; use evm_execution::Config; @@ -223,6 +224,7 @@ pub fn main(host: &mut Host) -> Result<(), anyhow::Error> { // 2. Fetch the per mode configuration of the kernel. Returns the default // configuration if it fails. let mut configuration = fetch_configuration(host); + let sequencer_pool_address = read_sequencer_pool_address(host); // Run the stage one, this is a no-op if the inbox was already consumed // by another kernel run. This ensures that if the migration does not @@ -239,9 +241,14 @@ pub fn main(host: &mut Host) -> Result<(), anyhow::Error> { let block_fees = retrieve_block_fees(host)?; // Start processing blueprints log!(host, Debug, "Entering stage two."); - if let block::ComputationResult::RebootNeeded = - block::produce(host, chain_id, block_fees, &mut configuration) - .context("Failed during stage 2")? + if let block::ComputationResult::RebootNeeded = block::produce( + host, + chain_id, + block_fees, + &mut configuration, + sequencer_pool_address, + ) + .context("Failed during stage 2")? { host.mark_for_reboot()?; }; @@ -493,6 +500,7 @@ mod tests { DUMMY_CHAIN_ID, block_fees, &mut Configuration::default(), + None, ) .expect("Should have produced"); diff --git a/etherlink/tezt/lib/configuration.ml b/etherlink/tezt/lib/configuration.ml index 7f6c3b36e846..b3a0c47e7d8c 100644 --- a/etherlink/tezt/lib/configuration.ml +++ b/etherlink/tezt/lib/configuration.ml @@ -10,9 +10,9 @@ let default_bootstrap_account_balance = Wei.of_eth_int 9999 let make_config ?kernel_root_hash ?bootstrap_accounts ?ticketer ?administrator ?kernel_governance ?kernel_security_governance ?sequencer_governance - ?sequencer ?delayed_bridge ?(da_fee_per_byte = Wei.zero) - ?minimum_base_fee_per_gas ?delayed_inbox_timeout ?delayed_inbox_min_levels - () = + ?sequencer ?sequencer_pool_address ?delayed_bridge + ?(da_fee_per_byte = Wei.zero) ?minimum_base_fee_per_gas + ?delayed_inbox_timeout ?delayed_inbox_min_levels () = let open Sc_rollup_helpers.Installer_kernel_config in let kernel_root_hash = Option.fold @@ -130,11 +130,23 @@ let make_config ?kernel_root_hash ?bootstrap_accounts ?ticketer ?administrator ~none:[] minimum_base_fee_per_gas in + let sequencer_pool_address = + match sequencer_pool_address with + | Some addr -> + let addr = + if String.starts_with addr ~prefix:"0x" then + let n = String.length addr in + String.sub addr 2 (n - 2) + else failwith "sequencer pool address should start with 0x" + in + [Set {value = addr; to_ = Durable_storage_path.sequencer_pool_address}] + | None -> [] + in match kernel_root_hash @ ticketer @ bootstrap_accounts @ administrator @ kernel_governance @ kernel_security_governance @ sequencer_governance @ sequencer @ delayed_bridge @ da_fee_per_byte @ minimum_base_fee_per_gas - @ delayed_inbox_timeout @ delayed_inbox_min_levels + @ delayed_inbox_timeout @ delayed_inbox_min_levels @ sequencer_pool_address with | [] -> None | res -> Some (`Config res) diff --git a/etherlink/tezt/lib/configuration.mli b/etherlink/tezt/lib/configuration.mli index 5f31db609ce3..87bbd8145465 100644 --- a/etherlink/tezt/lib/configuration.mli +++ b/etherlink/tezt/lib/configuration.mli @@ -19,6 +19,7 @@ val make_config : ?kernel_security_governance:string -> ?sequencer_governance:string -> ?sequencer:string -> + ?sequencer_pool_address:string -> ?delayed_bridge:string -> ?da_fee_per_byte:Wei.t -> ?minimum_base_fee_per_gas:Wei.t -> diff --git a/etherlink/tezt/lib/durable_storage_path.ml b/etherlink/tezt/lib/durable_storage_path.ml index 7cff81911bd8..14b010b80733 100644 --- a/etherlink/tezt/lib/durable_storage_path.ml +++ b/etherlink/tezt/lib/durable_storage_path.ml @@ -69,6 +69,8 @@ let ticketer = evm "/ticketer" let sequencer = evm "/sequencer" +let sequencer_pool_address = evm "/sequencer_pool_address" + let kernel_boot_wasm = kernel "/boot.wasm" let delayed_bridge_path = evm "/delayed_bridge" diff --git a/etherlink/tezt/lib/durable_storage_path.mli b/etherlink/tezt/lib/durable_storage_path.mli index 7fb870259ebf..30e53116ae4a 100644 --- a/etherlink/tezt/lib/durable_storage_path.mli +++ b/etherlink/tezt/lib/durable_storage_path.mli @@ -75,6 +75,9 @@ val ticketer : path (** [sequencer] is the path to the sequencer flag. *) val sequencer : path +(** [sequencer_pool_address] is the path to the L2 address credited with DA fees. *) +val sequencer_pool_address : path + (** [kernel_boot_wasm] is the path to the kernel `boot.wasm`. *) val kernel_boot_wasm : path diff --git a/etherlink/tezt/tests/evm_sequencer.ml b/etherlink/tezt/tests/evm_sequencer.ml index 34e5ed492c98..a9d4f7f99ccc 100644 --- a/etherlink/tezt/tests/evm_sequencer.ml +++ b/etherlink/tezt/tests/evm_sequencer.ml @@ -134,8 +134,9 @@ let setup_sequencer ?(devmode = true) ?config ?genesis_timestamp ?max_blueprints_catchup ?catchup_cooldown ?delayed_inbox_timeout ?delayed_inbox_min_levels ?max_number_of_chunks ?(bootstrap_accounts = Eth_account.bootstrap_accounts) - ?(sequencer = Constant.bootstrap1) ?(kernel = Constant.WASM.evm_kernel) - ?da_fee ?minimum_base_fee_per_gas ?preimages_dir protocol = + ?(sequencer = Constant.bootstrap1) ?sequencer_pool_address + ?(kernel = Constant.WASM.evm_kernel) ?da_fee ?minimum_base_fee_per_gas + ?preimages_dir protocol = let* node, client = setup_l1 ?timestamp:genesis_timestamp protocol in let* l1_contracts = setup_l1_contracts client in let sc_rollup_node = @@ -162,6 +163,7 @@ let setup_sequencer ?(devmode = true) ?config ?genesis_timestamp ?da_fee_per_byte:da_fee ?delayed_inbox_timeout ?delayed_inbox_min_levels + ?sequencer_pool_address () in let config = @@ -1210,6 +1212,73 @@ let test_observer_forwards_transaction = "Missing receipt in the sequencer node for transaction successfully \ injected in the observer" +let test_sequencer_is_reimbursed = + Protocol.register_test + ~__FILE__ + ~tags:["evm"; "sequencer"; "transaction"] + ~title:"Sequencer is reimbursed for DA fees" + ~uses + @@ fun protocol -> + (* Start the evm node *) + let tbb = 1. in + (* We use an arbitrary address for the pool address, the goal is just to + verify its balance increases. *) + let sequencer_pool_address = "0xb7a97043983f24991398e5a82f63f4c58a417185" in + let* {sequencer = sequencer_node; _} = + setup_sequencer + ~da_fee:Wei.one + ~time_between_blocks:(Time_between_blocks tbb) + ~sequencer_pool_address + protocol + in + + let* balance = + Eth_cli.balance + ~account:sequencer_pool_address + ~endpoint:Evm_node.(endpoint sequencer_node) + in + + Check.((Wei.zero = balance) Wei.typ) + ~error_msg:"Balance of the sequencer address pool should be null" ; + + (* Ensure the sequencer has produced the block. *) + let* () = + Evm_node.wait_for_blueprint_applied ~timeout:10.0 sequencer_node 1 + in + + let* txn = + Eth_cli.transaction_send + ~source_private_key:Eth_account.bootstrap_accounts.(1).private_key + ~to_public_key:Eth_account.bootstrap_accounts.(2).address + ~value:Wei.one + ~endpoint:(Evm_node.endpoint sequencer_node) + () + in + + let* receipt = + Eth_cli.get_receipt ~endpoint:(Evm_node.endpoint sequencer_node) ~tx:txn + in + + match receipt with + | Some receipt when receipt.status -> + let* balance = + Eth_cli.balance + ~account:sequencer_pool_address + ~endpoint:Evm_node.(endpoint sequencer_node) + in + + Check.((Wei.zero < balance) Wei.typ) + ~error_msg:"Balance of the sequencer address pool should not be null" ; + unit + | Some _ -> + Test.fail + "transaction receipt received from the sequencer, but transaction \ + failed" + | None -> + Test.fail + "Missing receipt in the sequencer node for transaction successfully \ + injected in the observer" + (** This tests the situation where the kernel has an upgrade and the sequencer upgrade by following the event of the kernel. *) let test_self_upgrade_kernel = @@ -2580,6 +2649,7 @@ let () = test_init_from_rollup_node_with_delayed_inbox protocols ; test_observer_applies_blueprint protocols ; test_observer_forwards_transaction protocols ; + test_sequencer_is_reimbursed protocols ; test_upgrade_kernel_auto_sync protocols ; test_self_upgrade_kernel protocols ; test_force_kernel_upgrade protocols ; -- GitLab