Skip to content

Commit

Permalink
refactor: remove-is-next-author
Browse files Browse the repository at this point in the history
  • Loading branch information
@sameh-farouk
sameh-farouk committed Jul 18, 2024
1 parent 5a9ad37 commit b5538c3
Show file tree
Hide file tree
Showing 4 changed files with 161 additions and 102 deletions.
137 changes: 69 additions & 68 deletions substrate-node/pallets/pallet-smart-contract/src/billing.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,19 @@ use crate::*;
use frame_support::{
dispatch::{DispatchErrorWithPostInfo, DispatchResultWithPostInfo},
ensure,
traits::{Currency, ExistenceRequirement, LockableCurrency, OnUnbalanced, WithdrawReasons},
pallet_prelude::Pays,
traits::{
Currency, ExistenceRequirement, LockableCurrency, OnUnbalanced,
WithdrawReasons,
},
};
use frame_system::{
offchain::{SendSignedTransaction, SignMessage, Signer},
offchain::{SendSignedTransaction, Signer},
pallet_prelude::BlockNumberFor,
};
use sp_core::Get;
use sp_runtime::{
traits::{CheckedAdd, CheckedSub, Convert, Zero},
traits::{CheckedAdd, CheckedSub, Zero},
DispatchResult, Perbill, SaturatedConversion,
};
use sp_std::vec::Vec;
Expand Down Expand Up @@ -39,7 +43,7 @@ impl<T: Config> Pallet<T> {

for contract_id in contract_ids {
if let Some(c) = Contracts::<T>::get(contract_id) {
if let types::ContractData::NodeContract(node_contract) = c.contract_type {
if let types::ContractData::NodeContract(node_contract) = c.contract_type.clone() {
// Is there IP consumption to bill?
let bill_ip = node_contract.public_ips > 0;

Expand All @@ -54,53 +58,62 @@ impl<T: Config> Pallet<T> {

// Don't bill if no IP/CU/SU/NU to be billed
if !bill_ip && !bill_cu_su && !bill_nu {
log::debug!(
"Skipping billing node contract {:?}, no IP/CU/SU/NU to bill",
contract_id,
);
continue;
}
}
log::info!("Starting billing contract {:?}, type: {:?}", contract_id, c.contract_type);
let res = Self::bill_contract_using_signed_transaction(contract_id);
if res.is_ok() {
log::info!("Successfully submitted signed transaction for contract {:?}",
contract_id,
);
}
let _res = Self::bill_contract_using_signed_transaction(contract_id);
}
}
log::debug!("Finished billing contracts at block {:?}", block_number);
}

pub fn bill_contract_using_signed_transaction(contract_id: u64) -> Result<(), Error<T>> {
let signer = Signer::<T, <T as pallet::Config>::AuthorityId>::any_account();

// Only allow the author of the next block to trigger the billing
Self::is_next_block_author(&signer)?;
let signer = Signer::<T, <T as pallet::Config>::AuthorityId>::all_accounts();

// check if the account can sign the transaction
if !signer.can_sign() {
log::error!(
"failed billing contract {:?} account cannot be used to sign transaction",
"failed billing contract {:?}, account cannot be used to sign transaction",
contract_id,
);
return Err(<Error<T>>::OffchainSignedTxCannotSign);
}

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
// this means the transaction was already send before (probably by another node)
// unfortunately the error is always empty (substrate just logs the error and
// returns Err())
if res.is_err() {
for (_, res) in &result {
if res.is_ok() {
return Ok(());
}
}
log::error!(
"All local accounts failed to submit signed transaction for contract {:?}",
contract_id,
);
for (_, res) in result {
if let Err(e) = res {
log::error!(
"signed transaction failed for billing contract {:?} using account {:?}",
contract_id,
acc.id
"error: {:?}",
e,
);
return Err(<Error<T>>::OffchainSignedTxAlreadySent);
}
return Ok(());
}
log::error!("No local account available");
return Err(<Error<T>>::OffchainSignedTxNoLocalAccountAvailable);
return Err(<Error<T>>::OffchainSignedTxAlreadySent);
}

// Bills a contract (NodeContract, NameContract or RentContract)
// Calculates how much TFT is due by the user and distributes the rewards
pub fn bill_contract(contract_id: u64) -> DispatchResultWithPostInfo {
pub fn bill_contract(contract_id: u64, is_validator: bool) -> DispatchResultWithPostInfo {
let mut contract = Contracts::<T>::get(contract_id).ok_or(Error::<T>::ContractNotExists)?;

// Bill rent contract only if node is online
Expand All @@ -109,8 +122,15 @@ impl<T: Config> Pallet<T> {
// No need for preliminary call to contains_key() because default node power value is Up
let node_power = pallet_tfgrid::NodePower::<T>::get(node.id);
if node_power.is_standby() {
log::debug!(
"Skipping billing rent contract {:?}, node {:?} is in standby",
contract_id,
node.id,
);
return Ok(().into());
}
} else {
return Err(Error::<T>::NodeNotExists.into());
}
}

Expand All @@ -126,25 +146,38 @@ impl<T: Config> Pallet<T> {

// Calculate amount of seconds elapsed based on the contract lock struct
let mut contract_lock = ContractLock::<T>::get(contract.contract_id);
let seconds_elapsed = now.checked_sub(contract_lock.lock_updated).unwrap_or(0);
let seconds_elapsed = now.checked_sub(contract_lock.lock_updated).unwrap_or_else(|| {
log::warn!(
"error while calculating seconds elapsed, now: {:?}, lock_updated: {:?}, 0 assumed",
now,
contract_lock.lock_updated
);
0
});

// Calculate total amount due
let (regular_amount_due, discount_received) =
contract.calculate_contract_cost_tft(total_balance, seconds_elapsed)?;
contract.calculate_contract_cost_tft(total_balance, seconds_elapsed).map_err(|e| {
log::error!("error while calculating contract cost: {:?}", e);
e
})?;
let extra_amount_due = match &contract.contract_type {
types::ContractData::RentContract(rc) => {
contract.calculate_extra_fee_cost_tft(rc.node_id, seconds_elapsed)?
contract.calculate_extra_fee_cost_tft(rc.node_id, seconds_elapsed).map_err(|e| {
log::error!("error while calculating extra fee cost: {:?}", e);
e
})?
}
_ => BalanceOf::<T>::zero(),
};
let amount_due = regular_amount_due
.checked_add(&extra_amount_due)
.unwrap_or(BalanceOf::<T>::zero());
.unwrap_or(BalanceOf::<T>::zero()); // TODO: IMO this is bad, we should return an error here

// If there is nothing to be paid and the contract is not in state delete, return
// Can be that the users cancels the contract in the same block that it's getting billed
// where elapsed seconds would be 0, but we still have to distribute rewards
if amount_due == BalanceOf::<T>::zero() && !contract.is_state_delete() {
if amount_due.is_zero() && !contract.is_state_delete() {
log::debug!("amount to be billed is 0, nothing to do");
return Ok(().into());
};
Expand Down Expand Up @@ -214,7 +247,12 @@ impl<T: Config> Pallet<T> {

log::info!("successfully billed contract with id {:?}", contract_id,);

Ok(().into())
if is_validator {
// Exempt fees for validators
Ok(Pays::No.into())
} else {
Ok(Pays::Yes.into())
}
}

fn handle_grace(
Expand Down Expand Up @@ -717,43 +755,6 @@ impl<T: Config> Pallet<T> {
}
}

// Validates if the given signer is the next block author based on the validators in session
// This can be used if an extrinsic should be refunded by the author in the same block
// It also requires that the keytype inserted for the offchain workers is the validator key
fn is_next_block_author(
signer: &Signer<T, <T as Config>::AuthorityId>,
) -> Result<(), Error<T>> {
let author = <pallet_authorship::Pallet<T>>::author();
let validators = <pallet_session::Pallet<T>>::validators();

// Sign some arbitrary data in order to get the AccountId, maybe there is another way to do this?
let signed_message = signer.sign_message(&[0]);
if let Some(signed_message_data) = signed_message {
if let Some(block_author) = author {
let validator =
<T as pallet_session::Config>::ValidatorIdOf::convert(block_author.clone())
.ok_or(Error::<T>::IsNotAnAuthority)?;

let validator_count = validators.len();
let author_index = (validators.iter().position(|a| a == &validator).unwrap_or(0)
+ 1)
% validator_count;

let signer_validator_account =
<T as pallet_session::Config>::ValidatorIdOf::convert(
signed_message_data.0.id.clone(),
)
.ok_or(Error::<T>::IsNotAnAuthority)?;

if signer_validator_account != validators[author_index] {
return Err(Error::<T>::WrongAuthority);
}
}
}

Ok(().into())
}

pub fn get_current_timestamp_in_secs() -> u64 {
<pallet_timestamp::Pallet<T>>::get().saturated_into::<u64>() / 1000
}
Expand Down
15 changes: 10 additions & 5 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 @@ -321,7 +321,8 @@ impl<T: Config> Pallet<T> {
}

Self::update_contract_state(contract, &types::ContractState::Deleted(cause))?;
Self::bill_contract(contract.contract_id)?;
log::debug!("Billing for contract {} kicked in due to cancel request", contract.contract_id);
Self::bill_contract(contract.contract_id, false)?;

Ok(().into())
}
Expand Down Expand Up @@ -370,13 +371,13 @@ impl<T: Config> Pallet<T> {
}
};

log::debug!("removing contract");
log::debug!("removing contract {}", contract_id);
Contracts::<T>::remove(contract_id);
ContractLock::<T>::remove(contract_id);

// Clean up contract from billing loop
// This is the only place it should be done
log::debug!("cleaning up deleted contract from billing loop");
log::debug!("cleaning up deleted contract {} from billing loop", contract_id);
Self::remove_contract_from_billing_loop(contract_id)?;

Ok(().into())
Expand Down Expand Up @@ -702,7 +703,9 @@ impl<T: Config> ChangeNode<LocationOf<T>, InterfaceOf<T>, SerialNumberOf<T>> for
&mut contract,
&types::ContractState::Deleted(types::Cause::CanceledByUser),
);
let _ = Self::bill_contract(node_contract_id);
log::debug!("Billing for node contract {} kicked in due to node deletion", contract.contract_id);
let _ = Self::bill_contract(node_contract_id, false);

}
}

Expand All @@ -714,7 +717,8 @@ impl<T: Config> ChangeNode<LocationOf<T>, InterfaceOf<T>, SerialNumberOf<T>> for
&mut contract,
&types::ContractState::Deleted(types::Cause::CanceledByUser),
);
let _ = Self::bill_contract(contract.contract_id);
log::debug!("Billing for rent contract {} kicked in due to node deletion", contract.contract_id);
let _ = Self::bill_contract(contract.contract_id, false);
}
}
}
Expand All @@ -729,6 +733,7 @@ impl<T: Config> ChangeNode<LocationOf<T>, InterfaceOf<T>, SerialNumberOf<T>> for
let now = Self::get_current_timestamp_in_secs();
contract_lock.lock_updated = now;
ContractLock::<T>::insert(rc_id, &contract_lock);
log::debug!("Rented node {} is back up, updated contract lock_updated for contract {}", node.id, rc_id);
}
}
}
Expand Down
25 changes: 24 additions & 1 deletion substrate-node/pallets/pallet-smart-contract/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ pub mod pallet {
};
use parity_scale_codec::FullCodec;
use sp_core::H256;
use sp_runtime::traits::Convert;
use sp_std::{
convert::{TryFrom, TryInto},
fmt::Debug,
Expand Down Expand Up @@ -195,6 +196,10 @@ pub mod pallet {
#[pallet::getter(fn dedicated_nodes_extra_fee)]
pub type DedicatedNodesExtraFee<T> = StorageMap<_, Blake2_128Concat, u32, u64, ValueQuery>;

#[pallet::storage]
#[pallet::getter(fn seen_values)]
pub type SeenContracts<T> = StorageValue<_, Vec<u64>, ValueQuery>;

#[pallet::config]
pub trait Config:
CreateSignedTransaction<Call<Self>>
Expand Down Expand Up @@ -516,7 +521,21 @@ pub mod pallet {
contract_id: u64,
) -> DispatchResultWithPostInfo {
let _account_id = ensure_signed(origin)?;
Self::bill_contract(contract_id)

let mut seen_contracts = SeenContracts::<T>::get();
ensure!(
!seen_contracts.contains(&contract_id),
"this contract already processed in this block",
);
seen_contracts.push(contract_id);
SeenContracts::<T>::put(seen_contracts);

let validators = pallet_session::Pallet::<T>::validators();
let is_validator =
<T as pallet_session::Config>::ValidatorIdOf::convert(_account_id.clone())
.map_or(false, |validator_id| validators.contains(&validator_id));

Self::bill_contract(contract_id, is_validator)
}

#[pallet::call_index(11)]
Expand Down Expand Up @@ -663,5 +682,9 @@ pub mod pallet {
fn offchain_worker(block_number: BlockNumberFor<T>) {
Self::bill_contracts_for_block(block_number);
}

fn on_finalize(_n: BlockNumberFor<T>) {
SeenContracts::<T>::kill();
}
}
}
Loading

0 comments on commit b5538c3

Please sign in to comment.