From bf654f22654bcd99a0736b8161f2d2f3ae9c4a82 Mon Sep 17 00:00:00 2001 From: Christoph Burgdorf Date: Mon, 5 Jul 2021 17:27:07 +0200 Subject: [PATCH] Propagate certain reverts as panics Fixes #339 --- crates/test-files/fixtures/features/revert.fe | 2 +- crates/test-utils/src/lib.rs | 18 +++-- crates/tests/src/demo_erc20.rs | 1 + crates/tests/src/features.rs | 21 +++++- crates/yulgen/src/constants.rs | 7 ++ crates/yulgen/src/mappers/functions.rs | 5 +- crates/yulgen/src/runtime/functions/math.rs | 66 +++++++++++-------- crates/yulgen/src/runtime/functions/mod.rs | 9 ++- crates/yulgen/src/runtime/functions/revert.rs | 23 ++++++- fuzz/src/erc20.rs | 1 + newsfragments/476.feature.md | 10 +++ 11 files changed, 125 insertions(+), 38 deletions(-) create mode 100644 newsfragments/476.feature.md diff --git a/crates/test-files/fixtures/features/revert.fe b/crates/test-files/fixtures/features/revert.fe index f2b30e2467..1b3aecbc50 100644 --- a/crates/test-files/fixtures/features/revert.fe +++ b/crates/test-files/fixtures/features/revert.fe @@ -14,4 +14,4 @@ contract Foo: revert Error(msg=1, val=true) pub def revert_other_error(): - revert OtherError(msg=1, val=true) \ No newline at end of file + revert OtherError(msg=1, val=true) diff --git a/crates/test-utils/src/lib.rs b/crates/test-utils/src/lib.rs index ece68c4423..e0af52bcd6 100644 --- a/crates/test-utils/src/lib.rs +++ b/crates/test-utils/src/lib.rs @@ -109,11 +109,9 @@ impl ContractHarness { executor: &mut Executor, name: &str, input: &[ethabi::Token], + revert_data: &[u8], ) { - match self.capture_call(executor, name, input) { - evm::Capture::Exit((ExitReason::Revert(_), _)) => {} - _ => panic!("function did not revert"), - } + validate_revert(self.capture_call(executor, name, input), revert_data) } // Executor must be passed by value to get emitted events. @@ -204,6 +202,18 @@ pub fn validate_revert( }; } +pub fn encoded_panic_assert() -> Vec { + encode_revert("Panic(uint256)", &[uint_token(0x01)]) +} + +pub fn encoded_over_or_underflow() -> Vec { + encode_revert("Panic(uint256)", &[uint_token(0x11)]) +} + +pub fn encoded_div_or_mod_by_zero() -> Vec { + encode_revert("Panic(uint256)", &[uint_token(0x12)]) +} + #[allow(dead_code)] #[cfg(feature = "solc-backend")] pub fn deploy_contract( diff --git a/crates/tests/src/demo_erc20.rs b/crates/tests/src/demo_erc20.rs index 75b3527d8f..becba39a03 100644 --- a/crates/tests/src/demo_erc20.rs +++ b/crates/tests/src/demo_erc20.rs @@ -104,6 +104,7 @@ fn erc20_token() { address_token(bob), uint_token_from_dec_str("5000000000000000"), ], + &encoded_panic_assert(), ); harness.test_function( &mut executor, diff --git a/crates/tests/src/features.rs b/crates/tests/src/features.rs index 2214a74d01..b803a300a6 100644 --- a/crates/tests/src/features.rs +++ b/crates/tests/src/features.rs @@ -79,7 +79,7 @@ fn test_assert() { validate_revert( harness.capture_call(&mut executor, "bar", &[uint_token(4)]), - &[], + &encoded_panic_assert(), ); assert!(matches!( @@ -682,10 +682,12 @@ fn checked_arithmetic() { // ADDITION // unsigned: max_value + 1 fails + harness.test_function_reverts( &mut executor, &format!("add_u{}", config.size), &[config.u_max.clone(), uint_token(1)], + &encoded_over_or_underflow(), ); // unsigned: max_value + 0 works @@ -701,6 +703,7 @@ fn checked_arithmetic() { &mut executor, &format!("add_i{}", config.size), &[config.i_max.clone(), int_token(1)], + &encoded_over_or_underflow(), ); // signed: max_value + 0 works @@ -716,6 +719,7 @@ fn checked_arithmetic() { &mut executor, &format!("add_i{}", config.size), &[config.i_min.clone(), int_token(-1)], + &encoded_over_or_underflow(), ); // signed: min_value + 0 works @@ -732,6 +736,7 @@ fn checked_arithmetic() { &mut executor, &format!("sub_u{}", config.size), &[config.u_min.clone(), uint_token(1)], + &encoded_over_or_underflow(), ); // unsigned: min_value - 0 works @@ -747,6 +752,7 @@ fn checked_arithmetic() { &mut executor, &format!("sub_i{}", config.size), &[config.i_min.clone(), int_token(1)], + &encoded_over_or_underflow(), ); // signed: min_value - 0 works @@ -762,6 +768,7 @@ fn checked_arithmetic() { &mut executor, &format!("sub_i{}", config.size), &[config.i_max.clone(), int_token(-1)], + &encoded_over_or_underflow(), ); // signed: max_value - -0 works @@ -778,6 +785,7 @@ fn checked_arithmetic() { &mut executor, &format!("div_u{}", config.size), &[config.u_max.clone(), uint_token(0)], + &encoded_div_or_mod_by_zero(), ); // unsigned: 3 / 2 works @@ -793,6 +801,7 @@ fn checked_arithmetic() { &mut executor, &format!("div_i{}", config.size), &[config.i_max.clone(), int_token(0)], + &encoded_div_or_mod_by_zero(), ); // signed: min_value / -1 fails @@ -800,6 +809,7 @@ fn checked_arithmetic() { &mut executor, &format!("div_i{}", config.size), &[config.i_min.clone(), int_token(-1)], + &encoded_over_or_underflow(), ); // signed: 3 / -2 works @@ -816,6 +826,7 @@ fn checked_arithmetic() { &mut executor, &format!("pow_u{}", config.size), &[config.u_max.clone(), uint_token(2)], + &encoded_over_or_underflow(), ); // unsigned: 2 ** (bit_len-1) works @@ -833,6 +844,7 @@ fn checked_arithmetic() { &mut executor, &format!("pow_i{}", config.size), &[config.i_max.clone(), uint_token(2)], + &encoded_over_or_underflow(), ); // signed: min ** 3 fails (underflow) @@ -840,6 +852,7 @@ fn checked_arithmetic() { &mut executor, &format!("pow_i{}", config.size), &[config.i_min.clone(), uint_token(3)], + &encoded_over_or_underflow(), ); // signed: 2 ** (bit_len-2) works @@ -868,6 +881,7 @@ fn checked_arithmetic() { &mut executor, &format!("mod_u{}", config.size), &[config.u_max.clone(), uint_token(0)], + &encoded_div_or_mod_by_zero(), ); // unsigned: max_value % 2 works @@ -883,6 +897,7 @@ fn checked_arithmetic() { &mut executor, &format!("mod_i{}", config.size), &[config.i_max.clone(), int_token(0)], + &encoded_div_or_mod_by_zero(), ); // unsigned: max_value % 2 works @@ -915,6 +930,7 @@ fn checked_arithmetic() { &mut executor, &format!("mul_u{}", config.size), &[config.u_max.clone(), uint_token(2)], + &encoded_over_or_underflow(), ); // unsigned: max_value * 1 works @@ -930,6 +946,7 @@ fn checked_arithmetic() { &mut executor, &format!("mul_i{}", config.size), &[config.i_max.clone(), int_token(2)], + &encoded_over_or_underflow(), ); // signed: max_value * 1 works @@ -945,6 +962,7 @@ fn checked_arithmetic() { &mut executor, &format!("mul_i{}", config.size), &[config.i_max.clone(), int_token(-2)], + &encoded_over_or_underflow(), ); // signed: min_value * -2 fails @@ -952,6 +970,7 @@ fn checked_arithmetic() { &mut executor, &format!("mul_i{}", config.size), &[config.i_min.clone(), int_token(-2)], + &encoded_over_or_underflow(), ); harness.test_function( diff --git a/crates/yulgen/src/constants.rs b/crates/yulgen/src/constants.rs index 7056c91e5a..e3f5a98b16 100644 --- a/crates/yulgen/src/constants.rs +++ b/crates/yulgen/src/constants.rs @@ -57,3 +57,10 @@ pub fn numeric_min_max() -> HashMap ) } } + +// Panic codes as defined by solidity +// https://docs.soliditylang.org/en/v0.8.6/control-structures.html?highlight=0x12#panic-via-assert-and-error-via-require + +pub const PANIC_FAILED_ASSERTION: usize = 0x01; +pub const PANIC_OVER_OR_UNDERFLOW: usize = 0x11; +pub const PANIC_DIV_OR_MOD_BY_ZERO: usize = 0x12; diff --git a/crates/yulgen/src/mappers/functions.rs b/crates/yulgen/src/mappers/functions.rs index ef4c8215b2..c8f3b1448d 100644 --- a/crates/yulgen/src/mappers/functions.rs +++ b/crates/yulgen/src/mappers/functions.rs @@ -1,3 +1,4 @@ +use crate::constants::PANIC_FAILED_ASSERTION; use crate::mappers::{assignments, declarations, expressions}; use crate::names; use crate::operations::abi as abi_operations; @@ -189,7 +190,9 @@ fn assert(context: &mut Context, stmt: &Node) -> yul::Statement { } unreachable!() } - None => statement! { if (iszero([test])) { (revert(0, 0)) } }, + None => { + statement! { if (iszero([test])) { (revert_with_panic([literal_expression! {(PANIC_FAILED_ASSERTION)}])) } } + } }; } diff --git a/crates/yulgen/src/runtime/functions/math.rs b/crates/yulgen/src/runtime/functions/math.rs index fc5eef913a..77e302d627 100644 --- a/crates/yulgen/src/runtime/functions/math.rs +++ b/crates/yulgen/src/runtime/functions/math.rs @@ -1,4 +1,4 @@ -use crate::constants::numeric_min_max; +use crate::constants::{numeric_min_max, PANIC_DIV_OR_MOD_BY_ZERO, PANIC_OVER_OR_UNDERFLOW}; use crate::names; use fe_analyzer::namespace::types::Integer; use yultsur::*; @@ -109,10 +109,22 @@ pub fn all() -> Vec { .concat() } +fn revert_with_over_or_under_flow() -> yul::Statement { + statement!(revert_with_panic([ + literal_expression! {(PANIC_OVER_OR_UNDERFLOW)} + ])) +} + +fn revert_with_div_or_mod_by_zero() -> yul::Statement { + statement!(revert_with_panic([ + literal_expression! {(PANIC_DIV_OR_MOD_BY_ZERO)} + ])) +} + fn checked_mod_unsigned() -> yul::Statement { function_definition! { function checked_mod_unsigned(val1, val2) -> result { - (if (iszero(val2)) { (revert(0, 0)) }) + (if (iszero(val2)) { [revert_with_div_or_mod_by_zero()] }) (result := mod(val1, val2)) } } @@ -121,7 +133,7 @@ fn checked_mod_unsigned() -> yul::Statement { fn checked_mod_signed() -> yul::Statement { function_definition! { function checked_mod_signed(val1, val2) -> result { - (if (iszero(val2)) { (revert(0, 0)) }) + (if (iszero(val2)) { [revert_with_div_or_mod_by_zero()] }) (result := smod(val1, val2)) } } @@ -137,7 +149,7 @@ fn checked_mul_unsigned(size: Integer) -> yul::Statement { function_definition! { function [fn_name](val1, val2) -> product { // overflow, if val1 != 0 and val2 > (max_value / val1) - (if (and((iszero((iszero(val1)))), (gt(val2, (div([max_value], val1)))))) { (revert(0, 0)) }) + (if (and((iszero((iszero(val1)))), (gt(val2, (div([max_value], val1)))))) { [revert_with_over_or_under_flow()] }) (product := mul(val1, val2)) } } @@ -153,13 +165,13 @@ fn checked_mul_signed(size: Integer) -> yul::Statement { function_definition! { function [fn_name](val1, val2) -> product { // overflow, if val1 > 0, val2 > 0 and val1 > (max_value / val2) - (if (and((and((sgt(val1, 0)), (sgt(val2, 0)))), (gt(val1, (div([max_value.clone()], val2)))))) { (revert(0, 0)) }) + (if (and((and((sgt(val1, 0)), (sgt(val2, 0)))), (gt(val1, (div([max_value.clone()], val2)))))) { [revert_with_over_or_under_flow()] }) // underflow, if val1 > 0, val2 < 0 and val2 < (min_value / val1) - (if (and((and((sgt(val1, 0)), (slt(val2, 0)))), (slt(val2, (sdiv([min_value.clone()], val1)))))) { (revert(0, 0)) }) + (if (and((and((sgt(val1, 0)), (slt(val2, 0)))), (slt(val2, (sdiv([min_value.clone()], val1)))))) { [revert_with_over_or_under_flow()] }) // underflow, if val1 < 0, val2 > 0 and val1 < (min_value / val2) - (if (and((and((slt(val1, 0)), (sgt(val2, 0)))), (slt(val1, (sdiv([min_value], val2)))))) { (revert(0, 0)) }) + (if (and((and((slt(val1, 0)), (sgt(val2, 0)))), (slt(val1, (sdiv([min_value], val2)))))) { [revert_with_over_or_under_flow()] }) // overflow, if val1 < 0, val2 < 0 and val1 < (max_value / val2) - (if (and((and((slt(val1, 0)), (slt(val2, 0)))), (slt(val1, (sdiv([max_value], val2)))))) { (revert(0, 0)) }) + (if (and((and((slt(val1, 0)), (slt(val2, 0)))), (slt(val1, (sdiv([max_value], val2)))))) { [revert_with_over_or_under_flow()] }) (product := mul(val1, val2)) } } @@ -175,7 +187,7 @@ fn checked_add_unsigned(size: Integer) -> yul::Statement { function_definition! { function [fn_name](val1, val2) -> sum { // overflow, if val1 > (max_value - val2) - (if (gt(val1, (sub([max_value], val2)))) { (revert(0, 0)) }) + (if (gt(val1, (sub([max_value], val2)))) { [revert_with_over_or_under_flow()] }) (sum := add(val1, val2)) } } @@ -190,9 +202,9 @@ fn checked_add_signed(size: Integer) -> yul::Statement { function_definition! { function [fn_name](val1, val2) -> sum { // overflow, if val1 >= 0 and val2 > (max_value - val1) - (if (and((iszero((slt(val1, 0)))), (sgt(val2, (sub([max_value], val1)))))) { (revert(0, 0)) }) + (if (and((iszero((slt(val1, 0)))), (sgt(val2, (sub([max_value], val1)))))) { [revert_with_over_or_under_flow()] }) // underflow, if val1 < 0 and val2 < (min_val - val1) - (if (and((slt(val1, 0)), (slt(val2, (sub([min_value], val1)))))) { (revert(0, 0)) }) + (if (and((slt(val1, 0)), (slt(val2, (sub([min_value], val1)))))) { [revert_with_over_or_under_flow()] }) (sum := add(val1, val2)) } } @@ -201,7 +213,7 @@ fn checked_add_signed(size: Integer) -> yul::Statement { fn checked_div_unsigned() -> yul::Statement { function_definition! { function checked_div_unsigned(val1, val2) -> result { - (if (iszero(val2)) { (revert(0, 0)) }) + (if (iszero(val2)) { [revert_with_div_or_mod_by_zero()] }) (result := div(val1, val2)) } } @@ -215,10 +227,10 @@ fn checked_div_signed(size: Integer) -> yul::Statement { let fn_name = names::checked_div(&size); function_definition! { function [fn_name](val1, val2) -> result { - (if (iszero(val2)) { (revert(0, 0)) }) + (if (iszero(val2)) { [revert_with_div_or_mod_by_zero()] }) // overflow for min_val / -1 - (if (and( (eq(val1, [min_value])), (eq(val2, (sub(0, 1))))) ) { (revert(0, 0)) }) + (if (and( (eq(val1, [min_value])), (eq(val2, (sub(0, 1))))) ) { [revert_with_over_or_under_flow()] }) (result := sdiv(val1, val2)) } } @@ -228,7 +240,7 @@ fn checked_sub_unsigned() -> yul::Statement { function_definition! { function checked_sub_unsigned(val1, val2) -> diff { // underflow, if val2 > val1 - (if (lt(val1, val2)) { (revert(0, 0)) }) + (if (lt(val1, val2)) { [revert_with_over_or_under_flow()] }) (diff := sub(val1, val2)) } } @@ -244,9 +256,9 @@ fn checked_sub_signed(size: Integer) -> yul::Statement { function_definition! { function [fn_name](val1, val2) -> diff { // underflow, if val2 >= 0 and val1 < (min_value + val2) - (if (and((iszero((slt(val2, 0)))), (slt(val1, (add([min_value], val2)))))) { (revert(0, 0)) }) + (if (and((iszero((slt(val2, 0)))), (slt(val1, (add([min_value], val2)))))) { [revert_with_over_or_under_flow()] }) // overflow, if val2 < 0 and val1 > (max_value + val2) - (if (and((slt(val2, 0)), (sgt(val1, (add([max_value], val2)))))) { (revert(0, 0)) }) + (if (and((slt(val2, 0)), (sgt(val1, (add([max_value], val2)))))) { [revert_with_over_or_under_flow()] }) (diff := sub(val1, val2)) } } @@ -290,7 +302,7 @@ fn checked_exp_helper() -> yul::Statement { (for {} (gt(exponent, 1)) {} { // overflow check for base * base - (if (gt(base, (div(max, base)))) { (revert(0, 0)) }) + (if (gt(base, (div(max, base)))) { [revert_with_over_or_under_flow()] }) (if (and(exponent, 1)) { // No checks for power := mul(power, base) needed, because the check // for base * base above is sufficient, since: @@ -343,12 +355,12 @@ fn checked_exp_signed() -> yul::Statement { switch (sgt(base, 0)) (case 1 { (if (gt(base, (div(max, base)))) { - (revert(0, 0)) + [revert_with_over_or_under_flow()] }) }) (case 0 { (if (slt(base, (sdiv(max, base)))) { - (revert(0, 0)) + [revert_with_over_or_under_flow()] }) }) }]) @@ -359,8 +371,8 @@ fn checked_exp_signed() -> yul::Statement { (exponent := shr(1, exponent)) // // Below this point, base is always positive. ([checked_exp_helper_call]) // power = 1, base = 16 which is wrong - (if (and((sgt(power, 0)), (gt(power, (div(max, base)))))) { (revert(0, 0)) }) - (if (and((slt(power, 0)), (slt(power, (sdiv(min, base)))))) { (revert(0, 0)) }) + (if (and((sgt(power, 0)), (gt(power, (div(max, base)))))) { [revert_with_over_or_under_flow()] }) + (if (and((slt(power, 0)), (slt(power, (sdiv(min, base)))))) { [revert_with_over_or_under_flow()] }) (power := (mul(power, base))) } } @@ -397,28 +409,28 @@ fn checked_exp_unsigned() -> yul::Statement { }) (case 2 { (if (gt(exponent, 255)) { - (revert(0, 0)) + [revert_with_over_or_under_flow()] }) (power := (exp(2, exponent))) (if (gt(power, max)) { - (revert(0, 0)) + [revert_with_over_or_under_flow()] }) (leave) }) }]) - (if (and((sgt(power, 0)), (gt(power, (div(max, base)))))) { (revert(0, 0)) }) + (if (and((sgt(power, 0)), (gt(power, (div(max, base)))))) { [revert_with_over_or_under_flow()] }) (if (or((and((lt(base, 11)), (lt(exponent, 78)))), (and((lt(base, 307)), (lt(exponent, 32)))))) { (power := (exp(base, exponent))) (if (gt(power, max)) { - (revert(0, 0)) + [revert_with_over_or_under_flow()] }) (leave) }) ([checked_exp_helper_call]) (if (gt(power, (div(max, base)))) { - (revert(0, 0)) + [revert_with_over_or_under_flow()] }) (power := (mul(power, base))) } diff --git a/crates/yulgen/src/runtime/functions/mod.rs b/crates/yulgen/src/runtime/functions/mod.rs index 60b104411f..3d27785c34 100644 --- a/crates/yulgen/src/runtime/functions/mod.rs +++ b/crates/yulgen/src/runtime/functions/mod.rs @@ -9,5 +9,12 @@ pub mod structs; /// Returns all functions that should be available during runtime. pub fn std() -> Vec { - [contracts::all(), abi::all(), data::all(), math::all()].concat() + [ + contracts::all(), + abi::all(), + data::all(), + math::all(), + revert::all(), + ] + .concat() } diff --git a/crates/yulgen/src/runtime/functions/revert.rs b/crates/yulgen/src/runtime/functions/revert.rs index fe27aa96f6..297e2bb227 100644 --- a/crates/yulgen/src/runtime/functions/revert.rs +++ b/crates/yulgen/src/runtime/functions/revert.rs @@ -1,8 +1,7 @@ use crate::names; use crate::names::abi as abi_names; use fe_abi::utils as abi_utils; -use fe_analyzer::namespace::types::Struct; -use fe_analyzer::namespace::types::{AbiEncoding, FixedSize}; +use fe_analyzer::namespace::types::{AbiEncoding, FixedSize, Struct, U256}; use yultsur::*; fn selector(name: &str, params: &[FixedSize]) -> yul::Expression { @@ -28,7 +27,20 @@ pub fn generate_struct_revert(val: &Struct) -> yul::Statement { generate_revert_fn(&val.name, &[FixedSize::Struct(val.clone())], &struct_fields) } -/// Generate a YUL function that can be used to revert with data +/// Generate a YUL function to revert with panic codes +pub fn generate_revert_fn_for_panic() -> yul::Statement { + let selector = selector("Panic", &[FixedSize::Base(U256)]); + + return function_definition! { + function revert_with_panic(error_code) { + (let ptr := alloc_mstoren([selector], 4)) + (pop((alloc_mstoren(error_code, 32)))) + (revert(ptr, (add(4, 32)))) + } + }; +} + +/// Generate a YUL function to revert with data pub fn generate_revert_fn( name: &str, encoding_params: &[FixedSize], @@ -48,3 +60,8 @@ pub fn generate_revert_fn( } }; } + +/// Return all revert runtime functions +pub fn all() -> Vec { + vec![generate_revert_fn_for_panic()] +} diff --git a/fuzz/src/erc20.rs b/fuzz/src/erc20.rs index a7d9ec32d8..45b83edd38 100644 --- a/fuzz/src/erc20.rs +++ b/fuzz/src/erc20.rs @@ -174,6 +174,7 @@ pub fn erc20_transfer(info: TransferInfo, mut executor: Executor, mut harness: C &mut executor, "transferFrom", &[address_token(info.to), address_token(BOB), value.clone()], + &[], ); harness.test_function( &mut executor, diff --git a/newsfragments/476.feature.md b/newsfragments/476.feature.md new file mode 100644 index 0000000000..0c37912644 --- /dev/null +++ b/newsfragments/476.feature.md @@ -0,0 +1,10 @@ +Encode certain reverts as panics. + +With this change, the following reverts are encoded as `Panic(uint256)` with +the following panic codes: + +`0x01`: An assertion that failed and did not specify an error message +`0x11`: An arithmetic expression resulted in an over- or underflow +`0x12`: An arithmetic expression divided or modulo by zero + +The panic codes are aligned with [the panic codes that Solidity uses](https://docs.soliditylang.org/en/v0.8.4/control-structures.html?highlight=Panic#panic-via-assert-and-error-via-require).