From bba0d6429ac05bfea389ab0a994e895eb820fba2 Mon Sep 17 00:00:00 2001 From: Alex Hansen Date: Fri, 28 Jan 2022 09:10:43 -0800 Subject: [PATCH] Revert instead of return when no function selectors match (#639) * revert instead of return on function selector miss * merge from master; reintroduce dependencies test * Expect reverts in contract tests --- sway-core/src/asm_generation/mod.rs | 7 ++++--- test/src/e2e_vm_tests/mod.rs | 8 +++++--- .../src/e2e_vm_tests/test_programs/bal_opcode/src/main.sw | 2 +- .../test_programs/call_basic_storage/src/main.sw | 2 +- .../test_programs/call_increment_contract/src/main.sw | 2 +- .../test_programs/caller_auth_test/src/main.sw | 2 +- .../test_programs/caller_context_test/src/main.sw | 2 +- .../e2e_vm_tests/test_programs/contract_call/src/main.sw | 2 +- 8 files changed, 15 insertions(+), 12 deletions(-) diff --git a/sway-core/src/asm_generation/mod.rs b/sway-core/src/asm_generation/mod.rs index 8524cd09f4d..d6fdaaac4e6 100644 --- a/sway-core/src/asm_generation/mod.rs +++ b/sway-core/src/asm_generation/mod.rs @@ -1249,13 +1249,14 @@ fn build_contract_abi_switch( }); } - // if none of the selectors matched, then ret + // if none of the selectors matched, then revert asm_buf.push(Op { // see https://github.com/FuelLabs/sway/issues/97#issuecomment-875674105 - opcode: Either::Left(VirtualOp::RET(VirtualRegister::Constant( + // and https://github.com/FuelLabs/sway/issues/444#issuecomment-1012507337 + opcode: Either::Left(VirtualOp::RVRT(VirtualRegister::Constant( ConstantRegister::Zero, ))), - comment: "return if no selectors matched".into(), + comment: "revert if no selectors matched".into(), owning_span: None, }); diff --git a/test/src/e2e_vm_tests/mod.rs b/test/src/e2e_vm_tests/mod.rs index ad6c351ccb6..a792a21bbe2 100644 --- a/test/src/e2e_vm_tests/mod.rs +++ b/test/src/e2e_vm_tests/mod.rs @@ -14,8 +14,10 @@ pub fn run(filter_regex: Option) { let positive_project_names = vec![ ("asm_expr_basic", ProgramState::Return(6)), ("basic_func_decl", ProgramState::Return(1)), // 1 == true - ("contract_abi_impl", ProgramState::Return(0)), - // TEMPORARILY DISABLED DUE TO OOM ("dependencies", ProgramState::Return(0)), // 0 == false + // contracts revert because this test runs them against the VM + // and no selectors will match + ("contract_abi_impl", ProgramState::Revert(0)), + ("dependencies", ProgramState::Return(0)), // 0 == false ("if_elseif_enum", ProgramState::Return(10)), ("tuple_types", ProgramState::Return(123)), ("out_of_order_decl", ProgramState::Return(1)), @@ -74,7 +76,7 @@ pub fn run(filter_regex: Option) { ("block_height", ProgramState::Return(1)), // true ("b512_test", ProgramState::Return(1)), // true ("block_height", ProgramState::Return(1)), // true - ("valid_impurity", ProgramState::Return(0)), // false + ("valid_impurity", ProgramState::Revert(0)), // false ("trait_override_bug", ProgramState::Return(7)), ("if_implicit_unit", ProgramState::Return(0)), ("modulo_uint_test", ProgramState::Return(1)), // true diff --git a/test/src/e2e_vm_tests/test_programs/bal_opcode/src/main.sw b/test/src/e2e_vm_tests/test_programs/bal_opcode/src/main.sw index ee4064f01bc..33b5095f358 100644 --- a/test/src/e2e_vm_tests/test_programs/bal_opcode/src/main.sw +++ b/test/src/e2e_vm_tests/test_programs/bal_opcode/src/main.sw @@ -6,7 +6,7 @@ use balance_test_abi::BalanceTest; fn main() -> bool{ // @todo switch to using ContractId when abi signature changes. - let balance_test_contract_id = 0x2152e04a705351b6483514d212a333090f7c5f40cb0b9b802089aaa33572e501; + let balance_test_contract_id = 0x6b5677971f7d0e94d76c18f268d8ccffd04b5b3f3bdb2f1da119b76e376dcf04; let balance_test_contract = abi(BalanceTest, balance_test_contract_id); let number = balance_test_contract.get_42(1000, 0, ETH_ID, ()); diff --git a/test/src/e2e_vm_tests/test_programs/call_basic_storage/src/main.sw b/test/src/e2e_vm_tests/test_programs/call_basic_storage/src/main.sw index 086e59df826..f7a106497e9 100644 --- a/test/src/e2e_vm_tests/test_programs/call_basic_storage/src/main.sw +++ b/test/src/e2e_vm_tests/test_programs/call_basic_storage/src/main.sw @@ -2,7 +2,7 @@ script; use basic_storage_abi::{StoreU64, StoreU64Request}; fn main() -> u64 { - let addr = abi(StoreU64, 0x410eab113ce1c194952b92295f3d156bce478633feb2e0117360ff28b034a751); + let addr = abi(StoreU64, 0x68a009769e8266282e0b3186602373a0fc65f08c35d260392c8cb12fbcd61277); let req = StoreU64Request { key: 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff, value: 4242 diff --git a/test/src/e2e_vm_tests/test_programs/call_increment_contract/src/main.sw b/test/src/e2e_vm_tests/test_programs/call_increment_contract/src/main.sw index 8abd4b8968d..f5b066e0b4a 100644 --- a/test/src/e2e_vm_tests/test_programs/call_increment_contract/src/main.sw +++ b/test/src/e2e_vm_tests/test_programs/call_increment_contract/src/main.sw @@ -2,7 +2,7 @@ script; use increment_abi::Incrementor; use std::constants::ETH_ID; fn main() { - let abi = abi(Incrementor, 0xf7f9d5f37723833e266ff185bd3ded8af980f50703b1719dd47a446af2dabc70); + let abi = abi(Incrementor, 0x19a4738f92544ccf46d2de5b84e273507512e42058dd8efd652546f576ac8bc0); abi.initialize(10000, 0, ETH_ID, 0); // comment this line out to just increment without initializing abi.increment(10000, 0, ETH_ID, 5); let result = abi.increment(10000, 0, ETH_ID, 5); diff --git a/test/src/e2e_vm_tests/test_programs/caller_auth_test/src/main.sw b/test/src/e2e_vm_tests/test_programs/caller_auth_test/src/main.sw index 71290a3826f..4134e7dfd29 100644 --- a/test/src/e2e_vm_tests/test_programs/caller_auth_test/src/main.sw +++ b/test/src/e2e_vm_tests/test_programs/caller_auth_test/src/main.sw @@ -4,7 +4,7 @@ use auth_testing_abi::AuthTesting; // should be false in the case of a script fn main() -> bool { - let caller = abi(AuthTesting, 0x27829e78404b18c037b15bfba5110c613a83ea22c718c8b51596e17c9cb1cd6f); + let caller = abi(AuthTesting, 0xf8aa0c04665af0fd65a6ea6a05e42a57ec737d953af70a200a10bc3c0eec4553); caller.returns_gm_one(1000, 0, ETH_ID, ()) } diff --git a/test/src/e2e_vm_tests/test_programs/caller_context_test/src/main.sw b/test/src/e2e_vm_tests/test_programs/caller_context_test/src/main.sw index 1ed2478337c..f275e39e269 100644 --- a/test/src/e2e_vm_tests/test_programs/caller_context_test/src/main.sw +++ b/test/src/e2e_vm_tests/test_programs/caller_context_test/src/main.sw @@ -6,7 +6,7 @@ fn main() -> bool { let gas: u64 = 1000; let amount: u64 = 11; let other_contract_id = ~ContractId::from(0x27829e78404b18c037b15bfba5110c613a83ea22c718c8b51596e17c9cb1cd6f); - let deployed_contract_id = 0x2152e04a705351b6483514d212a333090f7c5f40cb0b9b802089aaa33572e501; + let deployed_contract_id = 0x2890a0a3fb38e88d4ef887d78db0cd4483583a00506c30b572dff1aa73305b3e; let test_contract = abi(ContextTesting, deployed_contract_id); diff --git a/test/src/e2e_vm_tests/test_programs/contract_call/src/main.sw b/test/src/e2e_vm_tests/test_programs/contract_call/src/main.sw index 42b84d31332..86f758f8316 100644 --- a/test/src/e2e_vm_tests/test_programs/contract_call/src/main.sw +++ b/test/src/e2e_vm_tests/test_programs/contract_call/src/main.sw @@ -13,7 +13,7 @@ abi MyContract { } fn main() -> u64 { - let x = abi(MyContract, 0x6c626fddd128e24e6805fe1779779f14097d34086c571dd8df1c78ac4bb9a78b); + let x = abi(MyContract, 0x3dba0a4455b598b7655a7fb430883d96c9527ef275b49739e7b0ad12f8280eae); let asset_id = 0x7777_7777_7777_7777_7777_7777_7777_7777_7777_7777_7777_7777_7777_7777_7777_7777; let input = InputStruct { field_1: true,