From d040dce6e9c995f9ad2b935bc5be2549eca96c88 Mon Sep 17 00:00:00 2001 From: Roman Krasiuk Date: Tue, 27 Aug 2024 21:25:08 +0200 Subject: [PATCH 1/5] feat(debug): engine reorg util depth --- crates/engine/util/src/lib.rs | 12 ++- crates/engine/util/src/reorg.rs | 170 +++++++++++++++++--------------- 2 files changed, 103 insertions(+), 79 deletions(-) diff --git a/crates/engine/util/src/lib.rs b/crates/engine/util/src/lib.rs index 733475e15488..e8fab4b6b097 100644 --- a/crates/engine/util/src/lib.rs +++ b/crates/engine/util/src/lib.rs @@ -101,11 +101,19 @@ pub trait EngineMessageStreamExt: evm_config: Evm, payload_validator: ExecutionPayloadValidator, frequency: usize, + depth: Option, ) -> EngineReorg where Self: Sized, { - EngineReorg::new(self, provider, evm_config, payload_validator, frequency) + EngineReorg::new( + self, + provider, + evm_config, + payload_validator, + frequency, + depth.unwrap_or_default(), + ) } /// If frequency is [Some], returns the stream that creates reorgs with @@ -116,6 +124,7 @@ pub trait EngineMessageStreamExt: evm_config: Evm, payload_validator: ExecutionPayloadValidator, frequency: Option, + depth: Option, ) -> Either, Self> where Self: Sized, @@ -127,6 +136,7 @@ pub trait EngineMessageStreamExt: evm_config, payload_validator, frequency, + depth.unwrap_or_default(), )) } else { Either::Right(self) diff --git a/crates/engine/util/src/reorg.rs b/crates/engine/util/src/reorg.rs index b9a76cb1c124..547cb0272a15 100644 --- a/crates/engine/util/src/reorg.rs +++ b/crates/engine/util/src/reorg.rs @@ -61,6 +61,8 @@ pub struct EngineReorg { payload_validator: ExecutionPayloadValidator, /// The frequency of reorgs. frequency: usize, + /// The depth of reorgs. + depth: usize, /// The number of forwarded forkchoice states. /// This is reset after a reorg. forkchoice_states_forwarded: usize, @@ -80,6 +82,7 @@ impl EngineReorg Self { Self { stream, @@ -87,6 +90,7 @@ impl EngineReorg { - if this.forkchoice_states_forwarded > this.frequency { - if let Some(last_forkchoice_state) = this - .last_forkchoice_state - // Only enter reorg state if new payload attaches to current head. - .filter(|state| state.head_block_hash == payload.parent_hash()) - { - // Enter the reorg state. - // The current payload will be immediately forwarded by being in front - // of the queue. Then we attempt to reorg the current head by generating - // a payload that attaches to the head's parent and is based on the - // non-conflicting transactions (txs from block `n + 1` that are valid - // at block `n` according to consensus checks) from the current payload - // as well as the corresponding forkchoice state. We will rely on CL to - // reorg us back to canonical chain. - // TODO: This is an expensive blocking operation, ideally it's spawned - // as a task so that the stream could yield the control back. - let (reorg_payload, reorg_cancun_fields) = match create_reorg_head( - this.provider, - this.evm_config, - this.payload_validator, - payload.clone(), - cancun_fields.clone(), - ) { - Ok(result) => result, - Err(error) => { - error!(target: "engine::stream::reorg", %error, "Error attempting to create reorg head"); - // Forward the payload and attempt to create reorg on top of the - // next one - return Poll::Ready(Some(BeaconEngineMessage::NewPayload { - payload, - cancun_fields, - tx, - })) - } - }; - let reorg_forkchoice_state = ForkchoiceState { - finalized_block_hash: last_forkchoice_state.finalized_block_hash, - safe_block_hash: last_forkchoice_state.safe_block_hash, - head_block_hash: reorg_payload.block_hash(), - }; - - let (reorg_payload_tx, reorg_payload_rx) = oneshot::channel(); - let (reorg_fcu_tx, reorg_fcu_rx) = oneshot::channel(); - this.reorg_responses.extend([ - Box::pin(reorg_payload_rx.map_ok(Either::Left)) as ReorgResponseFut, - Box::pin(reorg_fcu_rx.map_ok(Either::Right)) as ReorgResponseFut, - ]); - - *this.state = EngineReorgState::Reorg { - queue: VecDeque::from([ - // Current payload - BeaconEngineMessage::NewPayload { payload, cancun_fields, tx }, - // Reorg payload - BeaconEngineMessage::NewPayload { - payload: reorg_payload, - cancun_fields: reorg_cancun_fields, - tx: reorg_payload_tx, - }, - // Reorg forkchoice state - BeaconEngineMessage::ForkchoiceUpdated { - state: reorg_forkchoice_state, - payload_attrs: None, - tx: reorg_fcu_tx, - }, - ]), - }; - continue + let item = match (next, &this.last_forkchoice_state) { + ( + Some(BeaconEngineMessage::NewPayload { payload, cancun_fields, tx }), + Some(last_forkchoice_state), + ) if this.forkchoice_states_forwarded <= this.frequency && + // Only enter reorg state if new payload attaches to current head. + last_forkchoice_state.head_block_hash == payload.parent_hash() => + { + // Enter the reorg state. + // The current payload will be immediately forwarded by being in front of the + // queue. Then we attempt to reorg the current head by generating a payload that + // attaches to the head's parent and is based on the non-conflicting + // transactions (txs from block `n + 1` that are valid at block `n` according to + // consensus checks) from the current payload as well as the corresponding + // forkchoice state. We will rely on CL to reorg us back to canonical chain. + // TODO: This is an expensive blocking operation, ideally it's spawned as a task + // so that the stream could yield the control back. + let (reorg_payload, reorg_cancun_fields) = match create_reorg_head( + this.provider, + this.evm_config, + this.payload_validator, + *this.depth, + payload.clone(), + cancun_fields.clone(), + ) { + Ok(result) => result, + Err(error) => { + error!(target: "engine::stream::reorg", %error, "Error attempting to create reorg head"); + // Forward the payload and attempt to create reorg on top of + // the next one + return Poll::Ready(Some(BeaconEngineMessage::NewPayload { + payload, + cancun_fields, + tx, + })) } - } - Some(BeaconEngineMessage::NewPayload { payload, cancun_fields, tx }) + }; + let reorg_forkchoice_state = ForkchoiceState { + finalized_block_hash: last_forkchoice_state.finalized_block_hash, + safe_block_hash: last_forkchoice_state.safe_block_hash, + head_block_hash: reorg_payload.block_hash(), + }; + + let (reorg_payload_tx, reorg_payload_rx) = oneshot::channel(); + let (reorg_fcu_tx, reorg_fcu_rx) = oneshot::channel(); + this.reorg_responses.extend([ + Box::pin(reorg_payload_rx.map_ok(Either::Left)) as ReorgResponseFut, + Box::pin(reorg_fcu_rx.map_ok(Either::Right)) as ReorgResponseFut, + ]); + + let queue = VecDeque::from([ + // Current payload + BeaconEngineMessage::NewPayload { payload, cancun_fields, tx }, + // Reorg payload + BeaconEngineMessage::NewPayload { + payload: reorg_payload, + cancun_fields: reorg_cancun_fields, + tx: reorg_payload_tx, + }, + // Reorg forkchoice state + BeaconEngineMessage::ForkchoiceUpdated { + state: reorg_forkchoice_state, + payload_attrs: None, + tx: reorg_fcu_tx, + }, + ]); + *this.state = EngineReorgState::Reorg { queue }; + continue } - Some(BeaconEngineMessage::ForkchoiceUpdated { state, payload_attrs, tx }) => { + (Some(BeaconEngineMessage::ForkchoiceUpdated { state, payload_attrs, tx }), _) => { // Record last forkchoice state forwarded to the engine. // We do not care if it's valid since engine should be able to handle // reorgs that rely on invalid forkchoice state. @@ -219,7 +219,7 @@ where *this.forkchoice_states_forwarded += 1; Some(BeaconEngineMessage::ForkchoiceUpdated { state, payload_attrs, tx }) } - item => item, + (item, _) => item, }; return Poll::Ready(item) } @@ -230,6 +230,7 @@ fn create_reorg_head( provider: &Provider, evm_config: &Evm, payload_validator: &ExecutionPayloadValidator, + mut depth: usize, next_payload: ExecutionPayload, next_cancun_fields: Option, ) -> RethResult<(ExecutionPayload, Option)> @@ -244,10 +245,23 @@ where .ensure_well_formed_payload(next_payload, next_cancun_fields.into()) .map_err(RethError::msg)?; - // Fetch reorg target block and its parent - let reorg_target = provider - .block_by_hash(next_block.parent_hash)? - .ok_or_else(|| ProviderError::HeaderNotFound(next_block.parent_hash.into()))?; + // Fetch reorg target block depending on its depth and its parent. + let mut parent_hash = next_block.parent_hash; + let mut candidate_transactions = next_block.body; + let reorg_target = 'target: { + loop { + let reorg_target = provider + .block_by_hash(parent_hash)? + .ok_or_else(|| ProviderError::HeaderNotFound(parent_hash.into()))?; + if depth == 0 { + break 'target reorg_target + } + + depth -= 1; + parent_hash = reorg_target.parent_hash; + candidate_transactions = reorg_target.body; + } + }; let reorg_target_parent = provider .block_by_hash(reorg_target.parent_hash)? .ok_or_else(|| ProviderError::HeaderNotFound(reorg_target.parent_hash.into()))?; @@ -287,7 +301,7 @@ where let mut transactions = Vec::new(); let mut receipts = Vec::new(); let mut versioned_hashes = Vec::new(); - for tx in next_block.body { + for tx in candidate_transactions { // ensure we still have capacity for this transaction if cumulative_gas_used + tx.gas_limit() > reorg_target.gas_limit { continue From d5e48f199267a556eb762a66ecb0f5a92e489cde Mon Sep 17 00:00:00 2001 From: Roman Krasiuk Date: Tue, 27 Aug 2024 21:35:46 +0200 Subject: [PATCH 2/5] enable --- crates/node/builder/src/launch/engine.rs | 1 + crates/node/builder/src/launch/mod.rs | 1 + crates/node/core/src/args/debug.rs | 4 ++++ 3 files changed, 6 insertions(+) diff --git a/crates/node/builder/src/launch/engine.rs b/crates/node/builder/src/launch/engine.rs index 0f3f9dc95e70..da77057e47c2 100644 --- a/crates/node/builder/src/launch/engine.rs +++ b/crates/node/builder/src/launch/engine.rs @@ -150,6 +150,7 @@ where ctx.components().evm_config().clone(), reth_payload_validator::ExecutionPayloadValidator::new(ctx.chain_spec()), node_config.debug.reorg_frequency, + node_config.debug.reorg_depth, ) // Store messages _after_ skipping so that `replay-engine` command // would replay only the messages that were observed by the engine diff --git a/crates/node/builder/src/launch/mod.rs b/crates/node/builder/src/launch/mod.rs index b46747f3ca99..f56600fa5086 100644 --- a/crates/node/builder/src/launch/mod.rs +++ b/crates/node/builder/src/launch/mod.rs @@ -198,6 +198,7 @@ where ctx.components().evm_config().clone(), reth_payload_validator::ExecutionPayloadValidator::new(ctx.chain_spec()), node_config.debug.reorg_frequency, + node_config.debug.reorg_depth, ) // Store messages _after_ skipping so that `replay-engine` command // would replay only the messages that were observed by the engine diff --git a/crates/node/core/src/args/debug.rs b/crates/node/core/src/args/debug.rs index 010e9112c437..084e5cdc82c1 100644 --- a/crates/node/core/src/args/debug.rs +++ b/crates/node/core/src/args/debug.rs @@ -54,6 +54,10 @@ pub struct DebugArgs { #[arg(long = "debug.reorg-frequency", help_heading = "Debug")] pub reorg_frequency: Option, + /// The reorg depth for chain reorgs. + #[arg(long = "debug.reorg-depth", requires = "reorg_frequency", help_heading = "Debug")] + pub reorg_depth: Option, + /// The path to store engine API messages at. /// If specified, all of the intercepted engine API messages /// will be written to specified location. From 1813afdda124a120c2f9d9d1db31a7005b09b26c Mon Sep 17 00:00:00 2001 From: Roman Krasiuk Date: Tue, 27 Aug 2024 21:41:41 +0200 Subject: [PATCH 3/5] upd book --- book/cli/reth/node.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/book/cli/reth/node.md b/book/cli/reth/node.md index b3c51fdf416c..9217dc14fa24 100644 --- a/book/cli/reth/node.md +++ b/book/cli/reth/node.md @@ -522,6 +522,9 @@ Debug: --debug.reorg-frequency If provided, the chain will be reorged at specified frequency + --debug.reorg-depth + The reorg depth for chain reorgs + --debug.engine-api-store The path to store engine API messages at. If specified, all of the intercepted engine API messages will be written to specified location From 948138adf0269ffaa35dadc83f0ed18e2587aa6c Mon Sep 17 00:00:00 2001 From: Roman Krasiuk Date: Tue, 27 Aug 2024 21:51:08 +0200 Subject: [PATCH 4/5] add log --- crates/engine/util/src/reorg.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/crates/engine/util/src/reorg.rs b/crates/engine/util/src/reorg.rs index 547cb0272a15..560263172eaf 100644 --- a/crates/engine/util/src/reorg.rs +++ b/crates/engine/util/src/reorg.rs @@ -246,19 +246,19 @@ where .map_err(RethError::msg)?; // Fetch reorg target block depending on its depth and its parent. - let mut parent_hash = next_block.parent_hash; + let mut previous_hash = next_block.parent_hash; let mut candidate_transactions = next_block.body; let reorg_target = 'target: { loop { let reorg_target = provider - .block_by_hash(parent_hash)? - .ok_or_else(|| ProviderError::HeaderNotFound(parent_hash.into()))?; + .block_by_hash(previous_hash)? + .ok_or_else(|| ProviderError::HeaderNotFound(previous_hash.into()))?; if depth == 0 { break 'target reorg_target } depth -= 1; - parent_hash = reorg_target.parent_hash; + previous_hash = reorg_target.parent_hash; candidate_transactions = reorg_target.body; } }; @@ -266,6 +266,8 @@ where .block_by_hash(reorg_target.parent_hash)? .ok_or_else(|| ProviderError::HeaderNotFound(reorg_target.parent_hash.into()))?; + debug!(target: "engine::stream::reorg", number = reorg_target.number, hash = %previous_hash, "Selected reorg target"); + // Configure state let state_provider = provider.state_by_block_hash(reorg_target.parent_hash)?; let mut state = State::builder() From 4ee0a22ec591bcf1daddd19d1f98c1f7700b2bc3 Mon Sep 17 00:00:00 2001 From: Roman Krasiuk Date: Tue, 27 Aug 2024 21:52:35 +0200 Subject: [PATCH 5/5] fix freq check --- crates/engine/util/src/reorg.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/engine/util/src/reorg.rs b/crates/engine/util/src/reorg.rs index 560263172eaf..37d71f25f630 100644 --- a/crates/engine/util/src/reorg.rs +++ b/crates/engine/util/src/reorg.rs @@ -146,7 +146,7 @@ where ( Some(BeaconEngineMessage::NewPayload { payload, cancun_fields, tx }), Some(last_forkchoice_state), - ) if this.forkchoice_states_forwarded <= this.frequency && + ) if this.forkchoice_states_forwarded > this.frequency && // Only enter reorg state if new payload attaches to current head. last_forkchoice_state.head_block_hash == payload.parent_hash() => {