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.

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

Fix saturating overflows (SlowMist N1) #136

Merged
merged 3 commits into from
Nov 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
Loading