Skip to content

Commit bfe331a

Browse files
committed
chain/ethereum, graph: Fix stale revert target in polling block stream
Change `TriggersAdapter::is_on_main_chain` to return `Option<BlockPtr>` instead of `bool`. `None` means the block is on the main chain; `Some(canonical_parent)` provides the revert target directly from the canonical chain. Previously, when a subgraph far behind chain head sat on a stale block from an old reorg, the revert target was derived from the stale block's cached parent_hash — a hash from the pre-reorg chain that may no longer exist. This caused permanent revert failures and an infinite retry loop. Now the Firehose path returns the canonical block's parent_ptr() and the RPC path uses next_existing_ptr_to_number(N-1), both sourced from the canonical chain.
1 parent 9e65f6f commit bfe331a

8 files changed

Lines changed: 37 additions & 46 deletions

File tree

chain/ethereum/src/chain.rs

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1023,7 +1023,7 @@ impl TriggersAdapterTrait<Chain> for TriggersAdapter {
10231023
}
10241024
}
10251025

1026-
async fn is_on_main_chain(&self, ptr: BlockPtr) -> Result<bool, Error> {
1026+
async fn is_on_main_chain(&self, ptr: BlockPtr) -> Result<Option<BlockPtr>, Error> {
10271027
match &*self.chain_client {
10281028
ChainClient::Firehose(endpoints) => {
10291029
let endpoint = endpoints.endpoint().await?;
@@ -1034,15 +1034,34 @@ impl TriggersAdapterTrait<Chain> for TriggersAdapter {
10341034
"Failed to fetch block {} from firehose",
10351035
ptr.number
10361036
))?;
1037-
Ok(block.hash() == ptr.hash)
1037+
if block.hash() == ptr.hash {
1038+
Ok(None)
1039+
} else {
1040+
Ok(Some(block.parent_ptr().ok_or_else(|| {
1041+
anyhow!(
1042+
"canonical block at {} has no parent; cannot determine revert target",
1043+
ptr.number
1044+
)
1045+
})?))
1046+
}
10381047
}
10391048
ChainClient::Rpc(adapter) => {
10401049
let adapter = adapter
10411050
.cheapest()
10421051
.await
10431052
.ok_or_else(|| anyhow!("unable to get adapter for is_on_main_chain"))?;
10441053

1045-
adapter.is_on_main_chain(&self.logger, ptr).await
1054+
let canonical = adapter
1055+
.next_existing_ptr_to_number(&self.logger, ptr.number)
1056+
.await?;
1057+
if canonical == ptr {
1058+
Ok(None)
1059+
} else {
1060+
let parent = adapter
1061+
.next_existing_ptr_to_number(&self.logger, ptr.number - 1)
1062+
.await?;
1063+
Ok(Some(parent))
1064+
}
10461065
}
10471066
}
10481067
}

chain/ethereum/src/ethereum_adapter.rs

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -884,29 +884,6 @@ impl EthereumAdapter {
884884
.map(|b| BlockPtr::from((b.header.hash, b.header.number)))
885885
}
886886

887-
/// Check if `block_ptr` refers to a block that is on the main chain, according to the Ethereum
888-
/// node.
889-
///
890-
/// Careful: don't use this function without considering race conditions.
891-
/// Chain reorgs could happen at any time, and could affect the answer received.
892-
/// Generally, it is only safe to use this function with blocks that have received enough
893-
/// confirmations to guarantee no further reorgs, **and** where the Ethereum node is aware of
894-
/// those confirmations.
895-
/// If the Ethereum node is far behind in processing blocks, even old blocks can be subject to
896-
/// reorgs.
897-
pub(crate) async fn is_on_main_chain(
898-
&self,
899-
logger: &Logger,
900-
block_ptr: BlockPtr,
901-
) -> Result<bool, Error> {
902-
// TODO: This considers null blocks, but we could instead bail if we encounter one as a
903-
// small optimization.
904-
let canonical_block = self
905-
.next_existing_ptr_to_number(logger, block_ptr.number)
906-
.await?;
907-
Ok(canonical_block == block_ptr)
908-
}
909-
910887
pub(crate) fn logs_in_block_range(
911888
&self,
912889
logger: &Logger,

chain/ethereum/src/polling_block_stream.rs

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -272,20 +272,14 @@ impl PollingBlockStreamContext {
272272
// This allows us to ask the node: does subgraph_ptr point to a block that was
273273
// permanently accepted into the main chain, or does it point to a block that was
274274
// uncled?
275-
let is_on_main_chain = match &subgraph_ptr {
275+
let canonical_parent = match &subgraph_ptr {
276276
Some(ptr) => ctx.adapter.is_on_main_chain(ptr.clone()).await?,
277-
None => true,
277+
None => None,
278278
};
279-
if !is_on_main_chain {
279+
if let Some(canonical_parent) = canonical_parent {
280280
// The subgraph ptr points to a block that was uncled.
281-
// We need to revert this block.
282-
//
283-
// Note: We can safely unwrap the subgraph ptr here, because
284-
// if it was `None`, `is_on_main_chain` would be true.
285-
let from = subgraph_ptr.unwrap();
286-
let parent = self.parent_ptr(&from, "is_on_main_chain").await?;
287-
288-
return Ok(ReconciliationStep::Revert(parent));
281+
// Revert to the canonical parent provided by is_on_main_chain.
282+
return Ok(ReconciliationStep::Revert(canonical_parent));
289283
}
290284

291285
// The subgraph ptr points to a block on the main chain.

chain/near/src/chain.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,7 @@ impl TriggersAdapterTrait<Chain> for TriggersAdapter {
326326
Ok(BlockWithTriggers::new(block, trigger_data, logger))
327327
}
328328

329-
async fn is_on_main_chain(&self, _ptr: BlockPtr) -> Result<bool, Error> {
329+
async fn is_on_main_chain(&self, _ptr: BlockPtr) -> Result<Option<BlockPtr>, Error> {
330330
panic!("Should never be called since not used by FirehoseBlockStream")
331331
}
332332

gnd/src/commands/test/noop.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,8 @@ impl<C: Blockchain> TriggersAdapter<C> for NoopTriggersAdapter<C> {
105105
Ok(BlockWithTriggers::new(block, Vec::new(), &logger))
106106
}
107107

108-
async fn is_on_main_chain(&self, _ptr: BlockPtr) -> Result<bool, Error> {
109-
Ok(true)
108+
async fn is_on_main_chain(&self, _ptr: BlockPtr) -> Result<Option<BlockPtr>, Error> {
109+
Ok(None)
110110
}
111111

112112
async fn parent_ptr(&self, block: &BlockPtr) -> Result<Option<BlockPtr>, Error> {

graph/src/blockchain/block_stream.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -550,7 +550,7 @@ impl<C: Blockchain> TriggersAdapterWrapper<C> {
550550
.await
551551
}
552552

553-
pub async fn is_on_main_chain(&self, ptr: BlockPtr) -> Result<bool, Error> {
553+
pub async fn is_on_main_chain(&self, ptr: BlockPtr) -> Result<Option<BlockPtr>, Error> {
554554
self.adapter.is_on_main_chain(ptr).await
555555
}
556556

@@ -610,9 +610,10 @@ pub trait TriggersAdapter<C: Blockchain>: Send + Sync {
610610
filter: &C::TriggerFilter,
611611
) -> Result<BlockWithTriggers<C>, Error>;
612612

613-
/// Return `true` if the block with the given hash and number is on the
614-
/// main chain, i.e., the chain going back from the current chain head.
615-
async fn is_on_main_chain(&self, ptr: BlockPtr) -> Result<bool, Error>;
613+
/// Check whether the block is on the main chain. Returns `None` if it
614+
/// is, or `Some(revert_to)` with the canonical parent pointer to revert
615+
/// to if the block has been reorged out.
616+
async fn is_on_main_chain(&self, ptr: BlockPtr) -> Result<Option<BlockPtr>, Error>;
616617

617618
/// Get pointer to parent of `block`. This is called when reverting `block`.
618619
async fn parent_ptr(&self, block: &BlockPtr) -> Result<Option<BlockPtr>, Error>;

graph/src/blockchain/mock.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ impl TriggersAdapter<MockBlockchain> for MockTriggersAdapter {
307307
todo!()
308308
}
309309

310-
async fn is_on_main_chain(&self, _ptr: BlockPtr) -> Result<bool, Error> {
310+
async fn is_on_main_chain(&self, _ptr: BlockPtr) -> Result<Option<BlockPtr>, Error> {
311311
todo!()
312312
}
313313

tests/src/fixture/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1080,7 +1080,7 @@ impl<C: Blockchain> TriggersAdapter<C> for MockTriggersAdapter<C> {
10801080
(self.triggers_in_block)(block)
10811081
}
10821082

1083-
async fn is_on_main_chain(&self, _ptr: BlockPtr) -> Result<bool, Error> {
1083+
async fn is_on_main_chain(&self, _ptr: BlockPtr) -> Result<Option<BlockPtr>, Error> {
10841084
todo!()
10851085
}
10861086

0 commit comments

Comments
 (0)