From cc36cfaccc3b8ff25391e1fd5f8a7d553956a452 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Cauderlier?= Date: Fri, 6 Dec 2024 14:52:59 +0100 Subject: [PATCH 1/5] Etherlink/Kernel: drop sequencer BP signatures after checking them --- etherlink/kernel_evm/kernel/src/inbox.rs | 2 +- etherlink/kernel_evm/kernel/src/parsing.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/etherlink/kernel_evm/kernel/src/inbox.rs b/etherlink/kernel_evm/kernel/src/inbox.rs index 659aeee1ad9a..ddf7b9a50b71 100644 --- a/etherlink/kernel_evm/kernel/src/inbox.rs +++ b/etherlink/kernel_evm/kernel/src/inbox.rs @@ -340,7 +340,7 @@ impl InputHandler for SequencerInput { delayed_inbox.save_transaction(host, *tx, previous_timestamp, level) } Self::SequencerBlueprint(SequencerBlueprint(seq_blueprint)) => { - handle_blueprint_chunk(host, seq_blueprint.blueprint) + handle_blueprint_chunk(host, seq_blueprint) } Self::SequencerBlueprint( InvalidNumberOfChunks | InvalidSignature | InvalidNumber | Unparsable, diff --git a/etherlink/kernel_evm/kernel/src/parsing.rs b/etherlink/kernel_evm/kernel/src/parsing.rs index def3584379a9..2499f301e0c8 100644 --- a/etherlink/kernel_evm/kernel/src/parsing.rs +++ b/etherlink/kernel_evm/kernel/src/parsing.rs @@ -113,7 +113,7 @@ pub enum ProxyInput { #[derive(Debug, PartialEq, Clone)] pub enum SequencerBlueprintRes { - SequencerBlueprint(SequencerBlueprint), + SequencerBlueprint(UnsignedSequencerBlueprint), InvalidNumberOfChunks, InvalidSignature, InvalidNumber, @@ -352,7 +352,7 @@ pub fn parse_blueprint_chunk( .unwrap_or(false); if correctly_signed { - SequencerBlueprintRes::SequencerBlueprint(seq_blueprint) + SequencerBlueprintRes::SequencerBlueprint(unsigned_seq_blueprint) } else { SequencerBlueprintRes::InvalidSignature } -- GitLab From b018b48143147c4b5babe272e6de4b943e6b43e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Cauderlier?= Date: Mon, 9 Dec 2024 10:04:42 +0100 Subject: [PATCH 2/5] Etherlink/Kernel: parse_{unsigned_,}blueprint_chunk return same type This commit changes the return type of the parse_unsigned_blueprint_chunk function to make it return the same type as parse_blueprint_chunk. In particular, the function now distinguishes RLP deserialization errors from invalid number of chunks. --- etherlink/kernel_evm/kernel/src/dal.rs | 17 +++++++++++------ etherlink/kernel_evm/kernel/src/parsing.rs | 18 +++++++++--------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/etherlink/kernel_evm/kernel/src/dal.rs b/etherlink/kernel_evm/kernel/src/dal.rs index 5c538fa184ca..d2ec4b8972a6 100644 --- a/etherlink/kernel_evm/kernel/src/dal.rs +++ b/etherlink/kernel_evm/kernel/src/dal.rs @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: MIT -use crate::parsing::parse_unsigned_blueprint_chunk; +use crate::parsing::{parse_unsigned_blueprint_chunk, SequencerBlueprintRes}; use crate::sequencer_blueprint::UnsignedSequencerBlueprint; use rlp::{DecoderError, PayloadInfo}; use tezos_evm_logging::{log, Level::*}; @@ -73,11 +73,16 @@ fn parse_unsigned_sequencer_blueprint( bytes: &[u8], ) -> (Option, usize) { if let Result::Ok(chunk_length) = rlp_length(bytes) { - let unsigned_chunk = parse_unsigned_blueprint_chunk(&bytes[..chunk_length]); - ( - unsigned_chunk.map(ParsedInput::UnsignedSequencerBlueprint), - chunk_length + TAG_SIZE, - ) + match parse_unsigned_blueprint_chunk(&bytes[..chunk_length]) { + SequencerBlueprintRes::SequencerBlueprint(unsigned_chunk) => ( + Some(ParsedInput::UnsignedSequencerBlueprint(unsigned_chunk)), + chunk_length + TAG_SIZE, + ), + SequencerBlueprintRes::InvalidNumberOfChunks + | SequencerBlueprintRes::InvalidSignature + | SequencerBlueprintRes::InvalidNumber + | SequencerBlueprintRes::Unparsable => (None, chunk_length + TAG_SIZE), + } } else { log!(host, Debug, "Read an invalid chunk from slot."); (None, TAG_SIZE) diff --git a/etherlink/kernel_evm/kernel/src/parsing.rs b/etherlink/kernel_evm/kernel/src/parsing.rs index 2499f301e0c8..30a0d1d3dc38 100644 --- a/etherlink/kernel_evm/kernel/src/parsing.rs +++ b/etherlink/kernel_evm/kernel/src/parsing.rs @@ -309,18 +309,18 @@ pub struct SequencerParsingContext { pub head_level: Option, } -pub fn parse_unsigned_blueprint_chunk( - bytes: &[u8], -) -> Option { +pub fn parse_unsigned_blueprint_chunk(bytes: &[u8]) -> SequencerBlueprintRes { // Parse an unsigned sequencer blueprint - let unsigned_seq_blueprint: UnsignedSequencerBlueprint = - parsable!(FromRlpBytes::from_rlp_bytes(bytes).ok()); + match UnsignedSequencerBlueprint::from_rlp_bytes(bytes).ok() { + None => SequencerBlueprintRes::Unparsable, + Some(unsigned_seq_blueprint) => { + if MAXIMUM_NUMBER_OF_CHUNKS < unsigned_seq_blueprint.nb_chunks { + return SequencerBlueprintRes::InvalidNumberOfChunks; + } - if MAXIMUM_NUMBER_OF_CHUNKS < unsigned_seq_blueprint.nb_chunks { - return None; + SequencerBlueprintRes::SequencerBlueprint(unsigned_seq_blueprint) + } } - - Some(unsigned_seq_blueprint) } pub fn parse_blueprint_chunk( -- GitLab From a8b71349e7ee1b924556faa48d8e6a6a919e48bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Cauderlier?= Date: Mon, 9 Dec 2024 12:08:20 +0100 Subject: [PATCH 3/5] Etherlink/Kernel/DAL: ignore invalid inputs instead of dropping the whole DAL slot When parsing the content of a DAL slot, there are currently four possible errors: - unknown tag, - failure to compute the length of a blueprint chunk, - RLP deserialization error of the chunk, - the total number of chunks for this blueprint is above the limit. In the three first cases, we don't know where to resume parsing so we simply drop the whole DAL slot. In the last case however, it is possible to drop the invalid chunk and resume parsing after it. This is what this commit does. --- etherlink/kernel_evm/kernel/src/dal.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/etherlink/kernel_evm/kernel/src/dal.rs b/etherlink/kernel_evm/kernel/src/dal.rs index d2ec4b8972a6..1b0b8c6781ad 100644 --- a/etherlink/kernel_evm/kernel/src/dal.rs +++ b/etherlink/kernel_evm/kernel/src/dal.rs @@ -18,6 +18,7 @@ const DAL_PADDING_TAG: u8 = 0; enum ParsedInput { UnsignedSequencerBlueprint(UnsignedSequencerBlueprint), + InvalidInput, Padding, } @@ -80,8 +81,10 @@ fn parse_unsigned_sequencer_blueprint( ), SequencerBlueprintRes::InvalidNumberOfChunks | SequencerBlueprintRes::InvalidSignature - | SequencerBlueprintRes::InvalidNumber - | SequencerBlueprintRes::Unparsable => (None, chunk_length + TAG_SIZE), + | SequencerBlueprintRes::InvalidNumber => { + (Some(ParsedInput::InvalidInput), chunk_length + TAG_SIZE) + } + SequencerBlueprintRes::Unparsable => (None, chunk_length + TAG_SIZE), } } else { log!(host, Debug, "Read an invalid chunk from slot."); @@ -144,6 +147,8 @@ fn parse_slot( None => return buffer, // Once an unparsable input has been read, // stop reading and return the list of chunks read. Some(ParsedInput::UnsignedSequencerBlueprint(b)) => buffer.push(b), + // Invalid inputs are ignored. + Some(ParsedInput::InvalidInput) => {} Some(ParsedInput::Padding) => return buffer, } -- GitLab From ce33f3855400a1749f2f0458298b9df02fd8104f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Cauderlier?= Date: Mon, 9 Dec 2024 13:15:43 +0100 Subject: [PATCH 4/5] Etherlink/Kernel/DAL: test recovery after parsing an invalid chunk --- etherlink/kernel_evm/kernel/src/dal.rs | 39 ++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/etherlink/kernel_evm/kernel/src/dal.rs b/etherlink/kernel_evm/kernel/src/dal.rs index 1b0b8c6781ad..7c5fb2d7389c 100644 --- a/etherlink/kernel_evm/kernel/src/dal.rs +++ b/etherlink/kernel_evm/kernel/src/dal.rs @@ -425,6 +425,45 @@ pub mod tests { assert_eq!(Some(vec![]), parsed_chunks) } + #[test] + fn test_parse_slot_resume_after_invalid_chunk() { + let mut host = MockKernelHost::default(); + + let valid_blueprint_chunks_1 = chunk_blueprint(dummy_big_blueprint(1)); + + let invalid_blueprint_chunks = { + let mut chunks = chunk_blueprint(dummy_big_blueprint(1)); + for chunk in chunks.iter_mut() { + chunk.nb_chunks = crate::blueprint_storage::MAXIMUM_NUMBER_OF_CHUNKS + 1 + } + chunks + }; + + let valid_blueprint_chunks_2 = chunk_blueprint(dummy_big_blueprint(1)); + + let mut chunks = vec![]; + chunks.extend(valid_blueprint_chunks_1.clone()); + chunks.extend(invalid_blueprint_chunks); + chunks.extend(valid_blueprint_chunks_2.clone()); + + let mut expected_chunks = vec![]; + expected_chunks.extend(valid_blueprint_chunks_1); + expected_chunks.extend(valid_blueprint_chunks_2); + + let dal_parameters = host.reveal_dal_parameters(); + let published_level = host.host.level() - (dal_parameters.attestation_lag as u32); + prepare_dal_slot(&mut host, &chunks, published_level as i32, 0); + + let chunks_from_slot = fetch_and_parse_sequencer_blueprint_from_dal( + &mut host, + &dal_parameters, + 0, + published_level, + ); + + assert_eq!(Some(expected_chunks), chunks_from_slot) + } + #[test] fn test_parse_slot_with_invalid_first_chunk() { // The tag announces a chunk, the data is not an RLP encoded chunk -- GitLab From caaafe726099c31ddbd4d303ee20a00fa1317d79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Cauderlier?= Date: Mon, 9 Dec 2024 13:24:51 +0100 Subject: [PATCH 5/5] Etherlink/Kernel/Changelog: mention !15926 --- etherlink/CHANGES_KERNEL.md | 1 + 1 file changed, 1 insertion(+) diff --git a/etherlink/CHANGES_KERNEL.md b/etherlink/CHANGES_KERNEL.md index ce12e88a0ac2..0570ad4c703d 100644 --- a/etherlink/CHANGES_KERNEL.md +++ b/etherlink/CHANGES_KERNEL.md @@ -21,6 +21,7 @@ - Avoid writing gas price twice per block by adding it to block-in-progress. (!15574) - Transaction validation support in simulation is dropped, the validation must be done outside of the kernel. (!15958) +- Parsing a DAL slot is now resumed after invalid inputs. (!15926) ### Bug fixes -- GitLab