refactor: Consolidate V3 opt-out metadata events and fix the order of new storage, events and errors declarations

Author: sameh-farouk

substrate-node/pallets/pallet-tfgrid/src/benchmarking.rs

@@ -873,7 +873,7 @@ benchmarks! {
     verify {
         let stored = TfgridModule::<T>::node_v3_opt_out_metadata(node_id).unwrap();
         assert_eq!(stored.to_vec(), metadata);
-        assert_has_event::<T>(Event::NodeV3OptOutMetadataSet { node_id, metadata }.into());
+        assert_has_event::<T>(Event::NodeV3OptOutMetadataUpdated { node_id, metadata: Some(metadata) }.into());
     }
 
     // add_twin_admin()

substrate-node/pallets/pallet-tfgrid/src/lib.rs

@@ -254,26 +254,6 @@ pub mod pallet {
     #[pallet::getter(fn zos_version)]
     pub type ZosVersion<T> = StorageValue<_, Vec<u8>, ValueQuery>;
 
-    // Keyed by node_id. Present = node has opted out of v3 billing.
-    #[pallet::storage]
-    #[pallet::getter(fn node_v3_billing_opt_out)]
-    pub type NodeV3BillingOptOut<T> =
-        StorageMap<_, Blake2_128Concat, u32, u64, OptionQuery>;
-
-    // Keyed by node_id. Stores optional metadata for opted-out nodes (e.g. v4 account).
-    // Max 256 bytes of arbitrary UTF-8 content. Node must be opted out before metadata can be set.
-    #[pallet::storage]
-    #[pallet::getter(fn node_v3_opt_out_metadata)]
-    pub type NodeV3OptOutMetadata<T> =
-        StorageMap<_, Blake2_128Concat, u32, BoundedVec<u8, ConstU32<256>>, OptionQuery>;
-
-    // Global list of accounts authorized to deploy on opted-out nodes.
-    // None = list not initialized (treat as empty = no one allowed).
-    // Bounded to prevent unbounded storage growth.
-    #[pallet::storage]
-    #[pallet::getter(fn allowed_twin_admins)]
-    pub type AllowedTwinAdmins<T: Config> =
-        StorageValue<_, BoundedVec<T::AccountId, T::MaxTwinAdmins>, OptionQuery>;
 
     // This storage map maps a node ID to a power state, they node can modify this state
     // to indicate that it has shut down or came back alive
@@ -312,6 +292,27 @@ pub mod pallet {
     pub type PendingTransferByTwin<T: Config> =
         StorageMap<_, Blake2_128Concat, u32, u64, OptionQuery>;
 
+    // Keyed by node_id. Present = node has opted out of v3 billing.
+    #[pallet::storage]
+    #[pallet::getter(fn node_v3_billing_opt_out)]
+    pub type NodeV3BillingOptOut<T> =
+        StorageMap<_, Blake2_128Concat, u32, u64, OptionQuery>;
+
+    // Keyed by node_id. Stores optional metadata for opted-out nodes (e.g. v4 account).
+    // Max 256 bytes of arbitrary UTF-8 content. Node must be opted out before metadata can be set.
+    #[pallet::storage]
+    #[pallet::getter(fn node_v3_opt_out_metadata)]
+    pub type NodeV3OptOutMetadata<T> =
+        StorageMap<_, Blake2_128Concat, u32, BoundedVec<u8, ConstU32<256>>, OptionQuery>;
+
+    // Global list of accounts authorized to deploy on opted-out nodes.
+    // None = list not initialized (treat as empty = no one allowed).
+    // Bounded to prevent unbounded storage growth.
+    #[pallet::storage]
+    #[pallet::getter(fn allowed_twin_admins)]
+    pub type AllowedTwinAdmins<T: Config> =
+        StorageValue<_, BoundedVec<T::AccountId, T::MaxTwinAdmins>, OptionQuery>;
+
     #[pallet::config]
     pub trait Config: frame_system::Config + pallet_timestamp::Config {
         type RuntimeEvent: From<Event<Self>> + IsType<<Self as frame_system::Config>::RuntimeEvent>;
@@ -494,12 +495,6 @@ pub mod pallet {
             power_state: PowerState<BlockNumberFor<T>>,
         },
 
-        // V3 billing opt-out
-        NodeV3BillingOptedOut { node_id: u32, opted_out_at: u64 },
-        NodeV3OptOutMetadataSet { node_id: u32, metadata: Vec<u8> },
-        NodeV3OptOutMetadataCleared { node_id: u32 },
-        TwinAdminAdded(T::AccountId),
-        TwinAdminRemoved(T::AccountId),
 
         // Twin ownership transfer lifecycle
         TwinTransferRequested {
@@ -520,6 +515,12 @@ pub mod pallet {
             from: T::AccountId,
             to: T::AccountId,
         },
+
+        // V3 billing opt-out
+        NodeV3BillingOptedOut { node_id: u32, opted_out_at: u64 },
+        TwinAdminAdded(T::AccountId),
+        TwinAdminRemoved(T::AccountId),
+        NodeV3OptOutMetadataUpdated { node_id: u32, metadata: Option<Vec<u8>> },
     }
 
     #[pallet::error]
@@ -664,11 +665,11 @@ pub mod pallet {
 
         // V3 billing opt-out errors
         NodeV3BillingOptOutAlreadyEnabled,
-        NodeNotOptedOutOfV3Billing,
-        NodeV3OptOutMetadataTooLong,
         AlreadyTwinAdmin,
         NotTwinAdmin,
         TwinAdminListFull,
+        NodeNotOptedOutOfV3Billing,
+        NodeV3OptOutMetadataTooLong,
     }
 
     #[pallet::genesis_config]
@@ -1375,20 +1376,6 @@ pub mod pallet {
             Self::_opt_out_of_v3_billing(account_id, node_id)
         }
 
-        // Farmer sets metadata for an opted-out node (e.g. v4 account address).
-        // Node must already be opted out. Caller must be the farm owner.
-        // Pass empty metadata to clear.
-        #[pallet::call_index(46)]
-        #[pallet::weight(<T as Config>::WeightInfo::set_node_v3_opt_out_metadata())]
-        pub fn set_node_v3_opt_out_metadata(
-            origin: OriginFor<T>,
-            node_id: u32,
-            metadata: Vec<u8>,
-        ) -> DispatchResultWithPostInfo {
-            let account_id = ensure_signed(origin)?;
-            Self::_set_node_v3_opt_out_metadata(account_id, node_id, metadata)
-        }
-
         // Council adds an account to the twin admin list (allowed to deploy on opted-out nodes)
         #[pallet::call_index(44)]
         #[pallet::weight(<T as Config>::WeightInfo::add_twin_admin())]
@@ -1410,5 +1397,19 @@ pub mod pallet {
             T::RestrictedOrigin::ensure_origin(origin)?;
             Self::_remove_twin_admin(account)
         }
+
+        // Farmer sets metadata for an opted-out node (e.g. v4 account address).
+        // Node must already be opted out. Caller must be the farm owner.
+        // Pass empty metadata to clear.
+        #[pallet::call_index(46)]
+        #[pallet::weight(<T as Config>::WeightInfo::set_node_v3_opt_out_metadata())]
+        pub fn set_node_v3_opt_out_metadata(
+            origin: OriginFor<T>,
+            node_id: u32,
+            metadata: Vec<u8>,
+        ) -> DispatchResultWithPostInfo {
+            let account_id = ensure_signed(origin)?;
+            Self::_set_node_v3_opt_out_metadata(account_id, node_id, metadata)
+        }
     }
 }

substrate-node/pallets/pallet-tfgrid/src/tests.rs

@@ -2948,9 +2948,9 @@ fn test_set_node_v3_opt_out_metadata_works() {
         let our_events = System::events();
         assert!(our_events.iter().any(|e| matches!(
             &e.event,
-            MockEvent::TfgridModule(TfgridEvent::<TestRuntime>::NodeV3OptOutMetadataSet {
+            MockEvent::TfgridModule(TfgridEvent::<TestRuntime>::NodeV3OptOutMetadataUpdated {
                 node_id: 1,
-                ..
+                metadata: Some(_),
             })
         )));
     });
@@ -2988,8 +2988,9 @@ fn test_set_node_v3_opt_out_metadata_clear_works() {
         let our_events = System::events();
         assert!(our_events.iter().any(|e| matches!(
             &e.event,
-            MockEvent::TfgridModule(TfgridEvent::<TestRuntime>::NodeV3OptOutMetadataCleared {
+            MockEvent::TfgridModule(TfgridEvent::<TestRuntime>::NodeV3OptOutMetadataUpdated {
                 node_id: 1,
+                metadata: None,
             })
         )));
     });

substrate-node/pallets/pallet-tfgrid/src/v3_billing_opt_out.rs

@@ -66,12 +66,12 @@ impl<T: Config> crate::Pallet<T> {
 
         if metadata.is_empty() {
             NodeV3OptOutMetadata::<T>::remove(node_id);
-            Self::deposit_event(Event::NodeV3OptOutMetadataCleared { node_id });
+            Self::deposit_event(Event::NodeV3OptOutMetadataUpdated { node_id, metadata: None });
         } else {
             let bounded: BoundedVec<u8, frame_support::traits::ConstU32<256>> =
                 metadata.clone().try_into().map_err(|_| Error::<T>::NodeV3OptOutMetadataTooLong)?;
             NodeV3OptOutMetadata::<T>::insert(node_id, bounded);
-            Self::deposit_event(Event::NodeV3OptOutMetadataSet { node_id, metadata });
+            Self::deposit_event(Event::NodeV3OptOutMetadataUpdated { node_id, metadata: Some(metadata) });
         }
 
         Ok(().into())