Skip to content

Commit

Permalink
Fix saturating overflows (SlowMist N1) (#136)
Browse files Browse the repository at this point in the history
  • Loading branch information
clostao authored Nov 13, 2023
1 parent 566a3c7 commit a31b427
Show file tree
Hide file tree
Showing 7 changed files with 127 additions and 57 deletions.
52 changes: 39 additions & 13 deletions pallets/dnt-fee-controller/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ pub mod pallet {
ERC20WithdrawFailed,
ERC20DepositFailed,
FeeVaultOverflow,
ArithmeticError,
InvalidPercentage,
}

Expand Down Expand Up @@ -105,9 +106,14 @@ pub mod pallet {
) -> Result<(), Self::Error> {
let fee_vault = FeeVaultPrecompileAddressStorage::<T>::get().unwrap();
let mapped_amount = amount
.saturating_mul(conversion_rate.0)
.div_mod(conversion_rate.1)
.0;
.checked_mul(conversion_rate.0)
.map(|v| v.div_mod(conversion_rate.1).0);

let mapped_amount = match mapped_amount {
Some(amount) => amount,
None => return Err(Error::ArithmeticError),
};

T::ERC20Manager::withdraw_amount(token, from, mapped_amount)
.map_err(|_| Error::<T>::ERC20WithdrawFailed)?;
T::ERC20Manager::deposit_amount(token, fee_vault, mapped_amount)
Expand All @@ -123,11 +129,22 @@ pub mod pallet {
paid_amount: U256,
actual_amount: U256,
) -> Result<(), Self::Error> {
let over_fee = paid_amount.saturating_sub(actual_amount);
let over_fee = paid_amount.checked_sub(actual_amount);

let over_fee = match over_fee {
Some(amount) => amount,
None => return Err(Error::ArithmeticError),
};

let mapped_amount = over_fee
.saturating_mul(conversion_rate.0)
.div_mod(conversion_rate.1)
.0;
.checked_mul(conversion_rate.0)
.map(|v| v.div_mod(conversion_rate.1).0);

let mapped_amount = match mapped_amount {
Some(amount) => amount,
None => return Err(Error::ArithmeticError),
};

let fee_vault = FeeVaultPrecompileAddressStorage::<T>::get().unwrap();
T::ERC20Manager::withdraw_amount(token, fee_vault, mapped_amount)
.map_err(|_| Error::<T>::ERC20WithdrawFailed)?;
Expand All @@ -145,19 +162,28 @@ pub mod pallet {
to: Option<H160>,
) -> Result<(U256, U256), Self::Error> {
let fee_in_user_token = actual_amount
.saturating_mul(conversion_rate.0)
.div_mod(conversion_rate.1)
.0;
.checked_mul(conversion_rate.0)
.map(|v| v.div_mod(conversion_rate.1).0);

let fee_in_user_token = match fee_in_user_token {
Some(amount) => amount,
None => return Err(Error::ArithmeticError),
};

let validator_share = match to {
None => 100.into(),
Some(_) => ValidatorPercentageStorage::<T>::get().unwrap(),
};

let validator_fee = fee_in_user_token
.saturating_mul(validator_share.into())
.div_mod(U256::from(100))
.0;
.checked_mul(validator_share.into())
.map(|v| v.div_mod(U256::from(100)).0);

let validator_fee = match validator_fee {
Some(a) => a,
None => return Err(Error::ArithmeticError),
};

let dapp_fee = fee_in_user_token - validator_fee;

pallet_fee_rewards_vault::Pallet::<T>::add_claimable_reward(
Expand Down
49 changes: 36 additions & 13 deletions pallets/sponsored-transactions/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,12 +137,17 @@ pub mod pallet {

let (transaction_fee_token, conversion_rate) = Self::get_fee_token_info(&from);

let max_gas_used = match gas_limit.checked_mul(gas_price.into()) {
Some(a) => a,
None => return Err(DispatchError::Other("Arithmetic error due to overflow.")),
};

Self::transfer_fee_token(
&transaction_fee_token,
conversion_rate,
&meta_trx_sponsor,
&from,
gas_limit.saturating_mul(gas_price.into()),
max_gas_used,
)
.map_err(|_| DispatchError::Other("Failed to borrow fee token"))?;

Expand All @@ -151,16 +156,25 @@ pub mod pallet {
let dispatch = pallet_ethereum::Pallet::<T>::transact(origin, transaction)
.map_err(|_| DispatchError::Other("Signature doesn't meet with sponsor address"))?;

let gas_used = Self::gas_from_actual_weight(dispatch.actual_weight.unwrap());
let gas_used = Self::gas_from_actual_weight(dispatch.actual_weight.unwrap())
.map_err(|_| DispatchError::Other("Arithmetic error due to overflow."))?;

let gas_left = gas_limit.saturating_sub(gas_used.into());
let gas_left = match gas_limit.checked_sub(gas_used.into()) {
Some(v) => v,
None => 0.into(),
};

let refunding_amount = match gas_left.checked_mul(gas_price.into()) {
Some(amount) => amount,
None => return Err(DispatchError::Other("Arithmetic error due to overflow.")),
};

Self::transfer_fee_token(
&transaction_fee_token,
conversion_rate,
&from,
&meta_trx_sponsor,
gas_left.saturating_mul(gas_price.into()),
refunding_amount,
)
.map_err(|_| DispatchError::Other("Failed to refund fee token"))?;

Expand Down Expand Up @@ -281,14 +295,19 @@ pub mod pallet {
}
}

fn gas_from_actual_weight(weight: Weight) -> u64 {
let actual_weight = weight.saturating_add(
T::BlockWeights::get()
fn gas_from_actual_weight(weight: Weight) -> Result<u64, ()> {
let actual_weight = match weight.checked_add(
&T::BlockWeights::get()
.get(frame_support::dispatch::DispatchClass::Normal)
.base_extrinsic,
);
) {
Some(v) => v,
None => return Err(()),
};

<T as pallet_evm::Config>::GasWeightMapping::weight_to_gas(actual_weight)
Ok(<T as pallet_evm::Config>::GasWeightMapping::weight_to_gas(
actual_weight,
))
}

fn ensure_sponsor_balance(sponsor: H160, token: H160, amount: U256) -> Result<(), ()> {
Expand All @@ -307,10 +326,14 @@ pub mod pallet {
payee: &H160,
amount: U256,
) -> Result<(), ()> {
let actual_amount = amount
.saturating_mul(conversion_rate.0)
.div_mod(conversion_rate.1)
.0;
let actual_amount = match amount
.checked_mul(conversion_rate.0)
.map(|v| v.div_mod(conversion_rate.1).0)
{
Some(v) => v,
None => return Err(()),
};

T::ERC20Manager::withdraw_amount(token.clone(), payer.clone(), actual_amount)
.map_err(|_| {})?;

Expand Down
4 changes: 2 additions & 2 deletions pallets/sponsored-transactions/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,9 @@ fn check_correct_fee_management(called_arguments: Vec<(bool, H160, H160, U256)>)
for (is_deposit, token, _, amount) in called_arguments.iter() {
assert!(token.eq(&called_arguments[0].1));
if *is_deposit {
total_deposited = amount.saturating_add(total_deposited);
total_deposited = amount.checked_add(total_deposited).unwrap();
} else {
total_withdrawn = amount.saturating_add(total_withdrawn);
total_withdrawn = amount.checked_add(total_withdrawn).unwrap();
}
}
assert_eq!(total_deposited, total_withdrawn);
Expand Down
7 changes: 6 additions & 1 deletion pallets/validator-set/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -465,10 +465,15 @@ impl<T: Config> Pallet<T> {
fn do_remove_validator(validator_id: T::AccountId) -> DispatchResult {
let mut validators = <Validators<T>>::get();

let validators_count = match validators.len().checked_sub(1) {
Some(v) => v,
None => return Err(DispatchError::Other("Arithmetic error due to underflow.")),
};

// Ensuring that the post removal, target validator count doesn't go
// below the minimum.
ensure!(
validators.len().saturating_sub(1) as u32 >= T::MinAuthorities::get(),
validators_count as u32 >= T::MinAuthorities::get(),
Error::<T>::TooLowValidatorCount
);

Expand Down
58 changes: 33 additions & 25 deletions pallets/zero-gas-transactions/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ pub mod pallet {
use frame_system::pallet_prelude::*;
use pallet_evm::GasWeightMapping;
use pallet_user_fee_selector::UserFeeTokenController;
use sp_core::H256;
use sp_core::U256;
use sp_std::vec;
use sp_core::{H256};
use sp_std::{vec::Vec};
use sp_std::vec::Vec;

pub use fp_rpc::TransactionStatus;

Expand Down Expand Up @@ -52,21 +52,23 @@ pub mod pallet {

fn validate_unsigned(_source: TransactionSource, call: &Self::Call) -> TransactionValidity {
match call {
Call::send_zero_gas_transaction { transaction, validator_signature } => {
Call::send_zero_gas_transaction {
transaction,
validator_signature,
} => {
let from =
Self::ensure_transaction_signature(transaction.clone()).map_err(|_| {
TransactionValidityError::Invalid(InvalidTransaction::BadProof)
})?;

let current_block_validator = <pallet_evm::Pallet<T>>::find_author();
let current_block_validator = <pallet_evm::Pallet<T>>::find_author();

Self::ensure_zero_gas_transaction(
transaction.clone(),
current_block_validator,
validator_signature.clone()
).map_err(|_| {
TransactionValidityError::Invalid(InvalidTransaction::BadProof)
})?;
Self::ensure_zero_gas_transaction(
transaction.clone(),
current_block_validator,
validator_signature.clone(),
)
.map_err(|_| TransactionValidityError::Invalid(InvalidTransaction::BadProof))?;

let transaction_data: TransactionData = transaction.into();

Expand Down Expand Up @@ -116,9 +118,9 @@ pub mod pallet {
let current_block_validator = <pallet_evm::Pallet<T>>::find_author();

Self::ensure_zero_gas_transaction(
transaction.clone(),
current_block_validator,
validator_signature
transaction.clone(),
current_block_validator,
validator_signature,
)
.map_err(|_| DispatchError::Other("Invalid zero gas transaction signature"))?;

Expand All @@ -127,7 +129,8 @@ pub mod pallet {
let dispatch = pallet_ethereum::Pallet::<T>::transact(origin, transaction)
.map_err(|_| DispatchError::Other("Signature doesn't meet with sponsor address"))?;

let used_gas = Self::gas_from_actual_weight(dispatch.actual_weight.unwrap());
let used_gas = Self::gas_from_actual_weight(dispatch.actual_weight.unwrap())
.map_err(|_| DispatchError::Other("Arithmetic error due to overflows"))?;

Ok(frame_support::dispatch::PostDispatchInfo {
actual_weight: Some(T::GasWeightMapping::gas_to_weight(
Expand All @@ -140,14 +143,19 @@ pub mod pallet {
}

impl<T: Config> Pallet<T> {
fn gas_from_actual_weight(weight: Weight) -> u64 {
let actual_weight = weight.saturating_add(
T::BlockWeights::get()
fn gas_from_actual_weight(weight: Weight) -> Result<u64, ()> {
let actual_weight = match weight.checked_add(
&T::BlockWeights::get()
.get(frame_support::dispatch::DispatchClass::Normal)
.base_extrinsic,
);
) {
Some(v) => v,
None => return Err(()),
};

<T as pallet_evm::Config>::GasWeightMapping::weight_to_gas(actual_weight)
Ok(<T as pallet_evm::Config>::GasWeightMapping::weight_to_gas(
actual_weight,
))
}

fn ensure_transaction_signature(
Expand Down Expand Up @@ -196,18 +204,18 @@ pub mod pallet {
let eip191_message =
stbl_tools::eth::build_eip191_message_hash(zero_gas_trx_internal_message);

let zero_gas_trx_signer_address = Self::get_zero_gas_trx_signer(
validator_signature.clone(),
eip191_message.clone(),
);
let zero_gas_trx_signer_address =
Self::get_zero_gas_trx_signer(validator_signature.clone(), eip191_message.clone());

match zero_gas_trx_signer_address {
Some(address) if address == expected_validator => Ok(()),
_ => Err(()),
}
}

pub fn get_zero_gas_transaction_signing_message(trx: pallet_ethereum::Transaction) -> Vec<u8> {
pub fn get_zero_gas_transaction_signing_message(
trx: pallet_ethereum::Transaction,
) -> Vec<u8> {
let mut message: Vec<u8> = Vec::new();

let trx_hash = trx.hash();
Expand Down
4 changes: 2 additions & 2 deletions pallets/zero-gas-transactions/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,9 @@ fn check_correct_fee_management(called_arguments: Vec<(bool, H160, H160, U256)>)
for (is_deposit, token, _, amount) in called_arguments.iter() {
assert!(token.eq(&called_arguments[0].1));
if *is_deposit {
total_deposited = amount.saturating_add(total_deposited);
total_deposited = amount.checked_add(total_deposited).unwrap();
} else {
total_withdrawn = amount.saturating_add(total_withdrawn);
total_withdrawn = amount.checked_add(total_withdrawn).unwrap();
}
}
assert_eq!(total_deposited, total_withdrawn);
Expand Down
10 changes: 9 additions & 1 deletion primitives/runner/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,15 @@ where
let (base_fee, mut weight) = T::FeeCalculator::min_gas_price();
let (source_account, inner_weight) = Pallet::<T>::account_basic(&source);

weight = weight.saturating_add(inner_weight);
weight = match weight.checked_add(&inner_weight) {
Some(v) => v,
None => {
return Err(RunnerError {
error: Self::Error::FeeOverflow,
weight,
})
}
};

let _ = fp_evm::CheckEvmTransaction::<Self::Error>::new(
fp_evm::CheckEvmTransactionConfig {
Expand Down

0 comments on commit a31b427

Please sign in to comment.