From b2f6347637df6fc46fec855bf1bb000cd74ade84 Mon Sep 17 00:00:00 2001 From: Christoph Burgdorf Date: Fri, 26 Mar 2021 18:48:19 +0100 Subject: [PATCH 1/3] Add tests that cover how solidity handles revert reason strings --- .../tests/fixtures/solidity/revert_test.sol | 13 ++ compiler/tests/solidity.rs | 44 ++++++ compiler/tests/utils.rs | 125 +++++++++++++++++- newsfragments/342.internal.md | 2 + 4 files changed, 177 insertions(+), 7 deletions(-) create mode 100644 compiler/tests/fixtures/solidity/revert_test.sol create mode 100644 compiler/tests/solidity.rs create mode 100644 newsfragments/342.internal.md diff --git a/compiler/tests/fixtures/solidity/revert_test.sol b/compiler/tests/fixtures/solidity/revert_test.sol new file mode 100644 index 0000000000..90728fe9d2 --- /dev/null +++ b/compiler/tests/fixtures/solidity/revert_test.sol @@ -0,0 +1,13 @@ +contract Foo { + function revert_me() public pure returns(uint){ + revert("Not enough Ether provided."); + } + + function revert_with_long_string() public pure returns(uint){ + revert("A muuuuuch longer reason string that consumes multiple words"); + } + + function revert_with_empty_string() public pure returns(uint){ + revert(""); + } +} \ No newline at end of file diff --git a/compiler/tests/solidity.rs b/compiler/tests/solidity.rs new file mode 100644 index 0000000000..c79be442ff --- /dev/null +++ b/compiler/tests/solidity.rs @@ -0,0 +1,44 @@ +//! Solidity tests that help us prove assumptions about how Solidty handles +//! certain things + +#![cfg(feature = "solc-backend")] +use rstest::rstest; + +mod utils; +use utils::*; + +#[rstest( + method, + reason, + case("revert_me", "Not enough Ether provided."), + case( + "revert_with_long_string", + "A muuuuuch longer reason string that consumes multiple words" + ), + case("revert_with_empty_string", "") +)] +fn test_revert_string_reason(method: &str, reason: &str) { + with_executor(&|mut executor| { + let harness = deploy_solidity_contract(&mut executor, "revert_test.sol", "Foo", &[]); + + let exit = harness.capture_call(&mut executor, method, &[]); + + let expected_reason = format!("0x{}", hex::encode(encode_error_reason(reason))); + if let evm::Capture::Exit((evm::ExitReason::Revert(_), output)) = exit { + assert_eq!(format!("0x{}", hex::encode(&output)), expected_reason); + } else { + panic!("failed") + }; + }) +} + +#[rstest(reason_str, expected_encoding, + case("Not enough Ether provided.", "0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000001a4e6f7420656e6f7567682045746865722070726f76696465642e000000000000"), + case("A muuuuuch longer reason string that consumes multiple words", "0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000003c41206d75757575756368206c6f6e67657220726561736f6e20737472696e67207468617420636f6e73756d6573206d756c7469706c6520776f72647300000000"), + case("", "0x08c379a000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000000"), + case("foo", "0x08c379a000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000003666f6f0000000000000000000000000000000000000000000000000000000000"), +)] +fn test_revert_reason_encoding(reason_str: &str, expected_encoding: &str) { + let encoded = encode_error_reason(reason_str); + assert_eq!(format!("0x{}", hex::encode(&encoded)), expected_encoding); +} diff --git a/compiler/tests/utils.rs b/compiler/tests/utils.rs index 1aa2c96806..d573f51474 100644 --- a/compiler/tests/utils.rs +++ b/compiler/tests/utils.rs @@ -202,9 +202,64 @@ pub fn deploy_contract( .contracts .get(contract_name) .expect("could not find contract in fixture"); - let abi = ethabi::Contract::load(StringReader::new(&compiled_contract.json_abi)) - .expect("unable to load the ABI"); - let mut bytecode = hex::decode(&compiled_contract.bytecode).expect("failed to decode bytecode"); + + return _deploy_contract( + executor, + &compiled_contract.bytecode, + &compiled_contract.json_abi, + init_params, + ); +} + +#[allow(dead_code)] +pub fn deploy_solidity_contract( + executor: &mut Executor, + fixture: &str, + contract_name: &str, + init_params: &[ethabi::Token], +) -> ContractHarness { + let src = fs::read_to_string(format!("tests/fixtures/solidity/{}", fixture)) + .expect("unable to read fixture file") + .replace("\n", "") + .replace("\"", "\\\""); + + let (bytecode, abi) = + compile_solidity_contract(contract_name, &src).expect("Could not compile contract"); + + return _deploy_contract(executor, &bytecode, &abi, init_params); +} + +#[allow(dead_code)] +pub fn encode_error_reason(reason: &str) -> Vec { + // Function selector for Error(string) + const SELECTOR: &str = "08c379a0"; + // Data offset + const DATA_OFFSET: &str = "0000000000000000000000000000000000000000000000000000000000000020"; + + // Length of the string padded to 32 bit hex + let string_len = format!("{:0>64x}", reason.len()); + + let mut string_bytes = reason.as_bytes().to_vec(); + while string_bytes.len() % 32 != 0 { + string_bytes.push(0) + } + // The bytes of the string itself, right padded to consume a multiple of 32 + // bytes + let string_bytes = hex::encode(&string_bytes); + + let all = format!("{}{}{}{}", SELECTOR, DATA_OFFSET, string_len, string_bytes); + hex::decode(&all).expect(&format!("No valid hex: {}", &all)) +} + +fn _deploy_contract( + executor: &mut Executor, + bytecode: &str, + abi: &str, + init_params: &[ethabi::Token], +) -> ContractHarness { + let abi = ethabi::Contract::load(StringReader::new(abi)).expect("unable to load the ABI"); + + let mut bytecode = hex::decode(bytecode).expect("failed to decode bytecode"); if let Some(constructor) = &abi.constructor { bytecode = constructor.encode_input(bytecode, init_params).unwrap() @@ -225,6 +280,36 @@ pub fn deploy_contract( panic!("Failed to create contract") } +pub fn compile_solidity_contract(name: &str, solidity_src: &str) -> Result<(String, String), ()> { + let solc_config = r#" + { + "language": "Solidity", + "sources": { "input.sol": { "content": "{src}" } }, + "settings": { + "outputSelection": { "*": { "*": ["*"], "": [ "*" ] } } + } + } + "#; + let solc_config = solc_config.replace("{src}", &solidity_src); + + let raw_output = solc::compile(&solc_config); + + let output: serde_json::Value = + serde_json::from_str(&raw_output).expect("Unable to compile contract"); + + let bytecode = output["contracts"]["input.sol"][name]["evm"]["bytecode"]["object"] + .to_string() + .replace("\"", ""); + + let abi = output["contracts"]["input.sol"][name]["abi"].to_string(); + + if [&bytecode, &abi].iter().any(|val| val == &"null") { + return Err(()); + } + + Ok((bytecode, abi)) +} + #[allow(dead_code)] pub fn load_contract(address: H160, fixture: &str, contract_name: &str) -> ContractHarness { let src = fs::read_to_string(format!("tests/fixtures/{}", fixture)) @@ -246,6 +331,34 @@ pub fn test_runtime_functions( functions: Vec, test_statements: Vec, ) { + let (reason, _) = execute_runtime_functions(executor, functions, test_statements); + if !matches!(reason, ExitReason::Succeed(_)) { + panic!("Runtime function test failed: {:?}", reason) + } +} + +#[allow(dead_code)] +pub fn test_runtime_functions_revert( + executor: &mut Executor, + functions: Vec, + test_statements: Vec, + expected_output: &[u8], +) { + let (reason, output) = execute_runtime_functions(executor, functions, test_statements); + if output != expected_output { + panic!("Runtime function test failed (wrong output): {:?}", output) + } + + if !matches!(reason, ExitReason::Revert(_)) { + panic!("Runtime function did not revert: {:?}", reason) + } +} + +fn execute_runtime_functions( + executor: &mut Executor, + functions: Vec, + test_statements: Vec, +) -> (ExitReason, Vec) { let all_statements = [functions, test_statements].concat(); let yul_code = yul::Object { name: identifier! { Contract }, @@ -259,7 +372,7 @@ pub fn test_runtime_functions( .expect("failed to compile Yul"); let bytecode = hex::decode(&bytecode).expect("failed to decode bytecode"); - if let evm::Capture::Exit((reason, _, _)) = executor.create( + if let evm::Capture::Exit((reason, _, output)) = executor.create( address(DEFAULT_CALLER), evm_runtime::CreateScheme::Legacy { caller: address(DEFAULT_CALLER), @@ -268,9 +381,7 @@ pub fn test_runtime_functions( bytecode, None, ) { - if !matches!(reason, ExitReason::Succeed(_)) { - panic!("Runtime function test failed: {:?}", reason) - } + (reason, output) } else { panic!("EVM trap during test") } diff --git a/newsfragments/342.internal.md b/newsfragments/342.internal.md new file mode 100644 index 0000000000..5997affe1c --- /dev/null +++ b/newsfragments/342.internal.md @@ -0,0 +1,2 @@ +Added support for running tests against solidity fixtures. +Also added tests that cover how solidity encodes revert reason strings. \ No newline at end of file From 78518a513e9b299f9531bdeca80400f840685ce6 Mon Sep 17 00:00:00 2001 From: Christoph Burgdorf Date: Mon, 19 Apr 2021 16:21:52 +0200 Subject: [PATCH 2/3] Add revert_with_reason_string runtime function --- compiler/src/yul/runtime/functions/data.rs | 56 +++++++++---- compiler/tests/runtime.rs | 93 +++++++++++++--------- compiler/tests/utils.rs | 79 +++++++++++++----- 3 files changed, 155 insertions(+), 73 deletions(-) diff --git a/compiler/src/yul/runtime/functions/data.rs b/compiler/src/yul/runtime/functions/data.rs index 8b8fc49355..fabe5075b8 100644 --- a/compiler/src/yul/runtime/functions/data.rs +++ b/compiler/src/yul/runtime/functions/data.rs @@ -3,30 +3,31 @@ use yultsur::*; /// Return all data runtime functions pub fn all() -> Vec { vec![ - avail(), - alloc(), alloc_mstoren(), - free(), + alloc(), + avail(), + bytes_mcopys(), + bytes_scopym(), + bytes_scopys(), + bytes_sloadn(), + bytes_sstoren(), ccopym(), + ceil32(), + cloadn(), + free(), load_data_string(), + map_value_ptr(), + mcopym(), mcopys(), + mloadn(), + mstoren(), + revert_with_reason_string(), scopym(), - mcopym(), scopys(), - mloadn(), + set_zero(), sloadn(), - bytes_sloadn(), - cloadn(), - mstoren(), sstoren(), - bytes_sstoren(), - bytes_mcopys(), - bytes_scopym(), - bytes_scopys(), - map_value_ptr(), - ceil32(), ternary(), - set_zero(), ] } @@ -380,3 +381,28 @@ pub fn load_data_string() -> yul::Statement { } } } + +/// Revert with encoded reason string +pub fn revert_with_reason_string() -> yul::Statement { + function_definition! { + function revert_with_reason_string(reason) { + // Function selector for Error(string) + (let ptr := alloc_mstoren(0x08C379A0, 4)) + + // Write the (fixed) data offset into the next 32 bytes of memory + (pop((alloc_mstoren(0x0000000000000000000000000000000000000000000000000000000000000020, 32)))) + + // Read the size of the string + (let reason_size := mloadn(reason, 32)) + + //Copy the whole reason string (length + data) to the current segment of memory + (pop((mcopym(reason , (add(reason_size, 32)))))) + + // Right pad the reason bytes to a multiple of 32 bytes + (let padding := sub((ceil32(reason_size)), reason_size)) + (pop((alloc(padding)))) + + (revert(ptr, (add(68, (add(reason_size, padding)))))) + } + } +} diff --git a/compiler/tests/runtime.rs b/compiler/tests/runtime.rs index 7dc2d3db8e..cc1cbcf417 100644 --- a/compiler/tests/runtime.rs +++ b/compiler/tests/runtime.rs @@ -2,6 +2,7 @@ #![cfg(feature = "solc-backend")] use fe_compiler::yul::runtime::functions; +use rstest::rstest; use yultsur::*; mod utils; @@ -11,6 +12,7 @@ use fe_analyzer::namespace::types::{ Integer, Struct, }; +use fe_common::utils::keccak; use utils::*; macro_rules! assert_eq { @@ -23,13 +25,37 @@ macro_rules! assert_eq { }; } +#[rstest( + reason, + case("foo"), + case("A very looooooooooooooong reason that consumes multiple words") +)] +fn test_revert_with_reason_string(reason: &str) { + let reason_id = format!(r#""{}""#, keccak::full(reason.as_bytes())); + + with_executor(&|mut executor| { + test_runtime_functions_revert( + &mut executor, + Runtime::default() + .with_data( + vec![yul::Data { name: keccak::full(reason.as_bytes()), value: reason.to_owned() }] + ) + .with_test_statements( + statements! { + (let reason := load_data_string((dataoffset([literal_expression! { (reason_id) }])), (datasize([literal_expression! { (reason_id) }])))) + (revert_with_reason_string(reason)) + }), + &encode_error_reason(reason) + ); + }) +} + #[test] fn test_runtime_alloc_and_avail() { with_executor(&|mut executor| { test_runtime_functions( &mut executor, - functions::std(), - statements! { + Runtime::default().with_test_statements(statements! { (let a := avail()) (let b := alloc(5)) (let c := alloc(10)) @@ -38,7 +64,7 @@ fn test_runtime_alloc_and_avail() { [assert_eq!(b, a)] [assert_eq!(c, (add(b, 5)))] [assert_eq!(d, (add(c, 10)))] - }, + }), ); }) } @@ -48,8 +74,7 @@ fn test_runtime_mcopys() { with_executor(&|mut executor| { test_runtime_functions( &mut executor, - functions::std(), - statements! { + Runtime::default().with_test_statements(statements! { (let a := 0x0111114211111111011111112342311101111112221151110111111111111111) (let b := 0x0111111234111111011123411111111101112431111111110111111234411111) (let c := 0x0111341111111111011111111123411101111123411111110111111234111111) @@ -75,7 +100,7 @@ fn test_runtime_mcopys() { [assert_eq!(b, (sload(47)))] [assert_eq!(c, (sload(48)))] [assert_eq!(d, (sload(49)))] - }, + }), ); }) } @@ -85,8 +110,7 @@ fn test_runtime_scopym() { with_executor(&|mut executor| { test_runtime_functions( &mut executor, - functions::std(), - statements! { + Runtime::default().with_test_statements(statements! { (let a := 0x0111114211111111011111112342311101111112221151110111111111111111) (let b := 0x0111111234111111011123411111111101112431111111110111111234411111) (let c := 0x0111341111111111011111111123411101111123411111110111111234111111) @@ -118,7 +142,7 @@ fn test_runtime_scopym() { [assert_eq!(b, (mload(ptr6)))] [assert_eq!(c, (mload(ptr7)))] [assert_eq!(d, (mload(ptr8)))] - }, + }), ); }) } @@ -128,8 +152,7 @@ fn test_runtime_mloadn() { with_executor(&|mut executor| { test_runtime_functions( &mut executor, - functions::std(), - statements! { + Runtime::default().with_test_statements(statements! { (let a := 0x4200000000000000000000000000000000000000000000000000000000420026) (mstore(100, a)) @@ -137,7 +160,7 @@ fn test_runtime_mloadn() { [assert_eq!(0x420026, (mloadn(129, 3)))] [assert_eq!(0x26, (mloadn(130, 2)))] [assert_eq!(0x26, (mloadn(131, 1)))] - }, + }), ); }) } @@ -147,8 +170,7 @@ fn test_runtime_storage_sanity() { with_executor(&|mut executor| { test_runtime_functions( &mut executor, - vec![], - statements! { + Runtime::new().with_test_statements(statements! { (let a := 0x4200000000000000000000000000000000000000000000000000000000000026) (let b := 0x9900000000000000000000000000000000000000000000000000000000000077) (sstore(0, a)) @@ -156,7 +178,7 @@ fn test_runtime_storage_sanity() { [assert_eq!(a, (sload(0)))] [assert_eq!(b, (sload(1)))] - }, + }), ); }) } @@ -166,8 +188,7 @@ fn test_runtime_sloadn() { with_executor(&|mut executor| { test_runtime_functions( &mut executor, - functions::std(), - statements! { + Runtime::default().with_test_statements(statements! { (let a := 0x4200000000000000000000000000000000000000000000000000000000000026) (let b := 0x9900530000003900000000000000000000000000000000000000000000000077) (sstore(1000, a)) @@ -184,7 +205,7 @@ fn test_runtime_sloadn() { [assert_eq!(0x77, (sloadn(1001, 31, 1)))] [assert_eq!(0x990053, (sloadn(1001, 0, 3)))] [assert_eq!(0x5300000039, (sloadn(1001, 2, 5)))] - }, + }), ); }) } @@ -194,8 +215,7 @@ fn test_runtime_sstoren() { with_executor(&|mut executor| { test_runtime_functions( &mut executor, - functions::std(), - statements! { + Runtime::default().with_test_statements(statements! { (let a := 0x0111111111111111011111111111111101111111111111110111111111111111) // dashes indicate which bytes are to be replaced in this test // 0----2 8----10 15----------------23 31--32 @@ -213,7 +233,7 @@ fn test_runtime_sstoren() { (sstoren(1000, 0, 32, c)) [assert_eq!(c, (sload(1000)))] - }, + }), ); }) } @@ -223,11 +243,10 @@ fn test_runtime_ceil32() { with_executor(&|mut executor| { test_runtime_functions( &mut executor, - functions::std(), - statements! { + Runtime::default().with_test_statements(statements! { [assert_eq!(32, (ceil32(29)))] [assert_eq!(256, (ceil32(225)))] - }, + }), ); }) } @@ -237,14 +256,13 @@ fn test_runtime_ternary() { with_executor(&|mut executor| { test_runtime_functions( &mut executor, - functions::std(), - statements! { + Runtime::default().with_test_statements(statements! { (let a := ternary(0, 42, 26)) (let b := ternary(1, 42, 26)) [assert_eq!(a, 26)] [assert_eq!(b, 42)] - }, + }), ); }) } @@ -254,8 +272,7 @@ fn test_runtime_abi_unpack() { with_executor(&|mut executor| { test_runtime_functions( &mut executor, - functions::std(), - statements! { + Runtime::default().with_test_statements(statements! { (let a := 0x0042002600530000000000000000000000000000000000000000000000000000) (let packed := alloc_mstoren(a, 32)) (let unpacked := avail()) @@ -268,7 +285,7 @@ fn test_runtime_abi_unpack() { [assert_eq!(elem0, 0x0042)] [assert_eq!(elem1, 0x0026)] [assert_eq!(elem2, 0x0053)] - }, + }), ); }) } @@ -278,7 +295,7 @@ fn test_keccak256() { with_executor(&|mut executor| { test_runtime_functions( &mut executor, - functions::std(), + Runtime::default().with_test_statements( statements! { (let num := 63806209331542711802848847270949280092855778197726125910674179583545433573378) (let result :=109966633016701122630199943745061001312678661825260870342362413625737614346915) @@ -288,6 +305,7 @@ fn test_keccak256() { (mstore(0, num)) [assert_eq!(result, (keccak256(0, 32)))] }, + ) ); }) } @@ -297,8 +315,7 @@ fn test_runtime_set_zero() { with_executor(&|mut executor| { test_runtime_functions( &mut executor, - functions::std(), - statements! { + Runtime::default().with_test_statements(statements! { (let a := 0x1111111111111111111111111111111111111111111111111111111111111111) (let b := 0x1111110000000000000000000000000000000000000000000000001111111111) (let c := 0x1111100000000000000000000000000000000000000000000000000000000000) @@ -307,7 +324,7 @@ fn test_runtime_set_zero() { [assert_eq!(0x11, (set_zero(0, 248, a)))] [assert_eq!(b, (set_zero(24, 216, a)))] [assert_eq!(c, (set_zero(20, 256, a)))] - }, + }), ); }) } @@ -330,7 +347,7 @@ fn test_runtime_house_struct() { with_executor(&|mut executor| { test_runtime_functions( &mut executor, - [functions::std(), house_api.clone()].concat(), + Runtime::new().with_functions([functions::std(), house_api.clone()].concat()).with_test_statements( statements! { (let price := 42) (let size := 26) @@ -369,6 +386,7 @@ fn test_runtime_house_struct() { [assert_eq!(rooms, (bytes_sloadn((struct_House_get_rooms_ptr(house_storage)), 1)))] [assert_eq!(vacant, (bytes_sloadn((struct_House_get_vacant_ptr(house_storage)), 1)))] }, + ) ); }) } @@ -378,10 +396,9 @@ fn checked_exp_signed() { with_executor(&|mut executor| { test_runtime_functions( &mut executor, - functions::std(), - statements! { + Runtime::default().with_test_statements(statements! { [assert_eq!(4, (checked_exp_signed(2, 2, 0, 100)))] - }, + }), ); }) } diff --git a/compiler/tests/utils.rs b/compiler/tests/utils.rs index d573f51474..7c8f7b451e 100644 --- a/compiler/tests/utils.rs +++ b/compiler/tests/utils.rs @@ -4,6 +4,7 @@ use evm_runtime::{ Handler, }; use fe_compiler as compiler; +use fe_compiler::yul::runtime::functions; use primitive_types::{ H160, H256, @@ -326,12 +327,8 @@ pub fn load_contract(address: H160, fixture: &str, contract_name: &str) -> Contr } #[allow(dead_code)] -pub fn test_runtime_functions( - executor: &mut Executor, - functions: Vec, - test_statements: Vec, -) { - let (reason, _) = execute_runtime_functions(executor, functions, test_statements); +pub fn test_runtime_functions(executor: &mut Executor, runtime: Runtime) { + let (reason, _) = execute_runtime_functions(executor, runtime); if !matches!(reason, ExitReason::Succeed(_)) { panic!("Runtime function test failed: {:?}", reason) } @@ -340,11 +337,10 @@ pub fn test_runtime_functions( #[allow(dead_code)] pub fn test_runtime_functions_revert( executor: &mut Executor, - functions: Vec, - test_statements: Vec, + runtime: Runtime, expected_output: &[u8], ) { - let (reason, output) = execute_runtime_functions(executor, functions, test_statements); + let (reason, output) = execute_runtime_functions(executor, runtime); if output != expected_output { panic!("Runtime function test failed (wrong output): {:?}", output) } @@ -354,20 +350,63 @@ pub fn test_runtime_functions_revert( } } -fn execute_runtime_functions( - executor: &mut Executor, +pub struct Runtime { functions: Vec, test_statements: Vec, -) -> (ExitReason, Vec) { - let all_statements = [functions, test_statements].concat(); - let yul_code = yul::Object { - name: identifier! { Contract }, - code: code! { [all_statements...] }, - objects: vec![], - data: vec![], + data: Vec, +} + +#[allow(dead_code)] +impl Runtime { + /// Create a `Runtime` instance with all `std` functions. + pub fn default() -> Runtime { + Runtime::new().with_functions(functions::std()) + } + + /// Create a new `Runtime` instance. + pub fn new() -> Runtime { + Runtime { + functions: vec![], + test_statements: vec![], + data: vec![], + } } - .to_string() - .replace("\"", "\\\""); + + /// Add the given set of functions + pub fn with_functions(self, fns: Vec) -> Runtime { + Runtime { + functions: fns, + ..self + } + } + + /// Add the given set of test statements + pub fn with_test_statements(self, statements: Vec) -> Runtime { + Runtime { + test_statements: statements, + ..self + } + } + + // Add the given set of data + pub fn with_data(self, data: Vec) -> Runtime { + Runtime { data: data, ..self } + } + + /// Generate the top level YUL object + pub fn to_yul(&self) -> yul::Object { + let all_statements = [self.functions.clone(), self.test_statements.clone()].concat(); + yul::Object { + name: identifier! { Contract }, + code: code! { [all_statements...] }, + objects: vec![], + data: self.data.clone(), + } + } +} + +fn execute_runtime_functions(executor: &mut Executor, runtime: Runtime) -> (ExitReason, Vec) { + let yul_code = runtime.to_yul().to_string().replace("\"", "\\\""); let bytecode = compiler::evm::compile_single_contract("Contract", yul_code, false) .expect("failed to compile Yul"); let bytecode = hex::decode(&bytecode).expect("failed to decode bytecode"); From cf736bfefcddd3bdee4ae02a4f0fc2793b5e748a Mon Sep 17 00:00:00 2001 From: Christoph Burgdorf Date: Mon, 19 Apr 2021 16:59:58 +0200 Subject: [PATCH 3/3] Add support for revert reason strings in assert statement Closes #288 --- compiler/src/yul/mappers/functions.rs | 11 +++++-- compiler/tests/features.rs | 34 ++++++++++++++++++---- compiler/tests/fixtures/features/assert.fe | 8 ++++- newsfragments/288.feature.md | 17 +++++++++++ 4 files changed, 61 insertions(+), 9 deletions(-) create mode 100644 newsfragments/288.feature.md diff --git a/compiler/src/yul/mappers/functions.rs b/compiler/src/yul/mappers/functions.rs index f34ea63b80..b463e510b8 100644 --- a/compiler/src/yul/mappers/functions.rs +++ b/compiler/src/yul/mappers/functions.rs @@ -188,10 +188,15 @@ fn emit(context: &Context, stmt: &Node) -> yul::Statement { } fn assert(context: &Context, stmt: &Node) -> yul::Statement { - if let fe::FuncStmt::Assert { test, msg: _ } = &stmt.kind { + if let fe::FuncStmt::Assert { test, msg } = &stmt.kind { let test = expressions::expr(context, test); - - return statement! { if (iszero([test])) { (revert(0, 0)) } }; + return match msg { + Some(val) => { + let msg = expressions::expr(context, val); + statement! { if (iszero([test])) { (revert_with_reason_string([msg])) } } + } + None => statement! { if (iszero([test])) { (revert(0, 0)) } }, + }; } unreachable!() diff --git a/compiler/tests/features.rs b/compiler/tests/features.rs index 7bc129eb7f..b1531fd905 100644 --- a/compiler/tests/features.rs +++ b/compiler/tests/features.rs @@ -75,17 +75,41 @@ fn test_assert() { let exit1 = harness.capture_call(&mut executor, "bar", &[uint_token(4)]); - assert!(matches!( - exit1, - evm::Capture::Exit((evm::ExitReason::Revert(_), _)) - )); + match exit1 { + evm::Capture::Exit((evm::ExitReason::Revert(_), output)) => assert_eq!(output.len(), 0), + _ => panic!("Did not revert correctly"), + } let exit2 = harness.capture_call(&mut executor, "bar", &[uint_token(42)]); assert!(matches!( exit2, evm::Capture::Exit((evm::ExitReason::Succeed(_), _)) - )) + )); + + let exit3 = + harness.capture_call(&mut executor, "revert_with_static_string", &[uint_token(4)]); + + match exit3 { + evm::Capture::Exit((evm::ExitReason::Revert(_), output)) => { + assert_eq!(output, encode_error_reason("Must be greater than five")) + } + _ => panic!("Did not revert correctly"), + } + + let reason = "A very looooooooooooooong reason that consumes multiple words"; + let exit4 = harness.capture_call( + &mut executor, + "revert_with", + &[uint_token(4), string_token(&reason)], + ); + + match exit4 { + evm::Capture::Exit((evm::ExitReason::Revert(_), output)) => { + assert_eq!(output, encode_error_reason(reason)) + } + _ => panic!("Did not revert correctly"), + } }) } diff --git a/compiler/tests/fixtures/features/assert.fe b/compiler/tests/fixtures/features/assert.fe index fc0c768778..92a4bca3fa 100644 --- a/compiler/tests/fixtures/features/assert.fe +++ b/compiler/tests/fixtures/features/assert.fe @@ -1,3 +1,9 @@ contract Foo: pub def bar(baz: u256): - assert baz > 5 \ No newline at end of file + assert baz > 5 + + pub def revert_with_static_string(baz: u256): + assert baz > 5, "Must be greater than five" + + pub def revert_with(baz: u256, reason: string1000): + assert baz > 5, reason \ No newline at end of file diff --git a/newsfragments/288.feature.md b/newsfragments/288.feature.md new file mode 100644 index 0000000000..379208b6d6 --- /dev/null +++ b/newsfragments/288.feature.md @@ -0,0 +1,17 @@ +Support for revert messages in assert statements + +E.g + +``` +assert a == b, "my revert statement" +``` + +The provided string is abi-encoded as if it were a call +to a function `Error(string)`. For example, the revert string `"Not enough Ether provided."` returns the following hexadecimal as error return data: + +``` +0x08c379a0 // Function selector for Error(string) +0x0000000000000000000000000000000000000000000000000000000000000020 // Data offset +0x000000000000000000000000000000000000000000000000000000000000001a // String length +0x4e6f7420656e6f7567682045746865722070726f76696465642e000000000000 // String data +``` \ No newline at end of file