Skip to content

Commit

Permalink
Ensure no error is possible in apply_validated_transaction and fix …
Browse files Browse the repository at this point in the history
…`correct_and_deposit_fee` (#497)

* Ensure no error is possible in `apply_validated_transaction`

* Fix lints

* Fix correct_and_deposit_fee should not be able to error

* Change ensure to assert

* Fix tests
  • Loading branch information
sorpaas authored Oct 13, 2021
1 parent 146bb48 commit 334409c
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 25 deletions.
24 changes: 10 additions & 14 deletions frame/ethereum/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,9 +210,7 @@ pub mod pallet {
Self::validate_transaction_in_block(source, &transaction).expect(
"pre-block transaction verification failed; the block cannot be built",
);
Self::apply_validated_transaction(source, transaction).expect(
"pre-block transaction execution failed; the block cannot be built",
);
Self::apply_validated_transaction(source, transaction);
}
}

Expand All @@ -233,12 +231,12 @@ pub mod pallet {
) -> DispatchResultWithPostInfo {
let source = ensure_ethereum_transaction(origin)?;
// Disable transact functionality if PreLog exist.
ensure!(
assert!(
fp_consensus::find_pre_log(&frame_system::Pallet::<T>::digest()).is_err(),
Error::<T>::PreLogExists,
"pre log already exists; block is invalid",
);

Self::apply_validated_transaction(source, transaction)
Ok(Self::apply_validated_transaction(source, transaction))
}
}

Expand Down Expand Up @@ -457,10 +455,7 @@ impl<T: Config> Pallet<T> {
builder.build()
}

fn apply_validated_transaction(
source: H160,
transaction: Transaction,
) -> DispatchResultWithPostInfo {
fn apply_validated_transaction(source: H160, transaction: Transaction) -> PostDispatchInfo {
let transaction_hash =
H256::from_slice(Keccak256::digest(&rlp::encode(&transaction)).as_slice());
let transaction_index = Pending::<T>::get().len() as u32;
Expand All @@ -474,7 +469,8 @@ impl<T: Config> Pallet<T> {
Some(transaction.nonce),
transaction.action,
None,
)?;
)
.expect("transaction is already validated; error indicates that the block is invalid");

let (reason, status, used_gas, dest) = match info {
CallOrCreateInfo::Call(info) => (
Expand Down Expand Up @@ -535,13 +531,13 @@ impl<T: Config> Pallet<T> {
transaction_hash,
reason,
));
Ok(PostDispatchInfo {

PostDispatchInfo {
actual_weight: Some(T::GasWeightMapping::gas_to_weight(
used_gas.unique_saturated_into(),
)),
pays_fee: Pays::No,
})
.into()
}
}

/// Get the transaction status with given index.
Expand Down
6 changes: 3 additions & 3 deletions frame/ethereum/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ use frame_support::{
unsigned::{TransactionValidityError, ValidateUnsigned},
};
use rustc_hex::{FromHex, ToHex};
use sp_runtime::traits::Applyable;
use sp_runtime::transaction_validity::{
InvalidTransaction, TransactionSource, ValidTransactionBuilder,
use sp_runtime::{
traits::Applyable,
transaction_validity::{InvalidTransaction, TransactionSource, ValidTransactionBuilder},
};
use std::str::FromStr;

Expand Down
11 changes: 5 additions & 6 deletions frame/evm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -655,7 +655,7 @@ pub trait OnChargeEVMTransaction<T: Config> {
who: &H160,
corrected_fee: U256,
already_withdrawn: Self::LiquidityInfo,
) -> Result<(), Error<T>>;
);
}

/// Implements the transaction payment for a pallet implementing the `Currency`
Expand Down Expand Up @@ -697,7 +697,7 @@ where
who: &H160,
corrected_fee: U256,
already_withdrawn: Self::LiquidityInfo,
) -> Result<(), Error<T>> {
) {
if let Some(paid) = already_withdrawn {
let account_id = T::AddressMapping::into_account_id(*who);

Expand All @@ -714,10 +714,9 @@ where
let adjusted_paid = paid
.offset(refund_imbalance)
.same()
.map_err(|_| Error::<T>::BalanceLow)?;
.unwrap_or_else(|_| C::NegativeImbalance::zero());
OU::on_unbalanced(adjusted_paid);
}
Ok(())
}
}

Expand All @@ -743,7 +742,7 @@ impl<T> OnChargeEVMTransaction<T> for ()
who: &H160,
corrected_fee: U256,
already_withdrawn: Self::LiquidityInfo,
) -> Result<(), Error<T>> {
EVMCurrencyAdapter::<<T as Config>::Currency, ()>::correct_and_deposit_fee(who, corrected_fee, already_withdrawn)
) {
<EVMCurrencyAdapter::<<T as Config>::Currency, ()> as OnChargeEVMTransaction<T>>::correct_and_deposit_fee(who, corrected_fee, already_withdrawn)
}
}
2 changes: 1 addition & 1 deletion frame/evm/src/runner/stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ impl<T: Config> Runner<T> {
);

// Refund fees to the `source` account if deducted more before,
T::OnChargeTransaction::correct_and_deposit_fee(&source, actual_fee, fee)?;
T::OnChargeTransaction::correct_and_deposit_fee(&source, actual_fee, fee);

let state = executor.into_state();

Expand Down
2 changes: 1 addition & 1 deletion frame/evm/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ fn fee_deduction() {
assert_eq!(Balances::free_balance(&substrate_addr), 90);

// Refund fees as 5 units
<<Test as Config>::OnChargeTransaction as OnChargeEVMTransaction<Test>>::correct_and_deposit_fee(&evm_addr, U256::from(5), imbalance).unwrap();
<<Test as Config>::OnChargeTransaction as OnChargeEVMTransaction<Test>>::correct_and_deposit_fee(&evm_addr, U256::from(5), imbalance);
assert_eq!(Balances::free_balance(&substrate_addr), 95);
});
}
Expand Down

0 comments on commit 334409c

Please sign in to comment.