jleony/nanoreth
1
0
Fork 0
mirror of https://github.com/hl-archive-node/nanoreth.git synced 2025-12-06 10:59:55 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: don't send engine events for dupe payloads (#3231)

Browse Source
This commit is contained in:
Bjerg
2023-06-19 15:25:21 +02:00
committed by GitHub
parent 71d6d7b480
commit c46c11b3d7
5 changed files with 74 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

45
crates/blockchain-tree/src/blockchain_tree.rs
View File

@ -8,7 +8,7 @@ use reth_db::{cursor::DbCursorRO, database::Database, tables, transaction::DbTx}
use reth_interfaces::{
blockchain_tree::{
error::{BlockchainTreeError, InsertBlockError, InsertBlockErrorKind},
BlockStatus, CanonicalOutcome,
BlockStatus, CanonicalOutcome, InsertPayloadOk,
},
consensus::{Consensus, ConsensusError},
executor::{BlockExecutionError, BlockValidationError},
@ -595,7 +595,7 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
pub fn insert_block_without_senders(
&mut self,
block: SealedBlock,
) -> Result<BlockStatus, InsertBlockError> {
) -> Result<InsertPayloadOk, InsertBlockError> {
match block.try_seal_with_senders() {
Ok(block) => self.insert_block(block),
Err(block) => Err(InsertBlockError::sender_recovery_error(block)),
@ -683,10 +683,10 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
pub fn insert_block(
&mut self,
block: SealedBlockWithSenders,
) -> Result<BlockStatus, InsertBlockError> {
) -> Result<InsertPayloadOk, InsertBlockError> {
// check if we already have this block
match self.is_block_known(block.num_hash()) {
Ok(Some(status)) => return Ok(status),
Ok(Some(status)) => return Ok(InsertPayloadOk::AlreadySeen(status)),
Err(err) => return Err(InsertBlockError::new(block.block, err)),
_ => {}
}
@ -696,7 +696,7 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
return Err(InsertBlockError::consensus_error(err, block.block))
}
self.try_insert_validated_block(block)
Ok(InsertPayloadOk::Inserted(self.try_insert_validated_block(block)?))
}
/// Finalize blocks up until and including `finalized_block`, and remove them from the tree.
@ -1220,7 +1220,9 @@ mod tests {
// block 2 parent is not known, block2 is buffered.
assert_eq!(
tree.insert_block(block2.clone()).unwrap(),
BlockStatus::Disconnected { missing_parent: block2.parent_num_hash() }
InsertPayloadOk::Inserted(BlockStatus::Disconnected {
missing_parent: block2.parent_num_hash()
})
);
// Buffered block: [block2]
@ -1248,7 +1250,10 @@ mod tests {
assert_eq!(tree.is_block_known(old_block).unwrap_err().as_tree_error(), Some(err));
// insert block1 and buffered block2 is inserted
assert_eq!(tree.insert_block(block1.clone()).unwrap(), BlockStatus::Valid);
assert_eq!(
tree.insert_block(block1.clone()).unwrap(),
InsertPayloadOk::Inserted(BlockStatus::Valid)
);
// Buffered blocks: []
// Trie state:
@ -1267,11 +1272,17 @@ mod tests {
.with_pending_blocks((block1.number, HashSet::from([block1.hash])))
.assert(&tree);
// already inserted block will return true.
assert_eq!(tree.insert_block(block1.clone()).unwrap(), BlockStatus::Valid);
// already inserted block will `InsertPayloadOk::AlreadySeen(_)`
assert_eq!(
tree.insert_block(block1.clone()).unwrap(),
InsertPayloadOk::AlreadySeen(BlockStatus::Valid)
);
// block two is already inserted.
assert_eq!(tree.insert_block(block2.clone()).unwrap(), BlockStatus::Valid);
assert_eq!(
tree.insert_block(block2.clone()).unwrap(),
InsertPayloadOk::AlreadySeen(BlockStatus::Valid)
);
// make block1 canonical
assert!(tree.make_canonical(&block1.hash()).is_ok());
@ -1308,7 +1319,10 @@ mod tests {
block2a.hash = block2a_hash;
// reinsert two blocks that point to canonical chain
assert_eq!(tree.insert_block(block1a.clone()).unwrap(), BlockStatus::Accepted);
assert_eq!(
tree.insert_block(block1a.clone()).unwrap(),
InsertPayloadOk::Inserted(BlockStatus::Accepted)
);
TreeTester::default()
.with_chain_num(1)
@ -1320,7 +1334,10 @@ mod tests {
.with_pending_blocks((block2.number + 1, HashSet::from([])))
.assert(&tree);
assert_eq!(tree.insert_block(block2a.clone()).unwrap(), BlockStatus::Accepted);
assert_eq!(
tree.insert_block(block2a.clone()).unwrap(),
InsertPayloadOk::Inserted(BlockStatus::Accepted)
);
// Trie state:
// b2 b2a (side chain)
// | /
@ -1504,7 +1521,9 @@ mod tests {
assert_eq!(
tree.insert_block(block2b.clone()).unwrap(),
BlockStatus::Disconnected { missing_parent: block2b.parent_num_hash() }
InsertPayloadOk::Inserted(BlockStatus::Disconnected {
missing_parent: block2b.parent_num_hash()
})
);
TreeTester::default()

9
crates/blockchain-tree/src/shareable.rs
View File

@ -4,8 +4,8 @@ use parking_lot::RwLock;
use reth_db::database::Database;
use reth_interfaces::{
blockchain_tree::{
error::InsertBlockError, BlockStatus, BlockchainTreeEngine, BlockchainTreeViewer,
CanonicalOutcome,
error::InsertBlockError, BlockchainTreeEngine, BlockchainTreeViewer, CanonicalOutcome,
InsertPayloadOk,
},
consensus::Consensus,
Error,
@ -44,7 +44,10 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTreeEngine
self.tree.write().buffer_block(block)
}
fn insert_block(&self, block: SealedBlockWithSenders) -> Result<BlockStatus, InsertBlockError> {
fn insert_block(
&self,
block: SealedBlockWithSenders,
) -> Result<InsertPayloadOk, InsertBlockError> {
trace!(target: "blockchain_tree", hash=?block.hash, number=block.number, parent_hash=?block.parent_hash, "Inserting block");
self.tree.write().insert_block(block)
}

20
crates/consensus/beacon/src/engine/mod.rs
View File

@ -60,6 +60,7 @@ pub(crate) mod sync;
use crate::engine::forkchoice::{ForkchoiceStateHash, ForkchoiceStateTracker};
pub use event::BeaconConsensusEngineEvent;
use reth_interfaces::blockchain_tree::InsertPayloadOk;
/// The maximum number of invalid headers that can be tracked by the engine.
const MAX_INVALID_HEADERS: u32 = 512u32;
@ -893,16 +894,17 @@ where
let mut latest_valid_hash = None;
let block = Arc::new(block);
let status = match status {
BlockStatus::Valid => {
InsertPayloadOk::Inserted(BlockStatus::Valid) => {
latest_valid_hash = Some(block_hash);
self.listeners.notify(BeaconConsensusEngineEvent::CanonicalBlockAdded(block));
PayloadStatusEnum::Valid
}
BlockStatus::Accepted => {
InsertPayloadOk::Inserted(BlockStatus::Accepted) => {
self.listeners.notify(BeaconConsensusEngineEvent::ForkBlockAdded(block));
PayloadStatusEnum::Accepted
}
BlockStatus::Disconnected { .. } => {
InsertPayloadOk::Inserted(BlockStatus::Disconnected { .. }) |
InsertPayloadOk::AlreadySeen(BlockStatus::Disconnected { .. }) => {
// check if the block's parent is already marked as invalid
if let Some(status) =
self.check_invalid_ancestor_with_head(block.parent_hash, block.hash)
@ -913,6 +915,11 @@ where
// not known to be invalid, but we don't know anything else
PayloadStatusEnum::Syncing
}
InsertPayloadOk::AlreadySeen(BlockStatus::Valid) => {
latest_valid_hash = Some(block_hash);
PayloadStatusEnum::Valid
}
InsertPayloadOk::AlreadySeen(BlockStatus::Accepted) => PayloadStatusEnum::Accepted,
};
Ok(PayloadStatus::new(status, latest_valid_hash))
}
@ -1014,18 +1021,19 @@ where
match self.blockchain.insert_block_without_senders(block) {
Ok(status) => {
match status {
BlockStatus::Valid => {
InsertPayloadOk::Inserted(BlockStatus::Valid) => {
// block is connected to the current canonical head and is valid.
self.try_make_sync_target_canonical(num_hash);
}
BlockStatus::Accepted => {
InsertPayloadOk::Inserted(BlockStatus::Accepted) => {
// block is connected to the canonical chain, but not the current head
self.try_make_sync_target_canonical(num_hash);
}
BlockStatus::Disconnected { missing_parent } => {
InsertPayloadOk::Inserted(BlockStatus::Disconnected { missing_parent }) => {
// continue downloading the missing parent
self.sync.download_full_block(missing_parent.hash);
}
_ => (),
}
}
Err(err) => {

19
crates/interfaces/src/blockchain_tree/mod.rs
View File

@ -21,7 +21,7 @@ pub trait BlockchainTreeEngine: BlockchainTreeViewer + Send + Sync {
fn insert_block_without_senders(
&self,
block: SealedBlock,
) -> Result<BlockStatus, InsertBlockError> {
) -> Result<InsertPayloadOk, InsertBlockError> {
match block.try_seal_with_senders() {
Ok(block) => self.insert_block(block),
Err(block) => Err(InsertBlockError::sender_recovery_error(block)),
@ -43,7 +43,10 @@ pub trait BlockchainTreeEngine: BlockchainTreeViewer + Send + Sync {
fn buffer_block(&self, block: SealedBlockWithSenders) -> Result<(), InsertBlockError>;
/// Insert block with senders
fn insert_block(&self, block: SealedBlockWithSenders) -> Result<BlockStatus, InsertBlockError>;
fn insert_block(
&self,
block: SealedBlockWithSenders,
) -> Result<InsertPayloadOk, InsertBlockError>;
/// Finalize blocks up until and including `finalized_block`, and remove them from the tree.
fn finalize_block(&self, finalized_block: BlockNumber);
@ -135,6 +138,18 @@ pub enum BlockStatus {
},
}
/// How a payload was inserted if it was valid.
///
/// If the payload was valid, but has already been seen, [`InsertPayloadOk::AlreadySeen(_)`] is
/// returned, otherwise [`InsertPayloadOk::Inserted(_)`] is returned.
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub enum InsertPayloadOk {
/// The payload was valid, but we have already seen it.
AlreadySeen(BlockStatus),
/// The payload was valid and inserted into the tree.
Inserted(BlockStatus),
}
/// Allows read only functionality on the blockchain tree.
///
/// Tree contains all blocks that are not canonical that can potentially be included

8
crates/storage/provider/src/providers/mod.rs
View File

@ -7,7 +7,7 @@ use crate::{
};
use reth_db::{database::Database, models::StoredBlockBodyIndices};
use reth_interfaces::{
blockchain_tree::{BlockStatus, BlockchainTreeEngine, BlockchainTreeViewer},
blockchain_tree::{BlockchainTreeEngine, BlockchainTreeViewer},
consensus::ForkchoiceState,
Error, Result,
};
@ -37,7 +37,9 @@ mod state;
use crate::{providers::chain_info::ChainInfoTracker, traits::BlockSource};
pub use database::*;
pub use post_state_provider::PostStateProvider;
use reth_interfaces::blockchain_tree::{error::InsertBlockError, CanonicalOutcome};
use reth_interfaces::blockchain_tree::{
error::InsertBlockError, CanonicalOutcome, InsertPayloadOk,
};
/// The main type for interacting with the blockchain.
///
@ -537,7 +539,7 @@ where
fn insert_block(
&self,
block: SealedBlockWithSenders,
) -> std::result::Result<BlockStatus, InsertBlockError> {
) -> std::result::Result<InsertPayloadOk, InsertBlockError> {
self.tree.insert_block(block)
}