diff --git a/crates/blockchain-tree/src/blockchain_tree.rs b/crates/blockchain-tree/src/blockchain_tree.rs index 150928acea..8f654c1358 100644 --- a/crates/blockchain-tree/src/blockchain_tree.rs +++ b/crates/blockchain-tree/src/blockchain_tree.rs @@ -167,7 +167,7 @@ impl BlockchainTree } // 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 BlockchainTree } // 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 BlockchainTree } } + /// Determines whether or not a block is canonical, checking the db if necessary. + pub fn is_block_hash_canonical(&self, hash: &BlockHash) -> Result { + // 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 BlockchainTree 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: diff --git a/crates/consensus/beacon/src/engine/mod.rs b/crates/consensus/beacon/src/engine/mod.rs index ec39970212..b8e2223a56 100644 --- a/crates/consensus/beacon/src/engine/mod.rs +++ b/crates/consensus/beacon/src/engine/mod.rs @@ -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 = diff --git a/crates/primitives/src/chain/spec.rs b/crates/primitives/src/chain/spec.rs index 1fb1854146..d29225eff3 100644 --- a/crates/primitives/src/chain/spec.rs +++ b/crates/primitives/src/chain/spec.rs @@ -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));