From 59bffd411202ad7c0869868f50c2fdb500d9be97 Mon Sep 17 00:00:00 2001 From: Dan Cline <6798349+Rjected@users.noreply.github.com> Date: Thu, 7 Sep 2023 06:57:31 -0400 Subject: [PATCH] fix: filter out pre-genesis timestamp forks (#4501) --- crates/net/eth-wire/src/ethstream.rs | 8 +-- crates/primitives/src/chain/spec.rs | 91 +++++++++++++++++++++++----- crates/primitives/src/forkid.rs | 12 +++- 3 files changed, 91 insertions(+), 20 deletions(-) diff --git a/crates/net/eth-wire/src/ethstream.rs b/crates/net/eth-wire/src/ethstream.rs index 70472e9e9f..3c06b10d4c 100644 --- a/crates/net/eth-wire/src/ethstream.rs +++ b/crates/net/eth-wire/src/ethstream.rs @@ -333,7 +333,7 @@ mod tests { #[tokio::test] async fn can_handshake() { let genesis = H256::random(); - let fork_filter = ForkFilter::new(Head::default(), genesis, Vec::new()); + let fork_filter = ForkFilter::new(Head::default(), genesis, 0, Vec::new()); let status = Status { version: EthVersion::Eth67 as u8, @@ -380,7 +380,7 @@ mod tests { #[tokio::test] async fn pass_handshake_on_low_td_bitlen() { let genesis = H256::random(); - let fork_filter = ForkFilter::new(Head::default(), genesis, Vec::new()); + let fork_filter = ForkFilter::new(Head::default(), genesis, 0, Vec::new()); let status = Status { version: EthVersion::Eth67 as u8, @@ -427,7 +427,7 @@ mod tests { #[tokio::test] async fn fail_handshake_on_high_td_bitlen() { let genesis = H256::random(); - let fork_filter = ForkFilter::new(Head::default(), genesis, Vec::new()); + let fork_filter = ForkFilter::new(Head::default(), genesis, 0, Vec::new()); let status = Status { version: EthVersion::Eth67 as u8, @@ -568,7 +568,7 @@ mod tests { ); let genesis = H256::random(); - let fork_filter = ForkFilter::new(Head::default(), genesis, Vec::new()); + let fork_filter = ForkFilter::new(Head::default(), genesis, 0, Vec::new()); let status = Status { version: EthVersion::Eth67 as u8, diff --git a/crates/primitives/src/chain/spec.rs b/crates/primitives/src/chain/spec.rs index 39782cdd32..f73bcc9830 100644 --- a/crates/primitives/src/chain/spec.rs +++ b/crates/primitives/src/chain/spec.rs @@ -339,6 +339,11 @@ impl ChainSpec { } } + /// Get the timestamp of the genesis block. + pub fn genesis_timestamp(&self) -> u64 { + self.genesis.timestamp + } + /// Returns the final total difficulty if the given block number is after the Paris hardfork. /// /// Note: technically this would also be valid for the block before the paris upgrade, but this @@ -405,7 +410,7 @@ impl ChainSpec { }) }); - ForkFilter::new(head, self.genesis_hash(), forks) + ForkFilter::new(head, self.genesis_hash(), self.genesis_timestamp(), forks) } /// Compute the [`ForkId`] for the given [`Head`] folowing eip-6122 spec @@ -434,19 +439,22 @@ impl ChainSpec { } // timestamp are ALWAYS applied after the merge. - for (_, cond) in self.forks_iter() { - if let ForkCondition::Timestamp(timestamp) = cond { - if cond.active_at_head(head) { - if timestamp != current_applied { - forkhash += timestamp; - current_applied = timestamp; - } - } else { - // can safely return here because we have already handled all block forks and - // have handled all active timestamp forks, and set the next value to the - // timestamp that is known but not active yet - return ForkId { hash: forkhash, next: timestamp } + // + // this filter ensures that no block-based forks are returned + for timestamp in self.forks_iter().filter_map(|(_, cond)| { + cond.as_timestamp().filter(|time| time > &self.genesis.timestamp) + }) { + let cond = ForkCondition::Timestamp(timestamp); + if cond.active_at_head(head) { + if timestamp != current_applied { + forkhash += timestamp; + current_applied = timestamp; } + } else { + // can safely return here because we have already handled all block forks and + // have handled all active timestamp forks, and set the next value to the + // timestamp that is known but not active yet + return ForkId { hash: forkhash, next: timestamp } } } @@ -594,7 +602,7 @@ impl From for ChainSpec { } /// A helper to build custom chain specs -#[derive(Debug, Default)] +#[derive(Debug, Default, Clone)] pub struct ChainSpecBuilder { chain: Option, genesis: Option, @@ -1493,6 +1501,61 @@ Post-merge hard forks (timestamp based): ); } + /// Constructs a [ChainSpec] with the given [ChainSpecBuilder], shanghai, and cancun fork + /// timestamps. + fn construct_chainspec( + builder: ChainSpecBuilder, + shanghai_time: u64, + cancun_time: u64, + ) -> ChainSpec { + builder + .with_fork(Hardfork::Shanghai, ForkCondition::Timestamp(shanghai_time)) + .with_fork(Hardfork::Cancun, ForkCondition::Timestamp(cancun_time)) + .build() + } + + /// Tests that time-based forks which are active at genesis are not included in forkid hash. + /// + /// This is based off of the test vectors here: + /// + #[test] + fn test_timestamp_fork_in_genesis() { + let timestamp = 1690475657u64; + let default_spec_builder = ChainSpecBuilder::default() + .chain(Chain::Id(1337)) + .genesis(Genesis::default().with_timestamp(timestamp)) + .paris_activated(); + + // test format: (chain spec, expected next value) - the forkhash will be determined by the + // genesis hash of the constructed chainspec + let tests = [ + ( + construct_chainspec(default_spec_builder.clone(), timestamp - 1, timestamp + 1), + timestamp + 1, + ), + ( + construct_chainspec(default_spec_builder.clone(), timestamp, timestamp + 1), + timestamp + 1, + ), + ( + construct_chainspec(default_spec_builder.clone(), timestamp + 1, timestamp + 2), + timestamp + 1, + ), + ]; + + for (spec, expected_timestamp) in tests { + let got_forkid = spec.fork_id(&Head { number: 0, timestamp: 0, ..Default::default() }); + // This is slightly different from the geth test because we use the shanghai timestamp + // to determine whether or not to include a withdrawals root in the genesis header. + // This makes the genesis hash different, and as a result makes the ChainSpec fork hash + // different. + let genesis_hash = spec.genesis_hash(); + let expected_forkid = + ForkId { hash: ForkHash::from(genesis_hash), next: expected_timestamp }; + assert_eq!(got_forkid, expected_forkid); + } + } + /// Checks that the fork is not active at a terminal ttd block. #[test] fn check_terminal_ttd() { diff --git a/crates/primitives/src/forkid.rs b/crates/primitives/src/forkid.rs index a737e8430f..5132a084e9 100644 --- a/crates/primitives/src/forkid.rs +++ b/crates/primitives/src/forkid.rs @@ -165,6 +165,7 @@ pub struct ForkFilter { /// [eip-6122]: https://eips.ethereum.org/EIPS/eip-6122 forks: BTreeMap, + /// The current head, used to select forks that are active locally. head: Head, cache: Cache, @@ -173,17 +174,22 @@ pub struct ForkFilter { impl ForkFilter { /// Create the filter from provided head, genesis block hash, past forks and expected future /// forks. - pub fn new(head: Head, genesis: H256, forks: F) -> Self + pub fn new(head: Head, genesis_hash: H256, genesis_timestamp: u64, forks: F) -> Self where F: IntoIterator, { - let genesis_fork_hash = ForkHash::from(genesis); + let genesis_fork_hash = ForkHash::from(genesis_hash); let mut forks = forks.into_iter().collect::>(); forks.remove(&ForkFilterKey::Time(0)); forks.remove(&ForkFilterKey::Block(0)); let forks = forks .into_iter() + // filter out forks that are pre-genesis by timestamp + .filter(|key| match key { + ForkFilterKey::Block(_) => true, + ForkFilterKey::Time(time) => *time > genesis_timestamp, + }) .fold( (BTreeMap::from([(ForkFilterKey::Block(0), genesis_fork_hash)]), genesis_fork_hash), |(mut acc, base_hash), key| { @@ -395,6 +401,7 @@ mod tests { let mut filter = ForkFilter::new( Head { number: 0, ..Default::default() }, GENESIS_HASH, + 0, vec![ ForkFilterKey::Block(1_150_000), ForkFilterKey::Block(1_920_000), @@ -568,6 +575,7 @@ mod tests { let mut fork_filter = ForkFilter::new( Head { number: 0, ..Default::default() }, GENESIS_HASH, + 0, vec![ForkFilterKey::Block(b1), ForkFilterKey::Block(b2)], );