Skip to content

Commit b690876

Browse files
committed
fix: address PR concerns
1 parent 371a083 commit b690876

File tree

3 files changed

+25
-23
lines changed

3 files changed

+25
-23
lines changed

bin/portal-bridge/src/bridge/e2hs.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ impl E2HSBridge {
9898
Ok(Self {
9999
gossiper,
100100
block_semaphore,
101-
header_validator: HeaderValidator::default(),
101+
header_validator: HeaderValidator::new_without_historical_summaries(),
102102
block_range,
103103
random_fill,
104104
e2hs_files,

crates/validation/src/header_validator.rs

+21-17
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,14 @@ use crate::{
2020
oracle::HeaderOracle,
2121
};
2222

23-
#[derive(Debug, Default, Clone)]
23+
#[derive(Debug, Clone)]
2424
pub enum HistoricalSummariesProvider {
2525
/// The historical summaries are provided by the header oracle.
2626
HeaderOracle(Arc<RwLock<HeaderOracle>>),
2727
/// The historical summaries are provided by passed historical summaries.
2828
HistoricalSummaries(HistoricalSummaries),
2929
/// We don't have historical summaries, for example the E2HS bridge doesn't use a provider and
3030
/// hence can't get HistoricalSummaries. HistoricalSummary proof validation will fail.
31-
#[default]
3231
None,
3332
}
3433

@@ -64,9 +63,7 @@ impl HistoricalSummariesProvider {
6463

6564
/// HeaderValidator is responsible for validating pre-merge and post-merge headers with their
6665
/// respective proofs.
67-
///
68-
/// Default can only be used for validating pre-capella headers.
69-
#[derive(Debug, Clone, Default)]
66+
#[derive(Debug, Clone)]
7067
pub struct HeaderValidator {
7168
/// Pre-merge accumulator used to validate pre-merge headers.
7269
pub pre_merge_acc: PreMergeAccumulator,
@@ -78,16 +75,21 @@ pub struct HeaderValidator {
7875

7976
impl HeaderValidator {
8077
pub fn new(historical_summaries_provider: HistoricalSummariesProvider) -> Self {
81-
let pre_merge_acc = PreMergeAccumulator::default();
82-
let historical_roots_acc = HistoricalRootsAccumulator::default();
83-
8478
Self {
85-
pre_merge_acc,
86-
historical_roots_acc,
79+
pre_merge_acc: PreMergeAccumulator::default(),
80+
historical_roots_acc: HistoricalRootsAccumulator::default(),
8781
historical_summaries_provider,
8882
}
8983
}
9084

85+
pub fn new_without_historical_summaries() -> Self {
86+
Self {
87+
pre_merge_acc: PreMergeAccumulator::default(),
88+
historical_roots_acc: HistoricalRootsAccumulator::default(),
89+
historical_summaries_provider: HistoricalSummariesProvider::None,
90+
}
91+
}
92+
9193
pub async fn validate_header_with_proof(
9294
&self,
9395
header_with_proof: &HeaderWithProof,
@@ -152,7 +154,7 @@ impl HeaderValidator {
152154
));
153155
}
154156

155-
// Verify the chain of proofs for post-merge/pre-capella block header
157+
// Verify the chain of proofs for post-capella block header
156158
Self::verify_beacon_block_proof(
157159
header_hash,
158160
&proof.execution_block_proof,
@@ -347,7 +349,7 @@ mod test {
347349
header,
348350
proof: BlockHeaderProof::HistoricalHashes(trin_proof),
349351
};
350-
HeaderValidator::default()
352+
HeaderValidator::new_without_historical_summaries()
351353
.validate_header_with_proof(&header_with_proof)
352354
.await
353355
.unwrap();
@@ -372,7 +374,7 @@ mod test {
372374
header,
373375
proof: BlockHeaderProof::HistoricalHashes(proof),
374376
};
375-
HeaderValidator::default()
377+
HeaderValidator::new_without_historical_summaries()
376378
.validate_header_with_proof(&header_with_proof)
377379
.await
378380
.unwrap();
@@ -401,7 +403,7 @@ mod test {
401403
header,
402404
proof: BlockHeaderProof::HistoricalHashes(proof),
403405
};
404-
assert!(HeaderValidator::default()
406+
assert!(HeaderValidator::new_without_historical_summaries()
405407
.validate_header_with_proof(&header_with_proof)
406408
.await
407409
.unwrap_err()
@@ -418,7 +420,7 @@ mod test {
418420
header: future_header,
419421
proof: BlockHeaderProof::HistoricalHashes(Default::default()),
420422
};
421-
HeaderValidator::default()
423+
HeaderValidator::new_without_historical_summaries()
422424
.validate_header_with_proof(&future_hwp)
423425
.await
424426
.unwrap();
@@ -441,7 +443,7 @@ mod test {
441443
let historical_roots_block_proof: BlockProofHistoricalRoots =
442444
serde_yaml::from_value(value).unwrap();
443445

444-
let header_validator = HeaderValidator::default();
446+
let header_validator = HeaderValidator::new_without_historical_summaries();
445447
header_validator
446448
.verify_historical_roots(block_number, header_hash, &historical_roots_block_proof)
447449
.unwrap();
@@ -552,7 +554,9 @@ mod test {
552554
#[tokio::test]
553555
async fn header_oracle_bootstraps_with_default_pre_merge_acc() {
554556
assert_eq!(
555-
HeaderValidator::default().pre_merge_acc.tree_hash_root(),
557+
HeaderValidator::new_without_historical_summaries()
558+
.pre_merge_acc
559+
.tree_hash_root(),
556560
B256::from_str(DEFAULT_PRE_MERGE_ACC_HASH).unwrap(),
557561
);
558562
}

crates/validation/src/oracle.rs

+3-5
Original file line numberDiff line numberDiff line change
@@ -222,18 +222,16 @@ impl HeaderOracle {
222222
tx.send(request)?;
223223

224224
let content = match resp_rx.recv().await {
225-
Some(val) => {
226-
val.map_err(|err| anyhow!("Chain history subnetwork request error: {err:?}"))?
227-
}
228-
None => return Err(anyhow!("No response from chain history subnetwork")),
225+
Some(val) => val.map_err(|err| anyhow!("Beacon subnetwork request error: {err:?}"))?,
226+
None => return Err(anyhow!("No response from Beacon subnetwork")),
229227
};
230228
let content: Bytes = serde_json::from_value(content)?;
231229
let content: BeaconContentValue = BeaconContentValue::decode(&content_key, &content)?;
232230

233231
match content {
234232
BeaconContentValue::HistoricalSummariesWithProof(content) => Ok(content.historical_summaries_with_proof.historical_summaries),
235233
_ => Err(anyhow!(
236-
"Invalid HistoryContentValue received from HeaderWithProof lookup, expected BlockHeaderWithProof: {content:?}"
234+
"Invalid BeaconContentValue received from HistoricalSummaries local_content, expected HistoricalSummariesWithProof: {content:?}"
237235
)),
238236
}
239237
}

0 commit comments

Comments
 (0)