diff --git a/Cargo.lock b/Cargo.lock index 22b7084a6b..b08d323fa0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -575,6 +575,7 @@ dependencies = [ "insta", "pretty_assertions", "primitive-types", + "proptest", "rand 0.7.3", "rstest", "wasm-bindgen-test", @@ -1193,6 +1194,29 @@ dependencies = [ "unicode-xid", ] +[[package]] +name = "proptest" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1e0d9cc07f18492d879586c92b485def06bc850da3118075cd45d50e9c95b0e5" +dependencies = [ + "bitflags", + "byteorder", + "lazy_static", + "num-traits", + "quick-error", + "rand 0.8.4", + "rand_chacha 0.3.1", + "rand_xorshift", + "regex-syntax", +] + +[[package]] +name = "quick-error" +version = "2.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a993555f31e5a609f617c12db6250dedcac1b0a85076912c436e6fc9b2c8e6a3" + [[package]] name = "quote" version = "1.0.9" @@ -1279,6 +1303,15 @@ dependencies = [ "rand_core 0.5.1", ] +[[package]] +name = "rand_xorshift" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d25bf25ec5ae4a3f1b92f929810509a2f53d7dca2f50b794ff57e3face536c8f" +dependencies = [ + "rand_core 0.6.3", +] + [[package]] name = "redox_syscall" version = "0.1.57" diff --git a/Makefile b/Makefile index 7bf25c31d1..aacfe1a0d0 100644 --- a/Makefile +++ b/Makefile @@ -73,7 +73,7 @@ docker-wasm-test: .PHONY: coverage coverage: - cargo tarpaulin --workspace --all-features --verbose --timeout 120 --exclude-files 'tests/*' --exclude-files 'main.rs' --out xml html + cargo tarpaulin --workspace --all-features --verbose --timeout 120 --exclude-files 'tests/*' --exclude-files 'main.rs' --out xml html -- --skip differential:: .PHONY: clippy clippy: diff --git a/crates/test-files/fixtures/differential/math_i8.fe b/crates/test-files/fixtures/differential/math_i8.fe new file mode 100644 index 0000000000..11cc7f5216 --- /dev/null +++ b/crates/test-files/fixtures/differential/math_i8.fe @@ -0,0 +1,43 @@ +contract Foo: + + pub fn add(val1: i8, val2: i8) -> i8: + return val1 + val2 + + pub fn subtract(val1: i8, val2: i8) -> i8: + return val1 - val2 + + pub fn multiply(val1: i8, val2: i8) -> i8: + return val1 * val2 + + pub fn divide(val1: i8, val2: i8) -> i8: + return val1 / val2 + + pub fn pow(val1: i8, val2: u8) -> i8: + return val1 ** val2 + + pub fn modulo(val1: i8, val2: i8) -> i8: + return val1 % val2 + + pub fn rightshift(val1: i8, val2: u8) -> i8: + return val1 >> val2 + + pub fn leftshift(val1: i8, val2: u8) -> i8: + return val1 << val2 + + pub fn invert(val1: i8) -> i8: + return ~val1 + + pub fn cast1(val1: i8) -> u8: + return u8(val1) + + pub fn cast2(val1: i8) -> u16: + return u16(u8(val1)) + + pub fn cast3(val1: i8) -> u16: + return u16(i16(val1)) + + pub fn order_of_operation(val1: i8, val2: i8, val3: u8) -> i8: + return val1 - val2 * (val1 + val2 / 4 * val1 - val2**val3) + val1 + + pub fn negate(val1: i8) -> i8: + return -val1 \ No newline at end of file diff --git a/crates/test-files/fixtures/differential/math_i8.sol b/crates/test-files/fixtures/differential/math_i8.sol new file mode 100644 index 0000000000..33184a7667 --- /dev/null +++ b/crates/test-files/fixtures/differential/math_i8.sol @@ -0,0 +1,58 @@ +contract Foo { + + function add(int8 val1, int8 val2) public pure returns (int8){ + return val1 + val2; + } + + function subtract(int8 val1, int8 val2) public pure returns (int8){ + return val1 - val2; + } + + function multiply(int8 val1, int8 val2) public pure returns (int8){ + return val1 * val2; + } + + function divide(int8 val1, int8 val2) public pure returns (int8){ + return val1 / val2; + } + + function pow(int8 val1, uint8 val2) public pure returns (int8) { + return int8(uint8(val1) ** val2); + } + + function modulo(int8 val1, int8 val2) public pure returns (int8){ + return val1 % val2; + } + + function leftshift(int8 val1, uint8 val2) public pure returns (int8){ + return val1 << val2; + } + + function rightshift(int8 val1, uint8 val2) public pure returns (int8){ + return val1 >> val2; + } + + function invert(int8 val1) public pure returns (int8){ + return ~val1; + } + + function cast1(int8 val1) public pure returns (uint8){ + return uint8(val1); + } + + function cast2(int8 val1) public pure returns (uint16){ + return uint16(uint8(val1)); + } + + function cast3(int8 val1) public pure returns (uint16){ + return uint16(int16(val1)); + } + + function order_of_operation(int8 val1, int8 val2, uint8 val3) public pure returns (int8) { + return val1 - val2 * (val1 + val2 / 4 * val1 - val2**val3) + val1; + } + + function negate(int8 val1) public pure returns (int8){ + return -val1; + } +} \ No newline at end of file diff --git a/crates/test-files/fixtures/differential/math_u8.fe b/crates/test-files/fixtures/differential/math_u8.fe new file mode 100644 index 0000000000..7fad04d6e1 --- /dev/null +++ b/crates/test-files/fixtures/differential/math_u8.fe @@ -0,0 +1,49 @@ +contract Foo: + + pub fn add(val1: u8, val2: u8) -> u8: + return val1 + val2 + + pub fn subtract(val1: u8, val2: u8) -> u8: + return val1 - val2 + + pub fn multiply(val1: u8, val2: u8) -> u8: + return val1 * val2 + + pub fn divide(val1: u8, val2: u8) -> u8: + return val1 / val2 + + pub fn pow(val1: u8, val2: u8) -> u8: + return val1 ** val2 + + pub fn modulo(val1: u8, val2: u8) -> u8: + return val1 % val2 + + pub fn rightshift(val1: u8, val2: u8) -> u8: + return val1 >> val2 + + pub fn leftshift(val1: u8, val2: u8) -> u8: + return val1 << val2 + + pub fn invert(val1: u8) -> u8: + return ~val1 + + pub fn bit_and(val1: u8, val2: u8) -> u8: + return val1 & val2 + + pub fn bit_or(val1: u8, val2: u8) -> u8: + return val1 | val2 + + pub fn bit_xor(val1: u8, val2: u8) -> u8: + return val1 ^ val2 + + pub fn cast1(val1: u8) -> i8: + return i8(val1) + + pub fn cast2(val1: u8) -> i16: + return i16(i8(val1)) + + pub fn cast3(val1: u8) -> i16: + return i16(u16(val1)) + + pub fn order_of_operation(val1: u8, val2: u8) -> u8: + return val1 - val2 * (val1 + val2 / 4 * val1 - val2**val1) + val1 \ No newline at end of file diff --git a/crates/test-files/fixtures/differential/math_u8.sol b/crates/test-files/fixtures/differential/math_u8.sol new file mode 100644 index 0000000000..d4b55c4235 --- /dev/null +++ b/crates/test-files/fixtures/differential/math_u8.sol @@ -0,0 +1,66 @@ +contract Foo { + + function add(uint8 val1, uint8 val2) public pure returns (uint8){ + return val1 + val2; + } + + function subtract(uint8 val1, uint8 val2) public pure returns (uint8){ + return val1 - val2; + } + + function multiply(uint8 val1, uint8 val2) public pure returns (uint8){ + return val1 * val2; + } + + function divide(uint8 val1, uint8 val2) public pure returns (uint8){ + return val1 / val2; + } + + function pow(uint8 val1, uint8 val2) public pure returns (uint8){ + return val1 ** val2; + } + + function modulo(uint8 val1, uint8 val2) public pure returns (uint8){ + return val1 % val2; + } + + function leftshift(uint8 val1, uint8 val2) public pure returns (uint8){ + return val1 << val2; + } + + function rightshift(uint8 val1, uint8 val2) public pure returns (uint8){ + return val1 >> val2; + } + + function invert(uint8 val1) public pure returns (uint8){ + return ~val1; + } + + function bit_and(uint8 val1, uint8 val2) public pure returns (uint8){ + return val1 & val2; + } + + function bit_or(uint8 val1, uint8 val2) public pure returns (uint8){ + return val1 | val2; + } + + function bit_xor(uint8 val1, uint8 val2) public pure returns (uint8){ + return val1 ^ val2; + } + + function cast1(uint8 val1) public pure returns (int8){ + return int8(val1); + } + + function cast2(uint8 val1) public pure returns (int16){ + return int16(int8(val1)); + } + + function cast3(uint8 val1) public pure returns (int16){ + return int16(uint16(val1)); + } + + function order_of_operation(uint8 val1, uint8 val2) public pure returns (uint8){ + return val1 - val2 * (val1 + val2 / 4 * val1 - val2**val1) + val1; + } +} \ No newline at end of file diff --git a/crates/tests/Cargo.toml b/crates/tests/Cargo.toml index ee3223fb7f..eba19de29e 100644 --- a/crates/tests/Cargo.toml +++ b/crates/tests/Cargo.toml @@ -33,3 +33,11 @@ wasm-bindgen-test = "0.3.24" [features] solc-backend = ["fe-yulc/solc-backend"] + +[dev-dependencies.proptest] +version = "1.0.0" +# The default feature set includes things like process forking which are not +# supported in Web Assembly. +default-features = false +# Enable using the `std` crate. +features = ["std"] \ No newline at end of file diff --git a/crates/tests/proptest-regressions/differential.txt b/crates/tests/proptest-regressions/differential.txt new file mode 100644 index 0000000000..4249ec8002 --- /dev/null +++ b/crates/tests/proptest-regressions/differential.txt @@ -0,0 +1,8 @@ +# Seeds for failure cases proptest has generated in the past. It is +# automatically read and these particular cases re-run before any +# novel cases are generated. +# +# It is recommended to check this file in to source control so that +# everyone who runs the test benefits from these saved cases. +cc 71897591107cbd3f0d248e1ecbece406820f797206e4f43ea02d53a1ebc5ad92 # shrinks to val = 0, val2 = 0, val3 = 128, val4 = 0 +cc 6f70182204124ed76e435a25baf2365d922abac6cb4647dcb86cdd4b339fab31 # shrinks to val = -128, val2 = 0, val3 = 0, val4 = 0 diff --git a/crates/tests/src/differential.rs b/crates/tests/src/differential.rs new file mode 100644 index 0000000000..3d56f79c9c --- /dev/null +++ b/crates/tests/src/differential.rs @@ -0,0 +1,148 @@ +//! Tests that check for differences between Solidity and Fe implementations of similar contracts +#![cfg(feature = "solc-backend")] +use proptest::prelude::*; + +use fe_compiler_test_utils::*; +use fe_compiler_test_utils::{self as test_utils}; + +struct DualHarness { + fe_harness: ContractHarness, + solidity_harness: ContractHarness, +} + +struct CaptureResult<'a> { + fe_capture: evm::Capture<(evm::ExitReason, Vec), std::convert::Infallible>, + solidity_capture: evm::Capture<(evm::ExitReason, Vec), std::convert::Infallible>, + name: &'a str, + input: &'a [ethabi::Token], +} + +impl<'a> CaptureResult<'a> { + pub fn assert_perfomed_equal(&self) { + assert_eq!( + self.fe_capture, self.solidity_capture, + "Called {} with input: {:?}", + self.name, self.input + ) + } + + pub fn assert_reverted(&self) { + if !matches!( + (self.fe_capture.clone(), self.solidity_capture.clone()), + ( + evm::Capture::Exit((evm::ExitReason::Revert(_), _)), + evm::Capture::Exit((evm::ExitReason::Revert(_), _)) + ) + ) { + panic!( + "Asserted both revert but was: Fe: {:?} Solidity: {:?}", + self.fe_capture, self.solidity_capture + ) + } + } + + pub fn performed_equal(&self) -> bool { + self.fe_capture == self.solidity_capture + } +} + +impl<'a> DualHarness { + pub fn from_fixture( + executor: &mut Executor, + fixture: &str, + contract_name: &str, + init_params: &[ethabi::Token], + ) -> DualHarness { + let fe_harness = test_utils::deploy_contract( + executor, + &format!("differential/{}.fe", fixture), + contract_name, + init_params, + ); + let solidity_harness = test_utils::deploy_solidity_contract( + executor, + &format!("differential/{}.sol", fixture), + contract_name, + init_params, + ); + DualHarness { + fe_harness, + solidity_harness, + } + } + + pub fn capture_call( + &self, + executor: &mut Executor, + name: &'a str, + input: &'a [ethabi::Token], + ) -> CaptureResult<'a> { + let fe_capture = self.fe_harness.capture_call(executor, name, input); + let solidity_capture = self.solidity_harness.capture_call(executor, name, input); + + CaptureResult { + fe_capture, + solidity_capture, + name, + input, + } + } +} + +proptest! { + + #[test] + fn math_u8(val in 0u8..=255, val2 in 0u8..=255) { + with_executor(&|mut executor| { + + let harness = DualHarness::from_fixture(&mut executor, "math_u8", "Foo", &[]); + + harness.capture_call(&mut executor, "add", &[uint_token(val.into()), uint_token(val2.into())]).assert_perfomed_equal(); + harness.capture_call(&mut executor, "subtract", &[uint_token(val.into()), uint_token(val2.into())]).assert_perfomed_equal(); + harness.capture_call(&mut executor, "divide", &[uint_token(val.into()), uint_token(val2.into())]).assert_perfomed_equal(); + harness.capture_call(&mut executor, "multiply", &[uint_token(val.into()), uint_token(val2.into())]).assert_perfomed_equal(); + harness.capture_call(&mut executor, "pow", &[uint_token(val.into()), uint_token(val2.into())]).assert_perfomed_equal(); + harness.capture_call(&mut executor, "modulo", &[uint_token(val.into()), uint_token(val2.into())]).assert_perfomed_equal(); + harness.capture_call(&mut executor, "leftshift", &[uint_token(val.into()), uint_token(val2.into())]).assert_perfomed_equal(); + harness.capture_call(&mut executor, "rightshift", &[uint_token(val.into()), uint_token(val2.into())]).assert_perfomed_equal(); + harness.capture_call(&mut executor, "order_of_operation", &[uint_token(val.into()), uint_token(val2.into())]).assert_perfomed_equal(); + harness.capture_call(&mut executor, "invert", &[uint_token(val.into())]).assert_perfomed_equal(); + harness.capture_call(&mut executor, "bit_and", &[uint_token(val.into()), uint_token(val2.into())]).assert_perfomed_equal(); + harness.capture_call(&mut executor, "bit_or", &[uint_token(val.into()), uint_token(val2.into())]).assert_perfomed_equal(); + harness.capture_call(&mut executor, "bit_xor", &[uint_token(val.into()), uint_token(val2.into())]).assert_perfomed_equal(); + harness.capture_call(&mut executor, "cast1", &[uint_token(val.into())]).assert_perfomed_equal(); + harness.capture_call(&mut executor, "cast2", &[uint_token(val.into())]).assert_perfomed_equal(); + harness.capture_call(&mut executor, "cast3", &[uint_token(val.into())]).assert_perfomed_equal(); + }); + } + + #[test] + fn math_i8(val in -128i8..=127i8, val2 in -128i8..=127i8, val3 in 0u8..=255, val4 in 0u8..=255) { + with_executor(&|mut executor| { + let harness = DualHarness::from_fixture(&mut executor, "math_i8", "Foo", &[]); + + harness.capture_call(&mut executor, "add", &[int_token(val.into()), int_token(val2.into())]).assert_perfomed_equal(); + harness.capture_call(&mut executor, "subtract", &[int_token(val.into()), int_token(val2.into())]).assert_perfomed_equal(); + harness.capture_call(&mut executor, "divide", &[int_token(val.into()), int_token(val2.into())]).assert_perfomed_equal(); + harness.capture_call(&mut executor, "multiply", &[int_token(val.into()), int_token(val2.into())]).assert_perfomed_equal(); + let inputs = &[int_token(val3.into()), uint_token(val4.into())]; + let result = harness.capture_call(&mut executor, "pow", inputs); + if !result.performed_equal() { + // If they didn't performed equal then we further assert that at least both reverted. Solidity reverts + // with zero revert data when a pow operation overflows whereas Fe uses a proper revert error. + // We tolerate the divergence and assume its a TODO on the Solidity side :) + result.assert_reverted(); + } + + harness.capture_call(&mut executor, "modulo", &[int_token(val.into()), int_token(val2.into())]).assert_perfomed_equal(); + harness.capture_call(&mut executor, "leftshift", &[int_token(val.into()), uint_token(val3.into())]).assert_perfomed_equal(); + harness.capture_call(&mut executor, "rightshift", &[int_token(val.into()), uint_token(val3.into())]).assert_perfomed_equal(); + harness.capture_call(&mut executor, "order_of_operation", &[int_token(val.into()), int_token(val2.into()), uint_token(val3.into())]).assert_perfomed_equal(); + harness.capture_call(&mut executor, "invert", &[int_token(val.into())]).assert_perfomed_equal(); + harness.capture_call(&mut executor, "cast1", &[int_token(val.into())]).assert_perfomed_equal(); + harness.capture_call(&mut executor, "cast2", &[int_token(val.into())]).assert_perfomed_equal(); + harness.capture_call(&mut executor, "cast3", &[int_token(val.into())]).assert_perfomed_equal(); + harness.capture_call(&mut executor, "negate", &[int_token(val.into())]).assert_perfomed_equal(); + }); + } +} diff --git a/crates/tests/src/lib.rs b/crates/tests/src/lib.rs index 00efa25b58..d7c18ff9d0 100644 --- a/crates/tests/src/lib.rs +++ b/crates/tests/src/lib.rs @@ -7,6 +7,8 @@ mod demo_guestbook; #[cfg(test)] mod demo_uniswap; #[cfg(test)] +mod differential; +#[cfg(test)] mod features; #[cfg(test)] mod runtime; diff --git a/crates/yulgen/src/mappers/expressions.rs b/crates/yulgen/src/mappers/expressions.rs index 8b48d9c197..73d74db826 100644 --- a/crates/yulgen/src/mappers/expressions.rs +++ b/crates/yulgen/src/mappers/expressions.rs @@ -312,8 +312,19 @@ pub fn expr_unary_operation(context: &mut FnContext, exp: &Node) -> yu return match &op.kind { fe::UnaryOperator::USub => { - let zero = literal_expression! {0}; - expression! { sub([zero], [yul_operand]) } + match typ { + Type::Base(Base::Numeric(integer)) => { + if let fe::Expr::Num(_) = &operand.kind { + // Literals are checked at compile time (e.g. -128) so there's no point + // in adding a runtime check. + let zero = literal_expression! {0}; + expression! { sub([zero], [yul_operand]) } + } else { + expression! { [names::checked_neg(integer)]([yul_operand]) } + } + } + _ => unreachable!(), + } } fe::UnaryOperator::Not => expression! { iszero([yul_operand]) }, fe::UnaryOperator::Invert => match typ { diff --git a/crates/yulgen/src/names/mod.rs b/crates/yulgen/src/names/mod.rs index e273a033ac..ea79f6f6e3 100644 --- a/crates/yulgen/src/names/mod.rs +++ b/crates/yulgen/src/names/mod.rs @@ -5,6 +5,11 @@ use yultsur::*; pub mod abi; +/// Generate a function name to perform checked negation +pub fn checked_neg(size: &Integer) -> yul::Identifier { + identifier! {(format!("checked_neg_{}", size.as_ref().to_lowercase()))} +} + /// Generate a function name to perform checked addition pub fn checked_add(size: &Integer) -> yul::Identifier { identifier! {(format!("checked_add_{}", size.as_ref().to_lowercase()))} diff --git a/crates/yulgen/src/runtime/functions/math.rs b/crates/yulgen/src/runtime/functions/math.rs index cae13cdb41..d384165ae4 100644 --- a/crates/yulgen/src/runtime/functions/math.rs +++ b/crates/yulgen/src/runtime/functions/math.rs @@ -4,6 +4,19 @@ use crate::operations::revert as revert_operations; use fe_analyzer::namespace::types::Integer; use yultsur::*; +/// Return a vector of runtime functions for negations with over-/underflow +/// protection +pub fn checked_neg_fns() -> Vec { + vec![ + checked_neg_signed(Integer::I256), + checked_neg_signed(Integer::I128), + checked_neg_signed(Integer::I64), + checked_neg_signed(Integer::I32), + checked_neg_signed(Integer::I16), + checked_neg_signed(Integer::I8), + ] +} + /// Return a vector of runtime functions for additions with over-/underflow /// protection pub fn checked_add_fns() -> Vec { @@ -122,6 +135,7 @@ pub fn all() -> Vec { checked_mod_fns(), checked_mul_fns(), checked_sub_fns(), + checked_neg_fns(), adjust_numeric_size_fns(), ] .concat() @@ -135,6 +149,21 @@ fn revert_with_div_or_mod_by_zero() -> yul::Statement { revert_operations::panic_revert(PANIC_DIV_OR_MOD_BY_ZERO) } +fn checked_neg_signed(size: Integer) -> yul::Statement { + if !size.is_signed() { + panic!("Expected signed integer") + } + let (min_value, _) = get_min_max(size); + let fn_name = names::checked_neg(&size); + function_definition! { + function [fn_name](val1) -> result { + // overflow for min_val + (if ((eq(val1, [min_value]))) { [revert_with_over_or_under_flow()] }) + (result := sub(0, val1)) + } + } +} + fn checked_mod_unsigned() -> yul::Statement { function_definition! { function checked_mod_unsigned(val1, val2) -> result { diff --git a/newsfragments/578.bugfix.md b/newsfragments/578.bugfix.md new file mode 100644 index 0000000000..63fca2868a --- /dev/null +++ b/newsfragments/578.bugfix.md @@ -0,0 +1,8 @@ +Ensure negation is checked and reverts with over/underflow if needed. + +Example: + +The minimum value for an `i8` is `-128` but the maximum value of an `i8` +is `127` which means that negating `-128` should lead to an overflow since +`128` does not fit into an `i8`. Before this fix, negation operations where +not checked for over/underflow resulting in returning the oversized value. diff --git a/newsfragments/578.internal.md b/newsfragments/578.internal.md new file mode 100644 index 0000000000..cb62def00b --- /dev/null +++ b/newsfragments/578.internal.md @@ -0,0 +1,7 @@ +Added a new category of tests: differential contract testing. + +Each of these tests is pased on a pair of contracts where one implementation +is written in Fe and the other one is written in Solidity. The implementations +should have the same public APIs and are assumed to always return identical +results given equal inputs. The inputs are randomly generated using `proptest` +and hence are expected to discover unknown bugs.