Skip to content

Commit 3102201

Browse files
authored
offchain: double index to overcome overrides (#2360)
# Goal The goal of this PR is to fix the bug in offchain worker Closes #2293 There are 2 kinds of errors: - Some MsaIds have multiple indexed keys which only one of them is valid (ex: 1394377) - Some MsaIds have no indexed keys (ex: 1414115) The main issue is that forks cause overwrites on offchain indexed events so we had to find a way to fix these overwrites. # Attempted solution We are now double indexing any event so that if one of them gets overwritten the other one would most likely get consumed. To do this we are getting a pseudo random index from 1 to 1000 and store that event inside a bucket associated with that block and index. The only tradeoff is that we have to look at all the buckets for each block inside offchain worker to fetch these stored events since there is no `iter_prefix` from inside offchain worker. # Discussion - To fix the existing wrong indices I tried to add a runtime but any code not called from offchain worker context can not access to any offchain worker related features such as locks and timestamps and ... - Since adding an extrinsic was the easiest way to handle it added a new extrinsic (similar to `remark`) which only adds an event to reindexing a certain msa id. This is not the best solution since we have to spend some token to reindex but it was the fastest and easiest to do. Currently it's not free but if desired we can make it free or capacity supported but will need to ensure it's not going to be abused - The other way to fix this is to add a new rpc and refactor the code such that it can be called from this rpc but it was a heavier lift # Further attempts that didn't work - Using the Remark event to inject the reindex data doesn't work since the event only stores the hash of the remark not the data - Using the Remark Extrinsic to inject the reindex data also doesn't work since it is only stored temporarily and if the offchain worker does not run in the the exact time-window that it's available it would be lost. # Checklist - [x] Unit Tests added? - [x] Benchmarks added? - [x] Spec version incremented?
1 parent 81d38f7 commit 3102201

File tree

6 files changed

+342
-91
lines changed

6 files changed

+342
-91
lines changed

pallets/msa/src/benchmarking.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,22 @@ mod benchmarks {
353353
Ok(())
354354
}
355355

356+
#[benchmark]
357+
fn reindex_offchain() -> Result<(), BenchmarkError> {
358+
let key = create_account::<T>("account", 0);
359+
let caller = whitelisted_caller();
360+
let msa_id = 1u64;
361+
let event = OffchainReplayEvent::MsaPallet(MsaOffchainReplayEvent::KeyReIndex {
362+
msa_id,
363+
index_key: Some(key),
364+
});
365+
366+
#[extrinsic_call]
367+
_(RawOrigin::Signed(caller), event);
368+
369+
Ok(())
370+
}
371+
356372
impl_benchmark_test_suite!(
357373
Msa,
358374
crate::tests::mock::new_test_ext_keystore(),

pallets/msa/src/lib.rs

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -427,7 +427,7 @@ pub mod pallet {
427427
Self::create_account(public_key, |_| -> DispatchResult { Ok(()) })?;
428428

429429
let event = Event::MsaCreated { msa_id: new_msa_id, key: new_public_key };
430-
offchain_index_event::<T>(&event, new_msa_id);
430+
offchain_index_event::<T>(Some(&event), new_msa_id);
431431
Self::deposit_event(event);
432432
Ok(())
433433
}
@@ -496,7 +496,7 @@ pub mod pallet {
496496

497497
let event =
498498
Event::MsaCreated { msa_id: new_delegator_msa_id, key: new_delegator_public_key };
499-
offchain_index_event::<T>(&event, new_delegator_msa_id);
499+
offchain_index_event::<T>(Some(&event), new_delegator_msa_id);
500500
Self::deposit_event(event);
501501
Self::deposit_event(Event::DelegationGranted {
502502
delegator_id: DelegatorId(new_delegator_msa_id),
@@ -688,7 +688,7 @@ pub mod pallet {
688688
msa_id,
689689
key: add_key_payload.new_public_key.clone(),
690690
};
691-
offchain_index_event::<T>(&event, msa_id);
691+
offchain_index_event::<T>(Some(&event), msa_id);
692692
Self::deposit_event(event);
693693
Ok(())
694694
},
@@ -725,7 +725,7 @@ pub mod pallet {
725725

726726
// Deposit the event
727727
let event = Event::PublicKeyDeleted { key: public_key_to_delete };
728-
offchain_index_event::<T>(&event, who_msa_id);
728+
offchain_index_event::<T>(Some(&event), who_msa_id);
729729
Self::deposit_event(event);
730730
},
731731
None => {
@@ -811,7 +811,7 @@ pub mod pallet {
811811
Some(msa_id) => {
812812
Self::delete_key_for_msa(msa_id, &who)?;
813813
let event = Event::PublicKeyDeleted { key: who };
814-
offchain_index_event::<T>(&event, msa_id);
814+
offchain_index_event::<T>(Some(&event), msa_id);
815815
Self::deposit_event(event);
816816
Self::deposit_event(Event::MsaRetired { msa_id });
817817
},
@@ -877,6 +877,35 @@ pub mod pallet {
877877
});
878878
Ok(())
879879
}
880+
881+
/// A generic endpoint to replay any offchain related event to fix any potential issues
882+
#[pallet::call_index(13)]
883+
#[pallet::weight(T::WeightInfo::reindex_offchain())]
884+
pub fn reindex_offchain(
885+
origin: OriginFor<T>,
886+
event: OffchainReplayEvent<T>,
887+
) -> DispatchResult {
888+
let _ = ensure_signed(origin)?;
889+
match event {
890+
OffchainReplayEvent::MsaPallet(MsaOffchainReplayEvent::KeyReIndex {
891+
msa_id,
892+
index_key,
893+
}) => {
894+
// don't need to check existence of msa_id since it would get checked on offchain side
895+
match index_key {
896+
Some(key) => {
897+
let event = Event::PublicKeyAdded { msa_id, key };
898+
offchain_index_event::<T>(Some(&event), msa_id);
899+
},
900+
None => {
901+
offchain_index_event::<T>(None, msa_id);
902+
},
903+
}
904+
},
905+
}
906+
907+
Ok(())
908+
}
880909
}
881910
}
882911

0 commit comments

Comments
 (0)