Skip to content

Commit 3f9fe44

Browse files
committed
Fix inconsistent Eth block visibility caused by missing/stale block-n… (polkadot-evm#1818)
* Fix inconsistent Eth block visibility caused by missing/stale block-number mappings in KV backend, while keeping recovery fast on long-running chains. Fix inconsistent Ethereum block visibility caused by missing/stale block-number mappings in KV backend, while keeping recovery fast on long-running chains. * rustfmt * repair number mappings and stabilize latest-indexed recovery * Fix KV mapping sync to write block-number mappings only for canonical blocks and auto-repair stale entries * rustfmt * clippy lints * Guard is_new_best remap logic with sync-time canonical check * Harden canonical mapping sync against stale import-time best and stabilize latest-index pointer * improve sync and add tests * Fix pointer initialization & adjust reorg test scenario * Change repair cursor behavior so blocks that cannot currently be repaired are retried (or persist unresolved ranges), instead of always advancing past them * clippy lint * prettier * fix test gas * ts-tests: wait for chain head block in `createAndFinalizeBlock` to reduce CI flakiness * ts-tests: harden pov-size and newHeads tests against CI timing races * improve tests
1 parent beba745 commit 3f9fe44

File tree

13 files changed

+1233
-206
lines changed

13 files changed

+1233
-206
lines changed

client/api/src/backend.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,17 @@ pub trait Backend<Block: BlockT>: Send + Sync {
4242
/// Get the ethereum block hash for a given block number.
4343
async fn block_hash_by_number(&self, block_number: u64) -> Result<Option<H256>, String>;
4444

45+
/// Persist or repair the ethereum block hash for a given block number.
46+
///
47+
/// Backends that cannot mutate mappings can rely on the default no-op.
48+
async fn set_block_hash_by_number(
49+
&self,
50+
_block_number: u64,
51+
_ethereum_block_hash: H256,
52+
) -> Result<(), String> {
53+
Ok(())
54+
}
55+
4556
/// Get the transaction metadata with the given ethereum block hash.
4657
async fn transaction_metadata(
4758
&self,

client/cli/src/frontier_db_cmd/mapping_db.rs

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -114,9 +114,11 @@ where
114114
.number())
115115
.unique_saturated_into();
116116

117-
self.backend
118-
.mapping()
119-
.write_hashes(commitment, block_number)?;
117+
self.backend.mapping().write_hashes(
118+
commitment,
119+
block_number,
120+
fc_db::kv::NumberMappingWrite::Write,
121+
)?;
120122
} else {
121123
return Err(self.key_not_empty_error(key));
122124
}
@@ -185,9 +187,11 @@ where
185187
.number())
186188
.unique_saturated_into();
187189

188-
self.backend
189-
.mapping()
190-
.write_hashes(commitment, block_number)?;
190+
self.backend.mapping().write_hashes(
191+
commitment,
192+
block_number,
193+
fc_db::kv::NumberMappingWrite::Write,
194+
)?;
191195
}
192196
}
193197
_ => return Err(self.key_value_error(key, value)),

client/db/src/kv/mod.rs

Lines changed: 88 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,9 @@ use fp_storage::{EthereumStorageSchema, PALLET_ETHEREUM_SCHEMA_CACHE};
4141
const DB_HASH_LEN: usize = 32;
4242
/// Hash type that this backend uses for the database.
4343
pub type DbHash = [u8; DB_HASH_LEN];
44-
45-
/// Maximum number of blocks to walk back when searching for an indexed canonical block.
46-
/// This limits the search depth when the cached `LATEST_CANONICAL_INDEXED_BLOCK` is stale
47-
/// (e.g., after a reorg or if it points to an unindexed block).
48-
const MAX_WALKBACK_DEPTH: u64 = 16;
44+
/// Maximum number of blocks inspected in a single recovery pass when the
45+
/// latest indexed canonical pointer is stale or missing.
46+
const INDEXED_RECOVERY_SCAN_LIMIT: u64 = 8192;
4947

5048
/// Database settings.
5149
pub struct DatabaseSettings {
@@ -66,6 +64,7 @@ pub(crate) mod columns {
6664
pub mod static_keys {
6765
pub const CURRENT_SYNCING_TIPS: &[u8] = b"CURRENT_SYNCING_TIPS";
6866
pub const LATEST_CANONICAL_INDEXED_BLOCK: &[u8] = b"LATEST_CANONICAL_INDEXED_BLOCK";
67+
pub const CANONICAL_NUMBER_REPAIR_CURSOR: &[u8] = b"CANONICAL_NUMBER_REPAIR_CURSOR";
6968
}
7069

7170
#[derive(Clone)]
@@ -89,6 +88,15 @@ impl<Block: BlockT, C: HeaderBackend<Block>> fc_api::Backend<Block> for Backend<
8988
self.mapping().block_hash_by_number(block_number)
9089
}
9190

91+
async fn set_block_hash_by_number(
92+
&self,
93+
block_number: u64,
94+
ethereum_block_hash: H256,
95+
) -> Result<(), String> {
96+
self.mapping()
97+
.set_block_hash_by_number(block_number, ethereum_block_hash)
98+
}
99+
92100
async fn transaction_metadata(
93101
&self,
94102
ethereum_transaction_hash: &H256,
@@ -115,28 +123,24 @@ impl<Block: BlockT, C: HeaderBackend<Block>> fc_api::Backend<Block> for Backend<
115123
// Users can check sync status via eth_syncing to determine if the node is
116124
// still catching up.
117125
let best_number: u64 = self.client.info().best_number.unique_saturated_into();
118-
let (block_number, should_persist_on_hit) =
119-
match self.mapping.latest_canonical_indexed_block_number()? {
120-
Some(cached) => {
121-
let clamped = cached.min(best_number);
122-
(clamped, clamped != cached)
123-
}
124-
None => (best_number, true),
125-
};
126126

127-
if let Some(canonical_hash) = self.indexed_canonical_hash_at(block_number)? {
128-
if should_persist_on_hit {
129-
self.mapping
130-
.set_latest_canonical_indexed_block(block_number)?;
131-
}
127+
// Fast path: if best is already indexed and canonical, use it directly.
128+
if let Some(canonical_hash) = self.indexed_canonical_hash_at(best_number)? {
129+
self.mapping
130+
.set_latest_canonical_indexed_block(best_number)?;
132131
return Ok(canonical_hash);
133132
}
134133

135-
// Cached canonical block is stale (reorg happened), or meta key was absent
136-
// and best block is not indexed yet. Walk back to the latest indexed
137-
// canonical block and persist the recovered pointer.
134+
// Use cached pointer only as a lower bound hint for recovery scan.
135+
let min_block_hint = self
136+
.mapping
137+
.latest_canonical_indexed_block_number()?
138+
.map(|cached| cached.min(best_number));
139+
140+
// Best block is not indexed yet or mapping is stale (reorg). Walk back to
141+
// the latest indexed canonical block and persist the recovered pointer.
138142
if let Some((recovered_number, recovered_hash)) =
139-
self.find_latest_indexed_canonical_block(block_number.saturating_sub(1))?
143+
self.find_latest_indexed_canonical_block(best_number.saturating_sub(1), min_block_hint)?
140144
{
141145
self.mapping
142146
.set_latest_canonical_indexed_block(recovered_number)?;
@@ -257,13 +261,18 @@ impl<Block: BlockT, C: HeaderBackend<Block>> Backend<Block, C> {
257261
}
258262

259263
/// Finds the latest indexed block that is on the canonical chain by walking
260-
/// backwards from `start_block`. Returns `None` if no indexed canonical block
261-
/// is found within `MAX_WALKBACK_DEPTH` blocks.
264+
/// backwards from `start_block`, bounded to `INDEXED_RECOVERY_SCAN_LIMIT`
265+
/// probes to keep lookups fast on long chains.
262266
fn find_latest_indexed_canonical_block(
263267
&self,
264268
start_block: u64,
269+
min_block_hint: Option<u64>,
265270
) -> Result<Option<(u64, Block::Hash)>, String> {
266-
let min_block = start_block.saturating_sub(MAX_WALKBACK_DEPTH);
271+
let scan_limit = INDEXED_RECOVERY_SCAN_LIMIT.saturating_sub(1);
272+
let min_block = start_block
273+
.saturating_sub(scan_limit)
274+
.max(min_block_hint.unwrap_or(0))
275+
.min(start_block);
267276
for block_number in (min_block..=start_block).rev() {
268277
if let Some(canonical_hash) = self.indexed_canonical_hash_at(block_number)? {
269278
return Ok(Some((block_number, canonical_hash)));
@@ -341,6 +350,12 @@ pub struct MappingCommitment<Block: BlockT> {
341350
pub ethereum_transaction_hashes: Vec<H256>,
342351
}
343352

353+
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
354+
pub enum NumberMappingWrite {
355+
Write,
356+
Skip,
357+
}
358+
344359
pub struct MappingDb<Block> {
345360
db: Arc<dyn Database<DbHash>>,
346361
write_lock: Arc<Mutex<()>>,
@@ -404,6 +419,7 @@ impl<Block: BlockT> MappingDb<Block> {
404419
&self,
405420
commitment: MappingCommitment<Block>,
406421
block_number: u64,
422+
number_mapping_write: NumberMappingWrite,
407423
) -> Result<(), String> {
408424
let _lock = self.write_lock.lock();
409425

@@ -431,12 +447,13 @@ impl<Block: BlockT> MappingDb<Block> {
431447
&substrate_hashes.encode(),
432448
);
433449

434-
// Write block number -> ethereum block hash mapping
435-
transaction.set(
436-
columns::BLOCK_NUMBER_MAPPING,
437-
&block_number.encode(),
438-
&commitment.ethereum_block_hash.encode(),
439-
);
450+
if number_mapping_write == NumberMappingWrite::Write {
451+
transaction.set(
452+
columns::BLOCK_NUMBER_MAPPING,
453+
&block_number.encode(),
454+
&commitment.ethereum_block_hash.encode(),
455+
);
456+
}
440457

441458
for (i, ethereum_transaction_hash) in commitment
442459
.ethereum_transaction_hashes
@@ -479,6 +496,22 @@ impl<Block: BlockT> MappingDb<Block> {
479496
}
480497
}
481498

499+
pub fn set_block_hash_by_number(
500+
&self,
501+
block_number: u64,
502+
ethereum_block_hash: H256,
503+
) -> Result<(), String> {
504+
let _lock = self.write_lock.lock();
505+
506+
let mut transaction = sp_database::Transaction::new();
507+
transaction.set(
508+
columns::BLOCK_NUMBER_MAPPING,
509+
&block_number.encode(),
510+
&ethereum_block_hash.encode(),
511+
);
512+
self.db.commit(transaction).map_err(|e| e.to_string())
513+
}
514+
482515
/// Returns the latest canonical indexed block number, or None if not set.
483516
pub fn latest_canonical_indexed_block_number(&self) -> Result<Option<u64>, String> {
484517
match self
@@ -502,4 +535,28 @@ impl<Block: BlockT> MappingDb<Block> {
502535
);
503536
self.db.commit(transaction).map_err(|e| e.to_string())
504537
}
538+
539+
/// Returns the canonical number-repair cursor, or None if not set.
540+
pub fn canonical_number_repair_cursor(&self) -> Result<Option<u64>, String> {
541+
match self
542+
.db
543+
.get(columns::META, static_keys::CANONICAL_NUMBER_REPAIR_CURSOR)
544+
{
545+
Some(raw) => Ok(Some(
546+
u64::decode(&mut &raw[..]).map_err(|e| format!("{e:?}"))?,
547+
)),
548+
None => Ok(None),
549+
}
550+
}
551+
552+
/// Sets the canonical number-repair cursor.
553+
pub fn set_canonical_number_repair_cursor(&self, block_number: u64) -> Result<(), String> {
554+
let mut transaction = sp_database::Transaction::new();
555+
transaction.set(
556+
columns::META,
557+
static_keys::CANONICAL_NUMBER_REPAIR_CURSOR,
558+
&block_number.encode(),
559+
);
560+
self.db.commit(transaction).map_err(|e| e.to_string())
561+
}
505562
}

client/db/src/sql/mod.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -794,6 +794,14 @@ impl<Block: BlockT<Hash = H256>> fc_api::Backend<Block> for Backend<Block> {
794794
.map_err(|e| format!("Failed to fetch block hash by number: {e}"))
795795
}
796796

797+
async fn set_block_hash_by_number(
798+
&self,
799+
_block_number: u64,
800+
_ethereum_block_hash: H256,
801+
) -> Result<(), String> {
802+
Ok(())
803+
}
804+
797805
async fn transaction_metadata(
798806
&self,
799807
ethereum_transaction_hash: &H256,

0 commit comments

Comments
 (0)