Skip to content

Commit

Permalink
Improve RPC error messages (#1176)
Browse files Browse the repository at this point in the history
* Refactor errors handle

* Rename and code clean

* Rename to `InvalidFeeInput`

* Fix rs test

* Add ts tests

* Fix InvalidFeeInput match
  • Loading branch information
boundless-forest authored Sep 4, 2023
1 parent 8b0ff04 commit 55e0861
Show file tree
Hide file tree
Showing 15 changed files with 168 additions and 95 deletions.
3 changes: 1 addition & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion client/rpc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ fc-api = { workspace = true }
fc-mapping-sync = { workspace = true }
fc-rpc-core = { workspace = true }
fc-storage = { workspace = true }
fp-ethereum = { workspace = true, features = ["default"] }
fp-evm = { workspace = true }
fp-rpc = { workspace = true, features = ["default"] }
fp-storage = { workspace = true, features = ["default"] }
Expand Down
7 changes: 5 additions & 2 deletions client/rpc/src/eth/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
use sc_transaction_pool_api::error::{Error as PError, IntoPoolError};
use sp_runtime::transaction_validity::InvalidTransaction;
// Frontier
use fp_ethereum::TransactionValidationError as VError;
use fp_evm::TransactionValidationError as VError;

// Formats the same way Geth node formats responses.
pub struct Geth;
Expand All @@ -43,9 +43,12 @@ impl Geth {
VError::InvalidSignature => "invalid sender".into(),
VError::GasLimitTooLow => "intrinsic gas too low".into(),
VError::GasLimitTooHigh => "exceeds block gas limit".into(),
VError::MaxFeePerGasTooLow => {
VError::GasPriceTooLow => "gas price less than block base fee".into(),
VError::PriorityFeeTooHigh => {
"max priority fee per gas higher than max fee per gas".into()
}
VError::InvalidFeeInput => "invalid fee input".into(),
_ => "transaction validation error".into(),
},
_ => "unknown error".into(),
},
Expand Down
42 changes: 24 additions & 18 deletions frame/ethereum/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ mod tests;
use ethereum_types::{Bloom, BloomInput, H160, H256, H64, U256};
use evm::ExitReason;
use fp_consensus::{PostLog, PreLog, FRONTIER_ENGINE_ID};
use fp_ethereum::{TransactionValidationError, ValidatedTransaction as ValidatedTransactionT};
use fp_ethereum::ValidatedTransaction as ValidatedTransactionT;
use fp_evm::{
CallOrCreateInfo, CheckEvmTransaction, CheckEvmTransactionConfig, InvalidEvmTransactionError,
CallOrCreateInfo, CheckEvmTransaction, CheckEvmTransactionConfig, TransactionValidationError,
};
use fp_storage::{EthereumStorageSchema, PALLET_ETHEREUM_SCHEMA};
use frame_support::{
Expand Down Expand Up @@ -979,36 +979,42 @@ impl<T: Config> BlockHashMapping for EthereumBlockHashMapping<T> {

pub struct InvalidTransactionWrapper(InvalidTransaction);

impl From<InvalidEvmTransactionError> for InvalidTransactionWrapper {
fn from(validation_error: InvalidEvmTransactionError) -> Self {
impl From<TransactionValidationError> for InvalidTransactionWrapper {
fn from(validation_error: TransactionValidationError) -> Self {
match validation_error {
InvalidEvmTransactionError::GasLimitTooLow => InvalidTransactionWrapper(
TransactionValidationError::GasLimitTooLow => InvalidTransactionWrapper(
InvalidTransaction::Custom(TransactionValidationError::GasLimitTooLow as u8),
),
InvalidEvmTransactionError::GasLimitTooHigh => InvalidTransactionWrapper(
TransactionValidationError::GasLimitTooHigh => InvalidTransactionWrapper(
InvalidTransaction::Custom(TransactionValidationError::GasLimitTooHigh as u8),
),
InvalidEvmTransactionError::GasPriceTooLow => {
InvalidTransactionWrapper(InvalidTransaction::Payment)
}
InvalidEvmTransactionError::PriorityFeeTooHigh => InvalidTransactionWrapper(
InvalidTransaction::Custom(TransactionValidationError::MaxFeePerGasTooLow as u8),
TransactionValidationError::PriorityFeeTooHigh => InvalidTransactionWrapper(
InvalidTransaction::Custom(TransactionValidationError::PriorityFeeTooHigh as u8),
),
InvalidEvmTransactionError::BalanceTooLow => {
TransactionValidationError::BalanceTooLow => {
InvalidTransactionWrapper(InvalidTransaction::Payment)
}
InvalidEvmTransactionError::TxNonceTooLow => {
TransactionValidationError::TxNonceTooLow => {
InvalidTransactionWrapper(InvalidTransaction::Stale)
}
InvalidEvmTransactionError::TxNonceTooHigh => {
TransactionValidationError::TxNonceTooHigh => {
InvalidTransactionWrapper(InvalidTransaction::Future)
}
InvalidEvmTransactionError::InvalidPaymentInput => {
InvalidTransactionWrapper(InvalidTransaction::Payment)
}
InvalidEvmTransactionError::InvalidChainId => InvalidTransactionWrapper(
TransactionValidationError::InvalidFeeInput => InvalidTransactionWrapper(
InvalidTransaction::Custom(TransactionValidationError::InvalidFeeInput as u8),
),
TransactionValidationError::InvalidChainId => InvalidTransactionWrapper(
InvalidTransaction::Custom(TransactionValidationError::InvalidChainId as u8),
),
TransactionValidationError::InvalidSignature => InvalidTransactionWrapper(
InvalidTransaction::Custom(TransactionValidationError::InvalidSignature as u8),
),
TransactionValidationError::GasPriceTooLow => InvalidTransactionWrapper(
InvalidTransaction::Custom(TransactionValidationError::GasPriceTooLow as u8),
),
TransactionValidationError::UnknownError => InvalidTransactionWrapper(
InvalidTransaction::Custom(TransactionValidationError::UnknownError as u8),
),
}
}
}
2 changes: 1 addition & 1 deletion frame/ethereum/src/tests/eip1559.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ fn transaction_with_invalid_chain_id_should_fail_in_block() {
assert_err!(
extrinsic.apply::<Test>(&dispatch_info, 0),
TransactionValidityError::Invalid(InvalidTransaction::Custom(
fp_ethereum::TransactionValidationError::InvalidChainId as u8,
fp_evm::TransactionValidationError::InvalidChainId as u8,
))
);
});
Expand Down
2 changes: 1 addition & 1 deletion frame/ethereum/src/tests/eip2930.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ fn transaction_with_invalid_chain_id_should_fail_in_block() {
assert_err!(
extrinsic.apply::<Test>(&dispatch_info, 0),
TransactionValidityError::Invalid(InvalidTransaction::Custom(
fp_ethereum::TransactionValidationError::InvalidChainId as u8,
fp_evm::TransactionValidationError::InvalidChainId as u8,
))
);
});
Expand Down
2 changes: 1 addition & 1 deletion frame/ethereum/src/tests/legacy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ fn transaction_with_invalid_chain_id_should_fail_in_block() {
assert_err!(
extrinsic.apply::<Test>(&dispatch_info, 0),
TransactionValidityError::Invalid(InvalidTransaction::Custom(
fp_ethereum::TransactionValidationError::InvalidChainId as u8,
fp_evm::TransactionValidationError::InvalidChainId as u8,
))
);
});
Expand Down
25 changes: 12 additions & 13 deletions frame/evm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,8 @@ use fp_account::AccountId20;
use fp_evm::GenesisAccount;
pub use fp_evm::{
Account, CallInfo, CreateInfo, ExecutionInfoV2 as ExecutionInfo, FeeCalculator,
InvalidEvmTransactionError, IsPrecompileResult, LinearCostPrecompile, Log, Precompile,
PrecompileFailure, PrecompileHandle, PrecompileOutput, PrecompileResult, PrecompileSet,
Vicinity,
IsPrecompileResult, LinearCostPrecompile, Log, Precompile, PrecompileFailure, PrecompileHandle,
PrecompileOutput, PrecompileResult, PrecompileSet, TransactionValidationError, Vicinity,
};

pub use self::{
Expand Down Expand Up @@ -495,17 +494,17 @@ pub mod pallet {
TransactionMustComeFromEOA,
}

impl<T> From<InvalidEvmTransactionError> for Error<T> {
fn from(validation_error: InvalidEvmTransactionError) -> Self {
impl<T> From<TransactionValidationError> for Error<T> {
fn from(validation_error: TransactionValidationError) -> Self {
match validation_error {
InvalidEvmTransactionError::GasLimitTooLow => Error::<T>::GasLimitTooLow,
InvalidEvmTransactionError::GasLimitTooHigh => Error::<T>::GasLimitTooHigh,
InvalidEvmTransactionError::GasPriceTooLow => Error::<T>::GasPriceTooLow,
InvalidEvmTransactionError::PriorityFeeTooHigh => Error::<T>::GasPriceTooLow,
InvalidEvmTransactionError::BalanceTooLow => Error::<T>::BalanceLow,
InvalidEvmTransactionError::TxNonceTooLow => Error::<T>::InvalidNonce,
InvalidEvmTransactionError::TxNonceTooHigh => Error::<T>::InvalidNonce,
InvalidEvmTransactionError::InvalidPaymentInput => Error::<T>::GasPriceTooLow,
TransactionValidationError::GasLimitTooLow => Error::<T>::GasLimitTooLow,
TransactionValidationError::GasLimitTooHigh => Error::<T>::GasLimitTooHigh,
TransactionValidationError::BalanceTooLow => Error::<T>::BalanceLow,
TransactionValidationError::TxNonceTooLow => Error::<T>::InvalidNonce,
TransactionValidationError::TxNonceTooHigh => Error::<T>::InvalidNonce,
TransactionValidationError::GasPriceTooLow => Error::<T>::GasPriceTooLow,
TransactionValidationError::PriorityFeeTooHigh => Error::<T>::GasPriceTooLow,
TransactionValidationError::InvalidFeeInput => Error::<T>::GasPriceTooLow,
_ => Error::<T>::Undefined,
}
}
Expand Down
2 changes: 0 additions & 2 deletions primitives/ethereum/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ targets = ["x86_64-unknown-linux-gnu"]
[dependencies]
ethereum = { workspace = true, features = ["with-codec"] }
ethereum-types = { workspace = true }
num_enum = { version = "0.6.1", default-features = false }
scale-codec = { package = "parity-scale-codec", workspace = true }
# Substrate
frame-support = { workspace = true }
Expand All @@ -26,7 +25,6 @@ default = ["std"]
std = [
"ethereum/std",
"ethereum-types/std",
"num_enum/std",
"scale-codec/std",
# Substrate
"frame-support/std",
Expand Down
13 changes: 0 additions & 13 deletions primitives/ethereum/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,6 @@ use frame_support::dispatch::{DispatchErrorWithPostInfo, PostDispatchInfo};
use scale_codec::{Decode, Encode};
use sp_std::{result::Result, vec::Vec};

#[repr(u8)]
#[derive(num_enum::FromPrimitive, num_enum::IntoPrimitive)]
pub enum TransactionValidationError {
#[allow(dead_code)]
#[num_enum(default)]
UnknownError,
InvalidChainId,
InvalidSignature,
GasLimitTooLow,
GasLimitTooHigh,
MaxFeePerGasTooLow,
}

pub trait ValidatedTransaction {
fn apply(
source: H160,
Expand Down
2 changes: 2 additions & 0 deletions primitives/evm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ targets = ["x86_64-unknown-linux-gnu"]

[dependencies]
evm = { workspace = true, features = ["with-codec"] }
num_enum = { version = "0.6.1", default-features = false }
scale-codec = { package = "parity-scale-codec", workspace = true }
scale-info = { workspace = true }
serde = { workspace = true, optional = true }
Expand All @@ -26,6 +27,7 @@ default = ["std"]
std = [
"evm/std",
"evm/with-serde",
"num_enum/std",
"serde/std",
"scale-codec/std",
# Substrate
Expand Down
2 changes: 1 addition & 1 deletion primitives/evm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ pub use self::{
},
validation::{
CheckEvmTransaction, CheckEvmTransactionConfig, CheckEvmTransactionInput,
InvalidEvmTransactionError,
TransactionValidationError,
},
};

Expand Down
Loading

0 comments on commit 55e0861

Please sign in to comment.