Skip to content

Commit 974c6d5

Browse files
MarcosNicolauJereSalo
authored andcommitted
fix(l1): hive engine-cancun suite ForkId tests (#1976)
**Motivation** Fix failing tests in the engine-cancun hive suite. **Description** This update fixes all tests matching the pattern `ForkId: *` in the `engine-cancun` suite. The issue resulted from not filtering out duplicate fork values and failing to exclude forks that occurred before the genesis block. Advances on #1285
1 parent 5d15606 commit 974c6d5

File tree

4 files changed

+54
-26
lines changed

4 files changed

+54
-26
lines changed

.github/workflows/ci_l1.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ jobs:
168168
test_pattern: engine-(auth|exchange-capabilities)/
169169
- name: "Cancun Engine tests"
170170
simulation: ethereum/engine
171-
test_pattern: "engine-cancun/Blob Transactions On Block 1|Blob Transaction Ordering, Single|Blob Transaction Ordering, Multiple Accounts|Replace Blob Transactions|Parallel Blob Transactions|ForkchoiceUpdatedV3|ForkchoiceUpdatedV2|ForkchoiceUpdated Version|GetPayload|NewPayloadV3 After Cancun|NewPayloadV3 Before Cancun|NewPayloadV3 Versioned Hashes|Incorrect BlobGasUsed|ParentHash equals BlockHash|RPC:|in ForkchoiceState|Unknown SafeBlockHash|Unknown FinalizedBlockHash|Unique|Re-Execute Payload|Multiple New Payloads|NewPayload with|Build Payload with|Re-org to Previously|Safe Re-Org to Side Chain|Transaction Re-Org|Re-Org Back into Canonical Chain, Depth=5|Suggested Fee Recipient Test|PrevRandao Opcode|Fork ID: Genesis=0|Fork ID: Genesis=1, Cancun=0|Fork ID: Genesis=1, Cancun=2 |Fork ID: Genesis=1, Cancun=2, BlocksBeforePeering=1|Fork ID: Genesis=1, Cancun=2, Shanghai=[^1]|Pre-Merge"
171+
test_pattern: "engine-cancun/Blob Transactions On Block 1|Blob Transaction Ordering, Single|Blob Transaction Ordering, Multiple Accounts|Replace Blob Transactions|Parallel Blob Transactions|ForkchoiceUpdatedV3|ForkchoiceUpdatedV2|ForkchoiceUpdated Version|GetPayload|NewPayloadV3 After Cancun|NewPayloadV3 Before Cancun|NewPayloadV3 Versioned Hashes|Incorrect BlobGasUsed|ParentHash equals BlockHash|RPC:|in ForkchoiceState|Unknown SafeBlockHash|Unknown FinalizedBlockHash|Unique|Re-Execute Payload|Multiple New Payloads|NewPayload with|Build Payload with|Re-org to Previously|Safe Re-Org to Side Chain|Transaction Re-Org|Re-Org Back into Canonical Chain, Depth=5|Suggested Fee Recipient Test|PrevRandao Opcode|Fork ID: *"
172172
- name: "Paris Engine tests"
173173
simulation: ethereum/engine
174174
test_pattern: "engine-api/RPC|Re-org to Previously Validated Sidechain Payload|Re-Org Back into Canonical Chain, Depth=5|Safe Re-Org|Transaction Re-Org|Inconsistent|Suggested Fee|PrevRandao Opcode Transactions|Fork ID|Unknown SafeBlockHash|Unknown FinalizedBlockHash|Unique Payload ID|Re-Execute Payload|Multiple New Payloads|NewPayload with|Payload Build|Build Payload"

crates/common/types/fork_id.rs

+25-15
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use ethrex_rlp::{
99
use ethereum_types::H32;
1010
use tracing::debug;
1111

12-
use super::{BlockHash, BlockNumber, ChainConfig};
12+
use super::{BlockHash, BlockHeader, BlockNumber, ChainConfig};
1313

1414
// See https://github.com/ethereum/go-ethereum/blob/530adfc8e3ef9c8b6356facecdec10b30fb81d7d/core/forkid/forkid.go#L51
1515
const TIMESTAMP_THRESHOLD: u64 = 1438269973;
@@ -23,11 +23,14 @@ pub struct ForkId {
2323
impl ForkId {
2424
pub fn new(
2525
chain_config: ChainConfig,
26-
genesis_hash: BlockHash,
26+
genesis_header: BlockHeader,
2727
head_timestamp: u64,
2828
head_block_number: u64,
2929
) -> Self {
30-
let (block_number_based_forks, timestamp_based_forks) = chain_config.gather_forks();
30+
let genesis_hash = genesis_header.compute_block_hash();
31+
let (block_number_based_forks, timestamp_based_forks) =
32+
chain_config.gather_forks(genesis_header);
33+
3134
let mut fork_next;
3235
let mut hasher = Hasher::new();
3336
// Calculate the starting checksum from the genesis hash
@@ -60,9 +63,11 @@ impl ForkId {
6063
latest_block_number: u64,
6164
head_timestamp: u64,
6265
chain_config: ChainConfig,
63-
genesis_hash: BlockHash,
66+
genesis_header: BlockHeader,
6467
) -> bool {
65-
let (block_number_based_forks, timestamp_based_forks) = chain_config.gather_forks();
68+
let genesis_hash = genesis_header.compute_block_hash();
69+
let (block_number_based_forks, timestamp_based_forks) =
70+
chain_config.gather_forks(genesis_header);
6671

6772
// Determine whether to compare the remote fork_next using a block number or a timestamp.
6873
let head = if head_timestamp >= TIMESTAMP_THRESHOLD {
@@ -220,17 +225,22 @@ mod tests {
220225
fn assert_test_cases(
221226
test_cases: Vec<TestCase>,
222227
chain_config: ChainConfig,
223-
genesis_hash: BlockHash,
228+
genesis_header: BlockHeader,
224229
) {
225230
for test_case in test_cases {
226-
let fork_id = ForkId::new(chain_config, genesis_hash, test_case.time, test_case.head);
231+
let fork_id = ForkId::new(
232+
chain_config,
233+
genesis_header.clone(),
234+
test_case.time,
235+
test_case.head,
236+
);
227237
assert_eq!(
228238
fork_id.is_valid(
229239
test_case.fork_id,
230240
test_case.head,
231241
test_case.time,
232242
chain_config,
233-
genesis_hash
243+
genesis_header.clone()
234244
),
235245
test_case.is_valid
236246
)
@@ -244,7 +254,7 @@ mod tests {
244254
let genesis_reader = BufReader::new(genesis_file);
245255
let genesis: Genesis =
246256
serde_json::from_reader(genesis_reader).expect("Failed to read genesis file");
247-
let genesis_hash = genesis.get_block().hash();
257+
let genesis_header = genesis.get_block().header;
248258

249259
// See https://github.com/ethereum/go-ethereum/blob/4d94bd83b20ce430e435f3107f29632c627cfb26/core/forkid/forkid_test.go#L98
250260
let test_cases: Vec<TestCase> = vec![
@@ -303,17 +313,17 @@ mod tests {
303313
is_valid: true,
304314
},
305315
];
306-
assert_test_cases(test_cases, genesis.config, genesis_hash);
316+
assert_test_cases(test_cases, genesis.config, genesis_header);
307317
}
308318

309-
fn get_sepolia_genesis() -> (Genesis, BlockHash) {
319+
fn get_sepolia_genesis() -> (Genesis, BlockHeader) {
310320
let genesis_file = std::fs::File::open("../../cmd/ethrex/networks/sepolia/genesis.json")
311321
.expect("Failed to open genesis file");
312322
let genesis_reader = BufReader::new(genesis_file);
313323
let genesis: Genesis =
314324
serde_json::from_reader(genesis_reader).expect("Failed to read genesis file");
315-
let genesis_hash = genesis.get_block().hash();
316-
(genesis, genesis_hash)
325+
let genesis_header = genesis.get_block().header;
326+
(genesis, genesis_header)
317327
}
318328
#[test]
319329
fn sepolia_test_cases() {
@@ -417,14 +427,14 @@ mod tests {
417427
local_head_block_number,
418428
0,
419429
ChainConfig::default(),
420-
BlockHash::random(),
430+
BlockHeader::default(),
421431
);
422432
let result_b = local_b.is_valid(
423433
remote,
424434
local_head_block_number,
425435
0,
426436
ChainConfig::default(),
427-
BlockHash::random(),
437+
BlockHeader::default(),
428438
);
429439
assert!(!result_a);
430440
assert!(!result_b);

crates/common/types/genesis.rs

+15-3
Original file line numberDiff line numberDiff line change
@@ -235,8 +235,8 @@ impl ChainConfig {
235235
self.get_fork(block_timestamp)
236236
}
237237

238-
pub fn gather_forks(&self) -> (Vec<u64>, Vec<u64>) {
239-
let block_number_based_forks: Vec<u64> = vec![
238+
pub fn gather_forks(&self, genesis_header: BlockHeader) -> (Vec<u64>, Vec<u64>) {
239+
let mut block_number_based_forks: Vec<u64> = vec![
240240
self.homestead_block,
241241
if self.dao_fork_support {
242242
self.dao_fork_block
@@ -261,7 +261,11 @@ impl ChainConfig {
261261
.flatten()
262262
.collect();
263263

264-
let timestamp_based_forks: Vec<u64> = vec![
264+
// Remove repeated values
265+
block_number_based_forks.sort();
266+
block_number_based_forks.dedup();
267+
268+
let mut timestamp_based_forks: Vec<u64> = vec![
265269
self.shanghai_time,
266270
self.cancun_time,
267271
self.prague_time,
@@ -271,6 +275,14 @@ impl ChainConfig {
271275
.flatten()
272276
.collect();
273277

278+
// Remove repeated values
279+
timestamp_based_forks.sort();
280+
timestamp_based_forks.dedup();
281+
282+
// Filter forks before genesis
283+
block_number_based_forks.retain(|block_number| *block_number != 0);
284+
timestamp_based_forks.retain(|block_timestamp| *block_timestamp > genesis_header.timestamp);
285+
274286
(block_number_based_forks, timestamp_based_forks)
275287
}
276288
}

crates/networking/p2p/rlpx/eth/backend.rs

+13-7
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,12 @@ pub fn get_status(storage: &Store, eth_version: u32) -> Result<StatusMessage, RL
2121

2222
let genesis = genesis_header.compute_block_hash();
2323
let block_hash = block_header.compute_block_hash();
24-
let fork_id = ForkId::new(chain_config, genesis, block_header.timestamp, block_number);
24+
let fork_id = ForkId::new(
25+
chain_config,
26+
genesis_header,
27+
block_header.timestamp,
28+
block_number,
29+
);
2530
Ok(StatusMessage {
2631
eth_version,
2732
network_id,
@@ -43,14 +48,14 @@ pub fn validate_status(
4348
let genesis_header = storage
4449
.get_block_header(0)?
4550
.ok_or(RLPxError::NotFound("Genesis Block".to_string()))?;
51+
let genesis_hash = genesis_header.compute_block_hash();
4652
let latest_block_number = storage.get_latest_block_number()?;
4753
let latest_block_header = storage
4854
.get_block_header(latest_block_number)?
4955
.ok_or(RLPxError::NotFound(format!("Block {latest_block_number}")))?;
50-
let genesis = genesis_header.compute_block_hash();
5156
let fork_id = ForkId::new(
5257
chain_config,
53-
genesis,
58+
genesis_header.clone(),
5459
latest_block_header.timestamp,
5560
latest_block_number,
5661
);
@@ -68,7 +73,7 @@ pub fn validate_status(
6873
));
6974
}
7075
//Check Genesis
71-
if msg_data.genesis != genesis {
76+
if msg_data.genesis != genesis_hash {
7277
return Err(RLPxError::HandshakeError(
7378
"Genesis does not match".to_string(),
7479
));
@@ -79,7 +84,7 @@ pub fn validate_status(
7984
latest_block_number,
8085
latest_block_header.timestamp,
8186
chain_config,
82-
genesis,
87+
genesis_header,
8388
) {
8489
return Err(RLPxError::HandshakeError("Invalid Fork Id".to_string()));
8590
}
@@ -115,8 +120,9 @@ mod tests {
115120
.expect("Failed to add genesis block to DB");
116121
let config = genesis.config;
117122
let total_difficulty = U256::from(config.terminal_total_difficulty.unwrap_or_default());
118-
let genesis_hash = genesis.get_block().hash();
119-
let fork_id = ForkId::new(config, genesis_hash, 2707305664, 123);
123+
let genesis_header = genesis.get_block().header;
124+
let genesis_hash = genesis_header.compute_block_hash();
125+
let fork_id = ForkId::new(config, genesis_header, 2707305664, 123);
120126

121127
let eth_version = 68;
122128
let message = StatusMessage {

0 commit comments

Comments
 (0)