optimize storage layout for TapestryStrands and fix OCW deadlock

Signed-off-by: Uncle Stinky <uncle.stinky@ghostchain.io>

Cargo.lock

@@ -1195,7 +1195,7 @@ dependencies = [
 
 [[package]]
 name = "casper-runtime"
-version = "4.0.19"
+version = "4.0.20"
 dependencies = [
  "casper-runtime-constants",
  "frame-benchmarking",
@@ -4114,7 +4114,7 @@ dependencies = [
 
 [[package]]
 name = "ghost-weaver"
-version = "0.0.26"
+version = "0.0.27"
 dependencies = [
  "frame-benchmarking",
  "frame-support",

pallets/weaver/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "ghost-weaver"
-version = "0.0.26"
+version = "0.0.27"
 description = "Weaving a secure cryptographic tapestry across different external chains."
 license.workspace = true
 authors.workspace = true

pallets/weaver/src/benchmarking.rs

@@ -77,21 +77,38 @@ mod benchmarks {
     fn weave_weft(
         a: Linear<1, { T::MaxAuthorities::get() as u32 }>
     ) -> Result<(), BenchmarkError> {
-        let weaving_session = 420;
+        let weaving_session: WeavingSession = 420;
-        let next_session = weaving_session + 1;
+        let next_session: WeavingSession = weaving_session + 1;
         let root_hash = H256::repeat_byte(0x69);
 
         let (network_id, authority, authority_index) = prepare_pallet::<T>(a, None);
 
-        let tapestry_key = (network_id, weaving_session, next_session, root_hash);
+        let next_session_bytes = next_session.to_be_bytes();
-        TapestryStrands::<T>::mutate(&tapestry_key, |tapestry| {
+        let weaving_session_bytes = weaving_session.to_be_bytes();
-            (0..a).for_each(|i| {
+
-                let index = i as AuthIndex;
+        let metadata: &[&[u8]] = &[
-                if index == authority_index { return; }
+            root_hash.as_ref(),
-                tapestry.insert(index);
+            next_session_bytes.as_ref(),
-            });
+            weaving_session_bytes.as_ref(),
+        ];
+
+        let tapestry_strand_key =
+            sequential_hash::<SubstrateBlake2Hasher, _>(metadata);
+
+        let mut bitmap = WeaversBitmap::<T>::empty(a);
+        (0..a).for_each(|i| {
+            let index = i as AuthIndex;
+            if index == authority_index { return; }
+            bitmap.insert(index);
         });
 
+        let mut tapestry_map = BoundedBTreeMap::<_, _, T::MaxAuthorities>::default();
+        tapestry_map
+            .try_insert(tapestry_strand_key.clone(), bitmap)
+            .map_err(|_| BenchmarkError::Stop("Failed to insert bitmap into benchmark map"))?;
+
+        TapestryStrands::<T>::insert(&network_id, tapestry_map);
+
         let current_block = T::BlockNumberProvider::current_block_number();
         let dummy_draft = TapestryDraftBuilder::default()
             .with_current_block(current_block)
@@ -112,18 +129,24 @@ mod benchmarks {
             .sign(&hash_attestation.encode())
             .expect("Signature should be created");
 
-        let tapestry_strand_key = (network_id, weaving_session, next_session, root_hash);
-
         let weaving_state = WeavingStates::<T>::get(&network_id);
         assert_eq!(weaving_state.hash, H256::default());
         assert_eq!(weaving_state.next_session, 0);
         assert_eq!(weaving_state.count, 0);
 
-        assert!(!TapestryStrands::<T>::get(&tapestry_strand_key).contains(authority_index));
+        let tapestry_strand = TapestryStrands::<T>::get(&network_id);
-        assert_eq!(
+        let maybe_tapestry_bitmap = tapestry_strand.get(&tapestry_strand_key);
-            TapestryStrands::<T>::get(&tapestry_strand_key).count_ones::<u32>(),
+
-            a - 1
+        let authority_does_not_exists = maybe_tapestry_bitmap
-        );
+            .map(|bitmap| !bitmap.contains(authority_index))
+            .ok_or(BenchmarkError::Stop("NetworkId should be registered"))?;
+
+        let tapestry_participants = maybe_tapestry_bitmap
+            .map(|bitmap| bitmap.count_ones::<u32>())
+            .ok_or(BenchmarkError::Stop("NetworkId should be registered"))?;
+
+        assert!(authority_does_not_exists);
+        assert_eq!(tapestry_participants, a - 1);
 
         #[extrinsic_call]
         _(RawOrigin::None, hash_attestation, signature);
@@ -133,11 +156,19 @@ mod benchmarks {
         assert_eq!(new_weaving_state.next_session, next_session);
         assert_eq!(new_weaving_state.count, a as AuthIndex);
 
-        assert!(TapestryStrands::<T>::get(&tapestry_strand_key).contains(authority_index));
+        let tapestry_strand = TapestryStrands::<T>::get(&network_id);
-        assert_eq!(
+        let maybe_tapestry_bitmap = tapestry_strand.get(&tapestry_strand_key);
-            TapestryStrands::<T>::get(&tapestry_strand_key).count_ones::<u32>(),
+
-            a
+        let authority_exists = maybe_tapestry_bitmap
-        );
+            .map(|bitmap| bitmap.contains(authority_index))
+            .ok_or(BenchmarkError::Stop("NetworkId should be registered"))?;
+
+        let tapestry_participants = maybe_tapestry_bitmap
+            .map(|bitmap| bitmap.count_ones::<u32>())
+            .ok_or(BenchmarkError::Stop("NetworkId should be registered"))?;
+
+        assert!(authority_exists);
+        assert_eq!(tapestry_participants, a);
 
         Ok(())
     }

pallets/weaver/src/lib.rs

@@ -31,14 +31,19 @@ use sp_runtime::{
 
 use ghost_helpers::{
     bounded_bitmap::{validate_bitmap_sizes, BoundedBitmap},
-    get_byzantium_quorum_threshold, get_submission_probability,
     networks::{NetworkData, NetworkType},
+    hash_chain::sequential_hash,
+    get_byzantium_quorum_threshold, get_submission_probability,
+    SubstrateBlake2Hasher,
 };
 use ghost_traits::{
-    bounded_bitmap::{BoundedBitmapReader, BoundedBitmapWriter},
+    bounded_bitmap::{
+        BoundedBitmapReader, BoundedBitmapWriter,
+        BoundedBitmapGenerator,
+    },
     networks::{
-        NetworkDataBasicHandler, NetworkDataInspectHandler, NetworkDataMutateHandler,
+        NetworkDataBasicHandler, NetworkDataInspectHandler,
-        NetworkRpcResolver,
+        NetworkDataMutateHandler, NetworkRpcResolver,
     },
 };
 
@@ -103,7 +108,7 @@ type WeaversBitmap<T> = BoundedBitmap<BitmapChunk, <T as Config>::MaxAuthorities
 pub mod pallet {
     use super::*;
 
-    const STORAGE_VERSION: StorageVersion = StorageVersion::new(1);
+    const STORAGE_VERSION: StorageVersion = StorageVersion::new(2);
 
     #[pallet::pallet]
     #[pallet::storage_version(STORAGE_VERSION)]
@@ -174,6 +179,7 @@ pub mod pallet {
         InvalidParticipantAttestation,
         NonExistentNetworkId,
         TooManyAttestations,
+        TooManyTapestries,
         TimeWentBackwards,
         WeavingIsActive,
         WeavingIsInactive,
@@ -222,15 +228,10 @@ pub mod pallet {
 
     #[pallet::storage]
     #[pallet::getter(fn tapestry_strands)]
-    pub(super) type TapestryStrands<T: Config> = StorageNMap<
+    pub(super) type TapestryStrands<T: Config> = StorageMap<
         _,
-        (
+        Twox64Concat, NetworkIdOf<T>,
-            NMapKey<Twox64Concat, NetworkIdOf<T>>,
+        BoundedBTreeMap<H256, WeaversBitmap<T>, T::MaxAuthorities>,
-            NMapKey<Twox64Concat, WeavingSession>,
-            NMapKey<Twox64Concat, WeavingSession>,
-            NMapKey<Twox64Concat, H256>,
-        ),
-        WeaversBitmap<T>,
         ValueQuery,
     >;
 
@@ -399,20 +400,46 @@ pub mod pallet {
                 Error::<T>::WeavingIsInactive,
             );
 
+            let authorities_len = Authorities::<T>::decode_len()
+                .unwrap_or(0);
+
             let root_hash = hash_attestation.root_hash;
             let next_session = hash_attestation.next_session;
             let weaving_session = hash_attestation.weaving_session();
 
+            let next_session_bytes = next_session.to_be_bytes();
+            let weaving_session_bytes = weaving_session.to_be_bytes();
+
+            let metadata: &[&[u8]] = &[
+                root_hash.as_ref(),
+                next_session_bytes.as_ref(),
+                weaving_session_bytes.as_ref(),
+            ];
+
+            let tapestry_strand_key =
+                sequential_hash::<SubstrateBlake2Hasher, _>(metadata);
+
             let weavers_count = TapestryStrands::<T>::try_mutate(
-                &(network_id, weaving_session, next_session, root_hash),
+                &network_id,
                 |tapestry_strand| -> Result<AuthIndex, DispatchError> {
+                    if !tapestry_strand.contains_key(&tapestry_strand_key) {
+                        let empty_bitmap = WeaversBitmap::<T>::empty(authorities_len);
+                        tapestry_strand
+                            .try_insert(tapestry_strand_key, empty_bitmap)
+                            .map_err(|_| Error::<T>::TooManyTapestries)?;
+                    }
+
+                    let tapestry_bitmap = tapestry_strand
+                        .get_mut(&tapestry_strand_key)
+                        .ok_or(Error::<T>::TooManyTapestries)?;
+
                     ensure!(
-                        !tapestry_strand.contains(authority_index),
+                        !tapestry_bitmap.contains(authority_index),
                         Error::<T>::AuthorityPartOfTrapestry,
                     );
 
-                    tapestry_strand.insert(authority_index);
+                    tapestry_bitmap.insert(authority_index);
-                    Ok(tapestry_strand.count_ones::<AuthIndex>())
+                    Ok(tapestry_bitmap.count_ones::<AuthIndex>())
                 },
             )?;
 
@@ -594,8 +621,9 @@ pub mod pallet {
                             .with_median_external(median_block)
                             .build();
 
+                        TapestryStrands::<T>::remove(&network_id);
                         TapestryDrafts::<T>::insert(&network_id, tapestry_draft);
-                        weight.saturating_accrue(T::DbWeight::get().writes(1));
+                        weight.saturating_accrue(T::DbWeight::get().writes(2));
                     }
                 }
             }
@@ -890,8 +918,19 @@ impl<T: Config> Pallet<T> {
                 network_id,
             ))?;
 
-        let tapestry_key = (network_id, weaving_session, first_next_session, first_hash);
+        let next_session_bytes = first_next_session.to_be_bytes();
-        let tapestry_strand = TapestryStrands::<T>::get(&tapestry_key);
+        let weaving_session_bytes = weaving_session.to_be_bytes();
+
+        let metadata: &[&[u8]] = &[
+            first_hash.as_ref(),
+            next_session_bytes.as_ref(),
+            weaving_session_bytes.as_ref(),
+        ];
+
+        let tapestry_strand_key =
+            sequential_hash::<SubstrateBlake2Hasher, _>(metadata);
+
+        let tapestry_strand = TapestryStrands::<T>::get(&network_id);
 
         let context = AttestationContext::default()
             .with_weaving_session(weaving_session)
@@ -899,7 +938,12 @@ impl<T: Config> Pallet<T> {
             .with_network_id(network_id);
 
         Self::local_authorities().for_each(|(authority_index, authority_key)| {
-            if tapestry_strand.contains(authority_index) {
+            let already_part_of_tapestry_strand = tapestry_strand
+                .get(&tapestry_strand_key)
+                .map(|bitmap| bitmap.contains(authority_index))
+                .unwrap_or(false);
+
+            if already_part_of_tapestry_strand {
                 log::error!(
                     target: LOG_TARGET,
                     "🕸️ Weft already weaved from authority #{:?} for network #{:?}.",
@@ -909,6 +953,23 @@ impl<T: Config> Pallet<T> {
                 return;
             }
 
+            let last_attested =
+                NetworkAttestedBlocks::<T>::get(&network_id, &authority_index);
+
+            let lock: BlockNumberFor<T> = LOCK_BLOCK_EXPIRATION.unique_saturated_into();
+            let cooling_off_until = last_attested.saturating_add(lock);
+
+            if current_block < cooling_off_until {
+                log::error!(
+                    target: LOG_TARGET,
+                    "🕸️ Weft is cooling for authority #{:?} for network #{:?} blocks left {:?}.",
+                    authority_index,
+                    network_id,
+                    cooling_off_until.saturating_sub(current_block)
+                );
+                return;
+            }
+
             let hash_attestation = context
                 .with_authority_index(authority_index)
                 .build_hash_attestation(first_hash, first_next_session);
@@ -1004,6 +1065,23 @@ impl<T: Config> Pallet<T> {
             .with_network_id(network_id);
 
         Self::local_authorities().for_each(|(authority_index, authority_key)| {
+            let last_attested =
+                NetworkAttestedBlocks::<T>::get(&network_id, &authority_index);
+
+            let lock: BlockNumberFor<T> = LOCK_BLOCK_EXPIRATION.unique_saturated_into();
+            let cooling_off_until = last_attested.saturating_add(lock);
+
+            if current_block < cooling_off_until {
+                log::error!(
+                    target: LOG_TARGET,
+                    "🕸️ Attestation is cooling for authority #{:?} for network #{:?} blocks left {:?}.",
+                    authority_index,
+                    network_id,
+                    cooling_off_until.saturating_sub(current_block)
+                );
+                return;
+            }
+
             let block_attestation = context
                 .with_authority_index(authority_index)
                 .build_block_attestation(finalized_block);

pallets/weaver/src/migrations/mod.rs

@@ -1,4 +1,5 @@
 pub mod v1;
+pub mod v2;
 
 pub type MigrateV0ToV1<T> = frame_support::migrations::VersionedMigration<
     0,
@@ -7,3 +8,11 @@ pub type MigrateV0ToV1<T> = frame_support::migrations::VersionedMigration<
 	crate::Pallet::<T>,
     <T as frame_system::Config>::DbWeight,
 >;
+
+pub type MigrateV1ToV2<T> = frame_support::migrations::VersionedMigration<
+    1,
+    2,
+    v2::UpdateTapestryStrandsStorageLayout<T>,
+	crate::Pallet::<T>,
+    <T as frame_system::Config>::DbWeight,
+>;

pallets/weaver/src/migrations/v2.rs

@@ -0,0 +1,33 @@
+use frame_support::{
+    traits::{Get, UncheckedOnRuntimeUpgrade},
+    weights::Weight,
+};
+
+use core::marker::PhantomData;
+use crate::{Config, LOG_TARGET};
+
+pub struct UpdateTapestryStrandsStorageLayout<T>(PhantomData<T>);
+impl<T: Config> UncheckedOnRuntimeUpgrade for UpdateTapestryStrandsStorageLayout<T> {
+    fn on_runtime_upgrade() -> Weight {
+        let clear_key = frame_support::storage::storage_prefix(
+            b"GhostWeaver",
+            b"TapestryStrands"
+        );
+
+        let removal_results = frame_support::storage::unhashed::clear_prefix(
+            &clear_key,
+            None,
+            None
+        );
+
+        let killed_keys = removal_results.unique;
+
+        log::info!(
+            target: LOG_TARGET,
+            "🧹 Success: Old StorageNMap for TapestryStrands wiped. Wiped {} keys from Trie.",
+            killed_keys
+        );
+
+        T::DbWeight::get().reads_writes(1, killed_keys as u64)
+    }
+}

runtime/casper/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "casper-runtime"
-version = "4.0.19"
+version = "4.0.20"
 build = "build.rs"
 description = "Runtime of the Casper Network"
 edition.workspace = true

runtime/casper/src/lib.rs

@@ -124,7 +124,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
     spec_name: create_runtime_str!("casper"),
     impl_name: create_runtime_str!("casper-svengali"),
     authoring_version: 0,
-    spec_version: 13,
+    spec_version: 14,
     impl_version: 0,
     apis: RUNTIME_API_VERSIONS,
     transaction_version: 1,
@@ -1037,6 +1037,7 @@ pub type SignedExtra = (
 /// Should be cleared after release.
 pub type Migrations = (
     ghost_weaver::migrations::MigrateV0ToV1<Runtime>,
+    ghost_weaver::migrations::MigrateV1ToV2<Runtime>,
 );
 
 /// Unchecked extrinsic type as expected by this runtime.