Skip to content

Commit

Permalink
Fix inconsistencies in node contracts states (#1010)
Browse files Browse the repository at this point in the history 
* fix: fix inconsistencies in node contracts states

* test: add tests for scenarios related to issue #1002
  • Loading branch information
@sameh-farouk
sameh-farouk authored Oct 2, 2024
1 parent cf46301 commit ac1b8d6
Show file tree
Hide file tree
Showing 12 changed files with 838 additions and 389 deletions.
12 changes: 6 additions & 6 deletions substrate-node/pallets/pallet-burning/src/weights.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
//! Autogenerated weights for pallet_burning
//!
//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev
//! DATE: 2024-09-24, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]`
//! DATE: 2024-10-01, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]`
//! WORST CASE MAP SIZE: `1000000`
//! HOSTNAME: `da081090f983`, CPU: `AMD Ryzen 7 5800X 8-Core Processor`
//! HOSTNAME: `585a9f003813`, CPU: `AMD Ryzen 7 5800X 8-Core Processor`
//! EXECUTION: , WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024
// Executed Command:
Expand Down Expand Up @@ -45,8 +45,8 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
// Proof Size summary in bytes:
// Measured: `109`
// Estimated: `1594`
// Minimum execution time: 26_741_000 picoseconds.
Weight::from_parts(27_212_000, 1594)
// Minimum execution time: 27_051_000 picoseconds.
Weight::from_parts(27_762_000, 1594)
.saturating_add(T::DbWeight::get().reads(1_u64))
.saturating_add(T::DbWeight::get().writes(1_u64))
}
Expand All @@ -60,8 +60,8 @@ impl WeightInfo for () {
// Proof Size summary in bytes:
// Measured: `109`
// Estimated: `1594`
// Minimum execution time: 26_741_000 picoseconds.
Weight::from_parts(27_212_000, 1594)
// Minimum execution time: 27_051_000 picoseconds.
Weight::from_parts(27_762_000, 1594)
.saturating_add(RocksDbWeight::get().reads(1_u64))
.saturating_add(RocksDbWeight::get().writes(1_u64))
}
Expand Down
36 changes: 18 additions & 18 deletions substrate-node/pallets/pallet-dao/src/weights.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
//! Autogenerated weights for pallet_dao
//!
//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev
//! DATE: 2024-09-24, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]`
//! DATE: 2024-10-01, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]`
//! WORST CASE MAP SIZE: `1000000`
//! HOSTNAME: `da081090f983`, CPU: `AMD Ryzen 7 5800X 8-Core Processor`
//! HOSTNAME: `585a9f003813`, CPU: `AMD Ryzen 7 5800X 8-Core Processor`
//! EXECUTION: , WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024
// Executed Command:
Expand Down Expand Up @@ -58,8 +58,8 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
// Proof Size summary in bytes:
// Measured: `208`
// Estimated: `4687`
// Minimum execution time: 19_888_000 picoseconds.
Weight::from_parts(20_368_000, 4687)
// Minimum execution time: 19_757_000 picoseconds.
Weight::from_parts(20_439_000, 4687)
.saturating_add(T::DbWeight::get().reads(4_u64))
.saturating_add(T::DbWeight::get().writes(5_u64))
}
Expand All @@ -77,8 +77,8 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
// Proof Size summary in bytes:
// Measured: `979`
// Estimated: `4444`
// Minimum execution time: 26_991_000 picoseconds.
Weight::from_parts(27_542_000, 4444)
// Minimum execution time: 26_691_000 picoseconds.
Weight::from_parts(27_332_000, 4444)
.saturating_add(T::DbWeight::get().reads(5_u64))
.saturating_add(T::DbWeight::get().writes(1_u64))
}
Expand All @@ -92,8 +92,8 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
// Proof Size summary in bytes:
// Measured: `487`
// Estimated: `4687`
// Minimum execution time: 18_766_000 picoseconds.
Weight::from_parts(19_256_000, 4687)
// Minimum execution time: 18_845_000 picoseconds.
Weight::from_parts(19_287_000, 4687)
.saturating_add(T::DbWeight::get().reads(3_u64))
.saturating_add(T::DbWeight::get().writes(1_u64))
}
Expand All @@ -111,8 +111,8 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
// Proof Size summary in bytes:
// Measured: `521`
// Estimated: `4687`
// Minimum execution time: 25_348_000 picoseconds.
Weight::from_parts(26_310_000, 4687)
// Minimum execution time: 25_638_000 picoseconds.
Weight::from_parts(26_180_000, 4687)
.saturating_add(T::DbWeight::get().reads(3_u64))
.saturating_add(T::DbWeight::get().writes(4_u64))
}
Expand All @@ -136,8 +136,8 @@ impl WeightInfo for () {
// Proof Size summary in bytes:
// Measured: `208`
// Estimated: `4687`
// Minimum execution time: 19_888_000 picoseconds.
Weight::from_parts(20_368_000, 4687)
// Minimum execution time: 19_757_000 picoseconds.
Weight::from_parts(20_439_000, 4687)
.saturating_add(RocksDbWeight::get().reads(4_u64))
.saturating_add(RocksDbWeight::get().writes(5_u64))
}
Expand All @@ -155,8 +155,8 @@ impl WeightInfo for () {
// Proof Size summary in bytes:
// Measured: `979`
// Estimated: `4444`
// Minimum execution time: 26_991_000 picoseconds.
Weight::from_parts(27_542_000, 4444)
// Minimum execution time: 26_691_000 picoseconds.
Weight::from_parts(27_332_000, 4444)
.saturating_add(RocksDbWeight::get().reads(5_u64))
.saturating_add(RocksDbWeight::get().writes(1_u64))
}
Expand All @@ -170,8 +170,8 @@ impl WeightInfo for () {
// Proof Size summary in bytes:
// Measured: `487`
// Estimated: `4687`
// Minimum execution time: 18_766_000 picoseconds.
Weight::from_parts(19_256_000, 4687)
// Minimum execution time: 18_845_000 picoseconds.
Weight::from_parts(19_287_000, 4687)
.saturating_add(RocksDbWeight::get().reads(3_u64))
.saturating_add(RocksDbWeight::get().writes(1_u64))
}
Expand All @@ -189,8 +189,8 @@ impl WeightInfo for () {
// Proof Size summary in bytes:
// Measured: `521`
// Estimated: `4687`
// Minimum execution time: 25_348_000 picoseconds.
Weight::from_parts(26_310_000, 4687)
// Minimum execution time: 25_638_000 picoseconds.
Weight::from_parts(26_180_000, 4687)
.saturating_add(RocksDbWeight::get().reads(3_u64))
.saturating_add(RocksDbWeight::get().writes(4_u64))
}
Expand Down
20 changes: 10 additions & 10 deletions substrate-node/pallets/pallet-kvstore/src/weights.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
//! Autogenerated weights for pallet_kvstore
//!
//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev
//! DATE: 2024-09-24, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]`
//! DATE: 2024-10-01, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]`
//! WORST CASE MAP SIZE: `1000000`
//! HOSTNAME: `da081090f983`, CPU: `AMD Ryzen 7 5800X 8-Core Processor`
//! HOSTNAME: `585a9f003813`, CPU: `AMD Ryzen 7 5800X 8-Core Processor`
//! EXECUTION: , WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024
// Executed Command:
Expand Down Expand Up @@ -46,8 +46,8 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
// Proof Size summary in bytes:
// Measured: `0`
// Estimated: `0`
// Minimum execution time: 7_054_000 picoseconds.
Weight::from_parts(7_274_000, 0)
// Minimum execution time: 6_903_000 picoseconds.
Weight::from_parts(7_153_000, 0)
.saturating_add(T::DbWeight::get().writes(1_u64))
}
/// Storage: `TFKVStore::TFKVStore` (r:1 w:1)
Expand All @@ -56,8 +56,8 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
// Proof Size summary in bytes:
// Measured: `146`
// Estimated: `3611`
// Minimum execution time: 12_263_000 picoseconds.
Weight::from_parts(12_824_000, 3611)
// Minimum execution time: 12_274_000 picoseconds.
Weight::from_parts(12_734_000, 3611)
.saturating_add(T::DbWeight::get().reads(1_u64))
.saturating_add(T::DbWeight::get().writes(1_u64))
}
Expand All @@ -71,8 +71,8 @@ impl WeightInfo for () {
// Proof Size summary in bytes:
// Measured: `0`
// Estimated: `0`
// Minimum execution time: 7_054_000 picoseconds.
Weight::from_parts(7_274_000, 0)
// Minimum execution time: 6_903_000 picoseconds.
Weight::from_parts(7_153_000, 0)
.saturating_add(RocksDbWeight::get().writes(1_u64))
}
/// Storage: `TFKVStore::TFKVStore` (r:1 w:1)
Expand All @@ -81,8 +81,8 @@ impl WeightInfo for () {
// Proof Size summary in bytes:
// Measured: `146`
// Estimated: `3611`
// Minimum execution time: 12_263_000 picoseconds.
Weight::from_parts(12_824_000, 3611)
// Minimum execution time: 12_274_000 picoseconds.
Weight::from_parts(12_734_000, 3611)
.saturating_add(RocksDbWeight::get().reads(1_u64))
.saturating_add(RocksDbWeight::get().writes(1_u64))
}
Expand Down
38 changes: 37 additions & 1 deletion substrate-node/pallets/pallet-smart-contract/src/billing.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -362,8 +362,14 @@ impl<T: Config> Pallet<T> {
has_sufficient_fund: bool,
current_block: u64,
) -> DispatchResultWithPostInfo {
// Add a defensive check to prevent a node contract on a rented node from transitioning to 'created' status without considering the state of the associated rent contract
let rent_is_suspended = matches!(contract.contract_type, types::ContractData::NodeContract(_))
&& ActiveRentContractForNode::<T>::get(contract.get_node_id())
.and_then(|id| Contracts::<T>::get(&id))
.map(|c| matches!(c.state, types::ContractState::GracePeriod(_)))
.unwrap_or(false);
match contract.state {
types::ContractState::GracePeriod(_) if has_sufficient_fund => {
types::ContractState::GracePeriod(_) if has_sufficient_fund && !rent_is_suspended => {
// Manage transition from GracePeriod to Created
log::info!("Contract {:?} is in grace period, but balance is recharged, moving to created state at block {:?}", contract.contract_id, current_block);
Self::update_contract_state(contract, &types::ContractState::Created)?;
Expand All @@ -386,6 +392,28 @@ impl<T: Config> Pallet<T> {
diff
);
if diff >= T::GracePeriod::get() {
// Ensure associated node contracts have no reserved balance.
// If none, proceed to move the contract to the 'deleted' state.
// Otherwise, wait.
match contract.contract_type {
types::ContractData::RentContract(_) => {
let active_node_contracts = ActiveNodeContracts::<T>::get(contract.get_node_id());
let rent_has_node_contracts_with_reserve =
active_node_contracts.iter().any(|id| {
ContractPaymentState::<T>::get(id).map_or(false, |s| s.has_reserve())
});

if rent_has_node_contracts_with_reserve {
log::debug!(
"Grace period on rented node expired, but one or more associated node contracts hold user funds. \
Rent contract deletion will be delayed until funds are distributed to beneficiaries."
);
return Ok(().into());
}
}
_ => (),
};

log::info!("Contract {:?} state changed to deleted at block {:?} due to an expired grace period.", contract.contract_id, current_block);
Self::deposit_event(Event::ContractGracePeriodElapsed {
contract_id: contract.contract_id,
Expand Down Expand Up @@ -734,6 +762,14 @@ impl<T: Config> Pallet<T> {
for ctr_id in active_node_contracts {
let mut ctr =
Contracts::<T>::get(ctr_id).ok_or(Error::<T>::ContractNotExists)?;

// Defensive check to prevent a node contract on a rented node from transitioning to 'created' status if there's an overdraft (unsettled public IP rent).
if ContractPaymentState::<T>::get(ctr_id).map_or_else(|| false, |ps| ps.has_overdraft())
&& matches!(state, types::ContractState::Created)
{
continue;
}

Self::update_contract_state(&mut ctr, &state)?;

match state {
Expand Down
46 changes: 31 additions & 15 deletions substrate-node/pallets/pallet-smart-contract/src/grid_contract.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,9 +272,12 @@ impl<T: Config> Pallet<T> {
// override values
contract.contract_type = types::ContractData::NodeContract(node_contract);

let state = contract.state.clone();
Self::update_contract_state(&mut contract, &state)?;

// Why we call update_contract_state if state can't be changed?
// Obviously, we just depened on the update_contract_state for writing the contract to the storage, but we can do it here
// let state = contract.state.clone();
// Self::update_contract_state(&mut contract, &state)?;
Contracts::<T>::insert(&contract.contract_id, contract.clone());

Self::deposit_event(Event::ContractUpdated(contract));

Ok(().into())
Expand Down Expand Up @@ -393,24 +396,35 @@ impl<T: Config> Pallet<T> {
fn remove_active_node_contract(node_id: u32, contract_id: u64) {
let mut contracts = ActiveNodeContracts::<T>::get(&node_id);

match contracts.iter().position(|id| id == &contract_id) {
Some(index) => {
contracts.remove(index);
}
None => (),
};
let initial_len = contracts.len();

contracts.retain(|&id| id != contract_id);

ActiveNodeContracts::<T>::insert(&node_id, &contracts);
// Only update the storage if the list length has changed, meaning a contract was removed
if contracts.len() != initial_len {
ActiveNodeContracts::<T>::insert(&node_id, contracts);
}
}

// Helper function that updates the contract state and manages storage accordingly
pub fn update_contract_state(
contract: &mut types::Contract<T>,
state: &types::ContractState,
) -> DispatchResultWithPostInfo {
// update the state and save the contract
contract.state = state.clone();
Contracts::<T>::insert(&contract.contract_id, contract.clone());
// Update the state and save the contract only when necessary.
if contract.state != *state {
match (&contract.state, state) {
// Transition from GracePeriod(x) to GracePeriod(y) is not allowed
(types::ContractState::GracePeriod(_), types::ContractState::GracePeriod(_)) => {
log::info!("Contract {:?} already in grace period, not changing to {:?} state", contract.contract_id, state);
},
// All other transitions are allowed
_ => {
contract.state = state.clone();
Contracts::<T>::insert(&contract.contract_id, contract.clone());
}
}
}

// if the contract is a name or rent contract, nothing to do left here
match contract.contract_type {
Expand All @@ -425,6 +439,7 @@ impl<T: Config> Pallet<T> {

let mut contracts = ActiveNodeContracts::<T>::get(&node_contract.node_id);

let original_len = contracts.len();
match contracts.iter().position(|id| id == &contract.contract_id) {
Some(index) => {
// if the new contract state is delete, remove the contract id from the map
Expand All @@ -439,8 +454,9 @@ impl<T: Config> Pallet<T> {
}
}
};

ActiveNodeContracts::<T>::insert(&node_contract.node_id, &contracts);
if contracts.len() != original_len {
ActiveNodeContracts::<T>::insert(&node_contract.node_id, &contracts);
};

Ok(().into())
}
Expand Down
Loading

0 comments on commit ac1b8d6

Please sign in to comment.