Skip to content

Commit

Permalink
runtime-sdk/modules/evm: Simplify revert reason encoding
Browse files Browse the repository at this point in the history 
Instead of attempting to decode simple ABI-encoded error strings, just
return the entire data, base64-encoded.
  • Loading branch information
@kostko
kostko committed Sep 8, 2023
1 parent 43bb365 commit f11c1f8
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 101 deletions.
45 changes: 4 additions & 41 deletions runtime-sdk/modules/evm/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,47 +214,10 @@ impl From<evm::ExitFatal> for Error {
fn process_evm_result(exit_reason: evm::ExitReason, data: Vec<u8>) -> Result<Vec<u8>, Error> {
match exit_reason {
evm::ExitReason::Succeed(_) => Ok(data),
evm::ExitReason::Revert(_) => {
if data.is_empty() {
return Err(Error::Reverted("no revert reason".to_string()));
}

// Decode revert reason, format is as follows:
//
// 08c379a0 <- Function selector
// 0000000000000000000000000000000000000000000000000000000000000020 <- Offset of string return value
// 0000000000000000000000000000000000000000000000000000000000000047 <- Length of string return value (the revert reason)
// 6d7946756e6374696f6e206f6e6c79206163636570747320617267756d656e74 <- First 32 bytes of the revert reason
// 7320776869636820617265206772656174686572207468616e206f7220657175 <- Next 32 bytes of the revert reason
// 616c20746f203500000000000000000000000000000000000000000000000000 <- Last 7 bytes of the revert reason
//
const ERROR_STRING_SELECTOR: &[u8] = &[0x08, 0xc3, 0x79, 0xa0]; // Keccak256("Error(string)")
const FIELD_OFFSET_START: usize = 4;
const FIELD_LENGTH_START: usize = FIELD_OFFSET_START + 32;
const FIELD_REASON_START: usize = FIELD_LENGTH_START + 32;
const MIN_SIZE: usize = FIELD_REASON_START;
const MAX_REASON_SIZE: usize = 1024;

let max_raw_len = data.len().clamp(0, MAX_REASON_SIZE);
if data.len() < MIN_SIZE || !data.starts_with(ERROR_STRING_SELECTOR) {
return Err(Error::Reverted(base64::encode(&data[..max_raw_len])));
}
// Decode and validate length.
let mut length =
primitive_types::U256::from(&data[FIELD_LENGTH_START..FIELD_LENGTH_START + 32])
.low_u32() as usize;
if FIELD_REASON_START + length > data.len() {
return Err(Error::Reverted(base64::encode(&data[..max_raw_len])));
}
// Make sure that this doesn't ever return huge reason values as this is at least
// somewhat contract-controlled.
if length > MAX_REASON_SIZE {
length = MAX_REASON_SIZE;
}
let reason =
String::from_utf8_lossy(&data[FIELD_REASON_START..FIELD_REASON_START + length]);
Err(Error::Reverted(reason.to_string()))
}
// Encode raw revert reason as Base64, up to the maximum length of 1024 bytes.
evm::ExitReason::Revert(_) => Err(Error::Reverted(base64::encode(
&data[..data.len().clamp(0, 1024)],
))),
evm::ExitReason::Error(err) => Err(err.into()),
evm::ExitReason::Fatal(err) => Err(err.into()),
}
Expand Down
31 changes: 31 additions & 0 deletions runtime-sdk/modules/evm/src/mock.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,3 +85,34 @@ pub fn load_contract_bytecode(raw: &str) -> Vec<u8> {
Vec::from_hex(raw.split_whitespace().collect::<String>())
.expect("compiled contract should be a valid hex string")
}

/// Decode a basic revert reason.
pub fn decode_reverted(msg: &str) -> Option<String> {
decode_reverted_abi(
msg,
ethabi::AbiError {
name: "Error".to_string(),
inputs: vec![ethabi::Param {
name: "message".to_string(),
kind: ethabi::ParamType::String,
internal_type: None,
}],
},
)?
.pop()
.unwrap()
.into_string()
}

/// Decode a revert reason accoording to the given API.
pub fn decode_reverted_abi(msg: &str, abi: ethabi::AbiError) -> Option<Vec<ethabi::Token>> {
// Trim the optional reverted prefix.
let msg = msg.trim_start_matches("reverted: ");

let raw = base64::decode(msg).ok()?;
// Strip (and validate) error signature.
let signature = abi.signature();
let raw = raw.strip_prefix(&signature.as_bytes()[..4])?;

Some(abi.decode(raw).unwrap())
}
6 changes: 3 additions & 3 deletions runtime-sdk/modules/evm/src/precompile/subcall.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ mod test {

use crate as evm;
use crate::{
mock::{load_contract_bytecode, EvmSigner},
mock::{decode_reverted, load_contract_bytecode, EvmSigner},
types::{self, H160},
Config as _,
};
Expand Down Expand Up @@ -364,7 +364,7 @@ mod test {
{
assert_eq!(module, "evm");
assert_eq!(code, 8);
assert_eq!(message, "reverted: subcall failed");
assert_eq!(decode_reverted(&message).unwrap(), "subcall failed");
} else {
panic!("call should fail due to delegatecall");
}
Expand Down Expand Up @@ -416,7 +416,7 @@ mod test {
{
assert_eq!(module, "evm");
assert_eq!(code, 8);
assert_eq!(message, "reverted: subcall failed");
assert_eq!(decode_reverted(&message).unwrap(), "subcall failed");
} else {
panic!("call should fail due to re-entrancy not being allowed");
}
Expand Down
71 changes: 14 additions & 57 deletions runtime-sdk/modules/evm/src/test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use oasis_runtime_sdk::{

use crate::{
derive_caller,
mock::{load_contract_bytecode, EvmSigner},
mock::{decode_reverted, load_contract_bytecode, EvmSigner},
process_evm_result,
types::{self, H160},
Config, Error, Genesis, Module as EVMModule,
Expand Down Expand Up @@ -815,65 +815,17 @@ fn test_c10l_evm_runtime() {
fn test_revert_reason_decoding() {
let long_reason = vec![0x61; 1050];
let long_reason_hex = hex::encode(&long_reason);
let long_reason_str = String::from_utf8(long_reason).unwrap();
let long_reason_hex = &[
"08c379a0\
0000000000000000000000000000000000000000000000000000000000000020\
000000000000000000000000000000000000000000000000000000000000041a",
&long_reason_hex,
]
.concat();

let tcs = vec![
// Valid values.
// Valid value.
(
"08c379a0\
0000000000000000000000000000000000000000000000000000000000000020\
0000000000000000000000000000000000000000000000000000000000000018\
4461692f696e73756666696369656e742d62616c616e63650000000000000000",
"Dai/insufficient-balance",
"00010203040506",
vec![0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06],
),
(
"08c379a0\
0000000000000000000000000000000000000000000000000000000000000020\
0000000000000000000000000000000000000000000000000000000000000047\
6d7946756e6374696f6e206f6e6c79206163636570747320617267756d656e74\
7320776869636820617265206772656174686572207468616e206f7220657175\
616c20746f203500000000000000000000000000000000000000000000000000",
"myFunction only accepts arguments which are greather than or equal to 5",
),
// Valid value, empty reason.
(
"08c379a0\
0000000000000000000000000000000000000000000000000000000000000020\
0000000000000000000000000000000000000000000000000000000000000000",
"",
),
// Valid value, reason too long and should be truncated.
(long_reason_hex, &long_reason_str[..1024]),
// Reason too long and should be truncated.
(&long_reason_hex, (&long_reason[..1024]).to_vec()),
// No revert reason.
("", "no revert reason"),
// Malformed output, incorrect selector and bad length.
(
"BADBADBADBADBADBAD",
"utututututut",
),
// Malformed output, bad selector.
(
"BAAAAAAD\
0000000000000000000000000000000000000000000000000000000000000020\
0000000000000000000000000000000000000000000000000000000000000018\
4461692f696e73756666696369656e742d62616c616e63650000000000000000",
"uqqqrQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABhEYWkvaW5zdWZmaWNpZW50LWJhbGFuY2UAAAAAAAAAAA==",
),
// Malformed output, corrupted length.
(
"08c379a0\
0000000000000000000000000000000000000000000000000000000000000020\
00000000000000000000000000000000000000000000000000000000FFFFFFFF\
4461692f696e73756666696369656e742d62616c616e63650000000000000000",
"CMN5oAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAP////9EYWkvaW5zdWZmaWNpZW50LWJhbGFuY2UAAAAAAAAAAA==",
),
("", b"".to_vec()),
];

for tc in tcs {
Expand All @@ -882,7 +834,9 @@ fn test_revert_reason_decoding() {
.unwrap_err();
match err {
Error::Reverted(reason) => {
assert_eq!(&reason, tc.1, "revert reason should be decoded correctly");
let decoded =
base64::decode(reason).expect("revert reason should be base64 encoded");
assert_eq!(decoded, tc.1, "revert reason should match expected value");
}
_ => panic!("expected Error::Reverted(_) variant"),
}
Expand Down Expand Up @@ -994,7 +948,10 @@ fn test_fee_refunds() {
{
assert_eq!(module, "evm");
assert_eq!(code, 8);
assert_eq!(message, "reverted: ERC20: transfer amount exceeds balance");
assert_eq!(
decode_reverted(&message).unwrap(),
"ERC20: transfer amount exceeds balance"
);
} else {
panic!("call should revert");
}
Expand Down

0 comments on commit f11c1f8

Please sign in to comment.