diff --git a/etherlink/kernel_evm/kernel/src/block.rs b/etherlink/kernel_evm/kernel/src/block.rs index 090f5f9afb6e73d3e5ce10d62a2d2ffe02598b1d..0edd2a6491e9331b3349c5dc29dbd949f7657ae0 100644 --- a/etherlink/kernel_evm/kernel/src/block.rs +++ b/etherlink/kernel_evm/kernel/src/block.rs @@ -67,13 +67,6 @@ fn compute( if block_in_progress.would_overflow() { // TODO: https://gitlab.com/tezos/tezos/-/issues/6094 // there should be an upper bound on gasLimit - - if host.reboot_left()? <= 1 { - // TODO: #5873 - // this case needs to be handle properly - log!(host, Info, "Warning: maximum number of reboots reached, some transactions were lost"); - return Ok(ComputationResult::Finished); - } return Ok(ComputationResult::RebootNeeded); } let transaction = block_in_progress.pop_tx().ok_or(Error::Reboot)?; @@ -180,7 +173,6 @@ fn compute_bip( &block_in_progress.estimated_ticks ); storage::store_block_in_progress(host, &block_in_progress)?; - storage::add_reboot_flag(host)?; host.mark_for_reboot()? } ComputationResult::Finished => { @@ -305,7 +297,9 @@ mod tests { }; use tezos_ethereum::tx_common::EthereumTransactionCommon; use tezos_ethereum::tx_signature::TxSignature; + use tezos_smart_rollup_core::SmartRollupCore; use tezos_smart_rollup_encoding::timestamp::Timestamp; + use tezos_smart_rollup_host::path::RefPath; use tezos_smart_rollup_mock::MockHost; fn blueprint(transactions: Vec) -> Blueprint { @@ -1195,6 +1189,18 @@ mod tests { const TOO_MANY_TRANSACTIONS: u64 = 500; + fn is_marked_for_reboot(host: &impl SmartRollupCore) -> bool { + const REBOOT_PATH: RefPath = RefPath::assert_from(b"/kernel/env/reboot"); + host.store_read_all(&REBOOT_PATH).is_ok() + } + + fn assert_marked_for_reboot(host: &impl SmartRollupCore) { + assert!( + is_marked_for_reboot(host), + "The kernel should have been marked for reboot" + ); + } + #[test] fn test_reboot_many_tx_one_proposal() { // init host @@ -1241,10 +1247,7 @@ mod tests { ); // test reboot is set - assert!( - storage::was_rebooted(&mut host).expect("Should have found flag"), - "Flag should be set" - ); + assert_marked_for_reboot(host.host) } #[test] @@ -1298,10 +1301,7 @@ mod tests { ); // test reboot is set - assert!( - storage::was_rebooted(&mut host).expect("Should have found flag"), - "Flag should be set" - ); + assert_marked_for_reboot(host.host); let bip = read_block_in_progress(&host) .expect("Should be able to read the block in progress") diff --git a/etherlink/kernel_evm/kernel/src/inbox.rs b/etherlink/kernel_evm/kernel/src/inbox.rs index 02c7798fe12cee97a1ff1b3e520c40ef939d3653..c3323d67e89797443369a939c8eaf01ff00caa57 100644 --- a/etherlink/kernel_evm/kernel/src/inbox.rs +++ b/etherlink/kernel_evm/kernel/src/inbox.rs @@ -180,17 +180,21 @@ pub fn read_input( smart_rollup_address: [u8; 20], ticketer: &Option, admin: &Option, + inbox_is_empty: &mut bool, ) -> Result { let input = host.read_input()?; match input { - Some(input) => Ok(InputResult::parse( - host, - input, - smart_rollup_address, - ticketer, - admin, - )), + Some(input) => { + *inbox_is_empty = false; + Ok(InputResult::parse( + host, + input, + smart_rollup_address, + ticketer, + admin, + )) + } None => Ok(InputResult::NoInput), } } @@ -279,16 +283,36 @@ pub fn read_inbox( smart_rollup_address: [u8; 20], ticketer: Option, admin: Option, -) -> Result { +) -> Result, anyhow::Error> { let mut res = InboxContent { kernel_upgrade: None, transactions: vec![], sequencer_blueprints: vec![], }; + // The mutable variable is used to retrieve the information of whether the + // inbox was empty or not. As we consume all the inbox in one go, if the + // variable remains true, that means that the inbox was already consumed + // during this kernel run. + let mut inbox_is_empty = true; loop { - match read_input(host, smart_rollup_address, &ticketer, &admin)? { + match read_input( + host, + smart_rollup_address, + &ticketer, + &admin, + &mut inbox_is_empty, + )? { InputResult::NoInput => { - return Ok(res); + if inbox_is_empty { + // If `inbox_is_empty` is true, that means we haven't see + // any input in the current call of `read_inbox`. Therefore, + // the inbox of this level has already been consumed. + return Ok(None); + } else { + // If it's a `NoInput` and `inbox_is_empty` is false, we + // have simply reached the end of the inbox. + return Ok(Some(res)); + } } InputResult::Unparsable => (), InputResult::Input(Input::SimpleTransaction(tx)) => { @@ -319,11 +343,7 @@ pub fn read_inbox( // simulation and all the previous and next transactions are // discarded. simulation::start_simulation_mode(host)?; - return Ok(InboxContent { - kernel_upgrade: None, - transactions: vec![], - sequencer_blueprints: vec![], - }); + return Ok(None); } InputResult::Input(Input::Info(info)) => { store_last_info_per_level_timestamp(host, info.predecessor_timestamp)?; @@ -465,8 +485,9 @@ mod tests { host.add_external(Bytes::from(input_to_bytes(SMART_ROLLUP_ADDRESS, input))); - let inbox_content = - read_inbox(&mut host, SMART_ROLLUP_ADDRESS, None, None).unwrap(); + let inbox_content = read_inbox(&mut host, SMART_ROLLUP_ADDRESS, None, None) + .unwrap() + .unwrap(); let expected_transactions = vec![Transaction { tx_hash, content: Ethereum(tx), @@ -488,8 +509,9 @@ mod tests { host.add_external(Bytes::from(input_to_bytes(SMART_ROLLUP_ADDRESS, input))) } - let inbox_content = - read_inbox(&mut host, SMART_ROLLUP_ADDRESS, None, None).unwrap(); + let inbox_content = read_inbox(&mut host, SMART_ROLLUP_ADDRESS, None, None) + .unwrap() + .unwrap(); let expected_transactions = vec![Transaction { tx_hash, content: Ethereum(tx), @@ -527,7 +549,9 @@ mod tests { let transfer_metadata = TransferMetadata::new(sender.clone(), source); host.add_transfer(payload, &transfer_metadata); - let inbox_content = read_inbox(&mut host, [0; 20], None, Some(sender)).unwrap(); + let inbox_content = read_inbox(&mut host, [0; 20], None, Some(sender)) + .unwrap() + .unwrap(); let expected_upgrade = Some(KernelUpgrade { preimage_hash }); assert_eq!(inbox_content.kernel_upgrade, expected_upgrade); } @@ -683,8 +707,9 @@ mod tests { host.add_external(Bytes::from(input_to_bytes(SMART_ROLLUP_ADDRESS, chunk0))); - let inbox_content = - read_inbox(&mut host, SMART_ROLLUP_ADDRESS, None, None).unwrap(); + let inbox_content = read_inbox(&mut host, SMART_ROLLUP_ADDRESS, None, None) + .unwrap() + .unwrap(); assert_eq!( inbox_content, InboxContent { @@ -698,8 +723,9 @@ mod tests { for input in inputs { host.add_external(Bytes::from(input_to_bytes(SMART_ROLLUP_ADDRESS, input))) } - let inbox_content = - read_inbox(&mut host, SMART_ROLLUP_ADDRESS, None, None).unwrap(); + let inbox_content = read_inbox(&mut host, SMART_ROLLUP_ADDRESS, None, None) + .unwrap() + .unwrap(); let expected_transactions = vec![Transaction { tx_hash, @@ -752,12 +778,31 @@ mod tests { host.add_external(framed); - let inbox_content = - read_inbox(&mut host, SMART_ROLLUP_ADDRESS, None, None).unwrap(); + let inbox_content = read_inbox(&mut host, SMART_ROLLUP_ADDRESS, None, None) + .unwrap() + .unwrap(); let expected_transactions = vec![Transaction { tx_hash, content: Ethereum(tx), }]; assert_eq!(inbox_content.transactions, expected_transactions); } + + #[test] + fn empty_inbox_returns_none() { + let mut host = MockHost::default(); + + // Even reading the inbox with only the default elements returns + // an empty inbox content. As we test in isolation there is nothing + // in the inbox, we mock it by adding a single input. + host.add_external(Bytes::from(vec![])); + let inbox_content = + read_inbox(&mut host, SMART_ROLLUP_ADDRESS, None, None).unwrap(); + assert!(inbox_content.is_some()); + + // Reading again the inbox returns no inbox content at all. + let inbox_content = + read_inbox(&mut host, SMART_ROLLUP_ADDRESS, None, None).unwrap(); + assert!(inbox_content.is_none()); + } } diff --git a/etherlink/kernel_evm/kernel/src/lib.rs b/etherlink/kernel_evm/kernel/src/lib.rs index a3dfdd5472ff6076773c02d122087c46eac06756..b1602f469e61d594c1b67387290ad36ef4e9a6b2 100644 --- a/etherlink/kernel_evm/kernel/src/lib.rs +++ b/etherlink/kernel_evm/kernel/src/lib.rs @@ -208,32 +208,48 @@ fn retrieve_base_fee_per_gas(host: &mut Host) -> Result(host: &mut Host) -> Result<(), anyhow::Error> { let chain_id = retrieve_chain_id(host).context("Failed to retrieve chain id")?; - if storage::was_rebooted(host)? { - // kernel was rebooted - log!( - host, - Info, - "Kernel was rebooted. Reboot left: {}\n", - host.reboot_left()? - ); - storage::delete_reboot_flag(host)?; - } else { - // first kernel run of the level - match stage_zero(host)? { - MigrationStatus::None | MigrationStatus::Done => { - set_kernel_version(host)?; - let smart_rollup_address = retrieve_smart_rollup_address(host) - .context("Failed to retrieve smart rollup address")?; - let ticketer = read_ticketer(host); - let admin = read_admin(host); - let is_sequencer = is_sequencer(host)?; - stage_one(host, smart_rollup_address, ticketer, admin, is_sequencer) - .context("Failed during stage 1")? - } - MigrationStatus::InProgress => return Ok(()), + + // We always start by doing the migration if needed. + match stage_zero(host)? { + MigrationStatus::None => { + // No migration in progress. However as we want to have the kernel + // version written in the storage, we check for its existence + // at every kernel run. + // The alternative is to enforce every new kernels use the + // installer configuration to initialize this value. + set_kernel_version(host)? + } + // If the migration is still in progress or was finished, we abort the + // current kernel run. + MigrationStatus::InProgress => { + host.mark_for_reboot()?; + return Ok(()); + } + MigrationStatus::Done => { + // If a migrtion was finished, we update the kernel version + // in the storage. + set_kernel_version(host)?; + host.mark_for_reboot()?; + return Ok(()); } }; + + // Fetch kernel metadata. + let smart_rollup_address = retrieve_smart_rollup_address(host) + .context("Failed to retrieve smart rollup address")?; + let ticketer = read_ticketer(host); + let admin = read_admin(host); + let is_sequencer = is_sequencer(host)?; let base_fee_per_gas = retrieve_base_fee_per_gas(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 + // consume all reboots. At least one reboot will be used to consume the + // inbox. + stage_one(host, smart_rollup_address, ticketer, admin, is_sequencer) + .context("Failed during stage 1")?; + + // Start processing blueprints stage_two(host, chain_id, base_fee_per_gas).context("Failed during stage 2") } @@ -331,8 +347,10 @@ mod tests { transaction::{TransactionHash, TransactionType}, tx_common::EthereumTransactionCommon, }; - use tezos_smart_rollup_core::PREIMAGE_HASH_SIZE; + use tezos_smart_rollup_core::{SmartRollupCore, PREIMAGE_HASH_SIZE}; + use tezos_smart_rollup_debug::Runtime; use tezos_smart_rollup_encoding::timestamp::Timestamp; + use tezos_smart_rollup_host::path::RefPath; use tezos_smart_rollup_mock::MockHost; const DUMMY_CHAIN_ID: U256 = U256::one(); @@ -414,6 +432,18 @@ mod tests { } } + fn is_marked_for_reboot(host: &impl SmartRollupCore) -> bool { + const REBOOT_PATH: RefPath = RefPath::assert_from(b"/kernel/env/reboot"); + host.store_read_all(&REBOOT_PATH).is_ok() + } + + fn assert_marked_for_reboot(host: &impl SmartRollupCore) { + assert!( + is_marked_for_reboot(host), + "The kernel should have been marked for reboot" + ); + } + #[test] fn test_reboot_during_block_production() { // init host @@ -475,9 +505,6 @@ mod tests { ); // test reboot is set - assert!( - storage::was_rebooted(&mut host).expect("Should have found flag"), - "Flag should be set" - ); + assert_marked_for_reboot(host.host) } } diff --git a/etherlink/kernel_evm/kernel/src/migration.rs b/etherlink/kernel_evm/kernel/src/migration.rs index 2c4f846f5461beff8db0e7438e75dd13aba804af..6982471f2951c8c6c81d9d5569401c716d525626 100644 --- a/etherlink/kernel_evm/kernel/src/migration.rs +++ b/etherlink/kernel_evm/kernel/src/migration.rs @@ -35,6 +35,17 @@ fn migrate_blueprint_storage(host: &mut Host) -> Result<(), Error // needed migration functions // - compile the kernel and run all the E2E migration tests to make sure all the // data is still available from the EVM proxy-node. +// +// /!\ +// If the migration takes more than 999 reboots, we will lose the inbox +// of a level. At least one reboot must be allocated to the stage one +// to consume the inbox. Therefore, if the migration happens to take more +// than 999 reboots, you have to rethink this. This limitation exists +// because we consider that the inbox should not be collected during +// a migration because it impacts the storage. We could in theory end up +// in an inconsistent storage. +// /!\ +// fn migration(host: &mut Host) -> Result { let current_version = read_storage_version(host)?; if STORAGE_VERSION == current_version + 1 { diff --git a/etherlink/kernel_evm/kernel/src/stage_one.rs b/etherlink/kernel_evm/kernel/src/stage_one.rs index 2cadd8ccc9428a3420641cc302d4cd1845e149ac..07eb3a0d338f8c699b75336b1764a5201ac44ff4 100644 --- a/etherlink/kernel_evm/kernel/src/stage_one.rs +++ b/etherlink/kernel_evm/kernel/src/stage_one.rs @@ -21,22 +21,24 @@ pub fn fetch_inbox_blueprints( ticketer: Option, admin: Option, ) -> Result<(), anyhow::Error> { - let InboxContent { + if let Some(InboxContent { kernel_upgrade, transactions, sequencer_blueprints: _, - } = read_inbox(host, smart_rollup_address, ticketer, admin)?; - let timestamp = current_timestamp(host); - let blueprint = Blueprint { - transactions, - timestamp, - }; - // Store the blueprint. - store_inbox_blueprint(host, blueprint)?; - // Store kernel upgrade. - if let Some(kernel_upgrade) = kernel_upgrade { - store_kernel_upgrade(host, &kernel_upgrade)?; - }; + }) = read_inbox(host, smart_rollup_address, ticketer, admin)? + { + let timestamp = current_timestamp(host); + let blueprint = Blueprint { + transactions, + timestamp, + }; + // Store the blueprint. + store_inbox_blueprint(host, blueprint)?; + // Store kernel upgrade. + if let Some(kernel_upgrade) = kernel_upgrade { + store_kernel_upgrade(host, &kernel_upgrade)?; + }; + } Ok(()) } @@ -46,27 +48,29 @@ fn fetch_sequencer_blueprints( ticketer: Option, admin: Option, ) -> Result<(), anyhow::Error> { - let InboxContent { + if let Some(InboxContent { kernel_upgrade, transactions: _, sequencer_blueprints, - } = read_inbox(host, smart_rollup_address, ticketer, admin)?; - // TODO: store delayed inbox messages (transactions). - // Store the blueprints. - for seq_blueprint in sequencer_blueprints { - log!( - host, - Debug, - "Storing chunk {} of sequencer blueprint number {}", - seq_blueprint.chunk_index, - seq_blueprint.number - ); - store_sequencer_blueprint(host, seq_blueprint)? + }) = read_inbox(host, smart_rollup_address, ticketer, admin)? + { + // TODO: store delayed inbox messages (transactions). + // Store the blueprints. + for seq_blueprint in sequencer_blueprints { + log!( + host, + Debug, + "Storing chunk {} of sequencer blueprint number {}", + seq_blueprint.chunk_index, + seq_blueprint.number + ); + store_sequencer_blueprint(host, seq_blueprint)? + } + // Store kernel upgrade. + if let Some(kernel_upgrade) = kernel_upgrade { + store_kernel_upgrade(host, &kernel_upgrade)?; + }; } - // Store kernel upgrade. - if let Some(kernel_upgrade) = kernel_upgrade { - store_kernel_upgrade(host, &kernel_upgrade)?; - }; Ok(()) } diff --git a/etherlink/kernel_evm/kernel/src/storage.rs b/etherlink/kernel_evm/kernel/src/storage.rs index a81b031b55ef773c2f2d49ad154f1fe63cc1929a..484396821c4dc9aa14b945c39f9f093cc1b6f427 100644 --- a/etherlink/kernel_evm/kernel/src/storage.rs +++ b/etherlink/kernel_evm/kernel/src/storage.rs @@ -41,9 +41,6 @@ const ADMIN: RefPath = RefPath::assert_from(b"/admin"); // Path to the block in progress, used between reboots const EVM_BLOCK_IN_PROGRESS: RefPath = RefPath::assert_from(b"/blocks/in_progress"); -// flag denoting reboot -const REBOOTED: RefPath = RefPath::assert_from(b"/reboot"); - const EVM_CURRENT_BLOCK: RefPath = RefPath::assert_from(b"/blocks/current"); pub const EVM_BLOCKS: RefPath = RefPath::assert_from(b"/blocks"); const BLOCK_NUMBER: RefPath = RefPath::assert_from(b"/number"); @@ -771,20 +768,6 @@ pub fn store_kernel_version( .map_err(Error::from) } -pub fn add_reboot_flag(host: &mut Host) -> Result<(), anyhow::Error> { - host.store_write(&REBOOTED, &[1], 0) - .context("Failed to set reboot flag") -} - -pub fn delete_reboot_flag(host: &mut Host) -> Result<(), anyhow::Error> { - host.store_delete(&REBOOTED) - .context("Failed to delete reboot flag") -} - -pub fn was_rebooted(host: &mut Host) -> Result { - Ok(host.store_read(&REBOOTED, 0, 0).is_ok()) -} - pub fn store_block_in_progress( host: &mut Host, bip: &BlockInProgress, @@ -884,28 +867,3 @@ mod internal_for_tests { #[cfg(test)] pub use internal_for_tests::*; - -#[cfg(test)] -mod tests { - use tezos_smart_rollup_mock::MockHost; - - use super::*; - #[test] - fn test_reboot_flag() { - let mut host = MockHost::default(); - - add_reboot_flag(&mut host).expect("Should have been able to set flag"); - - assert!(was_rebooted(&mut host).expect("should have found reboot flag")); - - delete_reboot_flag(&mut host).expect("Should have been able to delete flag"); - - assert!( - !was_rebooted(&mut host).expect("should not have failed without reboot flag") - ); - - add_reboot_flag(&mut host).expect("Should have been able to set flag"); - - assert!(was_rebooted(&mut host).expect("should have found reboot flag")); - } -} diff --git a/etherlink/kernel_evm/kernel/src/upgrade.rs b/etherlink/kernel_evm/kernel/src/upgrade.rs index 5e4754f13ba83340658cc798597a1edd85cb2991..376d202327581b7ff9e8e87ec512936e8cc85bfc 100644 --- a/etherlink/kernel_evm/kernel/src/upgrade.rs +++ b/etherlink/kernel_evm/kernel/src/upgrade.rs @@ -22,6 +22,11 @@ pub fn upgrade_kernel( .evaluate(host) .map_err(UpgradeProcessError::InternalUpgrade)?; + // Mark for reboot, the upgrade/migration will happen at next + // kernel run, it doesn't matter if it is within the Tezos level + // or not. + host.mark_for_reboot()?; + log!(host, Info, "Kernel is ready to be upgraded."); Ok(()) } diff --git a/tezt/tests/evm_rollup.ml b/tezt/tests/evm_rollup.ml index e854d42a4e77e773ede59a8bccd924e5cec8a625..81d8f59f7374b663a308d14df21d95687ce9d20f 100644 --- a/tezt/tests/evm_rollup.ml +++ b/tezt/tests/evm_rollup.ml @@ -2549,11 +2549,13 @@ let test_kernel_upgrade_failing_migration = @@ fun protocol -> let base_installee = "etherlink/kernel_evm/kernel/tests/resources" in let installee = "failed_migration" in - let* sc_rollup_node, node, client, evm_node, original_kernel_boot_wasm = - gen_test_kernel_upgrade ~base_installee ~installee protocol + let* sc_rollup_node, node, client, evm_node, _original_kernel_boot_wasm = + gen_test_kernel_upgrade + ~base_installee + ~installee + ~should_fail:true + protocol in - (* Fallback mechanism is triggered, no block is produced at that level. *) - let* _ = next_evm_level ~sc_rollup_node ~node ~client in (* We make sure that we can't read under the tmp file, after migration failed, everything is reverted. *) let* tmp_dummy = @@ -2567,21 +2569,17 @@ let test_kernel_upgrade_failing_migration = (match tmp_dummy with | Some _ -> failwith "Nothing should be readable under the temporary dir." | None -> ()) ; - let* kernel_after_migration_failed = get_kernel_boot_wasm ~sc_rollup_node in - (* The upgrade succeeded, but the fallback mechanism was activated, so the kernel - after the upgrade/migration is still the previous one. *) - Check.((original_kernel_boot_wasm = kernel_after_migration_failed) string) - ~error_msg:(sf "Unexpected `boot.wasm` after migration failed.") ; (* We ensure that the fallback mechanism went well by checking if the kernel still produces blocks since it has booted back to the previous, original kernel. *) + let* _ = next_evm_level ~sc_rollup_node ~node ~client in let endpoint = Evm_node.endpoint evm_node in check_block_progression ~sc_rollup_node ~node ~client ~endpoint - ~expected_block_level:2 + ~expected_block_level:3 let send_raw_transaction_request raw_tx = Evm_node.