Skip to content

Commit ac1b8d6

Browse files
sameh-farouksameh-farouk
authored
Fix inconsistencies in node contracts states (#1010)
* fix: fix inconsistencies in node contracts states * test: add tests for scenarios related to issue #1002
1 parent cf46301 commit ac1b8d6

File tree

12 files changed

+838
-389
lines changed

12 files changed

+838
-389
lines changed

‎substrate-node/pallets/pallet-burning/src/weights.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@
22
//! Autogenerated weights for pallet_burning
33
//!
44
//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev
5-
//! DATE: 2024-09-24, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]`
5+
//! DATE: 2024-10-01, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]`
66
//! WORST CASE MAP SIZE: `1000000`
7-
//! HOSTNAME: `da081090f983`, CPU: `AMD Ryzen 7 5800X 8-Core Processor`
7+
//! HOSTNAME: `585a9f003813`, CPU: `AMD Ryzen 7 5800X 8-Core Processor`
88
//! EXECUTION: , WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024
99
1010
// Executed Command:
@@ -45,8 +45,8 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
4545
// Proof Size summary in bytes:
4646
// Measured: `109`
4747
// Estimated: `1594`
48-
// Minimum execution time: 26_741_000 picoseconds.
49-
Weight::from_parts(27_212_000, 1594)
48+
// Minimum execution time: 27_051_000 picoseconds.
49+
Weight::from_parts(27_762_000, 1594)
5050
.saturating_add(T::DbWeight::get().reads(1_u64))
5151
.saturating_add(T::DbWeight::get().writes(1_u64))
5252
}
@@ -60,8 +60,8 @@ impl WeightInfo for () {
6060
// Proof Size summary in bytes:
6161
// Measured: `109`
6262
// Estimated: `1594`
63-
// Minimum execution time: 26_741_000 picoseconds.
64-
Weight::from_parts(27_212_000, 1594)
63+
// Minimum execution time: 27_051_000 picoseconds.
64+
Weight::from_parts(27_762_000, 1594)
6565
.saturating_add(RocksDbWeight::get().reads(1_u64))
6666
.saturating_add(RocksDbWeight::get().writes(1_u64))
6767
}

‎substrate-node/pallets/pallet-dao/src/weights.rs

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@
22
//! Autogenerated weights for pallet_dao
33
//!
44
//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev
5-
//! DATE: 2024-09-24, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]`
5+
//! DATE: 2024-10-01, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]`
66
//! WORST CASE MAP SIZE: `1000000`
7-
//! HOSTNAME: `da081090f983`, CPU: `AMD Ryzen 7 5800X 8-Core Processor`
7+
//! HOSTNAME: `585a9f003813`, CPU: `AMD Ryzen 7 5800X 8-Core Processor`
88
//! EXECUTION: , WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024
99
1010
// Executed Command:
@@ -58,8 +58,8 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
5858
// Proof Size summary in bytes:
5959
// Measured: `208`
6060
// Estimated: `4687`
61-
// Minimum execution time: 19_888_000 picoseconds.
62-
Weight::from_parts(20_368_000, 4687)
61+
// Minimum execution time: 19_757_000 picoseconds.
62+
Weight::from_parts(20_439_000, 4687)
6363
.saturating_add(T::DbWeight::get().reads(4_u64))
6464
.saturating_add(T::DbWeight::get().writes(5_u64))
6565
}
@@ -77,8 +77,8 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
7777
// Proof Size summary in bytes:
7878
// Measured: `979`
7979
// Estimated: `4444`
80-
// Minimum execution time: 26_991_000 picoseconds.
81-
Weight::from_parts(27_542_000, 4444)
80+
// Minimum execution time: 26_691_000 picoseconds.
81+
Weight::from_parts(27_332_000, 4444)
8282
.saturating_add(T::DbWeight::get().reads(5_u64))
8383
.saturating_add(T::DbWeight::get().writes(1_u64))
8484
}
@@ -92,8 +92,8 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
9292
// Proof Size summary in bytes:
9393
// Measured: `487`
9494
// Estimated: `4687`
95-
// Minimum execution time: 18_766_000 picoseconds.
96-
Weight::from_parts(19_256_000, 4687)
95+
// Minimum execution time: 18_845_000 picoseconds.
96+
Weight::from_parts(19_287_000, 4687)
9797
.saturating_add(T::DbWeight::get().reads(3_u64))
9898
.saturating_add(T::DbWeight::get().writes(1_u64))
9999
}
@@ -111,8 +111,8 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
111111
// Proof Size summary in bytes:
112112
// Measured: `521`
113113
// Estimated: `4687`
114-
// Minimum execution time: 25_348_000 picoseconds.
115-
Weight::from_parts(26_310_000, 4687)
114+
// Minimum execution time: 25_638_000 picoseconds.
115+
Weight::from_parts(26_180_000, 4687)
116116
.saturating_add(T::DbWeight::get().reads(3_u64))
117117
.saturating_add(T::DbWeight::get().writes(4_u64))
118118
}
@@ -136,8 +136,8 @@ impl WeightInfo for () {
136136
// Proof Size summary in bytes:
137137
// Measured: `208`
138138
// Estimated: `4687`
139-
// Minimum execution time: 19_888_000 picoseconds.
140-
Weight::from_parts(20_368_000, 4687)
139+
// Minimum execution time: 19_757_000 picoseconds.
140+
Weight::from_parts(20_439_000, 4687)
141141
.saturating_add(RocksDbWeight::get().reads(4_u64))
142142
.saturating_add(RocksDbWeight::get().writes(5_u64))
143143
}
@@ -155,8 +155,8 @@ impl WeightInfo for () {
155155
// Proof Size summary in bytes:
156156
// Measured: `979`
157157
// Estimated: `4444`
158-
// Minimum execution time: 26_991_000 picoseconds.
159-
Weight::from_parts(27_542_000, 4444)
158+
// Minimum execution time: 26_691_000 picoseconds.
159+
Weight::from_parts(27_332_000, 4444)
160160
.saturating_add(RocksDbWeight::get().reads(5_u64))
161161
.saturating_add(RocksDbWeight::get().writes(1_u64))
162162
}
@@ -170,8 +170,8 @@ impl WeightInfo for () {
170170
// Proof Size summary in bytes:
171171
// Measured: `487`
172172
// Estimated: `4687`
173-
// Minimum execution time: 18_766_000 picoseconds.
174-
Weight::from_parts(19_256_000, 4687)
173+
// Minimum execution time: 18_845_000 picoseconds.
174+
Weight::from_parts(19_287_000, 4687)
175175
.saturating_add(RocksDbWeight::get().reads(3_u64))
176176
.saturating_add(RocksDbWeight::get().writes(1_u64))
177177
}
@@ -189,8 +189,8 @@ impl WeightInfo for () {
189189
// Proof Size summary in bytes:
190190
// Measured: `521`
191191
// Estimated: `4687`
192-
// Minimum execution time: 25_348_000 picoseconds.
193-
Weight::from_parts(26_310_000, 4687)
192+
// Minimum execution time: 25_638_000 picoseconds.
193+
Weight::from_parts(26_180_000, 4687)
194194
.saturating_add(RocksDbWeight::get().reads(3_u64))
195195
.saturating_add(RocksDbWeight::get().writes(4_u64))
196196
}

‎substrate-node/pallets/pallet-kvstore/src/weights.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@
22
//! Autogenerated weights for pallet_kvstore
33
//!
44
//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev
5-
//! DATE: 2024-09-24, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]`
5+
//! DATE: 2024-10-01, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]`
66
//! WORST CASE MAP SIZE: `1000000`
7-
//! HOSTNAME: `da081090f983`, CPU: `AMD Ryzen 7 5800X 8-Core Processor`
7+
//! HOSTNAME: `585a9f003813`, CPU: `AMD Ryzen 7 5800X 8-Core Processor`
88
//! EXECUTION: , WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024
99
1010
// Executed Command:
@@ -46,8 +46,8 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
4646
// Proof Size summary in bytes:
4747
// Measured: `0`
4848
// Estimated: `0`
49-
// Minimum execution time: 7_054_000 picoseconds.
50-
Weight::from_parts(7_274_000, 0)
49+
// Minimum execution time: 6_903_000 picoseconds.
50+
Weight::from_parts(7_153_000, 0)
5151
.saturating_add(T::DbWeight::get().writes(1_u64))
5252
}
5353
/// Storage: `TFKVStore::TFKVStore` (r:1 w:1)
@@ -56,8 +56,8 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
5656
// Proof Size summary in bytes:
5757
// Measured: `146`
5858
// Estimated: `3611`
59-
// Minimum execution time: 12_263_000 picoseconds.
60-
Weight::from_parts(12_824_000, 3611)
59+
// Minimum execution time: 12_274_000 picoseconds.
60+
Weight::from_parts(12_734_000, 3611)
6161
.saturating_add(T::DbWeight::get().reads(1_u64))
6262
.saturating_add(T::DbWeight::get().writes(1_u64))
6363
}
@@ -71,8 +71,8 @@ impl WeightInfo for () {
7171
// Proof Size summary in bytes:
7272
// Measured: `0`
7373
// Estimated: `0`
74-
// Minimum execution time: 7_054_000 picoseconds.
75-
Weight::from_parts(7_274_000, 0)
74+
// Minimum execution time: 6_903_000 picoseconds.
75+
Weight::from_parts(7_153_000, 0)
7676
.saturating_add(RocksDbWeight::get().writes(1_u64))
7777
}
7878
/// Storage: `TFKVStore::TFKVStore` (r:1 w:1)
@@ -81,8 +81,8 @@ impl WeightInfo for () {
8181
// Proof Size summary in bytes:
8282
// Measured: `146`
8383
// Estimated: `3611`
84-
// Minimum execution time: 12_263_000 picoseconds.
85-
Weight::from_parts(12_824_000, 3611)
84+
// Minimum execution time: 12_274_000 picoseconds.
85+
Weight::from_parts(12_734_000, 3611)
8686
.saturating_add(RocksDbWeight::get().reads(1_u64))
8787
.saturating_add(RocksDbWeight::get().writes(1_u64))
8888
}

‎substrate-node/pallets/pallet-smart-contract/src/billing.rs

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -362,8 +362,14 @@ impl<T: Config> Pallet<T> {
362362
has_sufficient_fund: bool,
363363
current_block: u64,
364364
) -> DispatchResultWithPostInfo {
365+
// 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
366+
let rent_is_suspended = matches!(contract.contract_type, types::ContractData::NodeContract(_))
367+
&& ActiveRentContractForNode::<T>::get(contract.get_node_id())
368+
.and_then(|id| Contracts::<T>::get(&id))
369+
.map(|c| matches!(c.state, types::ContractState::GracePeriod(_)))
370+
.unwrap_or(false);
365371
match contract.state {
366-
types::ContractState::GracePeriod(_) if has_sufficient_fund => {
372+
types::ContractState::GracePeriod(_) if has_sufficient_fund && !rent_is_suspended => {
367373
// Manage transition from GracePeriod to Created
368374
log::info!("Contract {:?} is in grace period, but balance is recharged, moving to created state at block {:?}", contract.contract_id, current_block);
369375
Self::update_contract_state(contract, &types::ContractState::Created)?;
@@ -386,6 +392,28 @@ impl<T: Config> Pallet<T> {
386392
diff
387393
);
388394
if diff >= T::GracePeriod::get() {
395+
// Ensure associated node contracts have no reserved balance.
396+
// If none, proceed to move the contract to the 'deleted' state.
397+
// Otherwise, wait.
398+
match contract.contract_type {
399+
types::ContractData::RentContract(_) => {
400+
let active_node_contracts = ActiveNodeContracts::<T>::get(contract.get_node_id());
401+
let rent_has_node_contracts_with_reserve =
402+
active_node_contracts.iter().any(|id| {
403+
ContractPaymentState::<T>::get(id).map_or(false, |s| s.has_reserve())
404+
});
405+
406+
if rent_has_node_contracts_with_reserve {
407+
log::debug!(
408+
"Grace period on rented node expired, but one or more associated node contracts hold user funds. \
409+
Rent contract deletion will be delayed until funds are distributed to beneficiaries."
410+
);
411+
return Ok(().into());
412+
}
413+
}
414+
_ => (),
415+
};
416+
389417
log::info!("Contract {:?} state changed to deleted at block {:?} due to an expired grace period.", contract.contract_id, current_block);
390418
Self::deposit_event(Event::ContractGracePeriodElapsed {
391419
contract_id: contract.contract_id,
@@ -734,6 +762,14 @@ impl<T: Config> Pallet<T> {
734762
for ctr_id in active_node_contracts {
735763
let mut ctr =
736764
Contracts::<T>::get(ctr_id).ok_or(Error::<T>::ContractNotExists)?;
765+
766+
// 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).
767+
if ContractPaymentState::<T>::get(ctr_id).map_or_else(|| false, |ps| ps.has_overdraft())
768+
&& matches!(state, types::ContractState::Created)
769+
{
770+
continue;
771+
}
772+
737773
Self::update_contract_state(&mut ctr, &state)?;
738774

739775
match state {

‎substrate-node/pallets/pallet-smart-contract/src/grid_contract.rs

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -272,9 +272,12 @@ impl<T: Config> Pallet<T> {
272272
// override values
273273
contract.contract_type = types::ContractData::NodeContract(node_contract);
274274

275-
let state = contract.state.clone();
276-
Self::update_contract_state(&mut contract, &state)?;
277-
275+
// Why we call update_contract_state if state can't be changed?
276+
// Obviously, we just depened on the update_contract_state for writing the contract to the storage, but we can do it here
277+
// let state = contract.state.clone();
278+
// Self::update_contract_state(&mut contract, &state)?;
279+
Contracts::<T>::insert(&contract.contract_id, contract.clone());
280+
278281
Self::deposit_event(Event::ContractUpdated(contract));
279282

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

396-
match contracts.iter().position(|id| id == &contract_id) {
397-
Some(index) => {
398-
contracts.remove(index);
399-
}
400-
None => (),
401-
};
399+
let initial_len = contracts.len();
400+
401+
contracts.retain(|&id| id != contract_id);
402402

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

406409
// Helper function that updates the contract state and manages storage accordingly
407410
pub fn update_contract_state(
408411
contract: &mut types::Contract<T>,
409412
state: &types::ContractState,
410413
) -> DispatchResultWithPostInfo {
411-
// update the state and save the contract
412-
contract.state = state.clone();
413-
Contracts::<T>::insert(&contract.contract_id, contract.clone());
414+
// Update the state and save the contract only when necessary.
415+
if contract.state != *state {
416+
match (&contract.state, state) {
417+
// Transition from GracePeriod(x) to GracePeriod(y) is not allowed
418+
(types::ContractState::GracePeriod(_), types::ContractState::GracePeriod(_)) => {
419+
log::info!("Contract {:?} already in grace period, not changing to {:?} state", contract.contract_id, state);
420+
},
421+
// All other transitions are allowed
422+
_ => {
423+
contract.state = state.clone();
424+
Contracts::<T>::insert(&contract.contract_id, contract.clone());
425+
}
426+
}
427+
}
414428

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

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

442+
let original_len = contracts.len();
428443
match contracts.iter().position(|id| id == &contract.contract_id) {
429444
Some(index) => {
430445
// if the new contract state is delete, remove the contract id from the map
@@ -439,8 +454,9 @@ impl<T: Config> Pallet<T> {
439454
}
440455
}
441456
};
442-
443-
ActiveNodeContracts::<T>::insert(&node_contract.node_id, &contracts);
457+
if contracts.len() != original_len {
458+
ActiveNodeContracts::<T>::insert(&node_contract.node_id, &contracts);
459+
};
444460

445461
Ok(().into())
446462
}

0 commit comments

Comments
 (0)