From 4cb767e4fbd1527e80184bc66efe721a43cc01fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Halil=20Beglerovi=C4=87?= Date: Thu, 9 Feb 2023 01:08:12 +0100 Subject: [PATCH] feat: support all revert signals (#831) --- .github/workflows/ci.yml | 4 +- packages/fuels-core/src/constants.rs | 4 - packages/fuels-programs/src/constants.rs | 14 ++ packages/fuels-programs/src/contract.rs | 12 +- packages/fuels-programs/src/lib.rs | 1 + packages/fuels-programs/src/logs.rs | 50 +++---- packages/fuels-programs/src/script_calls.rs | 6 +- .../{assert_eq => asserts}/Forc.toml | 2 +- .../{assert_eq => asserts}/src/main.sw | 5 + packages/fuels/tests/logs.rs | 125 ++++++++++++++---- .../Forc.toml | 2 +- .../src/main.sw | 5 +- 12 files changed, 158 insertions(+), 72 deletions(-) create mode 100644 packages/fuels-programs/src/constants.rs rename packages/fuels/tests/contracts/{assert_eq => asserts}/Forc.toml (85%) rename packages/fuels/tests/contracts/{assert_eq => asserts}/src/main.sw (92%) rename packages/fuels/tests/scripts/{script_assert_eq => script_asserts}/Forc.toml (80%) rename packages/fuels/tests/scripts/{script_assert_eq => script_asserts}/src/main.sw (87%) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5fb20e5608..13ea91606f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -33,7 +33,9 @@ jobs: toolchain: ${{ env.RUST_VERSION }} # selecting a toolchain either by action or manual `rustup` calls should happen # before the cache plugin, as it uses the current rustc version as its cache key - - uses: Swatinem/rust-cache@v1 + - uses: Swatinem/rust-cache@v2 + with: + prefix-key: "v1-rust" - name: Set git config run: | diff --git a/packages/fuels-core/src/constants.rs b/packages/fuels-core/src/constants.rs index c3fe4c1884..9daaa7aaf7 100644 --- a/packages/fuels-core/src/constants.rs +++ b/packages/fuels-core/src/constants.rs @@ -15,8 +15,4 @@ pub const BASE_ASSET_ID: AssetId = AssetId::BASE; pub const BASE_MESSAGE_ID: MessageId = MessageId::zeroed(); pub const DEFAULT_GAS_ESTIMATION_TOLERANCE: f64 = 0.2; -pub const GAS_PRICE_FACTOR: u64 = 1_000_000_000; pub const MAX_GAS_PER_TX: u64 = 100_000_000; - -// Revert return value that indicates missing output variables -pub const FAILED_TRANSFER_TO_ADDRESS_SIGNAL: u64 = 0xffff_ffff_ffff_0001; diff --git a/packages/fuels-programs/src/constants.rs b/packages/fuels-programs/src/constants.rs new file mode 100644 index 0000000000..f906f7280d --- /dev/null +++ b/packages/fuels-programs/src/constants.rs @@ -0,0 +1,14 @@ +/// Revert with this value for a failing call to `require` +pub const FAILED_REQUIRE_SIGNAL: u64 = 0xffff_ffff_ffff_0000; + +/// Revert with this value for a failing call to `transfer_to_address`. +pub const FAILED_TRANSFER_TO_ADDRESS_SIGNAL: u64 = 0xffff_ffff_ffff_0001; + +/// Revert with this value for a failing call to `send_message`. +pub const FAILED_SEND_MESSAGE_SIGNAL: u64 = 0xffff_ffff_ffff_0002; + +/// Revert with this value for a failing call to `assert_eq`. +pub const FAILED_ASSERT_EQ_SIGNAL: u64 = 0xffff_ffff_ffff_0003; + +/// Revert with this value for a failing call to `assert`. +pub const FAILED_ASSERT_SIGNAL: u64 = 0xffff_ffff_ffff_0004; diff --git a/packages/fuels-programs/src/contract.rs b/packages/fuels-programs/src/contract.rs index 4eec917810..9ff38c68e1 100644 --- a/packages/fuels-programs/src/contract.rs +++ b/packages/fuels-programs/src/contract.rs @@ -16,7 +16,6 @@ use fuel_vm::fuel_asm::PanicReason; use fuels_core::{ abi_decoder::ABIDecoder, abi_encoder::{ABIEncoder, UnresolvedBytes}, - constants::FAILED_TRANSFER_TO_ADDRESS_SIGNAL, parameters::{CallParameters, StorageConfiguration, TxParameters}, }; use fuels_signers::{ @@ -33,8 +32,9 @@ use fuels_types::{ use crate::{ call_response::FuelCallResponse, + constants::FAILED_TRANSFER_TO_ADDRESS_SIGNAL, execution_script::ExecutableFuelCall, - logs::{decode_revert_error, LogDecoder}, + logs::{map_revert_error, LogDecoder}, }; /// How many times to attempt to resolve missing tx dependencies. @@ -682,7 +682,7 @@ where pub async fn call(self) -> Result> { Self::call_or_simulate(&self, false) .await - .map_err(|err| decode_revert_error(err, &self.log_decoder)) + .map_err(|err| map_revert_error(err, &self.log_decoder)) } /// Call a contract's method on the node, in a simulated manner, meaning the state of the @@ -693,7 +693,7 @@ where pub async fn simulate(self) -> Result> { Self::call_or_simulate(&self, true) .await - .map_err(|err| decode_revert_error(err, &self.log_decoder)) + .map_err(|err| map_revert_error(err, &self.log_decoder)) } /// Simulates a call without needing to resolve the generic for the return type @@ -827,7 +827,7 @@ impl MultiContractCallHandler { pub async fn call(&self) -> Result> { Self::call_or_simulate(self, false) .await - .map_err(|err| decode_revert_error(err, &self.log_decoder)) + .map_err(|err| map_revert_error(err, &self.log_decoder)) } /// Call contract methods on the node, in a simulated manner, meaning the state of the @@ -838,7 +838,7 @@ impl MultiContractCallHandler { pub async fn simulate(&self) -> Result> { Self::call_or_simulate(self, true) .await - .map_err(|err| decode_revert_error(err, &self.log_decoder)) + .map_err(|err| map_revert_error(err, &self.log_decoder)) } async fn call_or_simulate( diff --git a/packages/fuels-programs/src/lib.rs b/packages/fuels-programs/src/lib.rs index 283af6ce70..28ab2112c4 100644 --- a/packages/fuels-programs/src/lib.rs +++ b/packages/fuels-programs/src/lib.rs @@ -1,5 +1,6 @@ pub mod call_response; pub mod call_utils; +pub mod constants; pub mod contract; pub mod execution_script; pub mod logs; diff --git a/packages/fuels-programs/src/logs.rs b/packages/fuels-programs/src/logs.rs index 7feaac6769..5173d951b0 100644 --- a/packages/fuels-programs/src/logs.rs +++ b/packages/fuels-programs/src/logs.rs @@ -12,8 +12,10 @@ use fuels_types::{ traits::{Parameterize, Tokenizable}, }; -const REQUIRE_ID: u64 = 0xffff_ffff_ffff_0000; -const ASSERT_EQ_ID: u64 = 0xffff_ffff_ffff_0003; +use crate::constants::{ + FAILED_ASSERT_EQ_SIGNAL, FAILED_ASSERT_SIGNAL, FAILED_REQUIRE_SIGNAL, + FAILED_SEND_MESSAGE_SIGNAL, FAILED_TRANSFER_TO_ADDRESS_SIGNAL, +}; /// Holds a unique log ID #[derive(Debug, Clone, Default, PartialEq, Eq, Hash)] @@ -88,39 +90,37 @@ impl<'a, I: Iterator> ExtractLogIdData for I { } } -/// Decodes the logged type from the receipt of a `RevertTransactionError` if available -pub fn decode_revert_error(err: Error, log_decoder: &LogDecoder) -> Error { +/// Map the provided `RevertTransactionError` based on the `revert_id`. +/// If applicable, decode the logged types from the receipt. +pub fn map_revert_error(mut err: Error, log_decoder: &LogDecoder) -> Error { if let Error::RevertTransactionError { revert_id, - receipts, - .. - } = &err + ref receipts, + ref mut reason, + } = err { - match *revert_id { - REQUIRE_ID => return decode_require_revert(log_decoder, receipts), - ASSERT_EQ_ID => return decode_assert_eq_revert(log_decoder, receipts), + match revert_id { + FAILED_REQUIRE_SIGNAL => *reason = decode_require_revert(log_decoder, receipts), + FAILED_ASSERT_EQ_SIGNAL => *reason = decode_assert_eq_revert(log_decoder, receipts), + FAILED_ASSERT_SIGNAL => *reason = "assertion failed.".into(), + FAILED_SEND_MESSAGE_SIGNAL => *reason = "failed to send message.".into(), + FAILED_TRANSFER_TO_ADDRESS_SIGNAL => *reason = "failed transfer to address.".into(), _ => {} } } err } -fn decode_require_revert(log_decoder: &LogDecoder, receipts: &[Receipt]) -> Error { - let reason = log_decoder +fn decode_require_revert(log_decoder: &LogDecoder, receipts: &[Receipt]) -> String { + log_decoder .get_logs(receipts) .ok() .and_then(|logs| logs.last().cloned()) - .unwrap_or_else(|| "Failed to decode log from require revert".to_string()); - - Error::RevertTransactionError { - reason, - revert_id: REQUIRE_ID, - receipts: receipts.to_owned(), - } + .unwrap_or_else(|| "failed to decode log from require revert".to_string()) } -fn decode_assert_eq_revert(log_decoder: &LogDecoder, receipts: &[Receipt]) -> Error { - let reason = log_decoder +fn decode_assert_eq_revert(log_decoder: &LogDecoder, receipts: &[Receipt]) -> String { + log_decoder .get_logs(receipts) .ok() .and_then(|logs| { @@ -131,13 +131,7 @@ fn decode_assert_eq_revert(log_decoder: &LogDecoder, receipts: &[Receipt]) -> Er } None }) - .unwrap_or_else(|| "Failed to decode logs from assert_eq revert".to_string()); - - Error::RevertTransactionError { - reason, - revert_id: ASSERT_EQ_ID, - receipts: receipts.to_owned(), - } + .unwrap_or_else(|| "failed to decode logs from assert_eq revert".to_string()) } pub fn log_type_lookup( diff --git a/packages/fuels-programs/src/script_calls.rs b/packages/fuels-programs/src/script_calls.rs index c85ec2579e..a7e8c59627 100644 --- a/packages/fuels-programs/src/script_calls.rs +++ b/packages/fuels-programs/src/script_calls.rs @@ -20,7 +20,7 @@ use crate::{ call_utils::{generate_contract_inputs, generate_contract_outputs}, contract::{get_decoded_output, SettableContract}, execution_script::ExecutableFuelCall, - logs::{decode_revert_error, LogDecoder}, + logs::{map_revert_error, LogDecoder}, }; #[derive(Debug)] @@ -197,7 +197,7 @@ where pub async fn call(self) -> Result> { Self::call_or_simulate(&self, false) .await - .map_err(|err| decode_revert_error(err, &self.log_decoder)) + .map_err(|err| map_revert_error(err, &self.log_decoder)) } /// Call a script on the node, in a simulated manner, meaning the state of the @@ -208,7 +208,7 @@ where pub async fn simulate(self) -> Result> { Self::call_or_simulate(&self, true) .await - .map_err(|err| decode_revert_error(err, &self.log_decoder)) + .map_err(|err| map_revert_error(err, &self.log_decoder)) } /// Create a [`FuelCallResponse`] from call receipts diff --git a/packages/fuels/tests/contracts/assert_eq/Forc.toml b/packages/fuels/tests/contracts/asserts/Forc.toml similarity index 85% rename from packages/fuels/tests/contracts/assert_eq/Forc.toml rename to packages/fuels/tests/contracts/asserts/Forc.toml index fca1fe9e0f..e6d8b1f233 100644 --- a/packages/fuels/tests/contracts/assert_eq/Forc.toml +++ b/packages/fuels/tests/contracts/asserts/Forc.toml @@ -2,6 +2,6 @@ authors = ["Fuel Labs "] entry = "main.sw" license = "Apache-2.0" -name = "assert_eq" +name = "asserts" [dependencies] diff --git a/packages/fuels/tests/contracts/assert_eq/src/main.sw b/packages/fuels/tests/contracts/asserts/src/main.sw similarity index 92% rename from packages/fuels/tests/contracts/assert_eq/src/main.sw rename to packages/fuels/tests/contracts/asserts/src/main.sw index 7c1ee4d359..8978ccbdad 100644 --- a/packages/fuels/tests/contracts/assert_eq/src/main.sw +++ b/packages/fuels/tests/contracts/asserts/src/main.sw @@ -41,12 +41,17 @@ impl Eq for TestEnum { } abi TestContract { + fn assert_primitive(a: u64, b: u64); fn assert_eq_primitive(a: u64, b: u64); fn assert_eq_struct(test_struct: TestStruct, test_struct2: TestStruct); fn assert_eq_enum(test_enum: TestEnum, test_enum2: TestEnum); } impl TestContract for Contract { + fn assert_primitive(a: u64, b: u64) { + assert(a == b); + } + fn assert_eq_primitive(a: u64, b: u64) { assert_eq(a, b); } diff --git a/packages/fuels/tests/logs.rs b/packages/fuels/tests/logs.rs index b7a1c7c0fb..fbdf015a9f 100644 --- a/packages/fuels/tests/logs.rs +++ b/packages/fuels/tests/logs.rs @@ -424,7 +424,10 @@ async fn test_multi_call_contract_with_contract_logs() -> Result<()> { fn assert_revert_containing_msg(msg: &str, error: Error) { assert!(matches!(error, Error::RevertTransactionError { .. })); if let Error::RevertTransactionError { reason, .. } = error { - assert!(reason.contains(msg)); + assert!( + reason.contains(msg), + "message: \"{msg}\" not contained in reason: \"{reason}\"" + ); } } @@ -449,7 +452,7 @@ async fn test_require_log() -> Result<()> { .require_primitive() .call() .await - .expect_err("Should return a revert error"); + .expect_err("should return a revert error"); assert_revert_containing_msg("42", error); } @@ -458,7 +461,7 @@ async fn test_require_log() -> Result<()> { .require_string() .call() .await - .expect_err("Should return a revert error"); + .expect_err("should return a revert error"); assert_revert_containing_msg("fuel", error); } @@ -467,7 +470,7 @@ async fn test_require_log() -> Result<()> { .require_custom_generic() .call() .await - .expect_err("Should return a revert error"); + .expect_err("should return a revert error"); assert_revert_containing_msg("StructDeeplyNestedGeneric", error); } @@ -476,7 +479,7 @@ async fn test_require_log() -> Result<()> { .require_with_additional_logs() .call() .await - .expect_err("Should return a revert error"); + .expect_err("should return a revert error"); assert_revert_containing_msg("64", error); } @@ -516,7 +519,7 @@ async fn test_multi_call_require_log_single_contract() -> Result<()> { let error = multi_call_handler .call::<((), ())>() .await - .expect_err("Should return a revert error"); + .expect_err("should return a revert error"); assert_revert_containing_msg("fuel", error); } @@ -533,7 +536,7 @@ async fn test_multi_call_require_log_single_contract() -> Result<()> { let error = multi_call_handler .call::<((), ())>() .await - .expect_err("Should return a revert error"); + .expect_err("should return a revert error"); assert_revert_containing_msg("StructDeeplyNestedGeneric", error); } @@ -579,7 +582,7 @@ async fn test_multi_call_require_log_multi_contract() -> Result<()> { let error = multi_call_handler .call::<((), ())>() .await - .expect_err("Should return a revert error"); + .expect_err("should return a revert error"); assert_revert_containing_msg("fuel", error); } @@ -596,7 +599,7 @@ async fn test_multi_call_require_log_multi_contract() -> Result<()> { let error = multi_call_handler .call::<((), ())>() .await - .expect_err("Should return a revert error"); + .expect_err("should return a revert error"); assert_revert_containing_msg("StructDeeplyNestedGeneric", error); } @@ -888,7 +891,7 @@ async fn test_script_require_log() -> Result<()> { .main(MatchEnum::RequirePrimitive) .call() .await - .expect_err("Should return a revert error"); + .expect_err("should return a revert error"); assert_revert_containing_msg("42", error); } @@ -897,7 +900,7 @@ async fn test_script_require_log() -> Result<()> { .main(MatchEnum::RequireString) .call() .await - .expect_err("Should return a revert error"); + .expect_err("should return a revert error"); assert_revert_containing_msg("fuel", error); } @@ -907,7 +910,7 @@ async fn test_script_require_log() -> Result<()> { .main(MatchEnum::RequireCustomGeneric) .call() .await - .expect_err("Should return a revert error"); + .expect_err("should return a revert error"); assert_revert_containing_msg("StructDeeplyNestedGeneric", error); } @@ -917,7 +920,7 @@ async fn test_script_require_log() -> Result<()> { .main(MatchEnum::RequireWithAdditionalLogs) .call() .await - .expect_err("Should return a revert error"); + .expect_err("should return a revert error"); assert_revert_containing_msg("64", error); } @@ -961,7 +964,7 @@ async fn test_contract_require_from_contract() -> Result<()> { .set_contracts(&[&contract_instance]) .call() .await - .expect_err("Should return a revert error"); + .expect_err("should return a revert error"); assert_revert_containing_msg("require from contract", error); @@ -1023,7 +1026,7 @@ async fn test_multi_call_contract_require_from_contract() -> Result<()> { let error = multi_call_handler .call::<((), ())>() .await - .expect_err("Should return a revert error"); + .expect_err("should return a revert error"); assert_revert_containing_msg("require from contract", error); @@ -1057,7 +1060,7 @@ async fn test_script_require_from_contract() -> Result<()> { .set_contracts(&[&contract_instance]) .call() .await - .expect_err("Should return a revert error"); + .expect_err("should return a revert error"); assert_revert_containing_msg("require from contract", error); @@ -1070,12 +1073,12 @@ fn assert_assert_eq_containing_msg(left: T, right: T, error: Error) { } #[tokio::test] -async fn test_contract_assert_eq_log() -> Result<()> { +async fn test_contract_asserts_log() -> Result<()> { setup_contract_test!( Wallets("wallet"), Abigen( name = "LogContract", - abi = "packages/fuels/tests/contracts/assert_eq" + abi = "packages/fuels/tests/contracts/asserts" ), Deploy( name = "contract_instance", @@ -1088,11 +1091,23 @@ async fn test_contract_assert_eq_log() -> Result<()> { let a = 32; let b = 64; + let error = contract_methods + .assert_primitive(a, b) + .call() + .await + .expect_err("should return a revert error"); + + assert_revert_containing_msg("assertion failed", error); + } + { + let a = 32; + let b = 64; + let error = contract_methods .assert_eq_primitive(a, b) .call() .await - .expect_err("Should return a revert error"); + .expect_err("should return a revert error"); assert_assert_eq_containing_msg(a, b, error); } @@ -1111,7 +1126,7 @@ async fn test_contract_assert_eq_log() -> Result<()> { .assert_eq_struct(test_struct.clone(), test_struct2.clone()) .call() .await - .expect_err("Should return a revert error"); + .expect_err("should return a revert error"); assert_assert_eq_containing_msg(test_struct, test_struct2, error); } @@ -1123,7 +1138,7 @@ async fn test_contract_assert_eq_log() -> Result<()> { .assert_eq_enum(test_enum.clone(), test_enum2.clone()) .call() .await - .expect_err("Should return a revert error"); + .expect_err("should return a revert error"); assert_assert_eq_containing_msg(test_enum, test_enum2, error); } @@ -1132,16 +1147,28 @@ async fn test_contract_assert_eq_log() -> Result<()> { } #[tokio::test] -async fn test_script_assert_eq_log() -> Result<()> { +async fn test_script_asserts_log() -> Result<()> { abigen!(Script( name = "log_script", - abi = "packages/fuels/tests/scripts/script_assert_eq/out/debug/script_assert_eq-abi.json" + abi = "packages/fuels/tests/scripts/script_asserts/out/debug/script_asserts-abi.json" )); let wallet = launch_provider_and_get_wallet().await; - let bin_path = "../fuels/tests/scripts/script_assert_eq/out/debug/script_assert_eq.bin"; + let bin_path = "../fuels/tests/scripts/script_asserts/out/debug/script_asserts.bin"; let instance = log_script::new(wallet.clone(), bin_path); + { + let a = 32; + let b = 64; + + let error = instance + .main(MatchEnum::AssertPrimitive((a, b))) + .call() + .await + .expect_err("should return a revert error"); + + assert_revert_containing_msg("assertion failed", error); + } { let a = 32; let b = 64; @@ -1150,7 +1177,7 @@ async fn test_script_assert_eq_log() -> Result<()> { .main(MatchEnum::AssertEqPrimitive((a, b))) .call() .await - .expect_err("Should return a revert error"); + .expect_err("should return a revert error"); assert_assert_eq_containing_msg(a, b, error); } @@ -1172,7 +1199,7 @@ async fn test_script_assert_eq_log() -> Result<()> { ))) .call() .await - .expect_err("Should return a revert error"); + .expect_err("should return a revert error"); assert_assert_eq_containing_msg(test_struct, test_struct2, error); } @@ -1187,10 +1214,54 @@ async fn test_script_assert_eq_log() -> Result<()> { ))) .call() .await - .expect_err("Should return a revert error"); + .expect_err("should return a revert error"); assert_assert_eq_containing_msg(test_enum, test_enum2, error); } Ok(()) } + +#[tokio::test] +async fn contract_token_ops_error_messages() -> Result<()> { + setup_contract_test!( + Wallets("wallet"), + Abigen( + name = "TestContract", + abi = "packages/fuels/tests/contracts/token_ops" + ), + Deploy( + name = "contract_instance", + contract = "TestContract", + wallet = "wallet" + ), + ); + let contract_methods = contract_instance.methods(); + + { + let base_layer_address = Bits256([1u8; 32]); + let amount = 1000; + + let error = contract_methods + .send_message(base_layer_address, amount) + .call() + .await + .expect_err("should return a revert error"); + + assert_revert_containing_msg("failed to send message", error); + } + { + let contract_id = contract_instance.contract_id().into(); + let address = wallet.address().into(); + + let error = contract_methods + .transfer_coins_to_output(1_000_000, contract_id, address) + .call() + .await + .expect_err("should return a revert error"); + + assert_revert_containing_msg("failed transfer to address", error); + } + + Ok(()) +} diff --git a/packages/fuels/tests/scripts/script_assert_eq/Forc.toml b/packages/fuels/tests/scripts/script_asserts/Forc.toml similarity index 80% rename from packages/fuels/tests/scripts/script_assert_eq/Forc.toml rename to packages/fuels/tests/scripts/script_asserts/Forc.toml index 5adacb43fb..3a56915964 100644 --- a/packages/fuels/tests/scripts/script_assert_eq/Forc.toml +++ b/packages/fuels/tests/scripts/script_asserts/Forc.toml @@ -2,6 +2,6 @@ authors = ["Fuel Labs "] entry = "main.sw" license = "Apache-2.0" -name = "script_assert_eq" +name = "script_asserts" [dependencies] diff --git a/packages/fuels/tests/scripts/script_assert_eq/src/main.sw b/packages/fuels/tests/scripts/script_asserts/src/main.sw similarity index 87% rename from packages/fuels/tests/scripts/script_assert_eq/src/main.sw rename to packages/fuels/tests/scripts/script_asserts/src/main.sw index 1459be4461..0199bcc4ed 100644 --- a/packages/fuels/tests/scripts/script_assert_eq/src/main.sw +++ b/packages/fuels/tests/scripts/script_asserts/src/main.sw @@ -41,13 +41,16 @@ impl Eq for TestEnum { } enum MatchEnum { + AssertPrimitive: (u64, u64), AssertEqPrimitive: (u64, u64), AssertEqStruct: (TestStruct, TestStruct), AssertEqEnum: (TestEnum, TestEnum), } fn main(match_enum: MatchEnum) { - if let MatchEnum::AssertEqPrimitive((a, b)) = match_enum { + if let MatchEnum::AssertPrimitive((a, b)) = match_enum { + assert(a == b); + } else if let MatchEnum::AssertEqPrimitive((a, b)) = match_enum { assert_eq(a, b); } else if let MatchEnum::AssertEqStruct((test_struct, test_struct2)) = match_enum {