From 3a9f332656b1b6244fee7e2f599f5605d07d7c29 Mon Sep 17 00:00:00 2001 From: Erwan Renaut <73958772+renauter@users.noreply.github.com> Date: Wed, 24 Jan 2024 13:20:32 -0300 Subject: [PATCH] feat(pallet-tfgrid): rework change power target on node (#934) --- clients/tfchain-client-go/utils.go | 1 + .../0019-rework_change_power_target.md | 15 ++++ substrate-node/pallets/pallet-dao/src/mock.rs | 10 ++- .../src/grid_contract.rs | 14 +++- .../pallets/pallet-smart-contract/src/mock.rs | 12 ++- .../pallet-smart-contract/src/tests.rs | 78 +++++++++++++++++++ .../pallets/pallet-tfgrid/src/lib.rs | 5 +- .../pallets/pallet-tfgrid/src/mock.rs | 21 +++-- .../pallets/pallet-tfgrid/src/node.rs | 27 +++---- substrate-node/runtime/src/lib.rs | 10 ++- substrate-node/support/src/traits.rs | 4 + 11 files changed, 170 insertions(+), 27 deletions(-) create mode 100644 docs/architecture/0019-rework_change_power_target.md diff --git a/clients/tfchain-client-go/utils.go b/clients/tfchain-client-go/utils.go index 1ec6c3356..1151a489d 100644 --- 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 new file mode 100644 index 000000000..d30252fe8 --- /dev/null +++ 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 index a4fd93313..f11eac99a 100644 --- 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; @@ -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; 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 index c6ef076d9..e8bee251e 100644 --- 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 Pallet { let rent_contract = Self::get_rent_contract(&contract)?; let active_node_contracts = ActiveNodeContracts::::get(rent_contract.node_id); ensure!( - active_node_contracts.len() == 0, + active_node_contracts.is_empty(), Error::::NodeHasActiveContracts ); } @@ -657,8 +657,7 @@ impl Pallet { // Make sure there is no active node or rent contract on this node ensure!( - ActiveRentContractForNode::::get(node_id).is_none() - && ActiveNodeContracts::::get(&node_id).is_empty(), + Self::node_has_no_active_contracts(node_id), Error::::NodeHasActiveContracts ); @@ -735,6 +734,13 @@ impl ChangeNode, InterfaceOf, SerialNumberOf> for } } +impl NodeActiveContracts for Pallet { + fn node_has_no_active_contracts(node_id: u32) -> bool { + ActiveNodeContracts::::get(node_id).is_empty() + && ActiveRentContractForNode::::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 index a970dbf30..e19775bb9 100644 --- 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, Storage, Event}, Balances: pallet_balances::{Pallet, Call, Storage, Config, Event}, - TfgridModule: pallet_tfgrid::{Pallet, Call, Storage, Event}, + TfgridModule: pallet_tfgrid::{Pallet, Call, Storage, Event, Error}, Timestamp: pallet_timestamp::{Pallet, Call, Storage, Inherent}, SmartContractModule: pallet_smart_contract::{Pallet, Call, Storage, Event}, TFTPriceModule: pallet_tft_price::{Pallet, Call, Storage, Event}, @@ -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; 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 index bf32f510e..82ee73c40 100644 --- 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::::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::::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 index 2888d5e14..1fea9fcca 100644 --- 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 index 0f7372fa8..39eaa9534 100644 --- 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; pub(crate) type TfgridNode = crate::TfgridNode; pub struct NodeChanged; -impl tfchain_support::traits::ChangeNode for NodeChanged { +impl ChangeNode 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; type WeightInfo = weights::SubstrateWeight; 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 index a28fb15a0..2a219319b 100644 --- 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 Pallet { let mut node_power = NodePower::::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::::insert(node_id, node_power); @@ -361,31 +361,32 @@ impl Pallet { let twin_id = TwinIdByAccountID::::get(account_id).ok_or(Error::::TwinNotExists)?; let node = Nodes::::get(node_id).ok_or(Error::::NodeNotExists)?; let farm = Farms::::get(node.farm_id).ok_or(Error::::FarmNotExists)?; - ensure!( - twin_id == farm.twin_id, - Error::::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::::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::::NodeHasActiveContracts + ); + } - fn _change_power_target_on_node(node_id: u32, farm_id: u32, power_target: Power) { let mut node_power = NodePower::::get(node_id); node_power.target = power_target.clone(); NodePower::::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 index a395657fe..694786b73 100644 --- 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; type NodeChanged = NodeChanged; type PublicIpModifier = SmartContractModule; + type NodeActiveContracts = NodeActiveContractsType; type TermsAndConditions = pallet_tfgrid::terms_cond::TermsAndConditions; type MaxFarmNameLength = MaxFarmNameLength; type MaxFarmPublicIps = MaxFarmPublicIps; diff --git a/substrate-node/support/src/traits.rs b/substrate-node/support/src/traits.rs index 582ea3cfa..0050c4bb2 100644 --- a/substrate-node/support/src/traits.rs +++ b/substrate-node/support/src/traits.rs @@ -15,3 +15,7 @@ pub trait ChangeNode { pub trait PublicIpModifier { fn ip_removed(ip: &PublicIP); } + +pub trait NodeActiveContracts { + fn node_has_no_active_contracts(node_id: u32) -> bool; +}