Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix inconsistencies in node contracts states #1010

Merged
merged 10 commits into from
Oct 2, 2024
Merged

Fix inconsistencies in node contracts states #1010

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ce7bcd8
fix: fix inconsistencies in node contracts states
sameh-farouk Sep 22, 2024
e406ab8
test: add tests for scenarios related to issue #1002
sameh-farouk Sep 29, 2024
24ba390
refactor: small code refactor based on PR comment
sameh-farouk Sep 29, 2024
180cffd
chore: update benchmark `weights.rs` file for all pallets
github-actions[bot] Sep 29, 2024
0c3708b
Merge remote-tracking branch 'origin' into development_fix_sync_contr…
sameh-farouk Sep 29, 2024
9f75e2d
chore: fix typo in log comment
sameh-farouk Sep 30, 2024
566c9df
test: correct case 2
sameh-farouk Sep 30, 2024
3286f3e
refactor: minor test refactor
sameh-farouk Sep 30, 2024
ceff3a2
chore: fix typos
sameh-farouk Sep 30, 2024
a78fdb2
chore: update benchmark `weights.rs` file for all pallets
github-actions[bot] Oct 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions substrate-node/pallets/pallet-burning/src/weights.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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);
sameh-farouk marked this conversation as resolved.
Show resolved Hide resolved
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())
sameh-farouk marked this conversation as resolved.
Show resolved Hide resolved
&& 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
Open in desktop
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());
sameh-farouk marked this conversation as resolved.
Show resolved Hide resolved

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