diff --git a/native/src/client.rs b/native/src/client.rs index d62cd407..fbe2d48f 100644 --- a/native/src/client.rs +++ b/native/src/client.rs @@ -19,6 +19,8 @@ pub trait ClientProvider: Send + Sync { hash: H256, key: &StorageKey, ) -> polkadot_sdk::sp_blockchain::Result>; + /// Given a block's number, return the hash of that block. + fn hash(&self, number: u32) -> polkadot_sdk::sp_blockchain::Result>; } #[cfg(feature = "std")] @@ -54,6 +56,15 @@ pub trait Client { .map_err(|error| error.to_string()) }) } + + /// Given a block's number, return the hash of that block. + /// + /// Wraps [`ClientProvider::hash`], which is modeled off of `FullClient::hash`. + /// Returns `None` if the [`ClientExt`] extension is not registered. Otherwise, returns `Some(FullClient::hash(number))`. + fn hash(&mut self, number: u32) -> Option, String>> { + ExternalitiesExt::extension(self) + .map(|ClientExt(provider)| provider.hash(number).map_err(|error| error.to_string())) + } } #[cfg(all(test, feature = "std"))] @@ -83,6 +94,10 @@ mod tests { .clone() .map_err(polkadot_sdk::sp_blockchain::Error::UnknownBlock) } + + fn hash(&self, number: u32) -> polkadot_sdk::sp_blockchain::Result> { + Ok(Some(H256::repeat_byte(number as u8))) + } } #[test] diff --git a/node/src/client_provider.rs b/node/src/client_provider.rs index 220f5018..703d864b 100644 --- a/node/src/client_provider.rs +++ b/node/src/client_provider.rs @@ -21,4 +21,8 @@ impl ClientProvider for FullClientHandle { fn storage(&self, hash: H256, key: &StorageKey) -> sp_blockchain::Result> { self.0.storage(hash, key) } + + fn hash(&self, number: u32) -> polkadot_sdk::sp_blockchain::Result> { + self.0.hash(number) + } } diff --git a/pallets/prover_db_indexer/src/db_events.rs b/pallets/prover_db_indexer/src/db_events.rs index 9e3ad054..b85b3b4e 100644 --- a/pallets/prover_db_indexer/src/db_events.rs +++ b/pallets/prover_db_indexer/src/db_events.rs @@ -138,6 +138,7 @@ mod tests { ext.register_extension(MockClientProvider::client_ext( None, [Err("test error".to_string())], + [], )); ext.execute_with(|| { let err = db_events_at::(H256::zero()).err().unwrap(); @@ -150,7 +151,7 @@ mod tests { #[test] fn empty_when_no_value_at_key() { let mut ext = new_test_ext(); - ext.register_extension(MockClientProvider::client_ext(None, [Ok(None)])); + ext.register_extension(MockClientProvider::client_ext(None, [Ok(None)], [])); ext.execute_with(|| { assert!(matches!( db_events_at::(H256::zero()), @@ -165,6 +166,7 @@ mod tests { ext.register_extension(MockClientProvider::client_ext( None, [Ok(Some(StorageData(alloc::vec![0xFF, 0xFF, 0xFF])))], + [], )); ext.execute_with(|| { assert!(matches!( @@ -214,6 +216,7 @@ mod tests { ext.register_extension(MockClientProvider::client_ext( None, [Ok(Some(StorageData(codec::Encode::encode(&records))))], + [], )); ext.execute_with(|| { let events: Vec<_> = db_events_at::(H256::zero()).unwrap().collect(); diff --git a/pallets/prover_db_indexer/src/lib.rs b/pallets/prover_db_indexer/src/lib.rs index ce56e4a0..2a18b3ba 100644 --- a/pallets/prover_db_indexer/src/lib.rs +++ b/pallets/prover_db_indexer/src/lib.rs @@ -85,6 +85,17 @@ pub enum ConsumerError { /// Underlying error from [`crate::db_events::db_events_at`]. source: crate::db_events::DBEventError, }, + /// `HeaderBackend::hash` succeeded but returned `None`, meaning the + /// block number's header is not in the node's chain. + #[snafu(display("block number's header is not in the node's chain"))] + HeaderNotInChain, + /// The node's client failed to look up a block's hash by number. + #[snafu(display("client failed to look up block hash: {source}"))] + ClientHash { + /// Error from the underlying `HeaderBackend::hash` call. + #[snafu(source(false))] + source: alloc::string::String, + }, } #[polkadot_sdk::frame_support::pallet] @@ -197,9 +208,7 @@ pub mod pallet { block_num: u64, config: &ProverDbConsumerConfig, ) -> Result<(), ConsumerError> { - let bn: BlockNumberFor = - block_num.checked_into().context(BlockNumberOverflowSnafu)?; - for event in db_events_at::(frame_system::Pallet::::block_hash(bn))? { + for event in db_events_at::(Self::block_hash(block_num)?)? { match event { DBEvent::TableDropped(_, _, table, _) => { if table_matches_filters(&table, &config.include) { @@ -237,5 +246,21 @@ pub mod pallet { Ok(()) } + + /// Looks up a block number's hash, falling back to the node's client if `BlockHash` doesn't have it. + pub(crate) fn block_hash(block_num: u64) -> Result { + let bn: BlockNumberFor = + block_num.checked_into().context(BlockNumberOverflowSnafu)?; + let hash = match frame_system::pallet::BlockHash::::try_get(bn) { + Ok(hash) => hash, + Err(()) => native::client::client::hash( + block_num.checked_into().context(BlockNumberOverflowSnafu)?, + ) + .context(NoRegisteredClientSnafu)? + .map_err(|source| ClientHashSnafu { source }.build())? + .context(HeaderNotInChainSnafu)?, + }; + Ok(hash) + } } } diff --git a/pallets/prover_db_indexer/src/mock.rs b/pallets/prover_db_indexer/src/mock.rs index 15b42629..fb8aaaaf 100644 --- a/pallets/prover_db_indexer/src/mock.rs +++ b/pallets/prover_db_indexer/src/mock.rs @@ -220,6 +220,7 @@ pub struct MockClientProvider { storage_responses: std::sync::Mutex< std::collections::VecDeque, String>>, >, + hash_responses: std::sync::Mutex, String>>>, } #[cfg(all(test, feature = "std"))] @@ -230,10 +231,12 @@ impl MockClientProvider { storage_responses: impl IntoIterator< Item = Result, String>, >, + hash_responses: impl IntoIterator, String>>, ) -> native::client::ClientExt { native::client::ClientExt(std::sync::Arc::new(Self { finalized_state, storage_responses: std::sync::Mutex::new(storage_responses.into_iter().collect()), + hash_responses: std::sync::Mutex::new(hash_responses.into_iter().collect()), })) } } @@ -256,6 +259,15 @@ impl native::client::ClientProvider for MockClientProvider { .unwrap_or(Ok(None)) .map_err(polkadot_sdk::sp_blockchain::Error::UnknownBlock) } + + fn hash(&self, _number: u32) -> polkadot_sdk::sp_blockchain::Result> { + self.hash_responses + .lock() + .unwrap() + .pop_front() + .unwrap_or(Ok(None)) + .map_err(polkadot_sdk::sp_blockchain::Error::UnknownBlock) + } } /// Builds an `EventRecord` the way `frame_system` would have deposited it, diff --git a/pallets/prover_db_indexer/src/tests.rs b/pallets/prover_db_indexer/src/tests.rs index b186acac..ff55540f 100644 --- a/pallets/prover_db_indexer/src/tests.rs +++ b/pallets/prover_db_indexer/src/tests.rs @@ -3,6 +3,7 @@ use native_api::Api; use pallet_tables::{CommitmentCreationCmd, UpdateTable}; use polkadot_sdk::frame_support::traits::Hooks; use polkadot_sdk::frame_support::BoundedVec; +use polkadot_sdk::frame_system::pallet::BlockHash; use polkadot_sdk::frame_system::EventRecord; use polkadot_sdk::sp_core::offchain::testing::{PendingRequest, TestOffchainExt}; use polkadot_sdk::sp_core::offchain::{OffchainDbExt, OffchainWorkerExt}; @@ -62,11 +63,20 @@ fn setup_with_config( ext.register_extension(OffchainWorkerExt::new(offchain.clone())); ext.register_extension(OffchainDbExt::new(offchain)); ext.register_extension(MockClientProvider::client_ext( - Some((H256::zero(), finalized_block_num)), + Some(( + H256::repeat_byte(finalized_block_num as u8), + finalized_block_num, + )), events .into_iter() .map(|e| Ok(Some(StorageData(e.encode())))), + [], )); + ext.execute_with(|| { + for bn in 1..=finalized_block_num { + BlockHash::::insert(bn as u64, H256::repeat_byte(bn as u8)); + } + }); let mut config_store = std::collections::HashMap::new(); config_store.insert(PROVER_DB_CONFIG_URL_KEY.to_string(), MOCK_URL.to_string()); config_store.insert( @@ -500,3 +510,85 @@ fn ocw_with_empty_include_set_forwards_nothing() { ProverDbIndexer::offchain_worker(1); }); } + +fn advance_block(parent_hash: &mut H256, i: u64) -> Option { + System::initialize(&(i + 1), parent_hash, &Default::default()); + *parent_hash = System::finalize().hash(); + Some(*parent_hash) +} + +#[test] +fn block_hash_falls_back_to_client_once_frame_system_prunes_the_entry() { + let mut ext = new_test_ext(); + let fallback_hash = H256::repeat_byte(0xAB); + ext.register_extension(MockClientProvider::client_ext( + None, + [], + std::iter::repeat_n(Ok(Some(fallback_hash)), 6), + )); + + ext.execute_with(|| { + let mut h: Vec<_> = (0..6).scan(H256::zero(), advance_block).collect(); + for i in 0..5 { + assert_eq!(ProverDbIndexer::block_hash(i + 1).unwrap(), h[i as usize]); + } + h.extend((6..17).scan(h[5], advance_block)); + for i in 0..6 { + assert_eq!(ProverDbIndexer::block_hash(i + 1).unwrap(), fallback_hash); + } + for i in 6..16 { + assert_eq!(ProverDbIndexer::block_hash(i + 1).unwrap(), h[i as usize]); + } + }); +} + +#[test] +fn block_hash_returns_stored_hash_when_present() { + let mut ext = new_test_ext(); + ext.execute_with(|| { + let hash = H256::repeat_byte(7); + BlockHash::::insert(5u64, hash); + + assert_eq!(ProverDbIndexer::block_hash(5).unwrap(), hash); + }); +} + +#[test] +fn block_hash_errors_when_client_not_registered() { + let mut ext = new_test_ext(); + ext.execute_with(|| { + assert!(matches!( + ProverDbIndexer::block_hash(5), + Err(crate::ConsumerError::NoRegisteredClient) + )); + }); +} + +#[test] +fn block_hash_errors_when_client_hash_lookup_fails() { + let mut ext = new_test_ext(); + ext.register_extension(MockClientProvider::client_ext( + None, + [], + [Err("simulated client hash lookup failure".to_string())], + )); + ext.execute_with(|| { + assert!(matches!( + ProverDbIndexer::block_hash(5), + Err(crate::ConsumerError::ClientHash { source }) + if source == "UnknownBlock: simulated client hash lookup failure" + )); + }); +} + +#[test] +fn block_hash_errors_when_header_not_in_chain() { + let mut ext = new_test_ext(); + ext.register_extension(MockClientProvider::client_ext(None, [], [Ok(None)])); + ext.execute_with(|| { + assert!(matches!( + ProverDbIndexer::block_hash(5), + Err(crate::ConsumerError::HeaderNotInChain) + )); + }); +}