fix: check db and indices for canonical block (#2702)

This commit is contained in:
Dan Cline
2023-05-18 05:20:31 -04:00
committed by GitHub
parent 65b7702efe
commit 460bf13b63
3 changed files with 110 additions and 32 deletions

View File

@@ -167,7 +167,7 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
}
// check if block is part of canonical chain
if self.block_indices.canonical_hash(&block.number) == Some(block.hash) {
if self.is_block_hash_canonical(&block.hash)? {
return Ok(Some(BlockStatus::Valid))
}
@@ -275,7 +275,7 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
}
// if not found, check if the parent can be found inside canonical chain.
if Some(parent.hash) == self.block_indices.canonical_hash(&parent.number) {
if self.is_block_hash_canonical(&parent.hash)? {
return self.try_append_canonical_chain(block, parent)
}
@@ -701,6 +701,21 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
}
}
/// Determines whether or not a block is canonical, checking the db if necessary.
pub fn is_block_hash_canonical(&self, hash: &BlockHash) -> Result<bool, Error> {
// if the indices show that the block hash is not canonical, it's either in a sidechain or
// canonical, but in the db. If it is in a sidechain, it is not canonical. If it is not in
// the db, then it is not canonical.
if !self.block_indices.is_block_hash_canonical(hash) &&
(self.block_by_hash(*hash).is_some() ||
self.externals.shareable_db().header(hash)?.is_none())
{
return Ok(false)
}
Ok(true)
}
/// Make a block and its parent(s) part of the canonical chain.
///
/// # Note
@@ -718,7 +733,7 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
let old_buffered_blocks = self.buffered_blocks.parent_to_child.clone();
// If block is already canonical don't return error.
if self.block_indices.is_block_hash_canonical(block_hash) {
if self.is_block_hash_canonical(block_hash)? {
info!(target: "blockchain_tree", ?block_hash, "Block is already canonical");
let td = self
.externals
@@ -1038,6 +1053,9 @@ mod tests {
// genesis block 10 is already canonical
assert_eq!(tree.make_canonical(&H256::zero()), Ok(()));
// make sure is_block_hash_canonical returns true for genesis block
assert!(tree.is_block_hash_canonical(&H256::zero()).unwrap());
// make genesis block 10 as finalized
tree.finalize_block(10);
@@ -1218,6 +1236,13 @@ mod tests {
if *old.blocks() == BTreeMap::from([(block1.number,block1.clone()),(block2a.number,block2a.clone())])
&& *new.blocks() == BTreeMap::from([(block1a.number,block1a.clone())]));
// check that b2 and b1 are not canonical
assert!(!tree.is_block_hash_canonical(&block2.hash).unwrap());
assert!(!tree.is_block_hash_canonical(&block1.hash).unwrap());
// ensure that b1a is canonical
assert!(tree.is_block_hash_canonical(&block1a.hash).unwrap());
// make b2 canonical
assert_eq!(tree.make_canonical(&block2.hash()), Ok(()));
@@ -1246,6 +1271,9 @@ mod tests {
if *old.blocks() == BTreeMap::from([(block1a.number,block1a.clone())])
&& *new.blocks() == BTreeMap::from([(block1.number,block1.clone()),(block2.number,block2.clone())]));
// check that b2 is now canonical
assert!(tree.is_block_hash_canonical(&block2.hash).unwrap());
// finalize b1 that would make b1a removed from tree
tree.finalize_block(11);
// Trie state:

View File

@@ -1253,11 +1253,7 @@ mod tests {
..Default::default()
};
let rx_invalid = env.send_forkchoice_updated(forkchoice);
let expected_result = ForkchoiceUpdated::from_status(PayloadStatusEnum::Syncing);
assert_matches!(rx_invalid.await, Ok(result) => assert_eq!(result, expected_result));
let result = env.send_forkchoice_retry_on_syncing(forkchoice).await.unwrap();
let result = env.send_forkchoice_updated(forkchoice).await.unwrap();
let expected_result = ForkchoiceUpdated::new(PayloadStatus::new(
PayloadStatusEnum::Valid,
Some(block1.hash),
@@ -1297,13 +1293,15 @@ mod tests {
..Default::default()
};
let invalid_rx = env.send_forkchoice_updated(next_forkchoice_state);
// if we `await` in the assert, the forkchoice will poll after we've inserted the block,
// and it will return VALID instead of SYNCING
let invalid_rx = env.send_forkchoice_updated(next_forkchoice_state).await;
// Insert next head immediately after sending forkchoice update
insert_blocks(env.db.as_ref(), [&next_head].into_iter());
let expected_result = ForkchoiceUpdated::from_status(PayloadStatusEnum::Syncing);
assert_matches!(invalid_rx.await, Ok(result) => assert_eq!(result, expected_result));
assert_matches!(invalid_rx, Ok(result) => assert_eq!(result, expected_result));
let result = env.send_forkchoice_retry_on_syncing(next_forkchoice_state).await.unwrap();
let expected_result = ForkchoiceUpdated::from_status(PayloadStatusEnum::Valid)
@@ -1346,6 +1344,56 @@ mod tests {
drop(engine);
}
#[tokio::test]
async fn forkchoice_updated_pre_merge() {
let chain_spec = Arc::new(
ChainSpecBuilder::default()
.chain(MAINNET.chain)
.genesis(MAINNET.genesis.clone())
.london_activated()
.paris_at_ttd(U256::from(3))
.build(),
);
let (consensus_engine, env) = setup_consensus_engine(
chain_spec,
VecDeque::from([
Ok(ExecOutput { done: true, stage_progress: 0 }),
Ok(ExecOutput { done: true, stage_progress: 0 }),
]),
Vec::default(),
);
let genesis = random_block(0, None, None, Some(0));
let mut block1 = random_block(1, Some(genesis.hash), None, Some(0));
block1.header.difficulty = U256::from(1);
// a second pre-merge block
let mut block2 = random_block(1, Some(genesis.hash), None, Some(0));
block2.header.difficulty = U256::from(1);
// a transition block
let mut block3 = random_block(1, Some(genesis.hash), None, Some(0));
block3.header.difficulty = U256::from(1);
insert_blocks(env.db.as_ref(), [&genesis, &block1, &block2, &block3].into_iter());
let _engine = spawn_consensus_engine(consensus_engine);
let res = env
.send_forkchoice_updated(ForkchoiceState {
head_block_hash: block1.hash,
finalized_block_hash: block1.hash,
..Default::default()
})
.await;
assert_matches!(res, Ok(result) => {
let ForkchoiceUpdated { payload_status, .. } = result;
assert_matches!(payload_status.status, PayloadStatusEnum::Invalid { .. });
assert_eq!(payload_status.latest_valid_hash, Some(H256::zero()));
});
}
#[tokio::test]
async fn forkchoice_updated_invalid_pow() {
let chain_spec = Arc::new(
@@ -1378,24 +1426,12 @@ mod tests {
..Default::default()
})
.await;
let expected_result = ForkchoiceUpdated::from_status(PayloadStatusEnum::Syncing);
assert_matches!(res, Ok(result) => assert_eq!(result, expected_result));
let result = env
.send_forkchoice_retry_on_syncing(ForkchoiceState {
head_block_hash: block1.hash,
finalized_block_hash: block1.hash,
..Default::default()
})
.await
.unwrap();
let expected_result = ForkchoiceUpdated::from_status(PayloadStatusEnum::Invalid {
validation_error: BlockExecutionError::BlockPreMerge { hash: block1.hash }
.to_string(),
})
.with_latest_valid_hash(H256::zero());
assert_eq!(result, expected_result);
assert_matches!(res, Ok(result) => assert_eq!(result, expected_result));
}
}
@@ -1467,9 +1503,9 @@ mod tests {
..Default::default()
})
.await;
let expected_result =
ForkchoiceUpdated::new(PayloadStatus::from_status(PayloadStatusEnum::Syncing));
assert_matches!(res, Ok(result) => assert_eq!(result, expected_result));
let expected_result = PayloadStatus::from_status(PayloadStatusEnum::Valid)
.with_latest_valid_hash(block1.hash);
assert_matches!(res, Ok(ForkchoiceUpdated { payload_status, .. }) => assert_eq!(payload_status, expected_result));
// Send new payload
let result =
@@ -1509,9 +1545,9 @@ mod tests {
..Default::default()
})
.await;
let expected_result =
ForkchoiceUpdated::new(PayloadStatus::from_status(PayloadStatusEnum::Syncing));
assert_matches!(res, Ok(result) => assert_eq!(result, expected_result));
let expected_result = PayloadStatus::from_status(PayloadStatusEnum::Valid)
.with_latest_valid_hash(genesis.hash);
assert_matches!(res, Ok(ForkchoiceUpdated { payload_status, .. }) => assert_eq!(payload_status, expected_result));
// Send new payload
let block = random_block(2, Some(H256::random()), None, Some(0));
@@ -1561,9 +1597,13 @@ mod tests {
..Default::default()
})
.await;
let expected_result =
ForkchoiceUpdated::new(PayloadStatus::from_status(PayloadStatusEnum::Syncing));
assert_matches!(res, Ok(result) => assert_eq!(result, expected_result));
let expected_result = PayloadStatus::from_status(PayloadStatusEnum::Invalid {
validation_error: BlockExecutionError::BlockPreMerge { hash: block1.hash }
.to_string(),
})
.with_latest_valid_hash(H256::zero());
assert_matches!(res, Ok(ForkchoiceUpdated { payload_status, .. }) => assert_eq!(payload_status, expected_result));
// Send new payload
let result =

View File

@@ -397,6 +397,16 @@ impl ChainSpecBuilder {
self
}
/// Enable the Paris hardfork at the given TTD.
///
/// Does not set the merge netsplit block.
pub fn paris_at_ttd(self, ttd: U256) -> Self {
self.with_fork(
Hardfork::Paris,
ForkCondition::TTD { total_difficulty: ttd, fork_block: None },
)
}
/// Enable Frontier at genesis.
pub fn frontier_activated(mut self) -> Self {
self.hardforks.insert(Hardfork::Frontier, ForkCondition::Block(0));