diff --git a/substrate-node/pallets/pallet-smart-contract/src/billing.rs b/substrate-node/pallets/pallet-smart-contract/src/billing.rs index 1ad253516..f6973b14b 100644 --- a/substrate-node/pallets/pallet-smart-contract/src/billing.rs +++ b/substrate-node/pallets/pallet-smart-contract/src/billing.rs @@ -362,8 +362,14 @@ impl Pallet { 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::::get(contract.get_node_id()) + .and_then(|id| Contracts::::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)?; @@ -386,6 +392,26 @@ impl Pallet { 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. + let can_be_deleted = match contract.contract_type { + types::ContractData::RentContract(_) => { + let active_node_contracts = ActiveNodeContracts::::get(contract.get_node_id()); + !active_node_contracts.iter().any(|id| { + ContractPaymentState::::get(id).map_or(false, |s| s.has_reserve()) + }) + } + _ => true, + }; + if !can_be_deleted { + log::debug!( + "Grace period expired, but one or more associated node contracts have held 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, @@ -734,6 +760,14 @@ impl Pallet { for ctr_id in active_node_contracts { let mut ctr = Contracts::::get(ctr_id).ok_or(Error::::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::::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 { 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 bada34e30..fb3e12bb3 100644 --- a/substrate-node/pallets/pallet-smart-contract/src/grid_contract.rs +++ b/substrate-node/pallets/pallet-smart-contract/src/grid_contract.rs @@ -270,9 +270,12 @@ impl Pallet { // 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::::insert(&contract.contract_id, contract.clone()); + Self::deposit_event(Event::ContractUpdated(contract)); Ok(().into()) @@ -391,14 +394,14 @@ impl Pallet { fn remove_active_node_contract(node_id: u32, contract_id: u64) { let mut contracts = ActiveNodeContracts::::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::::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::::insert(&node_id, contracts); + } } // Helper function that updates the contract state and manages storage accordingly @@ -406,9 +409,20 @@ impl Pallet { contract: &mut types::Contract, state: &types::ContractState, ) -> DispatchResultWithPostInfo { - // update the state and save the contract - contract.state = state.clone(); - Contracts::::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::::insert(&contract.contract_id, contract.clone()); + } + } + } // if the contract is a name or rent contract, nothing to do left here match contract.contract_type { @@ -423,6 +437,7 @@ impl Pallet { let mut contracts = ActiveNodeContracts::::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 @@ -437,8 +452,9 @@ impl Pallet { } } }; - - ActiveNodeContracts::::insert(&node_contract.node_id, &contracts); + if contracts.len() != original_len { + ActiveNodeContracts::::insert(&node_contract.node_id, &contracts); + }; Ok(().into()) }