Skip to content
This repository has been archived by the owner on Oct 19, 2024. It is now read-only.

feature: contract revert trait #2182

Merged
merged 10 commits into from
Feb 28, 2023
35 changes: 33 additions & 2 deletions ethers-contract/ethers-contract-abigen/src/contract/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,20 @@ impl Context {
#[derive(Clone, #ethers_contract::EthAbiType, #derives)]
pub enum #enum_name {
#( #variants(#variants), )*
/// The standard solidity revert string, with selector
/// Error(string) -- 0x08c379a0
RevertString(::std::string::String),
}

impl #ethers_core::abi::AbiDecode for #enum_name {
fn decode(data: impl AsRef<[u8]>) -> ::core::result::Result<Self, #ethers_core::abi::AbiError> {
let data = data.as_ref();
// NB: This implementation does not include selector information, and ABI encoded types
// are incredibly ambiguous, so it's possible to have bad false positives. Instead, we default
// to a String to minimize amount of decoding attempts
if let Ok(decoded) = <::std::string::String as #ethers_core::abi::AbiDecode>::decode(data) {
gakonst marked this conversation as resolved.
Show resolved Hide resolved
return Ok(Self::RevertString(decoded))
}
Comment on lines +118 to +120
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be below the if let Ok in the macro below? So that we only decode as string as a fallback vs prioritizing it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implementation has never been safe for users to access directly, as it does not include selector information, and ABI encoded types are incredibly ambiguous. It has always thrown bad false positives.

My belief is that strings will be more common than other error types, even for contracts that do not revert, so putting a string in the first attempt results in the fewest decode attempts

#(
if let Ok(decoded) = <#variants as #ethers_core::abi::AbiDecode>::decode(data) {
return Ok(Self::#variants(decoded))
Expand All @@ -124,20 +133,42 @@ impl Context {
#(
Self::#variants(element) => #ethers_core::abi::AbiEncode::encode(element),
)*
Self::RevertString(s) => #ethers_core::abi::AbiEncode::encode(s),
}
}
}

impl #ethers_contract::ContractRevert for #enum_name {
fn valid_selector(selector: [u8; 4]) -> bool {
match selector {
// Error(string) -- 0x08c379a0 -- standard solidity revert
[0x08, 0xc3, 0x79, 0xa0] => true,
#(
_ if selector == <#variants as #ethers_contract::EthError>::selector() => true,
)*
_ => false,
}
}
}


impl ::core::fmt::Display for #enum_name {
fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result {
match self {
#(
Self::#variants(element) => ::core::fmt::Display::fmt(element, f)
),*
Self::#variants(element) => ::core::fmt::Display::fmt(element, f),
)*
Self::RevertString(s) => ::core::fmt::Display::fmt(s, f),
}
}
}

impl ::core::convert::From<::std::string::String> for #enum_name {
fn from(value: String) -> Self {
prestwich marked this conversation as resolved.
Show resolved Hide resolved
Self::RevertString(value)
}
}

#(
impl ::core::convert::From<#variants> for #enum_name {
fn from(value: #variants) -> Self {
Expand Down
11 changes: 10 additions & 1 deletion ethers-contract/src/call.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![allow(clippy::return_self_not_must_use)]

use crate::EthError;
use crate::{error::ContractRevert, EthError};

use super::base::{decode_function_data, AbiError};
use ethers_core::{
Expand Down Expand Up @@ -115,6 +115,15 @@ impl<M: Middleware> ContractError<M> {
self.as_revert().and_then(|data| Err::decode_with_selector(data))
}

/// Decode revert data into a [`ContractRevert`] type. Returns `None` if
/// decoding fails, or if this is not a revert
///
/// This is intended to be used with error enum outputs from `abigen!`
/// contracts
pub fn decode_contract_revert<Err: ContractRevert>(&self) -> Option<Err> {
self.as_revert().and_then(|data| Err::decode_with_selector(data))
}

/// Convert a [`MiddlewareError`] to a `ContractError`
pub fn from_middleware_error(e: M::Error) -> Self {
if let Some(data) = e.as_error_response().and_then(JsonRpcError::as_revert_data) {
Expand Down
43 changes: 41 additions & 2 deletions ethers-contract/src/error.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,46 @@
use ethers_core::{
abi::{AbiDecode, AbiEncode, Tokenizable},
types::{Bytes, Selector},
types::Selector,
utils::id,
};
use ethers_providers::JsonRpcError;
use std::borrow::Cow;

/// A trait for enums unifying [`EthError`] types. This trait is usually used
/// to represent the errors that a specific contract might throw. I.e. all
/// solidity custom errors + revert strings.
///
/// This trait should be accessed via
/// [`crate::ContractError::decode_contract_revert`]. It is generally
/// unnecessary to import this trait into your code.
///
/// # Implementor's Note
///
/// We do not recommend manual implementations of this trait. Instead, use the
/// automatically generated implementation in the [`crate::abigen`] macro
///
/// However, sophisticated users may wish to represent the errors of multiple
/// contracts as a single unified enum. E.g. if your contract calls Uniswap,
/// you may wish to implement this on `pub enum MyContractOrUniswapErrors`.
/// In that case, it should be straightforward to delegate to the inner types.
pub trait ContractRevert: AbiDecode + AbiEncode + Send + Sync {
/// Decode the error from EVM revert data including an Error selector
Comment on lines +26 to +27
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this only derived for enums?
because this clashes with EthError

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, this trait is only for unifying multiple potential ethereum errors into 1 type. Basically this should be thought of as "the errors associated with a specific contract's ABI".

I thought about breaking out a super-trait that had the shared methods on it, and decided that it was too complex for users

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i also thought about modifying AbiDecode to include decode_with_selector(selector: Selector, bytes: &[u8]) but didn't want to get into lower level changes

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can also consult the docstring, btw

/// A trait for enums unifying [`EthError`] types.
///
/// We do not recommend manual implementations of this trait. Instead, use the
/// automatically generated implementation in the [`crate::abigen`] macro```

fn decode_with_selector(data: &[u8]) -> Option<Self> {
if data.len() < 4 {
return None
}
let selector = data[..4].try_into().expect("checked by len");
if !Self::valid_selector(selector) {
return None
}
<Self as AbiDecode>::decode(&data[4..]).ok()
}

/// `true` if the selector corresponds to an error that this contract can
/// revert. False otherwise
fn valid_selector(selector: Selector) -> bool;
}

/// A helper trait for types that represents a custom error type
pub trait EthError: Tokenizable + AbiDecode + AbiEncode + Send + Sync {
/// Attempt to decode from a [`JsonRpcError`] by extracting revert data
Expand All @@ -16,7 +51,7 @@ pub trait EthError: Tokenizable + AbiDecode + AbiEncode + Send + Sync {
}

/// Decode the error from EVM revert data including an Error selector
fn decode_with_selector(data: &Bytes) -> Option<Self> {
fn decode_with_selector(data: &[u8]) -> Option<Self> {
// This will return none if selector mismatch.
<Self as AbiDecode>::decode(data.strip_prefix(&Self::selector())?).ok()
}
Expand All @@ -41,6 +76,10 @@ impl EthError for String {
fn abi_signature() -> Cow<'static, str> {
Cow::Borrowed("Error(string)")
}

fn selector() -> Selector {
[0x08, 0xc3, 0x79, 0xa0]
}
}

#[cfg(test)]
Expand Down
2 changes: 1 addition & 1 deletion ethers-contract/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ mod call;
pub use call::{ContractCall, ContractError, EthCall, FunctionCall};

mod error;
pub use error::EthError;
pub use error::{ContractRevert, EthError};

mod factory;
pub use factory::{ContractDeployer, ContractDeploymentTx, ContractFactory, DeploymentTxFactory};
Expand Down
12 changes: 9 additions & 3 deletions ethers-contract/tests/it/abigen.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
//! Test cases to validate the `abigen!` macro
use ethers_contract::{abigen, EthCall, EthEvent};
use ethers_contract::{abigen, ContractError, EthCall, EthError, EthEvent};
use ethers_core::{
abi::{AbiDecode, AbiEncode, Address, Tokenizable},
types::{transaction::eip2718::TypedTransaction, Eip1559TransactionRequest, U256},
types::{transaction::eip2718::TypedTransaction, Bytes, Eip1559TransactionRequest, U256},
utils::Anvil,
};
use ethers_providers::{MockProvider, Provider};
Expand Down Expand Up @@ -648,7 +648,13 @@ fn can_generate_seaport_1_0() {
let err = SeaportErrors::BadContractSignature(BadContractSignature::default());

let encoded = err.clone().encode();
assert_eq!(err, SeaportErrors::decode(encoded).unwrap());
assert_eq!(err, SeaportErrors::decode(encoded.clone()).unwrap());

let with_selector: Bytes =
BadContractSignature::selector().into_iter().chain(encoded).collect();
let contract_err = ContractError::<Provider<MockProvider>>::Revert(with_selector);

assert_eq!(contract_err.decode_contract_revert(), Some(err));

let _err = SeaportErrors::ConsiderationNotMet(ConsiderationNotMet {
order_index: U256::zero(),
Expand Down
12 changes: 12 additions & 0 deletions ethers-core/src/types/bytes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,18 @@ pub struct Bytes(
pub bytes::Bytes,
);

impl FromIterator<u8> for Bytes {
fn from_iter<T: IntoIterator<Item = u8>>(iter: T) -> Self {
iter.into_iter().collect::<bytes::Bytes>().into()
}
}

impl<'a> FromIterator<&'a u8> for Bytes {
fn from_iter<T: IntoIterator<Item = &'a u8>>(iter: T) -> Self {
iter.into_iter().copied().collect::<bytes::Bytes>().into()
}
}

impl Bytes {
/// Creates a new empty `Bytes`.
///
Expand Down