From e32983095bcc5720a25cb9ccad5faf0fe5992314 Mon Sep 17 00:00:00 2001
From: Benjamin Gallois <business@gallois.cc>
Date: Wed, 18 Dec 2024 13:26:34 +0100
Subject: [PATCH] Fix #264: Distance inherent data provider should not fail
(nodes/rust/duniter-v2s!294)
* inherent data provider cannot fail
* remove unused errors
---
client/distance/src/lib.rs | 116 ++++++++++++++++++---------------
node/src/service.rs | 4 +-
primitives/distance/src/lib.rs | 36 +++-------
3 files changed, 75 insertions(+), 81 deletions(-)
diff --git a/client/distance/src/lib.rs b/client/distance/src/lib.rs
index c36091612..88d15b88f 100644
--- a/client/distance/src/lib.rs
+++ b/client/distance/src/lib.rs
@@ -59,37 +59,35 @@ pub fn create_distance_inherent_data_provider<B, C, Backend>(
parent: B::Hash,
distance_dir: PathBuf,
owner_keys: &[sp_core::sr25519::Public],
-) -> Result<sp_distance::InherentDataProvider<IdtyIndex>, sc_client_api::blockchain::Error>
+) -> sp_distance::InherentDataProvider<IdtyIndex>
where
B: BlockT,
C: ProvideUncles<B> + StorageProvider<B, Backend>,
Backend: sc_client_api::Backend<B>,
IdtyIndex: Decode + Encode + PartialEq + TypeInfo,
{
- // Retrieve the period_index from storage. If storage is inaccessible or the data is corrupted,
- // return the appropriate error.
+ // Retrieve the period_index from storage.
let period_index = client
.storage(
parent,
&StorageKey(
frame_support::storage::storage_prefix(b"Distance", b"CurrentPeriodIndex").to_vec(),
),
- )?
- .map_or_else(
- || {
- Err(sc_client_api::blockchain::Error::Storage(
- "CurrentPeriodIndex value not found".to_string(),
- ))
- },
- |raw| {
- u32::decode(&mut &raw.0[..])
- .map_err(|e| sc_client_api::blockchain::Error::from_state(Box::new(e)))
- },
- )?;
+ )
+ .ok()
+ .flatten()
+ .and_then(|raw| u32::decode(&mut &raw.0[..]).ok());
+
+ // Return early if the storage is inaccessible or the data is corrupted.
+ let period_index = match period_index {
+ Some(index) => index,
+ None => {
+ log::error!("🧙 [distance inherent] PeriodIndex decoding failed.");
+ return sp_distance::InherentDataProvider::<IdtyIndex>::new(None);
+ }
+ };
// Retrieve the published_results from storage.
- // Return an error if the storage is inaccessible.
- // If accessible, continue execution. If None, it means there are no published_results at this block.
let published_results = client
.storage(
parent,
@@ -105,47 +103,61 @@ where
)
.to_vec(),
),
- )?
+ )
+ .ok()
+ .flatten()
.and_then(|raw| {
pallet_distance::EvaluationPool::<AccountId32, IdtyIndex>::decode(&mut &raw.0[..]).ok()
});
- // Have we already published a result for this period?
- if let Some(results) = published_results {
- // Find the account associated with the BABE key that is in our owner keys.
- let mut local_account = None;
- for key in owner_keys {
- // Session::KeyOwner is StorageMap<_, Twox64Concat, (KeyTypeId, Vec<u8>), AccountId32, OptionQuery>
- // Slices (variable length) and array (fixed length) are encoded differently, so the `.as_slice()` is needed
- let item_key = (sp_runtime::KeyTypeId(*b"babe"), key.0.as_slice()).encode();
- let mut storage_key =
- frame_support::storage::storage_prefix(b"Session", b"KeyOwner").to_vec();
- storage_key.extend_from_slice(&sp_core::twox_64(&item_key));
- storage_key.extend_from_slice(&item_key);
+ // Return early if the storage is inaccessible or the data is corrupted.
+ let published_results = match published_results {
+ Some(published_results) => published_results,
+ None => {
+ log::info!("🧙 [distance inherent] No published result at this block.");
+ return sp_distance::InherentDataProvider::<IdtyIndex>::new(None);
+ }
+ };
- if let Some(raw_data) = client.storage(parent, &StorageKey(storage_key))? {
- if let Ok(key_owner) = AccountId32::decode(&mut &raw_data.0[..]) {
- local_account = Some(key_owner);
- break;
- } else {
- log::warn!("🧙 [distance oracle] Cannot decode key owner value");
- }
+ // Find the account associated with the BABE key that is in our owner keys.
+ let mut local_account = None;
+ for key in owner_keys {
+ // Session::KeyOwner is StorageMap<_, Twox64Concat, (KeyTypeId, Vec<u8>), AccountId32, OptionQuery>
+ // Slices (variable length) and array (fixed length) are encoded differently, so the `.as_slice()` is needed
+ let item_key = (sp_runtime::KeyTypeId(*b"babe"), key.0.as_slice()).encode();
+ let mut storage_key =
+ frame_support::storage::storage_prefix(b"Session", b"KeyOwner").to_vec();
+ storage_key.extend_from_slice(&sp_core::twox_64(&item_key));
+ storage_key.extend_from_slice(&item_key);
+
+ if let Some(raw_data) = client
+ .storage(parent, &StorageKey(storage_key))
+ .ok()
+ .flatten()
+ {
+ if let Ok(key_owner) = AccountId32::decode(&mut &raw_data.0[..]) {
+ local_account = Some(key_owner);
+ break;
+ } else {
+ log::warn!("🧙 [distance inherent] Cannot decode key owner value");
}
}
- if let Some(local_account) = local_account {
- if results.evaluators.contains(&local_account) {
- log::debug!("🧙 [distance oracle] Already published a result for this period");
- return Ok(sp_distance::InherentDataProvider::<IdtyIndex>::new(None));
- }
- } else {
- log::error!("🧙 [distance oracle] Cannot find our BABE owner key");
- return Ok(sp_distance::InherentDataProvider::<IdtyIndex>::new(None));
+ }
+
+ // Have we already published a result for this period?
+ if let Some(local_account) = local_account {
+ if published_results.evaluators.contains(&local_account) {
+ log::debug!("🧙 [distance inherent] Already published a result for this period");
+ return sp_distance::InherentDataProvider::<IdtyIndex>::new(None);
}
+ } else {
+ log::error!("🧙 [distance inherent] Cannot find our BABE owner key");
+ return sp_distance::InherentDataProvider::<IdtyIndex>::new(None);
}
// Read evaluation result from file, if it exists
log::debug!(
- "🧙 [distance oracle] Reading evaluation result from file {:?}",
+ "🧙 [distance inherent] Reading evaluation result from file {:?}",
distance_dir.clone().join(period_index.to_string())
);
let evaluation_result = match std::fs::read(
@@ -155,20 +167,20 @@ where
Err(e) => {
match e.kind() {
std::io::ErrorKind::NotFound => {
- log::debug!("🧙 [distance oracle] Evaluation result file not found. Please ensure that the oracle version matches {}", VERSION_PREFIX);
+ log::debug!("🧙 [distance inherent] Evaluation result file not found. Please ensure that the oracle version matches {}", VERSION_PREFIX);
}
_ => {
log::error!(
- "🧙 [distance oracle] Cannot read distance evaluation result file: {e:?}"
+ "🧙 [distance inherent] Cannot read distance evaluation result file: {e:?}"
);
}
}
- return Ok(sp_distance::InherentDataProvider::<IdtyIndex>::new(None));
+ return sp_distance::InherentDataProvider::<IdtyIndex>::new(None);
}
};
- log::info!("🧙 [distance oracle] Providing evaluation result");
- Ok(sp_distance::InherentDataProvider::<IdtyIndex>::new(Some(
+ log::info!("🧙 [distance inherent] Providing evaluation result");
+ sp_distance::InherentDataProvider::<IdtyIndex>::new(Some(
sp_distance::ComputationResult::decode(&mut evaluation_result.as_slice()).unwrap(),
- )))
+ ))
}
diff --git a/node/src/service.rs b/node/src/service.rs
index 894644308..0593f8fe5 100644
--- a/node/src/service.rs
+++ b/node/src/service.rs
@@ -503,7 +503,7 @@ where
FullBackend,
>(
&*client, parent, distance_dir, &babe_owner_keys.clone()
- )?;
+ );
Ok((timestamp, babe, distance))
}
},
@@ -549,7 +549,7 @@ where
FullBackend,
>(
&*client, parent, distance_dir, &babe_owner_keys.clone()
- )?;
+ );
Ok((slot, timestamp, storage_proof, distance))
}
diff --git a/primitives/distance/src/lib.rs b/primitives/distance/src/lib.rs
index acaf53c38..c447da102 100644
--- a/primitives/distance/src/lib.rs
+++ b/primitives/distance/src/lib.rs
@@ -35,29 +35,14 @@ pub struct ComputationResult {
pub distances: scale_info::prelude::vec::Vec<Perbill>,
}
+/// Errors that can occur while checking the inherent data in `ProvideInherent::check_inherent` from pallet-distance.
#[derive(Encode, sp_runtime::RuntimeDebug)]
#[cfg_attr(feature = "std", derive(Decode, thiserror::Error))]
-pub enum InherentError {
- #[cfg_attr(feature = "std", error("InvalidComputationResultFile"))]
- InvalidComputationResultFile,
-}
+pub enum InherentError {}
impl IsFatalError for InherentError {
fn is_fatal_error(&self) -> bool {
- match self {
- InherentError::InvalidComputationResultFile => false,
- }
- }
-}
-
-impl InherentError {
- #[cfg(feature = "std")]
- pub fn try_from(id: &InherentIdentifier, mut data: &[u8]) -> Option<Self> {
- if id == &INHERENT_IDENTIFIER {
- <InherentError as codec::Decode>::decode(&mut data).ok()
- } else {
- None
- }
+ false
}
}
@@ -94,15 +79,12 @@ impl<IdtyIndex: Decode + Encode + PartialEq + TypeInfo + Send + Sync>
async fn try_handle_error(
&self,
- identifier: &InherentIdentifier,
- error: &[u8],
+ _identifier: &InherentIdentifier,
+ _error: &[u8],
) -> Option<Result<(), sp_inherents::Error>> {
- if *identifier != INHERENT_IDENTIFIER {
- return None;
- }
-
- Some(Err(sp_inherents::Error::Application(Box::from(
- InherentError::try_from(&INHERENT_IDENTIFIER, error)?,
- ))))
+ // No errors occur here.
+ // Errors handled here are emitted in the `ProvideInherent::check_inherent`
+ // (from pallet-distance) which is not implemented.
+ None
}
}
--
GitLab