From cf3075209ffdc9d82312793a4795acff36675a25 Mon Sep 17 00:00:00 2001 From: iqdecay Date: Mon, 28 Aug 2023 15:08:00 +0200 Subject: [PATCH 01/32] first draft --- packages/fuels-core/src/codec/abi_decoder.rs | 14 +++- packages/fuels-core/src/types/param_types.rs | 80 +++++++++++++++++-- packages/fuels-programs/src/call_utils.rs | 15 +++- packages/fuels-programs/src/receipt_parser.rs | 40 ++++++++-- packages/fuels/tests/types_contracts.rs | 61 ++++++++++++++ 5 files changed, 196 insertions(+), 14 deletions(-) diff --git a/packages/fuels-core/src/codec/abi_decoder.rs b/packages/fuels-core/src/codec/abi_decoder.rs index ecacff9e90..e33bc80e0a 100644 --- a/packages/fuels-core/src/codec/abi_decoder.rs +++ b/packages/fuels-core/src/codec/abi_decoder.rs @@ -272,13 +272,23 @@ impl ABIDecoder { /// * `data`: slice of encoded data on whose beginning we're expecting an encoded enum /// * `variants`: all types that this particular enum type could hold fn decode_enum(bytes: &[u8], variants: &EnumVariants) -> Result { + let mut data: Vec = bytes.clone().into(); let enum_width = variants.compute_encoding_width_of_enum(); - let discriminant = peek_u32(bytes)? as u8; + let discriminant = peek_u32(&data)? as u8; let selected_variant = variants.param_type_of_variant(discriminant)?; + if selected_variant.uses_heap_types() { + // remove the 3 WORDS that represent (ptr, cap, len) + // those 3 WORDS are leftovers from the concatenation of the data from the two + // `ReturnData` receipts. We need to remove them only if the selected variant is a + // heap type, otherwise we are erasing relevant data. + for _ in 8..32 { + data.remove(8); + } + } let words_to_skip = enum_width - selected_variant.compute_encoding_width(); - let enum_content_bytes = skip(bytes, words_to_skip * WORD_SIZE)?; + let enum_content_bytes = skip(&data, words_to_skip * WORD_SIZE)?; let result = Self::decode_token_in_enum(enum_content_bytes, variants, selected_variant)?; let selector = Box::new((discriminant, result.token, variants.clone())); diff --git a/packages/fuels-core/src/types/param_types.rs b/packages/fuels-core/src/types/param_types.rs index a615f41926..8eedc2200c 100644 --- a/packages/fuels-core/src/types/param_types.rs +++ b/packages/fuels-core/src/types/param_types.rs @@ -88,11 +88,44 @@ impl ParamType { // `Bytes` type nested, it is an exception that will be generalized as part of // https://github.com/FuelLabs/fuels-rs/discussions/944 ParamType::String => false, + // We are currently allowing enums that are wrapping around heap types to allow for + // `Result` to be returned from a contract + ParamType::Enum { .. } => self.enum_contains_nested_heap_types(), _ => self.uses_heap_types(), } } - fn uses_heap_types(&self) -> bool { + pub fn enum_contains_nested_heap_types(&self) -> bool { + match &self { + ParamType::Enum { variants, generics } => { + let mut param_types = chain!(variants.param_types(), generics); + param_types.any(|p| p.param_type_has_wrong_format()) + } + // This case should never happen because it is called on a match statement + _ => false, + } + } + + // This is called inside the `enum_contains_nested_heap_types` on each type contained in the + // enum + fn param_type_has_wrong_format(&self) -> bool { + match self { + ParamType::Array(param_type, ..) => param_type.uses_heap_types(), + ParamType::Tuple(param_types, ..) => Self::any_nested_heap_types(param_types), + ParamType::Enum { + generics, variants, .. + } => { + let variants_types = variants.param_types(); + Self::any_nested_heap_types(chain!(generics, variants_types)) + } + ParamType::Struct { + fields, generics, .. + } => Self::any_nested_heap_types(chain!(fields, generics)), + _ => false, + } + } + + pub fn uses_heap_types(&self) -> bool { match &self { ParamType::Vector(..) | ParamType::Bytes | ParamType::String => true, ParamType::Array(param_type, ..) => param_type.uses_heap_types(), @@ -131,6 +164,15 @@ impl ParamType { } // `Bytes` type is byte-packed in the VM, so it's the size of an u8 ParamType::Bytes | ParamType::String => Some(std::mem::size_of::()), + ParamType::Enum { variants, generics } => { + let variants_types = variants.param_types(); + for param in chain!(generics, variants_types) { + if param.is_vm_heap_type() { + return Some(param.heap_inner_element_size().unwrap()); + } + } + None + } _ => None, } } @@ -1349,11 +1391,11 @@ mod tests { variants: EnumVariants::new(param_types_no_nested_vec.clone())?, generics: param_types_no_nested_vec.clone(), }); - is_nested(ParamType::Enum { + not_nested(ParamType::Enum { variants: EnumVariants::new(param_types_nested_vec.clone())?, generics: param_types_no_nested_vec.clone(), }); - is_nested(ParamType::Enum { + not_nested(ParamType::Enum { variants: EnumVariants::new(param_types_no_nested_vec)?, generics: param_types_nested_vec, }); @@ -1406,13 +1448,13 @@ mod tests { variants: EnumVariants::new(param_types_nested_bytes.clone())?, generics: param_types_no_nested_bytes.clone(), }; - is_nested(nested_enum); + not_nested(nested_enum); let nested_enum = ParamType::Enum { variants: EnumVariants::new(param_types_no_nested_bytes)?, generics: param_types_nested_bytes, }; - is_nested(nested_enum); + not_nested(nested_enum); Ok(()) } @@ -1487,6 +1529,34 @@ mod tests { assert_eq!(param_type, ParamType::String); } + #[test] + fn my_function() -> Result<()> { + // Result when `Something` does not contain heap type should work! + let enum_no_heap_types = ParamType::Enum { + variants: EnumVariants::new(vec![ParamType::U64, ParamType::Bool])?, + generics: vec![], + }; + let r = ParamType::Enum { + variants: EnumVariants::new(vec![ParamType::Bytes, enum_no_heap_types])?, + generics: vec![], + }; + println!("{}", r.enum_contains_nested_heap_types()); + + let enum_heap_types = ParamType::Enum { + variants: EnumVariants::new(vec![ + ParamType::Vector(Box::from(ParamType::U64)), + ParamType::Bool, + ])?, + generics: vec![], + }; + let r = ParamType::Enum { + variants: EnumVariants::new(vec![ParamType::Bytes, enum_heap_types])?, + generics: vec![], + }; + println!("{}", r.enum_contains_nested_heap_types()); + Ok(()) + } + fn given_type_with_path(path: &str) -> Type { Type { type_field: format!("struct {path}"), diff --git a/packages/fuels-programs/src/call_utils.rs b/packages/fuels-programs/src/call_utils.rs index 21de11452f..b3f9ac3b93 100644 --- a/packages/fuels-programs/src/call_utils.rs +++ b/packages/fuels-programs/src/call_utils.rs @@ -145,7 +145,7 @@ pub(crate) async fn build_tx_from_contract_calls( fn compute_calls_instructions_len(calls: &[ContractCall]) -> usize { let n_heap_type_calls = calls .iter() - .filter(|c| c.output_param.is_vm_heap_type()) + .filter(|c| c.output_param.heap_inner_element_size().is_some()) .count(); let n_stack_type_calls = calls.len() - n_heap_type_calls; @@ -334,6 +334,19 @@ pub(crate) fn get_single_call_instructions( .to_vec(); // The instructions are different if you want to return data that was on the heap if let Some(inner_type_byte_size) = output_param_type.heap_inner_element_size() { + // This offset is here because if the variant type is an enum containing a heap type, then + // the first word must be skipped because it's the discriminant. The 3 words containing + // (ptr, cap, len) are at the end of the data of the enum, which depends on the encoding + // width. If the variant type does not contain a heap type, then the receipt generated will + // not be read anyway. + let offset = if matches!(output_param_type, ParamType::Enum { .. }) { + // 3 is not a "magic" number: skip 1 word for the discriminant, and the next 2 because + // we are looking left of the 3 words (ptr, cap, len), that are placed at the end + // of the data, on the right. + output_param_type.compute_encoding_width() - 3 + } else { + 0 + }; instructions.extend([ // The RET register contains the pointer address of the `CALL` return (a stack // address). diff --git a/packages/fuels-programs/src/receipt_parser.rs b/packages/fuels-programs/src/receipt_parser.rs index 3bdefff876..2430739557 100644 --- a/packages/fuels-programs/src/receipt_parser.rs +++ b/packages/fuels-programs/src/receipt_parser.rs @@ -61,6 +61,12 @@ impl ReceiptParser { contract_id: &ContractId, ) -> Option> { match output_param.get_return_location() { + ReturnLocation::ReturnData + if output_param.uses_heap_types() + && matches!(output_param, ParamType::Enum { .. }) => + { + self.extract_enum_heap_type_data(contract_id) + } ReturnLocation::ReturnData if output_param.is_vm_heap_type() => { self.extract_return_data_heap(contract_id) } @@ -69,6 +75,24 @@ impl ReceiptParser { } } + fn extract_enum_heap_type_data(&mut self, contract_id: &ContractId) -> Option> { + for (index, (current_receipt, next_receipt)) in + self.receipts.iter().tuple_windows().enumerate() + { + if let (Some(first_data), Some(second_data)) = + Self::extract_vec_data(current_receipt, next_receipt, contract_id) + { + let mut first_data = first_data.clone(); + let mut second_data = second_data.clone(); + self.receipts.drain(index..=index + 1); + first_data.append(&mut second_data); + + return Some(first_data); + } + } + None + } + fn extract_return_data(&mut self, contract_id: &ContractId) -> Option> { for (index, receipt) in self.receipts.iter_mut().enumerate() { if let Receipt::ReturnData { @@ -112,8 +136,10 @@ impl ReceiptParser { for (index, (current_receipt, next_receipt)) in self.receipts.iter().tuple_windows().enumerate() { - if let Some(data) = Self::extract_vec_data(current_receipt, next_receipt, contract_id) { - let data = data.clone(); + if let (_stack_data, Some(heap_data)) = + Self::extract_vec_data(current_receipt, next_receipt, contract_id) + { + let data = heap_data.clone(); self.receipts.drain(index..=index + 1); return Some(data); } @@ -122,10 +148,10 @@ impl ReceiptParser { } fn extract_vec_data<'a>( - current_receipt: &Receipt, + current_receipt: &'a Receipt, next_receipt: &'a Receipt, contract_id: &ContractId, - ) -> Option<&'a Vec> { + ) -> (Option<&'a Vec>, Option<&'a Vec>) { match (current_receipt, next_receipt) { ( Receipt::ReturnData { @@ -140,11 +166,13 @@ impl ReceiptParser { }, ) if *first_id == *contract_id && first_data.is_some() + // The second ReturnData receipt was added by a script instruction, its contract id + // is null && *second_id == ContractId::zeroed() => { - vec_data.as_ref() + (first_data.as_ref(), vec_data.as_ref()) } - _ => None, + _ => (None, None), } } } diff --git a/packages/fuels/tests/types_contracts.rs b/packages/fuels/tests/types_contracts.rs index dcd89acad0..2299b3b9c5 100644 --- a/packages/fuels/tests/types_contracts.rs +++ b/packages/fuels/tests/types_contracts.rs @@ -1,9 +1,14 @@ +#![allow(unused_imports, unused_variables, unused_mut, dead_code)] +use fuel_tx::Receipt; use std::{result::Result as StdResult, str::FromStr}; use fuels::{ prelude::*, types::{Bits256, EvmAddress, Identity, SizedAsciiString, B512, U256}, }; +use fuels_core::codec::ABIDecoder; +use fuels_core::fn_selector; +use fuels_core::types::param_types::ParamType; pub fn null_contract_id() -> Bech32ContractId { // a bech32 contract address that decodes to [0u8;32] @@ -1997,3 +2002,59 @@ async fn test_contract_std_lib_string() -> Result<()> { Ok(()) } + +fn treat_receipts(receipts: Vec) { + let filtered: Vec = receipts + .into_iter() + .clone() + .filter(|r| matches!(r, Receipt::ReturnData { .. })) + .collect(); + + match filtered.get(0).unwrap() { + Receipt::ReturnData { data, .. } => { + let decoded_bytes = + ABIDecoder::decode(&[ParamType::Bytes], &data.clone().unwrap()).unwrap(); + println!("{:?}", decoded_bytes); + } + _ => (), + } + match filtered.get(1).unwrap() { + Receipt::ReturnData { data, .. } => { + let decoded_bytes = + ABIDecoder::decode(&[ParamType::Bytes], &data.clone().unwrap()).unwrap(); + println!("{:?}", decoded_bytes); + } + _ => (), + } +} + +#[tokio::test] +async fn test_1_nested() -> Result<()> { + let wallet = launch_provider_and_get_wallet().await; + setup_program_test!( + Abigen(Contract( + name = "NestedHeap", + project = "packages/fuels/tests/types/contracts/nested_heap_type" + )), + Deploy( + name = "contract_instance", + contract = "NestedHeap", + wallet = "wallet" + ), + ); + let contract_methods = contract_instance.methods(); + + let resp = contract_methods.returns_bytes_result(true).call().await?; + // treat_receipts(resp.receipts); + println!("{:?}", resp.value); + let resp = contract_methods.returns_bytes_result(false).call().await?; + // treat_receipts(resp.receipts); + println!("{:?}", resp.value); + let resp = contract_methods.returns_vec_result(true).call().await?; + // treat_receipts(resp.receipts); + println!("{:?}", resp.value); + let resp = contract_methods.returns_vec_result(false).call().await?; + // treat_receipts(resp.receipts); + println!("{:?}", resp.value); + Ok(()) +} From 512975f631f4aaa161df1a776d66b2ee64b35f37 Mon Sep 17 00:00:00 2001 From: iqdecay Date: Mon, 28 Aug 2023 15:16:20 +0200 Subject: [PATCH 02/32] add test project --- .../contracts/nested_heap_type/Forc.toml | 7 +++ .../contracts/nested_heap_type/src/main.sw | 58 +++++++++++++++++++ 2 files changed, 65 insertions(+) create mode 100644 packages/fuels/tests/types/contracts/nested_heap_type/Forc.toml create mode 100644 packages/fuels/tests/types/contracts/nested_heap_type/src/main.sw diff --git a/packages/fuels/tests/types/contracts/nested_heap_type/Forc.toml b/packages/fuels/tests/types/contracts/nested_heap_type/Forc.toml new file mode 100644 index 0000000000..4e8efd5a40 --- /dev/null +++ b/packages/fuels/tests/types/contracts/nested_heap_type/Forc.toml @@ -0,0 +1,7 @@ +[project] +authors = ["Fuel Labs "] +entry = "main.sw" +license = "Apache-2.0" +name = "nested_heap_type" + +[dependencies] diff --git a/packages/fuels/tests/types/contracts/nested_heap_type/src/main.sw b/packages/fuels/tests/types/contracts/nested_heap_type/src/main.sw new file mode 100644 index 0000000000..f7a5ecc9ed --- /dev/null +++ b/packages/fuels/tests/types/contracts/nested_heap_type/src/main.sw @@ -0,0 +1,58 @@ +contract; + +use std::bytes::Bytes; + +pub enum TestError { + Something: [u8; 10], + Else: u64, +} + +pub enum Something { + a: (), + b: u64, +} + +abi MyContract { + fn returns_bytes_result(return_ok: bool) -> Result; + fn returns_vec_result(return_ok: bool) -> Result, TestError>; +} + +impl MyContract for Contract { + fn returns_bytes_result(return_ok: bool) -> Result { + return if return_ok { + let mut b = Bytes::new(); + b.push(1u8); + b.push(1u8); + b.push(1u8); + b.push(1u8); + Result::Ok(b) + } else { + Result::Err(TestError::Something([ + 255u8, + 255u8, + 255u8, + 255u8, + 255u8, + 255u8, + 255u8, + 255u8, + 255u8, + 255u8, + ])) + } + } + + fn returns_vec_result(return_ok: bool) -> Result, TestError> { + return if return_ok { + let mut v = Vec::new(); + v.push(2); + v.push(2); + v.push(2); + v.push(2); + v.push(2); + Result::Ok(v) + } else { + Result::Err(TestError::Else(7987)) + } + } +} From 871d3d68ad760c3348cb5130fe09b8ba4f69e880 Mon Sep 17 00:00:00 2001 From: iqdecay Date: Mon, 28 Aug 2023 20:44:14 +0200 Subject: [PATCH 03/32] wip --- packages/fuels-core/src/types/param_types.rs | 17 ++++------------ packages/fuels-programs/src/call_utils.rs | 12 +++++------ .../contracts/nested_heap_type/src/main.sw | 17 +++------------- packages/fuels/tests/types_contracts.rs | 20 ++++++++++--------- 4 files changed, 24 insertions(+), 42 deletions(-) diff --git a/packages/fuels-core/src/types/param_types.rs b/packages/fuels-core/src/types/param_types.rs index 8eedc2200c..23ed2f04d3 100644 --- a/packages/fuels-core/src/types/param_types.rs +++ b/packages/fuels-core/src/types/param_types.rs @@ -90,24 +90,15 @@ impl ParamType { ParamType::String => false, // We are currently allowing enums that are wrapping around heap types to allow for // `Result` to be returned from a contract - ParamType::Enum { .. } => self.enum_contains_nested_heap_types(), - _ => self.uses_heap_types(), - } - } - - pub fn enum_contains_nested_heap_types(&self) -> bool { - match &self { ParamType::Enum { variants, generics } => { let mut param_types = chain!(variants.param_types(), generics); param_types.any(|p| p.param_type_has_wrong_format()) } - // This case should never happen because it is called on a match statement - _ => false, + _ => self.uses_heap_types(), } } - // This is called inside the `enum_contains_nested_heap_types` on each type contained in the - // enum + // Called on each type contained in the enum fn param_type_has_wrong_format(&self) -> bool { match self { ParamType::Array(param_type, ..) => param_type.uses_heap_types(), @@ -1540,7 +1531,7 @@ mod tests { variants: EnumVariants::new(vec![ParamType::Bytes, enum_no_heap_types])?, generics: vec![], }; - println!("{}", r.enum_contains_nested_heap_types()); + assert_eq!(r.contains_nested_heap_types(), false); let enum_heap_types = ParamType::Enum { variants: EnumVariants::new(vec![ @@ -1553,7 +1544,7 @@ mod tests { variants: EnumVariants::new(vec![ParamType::Bytes, enum_heap_types])?, generics: vec![], }; - println!("{}", r.enum_contains_nested_heap_types()); + assert_eq!(r.contains_nested_heap_types(), true); Ok(()) } diff --git a/packages/fuels-programs/src/call_utils.rs b/packages/fuels-programs/src/call_utils.rs index b3f9ac3b93..7f20dbfe48 100644 --- a/packages/fuels-programs/src/call_utils.rs +++ b/packages/fuels-programs/src/call_utils.rs @@ -339,28 +339,28 @@ pub(crate) fn get_single_call_instructions( // (ptr, cap, len) are at the end of the data of the enum, which depends on the encoding // width. If the variant type does not contain a heap type, then the receipt generated will // not be read anyway. - let offset = if matches!(output_param_type, ParamType::Enum { .. }) { + let offset: u16 = if matches!(output_param_type, ParamType::Enum { .. }) { // 3 is not a "magic" number: skip 1 word for the discriminant, and the next 2 because // we are looking left of the 3 words (ptr, cap, len), that are placed at the end // of the data, on the right. - output_param_type.compute_encoding_width() - 3 + (output_param_type.compute_encoding_width() - 3) as u16 } else { 0 }; instructions.extend([ // The RET register contains the pointer address of the `CALL` return (a stack // address). - // The RETL register contains the length of the `CALL` return (=24 because the - // Vec/Bytes/String struct takes 3 WORDs). + // The RETL register contains the length of the `CALL` return (=24 in the case of + // Vec/Bytes/String because their struct takes 3 WORDs). // We don't actually need it unless the Vec/Bytes/String struct encoding changes in the // compiler. // Load the word located at the address contained in RET, it's a word that // translates to a heap address. 0x15 is a free register. - op::lw(0x15, RegId::RET, 0), + op::lw(0x15, RegId::RET, offset), // We know a Vec/Bytes/String struct has its third WORD contain the length of the // underlying vector, so use a 2 offset to store the length in 0x16, which is a free // register. - op::lw(0x16, RegId::RET, 2), + op::lw(0x16, RegId::RET, offset + 2), // The in-memory size of the type is (in-memory size of the inner type) * length op::muli(0x16, 0x16, inner_type_byte_size as u16), op::retd(0x15, 0x16), diff --git a/packages/fuels/tests/types/contracts/nested_heap_type/src/main.sw b/packages/fuels/tests/types/contracts/nested_heap_type/src/main.sw index f7a5ecc9ed..cca1dfd0a1 100644 --- a/packages/fuels/tests/types/contracts/nested_heap_type/src/main.sw +++ b/packages/fuels/tests/types/contracts/nested_heap_type/src/main.sw @@ -3,7 +3,7 @@ contract; use std::bytes::Bytes; pub enum TestError { - Something: [u8; 10], + Something: [u8; 5], Else: u64, } @@ -27,18 +27,7 @@ impl MyContract for Contract { b.push(1u8); Result::Ok(b) } else { - Result::Err(TestError::Something([ - 255u8, - 255u8, - 255u8, - 255u8, - 255u8, - 255u8, - 255u8, - 255u8, - 255u8, - 255u8, - ])) + Result::Err(TestError::Something([255u8, 255u8, 255u8, 255u8, 255u8])) } } @@ -52,7 +41,7 @@ impl MyContract for Contract { v.push(2); Result::Ok(v) } else { - Result::Err(TestError::Else(7987)) + Result::Err(TestError::Else(7777)) } } } diff --git a/packages/fuels/tests/types_contracts.rs b/packages/fuels/tests/types_contracts.rs index 2299b3b9c5..5c079d1b4e 100644 --- a/packages/fuels/tests/types_contracts.rs +++ b/packages/fuels/tests/types_contracts.rs @@ -2026,10 +2026,11 @@ fn treat_receipts(receipts: Vec) { } _ => (), } + println!("========================================") } #[tokio::test] -async fn test_1_nested() -> Result<()> { +async fn test_result_bytes_vec() -> Result<()> { let wallet = launch_provider_and_get_wallet().await; setup_program_test!( Abigen(Contract( @@ -2045,16 +2046,17 @@ async fn test_1_nested() -> Result<()> { let contract_methods = contract_instance.methods(); let resp = contract_methods.returns_bytes_result(true).call().await?; - // treat_receipts(resp.receipts); - println!("{:?}", resp.value); + let expected = Ok(Bytes(vec![1, 1, 1, 1])); + assert_eq!(resp.value, expected); let resp = contract_methods.returns_bytes_result(false).call().await?; - // treat_receipts(resp.receipts); - println!("{:?}", resp.value); + let expected = Err(TestError::Something([255u8, 255u8, 255u8, 255u8, 255u8])); + assert_eq!(resp.value, expected); + let resp = contract_methods.returns_vec_result(true).call().await?; - // treat_receipts(resp.receipts); - println!("{:?}", resp.value); + let expected = Ok(vec![2, 2, 2, 2, 2]); + assert_eq!(resp.value, expected); let resp = contract_methods.returns_vec_result(false).call().await?; - // treat_receipts(resp.receipts); - println!("{:?}", resp.value); + let expected = Err(TestError::Else(7777)); + assert_eq!(resp.value, expected); Ok(()) } From a6df54d18860be15d58609118893c03292376ad3 Mon Sep 17 00:00:00 2001 From: iqdecay Date: Tue, 29 Aug 2023 16:25:50 +0200 Subject: [PATCH 04/32] feat: cut the supplementary ReturnData receipt to 0 --- .../fuels-core/src/types/enum_variants.rs | 24 +++++- packages/fuels-programs/src/call_utils.rs | 73 +++++++++++-------- 2 files changed, 65 insertions(+), 32 deletions(-) diff --git a/packages/fuels-core/src/types/enum_variants.rs b/packages/fuels-core/src/types/enum_variants.rs index ef532bd876..652f938d5f 100644 --- a/packages/fuels-core/src/types/enum_variants.rs +++ b/packages/fuels-core/src/types/enum_variants.rs @@ -13,10 +13,16 @@ pub struct EnumVariants { impl EnumVariants { pub fn new(param_types: Vec) -> Result { - if !param_types.is_empty() { - Ok(EnumVariants { param_types }) + if param_types.is_empty() { + return Err(error!(InvalidData, "Enum variants can not be empty!")); + } + if param_types.iter().filter(|p| p.is_vm_heap_type()).count() > 1 { + Err(error!( + InvalidData, + "Enum variants can only contain one heap type" + )) } else { - Err(error!(InvalidData, "Enum variants can not be empty!")) + Ok(EnumVariants { param_types }) } } @@ -34,6 +40,18 @@ impl EnumVariants { }) } + pub fn get_heap_type_variant_discriminant(&self) -> Result { + for (discriminant, param) in self.param_types.iter().enumerate() { + if param.is_vm_heap_type() { + return Ok(discriminant as u8); + } + } + Err(error!( + InvalidData, + "There are no heap types inside {:?}", self + )) + } + pub fn only_units_inside(&self) -> bool { self.param_types .iter() diff --git a/packages/fuels-programs/src/call_utils.rs b/packages/fuels-programs/src/call_utils.rs index 7f20dbfe48..c7b983b69d 100644 --- a/packages/fuels-programs/src/call_utils.rs +++ b/packages/fuels-programs/src/call_utils.rs @@ -334,37 +334,52 @@ pub(crate) fn get_single_call_instructions( .to_vec(); // The instructions are different if you want to return data that was on the heap if let Some(inner_type_byte_size) = output_param_type.heap_inner_element_size() { - // This offset is here because if the variant type is an enum containing a heap type, then - // the first word must be skipped because it's the discriminant. The 3 words containing - // (ptr, cap, len) are at the end of the data of the enum, which depends on the encoding - // width. If the variant type does not contain a heap type, then the receipt generated will - // not be read anyway. - let offset: u16 = if matches!(output_param_type, ParamType::Enum { .. }) { - // 3 is not a "magic" number: skip 1 word for the discriminant, and the next 2 because - // we are looking left of the 3 words (ptr, cap, len), that are placed at the end - // of the data, on the right. - (output_param_type.compute_encoding_width() - 3) as u16 + if let ParamType::Enum { variants, .. } = output_param_type { + // This offset is here because if the variant type is an enum containing a heap type, then + // the first word must be skipped because it's the discriminant. The 3 words containing + // (ptr, cap, len) are at the end of the data of the enum, which depends on the encoding + // width. If the variant type does not contain a heap type, then the receipt generated will + // not be read anyway, so we can generate a receipt with empty return data. + // 3 is not a "magic" number: we are looking left of the 3 words (ptr, cap, len), + // that are placed at the end of the data, on the right. + let offset = (output_param_type.compute_encoding_width() - 3) as u16; + let selected_discriminant = + variants.get_heap_type_variant_discriminant().unwrap() as u16; + instructions.extend([ + // All the registers 0x15-0x18 are free + // The RET register contains the pointer address of the `CALL` return (a stack + // address). + // The RETL register contains the length of the `CALL` return (=24 in the case of + // Vec/Bytes/String because their struct takes 3 WORDs). We don't actually need it + // unless the Vec/Bytes/String struct encoding changes in the compiler. + // Load the word located at the address contained in RET, it's a word that translates to + // a heap address. We use the offset because in the case of an enum containing a heap + // type, we need to get the pointer that is located at the heap address pointed to by + // the RET register. + op::lw(0x15, RegId::RET, offset), + // We know a Vec/Bytes/String struct has its third WORD contain the length of the + // underlying vector, so use a 2 offset to store the length. + op::lw(0x16, RegId::RET, offset + 2), + // The in-memory size of the type is (in-memory size of the inner type) * length + op::muli(0x16, 0x16, inner_type_byte_size as u16), + // Load the selected discriminant to a free register + op::muli(0x17, RegId::ONE, selected_discriminant), + // the first word of the CALL return is the enum discriminant + op::lw(0x18, RegId::RET, 0), + // If the discriminant is not the one from the heap type, then jump ahead and + op::jnef(0x17, 0x18, RegId::ZERO, 1), + op::retd(0x15, 0x16), + op::retd(0x15, RegId::ZERO), + ]); } else { - 0 + // See above for explainations, no offset needed if the type is not an enum + instructions.extend([ + op::lw(0x15, RegId::RET, 0), + op::lw(0x16, RegId::RET, 2), + op::muli(0x16, 0x16, inner_type_byte_size as u16), + op::retd(0x15, 0x16), + ]); }; - instructions.extend([ - // The RET register contains the pointer address of the `CALL` return (a stack - // address). - // The RETL register contains the length of the `CALL` return (=24 in the case of - // Vec/Bytes/String because their struct takes 3 WORDs). - // We don't actually need it unless the Vec/Bytes/String struct encoding changes in the - // compiler. - // Load the word located at the address contained in RET, it's a word that - // translates to a heap address. 0x15 is a free register. - op::lw(0x15, RegId::RET, offset), - // We know a Vec/Bytes/String struct has its third WORD contain the length of the - // underlying vector, so use a 2 offset to store the length in 0x16, which is a free - // register. - op::lw(0x16, RegId::RET, offset + 2), - // The in-memory size of the type is (in-memory size of the inner type) * length - op::muli(0x16, 0x16, inner_type_byte_size as u16), - op::retd(0x15, 0x16), - ]); } #[allow(clippy::iter_cloned_collect)] From eeef19c5908642ac7596f838111c034e5e2d509c Mon Sep 17 00:00:00 2001 From: iqdecay Date: Tue, 29 Aug 2023 17:20:40 +0200 Subject: [PATCH 05/32] fix call instructions length --- packages/fuels-programs/src/call_utils.rs | 29 ++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/packages/fuels-programs/src/call_utils.rs b/packages/fuels-programs/src/call_utils.rs index c7b983b69d..6847013a5f 100644 --- a/packages/fuels-programs/src/call_utils.rs +++ b/packages/fuels-programs/src/call_utils.rs @@ -10,6 +10,7 @@ use fuels_core::{ offsets::call_script_data_offset, types::{ bech32::{Bech32Address, Bech32ContractId}, + enum_variants::EnumVariants, errors::{Error as FuelsError, Result}, input::Input, param_types::ParamType, @@ -143,11 +144,31 @@ pub(crate) async fn build_tx_from_contract_calls( /// Compute the length of the calling scripts for the two types of contract calls: those that return /// a heap type, and those that don't. fn compute_calls_instructions_len(calls: &[ContractCall]) -> usize { + let n_enum_heap_type_calls = calls + .iter() + .filter(|c| { + c.output_param.heap_inner_element_size().is_some() + && matches!(c.output_param, ParamType::Enum { .. }) + }) + .count(); + let n_heap_type_calls = calls .iter() .filter(|c| c.output_param.heap_inner_element_size().is_some()) - .count(); - let n_stack_type_calls = calls.len() - n_heap_type_calls; + .count() + - n_enum_heap_type_calls; + + let n_stack_type_calls = calls.len() - n_heap_type_calls - n_enum_heap_type_calls; + + let total_instructions_len_heap_enum_data = get_single_call_instructions( + &CallOpcodeParamsOffset::default(), + &ParamType::Enum { + variants: EnumVariants::new(vec![ParamType::Bytes]).unwrap(), + generics: vec![], + }, + ) + .len() + * n_enum_heap_type_calls; let total_instructions_len_stack_data = // Use placeholder for `call_param_offsets` and `output_param_type`, because the length of @@ -163,7 +184,9 @@ fn compute_calls_instructions_len(calls: &[ContractCall]) -> usize { .len() * n_heap_type_calls; - total_instructions_len_stack_data + total_instructions_len_heap_data + total_instructions_len_stack_data + + total_instructions_len_heap_data + + total_instructions_len_heap_enum_data } /// Compute how much of each asset is required based on all `CallParameters` of the `ContractCalls` From f2ced6dfe7027d5b624f911c697b1d250e7950dd Mon Sep 17 00:00:00 2001 From: iqdecay Date: Tue, 29 Aug 2023 17:25:13 +0200 Subject: [PATCH 06/32] add test --- packages/fuels/Forc.toml | 1 + .../Forc.toml | 2 +- .../src/main.sw | 16 +++++++++++----- packages/fuels/tests/types_contracts.rs | 16 ++++++++++++---- 4 files changed, 25 insertions(+), 10 deletions(-) rename packages/fuels/tests/types/contracts/{nested_heap_type => heap_type_in_enums}/Forc.toml (79%) rename packages/fuels/tests/types/contracts/{nested_heap_type => heap_type_in_enums}/src/main.sw (73%) diff --git a/packages/fuels/Forc.toml b/packages/fuels/Forc.toml index 2c6a9bcfcc..9070d0c67e 100644 --- a/packages/fuels/Forc.toml +++ b/packages/fuels/Forc.toml @@ -55,6 +55,7 @@ members = [ 'tests/types/contracts/enum_inside_struct', 'tests/types/contracts/evm_address', 'tests/types/contracts/generics', + 'tests/types/contracts/heap_type_in_enums', 'tests/types/contracts/identity', 'tests/types/contracts/native_types', 'tests/types/contracts/nested_structs', diff --git a/packages/fuels/tests/types/contracts/nested_heap_type/Forc.toml b/packages/fuels/tests/types/contracts/heap_type_in_enums/Forc.toml similarity index 79% rename from packages/fuels/tests/types/contracts/nested_heap_type/Forc.toml rename to packages/fuels/tests/types/contracts/heap_type_in_enums/Forc.toml index 4e8efd5a40..cb9028fd51 100644 --- a/packages/fuels/tests/types/contracts/nested_heap_type/Forc.toml +++ b/packages/fuels/tests/types/contracts/heap_type_in_enums/Forc.toml @@ -2,6 +2,6 @@ authors = ["Fuel Labs "] entry = "main.sw" license = "Apache-2.0" -name = "nested_heap_type" +name = "heap_type_in_enums" [dependencies] diff --git a/packages/fuels/tests/types/contracts/nested_heap_type/src/main.sw b/packages/fuels/tests/types/contracts/heap_type_in_enums/src/main.sw similarity index 73% rename from packages/fuels/tests/types/contracts/nested_heap_type/src/main.sw rename to packages/fuels/tests/types/contracts/heap_type_in_enums/src/main.sw index cca1dfd0a1..bb0d527f57 100644 --- a/packages/fuels/tests/types/contracts/nested_heap_type/src/main.sw +++ b/packages/fuels/tests/types/contracts/heap_type_in_enums/src/main.sw @@ -1,20 +1,17 @@ contract; use std::bytes::Bytes; +use std::string::String; pub enum TestError { Something: [u8; 5], Else: u64, } -pub enum Something { - a: (), - b: u64, -} - abi MyContract { fn returns_bytes_result(return_ok: bool) -> Result; fn returns_vec_result(return_ok: bool) -> Result, TestError>; + fn returns_string_result(return_ok: bool) -> Result; } impl MyContract for Contract { @@ -44,4 +41,13 @@ impl MyContract for Contract { Result::Err(TestError::Else(7777)) } } + + fn returns_string_result(return_ok: bool) -> Result { + return if return_ok { + let s = String::from_ascii_str("Hello World"); + Result::Ok(s) + } else { + Result::Err(TestError::Else(3333)) + } + } } diff --git a/packages/fuels/tests/types_contracts.rs b/packages/fuels/tests/types_contracts.rs index 5c079d1b4e..b8ffeaec03 100644 --- a/packages/fuels/tests/types_contracts.rs +++ b/packages/fuels/tests/types_contracts.rs @@ -2030,16 +2030,16 @@ fn treat_receipts(receipts: Vec) { } #[tokio::test] -async fn test_result_bytes_vec() -> Result<()> { +async fn test_heap_type_in_result() -> Result<()> { let wallet = launch_provider_and_get_wallet().await; setup_program_test!( Abigen(Contract( - name = "NestedHeap", - project = "packages/fuels/tests/types/contracts/nested_heap_type" + name = "HeapTypeInEnum", + project = "packages/fuels/tests/types/contracts/heap_type_in_enums" )), Deploy( name = "contract_instance", - contract = "NestedHeap", + contract = "HeapTypeInEnum", wallet = "wallet" ), ); @@ -2058,5 +2058,13 @@ async fn test_result_bytes_vec() -> Result<()> { let resp = contract_methods.returns_vec_result(false).call().await?; let expected = Err(TestError::Else(7777)); assert_eq!(resp.value, expected); + + let resp = contract_methods.returns_string_result(true).call().await?; + let expected = Ok("Hello World".to_string()); + assert_eq!(resp.value, expected); + let resp = contract_methods.returns_string_result(false).call().await?; + let expected = Err(TestError::Else(3333)); + assert_eq!(resp.value, expected); + Ok(()) } From 575da0ddf4b11404ac623be08901afc0411ebae5 Mon Sep 17 00:00:00 2001 From: iqdecay Date: Tue, 29 Aug 2023 17:45:02 +0200 Subject: [PATCH 07/32] add test for options --- .../contracts/heap_type_in_enums/src/main.sw | 39 +++++++++++++++++++ packages/fuels/tests/types_contracts.rs | 19 ++++++++- 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/packages/fuels/tests/types/contracts/heap_type_in_enums/src/main.sw b/packages/fuels/tests/types/contracts/heap_type_in_enums/src/main.sw index bb0d527f57..0a11035b20 100644 --- a/packages/fuels/tests/types/contracts/heap_type_in_enums/src/main.sw +++ b/packages/fuels/tests/types/contracts/heap_type_in_enums/src/main.sw @@ -12,6 +12,9 @@ abi MyContract { fn returns_bytes_result(return_ok: bool) -> Result; fn returns_vec_result(return_ok: bool) -> Result, TestError>; fn returns_string_result(return_ok: bool) -> Result; + fn returns_bytes_option(return_some: bool) -> Option; + fn returns_vec_option(return_some: bool) -> Option>; + fn returns_string_option(return_some: bool) -> Option; } impl MyContract for Contract { @@ -50,4 +53,40 @@ impl MyContract for Contract { Result::Err(TestError::Else(3333)) } } + + fn returns_bytes_option(return_some: bool) -> Option { + return if return_some { + let mut b = Bytes::new(); + b.push(1u8); + b.push(1u8); + b.push(1u8); + b.push(1u8); + Option::Some(b) + } else { + Option::None + } + } + + fn returns_vec_option(return_some: bool) -> Option> { + return if return_some { + let mut v = Vec::new(); + v.push(2); + v.push(2); + v.push(2); + v.push(2); + v.push(2); + Option::Some(v) + } else { + None + } + } + + fn returns_string_option(return_some: bool) -> Option { + return if return_some { + let s = String::from_ascii_str("Hello World"); + Option::Some(s) + } else { + None + } + } } diff --git a/packages/fuels/tests/types_contracts.rs b/packages/fuels/tests/types_contracts.rs index b8ffeaec03..e91d7ac56e 100644 --- a/packages/fuels/tests/types_contracts.rs +++ b/packages/fuels/tests/types_contracts.rs @@ -2030,7 +2030,7 @@ fn treat_receipts(receipts: Vec) { } #[tokio::test] -async fn test_heap_type_in_result() -> Result<()> { +async fn test_heap_type_in_enums() -> Result<()> { let wallet = launch_provider_and_get_wallet().await; setup_program_test!( Abigen(Contract( @@ -2066,5 +2066,22 @@ async fn test_heap_type_in_result() -> Result<()> { let expected = Err(TestError::Else(3333)); assert_eq!(resp.value, expected); + let resp = contract_methods.returns_bytes_option(true).call().await?; + let expected = Some(Bytes(vec![1, 1, 1, 1])); + assert_eq!(resp.value, expected); + let resp = contract_methods.returns_bytes_option(false).call().await?; + assert!(resp.value.is_none()); + + let resp = contract_methods.returns_vec_option(true).call().await?; + let expected = Some(vec![2, 2, 2, 2, 2]); + assert_eq!(resp.value, expected); + let resp = contract_methods.returns_vec_option(false).call().await?; + assert!(resp.value.is_none()); + + let resp = contract_methods.returns_string_option(true).call().await?; + let expected = Some("Hello World".to_string()); + assert_eq!(resp.value, expected); + let resp = contract_methods.returns_string_option(false).call().await?; + assert!(resp.value.is_none()); Ok(()) } From cc81c0e09a9842fb14703340261d23afea5c1463 Mon Sep 17 00:00:00 2001 From: iqdecay Date: Wed, 30 Aug 2023 15:44:36 +0200 Subject: [PATCH 08/32] test: add enum variants lib test --- .../fuels-core/src/types/enum_variants.rs | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/packages/fuels-core/src/types/enum_variants.rs b/packages/fuels-core/src/types/enum_variants.rs index 652f938d5f..93cfc08de0 100644 --- a/packages/fuels-core/src/types/enum_variants.rs +++ b/packages/fuels-core/src/types/enum_variants.rs @@ -82,3 +82,26 @@ impl EnumVariants { (biggest_variant_width - variant_width) * WORD_SIZE } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_enum_variants_can_have_only_one_heap_type() -> Result<()> { + let mut param_types = vec![ + ParamType::U64, + ParamType::Bool, + ParamType::Vector(Box::from(ParamType::U64)), + ]; + // it works if there is only one heap type + let _variants = EnumVariants::new(param_types.clone())?; + param_types.append(&mut vec![ParamType::Bytes]); + + let error = EnumVariants::new(param_types).expect_err("Should have failed"); + let expected_error = format!("Invalid data: Enum variants can only contain one heap type"); + assert_eq!(error.to_string(), expected_error); + + Ok(()) + } +} From 1d7a61a60d2589b245799813050569f689b3cf2a Mon Sep 17 00:00:00 2001 From: iqdecay Date: Wed, 30 Aug 2023 16:53:51 +0200 Subject: [PATCH 09/32] add docs --- packages/fuels-core/src/types/param_types.rs | 21 ++++++++++---------- packages/fuels-programs/src/call_utils.rs | 16 +++++++-------- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/packages/fuels-core/src/types/param_types.rs b/packages/fuels-core/src/types/param_types.rs index 23ed2f04d3..d587efa07a 100644 --- a/packages/fuels-core/src/types/param_types.rs +++ b/packages/fuels-core/src/types/param_types.rs @@ -92,15 +92,16 @@ impl ParamType { // `Result` to be returned from a contract ParamType::Enum { variants, generics } => { let mut param_types = chain!(variants.param_types(), generics); - param_types.any(|p| p.param_type_has_wrong_format()) + param_types.any(|p| p.enum_param_types_use_heap_types()) } _ => self.uses_heap_types(), } } // Called on each type contained in the enum - fn param_type_has_wrong_format(&self) -> bool { + fn enum_param_types_use_heap_types(&self) -> bool { match self { + ParamType::Vector(..) | ParamType::Bytes | ParamType::String => false, ParamType::Array(param_type, ..) => param_type.uses_heap_types(), ParamType::Tuple(param_types, ..) => Self::any_nested_heap_types(param_types), ParamType::Enum { @@ -157,6 +158,7 @@ impl ParamType { ParamType::Bytes | ParamType::String => Some(std::mem::size_of::()), ParamType::Enum { variants, generics } => { let variants_types = variants.param_types(); + // There is at most one heap type per enum for param in chain!(generics, variants_types) { if param.is_vm_heap_type() { return Some(param.heap_inner_element_size().unwrap()); @@ -1429,23 +1431,20 @@ mod tests { }; is_nested(nested_struct); - let not_nested_enum = ParamType::Enum { + not_nested(ParamType::Enum { variants: EnumVariants::new(param_types_no_nested_bytes.clone())?, generics: param_types_no_nested_bytes.clone(), - }; - not_nested(not_nested_enum); + }); - let nested_enum = ParamType::Enum { + not_nested(ParamType::Enum { variants: EnumVariants::new(param_types_nested_bytes.clone())?, generics: param_types_no_nested_bytes.clone(), - }; - not_nested(nested_enum); + }); - let nested_enum = ParamType::Enum { + not_nested(ParamType::Enum { variants: EnumVariants::new(param_types_no_nested_bytes)?, generics: param_types_nested_bytes, - }; - not_nested(nested_enum); + }); Ok(()) } diff --git a/packages/fuels-programs/src/call_utils.rs b/packages/fuels-programs/src/call_utils.rs index 6847013a5f..203d497e94 100644 --- a/packages/fuels-programs/src/call_utils.rs +++ b/packages/fuels-programs/src/call_utils.rs @@ -358,13 +358,12 @@ pub(crate) fn get_single_call_instructions( // The instructions are different if you want to return data that was on the heap if let Some(inner_type_byte_size) = output_param_type.heap_inner_element_size() { if let ParamType::Enum { variants, .. } = output_param_type { - // This offset is here because if the variant type is an enum containing a heap type, then - // the first word must be skipped because it's the discriminant. The 3 words containing - // (ptr, cap, len) are at the end of the data of the enum, which depends on the encoding - // width. If the variant type does not contain a heap type, then the receipt generated will - // not be read anyway, so we can generate a receipt with empty return data. - // 3 is not a "magic" number: we are looking left of the 3 words (ptr, cap, len), - // that are placed at the end of the data, on the right. + // This offset is here because if the variant type is an enum containing a heap type, + // the 3 words containing (ptr, cap, len) are at the end of the data of the enum, which + // depends on the encoding width. If the variant type does not contain a heap type, then + // the receipt generated will not be read anyway, so we can generate a receipt with + // empty return data. 3 is not a "magic" number: we are looking left of the 3 words + // (ptr, cap, len), that are placed at the end of the data, on the right. let offset = (output_param_type.compute_encoding_width() - 3) as u16; let selected_discriminant = variants.get_heap_type_variant_discriminant().unwrap() as u16; @@ -390,12 +389,13 @@ pub(crate) fn get_single_call_instructions( // the first word of the CALL return is the enum discriminant op::lw(0x18, RegId::RET, 0), // If the discriminant is not the one from the heap type, then jump ahead and + // return an empty receipt. Otherwise return heap data with the right length. op::jnef(0x17, 0x18, RegId::ZERO, 1), op::retd(0x15, 0x16), op::retd(0x15, RegId::ZERO), ]); } else { - // See above for explainations, no offset needed if the type is not an enum + // See above for explanations, no offset needed if the type is not an enum instructions.extend([ op::lw(0x15, RegId::RET, 0), op::lw(0x16, RegId::RET, 2), From 003cf41fd0d640b97965484861bb02656f600c2a Mon Sep 17 00:00:00 2001 From: iqdecay Date: Wed, 30 Aug 2023 17:01:53 +0200 Subject: [PATCH 10/32] style: appease clippy and all formatting gods --- .../fuels-core/src/types/enum_variants.rs | 3 ++- packages/fuels-core/src/types/param_types.rs | 4 +-- packages/fuels/tests/types_contracts.rs | 26 ------------------- 3 files changed, 4 insertions(+), 29 deletions(-) diff --git a/packages/fuels-core/src/types/enum_variants.rs b/packages/fuels-core/src/types/enum_variants.rs index 93cfc08de0..c4dafb5720 100644 --- a/packages/fuels-core/src/types/enum_variants.rs +++ b/packages/fuels-core/src/types/enum_variants.rs @@ -99,7 +99,8 @@ mod tests { param_types.append(&mut vec![ParamType::Bytes]); let error = EnumVariants::new(param_types).expect_err("Should have failed"); - let expected_error = format!("Invalid data: Enum variants can only contain one heap type"); + let expected_error = + "Invalid data: Enum variants can only contain one heap type".to_string(); assert_eq!(error.to_string(), expected_error); Ok(()) diff --git a/packages/fuels-core/src/types/param_types.rs b/packages/fuels-core/src/types/param_types.rs index d587efa07a..63ce116405 100644 --- a/packages/fuels-core/src/types/param_types.rs +++ b/packages/fuels-core/src/types/param_types.rs @@ -1530,7 +1530,7 @@ mod tests { variants: EnumVariants::new(vec![ParamType::Bytes, enum_no_heap_types])?, generics: vec![], }; - assert_eq!(r.contains_nested_heap_types(), false); + assert!(!r.contains_nested_heap_types()); let enum_heap_types = ParamType::Enum { variants: EnumVariants::new(vec![ @@ -1543,7 +1543,7 @@ mod tests { variants: EnumVariants::new(vec![ParamType::Bytes, enum_heap_types])?, generics: vec![], }; - assert_eq!(r.contains_nested_heap_types(), true); + assert!(r.contains_nested_heap_types()); Ok(()) } diff --git a/packages/fuels/tests/types_contracts.rs b/packages/fuels/tests/types_contracts.rs index e91d7ac56e..357c0c8b6d 100644 --- a/packages/fuels/tests/types_contracts.rs +++ b/packages/fuels/tests/types_contracts.rs @@ -2003,32 +2003,6 @@ async fn test_contract_std_lib_string() -> Result<()> { Ok(()) } -fn treat_receipts(receipts: Vec) { - let filtered: Vec = receipts - .into_iter() - .clone() - .filter(|r| matches!(r, Receipt::ReturnData { .. })) - .collect(); - - match filtered.get(0).unwrap() { - Receipt::ReturnData { data, .. } => { - let decoded_bytes = - ABIDecoder::decode(&[ParamType::Bytes], &data.clone().unwrap()).unwrap(); - println!("{:?}", decoded_bytes); - } - _ => (), - } - match filtered.get(1).unwrap() { - Receipt::ReturnData { data, .. } => { - let decoded_bytes = - ABIDecoder::decode(&[ParamType::Bytes], &data.clone().unwrap()).unwrap(); - println!("{:?}", decoded_bytes); - } - _ => (), - } - println!("========================================") -} - #[tokio::test] async fn test_heap_type_in_enums() -> Result<()> { let wallet = launch_provider_and_get_wallet().await; From dc5d5dd1f44b40f4c1cfc9e598a96e4db87e5787 Mon Sep 17 00:00:00 2001 From: iqdecay Date: Thu, 31 Aug 2023 01:10:26 +0200 Subject: [PATCH 11/32] dedupe uses_heap_type function --- packages/fuels-core/src/codec/abi_decoder.rs | 2 +- packages/fuels-core/src/types/param_types.rs | 35 +++++-------------- packages/fuels-programs/src/receipt_parser.rs | 2 +- 3 files changed, 11 insertions(+), 28 deletions(-) diff --git a/packages/fuels-core/src/codec/abi_decoder.rs b/packages/fuels-core/src/codec/abi_decoder.rs index e33bc80e0a..73e6873468 100644 --- a/packages/fuels-core/src/codec/abi_decoder.rs +++ b/packages/fuels-core/src/codec/abi_decoder.rs @@ -277,7 +277,7 @@ impl ABIDecoder { let discriminant = peek_u32(&data)? as u8; let selected_variant = variants.param_type_of_variant(discriminant)?; - if selected_variant.uses_heap_types() { + if selected_variant.uses_heap_types(false) { // remove the 3 WORDS that represent (ptr, cap, len) // those 3 WORDS are leftovers from the concatenation of the data from the two // `ReturnData` receipts. We need to remove them only if the selected variant is a diff --git a/packages/fuels-core/src/types/param_types.rs b/packages/fuels-core/src/types/param_types.rs index 63ce116405..ed5464544e 100644 --- a/packages/fuels-core/src/types/param_types.rs +++ b/packages/fuels-core/src/types/param_types.rs @@ -82,7 +82,7 @@ impl ParamType { pub fn contains_nested_heap_types(&self) -> bool { match &self { - ParamType::Vector(param_type) => param_type.uses_heap_types(), + ParamType::Vector(param_type) => param_type.uses_heap_types(false), ParamType::Bytes => false, // Here, we return false because even though the `Token::String` type has an underlying // `Bytes` type nested, it is an exception that will be generalized as part of @@ -92,35 +92,18 @@ impl ParamType { // `Result` to be returned from a contract ParamType::Enum { variants, generics } => { let mut param_types = chain!(variants.param_types(), generics); - param_types.any(|p| p.enum_param_types_use_heap_types()) + param_types.any(|p| p.uses_heap_types(true)) } - _ => self.uses_heap_types(), + _ => self.uses_heap_types(false), } } - // Called on each type contained in the enum - fn enum_param_types_use_heap_types(&self) -> bool { - match self { - ParamType::Vector(..) | ParamType::Bytes | ParamType::String => false, - ParamType::Array(param_type, ..) => param_type.uses_heap_types(), - ParamType::Tuple(param_types, ..) => Self::any_nested_heap_types(param_types), - ParamType::Enum { - generics, variants, .. - } => { - let variants_types = variants.param_types(); - Self::any_nested_heap_types(chain!(generics, variants_types)) - } - ParamType::Struct { - fields, generics, .. - } => Self::any_nested_heap_types(chain!(fields, generics)), - _ => false, - } - } - - pub fn uses_heap_types(&self) -> bool { + pub fn uses_heap_types(&self, can_be_heap_type_itself: bool) -> bool { match &self { - ParamType::Vector(..) | ParamType::Bytes | ParamType::String => true, - ParamType::Array(param_type, ..) => param_type.uses_heap_types(), + ParamType::Vector(..) | ParamType::Bytes | ParamType::String => { + !can_be_heap_type_itself + } + ParamType::Array(param_type, ..) => param_type.uses_heap_types(false), ParamType::Tuple(param_types, ..) => Self::any_nested_heap_types(param_types), ParamType::Enum { generics, variants, .. @@ -138,7 +121,7 @@ impl ParamType { fn any_nested_heap_types<'a>(param_types: impl IntoIterator) -> bool { param_types .into_iter() - .any(|param_type| param_type.uses_heap_types()) + .any(|param_type| param_type.uses_heap_types(false)) } pub fn is_vm_heap_type(&self) -> bool { diff --git a/packages/fuels-programs/src/receipt_parser.rs b/packages/fuels-programs/src/receipt_parser.rs index 2430739557..e55d5e05c0 100644 --- a/packages/fuels-programs/src/receipt_parser.rs +++ b/packages/fuels-programs/src/receipt_parser.rs @@ -62,7 +62,7 @@ impl ReceiptParser { ) -> Option> { match output_param.get_return_location() { ReturnLocation::ReturnData - if output_param.uses_heap_types() + if output_param.uses_heap_types(false) && matches!(output_param, ParamType::Enum { .. }) => { self.extract_enum_heap_type_data(contract_id) From cc8c5db05fb0726e1c0ed36cced5c1e56715b7f7 Mon Sep 17 00:00:00 2001 From: iqdecay Date: Thu, 31 Aug 2023 01:21:26 +0200 Subject: [PATCH 12/32] add test for the discriminant --- .../fuels-core/src/types/enum_variants.rs | 33 ++++++++++++++++++- packages/fuels-programs/src/call_utils.rs | 2 +- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/packages/fuels-core/src/types/enum_variants.rs b/packages/fuels-core/src/types/enum_variants.rs index c4dafb5720..a33f14fc45 100644 --- a/packages/fuels-core/src/types/enum_variants.rs +++ b/packages/fuels-core/src/types/enum_variants.rs @@ -40,7 +40,7 @@ impl EnumVariants { }) } - pub fn get_heap_type_variant_discriminant(&self) -> Result { + pub fn heap_type_variant_discriminant(&self) -> Result { for (discriminant, param) in self.param_types.iter().enumerate() { if param.is_vm_heap_type() { return Ok(discriminant as u8); @@ -105,4 +105,35 @@ mod tests { Ok(()) } + + #[test] + fn test_get_heap_type_variant_discriminant() -> Result<()> { + let param_types = vec![ + ParamType::U64, + ParamType::Bool, + ParamType::Vector(Box::from(ParamType::U64)), + ]; + let variants = EnumVariants::new(param_types)?; + assert_eq!(variants.heap_type_variant_discriminant()?, 2); + + let param_types = vec![ + ParamType::Vector(Box::from(ParamType::U64)), + ParamType::U64, + ParamType::Bool, + ]; + let variants = EnumVariants::new(param_types)?; + assert_eq!(variants.heap_type_variant_discriminant()?, 0); + + let param_types = vec![ParamType::U64, ParamType::Bool]; + let variants = EnumVariants::new(param_types)?; + let error = variants + .heap_type_variant_discriminant() + .expect_err("Should have failed"); + let expected_error = format!( + "Invalid data: There are no heap types inside {:?}", + variants + ); + assert_eq!(error.to_string(), expected_error); + Ok(()) + } } diff --git a/packages/fuels-programs/src/call_utils.rs b/packages/fuels-programs/src/call_utils.rs index 203d497e94..1285392a4f 100644 --- a/packages/fuels-programs/src/call_utils.rs +++ b/packages/fuels-programs/src/call_utils.rs @@ -366,7 +366,7 @@ pub(crate) fn get_single_call_instructions( // (ptr, cap, len), that are placed at the end of the data, on the right. let offset = (output_param_type.compute_encoding_width() - 3) as u16; let selected_discriminant = - variants.get_heap_type_variant_discriminant().unwrap() as u16; + variants.heap_type_variant_discriminant().unwrap() as u16; instructions.extend([ // All the registers 0x15-0x18 are free // The RET register contains the pointer address of the `CALL` return (a stack From c13b581785b695081dcdf1168cd59323e494751f Mon Sep 17 00:00:00 2001 From: iqdecay Date: Thu, 31 Aug 2023 01:24:05 +0200 Subject: [PATCH 13/32] fix naming --- packages/fuels-core/src/types/param_types.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/fuels-core/src/types/param_types.rs b/packages/fuels-core/src/types/param_types.rs index ed5464544e..59ba7b9235 100644 --- a/packages/fuels-core/src/types/param_types.rs +++ b/packages/fuels-core/src/types/param_types.rs @@ -1503,8 +1503,7 @@ mod tests { } #[test] - fn my_function() -> Result<()> { - // Result when `Something` does not contain heap type should work! + fn test_double_nested_heap_type_enum() -> Result<()> { let enum_no_heap_types = ParamType::Enum { variants: EnumVariants::new(vec![ParamType::U64, ParamType::Bool])?, generics: vec![], From 8a3ed33343dea56c0e91112731140c309f2cb90d Mon Sep 17 00:00:00 2001 From: iqdecay Date: Thu, 31 Aug 2023 15:52:59 +0200 Subject: [PATCH 14/32] style: appease clippy and all formatting gods --- packages/fuels-programs/src/call_utils.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/fuels-programs/src/call_utils.rs b/packages/fuels-programs/src/call_utils.rs index 1285392a4f..d1966ea20c 100644 --- a/packages/fuels-programs/src/call_utils.rs +++ b/packages/fuels-programs/src/call_utils.rs @@ -365,8 +365,7 @@ pub(crate) fn get_single_call_instructions( // empty return data. 3 is not a "magic" number: we are looking left of the 3 words // (ptr, cap, len), that are placed at the end of the data, on the right. let offset = (output_param_type.compute_encoding_width() - 3) as u16; - let selected_discriminant = - variants.heap_type_variant_discriminant().unwrap() as u16; + let selected_discriminant = variants.heap_type_variant_discriminant().unwrap() as u16; instructions.extend([ // All the registers 0x15-0x18 are free // The RET register contains the pointer address of the `CALL` return (a stack From 52730bb0670372f3b1fe49055270a1cefacfc9c2 Mon Sep 17 00:00:00 2001 From: iqdecay Date: Thu, 7 Sep 2023 15:25:06 +0200 Subject: [PATCH 15/32] Update packages/fuels-core/src/types/enum_variants.rs Co-authored-by: Ahmed Sagdati <37515857+segfault-magnet@users.noreply.github.com> --- packages/fuels-core/src/types/enum_variants.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/packages/fuels-core/src/types/enum_variants.rs b/packages/fuels-core/src/types/enum_variants.rs index a33f14fc45..83ba061628 100644 --- a/packages/fuels-core/src/types/enum_variants.rs +++ b/packages/fuels-core/src/types/enum_variants.rs @@ -41,15 +41,11 @@ impl EnumVariants { } pub fn heap_type_variant_discriminant(&self) -> Result { - for (discriminant, param) in self.param_types.iter().enumerate() { - if param.is_vm_heap_type() { - return Ok(discriminant as u8); - } - } - Err(error!( - InvalidData, - "There are no heap types inside {:?}", self - )) + self.param_types() + .iter() + .enumerate() + .find_map(|(d, p)| p.is_vm_heap_type().then_some(d as u8)) + .ok_or_else(|| error!(InvalidData, "There are no heap types inside {:?}", self)) } pub fn only_units_inside(&self) -> bool { From 5f6fedc779819c38e07267e4dbf2292e618a5a76 Mon Sep 17 00:00:00 2001 From: iqdecay Date: Thu, 7 Sep 2023 15:17:03 +0200 Subject: [PATCH 16/32] style: improve skipping bytes --- packages/fuels-core/src/codec/abi_decoder.rs | 22 +++++++++----------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/packages/fuels-core/src/codec/abi_decoder.rs b/packages/fuels-core/src/codec/abi_decoder.rs index 73e6873468..871e7ee481 100644 --- a/packages/fuels-core/src/codec/abi_decoder.rs +++ b/packages/fuels-core/src/codec/abi_decoder.rs @@ -272,23 +272,21 @@ impl ABIDecoder { /// * `data`: slice of encoded data on whose beginning we're expecting an encoded enum /// * `variants`: all types that this particular enum type could hold fn decode_enum(bytes: &[u8], variants: &EnumVariants) -> Result { - let mut data: Vec = bytes.clone().into(); let enum_width = variants.compute_encoding_width_of_enum(); - let discriminant = peek_u32(&data)? as u8; + let discriminant = peek_u32(&bytes)? as u8; let selected_variant = variants.param_type_of_variant(discriminant)?; - if selected_variant.uses_heap_types(false) { + let skip_extra = if selected_variant.uses_heap_types(false) { // remove the 3 WORDS that represent (ptr, cap, len) - // those 3 WORDS are leftovers from the concatenation of the data from the two + // those 3 WORDS are leftovers from the concatenation of the bytes from the two // `ReturnData` receipts. We need to remove them only if the selected variant is a - // heap type, otherwise we are erasing relevant data. - for _ in 8..32 { - data.remove(8); - } - } - - let words_to_skip = enum_width - selected_variant.compute_encoding_width(); - let enum_content_bytes = skip(&data, words_to_skip * WORD_SIZE)?; + // heap type, otherwise we are erasing relevant bytes. + 3 + } else { + 0 + }; + let words_to_skip = enum_width - selected_variant.compute_encoding_width() + skip_extra; + let enum_content_bytes = skip(&bytes, words_to_skip * WORD_SIZE)?; let result = Self::decode_token_in_enum(enum_content_bytes, variants, selected_variant)?; let selector = Box::new((discriminant, result.token, variants.clone())); From 40e75e13f51581e6bdd3839d8281a88007735fa8 Mon Sep 17 00:00:00 2001 From: iqdecay Date: Thu, 7 Sep 2023 15:51:35 +0200 Subject: [PATCH 17/32] fix: fix potential buffer overflow error --- packages/fuels-programs/src/call_utils.rs | 23 +++++++++++-------- .../contracts/heap_type_in_enums/src/main.sw | 5 ++++ packages/fuels/tests/types_contracts.rs | 7 ++++++ 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/packages/fuels-programs/src/call_utils.rs b/packages/fuels-programs/src/call_utils.rs index d1966ea20c..d413c33513 100644 --- a/packages/fuels-programs/src/call_utils.rs +++ b/packages/fuels-programs/src/call_utils.rs @@ -368,29 +368,32 @@ pub(crate) fn get_single_call_instructions( let selected_discriminant = variants.heap_type_variant_discriminant().unwrap() as u16; instructions.extend([ // All the registers 0x15-0x18 are free + // Load the selected discriminant to a free register + op::muli(0x17, RegId::ONE, selected_discriminant), + // the first word of the CALL return is the enum discriminant. It is safe to load + // because the offset is 0. + op::lw(0x18, RegId::RET, 0), + // If the discriminant is not the one from the heap type, then jump ahead and + // return an empty receipt. Otherwise return heap data with the right length. + op::jnef(0x17, 0x18, RegId::ZERO, 3), + // ================= EXECUTED IF THE DISCRIMINANT POINTS TO A HEAP TYPE // The RET register contains the pointer address of the `CALL` return (a stack // address). - // The RETL register contains the length of the `CALL` return (=24 in the case of - // Vec/Bytes/String because their struct takes 3 WORDs). We don't actually need it - // unless the Vec/Bytes/String struct encoding changes in the compiler. // Load the word located at the address contained in RET, it's a word that translates to // a heap address. We use the offset because in the case of an enum containing a heap // type, we need to get the pointer that is located at the heap address pointed to by // the RET register. op::lw(0x15, RegId::RET, offset), + // The RETL register contains the length of the `CALL` return (=24 in the case of + // Vec/Bytes/String because their struct takes 3 WORDs). We don't actually need it + // unless the Vec/Bytes/String struct encoding changes in the compiler. // We know a Vec/Bytes/String struct has its third WORD contain the length of the // underlying vector, so use a 2 offset to store the length. op::lw(0x16, RegId::RET, offset + 2), // The in-memory size of the type is (in-memory size of the inner type) * length op::muli(0x16, 0x16, inner_type_byte_size as u16), - // Load the selected discriminant to a free register - op::muli(0x17, RegId::ONE, selected_discriminant), - // the first word of the CALL return is the enum discriminant - op::lw(0x18, RegId::RET, 0), - // If the discriminant is not the one from the heap type, then jump ahead and - // return an empty receipt. Otherwise return heap data with the right length. - op::jnef(0x17, 0x18, RegId::ZERO, 1), op::retd(0x15, 0x16), + // ================= EXECUTED IF THE DISCRIMINANT DOESN'T POINT TO A HEAP TYPE op::retd(0x15, RegId::ZERO), ]); } else { diff --git a/packages/fuels/tests/types/contracts/heap_type_in_enums/src/main.sw b/packages/fuels/tests/types/contracts/heap_type_in_enums/src/main.sw index 0a11035b20..a25db3b060 100644 --- a/packages/fuels/tests/types/contracts/heap_type_in_enums/src/main.sw +++ b/packages/fuels/tests/types/contracts/heap_type_in_enums/src/main.sw @@ -15,6 +15,7 @@ abi MyContract { fn returns_bytes_option(return_some: bool) -> Option; fn returns_vec_option(return_some: bool) -> Option>; fn returns_string_option(return_some: bool) -> Option; + fn would_raise_a_memory_overflow() -> Result; } impl MyContract for Contract { @@ -89,4 +90,8 @@ impl MyContract for Contract { None } } + + fn would_raise_a_memory_overflow() -> Result { + Result::Err(0x1111111111111111111111111111111111111111111111111111111111111111) + } } diff --git a/packages/fuels/tests/types_contracts.rs b/packages/fuels/tests/types_contracts.rs index 357c0c8b6d..d94fe92c3d 100644 --- a/packages/fuels/tests/types_contracts.rs +++ b/packages/fuels/tests/types_contracts.rs @@ -2057,5 +2057,12 @@ async fn test_heap_type_in_enums() -> Result<()> { assert_eq!(resp.value, expected); let resp = contract_methods.returns_string_option(false).call().await?; assert!(resp.value.is_none()); + + // If the LW(RET) instruction was not executed only conditionally, then the FuelVM would OOM. + let _ = contract_methods + .would_raise_a_memory_overflow() + .call() + .await?; + Ok(()) } From 2c530a9ffbb9ed5a35f85a037e01e3ee8931557f Mon Sep 17 00:00:00 2001 From: iqdecay Date: Thu, 7 Sep 2023 16:01:08 +0200 Subject: [PATCH 18/32] style: remove unused imports --- packages/fuels/tests/types_contracts.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/fuels/tests/types_contracts.rs b/packages/fuels/tests/types_contracts.rs index d94fe92c3d..d23e28839b 100644 --- a/packages/fuels/tests/types_contracts.rs +++ b/packages/fuels/tests/types_contracts.rs @@ -1,14 +1,9 @@ -#![allow(unused_imports, unused_variables, unused_mut, dead_code)] -use fuel_tx::Receipt; use std::{result::Result as StdResult, str::FromStr}; use fuels::{ prelude::*, types::{Bits256, EvmAddress, Identity, SizedAsciiString, B512, U256}, }; -use fuels_core::codec::ABIDecoder; -use fuels_core::fn_selector; -use fuels_core::types::param_types::ParamType; pub fn null_contract_id() -> Bech32ContractId { // a bech32 contract address that decodes to [0u8;32] From 52f5ab21ed3c4925fd712a1c0121af9a6d5f7bf4 Mon Sep 17 00:00:00 2001 From: iqdecay Date: Thu, 7 Sep 2023 16:02:34 +0200 Subject: [PATCH 19/32] style: rename function --- packages/fuels-programs/src/receipt_parser.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/fuels-programs/src/receipt_parser.rs b/packages/fuels-programs/src/receipt_parser.rs index e55d5e05c0..d6020c2472 100644 --- a/packages/fuels-programs/src/receipt_parser.rs +++ b/packages/fuels-programs/src/receipt_parser.rs @@ -80,7 +80,7 @@ impl ReceiptParser { self.receipts.iter().tuple_windows().enumerate() { if let (Some(first_data), Some(second_data)) = - Self::extract_vec_data(current_receipt, next_receipt, contract_id) + Self::extract_heap_data_from_receipts(current_receipt, next_receipt, contract_id) { let mut first_data = first_data.clone(); let mut second_data = second_data.clone(); @@ -137,7 +137,7 @@ impl ReceiptParser { self.receipts.iter().tuple_windows().enumerate() { if let (_stack_data, Some(heap_data)) = - Self::extract_vec_data(current_receipt, next_receipt, contract_id) + Self::extract_heap_data_from_receipts(current_receipt, next_receipt, contract_id) { let data = heap_data.clone(); self.receipts.drain(index..=index + 1); @@ -147,7 +147,7 @@ impl ReceiptParser { None } - fn extract_vec_data<'a>( + fn extract_heap_data_from_receipts<'a>( current_receipt: &'a Receipt, next_receipt: &'a Receipt, contract_id: &ContractId, From 434755834220f93ed8ddfca3d71901a4b2c21712 Mon Sep 17 00:00:00 2001 From: iqdecay Date: Thu, 7 Sep 2023 16:07:00 +0200 Subject: [PATCH 20/32] refactor: improve filtering in call utils --- packages/fuels-programs/src/call_utils.rs | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/packages/fuels-programs/src/call_utils.rs b/packages/fuels-programs/src/call_utils.rs index d413c33513..ab42e41451 100644 --- a/packages/fuels-programs/src/call_utils.rs +++ b/packages/fuels-programs/src/call_utils.rs @@ -144,20 +144,18 @@ pub(crate) async fn build_tx_from_contract_calls( /// Compute the length of the calling scripts for the two types of contract calls: those that return /// a heap type, and those that don't. fn compute_calls_instructions_len(calls: &[ContractCall]) -> usize { - let n_enum_heap_type_calls = calls - .iter() - .filter(|c| { - c.output_param.heap_inner_element_size().is_some() - && matches!(c.output_param, ParamType::Enum { .. }) - }) - .count(); + let heap_calls = || { + calls + .iter() + .map(|call| &call.output_param) + .filter(|param| param.uses_heap_types()) + }; - let n_heap_type_calls = calls - .iter() - .filter(|c| c.output_param.heap_inner_element_size().is_some()) - .count() - - n_enum_heap_type_calls; + let n_enum_heap_type_calls = heap_calls() + .filter(|param| matches!(param, ParamType::Enum { .. })) + .count(); + let n_heap_type_calls = heap_calls().count() - n_enum_heap_type_calls; let n_stack_type_calls = calls.len() - n_heap_type_calls - n_enum_heap_type_calls; let total_instructions_len_heap_enum_data = get_single_call_instructions( From 073e995d1d65af3d5fdeda906ee08bddd001a454 Mon Sep 17 00:00:00 2001 From: iqdecay Date: Fri, 8 Sep 2023 21:18:55 +0200 Subject: [PATCH 21/32] fix --- packages/fuels-core/src/codec/abi_decoder.rs | 4 ++-- packages/fuels-core/src/types/enum_variants.rs | 3 +++ packages/fuels-programs/src/call_utils.rs | 2 +- .../tests/types/contracts/heap_type_in_enums/src/main.sw | 9 +++++++++ packages/fuels/tests/types_contracts.rs | 2 ++ 5 files changed, 17 insertions(+), 3 deletions(-) diff --git a/packages/fuels-core/src/codec/abi_decoder.rs b/packages/fuels-core/src/codec/abi_decoder.rs index 871e7ee481..736459494d 100644 --- a/packages/fuels-core/src/codec/abi_decoder.rs +++ b/packages/fuels-core/src/codec/abi_decoder.rs @@ -279,8 +279,8 @@ impl ABIDecoder { let skip_extra = if selected_variant.uses_heap_types(false) { // remove the 3 WORDS that represent (ptr, cap, len) // those 3 WORDS are leftovers from the concatenation of the bytes from the two - // `ReturnData` receipts. We need to remove them only if the selected variant is a - // heap type, otherwise we are erasing relevant bytes. + // `ReturnData` receipts. We need to skip them only if the selected variant is a + // heap type, otherwise we are skipping relevant bytes. 3 } else { 0 diff --git a/packages/fuels-core/src/types/enum_variants.rs b/packages/fuels-core/src/types/enum_variants.rs index 83ba061628..98d4b459b6 100644 --- a/packages/fuels-core/src/types/enum_variants.rs +++ b/packages/fuels-core/src/types/enum_variants.rs @@ -16,6 +16,9 @@ impl EnumVariants { if param_types.is_empty() { return Err(error!(InvalidData, "Enum variants can not be empty!")); } + // There can only be one variant in the Enum that uses heap types. The reason is that + // for bytecode injection to get the heap data, we need the encoding width of the heap + // type. To simplify, we therefore allow only one heap type inside the enum. if param_types.iter().filter(|p| p.is_vm_heap_type()).count() > 1 { Err(error!( InvalidData, diff --git a/packages/fuels-programs/src/call_utils.rs b/packages/fuels-programs/src/call_utils.rs index ab42e41451..c65c1abe0d 100644 --- a/packages/fuels-programs/src/call_utils.rs +++ b/packages/fuels-programs/src/call_utils.rs @@ -148,7 +148,7 @@ fn compute_calls_instructions_len(calls: &[ContractCall]) -> usize { calls .iter() .map(|call| &call.output_param) - .filter(|param| param.uses_heap_types()) + .filter(|param| param.uses_heap_types(false)) }; let n_enum_heap_type_calls = heap_calls() diff --git a/packages/fuels/tests/types/contracts/heap_type_in_enums/src/main.sw b/packages/fuels/tests/types/contracts/heap_type_in_enums/src/main.sw index a25db3b060..7d47186555 100644 --- a/packages/fuels/tests/types/contracts/heap_type_in_enums/src/main.sw +++ b/packages/fuels/tests/types/contracts/heap_type_in_enums/src/main.sw @@ -8,6 +8,10 @@ pub enum TestError { Else: u64, } +pub struct Bimbam { + something: Bytes, +} + abi MyContract { fn returns_bytes_result(return_ok: bool) -> Result; fn returns_vec_result(return_ok: bool) -> Result, TestError>; @@ -16,6 +20,7 @@ abi MyContract { fn returns_vec_option(return_some: bool) -> Option>; fn returns_string_option(return_some: bool) -> Option; fn would_raise_a_memory_overflow() -> Result; + fn should_fail() -> Result; } impl MyContract for Contract { @@ -94,4 +99,8 @@ impl MyContract for Contract { fn would_raise_a_memory_overflow() -> Result { Result::Err(0x1111111111111111111111111111111111111111111111111111111111111111) } + + fn should_fail() -> Result { + Result::Err(071) + } } diff --git a/packages/fuels/tests/types_contracts.rs b/packages/fuels/tests/types_contracts.rs index d23e28839b..4e66593f25 100644 --- a/packages/fuels/tests/types_contracts.rs +++ b/packages/fuels/tests/types_contracts.rs @@ -2059,5 +2059,7 @@ async fn test_heap_type_in_enums() -> Result<()> { .call() .await?; + // let _ = contract_methods.should_fail().call().await?; + Ok(()) } From 9ddc3b90126e9075a51aa31c1d7314b3da3aeade Mon Sep 17 00:00:00 2001 From: iqdecay Date: Fri, 8 Sep 2023 21:42:41 +0200 Subject: [PATCH 22/32] fix: disallow multiple heap types in variants only in return types --- packages/fuels-core/src/codec/abi_decoder.rs | 43 +++++++++++++++++++ .../fuels-core/src/types/enum_variants.rs | 31 +------------ 2 files changed, 44 insertions(+), 30 deletions(-) diff --git a/packages/fuels-core/src/codec/abi_decoder.rs b/packages/fuels-core/src/codec/abi_decoder.rs index 736459494d..ac4cf20f6d 100644 --- a/packages/fuels-core/src/codec/abi_decoder.rs +++ b/packages/fuels-core/src/codec/abi_decoder.rs @@ -272,6 +272,21 @@ impl ABIDecoder { /// * `data`: slice of encoded data on whose beginning we're expecting an encoded enum /// * `variants`: all types that this particular enum type could hold fn decode_enum(bytes: &[u8], variants: &EnumVariants) -> Result { + // There can only be one variant using heap types in the Enum. The reason is that for + // bytecode injection to get the heap data, we need the encoding width of the heap type. To + // simplify, we therefore allow only one heap type inside the enum. + if variants + .param_types() + .iter() + .filter(|p| p.is_vm_heap_type()) + .count() + > 1 + { + return Err(error!( + InvalidData, + "Enum variants as return types can only contain one heap type" + )); + }; let enum_width = variants.compute_encoding_width_of_enum(); let discriminant = peek_u32(&bytes)? as u8; @@ -815,4 +830,32 @@ mod tests { assert!(matches!(error, Error::InvalidData(str) if str.starts_with(expected_msg))); Ok(()) } + + #[test] + fn decoding_enum_with_more_than_one_heap_type_variant_fails() -> Result<()> { + let mut param_types = vec![ + ParamType::U64, + ParamType::Bool, + ParamType::Vector(Box::from(ParamType::U64)), + ]; + // empty data + let data = [ + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + ]; + let variants = EnumVariants::new(param_types.clone())?; + // it works if there is only one heap type + let _ = ABIDecoder::decode_enum(&data, &variants)?; + + param_types.append(&mut vec![ParamType::Bytes]); + let variants = EnumVariants::new(param_types)?; + // fails if there is more than one variant using heap type in the enum + let error = ABIDecoder::decode_enum(&data, &variants).expect_err("Should fail"); + let expected_error = + "Invalid data: Enum variants as return types can only contain one heap type" + .to_string(); + assert_eq!(error.to_string(), expected_error); + + Ok(()) + } } diff --git a/packages/fuels-core/src/types/enum_variants.rs b/packages/fuels-core/src/types/enum_variants.rs index 98d4b459b6..626375cde5 100644 --- a/packages/fuels-core/src/types/enum_variants.rs +++ b/packages/fuels-core/src/types/enum_variants.rs @@ -16,17 +16,7 @@ impl EnumVariants { if param_types.is_empty() { return Err(error!(InvalidData, "Enum variants can not be empty!")); } - // There can only be one variant in the Enum that uses heap types. The reason is that - // for bytecode injection to get the heap data, we need the encoding width of the heap - // type. To simplify, we therefore allow only one heap type inside the enum. - if param_types.iter().filter(|p| p.is_vm_heap_type()).count() > 1 { - Err(error!( - InvalidData, - "Enum variants can only contain one heap type" - )) - } else { - Ok(EnumVariants { param_types }) - } + Ok(EnumVariants { param_types }) } pub fn param_types(&self) -> &[ParamType] { @@ -86,25 +76,6 @@ impl EnumVariants { mod tests { use super::*; - #[test] - fn test_enum_variants_can_have_only_one_heap_type() -> Result<()> { - let mut param_types = vec![ - ParamType::U64, - ParamType::Bool, - ParamType::Vector(Box::from(ParamType::U64)), - ]; - // it works if there is only one heap type - let _variants = EnumVariants::new(param_types.clone())?; - param_types.append(&mut vec![ParamType::Bytes]); - - let error = EnumVariants::new(param_types).expect_err("Should have failed"); - let expected_error = - "Invalid data: Enum variants can only contain one heap type".to_string(); - assert_eq!(error.to_string(), expected_error); - - Ok(()) - } - #[test] fn test_get_heap_type_variant_discriminant() -> Result<()> { let param_types = vec![ From 05c69bf1731bb65b38a0d5e4393e125ef8414056 Mon Sep 17 00:00:00 2001 From: iqdecay Date: Wed, 13 Sep 2023 14:06:29 +0200 Subject: [PATCH 23/32] feat: improve with segfault suggestion --- packages/fuels-core/src/codec/abi_decoder.rs | 84 ++-- .../fuels-core/src/types/enum_variants.rs | 20 +- packages/fuels-core/src/types/param_types.rs | 408 +++++++++--------- packages/fuels-programs/src/call_utils.rs | 126 +++--- packages/fuels-programs/src/contract.rs | 4 +- packages/fuels-programs/src/receipt_parser.rs | 10 +- packages/fuels/tests/types_contracts.rs | 72 ++-- 7 files changed, 374 insertions(+), 350 deletions(-) diff --git a/packages/fuels-core/src/codec/abi_decoder.rs b/packages/fuels-core/src/codec/abi_decoder.rs index ac4cf20f6d..92e0723246 100644 --- a/packages/fuels-core/src/codec/abi_decoder.rs +++ b/packages/fuels-core/src/codec/abi_decoder.rs @@ -53,16 +53,11 @@ impl ABIDecoder { /// The same as `decode` just for a single type. Used in most cases since /// contract functions can only return one type. pub fn decode_single(param_type: &ParamType, bytes: &[u8]) -> Result { + param_type.validate_is_decodable()?; Ok(Self::decode_param(param_type, bytes)?.token) } fn decode_param(param_type: &ParamType, bytes: &[u8]) -> Result { - if param_type.contains_nested_heap_types() { - return Err(error!( - InvalidData, - "Type {param_type:?} contains nested heap types (`Vec` or `Bytes`), this is not supported." - )); - } match param_type { ParamType::Unit => Self::decode_unit(bytes), ParamType::U8 => Self::decode_u8(bytes), @@ -137,6 +132,7 @@ impl ABIDecoder { let mut bytes_read = 0; for param_type in param_types { + param_type.validate_is_decodable()?; let res = Self::decode_param(param_type, skip(bytes, bytes_read)?)?; bytes_read += res.bytes_read; results.push(res.token); @@ -272,36 +268,17 @@ impl ABIDecoder { /// * `data`: slice of encoded data on whose beginning we're expecting an encoded enum /// * `variants`: all types that this particular enum type could hold fn decode_enum(bytes: &[u8], variants: &EnumVariants) -> Result { - // There can only be one variant using heap types in the Enum. The reason is that for - // bytecode injection to get the heap data, we need the encoding width of the heap type. To - // simplify, we therefore allow only one heap type inside the enum. - if variants - .param_types() - .iter() - .filter(|p| p.is_vm_heap_type()) - .count() - > 1 - { - return Err(error!( - InvalidData, - "Enum variants as return types can only contain one heap type" - )); - }; let enum_width = variants.compute_encoding_width_of_enum(); - let discriminant = peek_u32(&bytes)? as u8; + let discriminant = peek_u32(bytes)? as u8; let selected_variant = variants.param_type_of_variant(discriminant)?; - let skip_extra = if selected_variant.uses_heap_types(false) { - // remove the 3 WORDS that represent (ptr, cap, len) - // those 3 WORDS are leftovers from the concatenation of the bytes from the two - // `ReturnData` receipts. We need to skip them only if the selected variant is a - // heap type, otherwise we are skipping relevant bytes. - 3 - } else { - 0 - }; - let words_to_skip = enum_width - selected_variant.compute_encoding_width() + skip_extra; - let enum_content_bytes = skip(&bytes, words_to_skip * WORD_SIZE)?; + let skip_extra = variants + .heap_type_variant() + .is_some_and(|(heap_discriminant, _)| heap_discriminant == discriminant) + .then_some(3); + let words_to_skip = + enum_width - selected_variant.compute_encoding_width() + skip_extra.unwrap_or_default(); + let enum_content_bytes = skip(bytes, words_to_skip * WORD_SIZE)?; let result = Self::decode_token_in_enum(enum_content_bytes, variants, selected_variant)?; let selector = Box::new((discriminant, result.token, variants.clone())); @@ -844,18 +821,53 @@ mod tests { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ]; let variants = EnumVariants::new(param_types.clone())?; + let enum_param_type = ParamType::Enum { + variants, + generics: vec![], + }; // it works if there is only one heap type - let _ = ABIDecoder::decode_enum(&data, &variants)?; + let _ = ABIDecoder::decode_single(&enum_param_type, &data)?; param_types.append(&mut vec![ParamType::Bytes]); let variants = EnumVariants::new(param_types)?; + let enum_param_type = ParamType::Enum { + variants, + generics: vec![], + }; // fails if there is more than one variant using heap type in the enum - let error = ABIDecoder::decode_enum(&data, &variants).expect_err("Should fail"); + let error = ABIDecoder::decode_single(&enum_param_type, &data).expect_err("Should fail"); let expected_error = - "Invalid data: Enum variants as return types can only contain one heap type" + "Invalid type: Enums currently support only one heap-type variant. Found: 2" .to_string(); assert_eq!(error.to_string(), expected_error); Ok(()) } + + #[test] + fn enums_w_too_deeply_nested_heap_types_not_allowed() { + let param_types = vec![ + ParamType::U8, + ParamType::Struct { + fields: vec![ParamType::RawSlice], + generics: vec![], + }, + ]; + let variants = EnumVariants::new(param_types).unwrap(); + let enum_param_type = ParamType::Enum { + variants, + generics: vec![], + }; + + let err = ABIDecoder::decode_single(&enum_param_type, &[]).expect_err("should have failed"); + + let Error::InvalidType(msg) = err else { + panic!("Unexpected err: {err}"); + }; + + assert_eq!( + msg, + "Enums currently support only one level deep heap types." + ); + } } diff --git a/packages/fuels-core/src/types/enum_variants.rs b/packages/fuels-core/src/types/enum_variants.rs index 626375cde5..f82e40bcc2 100644 --- a/packages/fuels-core/src/types/enum_variants.rs +++ b/packages/fuels-core/src/types/enum_variants.rs @@ -33,12 +33,13 @@ impl EnumVariants { }) } - pub fn heap_type_variant_discriminant(&self) -> Result { + pub fn heap_type_variant(&self) -> Option<(u8, &ParamType)> { + // TODO: segfault this needs to look at only the parent heap types. Or maybe to extract + // this from here because this limitation is not present when enums are used as args. self.param_types() .iter() .enumerate() - .find_map(|(d, p)| p.is_vm_heap_type().then_some(d as u8)) - .ok_or_else(|| error!(InvalidData, "There are no heap types inside {:?}", self)) + .find_map(|(d, p)| p.needs_extra_data_receipt(false).then_some((d as u8, p))) } pub fn only_units_inside(&self) -> bool { @@ -84,7 +85,7 @@ mod tests { ParamType::Vector(Box::from(ParamType::U64)), ]; let variants = EnumVariants::new(param_types)?; - assert_eq!(variants.heap_type_variant_discriminant()?, 2); + assert_eq!(variants.heap_type_variant().unwrap().0, 2); let param_types = vec![ ParamType::Vector(Box::from(ParamType::U64)), @@ -92,18 +93,11 @@ mod tests { ParamType::Bool, ]; let variants = EnumVariants::new(param_types)?; - assert_eq!(variants.heap_type_variant_discriminant()?, 0); + assert_eq!(variants.heap_type_variant().unwrap().0, 0); let param_types = vec![ParamType::U64, ParamType::Bool]; let variants = EnumVariants::new(param_types)?; - let error = variants - .heap_type_variant_discriminant() - .expect_err("Should have failed"); - let expected_error = format!( - "Invalid data: There are no heap types inside {:?}", - variants - ); - assert_eq!(error.to_string(), expected_error); + assert!(variants.heap_type_variant().is_none()); Ok(()) } } diff --git a/packages/fuels-core/src/types/param_types.rs b/packages/fuels-core/src/types/param_types.rs index 16272fb556..d0cac851db 100644 --- a/packages/fuels-core/src/types/param_types.rs +++ b/packages/fuels-core/src/types/param_types.rs @@ -4,7 +4,6 @@ use fuel_abi_types::{ abi::program::{TypeApplication, TypeDeclaration}, utils::{extract_array_len, extract_generic_name, extract_str_len, has_tuple_format}, }; -use itertools::chain; use crate::{ constants::WORD_SIZE, @@ -80,75 +79,90 @@ impl ParamType { Ok(available_bytes / memory_size) } - pub fn contains_nested_heap_types(&self) -> bool { - match &self { - ParamType::Vector(param_type) => param_type.uses_heap_types(false), - ParamType::Bytes => false, - // Here, we return false because even though the `Token::String` type has an underlying - // `Bytes` type nested, it is an exception that will be generalized as part of - // https://github.com/FuelLabs/fuels-rs/discussions/944 - ParamType::String => false, - // We are currently allowing enums that are wrapping around heap types to allow for - // `Result` to be returned from a contract - ParamType::Enum { variants, generics } => { - let mut param_types = chain!(variants.param_types(), generics); - param_types.any(|p| p.uses_heap_types(true)) + pub fn children_need_extra_receipts(&self) -> bool { + match self { + ParamType::Array(inner, _) | ParamType::Vector(inner) => { + inner.needs_extra_data_receipt(false) } - _ => self.uses_heap_types(false), + ParamType::Struct { fields, .. } => fields + .iter() + .any(|param_type| param_type.needs_extra_data_receipt(false)), + ParamType::Enum { variants, .. } => variants + .param_types() + .iter() + .any(|param_type| param_type.needs_extra_data_receipt(false)), + ParamType::Tuple(inner_types) => inner_types + .iter() + .any(|param_type| param_type.needs_extra_data_receipt(false)), + _ => false, } } - pub fn uses_heap_types(&self, can_be_heap_type_itself: bool) -> bool { - match &self { - ParamType::Vector(..) | ParamType::Bytes | ParamType::String => { - !can_be_heap_type_itself - } - ParamType::Array(param_type, ..) => param_type.uses_heap_types(false), - ParamType::Tuple(param_types, ..) => Self::any_nested_heap_types(param_types), - ParamType::Enum { - generics, variants, .. - } => { - let variants_types = variants.param_types(); - Self::any_nested_heap_types(chain!(generics, variants_types)) + pub fn validate_is_decodable(&self) -> Result<()> { + match self { + ParamType::Enum { variants, .. } => { + let grandchildren_need_receipts = variants + .param_types() + .iter() + .any(|child| child.children_need_extra_receipts()); + if grandchildren_need_receipts { + return Err(error!( + InvalidType, + "Enums currently support only one level deep heap types." + )); + } + + let num_of_children_needing_receipts = variants + .param_types() + .iter() + .filter(|param_type| param_type.needs_extra_data_receipt(false)) + .count(); + + if num_of_children_needing_receipts > 1 { + Err(error!( + InvalidType, + "Enums currently support only one heap-type variant. Found: {num_of_children_needing_receipts}" + )) + } else { + Ok(()) + } } - ParamType::Struct { - fields, generics, .. - } => Self::any_nested_heap_types(chain!(fields, generics)), - _ => false, + _ if self.children_need_extra_receipts() => Err(error!( + InvalidType, + "Nested heap types are currently not supported except in Enums." + )), + _ => Ok(()), } } - fn any_nested_heap_types<'a>(param_types: impl IntoIterator) -> bool { - param_types - .into_iter() - .any(|param_type| param_type.uses_heap_types(false)) - } - - pub fn is_vm_heap_type(&self) -> bool { - matches!( - self, - ParamType::Vector(..) | ParamType::Bytes | ParamType::String - ) + pub fn needs_extra_data_receipt(&self, top_level_type: bool) -> bool { + match self { + ParamType::Vector(_) | ParamType::Bytes | ParamType::String => true, + ParamType::Array(inner, _) => inner.needs_extra_data_receipt(false), + ParamType::Struct { fields, .. } => fields + .iter() + .any(|param_type| param_type.needs_extra_data_receipt(false)), + ParamType::Enum { variants, .. } => variants + .param_types() + .iter() + .any(|param_type| param_type.needs_extra_data_receipt(false)), + ParamType::Tuple(elements) => elements + .iter() + .any(|param_type| param_type.needs_extra_data_receipt(false)), + ParamType::RawSlice => !top_level_type, + _ => false, + } } /// Compute the inner memory size of a containing heap type (`Bytes` or `Vec`s). - pub fn heap_inner_element_size(&self) -> Option { + pub fn heap_inner_element_size(&self, top_level_type: bool) -> Option { match &self { ParamType::Vector(inner_param_type) => { Some(inner_param_type.compute_encoding_width() * WORD_SIZE) } // `Bytes` type is byte-packed in the VM, so it's the size of an u8 ParamType::Bytes | ParamType::String => Some(std::mem::size_of::()), - ParamType::Enum { variants, generics } => { - let variants_types = variants.param_types(); - // There is at most one heap type per enum - for param in chain!(generics, variants_types) { - if param.is_vm_heap_type() { - return Some(param.heap_inner_element_size().unwrap()); - } - } - None - } + ParamType::RawSlice if !top_level_type => Some(ParamType::U64.compute_encoding_width()), _ => None, } } @@ -1313,122 +1327,125 @@ mod tests { Ok(()) } - #[test] - fn contains_nested_heap_types_false_on_simple_types() -> Result<()> { - // Simple types cannot have nested heap types - assert!(!ParamType::Unit.contains_nested_heap_types()); - assert!(!ParamType::U8.contains_nested_heap_types()); - assert!(!ParamType::U16.contains_nested_heap_types()); - assert!(!ParamType::U32.contains_nested_heap_types()); - assert!(!ParamType::U64.contains_nested_heap_types()); - assert!(!ParamType::Bool.contains_nested_heap_types()); - assert!(!ParamType::B256.contains_nested_heap_types()); - assert!(!ParamType::StringArray(10).contains_nested_heap_types()); - assert!(!ParamType::RawSlice.contains_nested_heap_types()); - assert!(!ParamType::Bytes.contains_nested_heap_types()); - assert!(!ParamType::String.contains_nested_heap_types()); - Ok(()) - } - - #[test] - fn test_complex_types_for_nested_heap_types_containing_vectors() -> Result<()> { - let base_vector = ParamType::Vector(Box::from(ParamType::U8)); - let param_types_no_nested_vec = vec![ParamType::U64, ParamType::U32]; - let param_types_nested_vec = vec![ParamType::Unit, ParamType::Bool, base_vector.clone()]; - - let is_nested = |param_type: ParamType| assert!(param_type.contains_nested_heap_types()); - let not_nested = |param_type: ParamType| assert!(!param_type.contains_nested_heap_types()); - - not_nested(base_vector.clone()); - is_nested(ParamType::Vector(Box::from(base_vector.clone()))); - - not_nested(ParamType::Array(Box::from(ParamType::U8), 10)); - is_nested(ParamType::Array(Box::from(base_vector), 10)); - - not_nested(ParamType::Tuple(param_types_no_nested_vec.clone())); - is_nested(ParamType::Tuple(param_types_nested_vec.clone())); - - not_nested(ParamType::Struct { - generics: param_types_no_nested_vec.clone(), - fields: param_types_no_nested_vec.clone(), - }); - is_nested(ParamType::Struct { - generics: param_types_nested_vec.clone(), - fields: param_types_no_nested_vec.clone(), - }); - is_nested(ParamType::Struct { - generics: param_types_no_nested_vec.clone(), - fields: param_types_nested_vec.clone(), - }); - - not_nested(ParamType::Enum { - variants: EnumVariants::new(param_types_no_nested_vec.clone())?, - generics: param_types_no_nested_vec.clone(), - }); - not_nested(ParamType::Enum { - variants: EnumVariants::new(param_types_nested_vec.clone())?, - generics: param_types_no_nested_vec.clone(), - }); - not_nested(ParamType::Enum { - variants: EnumVariants::new(param_types_no_nested_vec)?, - generics: param_types_nested_vec, - }); - Ok(()) - } - - #[test] - fn test_complex_types_for_nested_heap_types_containing_bytes() -> Result<()> { - let base_bytes = ParamType::Bytes; - let param_types_no_nested_bytes = vec![ParamType::U64, ParamType::U32]; - let param_types_nested_bytes = vec![ParamType::Unit, ParamType::Bool, base_bytes.clone()]; - - let is_nested = |param_type: ParamType| assert!(param_type.contains_nested_heap_types()); - let not_nested = |param_type: ParamType| assert!(!param_type.contains_nested_heap_types()); - - not_nested(base_bytes.clone()); - is_nested(ParamType::Vector(Box::from(base_bytes.clone()))); - - not_nested(ParamType::Array(Box::from(ParamType::U8), 10)); - is_nested(ParamType::Array(Box::from(base_bytes), 10)); - - not_nested(ParamType::Tuple(param_types_no_nested_bytes.clone())); - is_nested(ParamType::Tuple(param_types_nested_bytes.clone())); - - let not_nested_struct = ParamType::Struct { - generics: param_types_no_nested_bytes.clone(), - fields: param_types_no_nested_bytes.clone(), - }; - not_nested(not_nested_struct); - - let nested_struct = ParamType::Struct { - generics: param_types_nested_bytes.clone(), - fields: param_types_no_nested_bytes.clone(), - }; - is_nested(nested_struct); - - let nested_struct = ParamType::Struct { - generics: param_types_no_nested_bytes.clone(), - fields: param_types_nested_bytes.clone(), - }; - is_nested(nested_struct); - - not_nested(ParamType::Enum { - variants: EnumVariants::new(param_types_no_nested_bytes.clone())?, - generics: param_types_no_nested_bytes.clone(), - }); - - not_nested(ParamType::Enum { - variants: EnumVariants::new(param_types_nested_bytes.clone())?, - generics: param_types_no_nested_bytes.clone(), - }); - - not_nested(ParamType::Enum { - variants: EnumVariants::new(param_types_no_nested_bytes)?, - generics: param_types_nested_bytes, - }); - - Ok(()) - } + // TODO: segfault + // #[test] + // fn contains_nested_heap_types_false_on_simple_types() -> Result<()> { + // // Simple types cannot have nested heap types + // assert!(!ParamType::Unit.contains_nested_heap_types()); + // assert!(!ParamType::U8.contains_nested_heap_types()); + // assert!(!ParamType::U16.contains_nested_heap_types()); + // assert!(!ParamType::U32.contains_nested_heap_types()); + // assert!(!ParamType::U64.contains_nested_heap_types()); + // assert!(!ParamType::Bool.contains_nested_heap_types()); + // assert!(!ParamType::B256.contains_nested_heap_types()); + // assert!(!ParamType::StringArray(10).contains_nested_heap_types()); + // assert!(!ParamType::RawSlice.contains_nested_heap_types()); + // assert!(!ParamType::Bytes.contains_nested_heap_types()); + // assert!(!ParamType::String.contains_nested_heap_types()); + // Ok(()) + // } + + // TODO: segfault + // #[test] + // fn test_complex_types_for_nested_heap_types_containing_vectors() -> Result<()> { + // let base_vector = ParamType::Vector(Box::from(ParamType::U8)); + // let param_types_no_nested_vec = vec![ParamType::U64, ParamType::U32]; + // let param_types_nested_vec = vec![ParamType::Unit, ParamType::Bool, base_vector.clone()]; + // + // let is_nested = |param_type: ParamType| assert!(param_type.contains_nested_heap_types()); + // let not_nested = |param_type: ParamType| assert!(!param_type.contains_nested_heap_types()); + // + // not_nested(base_vector.clone()); + // is_nested(ParamType::Vector(Box::from(base_vector.clone()))); + // + // not_nested(ParamType::Array(Box::from(ParamType::U8), 10)); + // is_nested(ParamType::Array(Box::from(base_vector), 10)); + // + // not_nested(ParamType::Tuple(param_types_no_nested_vec.clone())); + // is_nested(ParamType::Tuple(param_types_nested_vec.clone())); + // + // not_nested(ParamType::Struct { + // generics: param_types_no_nested_vec.clone(), + // fields: param_types_no_nested_vec.clone(), + // }); + // is_nested(ParamType::Struct { + // generics: param_types_nested_vec.clone(), + // fields: param_types_no_nested_vec.clone(), + // }); + // is_nested(ParamType::Struct { + // generics: param_types_no_nested_vec.clone(), + // fields: param_types_nested_vec.clone(), + // }); + // + // not_nested(ParamType::Enum { + // variants: EnumVariants::new(param_types_no_nested_vec.clone())?, + // generics: param_types_no_nested_vec.clone(), + // }); + // not_nested(ParamType::Enum { + // variants: EnumVariants::new(param_types_nested_vec.clone())?, + // generics: param_types_no_nested_vec.clone(), + // }); + // not_nested(ParamType::Enum { + // variants: EnumVariants::new(param_types_no_nested_vec)?, + // generics: param_types_nested_vec, + // }); + // Ok(()) + // } + + // TODO: segfault + // #[test] + // fn test_complex_types_for_nested_heap_types_containing_bytes() -> Result<()> { + // let base_bytes = ParamType::Bytes; + // let param_types_no_nested_bytes = vec![ParamType::U64, ParamType::U32]; + // let param_types_nested_bytes = vec![ParamType::Unit, ParamType::Bool, base_bytes.clone()]; + // + // let is_nested = |param_type: ParamType| assert!(param_type.contains_nested_heap_types()); + // let not_nested = |param_type: ParamType| assert!(!param_type.contains_nested_heap_types()); + // + // not_nested(base_bytes.clone()); + // is_nested(ParamType::Vector(Box::from(base_bytes.clone()))); + // + // not_nested(ParamType::Array(Box::from(ParamType::U8), 10)); + // is_nested(ParamType::Array(Box::from(base_bytes), 10)); + // + // not_nested(ParamType::Tuple(param_types_no_nested_bytes.clone())); + // is_nested(ParamType::Tuple(param_types_nested_bytes.clone())); + // + // let not_nested_struct = ParamType::Struct { + // generics: param_types_no_nested_bytes.clone(), + // fields: param_types_no_nested_bytes.clone(), + // }; + // not_nested(not_nested_struct); + // + // let nested_struct = ParamType::Struct { + // generics: param_types_nested_bytes.clone(), + // fields: param_types_no_nested_bytes.clone(), + // }; + // is_nested(nested_struct); + // + // let nested_struct = ParamType::Struct { + // generics: param_types_no_nested_bytes.clone(), + // fields: param_types_nested_bytes.clone(), + // }; + // is_nested(nested_struct); + // + // not_nested(ParamType::Enum { + // variants: EnumVariants::new(param_types_no_nested_bytes.clone())?, + // generics: param_types_no_nested_bytes.clone(), + // }); + // + // not_nested(ParamType::Enum { + // variants: EnumVariants::new(param_types_nested_bytes.clone())?, + // generics: param_types_no_nested_bytes.clone(), + // }); + // + // not_nested(ParamType::Enum { + // variants: EnumVariants::new(param_types_no_nested_bytes)?, + // generics: param_types_nested_bytes, + // }); + // + // Ok(()) + // } #[test] fn try_vector_is_type_path_backward_compatible() { @@ -1500,32 +1517,33 @@ mod tests { assert_eq!(param_type, ParamType::String); } - #[test] - fn test_double_nested_heap_type_enum() -> Result<()> { - let enum_no_heap_types = ParamType::Enum { - variants: EnumVariants::new(vec![ParamType::U64, ParamType::Bool])?, - generics: vec![], - }; - let r = ParamType::Enum { - variants: EnumVariants::new(vec![ParamType::Bytes, enum_no_heap_types])?, - generics: vec![], - }; - assert!(!r.contains_nested_heap_types()); - - let enum_heap_types = ParamType::Enum { - variants: EnumVariants::new(vec![ - ParamType::Vector(Box::from(ParamType::U64)), - ParamType::Bool, - ])?, - generics: vec![], - }; - let r = ParamType::Enum { - variants: EnumVariants::new(vec![ParamType::Bytes, enum_heap_types])?, - generics: vec![], - }; - assert!(r.contains_nested_heap_types()); - Ok(()) - } + // TODO: segfault + // #[test] + // fn test_double_nested_heap_type_enum() -> Result<()> { + // let enum_no_heap_types = ParamType::Enum { + // variants: EnumVariants::new(vec![ParamType::U64, ParamType::Bool])?, + // generics: vec![], + // }; + // let r = ParamType::Enum { + // variants: EnumVariants::new(vec![ParamType::Bytes, enum_no_heap_types])?, + // generics: vec![], + // }; + // assert!(!r.contains_nested_heap_types()); + // + // let enum_heap_types = ParamType::Enum { + // variants: EnumVariants::new(vec![ + // ParamType::Vector(Box::from(ParamType::U64)), + // ParamType::Bool, + // ])?, + // generics: vec![], + // }; + // let r = ParamType::Enum { + // variants: EnumVariants::new(vec![ParamType::Bytes, enum_heap_types])?, + // generics: vec![], + // }; + // assert!(r.contains_nested_heap_types()); + // Ok(()) + // } fn given_type_with_path(path: &str) -> Type { Type { diff --git a/packages/fuels-programs/src/call_utils.rs b/packages/fuels-programs/src/call_utils.rs index c7a05c1234..efe3ffd999 100644 --- a/packages/fuels-programs/src/call_utils.rs +++ b/packages/fuels-programs/src/call_utils.rs @@ -10,7 +10,6 @@ use fuels_core::{ offsets::call_script_data_offset, types::{ bech32::{Bech32Address, Bech32ContractId}, - enum_variants::EnumVariants, errors::{Error as FuelsError, Result}, input::Input, param_types::ParamType, @@ -157,20 +156,7 @@ fn compute_calls_instructions_len(calls: &[ContractCall]) -> usize { call_opcode_params.gas_forwarded_offset = Some(0); } - let param_type = if c.output_param.uses_heap_types(false) - && matches!(c.output_param, ParamType::Enum { .. }) - { - ParamType::Enum { - variants: EnumVariants::new(vec![ParamType::Bytes]).unwrap(), - generics: vec![], - } - } else if c.output_param.uses_heap_types(false) { - ParamType::Vector(Box::from(ParamType::U64)) - } else { - ParamType::U64 - }; - - get_single_call_instructions(&call_opcode_params, ¶m_type).len() + get_single_call_instructions(&call_opcode_params, &c.output_param).len() }) .sum() } @@ -360,60 +346,70 @@ pub(crate) fn get_single_call_instructions( None => instructions.push(op::call(0x10, 0x11, 0x12, RegId::CGAS)), }; - // The instructions are different if you want to return data that was on the heap - if let Some(inner_type_byte_size) = output_param_type.heap_inner_element_size() { - if let ParamType::Enum { variants, .. } = output_param_type { - // This offset is here because if the variant type is an enum containing a heap type, - // the 3 words containing (ptr, cap, len) are at the end of the data of the enum, which - // depends on the encoding width. If the variant type does not contain a heap type, then - // the receipt generated will not be read anyway, so we can generate a receipt with - // empty return data. 3 is not a "magic" number: we are looking left of the 3 words - // (ptr, cap, len), that are placed at the end of the data, on the right. - let offset = (output_param_type.compute_encoding_width() - 3) as u16; - let selected_discriminant = variants.heap_type_variant_discriminant().unwrap() as u16; - instructions.extend([ - // All the registers 0x15-0x18 are free - // Load the selected discriminant to a free register - op::muli(0x17, RegId::ONE, selected_discriminant), - // the first word of the CALL return is the enum discriminant. It is safe to load - // because the offset is 0. - op::lw(0x18, RegId::RET, 0), - // If the discriminant is not the one from the heap type, then jump ahead and - // return an empty receipt. Otherwise return heap data with the right length. - op::jnef(0x17, 0x18, RegId::ZERO, 3), + instructions.extend(extract_heap_data(output_param_type)); + + #[allow(clippy::iter_cloned_collect)] + instructions.into_iter().collect::>() +} + +fn extract_heap_data(param_type: &ParamType) -> Vec { + match param_type { + ParamType::Enum { variants, .. } => { + eprintln!("Injecting for enums: start"); + let Some((discriminant, heap_type)) = variants.heap_type_variant() else { + eprintln!("Injecting for enums: wasnt a heap type"); + return vec![]; + }; + + let ptr_offset = (param_type.compute_encoding_width() - 3) as u16; + + [ + vec![ + // All the registers 0x15-0x18 are free + // Load the selected discriminant to a free register + op::movi(0x17, discriminant as u32), + // the first word of the CALL return is the enum discriminant. It is safe to load + // because the offset is 0. + op::lw(0x18, RegId::RET, 0), + // If the discriminant is not the one from the heap type, then jump ahead and + // return an empty receipt. Otherwise return heap data with the right length. + // TODO: segfault is this really 3 or 4? Look at spec + op::jnef(0x17, 0x18, RegId::ZERO, 3), + ], // ================= EXECUTED IF THE DISCRIMINANT POINTS TO A HEAP TYPE - // The RET register contains the pointer address of the `CALL` return (a stack - // address). - // Load the word located at the address contained in RET, it's a word that translates to - // a heap address. We use the offset because in the case of an enum containing a heap - // type, we need to get the pointer that is located at the heap address pointed to by - // the RET register. - op::lw(0x15, RegId::RET, offset), - // The RETL register contains the length of the `CALL` return (=24 in the case of - // Vec/Bytes/String because their struct takes 3 WORDs). We don't actually need it - // unless the Vec/Bytes/String struct encoding changes in the compiler. - // We know a Vec/Bytes/String struct has its third WORD contain the length of the - // underlying vector, so use a 2 offset to store the length. - op::lw(0x16, RegId::RET, offset + 2), - // The in-memory size of the type is (in-memory size of the inner type) * length - op::muli(0x16, 0x16, inner_type_byte_size as u16), - op::retd(0x15, 0x16), + extract_data_receipt(ptr_offset, false, heap_type), // ================= EXECUTED IF THE DISCRIMINANT DOESN'T POINT TO A HEAP TYPE - op::retd(0x15, RegId::ZERO), - ]); - } else { - // See above for explanations, no offset needed if the type is not an enum - instructions.extend([ - op::lw(0x15, RegId::RET, 0), - op::lw(0x16, RegId::RET, 2), - op::muli(0x16, 0x16, inner_type_byte_size as u16), - op::retd(0x15, 0x16), - ]); - }; + vec![op::retd(0x15, RegId::ZERO)], + ] + .concat() + } + _ => extract_data_receipt(0, true, param_type), } +} - #[allow(clippy::iter_cloned_collect)] - instructions.into_iter().collect::>() +fn extract_data_receipt( + ptr_offset: u16, + top_level_type: bool, + param_type: &ParamType, +) -> Vec { + eprintln!("Injecting: extract_data_receipt({ptr_offset}, {top_level_type}, {param_type:?})"); + let Some(inner_type_byte_size) = param_type.heap_inner_element_size(top_level_type) else { + return vec![]; + }; + + let len_offset = match (top_level_type, param_type) { + // A nested RawSlice shows up as ptr,len + (false, ParamType::RawSlice) => 1, + // Every other heap type (currently) shows up as ptr,cap,len + _ => 2, + }; + + vec![ + op::lw(0x15, RegId::RET, ptr_offset), + op::lw(0x16, RegId::RET, ptr_offset + len_offset), + op::muli(0x16, 0x16, inner_type_byte_size as u16), + op::retd(0x15, 0x16), + ] } /// Returns the assets and contracts that will be consumed ([`Input`]s) diff --git a/packages/fuels-programs/src/contract.rs b/packages/fuels-programs/src/contract.rs index 3274d86872..4e410bd074 100644 --- a/packages/fuels-programs/src/contract.rs +++ b/packages/fuels-programs/src/contract.rs @@ -805,7 +805,7 @@ impl MultiContractCallHandler { let number_of_heap_type_calls = self .contract_calls .iter() - .filter(|cc| cc.output_param.is_vm_heap_type()) + .filter(|cc| cc.output_param.needs_extra_data_receipt(true)) .count(); match number_of_heap_type_calls { @@ -816,7 +816,7 @@ impl MultiContractCallHandler { .last() .expect("is not empty") .output_param - .is_vm_heap_type() + .needs_extra_data_receipt(true) { Ok(()) } else { diff --git a/packages/fuels-programs/src/receipt_parser.rs b/packages/fuels-programs/src/receipt_parser.rs index d6020c2472..d06ab5b1e4 100644 --- a/packages/fuels-programs/src/receipt_parser.rs +++ b/packages/fuels-programs/src/receipt_parser.rs @@ -41,6 +41,10 @@ impl ReceiptParser { // During a script execution, the script's contract id is the **null** contract id .unwrap_or_else(ContractId::zeroed); + dbg!(&self.receipts); + + output_param.validate_is_decodable()?; + let data = self .extract_raw_data(output_param, &contract_id) .ok_or_else(|| Self::missing_receipts_error(output_param))?; @@ -60,14 +64,14 @@ impl ReceiptParser { output_param: &ParamType, contract_id: &ContractId, ) -> Option> { + let extra_receipts_needed = output_param.needs_extra_data_receipt(true); match output_param.get_return_location() { ReturnLocation::ReturnData - if output_param.uses_heap_types(false) - && matches!(output_param, ParamType::Enum { .. }) => + if extra_receipts_needed && matches!(output_param, ParamType::Enum { .. }) => { self.extract_enum_heap_type_data(contract_id) } - ReturnLocation::ReturnData if output_param.is_vm_heap_type() => { + ReturnLocation::ReturnData if extra_receipts_needed => { self.extract_return_data_heap(contract_id) } ReturnLocation::ReturnData => self.extract_return_data(contract_id), diff --git a/packages/fuels/tests/types_contracts.rs b/packages/fuels/tests/types_contracts.rs index 4e66593f25..e26a8954c2 100644 --- a/packages/fuels/tests/types_contracts.rs +++ b/packages/fuels/tests/types_contracts.rs @@ -2017,42 +2017,42 @@ async fn test_heap_type_in_enums() -> Result<()> { let resp = contract_methods.returns_bytes_result(true).call().await?; let expected = Ok(Bytes(vec![1, 1, 1, 1])); assert_eq!(resp.value, expected); - let resp = contract_methods.returns_bytes_result(false).call().await?; - let expected = Err(TestError::Something([255u8, 255u8, 255u8, 255u8, 255u8])); - assert_eq!(resp.value, expected); - - let resp = contract_methods.returns_vec_result(true).call().await?; - let expected = Ok(vec![2, 2, 2, 2, 2]); - assert_eq!(resp.value, expected); - let resp = contract_methods.returns_vec_result(false).call().await?; - let expected = Err(TestError::Else(7777)); - assert_eq!(resp.value, expected); - - let resp = contract_methods.returns_string_result(true).call().await?; - let expected = Ok("Hello World".to_string()); - assert_eq!(resp.value, expected); - let resp = contract_methods.returns_string_result(false).call().await?; - let expected = Err(TestError::Else(3333)); - assert_eq!(resp.value, expected); - - let resp = contract_methods.returns_bytes_option(true).call().await?; - let expected = Some(Bytes(vec![1, 1, 1, 1])); - assert_eq!(resp.value, expected); - let resp = contract_methods.returns_bytes_option(false).call().await?; - assert!(resp.value.is_none()); - - let resp = contract_methods.returns_vec_option(true).call().await?; - let expected = Some(vec![2, 2, 2, 2, 2]); - assert_eq!(resp.value, expected); - let resp = contract_methods.returns_vec_option(false).call().await?; - assert!(resp.value.is_none()); - - let resp = contract_methods.returns_string_option(true).call().await?; - let expected = Some("Hello World".to_string()); - assert_eq!(resp.value, expected); - let resp = contract_methods.returns_string_option(false).call().await?; - assert!(resp.value.is_none()); - + // let resp = contract_methods.returns_bytes_result(false).call().await?; + // let expected = Err(TestError::Something([255u8, 255u8, 255u8, 255u8, 255u8])); + // assert_eq!(resp.value, expected); + // + // let resp = contract_methods.returns_vec_result(true).call().await?; + // let expected = Ok(vec![2, 2, 2, 2, 2]); + // assert_eq!(resp.value, expected); + // let resp = contract_methods.returns_vec_result(false).call().await?; + // let expected = Err(TestError::Else(7777)); + // assert_eq!(resp.value, expected); + // + // let resp = contract_methods.returns_string_result(true).call().await?; + // let expected = Ok("Hello World".to_string()); + // assert_eq!(resp.value, expected); + // let resp = contract_methods.returns_string_result(false).call().await?; + // let expected = Err(TestError::Else(3333)); + // assert_eq!(resp.value, expected); + // + // let resp = contract_methods.returns_bytes_option(true).call().await?; + // let expected = Some(Bytes(vec![1, 1, 1, 1])); + // assert_eq!(resp.value, expected); + // let resp = contract_methods.returns_bytes_option(false).call().await?; + // assert!(resp.value.is_none()); + // + // let resp = contract_methods.returns_vec_option(true).call().await?; + // let expected = Some(vec![2, 2, 2, 2, 2]); + // assert_eq!(resp.value, expected); + // let resp = contract_methods.returns_vec_option(false).call().await?; + // assert!(resp.value.is_none()); + // + // let resp = contract_methods.returns_string_option(true).call().await?; + // let expected = Some("Hello World".to_string()); + // assert_eq!(resp.value, expected); + // let resp = contract_methods.returns_string_option(false).call().await?; + // assert!(resp.value.is_none()); + // // If the LW(RET) instruction was not executed only conditionally, then the FuelVM would OOM. let _ = contract_methods .would_raise_a_memory_overflow() From 45ce512f02ed1d49acba7e80d7ff74758b9b0862 Mon Sep 17 00:00:00 2001 From: iqdecay Date: Wed, 13 Sep 2023 16:08:02 +0200 Subject: [PATCH 24/32] test: improve testing for return type validation --- packages/fuels-core/src/types/param_types.rs | 356 ++++++++++++------- packages/fuels-programs/src/call_utils.rs | 5 +- 2 files changed, 222 insertions(+), 139 deletions(-) diff --git a/packages/fuels-core/src/types/param_types.rs b/packages/fuels-core/src/types/param_types.rs index d0cac851db..c1a11dbc88 100644 --- a/packages/fuels-core/src/types/param_types.rs +++ b/packages/fuels-core/src/types/param_types.rs @@ -1,3 +1,4 @@ +use itertools::chain; use std::{collections::HashMap, iter::zip}; use fuel_abi_types::{ @@ -84,12 +85,9 @@ impl ParamType { ParamType::Array(inner, _) | ParamType::Vector(inner) => { inner.needs_extra_data_receipt(false) } - ParamType::Struct { fields, .. } => fields - .iter() + ParamType::Struct { fields, generics } => chain!(fields, generics) .any(|param_type| param_type.needs_extra_data_receipt(false)), - ParamType::Enum { variants, .. } => variants - .param_types() - .iter() + ParamType::Enum { variants, generics } => chain!(variants.param_types(), generics) .any(|param_type| param_type.needs_extra_data_receipt(false)), ParamType::Tuple(inner_types) => inner_types .iter() @@ -100,10 +98,10 @@ impl ParamType { pub fn validate_is_decodable(&self) -> Result<()> { match self { - ParamType::Enum { variants, .. } => { - let grandchildren_need_receipts = variants - .param_types() - .iter() + ParamType::Enum { variants, generics } => { + let all_param_types = chain!(variants.param_types(), generics); + let grandchildren_need_receipts = all_param_types + .clone() .any(|child| child.children_need_extra_receipts()); if grandchildren_need_receipts { return Err(error!( @@ -112,12 +110,9 @@ impl ParamType { )); } - let num_of_children_needing_receipts = variants - .param_types() - .iter() + let num_of_children_needing_receipts = all_param_types .filter(|param_type| param_type.needs_extra_data_receipt(false)) .count(); - if num_of_children_needing_receipts > 1 { Err(error!( InvalidType, @@ -139,12 +134,9 @@ impl ParamType { match self { ParamType::Vector(_) | ParamType::Bytes | ParamType::String => true, ParamType::Array(inner, _) => inner.needs_extra_data_receipt(false), - ParamType::Struct { fields, .. } => fields - .iter() + ParamType::Struct { fields, generics } => chain!(fields, generics) .any(|param_type| param_type.needs_extra_data_receipt(false)), - ParamType::Enum { variants, .. } => variants - .param_types() - .iter() + ParamType::Enum { variants, generics } => chain!(variants.param_types(), generics) .any(|param_type| param_type.needs_extra_data_receipt(false)), ParamType::Tuple(elements) => elements .iter() @@ -1327,126 +1319,220 @@ mod tests { Ok(()) } - // TODO: segfault - // #[test] - // fn contains_nested_heap_types_false_on_simple_types() -> Result<()> { - // // Simple types cannot have nested heap types - // assert!(!ParamType::Unit.contains_nested_heap_types()); - // assert!(!ParamType::U8.contains_nested_heap_types()); - // assert!(!ParamType::U16.contains_nested_heap_types()); - // assert!(!ParamType::U32.contains_nested_heap_types()); - // assert!(!ParamType::U64.contains_nested_heap_types()); - // assert!(!ParamType::Bool.contains_nested_heap_types()); - // assert!(!ParamType::B256.contains_nested_heap_types()); - // assert!(!ParamType::StringArray(10).contains_nested_heap_types()); - // assert!(!ParamType::RawSlice.contains_nested_heap_types()); - // assert!(!ParamType::Bytes.contains_nested_heap_types()); - // assert!(!ParamType::String.contains_nested_heap_types()); - // Ok(()) - // } + #[test] + fn validate_is_decodable_simple_types() -> Result<()> { + assert!(ParamType::U8.validate_is_decodable().is_ok()); + assert!(ParamType::U16.validate_is_decodable().is_ok()); + assert!(ParamType::U32.validate_is_decodable().is_ok()); + assert!(ParamType::U64.validate_is_decodable().is_ok()); + assert!(ParamType::U128.validate_is_decodable().is_ok()); + assert!(ParamType::U256.validate_is_decodable().is_ok()); + assert!(ParamType::Bool.validate_is_decodable().is_ok()); + assert!(ParamType::B256.validate_is_decodable().is_ok()); + assert!(ParamType::Unit.validate_is_decodable().is_ok()); + assert!(ParamType::StringSlice.validate_is_decodable().is_ok()); + assert!(ParamType::StringArray(10).validate_is_decodable().is_ok()); + assert!(ParamType::RawSlice.validate_is_decodable().is_ok()); + assert!(ParamType::Bytes.validate_is_decodable().is_ok()); + assert!(ParamType::String.validate_is_decodable().is_ok()); + Ok(()) + } - // TODO: segfault - // #[test] - // fn test_complex_types_for_nested_heap_types_containing_vectors() -> Result<()> { - // let base_vector = ParamType::Vector(Box::from(ParamType::U8)); - // let param_types_no_nested_vec = vec![ParamType::U64, ParamType::U32]; - // let param_types_nested_vec = vec![ParamType::Unit, ParamType::Bool, base_vector.clone()]; - // - // let is_nested = |param_type: ParamType| assert!(param_type.contains_nested_heap_types()); - // let not_nested = |param_type: ParamType| assert!(!param_type.contains_nested_heap_types()); - // - // not_nested(base_vector.clone()); - // is_nested(ParamType::Vector(Box::from(base_vector.clone()))); - // - // not_nested(ParamType::Array(Box::from(ParamType::U8), 10)); - // is_nested(ParamType::Array(Box::from(base_vector), 10)); - // - // not_nested(ParamType::Tuple(param_types_no_nested_vec.clone())); - // is_nested(ParamType::Tuple(param_types_nested_vec.clone())); - // - // not_nested(ParamType::Struct { - // generics: param_types_no_nested_vec.clone(), - // fields: param_types_no_nested_vec.clone(), - // }); - // is_nested(ParamType::Struct { - // generics: param_types_nested_vec.clone(), - // fields: param_types_no_nested_vec.clone(), - // }); - // is_nested(ParamType::Struct { - // generics: param_types_no_nested_vec.clone(), - // fields: param_types_nested_vec.clone(), - // }); - // - // not_nested(ParamType::Enum { - // variants: EnumVariants::new(param_types_no_nested_vec.clone())?, - // generics: param_types_no_nested_vec.clone(), - // }); - // not_nested(ParamType::Enum { - // variants: EnumVariants::new(param_types_nested_vec.clone())?, - // generics: param_types_no_nested_vec.clone(), - // }); - // not_nested(ParamType::Enum { - // variants: EnumVariants::new(param_types_no_nested_vec)?, - // generics: param_types_nested_vec, - // }); - // Ok(()) - // } + #[test] + fn validate_is_decodable_complex_types_containing_bytes() -> Result<()> { + let param_types_containing_bytes = vec![ParamType::Bytes, ParamType::U64, ParamType::Bool]; + let param_types_no_bytes = vec![ParamType::U64, ParamType::U32]; + let nested_heap_type_error_message = "Invalid type: Nested heap types are currently not \ + supported except in Enums." + .to_string(); + let cannot_be_decoded = |p: ParamType| { + assert_eq!( + p.validate_is_decodable() + .expect_err(&*format!("Should not be decodable: {:?}", p)) + .to_string(), + nested_heap_type_error_message + ) + }; + let can_be_decoded = |p: ParamType| p.validate_is_decodable().is_ok(); - // TODO: segfault - // #[test] - // fn test_complex_types_for_nested_heap_types_containing_bytes() -> Result<()> { - // let base_bytes = ParamType::Bytes; - // let param_types_no_nested_bytes = vec![ParamType::U64, ParamType::U32]; - // let param_types_nested_bytes = vec![ParamType::Unit, ParamType::Bool, base_bytes.clone()]; - // - // let is_nested = |param_type: ParamType| assert!(param_type.contains_nested_heap_types()); - // let not_nested = |param_type: ParamType| assert!(!param_type.contains_nested_heap_types()); - // - // not_nested(base_bytes.clone()); - // is_nested(ParamType::Vector(Box::from(base_bytes.clone()))); - // - // not_nested(ParamType::Array(Box::from(ParamType::U8), 10)); - // is_nested(ParamType::Array(Box::from(base_bytes), 10)); - // - // not_nested(ParamType::Tuple(param_types_no_nested_bytes.clone())); - // is_nested(ParamType::Tuple(param_types_nested_bytes.clone())); - // - // let not_nested_struct = ParamType::Struct { - // generics: param_types_no_nested_bytes.clone(), - // fields: param_types_no_nested_bytes.clone(), - // }; - // not_nested(not_nested_struct); - // - // let nested_struct = ParamType::Struct { - // generics: param_types_nested_bytes.clone(), - // fields: param_types_no_nested_bytes.clone(), - // }; - // is_nested(nested_struct); - // - // let nested_struct = ParamType::Struct { - // generics: param_types_no_nested_bytes.clone(), - // fields: param_types_nested_bytes.clone(), - // }; - // is_nested(nested_struct); - // - // not_nested(ParamType::Enum { - // variants: EnumVariants::new(param_types_no_nested_bytes.clone())?, - // generics: param_types_no_nested_bytes.clone(), - // }); - // - // not_nested(ParamType::Enum { - // variants: EnumVariants::new(param_types_nested_bytes.clone())?, - // generics: param_types_no_nested_bytes.clone(), - // }); - // - // not_nested(ParamType::Enum { - // variants: EnumVariants::new(param_types_no_nested_bytes)?, - // generics: param_types_nested_bytes, - // }); - // - // Ok(()) - // } + can_be_decoded(ParamType::Array(Box::new(ParamType::U64), 10usize)); + cannot_be_decoded(ParamType::Array(Box::new(ParamType::Bytes), 10usize)); + + can_be_decoded(ParamType::Vector(Box::new(ParamType::U64))); + cannot_be_decoded(ParamType::Vector(Box::new(ParamType::Bytes))); + + cannot_be_decoded(ParamType::Struct { + fields: param_types_containing_bytes.clone(), + generics: param_types_no_bytes.clone(), + }); + cannot_be_decoded(ParamType::Struct { + generics: param_types_containing_bytes.clone(), + fields: param_types_no_bytes.clone(), + }); + + can_be_decoded(ParamType::Tuple(param_types_no_bytes.clone())); + cannot_be_decoded(ParamType::Tuple(param_types_containing_bytes.clone())); + + Ok(()) + } + + #[test] + fn validate_is_decodable_enum_containing_bytes() -> Result<()> { + let can_be_decoded = |p: ParamType| p.validate_is_decodable().is_ok(); + let param_types_containing_bytes = vec![ParamType::Bytes, ParamType::U64, ParamType::Bool]; + let param_types_no_bytes = vec![ParamType::U64, ParamType::U32]; + let variants_no_bytes_type = EnumVariants::new(param_types_no_bytes.clone())?; + let variants_one_bytes_type = EnumVariants::new(param_types_containing_bytes.clone())?; + let variants_two_bytes_type = EnumVariants::new(vec![ParamType::Bytes, ParamType::Bytes])?; + can_be_decoded(ParamType::Enum { + variants: variants_no_bytes_type.clone(), + generics: param_types_no_bytes.clone(), + }); + can_be_decoded(ParamType::Enum { + variants: variants_one_bytes_type.clone(), + generics: param_types_no_bytes.clone(), + }); + let expected = "Invalid type: Enums currently support only one heap-type variant. Found: 2" + .to_string(); + assert_eq!( + ParamType::Enum { + variants: variants_two_bytes_type.clone(), + generics: param_types_no_bytes.clone(), + } + .validate_is_decodable() + .expect_err("Should not be decodable") + .to_string(), + expected + ); + can_be_decoded(ParamType::Enum { + variants: variants_no_bytes_type, + generics: param_types_containing_bytes.clone(), + }); + can_be_decoded(ParamType::Enum { + variants: variants_one_bytes_type, + generics: param_types_containing_bytes.clone(), + }); + let expected = "Invalid type: Enums currently support only one heap-type variant. Found: 3" + .to_string(); + assert_eq!( + ParamType::Enum { + variants: variants_two_bytes_type.clone(), + generics: param_types_containing_bytes.clone(), + } + .validate_is_decodable() + .expect_err("Should not be decodable") + .to_string(), + expected + ); + + Ok(()) + } + + #[test] + fn validate_is_decodable_complex_types_containing_vector() -> Result<()> { + let param_types_containing_vector = vec![ + ParamType::Vector(Box::new(ParamType::U32)), + ParamType::U64, + ParamType::Bool, + ]; + let param_types_no_vector = vec![ParamType::U64, ParamType::U32]; + let nested_heap_type_error_message = "Invalid type: Nested heap types are currently not \ + supported except in Enums." + .to_string(); + let cannot_be_decoded = |p: ParamType| { + assert_eq!( + p.validate_is_decodable() + .expect_err(&*format!("Should not be decodable: {:?}", p)) + .to_string(), + nested_heap_type_error_message + ) + }; + let can_be_decoded = |p: ParamType| p.validate_is_decodable().is_ok(); + + can_be_decoded(ParamType::Array(Box::new(ParamType::U64), 10usize)); + cannot_be_decoded(ParamType::Array( + Box::new(ParamType::Vector(Box::new(ParamType::U8))), + 10usize, + )); + + can_be_decoded(ParamType::Vector(Box::new(ParamType::U64))); + cannot_be_decoded(ParamType::Vector(Box::new(ParamType::Vector(Box::new( + ParamType::U8, + ))))); + + cannot_be_decoded(ParamType::Struct { + fields: param_types_containing_vector.clone(), + generics: param_types_no_vector.clone(), + }); + cannot_be_decoded(ParamType::Struct { + generics: param_types_containing_vector.clone(), + fields: param_types_no_vector.clone(), + }); + can_be_decoded(ParamType::Tuple(param_types_no_vector.clone())); + cannot_be_decoded(ParamType::Tuple(param_types_containing_vector.clone())); + + Ok(()) + } + + #[test] + fn validate_is_decodable_enum_containing_vector() -> Result<()> { + let can_be_decoded = |p: ParamType| p.validate_is_decodable().is_ok(); + let param_types_containing_vector = vec![ + ParamType::Vector(Box::new(ParamType::Bool)), + ParamType::U64, + ParamType::Bool, + ]; + let param_types_no_vector = vec![ParamType::U64, ParamType::U32]; + let variants_no_vector_type = EnumVariants::new(param_types_no_vector.clone())?; + let variants_one_vector_type = EnumVariants::new(param_types_containing_vector.clone())?; + let variants_two_vector_type = EnumVariants::new(vec![ + ParamType::Vector(Box::new(ParamType::U8)), + ParamType::Vector(Box::new(ParamType::U16)), + ])?; + can_be_decoded(ParamType::Enum { + variants: variants_no_vector_type.clone(), + generics: param_types_no_vector.clone(), + }); + can_be_decoded(ParamType::Enum { + variants: variants_one_vector_type.clone(), + generics: param_types_no_vector.clone(), + }); + let expected = "Invalid type: Enums currently support only one heap-type variant. Found: 2" + .to_string(); + assert_eq!( + ParamType::Enum { + variants: variants_two_vector_type.clone(), + generics: param_types_no_vector.clone(), + } + .validate_is_decodable() + .expect_err("Should not be decodable") + .to_string(), + expected + ); + can_be_decoded(ParamType::Enum { + variants: variants_no_vector_type, + generics: param_types_containing_vector.clone(), + }); + can_be_decoded(ParamType::Enum { + variants: variants_one_vector_type, + generics: param_types_containing_vector.clone(), + }); + let expected = "Invalid type: Enums currently support only one heap-type variant. Found: 3" + .to_string(); + assert_eq!( + ParamType::Enum { + variants: variants_two_vector_type.clone(), + generics: param_types_containing_vector.clone(), + } + .validate_is_decodable() + .expect_err("Should not be decodable") + .to_string(), + expected + ); + + Ok(()) + } #[test] fn try_vector_is_type_path_backward_compatible() { // TODO: To be removed once https://github.com/FuelLabs/fuels-rs/issues/881 is unblocked. diff --git a/packages/fuels-programs/src/call_utils.rs b/packages/fuels-programs/src/call_utils.rs index efe3ffd999..ef92b8fa6d 100644 --- a/packages/fuels-programs/src/call_utils.rs +++ b/packages/fuels-programs/src/call_utils.rs @@ -355,9 +355,7 @@ pub(crate) fn get_single_call_instructions( fn extract_heap_data(param_type: &ParamType) -> Vec { match param_type { ParamType::Enum { variants, .. } => { - eprintln!("Injecting for enums: start"); let Some((discriminant, heap_type)) = variants.heap_type_variant() else { - eprintln!("Injecting for enums: wasnt a heap type"); return vec![]; }; @@ -373,7 +371,7 @@ fn extract_heap_data(param_type: &ParamType) -> Vec { op::lw(0x18, RegId::RET, 0), // If the discriminant is not the one from the heap type, then jump ahead and // return an empty receipt. Otherwise return heap data with the right length. - // TODO: segfault is this really 3 or 4? Look at spec + // Jump by (last argument + 1) instructions according to specs op::jnef(0x17, 0x18, RegId::ZERO, 3), ], // ================= EXECUTED IF THE DISCRIMINANT POINTS TO A HEAP TYPE @@ -392,7 +390,6 @@ fn extract_data_receipt( top_level_type: bool, param_type: &ParamType, ) -> Vec { - eprintln!("Injecting: extract_data_receipt({ptr_offset}, {top_level_type}, {param_type:?})"); let Some(inner_type_byte_size) = param_type.heap_inner_element_size(top_level_type) else { return vec![]; }; From ba5955c7b5acec0a1bd317fd4803ea745cefe568 Mon Sep 17 00:00:00 2001 From: iqdecay Date: Wed, 13 Sep 2023 16:24:38 +0200 Subject: [PATCH 25/32] reinstate tests --- packages/fuels-core/src/types/param_types.rs | 42 +++-------- packages/fuels/tests/types_contracts.rs | 74 ++++++++++---------- 2 files changed, 46 insertions(+), 70 deletions(-) diff --git a/packages/fuels-core/src/types/param_types.rs b/packages/fuels-core/src/types/param_types.rs index c1a11dbc88..33c726dac5 100644 --- a/packages/fuels-core/src/types/param_types.rs +++ b/packages/fuels-core/src/types/param_types.rs @@ -85,9 +85,12 @@ impl ParamType { ParamType::Array(inner, _) | ParamType::Vector(inner) => { inner.needs_extra_data_receipt(false) } - ParamType::Struct { fields, generics } => chain!(fields, generics) + ParamType::Struct { fields, .. } => fields + .into_iter() .any(|param_type| param_type.needs_extra_data_receipt(false)), - ParamType::Enum { variants, generics } => chain!(variants.param_types(), generics) + ParamType::Enum { variants, .. } => variants + .param_types() + .into_iter() .any(|param_type| param_type.needs_extra_data_receipt(false)), ParamType::Tuple(inner_types) => inner_types .iter() @@ -98,8 +101,8 @@ impl ParamType { pub fn validate_is_decodable(&self) -> Result<()> { match self { - ParamType::Enum { variants, generics } => { - let all_param_types = chain!(variants.param_types(), generics); + ParamType::Enum { variants, .. } => { + let all_param_types = chain!(variants.param_types()); let grandchildren_need_receipts = all_param_types .clone() .any(|child| child.children_need_extra_receipts()); @@ -116,7 +119,8 @@ impl ParamType { if num_of_children_needing_receipts > 1 { Err(error!( InvalidType, - "Enums currently support only one heap-type variant. Found: {num_of_children_needing_receipts}" + "Enums currently support only one heap-type variant. Found: \ + {num_of_children_needing_receipts}" )) } else { Ok(()) @@ -1603,34 +1607,6 @@ mod tests { assert_eq!(param_type, ParamType::String); } - // TODO: segfault - // #[test] - // fn test_double_nested_heap_type_enum() -> Result<()> { - // let enum_no_heap_types = ParamType::Enum { - // variants: EnumVariants::new(vec![ParamType::U64, ParamType::Bool])?, - // generics: vec![], - // }; - // let r = ParamType::Enum { - // variants: EnumVariants::new(vec![ParamType::Bytes, enum_no_heap_types])?, - // generics: vec![], - // }; - // assert!(!r.contains_nested_heap_types()); - // - // let enum_heap_types = ParamType::Enum { - // variants: EnumVariants::new(vec![ - // ParamType::Vector(Box::from(ParamType::U64)), - // ParamType::Bool, - // ])?, - // generics: vec![], - // }; - // let r = ParamType::Enum { - // variants: EnumVariants::new(vec![ParamType::Bytes, enum_heap_types])?, - // generics: vec![], - // }; - // assert!(r.contains_nested_heap_types()); - // Ok(()) - // } - fn given_type_with_path(path: &str) -> Type { Type { type_field: format!("struct {path}"), diff --git a/packages/fuels/tests/types_contracts.rs b/packages/fuels/tests/types_contracts.rs index e26a8954c2..01a504d858 100644 --- a/packages/fuels/tests/types_contracts.rs +++ b/packages/fuels/tests/types_contracts.rs @@ -2017,49 +2017,49 @@ async fn test_heap_type_in_enums() -> Result<()> { let resp = contract_methods.returns_bytes_result(true).call().await?; let expected = Ok(Bytes(vec![1, 1, 1, 1])); assert_eq!(resp.value, expected); - // let resp = contract_methods.returns_bytes_result(false).call().await?; - // let expected = Err(TestError::Something([255u8, 255u8, 255u8, 255u8, 255u8])); - // assert_eq!(resp.value, expected); - // - // let resp = contract_methods.returns_vec_result(true).call().await?; - // let expected = Ok(vec![2, 2, 2, 2, 2]); - // assert_eq!(resp.value, expected); - // let resp = contract_methods.returns_vec_result(false).call().await?; - // let expected = Err(TestError::Else(7777)); - // assert_eq!(resp.value, expected); - // - // let resp = contract_methods.returns_string_result(true).call().await?; - // let expected = Ok("Hello World".to_string()); - // assert_eq!(resp.value, expected); - // let resp = contract_methods.returns_string_result(false).call().await?; - // let expected = Err(TestError::Else(3333)); - // assert_eq!(resp.value, expected); - // - // let resp = contract_methods.returns_bytes_option(true).call().await?; - // let expected = Some(Bytes(vec![1, 1, 1, 1])); - // assert_eq!(resp.value, expected); - // let resp = contract_methods.returns_bytes_option(false).call().await?; - // assert!(resp.value.is_none()); - // - // let resp = contract_methods.returns_vec_option(true).call().await?; - // let expected = Some(vec![2, 2, 2, 2, 2]); - // assert_eq!(resp.value, expected); - // let resp = contract_methods.returns_vec_option(false).call().await?; - // assert!(resp.value.is_none()); - // - // let resp = contract_methods.returns_string_option(true).call().await?; - // let expected = Some("Hello World".to_string()); - // assert_eq!(resp.value, expected); - // let resp = contract_methods.returns_string_option(false).call().await?; - // assert!(resp.value.is_none()); - // + let resp = contract_methods.returns_bytes_result(false).call().await?; + let expected = Err(TestError::Something([255u8, 255u8, 255u8, 255u8, 255u8])); + assert_eq!(resp.value, expected); + + let resp = contract_methods.returns_vec_result(true).call().await?; + let expected = Ok(vec![2, 2, 2, 2, 2]); + assert_eq!(resp.value, expected); + let resp = contract_methods.returns_vec_result(false).call().await?; + let expected = Err(TestError::Else(7777)); + assert_eq!(resp.value, expected); + + let resp = contract_methods.returns_string_result(true).call().await?; + let expected = Ok("Hello World".to_string()); + assert_eq!(resp.value, expected); + let resp = contract_methods.returns_string_result(false).call().await?; + let expected = Err(TestError::Else(3333)); + assert_eq!(resp.value, expected); + + let resp = contract_methods.returns_bytes_option(true).call().await?; + let expected = Some(Bytes(vec![1, 1, 1, 1])); + assert_eq!(resp.value, expected); + let resp = contract_methods.returns_bytes_option(false).call().await?; + assert!(resp.value.is_none()); + + let resp = contract_methods.returns_vec_option(true).call().await?; + let expected = Some(vec![2, 2, 2, 2, 2]); + assert_eq!(resp.value, expected); + let resp = contract_methods.returns_vec_option(false).call().await?; + assert!(resp.value.is_none()); + + let resp = contract_methods.returns_string_option(true).call().await?; + let expected = Some("Hello World".to_string()); + assert_eq!(resp.value, expected); + let resp = contract_methods.returns_string_option(false).call().await?; + assert!(resp.value.is_none()); + // If the LW(RET) instruction was not executed only conditionally, then the FuelVM would OOM. let _ = contract_methods .would_raise_a_memory_overflow() .call() .await?; - // let _ = contract_methods.should_fail().call().await?; + let _ = contract_methods.should_fail().call().await?; Ok(()) } From efe02706c8b05ffe3c48fdd1cfb97ab4091c3e88 Mon Sep 17 00:00:00 2001 From: iqdecay Date: Thu, 14 Sep 2023 15:37:20 +0200 Subject: [PATCH 26/32] fix test for deep heap types --- .../types/contracts/heap_type_in_enums/src/main.sw | 8 +++++++- packages/fuels/tests/types_contracts.rs | 10 ++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/packages/fuels/tests/types/contracts/heap_type_in_enums/src/main.sw b/packages/fuels/tests/types/contracts/heap_type_in_enums/src/main.sw index 7d47186555..69d702fdd3 100644 --- a/packages/fuels/tests/types/contracts/heap_type_in_enums/src/main.sw +++ b/packages/fuels/tests/types/contracts/heap_type_in_enums/src/main.sw @@ -101,6 +101,12 @@ impl MyContract for Contract { } fn should_fail() -> Result { - Result::Err(071) + let mut b = Bytes::new(); + b.push(2u8); + b.push(2u8); + b.push(2u8); + Result::Ok(Bimbam { + something: b + }) } } diff --git a/packages/fuels/tests/types_contracts.rs b/packages/fuels/tests/types_contracts.rs index 2c8cbd8363..0584fef1d8 100644 --- a/packages/fuels/tests/types_contracts.rs +++ b/packages/fuels/tests/types_contracts.rs @@ -2057,7 +2057,13 @@ async fn test_heap_type_in_enums() -> Result<()> { .call() .await?; - // let _ = contract_methods.should_fail().call().await?; - + let resp = contract_methods + .should_fail() + .call() + .await + .expect_err("Should fail because it has a deeply nested heap type"); + let expected = + "Invalid type: Enums currently support only one level deep heap types.".to_string(); + assert_eq!(resp.to_string(), expected); Ok(()) } From c92c53ac78d6cd9995c1648a78a640af8c46fda4 Mon Sep 17 00:00:00 2001 From: iqdecay Date: Thu, 14 Sep 2023 15:40:16 +0200 Subject: [PATCH 27/32] style: appease clippy and all formatting gods --- packages/fuels-core/src/types/param_types.rs | 8 ++++---- .../types/contracts/heap_type_in_enums/src/main.sw | 12 +++++------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/packages/fuels-core/src/types/param_types.rs b/packages/fuels-core/src/types/param_types.rs index 9aa22da650..f3d834b149 100644 --- a/packages/fuels-core/src/types/param_types.rs +++ b/packages/fuels-core/src/types/param_types.rs @@ -90,11 +90,11 @@ impl ParamType { inner.needs_extra_data_receipt(false) } ParamType::Struct { fields, .. } => fields - .into_iter() + .iter() .any(|param_type| param_type.needs_extra_data_receipt(false)), ParamType::Enum { variants, .. } => variants .param_types() - .into_iter() + .iter() .any(|param_type| param_type.needs_extra_data_receipt(false)), ParamType::Tuple(inner_types) => inner_types .iter() @@ -1357,7 +1357,7 @@ mod tests { let cannot_be_decoded = |p: ParamType| { assert_eq!( p.validate_is_decodable() - .expect_err(&*format!("Should not be decodable: {:?}", p)) + .expect_err(&format!("Should not be decodable: {:?}", p)) .to_string(), nested_heap_type_error_message ) @@ -1451,7 +1451,7 @@ mod tests { let cannot_be_decoded = |p: ParamType| { assert_eq!( p.validate_is_decodable() - .expect_err(&*format!("Should not be decodable: {:?}", p)) + .expect_err(&format!("Should not be decodable: {:?}", p)) .to_string(), nested_heap_type_error_message ) diff --git a/packages/fuels/tests/types/contracts/heap_type_in_enums/src/main.sw b/packages/fuels/tests/types/contracts/heap_type_in_enums/src/main.sw index 69d702fdd3..ecd9a8c590 100644 --- a/packages/fuels/tests/types/contracts/heap_type_in_enums/src/main.sw +++ b/packages/fuels/tests/types/contracts/heap_type_in_enums/src/main.sw @@ -101,12 +101,10 @@ impl MyContract for Contract { } fn should_fail() -> Result { - let mut b = Bytes::new(); - b.push(2u8); - b.push(2u8); - b.push(2u8); - Result::Ok(Bimbam { - something: b - }) + let mut b = Bytes::new(); + b.push(2u8); + b.push(2u8); + b.push(2u8); + Result::Ok(Bimbam { something: b }) } } From 14789b482ef203c4d15e6bb68831f585b8fd0e6d Mon Sep 17 00:00:00 2001 From: iqdecay Date: Thu, 14 Sep 2023 15:42:07 +0200 Subject: [PATCH 28/32] style --- .../tests/types/contracts/heap_type_in_enums/src/main.sw | 4 ++-- packages/fuels/tests/types_contracts.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/fuels/tests/types/contracts/heap_type_in_enums/src/main.sw b/packages/fuels/tests/types/contracts/heap_type_in_enums/src/main.sw index ecd9a8c590..92f32c8d80 100644 --- a/packages/fuels/tests/types/contracts/heap_type_in_enums/src/main.sw +++ b/packages/fuels/tests/types/contracts/heap_type_in_enums/src/main.sw @@ -20,7 +20,7 @@ abi MyContract { fn returns_vec_option(return_some: bool) -> Option>; fn returns_string_option(return_some: bool) -> Option; fn would_raise_a_memory_overflow() -> Result; - fn should_fail() -> Result; + fn returns_a_heap_type_too_deep() -> Result; } impl MyContract for Contract { @@ -100,7 +100,7 @@ impl MyContract for Contract { Result::Err(0x1111111111111111111111111111111111111111111111111111111111111111) } - fn should_fail() -> Result { + fn returns_a_heap_type_too_deep() -> Result { let mut b = Bytes::new(); b.push(2u8); b.push(2u8); diff --git a/packages/fuels/tests/types_contracts.rs b/packages/fuels/tests/types_contracts.rs index 0584fef1d8..a50ea9c913 100644 --- a/packages/fuels/tests/types_contracts.rs +++ b/packages/fuels/tests/types_contracts.rs @@ -2058,7 +2058,7 @@ async fn test_heap_type_in_enums() -> Result<()> { .await?; let resp = contract_methods - .should_fail() + .returns_a_heap_type_too_deep() .call() .await .expect_err("Should fail because it has a deeply nested heap type"); From 3063c18b413191245d8e109a5a3a798586acf0c5 Mon Sep 17 00:00:00 2001 From: iqdecay Date: Thu, 14 Sep 2023 16:47:28 +0200 Subject: [PATCH 29/32] test: fix tests --- packages/fuels-core/src/types/param_types.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/fuels-core/src/types/param_types.rs b/packages/fuels-core/src/types/param_types.rs index f3d834b149..fcac539eee 100644 --- a/packages/fuels-core/src/types/param_types.rs +++ b/packages/fuels-core/src/types/param_types.rs @@ -1370,13 +1370,13 @@ mod tests { can_be_decoded(ParamType::Vector(Box::new(ParamType::U64))); cannot_be_decoded(ParamType::Vector(Box::new(ParamType::Bytes))); - cannot_be_decoded(ParamType::Struct { - fields: param_types_containing_bytes.clone(), + can_be_decoded(ParamType::Struct { generics: param_types_no_bytes.clone(), + fields: param_types_no_bytes.clone(), }); cannot_be_decoded(ParamType::Struct { - generics: param_types_containing_bytes.clone(), - fields: param_types_no_bytes.clone(), + fields: param_types_containing_bytes.clone(), + generics: param_types_no_bytes.clone(), }); can_be_decoded(ParamType::Tuple(param_types_no_bytes.clone())); @@ -1421,7 +1421,7 @@ mod tests { variants: variants_one_bytes_type, generics: param_types_containing_bytes.clone(), }); - let expected = "Invalid type: Enums currently support only one heap-type variant. Found: 3" + let expected = "Invalid type: Enums currently support only one heap-type variant. Found: 2" .to_string(); assert_eq!( ParamType::Enum { @@ -1469,13 +1469,13 @@ mod tests { ParamType::U8, ))))); - cannot_be_decoded(ParamType::Struct { - fields: param_types_containing_vector.clone(), + can_be_decoded(ParamType::Struct { + fields: param_types_no_vector.clone(), generics: param_types_no_vector.clone(), }); cannot_be_decoded(ParamType::Struct { - generics: param_types_containing_vector.clone(), - fields: param_types_no_vector.clone(), + generics: param_types_no_vector.clone(), + fields: param_types_containing_vector.clone(), }); can_be_decoded(ParamType::Tuple(param_types_no_vector.clone())); @@ -1527,7 +1527,7 @@ mod tests { variants: variants_one_vector_type, generics: param_types_containing_vector.clone(), }); - let expected = "Invalid type: Enums currently support only one heap-type variant. Found: 3" + let expected = "Invalid type: Enums currently support only one heap-type variant. Found: 2" .to_string(); assert_eq!( ParamType::Enum { From 6b138f597c1d02d99fd4248cf551db172802aaf4 Mon Sep 17 00:00:00 2001 From: iqdecay Date: Tue, 3 Oct 2023 14:50:39 +0200 Subject: [PATCH 30/32] Update packages/fuels-core/src/types/enum_variants.rs Co-authored-by: Ahmed Sagdati <37515857+segfault-magnet@users.noreply.github.com> --- packages/fuels-core/src/types/enum_variants.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/fuels-core/src/types/enum_variants.rs b/packages/fuels-core/src/types/enum_variants.rs index f82e40bcc2..9b39497828 100644 --- a/packages/fuels-core/src/types/enum_variants.rs +++ b/packages/fuels-core/src/types/enum_variants.rs @@ -34,8 +34,6 @@ impl EnumVariants { } pub fn heap_type_variant(&self) -> Option<(u8, &ParamType)> { - // TODO: segfault this needs to look at only the parent heap types. Or maybe to extract - // this from here because this limitation is not present when enums are used as args. self.param_types() .iter() .enumerate() From 3869d08738cbebed84dbddaad21c2c332b530186 Mon Sep 17 00:00:00 2001 From: iqdecay Date: Tue, 3 Oct 2023 14:51:18 +0200 Subject: [PATCH 31/32] Update packages/fuels-core/src/types/param_types.rs Co-authored-by: Ahmed Sagdati <37515857+segfault-magnet@users.noreply.github.com> --- packages/fuels-core/src/types/param_types.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/fuels-core/src/types/param_types.rs b/packages/fuels-core/src/types/param_types.rs index fcac539eee..f82f7e7df6 100644 --- a/packages/fuels-core/src/types/param_types.rs +++ b/packages/fuels-core/src/types/param_types.rs @@ -106,9 +106,9 @@ impl ParamType { pub fn validate_is_decodable(&self) -> Result<()> { match self { ParamType::Enum { variants, .. } => { - let all_param_types = chain!(variants.param_types()); + let all_param_types = variants.param_types(); let grandchildren_need_receipts = all_param_types - .clone() + .iter() .any(|child| child.children_need_extra_receipts()); if grandchildren_need_receipts { return Err(error!( @@ -118,6 +118,7 @@ impl ParamType { } let num_of_children_needing_receipts = all_param_types + .iter() .filter(|param_type| param_type.needs_extra_data_receipt(false)) .count(); if num_of_children_needing_receipts > 1 { From 02d07ca43dd8b15142f5b8d9e6cbfd87f3d7c854 Mon Sep 17 00:00:00 2001 From: iqdecay Date: Wed, 4 Oct 2023 22:13:56 +0200 Subject: [PATCH 32/32] rename function for extra receipts to avoid sounding like a setter --- .../fuels-core/src/types/enum_variants.rs | 2 +- packages/fuels-core/src/types/param_types.rs | 23 ++++++++++--------- packages/fuels-programs/src/contract.rs | 4 ++-- packages/fuels-programs/src/receipt_parser.rs | 2 +- 4 files changed, 16 insertions(+), 15 deletions(-) diff --git a/packages/fuels-core/src/types/enum_variants.rs b/packages/fuels-core/src/types/enum_variants.rs index 9b39497828..a709c11d15 100644 --- a/packages/fuels-core/src/types/enum_variants.rs +++ b/packages/fuels-core/src/types/enum_variants.rs @@ -37,7 +37,7 @@ impl EnumVariants { self.param_types() .iter() .enumerate() - .find_map(|(d, p)| p.needs_extra_data_receipt(false).then_some((d as u8, p))) + .find_map(|(d, p)| p.is_extra_receipt_needed(false).then_some((d as u8, p))) } pub fn only_units_inside(&self) -> bool { diff --git a/packages/fuels-core/src/types/param_types.rs b/packages/fuels-core/src/types/param_types.rs index f82f7e7df6..7818e19449 100644 --- a/packages/fuels-core/src/types/param_types.rs +++ b/packages/fuels-core/src/types/param_types.rs @@ -87,18 +87,18 @@ impl ParamType { pub fn children_need_extra_receipts(&self) -> bool { match self { ParamType::Array(inner, _) | ParamType::Vector(inner) => { - inner.needs_extra_data_receipt(false) + inner.is_extra_receipt_needed(false) } ParamType::Struct { fields, .. } => fields .iter() - .any(|param_type| param_type.needs_extra_data_receipt(false)), + .any(|param_type| param_type.is_extra_receipt_needed(false)), ParamType::Enum { variants, .. } => variants .param_types() .iter() - .any(|param_type| param_type.needs_extra_data_receipt(false)), + .any(|param_type| param_type.is_extra_receipt_needed(false)), ParamType::Tuple(inner_types) => inner_types .iter() - .any(|param_type| param_type.needs_extra_data_receipt(false)), + .any(|param_type| param_type.is_extra_receipt_needed(false)), _ => false, } } @@ -119,7 +119,7 @@ impl ParamType { let num_of_children_needing_receipts = all_param_types .iter() - .filter(|param_type| param_type.needs_extra_data_receipt(false)) + .filter(|param_type| param_type.is_extra_receipt_needed(false)) .count(); if num_of_children_needing_receipts > 1 { Err(error!( @@ -139,17 +139,18 @@ impl ParamType { } } - pub fn needs_extra_data_receipt(&self, top_level_type: bool) -> bool { + pub fn is_extra_receipt_needed(&self, top_level_type: bool) -> bool { match self { ParamType::Vector(_) | ParamType::Bytes | ParamType::String => true, - ParamType::Array(inner, _) => inner.needs_extra_data_receipt(false), - ParamType::Struct { fields, generics } => chain!(fields, generics) - .any(|param_type| param_type.needs_extra_data_receipt(false)), + ParamType::Array(inner, _) => inner.is_extra_receipt_needed(false), + ParamType::Struct { fields, generics } => { + chain!(fields, generics).any(|param_type| param_type.is_extra_receipt_needed(false)) + } ParamType::Enum { variants, generics } => chain!(variants.param_types(), generics) - .any(|param_type| param_type.needs_extra_data_receipt(false)), + .any(|param_type| param_type.is_extra_receipt_needed(false)), ParamType::Tuple(elements) => elements .iter() - .any(|param_type| param_type.needs_extra_data_receipt(false)), + .any(|param_type| param_type.is_extra_receipt_needed(false)), ParamType::RawSlice => !top_level_type, _ => false, } diff --git a/packages/fuels-programs/src/contract.rs b/packages/fuels-programs/src/contract.rs index 628d78611d..d42a7a5b34 100644 --- a/packages/fuels-programs/src/contract.rs +++ b/packages/fuels-programs/src/contract.rs @@ -826,7 +826,7 @@ impl MultiContractCallHandler { let number_of_heap_type_calls = self .contract_calls .iter() - .filter(|cc| cc.output_param.needs_extra_data_receipt(true)) + .filter(|cc| cc.output_param.is_extra_receipt_needed(true)) .count(); match number_of_heap_type_calls { @@ -837,7 +837,7 @@ impl MultiContractCallHandler { .last() .expect("is not empty") .output_param - .needs_extra_data_receipt(true) + .is_extra_receipt_needed(true) { Ok(()) } else { diff --git a/packages/fuels-programs/src/receipt_parser.rs b/packages/fuels-programs/src/receipt_parser.rs index 1267806809..7f7393764b 100644 --- a/packages/fuels-programs/src/receipt_parser.rs +++ b/packages/fuels-programs/src/receipt_parser.rs @@ -64,7 +64,7 @@ impl ReceiptParser { output_param: &ParamType, contract_id: &ContractId, ) -> Option> { - let extra_receipts_needed = output_param.needs_extra_data_receipt(true); + let extra_receipts_needed = output_param.is_extra_receipt_needed(true); match output_param.get_return_location() { ReturnLocation::ReturnData if extra_receipts_needed && matches!(output_param, ParamType::Enum { .. }) =>