feat(pallet-tfgrid): rework change power target on node (#934)

threefoldtech · Jan 24, 2024 · 3a9f332 · 3a9f332
1 parent 4f03e93
commit 3a9f332
Show file tree

Hide file tree

Showing 11 changed files with 170 additions and 27 deletions.
diff --git a/clients/tfchain-client-go/utils.go b/clients/tfchain-client-go/utils.go
@@ -243,6 +243,7 @@ var tfgridModuleErrors = []string{
 	"InvalidDocumentHashInput",
 	"InvalidPublicConfig",
 	"UnauthorizedToChangePowerTarget",
+	"NodeHasActiveContracts",
 	"InvalidRelayAddress",
 	"InvalidTimestampHint",
 }

diff --git a/docs/architecture/0019-rework_change_power_target.md b/docs/architecture/0019-rework_change_power_target.md
@@ -0,0 +1,15 @@
+# 19. Rework change power target on node
+
+Date: 2024-01-09
+
+## Status
+
+Accepted
+
+## Context
+
+See [here](https://github.com/threefoldtech/tfchain/issues/924) for more details.
+
+## Decision
+
+Make sure that node has no active contracts on it to be able to change its `PowerTarget` to `Down` when calling `change_power_target()` extrinsic.
diff --git a/substrate-node/pallets/pallet-dao/src/mock.rs b/substrate-node/pallets/pallet-dao/src/mock.rs
@@ -17,7 +17,7 @@ use sp_runtime::{
     BuildStorage,
 };
 use sp_std::convert::{TryFrom, TryInto};
-use tfchain_support::traits::{ChangeNode, PublicIpModifier};
+use tfchain_support::traits::{ChangeNode, NodeActiveContracts, PublicIpModifier};
 use tfchain_support::types::PublicIP;
 
 type Block = frame_system::mocking::MockBlock<TestRuntime>;
@@ -94,6 +94,13 @@ impl PublicIpModifier for PublicIpModifierType {
     fn ip_removed(_ip: &PublicIP) {}
 }
 
+pub struct NodeActiveContractsType;
+impl NodeActiveContracts for NodeActiveContractsType {
+    fn node_has_no_active_contracts(_node_id: u32) -> bool {
+        true
+    }
+}
+
 use crate::weights;
 impl pallet_dao::pallet::Config for TestRuntime {
     type RuntimeEvent = RuntimeEvent;
@@ -132,6 +139,7 @@ impl pallet_tfgrid::Config for TestRuntime {
     type WeightInfo = pallet_tfgrid::weights::SubstrateWeight<TestRuntime>;
     type NodeChanged = NodeChanged;
     type PublicIpModifier = PublicIpModifierType;
+    type NodeActiveContracts = NodeActiveContractsType;
     type TermsAndConditions = TestTermsAndConditions;
     type FarmName = TestFarmName;
     type MaxFarmNameLength = MaxFarmNameLength;

diff --git a/substrate-node/pallets/pallet-smart-contract/src/grid_contract.rs b/substrate-node/pallets/pallet-smart-contract/src/grid_contract.rs
@@ -9,7 +9,7 @@ use pallet_tfgrid::pallet::{InterfaceOf, LocationOf, SerialNumberOf, TfgridNode}
 use parity_scale_codec::{Decode, Encode, MaxEncodedLen};
 use sp_std::{marker::PhantomData, vec, vec::Vec};
 use tfchain_support::{
-    traits::{ChangeNode, PublicIpModifier},
+    traits::{ChangeNode, NodeActiveContracts, PublicIpModifier},
     types::PublicIP,
 };
 
@@ -315,7 +315,7 @@ impl<T: Config> Pallet<T> {
             let rent_contract = Self::get_rent_contract(&contract)?;
             let active_node_contracts = ActiveNodeContracts::<T>::get(rent_contract.node_id);
             ensure!(
-                active_node_contracts.len() == 0,
+                active_node_contracts.is_empty(),
                 Error::<T>::NodeHasActiveContracts
             );
         }
@@ -657,8 +657,7 @@ impl<T: Config> Pallet<T> {
 
         // Make sure there is no active node or rent contract on this node
         ensure!(
-            ActiveRentContractForNode::<T>::get(node_id).is_none()
-                && ActiveNodeContracts::<T>::get(&node_id).is_empty(),
+            Self::node_has_no_active_contracts(node_id),
             Error::<T>::NodeHasActiveContracts
         );
 
@@ -735,6 +734,13 @@ impl<T: Config> ChangeNode<LocationOf<T>, InterfaceOf<T>, SerialNumberOf<T>> for
     }
 }
 
+impl<T: Config> NodeActiveContracts for Pallet<T> {
+    fn node_has_no_active_contracts(node_id: u32) -> bool {
+        ActiveNodeContracts::<T>::get(node_id).is_empty()
+            && ActiveRentContractForNode::<T>::get(node_id).is_none()
+    }
+}
+
 /// A Name Contract Name.
 #[derive(Encode, Decode, RuntimeDebugNoBound, TypeInfo, MaxEncodedLen)]
 #[scale_info(skip_type_params(T))]

diff --git a/substrate-node/pallets/pallet-smart-contract/src/mock.rs b/substrate-node/pallets/pallet-smart-contract/src/mock.rs
@@ -46,7 +46,7 @@ use sp_std::{
 use std::{cell::RefCell, panic, thread};
 use tfchain_support::{
     constants::time::{MINUTES, SECS_PER_HOUR},
-    traits::{ChangeNode, PublicIpModifier},
+    traits::{ChangeNode, NodeActiveContracts, PublicIpModifier},
     types::PublicIP,
 };
 
@@ -103,7 +103,7 @@ construct_runtime!(
     {
         System: frame_system::{Pallet, Call, Config<T>, Storage, Event<T>},
         Balances: pallet_balances::{Pallet, Call, Storage, Config<T>, Event<T>},
-        TfgridModule: pallet_tfgrid::{Pallet, Call, Storage, Event<T>},
+        TfgridModule: pallet_tfgrid::{Pallet, Call, Storage, Event<T>, Error<T>},
         Timestamp: pallet_timestamp::{Pallet, Call, Storage, Inherent},
         SmartContractModule: pallet_smart_contract::{Pallet, Call, Storage, Event<T>},
         TFTPriceModule: pallet_tft_price::{Pallet, Call, Storage, Event<T>},
@@ -195,6 +195,13 @@ impl PublicIpModifier for PublicIpModifierType {
     }
 }
 
+pub struct NodeActiveContractsType;
+impl NodeActiveContracts for NodeActiveContractsType {
+    fn node_has_no_active_contracts(node_id: u32) -> bool {
+        SmartContractModule::node_has_no_active_contracts(node_id)
+    }
+}
+
 parameter_types! {
     pub const MaxFarmNameLength: u32 = 40;
     pub const MaxInterfaceIpsLength: u32 = 5;
@@ -222,6 +229,7 @@ impl pallet_tfgrid::Config for TestRuntime {
     type WeightInfo = pallet_tfgrid::weights::SubstrateWeight<TestRuntime>;
     type NodeChanged = NodeChanged;
     type PublicIpModifier = PublicIpModifierType;
+    type NodeActiveContracts = NodeActiveContractsType;
     type TermsAndConditions = TestTermsAndConditions;
     type FarmName = TestFarmName;
     type MaxFarmNameLength = MaxFarmNameLength;

diff --git a/substrate-node/pallets/pallet-smart-contract/src/tests.rs b/substrate-node/pallets/pallet-smart-contract/src/tests.rs
@@ -74,6 +74,33 @@ fn test_create_node_contract_on_standby_node_fails() {
     });
 }
 
+#[test]
+fn test_create_node_contract_and_switch_node_to_standby_fails() {
+    new_test_ext().execute_with(|| {
+        run_to_block(1, None);
+        prepare_farm_and_node();
+        let node_id = 1;
+
+        assert_ok!(SmartContractModule::create_node_contract(
+            RuntimeOrigin::signed(bob()),
+            node_id,
+            generate_deployment_hash(),
+            get_deployment_data(),
+            0,
+            None
+        ));
+
+        assert_noop!(
+            TfgridModule::change_power_target(
+                RuntimeOrigin::signed(alice()),
+                node_id,
+                tfchain_support::types::Power::Down
+            ),
+            pallet_tfgrid::Error::<TestRuntime>::NodeHasActiveContracts
+        );
+    });
+}
+
 #[test]
 fn test_create_node_contract_with_public_ips_works() {
     new_test_ext().execute_with(|| {
@@ -688,6 +715,57 @@ fn test_create_rent_contract_on_standby_node_works() {
     });
 }
 
+#[test]
+fn test_create_rent_contract_and_switch_node_to_standby_fails() {
+    new_test_ext().execute_with(|| {
+        run_to_block(1, None);
+        prepare_dedicated_farm_and_node();
+        let node_id = 1;
+
+        assert_ok!(SmartContractModule::create_rent_contract(
+            RuntimeOrigin::signed(bob()),
+            node_id,
+            None
+        ));
+
+        assert_noop!(
+            TfgridModule::change_power_target(
+                RuntimeOrigin::signed(alice()),
+                node_id,
+                tfchain_support::types::Power::Down
+            ),
+            pallet_tfgrid::Error::<TestRuntime>::NodeHasActiveContracts
+        );
+    });
+}
+
+#[test]
+fn test_create_rent_contract_on_standby_node_and_wake_it_up_works() {
+    new_test_ext().execute_with(|| {
+        run_to_block(1, None);
+        prepare_dedicated_farm_and_node();
+        let node_id = 1;
+
+        assert_ok!(TfgridModule::change_power_target(
+            RuntimeOrigin::signed(alice()),
+            node_id,
+            tfchain_support::types::Power::Down,
+        ));
+
+        assert_ok!(SmartContractModule::create_rent_contract(
+            RuntimeOrigin::signed(bob()),
+            node_id,
+            None
+        ));
+
+        assert_ok!(TfgridModule::change_power_target(
+            RuntimeOrigin::signed(alice()),
+            node_id,
+            tfchain_support::types::Power::Up,
+        ));
+    });
+}
+
 #[test]
 fn test_cancel_rent_contract_works() {
     new_test_ext().execute_with(|| {

diff --git a/substrate-node/pallets/pallet-tfgrid/src/lib.rs b/substrate-node/pallets/pallet-tfgrid/src/lib.rs
@@ -41,7 +41,7 @@ pub mod pallet {
     use sp_std::{convert::TryInto, fmt::Debug, vec, vec::Vec};
     use tfchain_support::{
         resources::Resources,
-        traits::{ChangeNode, PublicIpModifier},
+        traits::{ChangeNode, NodeActiveContracts, PublicIpModifier},
         types::*,
     };
 
@@ -281,6 +281,8 @@ pub mod pallet {
 
         type PublicIpModifier: PublicIpModifier;
 
+        type NodeActiveContracts: NodeActiveContracts;
+
         /// The type of terms and conditions.
         type TermsAndConditions: FullCodec
             + Debug
@@ -567,6 +569,7 @@ pub mod pallet {
 
         InvalidPublicConfig,
         UnauthorizedToChangePowerTarget,
+        NodeHasActiveContracts,
         InvalidRelayAddress,
         InvalidTimestampHint,
     }

diff --git a/substrate-node/pallets/pallet-tfgrid/src/mock.rs b/substrate-node/pallets/pallet-tfgrid/src/mock.rs
@@ -20,7 +20,10 @@ use sp_runtime::{
 use sp_std::prelude::*;
 
 use hex;
-use tfchain_support::types::PublicIP;
+use tfchain_support::{
+    traits::{ChangeNode, NodeActiveContracts, PublicIpModifier},
+    types::PublicIP,
+};
 
 pub type Signature = MultiSignature;
 
@@ -79,17 +82,24 @@ pub(crate) type Interface = crate::InterfaceOf<TestRuntime>;
 pub(crate) type TfgridNode = crate::TfgridNode<TestRuntime>;
 
 pub struct NodeChanged;
-impl tfchain_support::traits::ChangeNode<Loc, Interface, Serial> for NodeChanged {
+impl ChangeNode<Loc, Interface, Serial> for NodeChanged {
     fn node_changed(_old_node: Option<&TfgridNode>, _new_node: &TfgridNode) {}
     fn node_deleted(_node: &TfgridNode) {}
     fn node_power_state_changed(_node: &TfgridNode) {}
 }
 
-pub struct PublicIpModifier;
-impl tfchain_support::traits::PublicIpModifier for PublicIpModifier {
+pub struct PublicIpModifierType;
+impl PublicIpModifier for PublicIpModifierType {
     fn ip_removed(_ip: &PublicIP) {}
 }
 
+pub struct NodeActiveContractsType;
+impl NodeActiveContracts for NodeActiveContractsType {
+    fn node_has_no_active_contracts(_node_id: u32) -> bool {
+        true
+    }
+}
+
 parameter_types! {
     pub const MaxFarmNameLength: u32 = 40;
     pub const MaxInterfaceIpsLength: u32 = 5;
@@ -116,7 +126,8 @@ impl Config for TestRuntime {
     type RestrictedOrigin = EnsureRoot<Self::AccountId>;
     type WeightInfo = weights::SubstrateWeight<TestRuntime>;
     type NodeChanged = NodeChanged;
-    type PublicIpModifier = PublicIpModifier;
+    type PublicIpModifier = PublicIpModifierType;
+    type NodeActiveContracts = NodeActiveContractsType;
     type TermsAndConditions = TestTermsAndConditions;
     type FarmName = TestFarmName;
     type MaxFarmNameLength = MaxFarmNameLength;

diff --git a/substrate-node/pallets/pallet-tfgrid/src/node.rs b/substrate-node/pallets/pallet-tfgrid/src/node.rs
@@ -13,7 +13,7 @@ use sp_std::marker::PhantomData;
 use sp_std::{vec, vec::Vec};
 use tfchain_support::{
     resources::Resources,
-    traits::ChangeNode,
+    traits::{ChangeNode, NodeActiveContracts},
     types::{Interface, Node, NodeCertification, Power, PowerState, PublicConfig},
 };
 
@@ -335,7 +335,7 @@ impl<T: Config> Pallet<T> {
 
         let mut node_power = NodePower::<T>::get(node_id);
 
-        // if the power state is different from what is set, change it and emit event
+        // If the power state is different from what is set, change it and emit event
         if node_power.state != power_state {
             node_power.state = power_state.clone();
             NodePower::<T>::insert(node_id, node_power);
@@ -361,31 +361,32 @@ impl<T: Config> Pallet<T> {
         let twin_id = TwinIdByAccountID::<T>::get(account_id).ok_or(Error::<T>::TwinNotExists)?;
         let node = Nodes::<T>::get(node_id).ok_or(Error::<T>::NodeNotExists)?;
         let farm = Farms::<T>::get(node.farm_id).ok_or(Error::<T>::FarmNotExists)?;
-        ensure!(
-            twin_id == farm.twin_id,
-            Error::<T>::UnauthorizedToChangePowerTarget
-        );
+
         // Make sure only the farmer that owns this node can change the power target
         ensure!(
-            node.farm_id == farm.id,
+            twin_id == farm.twin_id,
             Error::<T>::UnauthorizedToChangePowerTarget
         );
 
-        Self::_change_power_target_on_node(node.id, node.farm_id, power_target);
-
-        Ok(().into())
-    }
+        // If power target is switched to Down, make sure there are no active contracts on node
+        if power_target == Power::Down {
+            ensure!(
+                T::NodeActiveContracts::node_has_no_active_contracts(node_id),
+                Error::<T>::NodeHasActiveContracts
+            );
+        }
 
-    fn _change_power_target_on_node(node_id: u32, farm_id: u32, power_target: Power) {
         let mut node_power = NodePower::<T>::get(node_id);
         node_power.target = power_target.clone();
         NodePower::<T>::insert(node_id, &node_power);
 
         Self::deposit_event(Event::PowerTargetChanged {
-            farm_id,
+            farm_id: node.farm_id,
             node_id,
             power_target,
         });
+
+        Ok(().into())
     }
 
     fn get_resources(

diff --git a/substrate-node/runtime/src/lib.rs b/substrate-node/runtime/src/lib.rs
@@ -28,7 +28,7 @@ use sp_version::NativeVersion;
 use sp_version::RuntimeVersion;
 use tfchain_support::{
     constants::time::*,
-    traits::{ChangeNode, PublicIpModifier},
+    traits::{ChangeNode, NodeActiveContracts, PublicIpModifier},
     types::PublicIP,
 };
 
@@ -343,6 +343,13 @@ impl PublicIpModifier for PublicIpModifierType {
     }
 }
 
+pub struct NodeActiveContractsType;
+impl NodeActiveContracts for NodeActiveContractsType {
+    fn node_has_no_active_contracts(node_id: u32) -> bool {
+        SmartContractModule::node_has_no_active_contracts(node_id)
+    }
+}
+
 parameter_types! {
     pub const MaxFarmNameLength: u32 = 40;
     pub const MaxInterfaceIpsLength: u32 = 10;
@@ -357,6 +364,7 @@ impl pallet_tfgrid::Config for Runtime {
     type WeightInfo = pallet_tfgrid::weights::SubstrateWeight<Runtime>;
     type NodeChanged = NodeChanged;
     type PublicIpModifier = SmartContractModule;
+    type NodeActiveContracts = NodeActiveContractsType;
     type TermsAndConditions = pallet_tfgrid::terms_cond::TermsAndConditions<Runtime>;
     type MaxFarmNameLength = MaxFarmNameLength;
     type MaxFarmPublicIps = MaxFarmPublicIps;