Skip to content

Commit e329830

Browse files
bgalloisHugo Trentesaux
bgallois
authored and
Hugo Trentesaux
committed
Fix #264: Distance inherent data provider should not fail (nodes/rust/duniter-v2s!294)
* inherent data provider cannot fail * remove unused errors
1 parent a0afb07 commit e329830

File tree

3 files changed

+75
-81
lines changed

3 files changed

+75
-81
lines changed

‎client/distance/src/lib.rs

+64-52
Original file line numberDiff line numberDiff line change
@@ -59,37 +59,35 @@ pub fn create_distance_inherent_data_provider<B, C, Backend>(
5959
parent: B::Hash,
6060
distance_dir: PathBuf,
6161
owner_keys: &[sp_core::sr25519::Public],
62-
) -> Result<sp_distance::InherentDataProvider<IdtyIndex>, sc_client_api::blockchain::Error>
62+
) -> sp_distance::InherentDataProvider<IdtyIndex>
6363
where
6464
B: BlockT,
6565
C: ProvideUncles<B> + StorageProvider<B, Backend>,
6666
Backend: sc_client_api::Backend<B>,
6767
IdtyIndex: Decode + Encode + PartialEq + TypeInfo,
6868
{
69-
// Retrieve the period_index from storage. If storage is inaccessible or the data is corrupted,
70-
// return the appropriate error.
69+
// Retrieve the period_index from storage.
7170
let period_index = client
7271
.storage(
7372
parent,
7473
&StorageKey(
7574
frame_support::storage::storage_prefix(b"Distance", b"CurrentPeriodIndex").to_vec(),
7675
),
77-
)?
78-
.map_or_else(
79-
|| {
80-
Err(sc_client_api::blockchain::Error::Storage(
81-
"CurrentPeriodIndex value not found".to_string(),
82-
))
83-
},
84-
|raw| {
85-
u32::decode(&mut &raw.0[..])
86-
.map_err(|e| sc_client_api::blockchain::Error::from_state(Box::new(e)))
87-
},
88-
)?;
76+
)
77+
.ok()
78+
.flatten()
79+
.and_then(|raw| u32::decode(&mut &raw.0[..]).ok());
80+
81+
// Return early if the storage is inaccessible or the data is corrupted.
82+
let period_index = match period_index {
83+
Some(index) => index,
84+
None => {
85+
log::error!("🧙 [distance inherent] PeriodIndex decoding failed.");
86+
return sp_distance::InherentDataProvider::<IdtyIndex>::new(None);
87+
}
88+
};
8989

9090
// Retrieve the published_results from storage.
91-
// Return an error if the storage is inaccessible.
92-
// If accessible, continue execution. If None, it means there are no published_results at this block.
9391
let published_results = client
9492
.storage(
9593
parent,
@@ -105,47 +103,61 @@ where
105103
)
106104
.to_vec(),
107105
),
108-
)?
106+
)
107+
.ok()
108+
.flatten()
109109
.and_then(|raw| {
110110
pallet_distance::EvaluationPool::<AccountId32, IdtyIndex>::decode(&mut &raw.0[..]).ok()
111111
});
112112

113-
// Have we already published a result for this period?
114-
if let Some(results) = published_results {
115-
// Find the account associated with the BABE key that is in our owner keys.
116-
let mut local_account = None;
117-
for key in owner_keys {
118-
// Session::KeyOwner is StorageMap<_, Twox64Concat, (KeyTypeId, Vec<u8>), AccountId32, OptionQuery>
119-
// Slices (variable length) and array (fixed length) are encoded differently, so the `.as_slice()` is needed
120-
let item_key = (sp_runtime::KeyTypeId(*b"babe"), key.0.as_slice()).encode();
121-
let mut storage_key =
122-
frame_support::storage::storage_prefix(b"Session", b"KeyOwner").to_vec();
123-
storage_key.extend_from_slice(&sp_core::twox_64(&item_key));
124-
storage_key.extend_from_slice(&item_key);
113+
// Return early if the storage is inaccessible or the data is corrupted.
114+
let published_results = match published_results {
115+
Some(published_results) => published_results,
116+
None => {
117+
log::info!("🧙 [distance inherent] No published result at this block.");
118+
return sp_distance::InherentDataProvider::<IdtyIndex>::new(None);
119+
}
120+
};
125121

126-
if let Some(raw_data) = client.storage(parent, &StorageKey(storage_key))? {
127-
if let Ok(key_owner) = AccountId32::decode(&mut &raw_data.0[..]) {
128-
local_account = Some(key_owner);
129-
break;
130-
} else {
131-
log::warn!("🧙 [distance oracle] Cannot decode key owner value");
132-
}
122+
// Find the account associated with the BABE key that is in our owner keys.
123+
let mut local_account = None;
124+
for key in owner_keys {
125+
// Session::KeyOwner is StorageMap<_, Twox64Concat, (KeyTypeId, Vec<u8>), AccountId32, OptionQuery>
126+
// Slices (variable length) and array (fixed length) are encoded differently, so the `.as_slice()` is needed
127+
let item_key = (sp_runtime::KeyTypeId(*b"babe"), key.0.as_slice()).encode();
128+
let mut storage_key =
129+
frame_support::storage::storage_prefix(b"Session", b"KeyOwner").to_vec();
130+
storage_key.extend_from_slice(&sp_core::twox_64(&item_key));
131+
storage_key.extend_from_slice(&item_key);
132+
133+
if let Some(raw_data) = client
134+
.storage(parent, &StorageKey(storage_key))
135+
.ok()
136+
.flatten()
137+
{
138+
if let Ok(key_owner) = AccountId32::decode(&mut &raw_data.0[..]) {
139+
local_account = Some(key_owner);
140+
break;
141+
} else {
142+
log::warn!("🧙 [distance inherent] Cannot decode key owner value");
133143
}
134144
}
135-
if let Some(local_account) = local_account {
136-
if results.evaluators.contains(&local_account) {
137-
log::debug!("🧙 [distance oracle] Already published a result for this period");
138-
return Ok(sp_distance::InherentDataProvider::<IdtyIndex>::new(None));
139-
}
140-
} else {
141-
log::error!("🧙 [distance oracle] Cannot find our BABE owner key");
142-
return Ok(sp_distance::InherentDataProvider::<IdtyIndex>::new(None));
145+
}
146+
147+
// Have we already published a result for this period?
148+
if let Some(local_account) = local_account {
149+
if published_results.evaluators.contains(&local_account) {
150+
log::debug!("🧙 [distance inherent] Already published a result for this period");
151+
return sp_distance::InherentDataProvider::<IdtyIndex>::new(None);
143152
}
153+
} else {
154+
log::error!("🧙 [distance inherent] Cannot find our BABE owner key");
155+
return sp_distance::InherentDataProvider::<IdtyIndex>::new(None);
144156
}
145157

146158
// Read evaluation result from file, if it exists
147159
log::debug!(
148-
"🧙 [distance oracle] Reading evaluation result from file {:?}",
160+
"🧙 [distance inherent] Reading evaluation result from file {:?}",
149161
distance_dir.clone().join(period_index.to_string())
150162
);
151163
let evaluation_result = match std::fs::read(
@@ -155,20 +167,20 @@ where
155167
Err(e) => {
156168
match e.kind() {
157169
std::io::ErrorKind::NotFound => {
158-
log::debug!("🧙 [distance oracle] Evaluation result file not found. Please ensure that the oracle version matches {}", VERSION_PREFIX);
170+
log::debug!("🧙 [distance inherent] Evaluation result file not found. Please ensure that the oracle version matches {}", VERSION_PREFIX);
159171
}
160172
_ => {
161173
log::error!(
162-
"🧙 [distance oracle] Cannot read distance evaluation result file: {e:?}"
174+
"🧙 [distance inherent] Cannot read distance evaluation result file: {e:?}"
163175
);
164176
}
165177
}
166-
return Ok(sp_distance::InherentDataProvider::<IdtyIndex>::new(None));
178+
return sp_distance::InherentDataProvider::<IdtyIndex>::new(None);
167179
}
168180
};
169181

170-
log::info!("🧙 [distance oracle] Providing evaluation result");
171-
Ok(sp_distance::InherentDataProvider::<IdtyIndex>::new(Some(
182+
log::info!("🧙 [distance inherent] Providing evaluation result");
183+
sp_distance::InherentDataProvider::<IdtyIndex>::new(Some(
172184
sp_distance::ComputationResult::decode(&mut evaluation_result.as_slice()).unwrap(),
173-
)))
185+
))
174186
}

‎node/src/service.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -503,7 +503,7 @@ where
503503
FullBackend,
504504
>(
505505
&*client, parent, distance_dir, &babe_owner_keys.clone()
506-
)?;
506+
);
507507
Ok((timestamp, babe, distance))
508508
}
509509
},
@@ -549,7 +549,7 @@ where
549549
FullBackend,
550550
>(
551551
&*client, parent, distance_dir, &babe_owner_keys.clone()
552-
)?;
552+
);
553553

554554
Ok((slot, timestamp, storage_proof, distance))
555555
}

‎primitives/distance/src/lib.rs

+9-27
Original file line numberDiff line numberDiff line change
@@ -35,29 +35,14 @@ pub struct ComputationResult {
3535
pub distances: scale_info::prelude::vec::Vec<Perbill>,
3636
}
3737

38+
/// Errors that can occur while checking the inherent data in `ProvideInherent::check_inherent` from pallet-distance.
3839
#[derive(Encode, sp_runtime::RuntimeDebug)]
3940
#[cfg_attr(feature = "std", derive(Decode, thiserror::Error))]
40-
pub enum InherentError {
41-
#[cfg_attr(feature = "std", error("InvalidComputationResultFile"))]
42-
InvalidComputationResultFile,
43-
}
41+
pub enum InherentError {}
4442

4543
impl IsFatalError for InherentError {
4644
fn is_fatal_error(&self) -> bool {
47-
match self {
48-
InherentError::InvalidComputationResultFile => false,
49-
}
50-
}
51-
}
52-
53-
impl InherentError {
54-
#[cfg(feature = "std")]
55-
pub fn try_from(id: &InherentIdentifier, mut data: &[u8]) -> Option<Self> {
56-
if id == &INHERENT_IDENTIFIER {
57-
<InherentError as codec::Decode>::decode(&mut data).ok()
58-
} else {
59-
None
60-
}
45+
false
6146
}
6247
}
6348

@@ -94,15 +79,12 @@ impl<IdtyIndex: Decode + Encode + PartialEq + TypeInfo + Send + Sync>
9479

9580
async fn try_handle_error(
9681
&self,
97-
identifier: &InherentIdentifier,
98-
error: &[u8],
82+
_identifier: &InherentIdentifier,
83+
_error: &[u8],
9984
) -> Option<Result<(), sp_inherents::Error>> {
100-
if *identifier != INHERENT_IDENTIFIER {
101-
return None;
102-
}
103-
104-
Some(Err(sp_inherents::Error::Application(Box::from(
105-
InherentError::try_from(&INHERENT_IDENTIFIER, error)?,
106-
))))
85+
// No errors occur here.
86+
// Errors handled here are emitted in the `ProvideInherent::check_inherent`
87+
// (from pallet-distance) which is not implemented.
88+
None
10789
}
10890
}

0 commit comments

Comments
 (0)