From 64cc4b94c11003133feb78d9b55ee6416ad9b285 Mon Sep 17 00:00:00 2001 From: Sophie Date: Tue, 28 Nov 2023 14:59:42 -0500 Subject: [PATCH 1/8] Repro of storage slot deserialization bug --- fuel-tx/src/transaction/types/storage.rs | 43 ++++++++++++++++++------ 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/fuel-tx/src/transaction/types/storage.rs b/fuel-tx/src/transaction/types/storage.rs index fcdc082d5f..7b67853d9e 100644 --- a/fuel-tx/src/transaction/types/storage.rs +++ b/fuel-tx/src/transaction/types/storage.rs @@ -1,18 +1,11 @@ use fuel_types::{ - canonical::{ - Deserialize, - Serialize, - }, - Bytes32, - Bytes64, + canonical::{Deserialize, Serialize}, + Bytes32, Bytes64, }; #[cfg(feature = "random")] use rand::{ - distributions::{ - Distribution, - Standard, - }, + distributions::{Distribution, Standard}, Rng, }; @@ -84,3 +77,33 @@ impl Ord for StorageSlot { self.key.cmp(&other.key) } } + +#[test] +#[cfg(feature = "serde")] +fn test_storage_slot_serialization() { + use std::fs::File; + use std::path::PathBuf; + + use rand::SeedableRng; + + let rng = &mut rand::rngs::StdRng::seed_from_u64(8586); + + let key: Bytes32 = rng.gen(); + let value: Bytes32 = rng.gen(); + + let slot = StorageSlot::new(key, value); + let slots = vec![slot.clone()]; + + // writes to file + let storage_slots_file = + File::create(PathBuf::from("storage-slots.json")).expect("create file"); + let res = serde_json::to_writer(&storage_slots_file, &slots).expect("write file"); + + // this fails + let storage_slots_file = + std::fs::File::open(PathBuf::from("storage-slots.json")).expect("open file"); + let storage_slots: Vec = + serde_json::from_reader(storage_slots_file).expect("read file"); + + assert_eq!(storage_slots.len(), 1); +} From 2fbd1fe2efff84530d1c70daf2c4229f6eb7aab9 Mon Sep 17 00:00:00 2001 From: Sophie Date: Tue, 28 Nov 2023 15:07:16 -0500 Subject: [PATCH 2/8] from string works --- fuel-tx/src/transaction/types/storage.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/fuel-tx/src/transaction/types/storage.rs b/fuel-tx/src/transaction/types/storage.rs index 7b67853d9e..4904889721 100644 --- a/fuel-tx/src/transaction/types/storage.rs +++ b/fuel-tx/src/transaction/types/storage.rs @@ -99,11 +99,16 @@ fn test_storage_slot_serialization() { File::create(PathBuf::from("storage-slots.json")).expect("create file"); let res = serde_json::to_writer(&storage_slots_file, &slots).expect("write file"); + // from string works + let slot_str = serde_json::to_string(&slots).expect("to string"); + let storage_slots: Vec = + serde_json::from_str(&slot_str).expect("read from string"); + assert_eq!(storage_slots.len(), 1); + // this fails let storage_slots_file = std::fs::File::open(PathBuf::from("storage-slots.json")).expect("open file"); let storage_slots: Vec = serde_json::from_reader(storage_slots_file).expect("read file"); - assert_eq!(storage_slots.len(), 1); } From 1072189f48e9f8fe2548d4745f57c34224367578 Mon Sep 17 00:00:00 2001 From: xgreenx Date: Tue, 28 Nov 2023 20:58:27 +0000 Subject: [PATCH 3/8] Fix compilation --- fuel-tx/Cargo.toml | 1 + fuel-tx/src/transaction/types/storage.rs | 22 +++++++++++++++------- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/fuel-tx/Cargo.toml b/fuel-tx/Cargo.toml index 75604fd5d6..0bfbbea2ef 100644 --- a/fuel-tx/Cargo.toml +++ b/fuel-tx/Cargo.toml @@ -38,6 +38,7 @@ quickcheck = "1.0" quickcheck_macros = "1.0" rand = { version = "0.8", default-features = false, features = ["std_rng"] } rstest = "0.15" +serde_json = { version = "1.0" } [features] default = ["fuel-asm/default", "fuel-crypto/default", "fuel-merkle/default", "fuel-types/default", "std"] diff --git a/fuel-tx/src/transaction/types/storage.rs b/fuel-tx/src/transaction/types/storage.rs index 4904889721..c601d7d922 100644 --- a/fuel-tx/src/transaction/types/storage.rs +++ b/fuel-tx/src/transaction/types/storage.rs @@ -1,11 +1,18 @@ use fuel_types::{ - canonical::{Deserialize, Serialize}, - Bytes32, Bytes64, + canonical::{ + Deserialize, + Serialize, + }, + Bytes32, + Bytes64, }; #[cfg(feature = "random")] use rand::{ - distributions::{Distribution, Standard}, + distributions::{ + Distribution, + Standard, + }, Rng, }; @@ -79,10 +86,11 @@ impl Ord for StorageSlot { } #[test] -#[cfg(feature = "serde")] fn test_storage_slot_serialization() { - use std::fs::File; - use std::path::PathBuf; + use std::{ + fs::File, + path::PathBuf, + }; use rand::SeedableRng; @@ -97,7 +105,7 @@ fn test_storage_slot_serialization() { // writes to file let storage_slots_file = File::create(PathBuf::from("storage-slots.json")).expect("create file"); - let res = serde_json::to_writer(&storage_slots_file, &slots).expect("write file"); + serde_json::to_writer(&storage_slots_file, &slots).expect("write file"); // from string works let slot_str = serde_json::to_string(&slots).expect("to string"); From 044c8bd96fc8fea25b2188bf01b0d2822856e22d Mon Sep 17 00:00:00 2001 From: xgreenx Date: Tue, 28 Nov 2023 21:00:09 +0000 Subject: [PATCH 4/8] Fix compilation --- fuel-tx/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fuel-tx/Cargo.toml b/fuel-tx/Cargo.toml index 0bfbbea2ef..71e6510ac7 100644 --- a/fuel-tx/Cargo.toml +++ b/fuel-tx/Cargo.toml @@ -29,7 +29,7 @@ strum_macros = { version = "0.24", optional = true } [dev-dependencies] bincode = { workspace = true } fuel-crypto = { workspace = true, default-features = false, features = ["random"] } -fuel-tx = { path = ".", features = ["builder", "random"] } +fuel-tx = { path = ".", features = ["builder", "random", "serde"] } fuel-tx-test-helpers = { path = "test-helpers" } fuel-types = { workspace = true, default-features = false, features = ["random"] } hex = { version = "0.4", default-features = false } From e1b72612f3791192d28b5052413297594e28aaa4 Mon Sep 17 00:00:00 2001 From: xgreenx Date: Tue, 28 Nov 2023 21:23:46 +0000 Subject: [PATCH 5/8] Fix the issue with deserialization --- fuel-tx/src/transaction/types/storage.rs | 18 +++++++++--------- fuel-types/src/array_types.rs | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/fuel-tx/src/transaction/types/storage.rs b/fuel-tx/src/transaction/types/storage.rs index c601d7d922..3dd6849461 100644 --- a/fuel-tx/src/transaction/types/storage.rs +++ b/fuel-tx/src/transaction/types/storage.rs @@ -102,20 +102,20 @@ fn test_storage_slot_serialization() { let slot = StorageSlot::new(key, value); let slots = vec![slot.clone()]; - // writes to file - let storage_slots_file = - File::create(PathBuf::from("storage-slots.json")).expect("create file"); - serde_json::to_writer(&storage_slots_file, &slots).expect("write file"); - - // from string works + // `from_str` works let slot_str = serde_json::to_string(&slots).expect("to string"); let storage_slots: Vec = serde_json::from_str(&slot_str).expect("read from string"); assert_eq!(storage_slots.len(), 1); - // this fails - let storage_slots_file = - std::fs::File::open(PathBuf::from("storage-slots.json")).expect("open file"); + let path = std::env::temp_dir().join(PathBuf::from("storage-slots.json")); + + // writes to file works + let storage_slots_file = File::create(&path).expect("create file"); + serde_json::to_writer(&storage_slots_file, &slots).expect("write file"); + + // `from_reader` works + let storage_slots_file = File::open(&path).expect("open file"); let storage_slots: Vec = serde_json::from_reader(storage_slots_file).expect("read file"); assert_eq!(storage_slots.len(), 1); diff --git a/fuel-types/src/array_types.rs b/fuel-types/src/array_types.rs index 7fdba1c0c1..0ce1d1fd0f 100644 --- a/fuel-types/src/array_types.rs +++ b/fuel-types/src/array_types.rs @@ -334,7 +334,7 @@ macro_rules! key_methods { { use serde::de::Error; if deserializer.is_human_readable() { - let s: &str = serde::Deserialize::deserialize(deserializer)?; + let s: String = serde::Deserialize::deserialize(deserializer)?; s.parse().map_err(D::Error::custom) } else { let bytes = deserializer.deserialize_bytes(BytesVisitor {})?; From 40b0828d600bd9bd3218bbc75ec625ba769c7da5 Mon Sep 17 00:00:00 2001 From: xgreenx Date: Tue, 28 Nov 2023 21:25:35 +0000 Subject: [PATCH 6/8] Fixed the issue with deseriaklization form the file --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5afd2899bb..92d1c7c5fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] +### Fixed + +- [#643](https://github.com/FuelLabs/fuel-vm/pull/643): Fixed json deserialization of array fuel types from the file. + ## [Version 0.43.0] ### Changed From d1a6954327e2ea82de7592c70bc52c53142a3bbe Mon Sep 17 00:00:00 2001 From: Sophie Date: Tue, 28 Nov 2023 16:55:49 -0500 Subject: [PATCH 7/8] clean up after test --- fuel-tx/src/transaction/types/storage.rs | 62 +++++++++++++++--------- 1 file changed, 39 insertions(+), 23 deletions(-) diff --git a/fuel-tx/src/transaction/types/storage.rs b/fuel-tx/src/transaction/types/storage.rs index 3dd6849461..88431e3a22 100644 --- a/fuel-tx/src/transaction/types/storage.rs +++ b/fuel-tx/src/transaction/types/storage.rs @@ -85,38 +85,54 @@ impl Ord for StorageSlot { } } -#[test] -fn test_storage_slot_serialization() { +#[cfg(test)] +mod tests { + use super::*; + use rand::SeedableRng; use std::{ - fs::File, + fs::{ + remove_file, + File, + }, path::PathBuf, }; - use rand::SeedableRng; + const FILE_PATH: &str = "storage-slots.json"; + + struct Cleanup; + impl Drop for Cleanup { + fn drop(&mut self) { + remove_file(FILE_PATH).expect("remove file"); + } + } - let rng = &mut rand::rngs::StdRng::seed_from_u64(8586); + #[test] + fn test_storage_slot_serialization() { + let _ = Cleanup; - let key: Bytes32 = rng.gen(); - let value: Bytes32 = rng.gen(); + let rng = &mut rand::rngs::StdRng::seed_from_u64(8586); + let key: Bytes32 = rng.gen(); + let value: Bytes32 = rng.gen(); - let slot = StorageSlot::new(key, value); - let slots = vec![slot.clone()]; + let slot = StorageSlot::new(key, value); + let slots = vec![slot.clone()]; - // `from_str` works - let slot_str = serde_json::to_string(&slots).expect("to string"); - let storage_slots: Vec = - serde_json::from_str(&slot_str).expect("read from string"); - assert_eq!(storage_slots.len(), 1); + // `from_str` works + let slot_str = serde_json::to_string(&slots).expect("to string"); + let storage_slots: Vec = + serde_json::from_str(&slot_str).expect("read from string"); + assert_eq!(storage_slots.len(), 1); - let path = std::env::temp_dir().join(PathBuf::from("storage-slots.json")); + let path = std::env::temp_dir().join(PathBuf::from(FILE_PATH)); - // writes to file works - let storage_slots_file = File::create(&path).expect("create file"); - serde_json::to_writer(&storage_slots_file, &slots).expect("write file"); + // writes to file works + let storage_slots_file = File::create(&path).expect("create file"); + serde_json::to_writer(&storage_slots_file, &slots).expect("write file"); - // `from_reader` works - let storage_slots_file = File::open(&path).expect("open file"); - let storage_slots: Vec = - serde_json::from_reader(storage_slots_file).expect("read file"); - assert_eq!(storage_slots.len(), 1); + // `from_reader` works + let storage_slots_file = File::open(&path).expect("open file"); + let storage_slots: Vec = + serde_json::from_reader(storage_slots_file).expect("read file"); + assert_eq!(storage_slots.len(), 1); + } } From afe3ac00fbdc7b5cc76f9ef72e45ab5d632471d7 Mon Sep 17 00:00:00 2001 From: xgreenx Date: Tue, 28 Nov 2023 23:09:56 +0000 Subject: [PATCH 8/8] We don't need to clean in temporary folder --- fuel-tx/src/transaction/types/storage.rs | 14 +------------- fuel-types/src/array_types.rs | 3 ++- 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/fuel-tx/src/transaction/types/storage.rs b/fuel-tx/src/transaction/types/storage.rs index 88431e3a22..b9be82e146 100644 --- a/fuel-tx/src/transaction/types/storage.rs +++ b/fuel-tx/src/transaction/types/storage.rs @@ -90,26 +90,14 @@ mod tests { use super::*; use rand::SeedableRng; use std::{ - fs::{ - remove_file, - File, - }, + fs::File, path::PathBuf, }; const FILE_PATH: &str = "storage-slots.json"; - struct Cleanup; - impl Drop for Cleanup { - fn drop(&mut self) { - remove_file(FILE_PATH).expect("remove file"); - } - } - #[test] fn test_storage_slot_serialization() { - let _ = Cleanup; - let rng = &mut rand::rngs::StdRng::seed_from_u64(8586); let key: Bytes32 = rng.gen(); let value: Bytes32 = rng.gen(); diff --git a/fuel-types/src/array_types.rs b/fuel-types/src/array_types.rs index 0ce1d1fd0f..ae351861fd 100644 --- a/fuel-types/src/array_types.rs +++ b/fuel-types/src/array_types.rs @@ -334,7 +334,8 @@ macro_rules! key_methods { { use serde::de::Error; if deserializer.is_human_readable() { - let s: String = serde::Deserialize::deserialize(deserializer)?; + let s: alloc::string::String = + serde::Deserialize::deserialize(deserializer)?; s.parse().map_err(D::Error::custom) } else { let bytes = deserializer.deserialize_bytes(BytesVisitor {})?;