diff --git a/substrate-node/pallets/pallet-smart-contract/src/lib.rs b/substrate-node/pallets/pallet-smart-contract/src/lib.rs index 6ff7dd3b5..2c47f4335 100644 --- a/substrate-node/pallets/pallet-smart-contract/src/lib.rs +++ b/substrate-node/pallets/pallet-smart-contract/src/lib.rs @@ -25,7 +25,6 @@ use pallet_tfgrid::types as pallet_tfgrid_types; use pallet_timestamp as timestamp; use sp_core::crypto::KeyTypeId; use sp_runtime::{ - offchain::storage::StorageValueRef, traits::{CheckedSub, SaturatedConversion}, Perbill, }; @@ -75,6 +74,7 @@ pub mod crypto { } pub mod cost; +pub mod migration; pub mod name_contract; pub mod types; pub mod weights; @@ -491,10 +491,9 @@ pub mod pallet { pub fn bill_contract_for_block( origin: OriginFor, contract_id: u64, - block_number: T::BlockNumber, ) -> DispatchResultWithPostInfo { let _account_id = ensure_signed(origin)?; - Self::_bill_contract_for_block(contract_id, block_number) + Self::_bill_contract(contract_id) } } @@ -505,14 +504,6 @@ pub mod pallet { // Index being current block number % (mod) Billing Frequency let current_index: u64 = block_number.saturated_into::() % BillingFrequency::::get(); - //reset the failed contract ids for next run - Self::save_failed_contract_ids_in_storage(Vec::new()); - // filter out the contracts that have been deleted in the meantime - contracts = contracts - .into_iter() - .filter(|contract_id| Contracts::::contains_key(contract_id)) - .collect::>(); - contracts.dedup(); let contracts = ContractsToBillAt::::get(current_index); if contracts.is_empty() { @@ -977,23 +968,17 @@ impl Pallet { return Err(>::OffchainSignedTxError); } - fn bill_contract_using_signed_transaction( - block_number: T::BlockNumber, - contract_id: u64, - ) -> Result<(), Error> { + fn bill_contract_using_signed_transaction(contract_id: u64) -> Result<(), Error> { let signer = Signer::::AuthorityId>::any_account(); if !signer.can_sign() { log::error!( - "failed billing contract {:?}: at block {:?} account cannot be used to sign transaction", + "failed billing contract {:?} account cannot be used to sign transaction", contract_id, - block_number ); return Err(>::OffchainSignedTxError); } - let result = signer.send_signed_transaction(|_acct| Call::bill_contract_for_block { - contract_id, - block_number, - }); + let result = + signer.send_signed_transaction(|_acct| Call::bill_contract_for_block { contract_id }); if let Some((acc, res)) = result { // if res is an error this means sending the transaction failed @@ -1002,9 +987,8 @@ impl Pallet { // returns Err()) if res.is_err() { log::error!( - "signed transaction failed for billing contract {:?} at block {:?} using account {:?}", + "signed transaction failed for billing contract {:?} using account {:?}", contract_id, - block_number, acc.id ); return Err(>::OffchainSignedTxError); @@ -1015,36 +999,6 @@ impl Pallet { return Err(>::OffchainSignedTxError); } - fn append_failed_contract_ids_to_storage(failed_id: u64) { - log::info!( - "billing contract {:?} will be retried in the next block", - failed_id - ); - - let mut failed_ids = Self::get_failed_contract_ids_from_storage(); - failed_ids.push(failed_id); - - Self::save_failed_contract_ids_in_storage(failed_ids); - } - - fn save_failed_contract_ids_in_storage(failed_ids: Vec) { - let s_contracts = - StorageValueRef::persistent(b"pallet-smart-contract::failed-contracts-when-billing"); - - s_contracts.set(&failed_ids); - } - - fn get_failed_contract_ids_from_storage() -> Vec { - let s_contracts = - StorageValueRef::persistent(b"pallet-smart-contract::failed-contracts-when-billing"); - - if let Ok(Some(failed_contract_ids)) = s_contracts.get::>() { - return failed_contract_ids; - } - - return Vec::new(); - } - // Bills a contract (NodeContract or NameContract) // Calculates how much TFT is due by the user and distributes the rewards fn bill_contract(contract_id: u64) -> DispatchResultWithPostInfo { @@ -1100,13 +1054,15 @@ impl Pallet { return Ok(().into()); } - // Handle contract lock operations - // skip when the contract status was grace and changed to deleted because that means - // the grace period ended and there were still no funds - if !(was_grace && matches!(contract.state, types::ContractState::Deleted(_))) { - Self::handle_lock(contract, amount_due)?; + // If still in grace period, no need to continue doing locking and other stuff + if matches!(contract.state, types::ContractState::GracePeriod(_)) { + log::debug!("contract {} is still in grace", contract.contract_id); + return Ok(().into()); } + // Handle contract lock operations + Self::handle_lock(contract, amount_due)?; + // Always emit a contract billed event let contract_bill = types::ContractBill { contract_id: contract.contract_id, @@ -1813,6 +1769,11 @@ impl Pallet { let now = >::block_number().saturated_into::(); now % BillingFrequency::::get() } + + pub fn get_contract_index() -> u64 { + let now = >::block_number().saturated_into::(); + now % BillingFrequency::::get() + } } impl ChangeNode, PubConfigOf, InterfaceOf, SerialNumberOf> diff --git a/substrate-node/pallets/pallet-smart-contract/src/migration.rs b/substrate-node/pallets/pallet-smart-contract/src/migration.rs new file mode 100644 index 000000000..76250c0e0 --- /dev/null +++ b/substrate-node/pallets/pallet-smart-contract/src/migration.rs @@ -0,0 +1,91 @@ +use super::*; +use frame_support::weights::Weight; + +pub mod v5 { + use super::*; + use crate::Config; + + use frame_support::{pallet_prelude::Weight, traits::OnRuntimeUpgrade}; + use sp_std::marker::PhantomData; + pub struct ContractMigrationV5(PhantomData); + + impl OnRuntimeUpgrade for ContractMigrationV5 { + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result<(), &'static str> { + info!("current pallet version: {:?}", PalletVersion::::get()); + assert!(PalletVersion::::get() == types::StorageVersion::V5); + + info!("👥 Smart Contract pallet to v6 passes PRE migrate checks ✅",); + Ok(()) + } + + fn on_runtime_upgrade() -> Weight { + migrate_to_version_6::() + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade() -> Result<(), &'static str> { + info!("current pallet version: {:?}", PalletVersion::::get()); + assert!(PalletVersion::::get() == types::StorageVersion::V6); + + info!( + "👥 Smart Contract pallet to {:?} passes POST migrate checks ✅", + PalletVersion::::get() + ); + + Ok(()) + } + } +} + +pub fn migrate_to_version_6() -> frame_support::weights::Weight { + if PalletVersion::::get() == types::StorageVersion::V5 { + info!( + " >>> Starting contract pallet migration, pallet version: {:?}", + PalletVersion::::get() + ); + + let mut migrated_count = 0; + + // Collect ContractsToBillAt storage in memory + let contracts_to_bill_at = ContractsToBillAt::::iter().collect::>(); + + // Remove all items under ContractsToBillAt + frame_support::migration::remove_storage_prefix( + b"SmartContractModule", + b"ContractsToBillAt", + b"", + ); + + let billing_freq = 600; + BillingFrequency::::put(billing_freq); + + for (block_number, contract_ids) in contracts_to_bill_at { + migrated_count += 1; + // Construct new index + let index = (block_number - 1) % billing_freq; + // Reinsert items under the new key + info!( + "inserted contracts:{:?} at index: {:?}", + contract_ids.clone(), + index + ); + ContractsToBillAt::::insert(index, contract_ids); + } + + info!( + " <<< Contracts storage updated! Migrated {} Contracts ✅", + migrated_count + ); + + // Update pallet storage version + PalletVersion::::set(types::StorageVersion::V6); + info!(" <<< Storage version upgraded"); + + // Return the weight consumed by the migration. + T::DbWeight::get().reads_writes(migrated_count as Weight + 1, migrated_count as Weight + 1) + } else { + info!(" >>> Unused migration"); + return 0; + } +} diff --git a/substrate-node/pallets/pallet-smart-contract/src/mock.rs b/substrate-node/pallets/pallet-smart-contract/src/mock.rs index c13f7590c..8031d4835 100644 --- a/substrate-node/pallets/pallet-smart-contract/src/mock.rs +++ b/substrate-node/pallets/pallet-smart-contract/src/mock.rs @@ -3,7 +3,7 @@ use std::{panic, thread}; use super::*; use crate::name_contract::NameContractName; -use crate::{self as pallet_smart_contract, types::BlockNumber}; +use crate::{self as pallet_smart_contract}; use codec::{alloc::sync::Arc, Decode}; use frame_support::{ construct_runtime, parameter_types, @@ -393,12 +393,13 @@ pub fn new_test_ext() -> sp_io::TestExternalities { t } pub type TransactionCall = pallet_smart_contract::Call; +pub type ExtrinsicResult = Result; #[derive(Default)] pub struct PoolState { /// A vector of calls that we expect should be executed - pub expected_calls: Vec<(TransactionCall, Result<(), ()>)>, - pub calls_to_execute: Vec<(TransactionCall, Result<(), ()>)>, + pub expected_calls: Vec<(TransactionCall, ExtrinsicResult, u64)>, + pub calls_to_execute: Vec<(TransactionCall, ExtrinsicResult, u64)>, pub i: usize, } @@ -406,57 +407,43 @@ impl PoolState { pub fn should_call_bill_contract( &mut self, contract_id: u64, - block_number: BlockNumber, - expected_result: Result<(), ()>, + expected_result: ExtrinsicResult, + block_number: u64, ) { self.expected_calls.push(( - crate::Call::bill_contract_for_block { - contract_id, - block_number, - }, + crate::Call::bill_contract_for_block { contract_id }, expected_result, + block_number, )); } - pub fn should_call(&mut self, expected_call: TransactionCall, expected_result: Result<(), ()>) { - self.expected_calls.push((expected_call, expected_result)); - } - - pub fn execute_calls_and_check_results(&mut self) { + pub fn execute_calls_and_check_results(&mut self, block_number: u64) { if self.calls_to_execute.len() == 0 { return; } - + // execute the calls that were submitted to the pool and compare the result for call_to_execute in self.calls_to_execute.iter() { let result = match call_to_execute.0 { // matches bill_contract_for_block - crate::Call::bill_contract_for_block { - contract_id, - block_number, - } => SmartContractModule::bill_contract_for_block( - Origin::signed(bob()), - contract_id, - block_number, - ), + crate::Call::bill_contract_for_block { contract_id } => { + SmartContractModule::bill_contract_for_block(Origin::signed(bob()), contract_id) + } // did not match anything => unkown call => this means you should add // a capture for that function here _ => panic!("Unknown call!"), }; - - let result = match result { - Ok(_) => Ok(()), - Err(_) => Err(()), - }; - + // the call should return what we expect it to return assert_eq!( call_to_execute.1, result, "The result of call to {:?} was not as expected!", call_to_execute.0 ); + + assert_eq!(block_number, call_to_execute.2); } - + self.calls_to_execute.clear(); } } @@ -471,10 +458,10 @@ impl Drop for PoolState { } /// Implementation of mocked transaction pool used for testing -/// -/// This transaction pool mocks submitting the transactions to the pool. It does +/// +/// This transaction pool mocks submitting the transactions to the pool. It does /// not execute the transactions. Instead it keeps them in list. It does compare -/// the submitted call to the expected call. +/// the submitted call to the expected call. #[derive(Default)] pub struct MockedTransactionPoolExt(Arc>); @@ -516,7 +503,10 @@ impl TransactionPool for MockedTransactionPoolExt { self.0.write().i = i + 1; // return the expected return value - return self.0.read().expected_calls[i].1; + return self.0.read().expected_calls[i] + .1 + .map_err(|_| ()) + .map(|_| ()); } // we should not end here as it would mean we did not expect any more calls diff --git a/substrate-node/pallets/pallet-smart-contract/src/test_utils.rs b/substrate-node/pallets/pallet-smart-contract/src/test_utils.rs index 1e8af36cc..55e4bcf7e 100644 --- a/substrate-node/pallets/pallet-smart-contract/src/test_utils.rs +++ b/substrate-node/pallets/pallet-smart-contract/src/test_utils.rs @@ -1,8 +1,6 @@ #![cfg(test)] -use crate::mock::{ - bob, Origin, PoolState, SmartContractModule, System, Timestamp, -}; +use crate::mock::{PoolState, SmartContractModule, System, Timestamp}; use codec::alloc::sync::Arc; use frame_support::traits::Hooks; diff --git a/substrate-node/pallets/pallet-smart-contract/src/tests.rs b/substrate-node/pallets/pallet-smart-contract/src/tests.rs index 75d2073c0..f33441a78 100644 --- a/substrate-node/pallets/pallet-smart-contract/src/tests.rs +++ b/substrate-node/pallets/pallet-smart-contract/src/tests.rs @@ -14,7 +14,7 @@ use pallet_tfgrid::{ ResourcesInput, }; use sp_core::H256; -use sp_runtime::{traits::SaturatedConversion, Perbill, Percent, assert_eq_error_rate}; +use sp_runtime::{assert_eq_error_rate, traits::SaturatedConversion, Perbill, Percent}; use sp_std::convert::{TryFrom, TryInto}; use substrate_fixed::types::U64F64; use tfchain_support::{ @@ -940,7 +940,6 @@ fn test_node_contract_billing_cycles() { .should_call_bill_contract(1, Ok(Pays::Yes.into()), 11); run_to_block(11, Some(&mut pool_state)); check_report_cost(1, amount_due_1, 11, discount_received); - pool_state.write().should_call_bill_contract(1, 11, Ok(())); let twin = TfgridModule::twins(twin_id).unwrap(); let usable_balance = Balances::usable_balance(&twin.account_id); @@ -1238,8 +1237,8 @@ fn test_node_contract_billing_cycles_cancel_contract_during_cycle_works() { fn test_node_contract_billing_fails() { let (mut ext, mut pool_state) = new_test_ext_with_pool_state(0); ext.execute_with(|| { - // Creates a farm and node and sets the price of tft to 0 which raises an error later run_to_block(1, Some(&mut pool_state)); + // Creates a farm and node and sets the price of tft to 0 which raises an error later prepare_farm_and_node(); assert_ok!(SmartContractModule::create_node_contract( @@ -1826,6 +1825,10 @@ fn test_rent_contract_canceled_due_to_out_of_funds_should_cancel_node_contracts_ let our_events = System::events(); assert_eq!(our_events.len(), 10); + for e in our_events.clone().iter() { + info!("event: {:?}", e); + } + assert_eq!( our_events[5], record(MockEvent::SmartContractModule(SmartContractEvent::<