Skip to content

Commit 1f3247b

Browse files
prestwichclaude
andcommitted
fix: address PR #100 review feedback
Add MissingBlock error variant for correct semantics, tighten RpcBlock field visibility, guard backfill_batch_size against zero, and document RPC reorg detection limitations and ChainReverted non-emission. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent dabcdde commit 1f3247b

3 files changed

Lines changed: 23 additions & 5 deletions

File tree

crates/host-rpc/src/error.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ pub enum RpcHostError {
1111
#[error("rpc error: {0}")]
1212
Rpc(#[from] RpcError<TransportErrorKind>),
1313

14+
/// The RPC node returned no block for the requested number.
15+
#[error("missing block {0}")]
16+
MissingBlock(u64),
17+
1418
/// Reorg deeper than the block buffer.
1519
#[error("reorg depth {depth} exceeds buffer capacity {capacity}")]
1620
ReorgTooDeep {

crates/host-rpc/src/notifier.rs

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ where
9191
.get_block_by_number(number.into())
9292
.full()
9393
.await?
94-
.ok_or(RpcHostError::SubscriptionClosed)?;
94+
.ok_or(RpcHostError::MissingBlock(number))?;
9595

9696
let rpc_receipts =
9797
self.provider.get_block_receipts(number.into()).await?.unwrap_or_default();
@@ -183,6 +183,11 @@ where
183183
///
184184
/// Returns the block number where the chain diverged (the first block
185185
/// that differs), or `None` if the fork is deeper than the buffer.
186+
///
187+
/// **Limitation:** This queries the RPC node for blocks on the new chain.
188+
/// If the node hasn't fully switched to the new chain, it may return
189+
/// stale blocks from the old chain, producing an incorrect fork point.
190+
/// This is an inherent limitation of RPC-based reorg detection.
186191
async fn find_fork_point(&self, new_header: &RpcHeader) -> Result<Option<u64>, RpcHostError> {
187192
// Walk the new chain backward from the new header's parent.
188193
let mut check_hash = new_header.parent_hash();
@@ -205,7 +210,7 @@ where
205210
.provider
206211
.get_block_by_number(check_number.into())
207212
.await?
208-
.ok_or(RpcHostError::SubscriptionClosed)?;
213+
.ok_or(RpcHostError::MissingBlock(check_number))?;
209214
check_hash = parent.header().parent_hash();
210215
check_number = check_number.saturating_sub(1);
211216
}
@@ -358,6 +363,15 @@ where
358363
}
359364
}
360365

366+
/// [`HostNotifier`] implementation for [`RpcHostNotifier`].
367+
///
368+
/// Note: this implementation never emits
369+
/// [`HostNotificationKind::ChainReverted`]. The `newHeads` WebSocket
370+
/// subscription only fires when a new block appears — a pure revert
371+
/// (blocks removed without a replacement chain) produces no new header and
372+
/// is therefore invisible to the subscription. Only
373+
/// [`HostNotificationKind::ChainCommitted`] and
374+
/// [`HostNotificationKind::ChainReorged`] are produced.
361375
impl<P> HostNotifier for RpcHostNotifier<P>
362376
where
363377
P: Provider + Clone + Send + Sync + 'static,
@@ -384,7 +398,7 @@ where
384398

385399
fn set_backfill_thresholds(&mut self, max_blocks: Option<u64>) {
386400
if let Some(max) = max_blocks {
387-
self.backfill_batch_size = max;
401+
self.backfill_batch_size = max.max(1);
388402
}
389403
}
390404

crates/host-rpc/src/segment.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@ use std::sync::Arc;
77
#[derive(Debug)]
88
pub struct RpcBlock {
99
/// The recovered block (with senders).
10-
pub block: RecoveredBlock,
10+
pub(crate) block: RecoveredBlock,
1111
/// The receipts for this block's transactions.
12-
pub receipts: Vec<ReceiptEnvelope>,
12+
pub(crate) receipts: Vec<ReceiptEnvelope>,
1313
}
1414

1515
impl RpcBlock {

0 commit comments

Comments
 (0)