From 486d06f83de6ea3411543f3034700b0157a65eb5 Mon Sep 17 00:00:00 2001 From: Andrea Cerone <22031682+acerone85@users.noreply.github.com> Date: Mon, 11 Nov 2024 10:38:10 +0100 Subject: [PATCH] Add the possibility to specify an offset for the read value in StorageRead::read (#863) * WIP: Change Storage Read trait * Modify StorageRead trait to specify offset as input * Add test for that new StroageRead::read behaves as expected for the Memory implementation * Add changelog * Simplify test * Change implementation to return Some(0) when contract is read at contract boundary * Make StorageRead::read fallible for MemoryStorage * StorageRef::read now takes an offset in input * Better use statements * Newline between versions in changelog --- CHANGELOG.md | 3 + fuel-storage/src/impls.rs | 9 +- fuel-storage/src/lib.rs | 8 +- .../src/interpreter/blockchain/code_tests.rs | 12 +- .../src/interpreter/blockchain/other_tests.rs | 10 +- .../src/interpreter/blockchain/smo_tests.rs | 9 +- fuel-vm/src/interpreter/blockchain/test.rs | 10 +- .../src/interpreter/blockchain/test/scwq.rs | 4 +- .../src/interpreter/blockchain/test/srwq.rs | 3 +- .../src/interpreter/blockchain/test/swwq.rs | 4 +- fuel-vm/src/interpreter/diff/storage.rs | 3 +- fuel-vm/src/interpreter/flow.rs | 2 +- fuel-vm/src/interpreter/flow/tests.rs | 13 +- fuel-vm/src/memory_client.rs | 10 +- fuel-vm/src/storage.rs | 5 +- fuel-vm/src/storage/interpreter.rs | 9 +- fuel-vm/src/storage/memory.rs | 249 +++++++++++++----- fuel-vm/src/storage/predicate.rs | 12 +- 18 files changed, 270 insertions(+), 105 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd232a56ee..4e6dfcfcdb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Fixed - [860](https://github.com/FuelLabs/fuel-vm/pull/860): Fixed missing fuzzing coverage report in CI. +### Breaking +- [863](https://github.com/FuelLabs/fuel-vm/pull/863): Changed StorageRead::read to load a serialized value starting from a offset. The function returns an optional value equal to the number of bytes read when defined, or none if the offset specified in input is outside the boundaries of the serialized value read. + ## [Version 0.58.2] ### Fixed diff --git a/fuel-storage/src/impls.rs b/fuel-storage/src/impls.rs index 1a11aa97a0..0f1517ae4c 100644 --- a/fuel-storage/src/impls.rs +++ b/fuel-storage/src/impls.rs @@ -101,9 +101,10 @@ impl<'a, T: StorageRead + StorageSize + ?Sized, Type: Mappable> fn read( &self, key: &::Key, + offset: usize, buf: &mut [u8], ) -> Result, Self::Error> { - >::read(self, key, buf) + >::read(self, key, offset, buf) } fn read_alloc( @@ -120,9 +121,10 @@ impl<'a, T: StorageRead + StorageSize + ?Sized, Type: Mappable> fn read( &self, key: &::Key, + offset: usize, buf: &mut [u8], ) -> Result, Self::Error> { - >::read(self, key, buf) + >::read(self, key, offset, buf) } fn read_alloc( @@ -199,9 +201,10 @@ impl<'a, T: StorageRead, Type: Mappable> StorageRef<'a, T, Type> { pub fn read( &self, key: &::Key, + offset: usize, buf: &mut [u8], ) -> Result, T::Error> { - self.0.read(key, buf) + self.0.read(key, offset, buf) } #[inline(always)] diff --git a/fuel-storage/src/lib.rs b/fuel-storage/src/lib.rs index 3ad59eb841..d0d6bb7f35 100644 --- a/fuel-storage/src/lib.rs +++ b/fuel-storage/src/lib.rs @@ -127,8 +127,12 @@ pub trait StorageRead: StorageInspect + StorageSize /// /// Returns None if the value does not exist. /// Otherwise, returns the number of bytes read. - fn read(&self, key: &Type::Key, buf: &mut [u8]) - -> Result, Self::Error>; + fn read( + &self, + key: &Type::Key, + offset: usize, + buf: &mut [u8], + ) -> Result, Self::Error>; /// Same as `read` but allocates a new buffer and returns it. /// diff --git a/fuel-vm/src/interpreter/blockchain/code_tests.rs b/fuel-vm/src/interpreter/blockchain/code_tests.rs index 731f0fe51a..c4ff590123 100644 --- a/fuel-vm/src/interpreter/blockchain/code_tests.rs +++ b/fuel-vm/src/interpreter/blockchain/code_tests.rs @@ -1,17 +1,19 @@ #![allow(clippy::cast_possible_truncation)] -use core::convert::Infallible; use alloc::vec; use super::*; use crate::{ interpreter::PanicContext, - storage::MemoryStorage, + storage::{ + MemoryStorage, + MemoryStorageError, + }, }; use fuel_tx::Contract; #[test] -fn test_load_contract_in_script() -> IoResult<(), Infallible> { +fn test_load_contract_in_script() -> IoResult<(), MemoryStorageError> { let mut storage = MemoryStorage::default(); let mut memory: MemoryInstance = vec![1u8; MEM_SIZE].try_into().unwrap(); let mut pc = 4; @@ -70,7 +72,7 @@ fn test_load_contract_in_script() -> IoResult<(), Infallible> { Ok(()) } #[test] -fn test_load_contract_in_call() -> IoResult<(), Infallible> { +fn test_load_contract_in_call() -> IoResult<(), MemoryStorageError> { let mut storage = MemoryStorage::default(); let mut memory: MemoryInstance = vec![1u8; MEM_SIZE].try_into().unwrap(); let mut pc = 4; @@ -130,7 +132,7 @@ fn test_load_contract_in_call() -> IoResult<(), Infallible> { } #[test] -fn test_code_copy() -> IoResult<(), Infallible> { +fn test_code_copy() -> IoResult<(), MemoryStorageError> { let mut storage = MemoryStorage::default(); let mut memory: MemoryInstance = vec![1u8; MEM_SIZE].try_into().unwrap(); let mut cgas = 1000; diff --git a/fuel-vm/src/interpreter/blockchain/other_tests.rs b/fuel-vm/src/interpreter/blockchain/other_tests.rs index 0189b4138c..a59378114a 100644 --- a/fuel-vm/src/interpreter/blockchain/other_tests.rs +++ b/fuel-vm/src/interpreter/blockchain/other_tests.rs @@ -2,8 +2,10 @@ use alloc::vec; -use crate::storage::MemoryStorage; -use core::convert::Infallible; +use crate::storage::{ + MemoryStorage, + MemoryStorageError, +}; use super::*; use crate::interpreter::PanicContext; @@ -25,7 +27,7 @@ fn test_burn( initialize: impl Into>, amount: Word, sub_id: [u8; 32], -) -> IoResult<(), Infallible> { +) -> IoResult<(), MemoryStorageError> { let mut storage = MemoryStorage::default(); let mut memory: MemoryInstance = vec![1u8; MEM_SIZE].try_into().unwrap(); let contract_id = ContractId::from([3u8; 32]); @@ -103,7 +105,7 @@ fn test_mint( initialize: impl Into>, amount: Word, sub_id: [u8; 32], -) -> IoResult<(), Infallible> { +) -> IoResult<(), MemoryStorageError> { let mut storage = MemoryStorage::default(); let mut memory: MemoryInstance = vec![1u8; MEM_SIZE].try_into().unwrap(); let contract_id = ContractId::from([3u8; 32]); diff --git a/fuel-vm/src/interpreter/blockchain/smo_tests.rs b/fuel-vm/src/interpreter/blockchain/smo_tests.rs index 144fdbdef1..05baf37930 100644 --- a/fuel-vm/src/interpreter/blockchain/smo_tests.rs +++ b/fuel-vm/src/interpreter/blockchain/smo_tests.rs @@ -1,7 +1,5 @@ #![allow(clippy::arithmetic_side_effects, clippy::cast_possible_truncation)] -use core::convert::Infallible; - use alloc::{ vec, vec::Vec, @@ -9,7 +7,10 @@ use alloc::{ use crate::{ interpreter::contract::balance as contract_balance, - storage::MemoryStorage, + storage::{ + MemoryStorage, + MemoryStorageError, + }, }; use super::*; @@ -206,7 +207,7 @@ fn test_smo( max_message_data_length, initial_balance, }: Input, -) -> Result> { +) -> Result> { let mut rng = StdRng::seed_from_u64(100); let base_asset_id = rng.gen(); diff --git a/fuel-vm/src/interpreter/blockchain/test.rs b/fuel-vm/src/interpreter/blockchain/test.rs index 7bbe2599a7..e9c3bb962d 100644 --- a/fuel-vm/src/interpreter/blockchain/test.rs +++ b/fuel-vm/src/interpreter/blockchain/test.rs @@ -4,16 +4,14 @@ use alloc::{ vec::Vec, }; -use core::{ - convert::Infallible, - ops::Range, -}; +use core::ops::Range; use crate::{ context::Context, storage::{ ContractsStateData, MemoryStorage, + MemoryStorageError, }, }; use test_case::test_case; @@ -64,7 +62,7 @@ fn test_state_read_word( fp: Word, insert: impl Into>, key: Word, -) -> Result<(Word, Word), RuntimeError> { +) -> Result<(Word, Word), RuntimeError> { let mut storage = MemoryStorage::default(); let mut memory: MemoryInstance = vec![1u8; MEM_SIZE].try_into().unwrap(); memory[0..ContractId::LEN].copy_from_slice(&[3u8; ContractId::LEN][..]); @@ -134,7 +132,7 @@ fn test_state_write_word( fp: Word, insert: bool, key: Word, -) -> Result> { +) -> Result> { let mut storage = MemoryStorage::default(); let mut memory: MemoryInstance = vec![1u8; MEM_SIZE].try_into().unwrap(); memory[0..ContractId::LEN].copy_from_slice(&[3u8; ContractId::LEN][..]); diff --git a/fuel-vm/src/interpreter/blockchain/test/scwq.rs b/fuel-vm/src/interpreter/blockchain/test/scwq.rs index 6c79165c7d..862439b2ee 100644 --- a/fuel-vm/src/interpreter/blockchain/test/scwq.rs +++ b/fuel-vm/src/interpreter/blockchain/test/scwq.rs @@ -9,6 +9,7 @@ use crate::storage::{ ContractsState, ContractsStateData, MemoryStorage, + MemoryStorageError, }; use super::*; @@ -95,7 +96,8 @@ struct SCWQInput { )] fn test_state_clear_qword( input: SCWQInput, -) -> Result<(Vec<([u8; 32], ContractsStateData)>, bool), RuntimeError> { +) -> Result<(Vec<([u8; 32], ContractsStateData)>, bool), RuntimeError> +{ let SCWQInput { input, storage_slots, diff --git a/fuel-vm/src/interpreter/blockchain/test/srwq.rs b/fuel-vm/src/interpreter/blockchain/test/srwq.rs index e1307f8e9c..076b6b6378 100644 --- a/fuel-vm/src/interpreter/blockchain/test/srwq.rs +++ b/fuel-vm/src/interpreter/blockchain/test/srwq.rs @@ -2,6 +2,7 @@ use crate::storage::{ ContractsState, ContractsStateData, MemoryStorage, + MemoryStorageError, }; use super::*; @@ -163,7 +164,7 @@ struct SRWQInput { )] fn test_state_read_qword( input: SRWQInput, -) -> Result<(MemoryInstance, bool), RuntimeError> { +) -> Result<(MemoryInstance, bool), RuntimeError> { let SRWQInput { storage_slots, context, diff --git a/fuel-vm/src/interpreter/blockchain/test/swwq.rs b/fuel-vm/src/interpreter/blockchain/test/swwq.rs index fdc86fee2e..925762e988 100644 --- a/fuel-vm/src/interpreter/blockchain/test/swwq.rs +++ b/fuel-vm/src/interpreter/blockchain/test/swwq.rs @@ -9,6 +9,7 @@ use crate::storage::{ ContractsState, ContractsStateData, MemoryStorage, + MemoryStorageError, }; use super::*; @@ -215,7 +216,8 @@ struct SWWQInput { )] fn test_state_write_qword( input: SWWQInput, -) -> Result<(Vec<([u8; 32], ContractsStateData)>, u64), RuntimeError> { +) -> Result<(Vec<([u8; 32], ContractsStateData)>, u64), RuntimeError> +{ let SWWQInput { input, storage_slots, diff --git a/fuel-vm/src/interpreter/diff/storage.rs b/fuel-vm/src/interpreter/diff/storage.rs index 69adfff6af..f885da0087 100644 --- a/fuel-vm/src/interpreter/diff/storage.rs +++ b/fuel-vm/src/interpreter/diff/storage.rs @@ -369,9 +369,10 @@ where fn read( &self, key: &::Key, + offset: usize, buf: &mut [u8], ) -> Result, Self::Error> { - >::read(&self.0, key, buf) + >::read(&self.0, key, offset, buf) } fn read_alloc( diff --git a/fuel-vm/src/interpreter/flow.rs b/fuel-vm/src/interpreter/flow.rs index 0eeca29f71..2b75aebe3b 100644 --- a/fuel-vm/src/interpreter/flow.rs +++ b/fuel-vm/src/interpreter/flow.rs @@ -638,7 +638,7 @@ where { let bytes_read = storage .storage::() - .read(contract, dst) + .read(contract, 0, dst) .map_err(RuntimeError::Storage)? .ok_or(PanicReason::ContractNotFound)?; if bytes_read != dst.len() { diff --git a/fuel-vm/src/interpreter/flow/tests.rs b/fuel-vm/src/interpreter/flow/tests.rs index d2c770ec2e..7e6c6c0a78 100644 --- a/fuel-vm/src/interpreter/flow/tests.rs +++ b/fuel-vm/src/interpreter/flow/tests.rs @@ -1,13 +1,14 @@ #![allow(clippy::arithmetic_side_effects, clippy::cast_possible_truncation)] -use core::convert::Infallible; - use alloc::{ vec, vec::Vec, }; -use crate::storage::MemoryStorage; +use crate::storage::{ + MemoryStorage, + MemoryStorageError, +}; use super::*; use crate::crypto; @@ -325,7 +326,7 @@ fn mem(set: &[(usize, Vec)]) -> MemoryInstance { ..Default::default() } => using check_output(Err(RuntimeError::Recoverable(PanicReason::NotEnoughBalance))); "Transfer too many coins internally" )] -fn test_prepare_call(input: Input) -> Result> { +fn test_prepare_call(input: Input) -> Result> { let Input { params, mut reg, @@ -395,8 +396,8 @@ fn test_prepare_call(input: Input) -> Result> { } fn check_output( - expected: Result>, -) -> impl FnOnce(Result>) { + expected: Result>, +) -> impl FnOnce(Result>) { move |result| match (expected, result) { (Ok(e), Ok(r)) => { assert_eq!(e.reg, r.reg); diff --git a/fuel-vm/src/memory_client.rs b/fuel-vm/src/memory_client.rs index a1367712ae..8fc8a57ad0 100644 --- a/fuel-vm/src/memory_client.rs +++ b/fuel-vm/src/memory_client.rs @@ -11,10 +11,12 @@ use crate::{ NotSupportedEcal, }, state::StateTransitionRef, - storage::MemoryStorage, + storage::{ + MemoryStorage, + MemoryStorageError, + }, transactor::Transactor, }; -use core::convert::Infallible; use fuel_tx::{ Blob, Create, @@ -103,7 +105,7 @@ where pub fn deploy( &mut self, tx: Checked, - ) -> Result> { + ) -> Result> { self.transactor.deploy(tx) } @@ -111,7 +113,7 @@ where pub fn upgrade( &mut self, tx: Checked, - ) -> Result> { + ) -> Result> { self.transactor.upgrade(tx) } diff --git a/fuel-vm/src/storage.rs b/fuel-vm/src/storage.rs index c59e543769..f31e7ee261 100644 --- a/fuel-vm/src/storage.rs +++ b/fuel-vm/src/storage.rs @@ -33,7 +33,10 @@ pub use interpreter::{ InterpreterStorage, }; #[cfg(feature = "test-helpers")] -pub use memory::MemoryStorage; +pub use memory::{ + MemoryStorage, + MemoryStorageError, +}; #[cfg(feature = "alloc")] use alloc::vec::Vec; diff --git a/fuel-vm/src/storage/interpreter.rs b/fuel-vm/src/storage/interpreter.rs index dfd39924a1..fe1a459587 100644 --- a/fuel-vm/src/storage/interpreter.rs +++ b/fuel-vm/src/storage/interpreter.rs @@ -167,9 +167,13 @@ pub trait InterpreterStorage: fn read_contract( &self, id: &ContractId, + offset: usize, writer: &mut [u8], ) -> Result, Self::DataError> { - Ok(StorageRead::::read(self, id, writer)?.map(|r| r as Word)) + Ok( + StorageRead::::read(self, id, offset, writer)? + .map(|r| r as Word), + ) } /// Append a contract to the chain, provided its identifier. @@ -370,9 +374,10 @@ where fn read_contract( &self, id: &ContractId, + offset: usize, writer: &mut [u8], ) -> Result, Self::DataError> { - ::read_contract(self.deref(), id, writer) + ::read_contract(self.deref(), id, offset, writer) } fn contract_state_range( diff --git a/fuel-vm/src/storage/memory.rs b/fuel-vm/src/storage/memory.rs index 1f1263f16e..4cd9e8b43a 100644 --- a/fuel-vm/src/storage/memory.rs +++ b/fuel-vm/src/storage/memory.rs @@ -1,15 +1,21 @@ #![allow(clippy::cast_possible_truncation)] -use crate::storage::{ - ContractsAssetKey, - ContractsAssets, - ContractsRawCode, - ContractsState, - ContractsStateData, - ContractsStateKey, - InterpreterStorage, - UploadedBytecode, - UploadedBytecodes, +use crate::{ + error::{ + InterpreterError, + RuntimeError, + }, + storage::{ + ContractsAssetKey, + ContractsAssets, + ContractsRawCode, + ContractsState, + ContractsStateData, + ContractsStateKey, + InterpreterStorage, + UploadedBytecode, + UploadedBytecodes, + }, }; use fuel_crypto::Hasher; @@ -47,7 +53,26 @@ use alloc::{ collections::BTreeMap, vec::Vec, }; -use core::convert::Infallible; + +/// Errors arising from accessing the memory storage. +#[derive(Debug, Clone, PartialEq, Eq, derive_more::Display)] +pub enum MemoryStorageError { + /// The offset specified for the serialized value exceeds its length + #[display(fmt = "Offset {_0} is greater than the length of the value {_1}")] + OffsetOutOfBounds(usize, usize), +} + +impl From for RuntimeError { + fn from(e: MemoryStorageError) -> Self { + RuntimeError::Storage(e) + } +} + +impl From for InterpreterError { + fn from(e: MemoryStorageError) -> Self { + InterpreterError::Storage(e) + } +} #[derive(Debug, Default, Clone, PartialEq, Eq)] struct MemoryStorageInner { @@ -202,13 +227,13 @@ impl Default for MemoryStorage { } impl StorageInspect for MemoryStorage { - type Error = Infallible; + type Error = MemoryStorageError; - fn get(&self, key: &ContractId) -> Result>, Infallible> { + fn get(&self, key: &ContractId) -> Result>, Self::Error> { Ok(self.memory.contracts.get(key).map(Cow::Borrowed)) } - fn contains_key(&self, key: &ContractId) -> Result { + fn contains_key(&self, key: &ContractId) -> Result { Ok(self.memory.contracts.contains_key(key)) } } @@ -218,17 +243,21 @@ impl StorageMutate for MemoryStorage { &mut self, key: &ContractId, value: &[u8], - ) -> Result, Infallible> { + ) -> Result, Self::Error> { Ok(self.memory.contracts.insert(*key, value.into())) } - fn take(&mut self, key: &ContractId) -> Result, Infallible> { + fn take(&mut self, key: &ContractId) -> Result, Self::Error> { Ok(self.memory.contracts.remove(key)) } } impl StorageWrite for MemoryStorage { - fn write_bytes(&mut self, key: &ContractId, buf: &[u8]) -> Result { + fn write_bytes( + &mut self, + key: &ContractId, + buf: &[u8], + ) -> Result { let size = buf.len(); self.memory.contracts.insert(*key, Contract::from(buf)); Ok(size) @@ -255,7 +284,7 @@ impl StorageWrite for MemoryStorage { } impl StorageSize for MemoryStorage { - fn size_of_value(&self, key: &ContractId) -> Result, Infallible> { + fn size_of_value(&self, key: &ContractId) -> Result, Self::Error> { Ok(self.memory.contracts.get(key).map(|c| c.as_ref().len())) } } @@ -264,13 +293,30 @@ impl StorageRead for MemoryStorage { fn read( &self, key: &ContractId, + offset: usize, buf: &mut [u8], ) -> Result, Self::Error> { - Ok(self.memory.contracts.get(key).map(|c| { - let len = buf.len().min(c.as_ref().len()); - buf.copy_from_slice(&c.as_ref()[..len]); - len - })) + self.memory + .contracts + .get(key) + .map(|c| { + let contract_len = c.as_ref().len(); + // We need to handle the case where the offset is greater than the length + // of the contract In this case we follow the same + // approach as `copy_from_slice_zero_fill` + if offset > contract_len { + return Err(MemoryStorageError::OffsetOutOfBounds( + offset, + contract_len, + )); + } + let starting_from_offset = &c.as_ref()[offset..]; + let len = buf.len().min(starting_from_offset.len()); + buf[..len].copy_from_slice(&starting_from_offset[..len]); + buf[len..].fill(0); + Ok(len) + }) + .transpose() } fn read_alloc(&self, key: &ContractId) -> Result>, Self::Error> { @@ -279,12 +325,12 @@ impl StorageRead for MemoryStorage { } impl StorageInspect for MemoryStorage { - type Error = Infallible; + type Error = MemoryStorageError; fn get( &self, key: &::Key, - ) -> Result>, Infallible> { + ) -> Result>, Self::Error> { Ok(self .memory .state_transition_bytecodes @@ -295,7 +341,7 @@ impl StorageInspect for MemoryStorage { fn contains_key( &self, key: &::Key, - ) -> Result { + ) -> Result { Ok(self.memory.state_transition_bytecodes.contains_key(key)) } } @@ -305,7 +351,7 @@ impl StorageMutate for MemoryStorage { &mut self, key: &::Key, value: &::Value, - ) -> Result, Infallible> { + ) -> Result, Self::Error> { Ok(self .memory .state_transition_bytecodes @@ -315,25 +361,25 @@ impl StorageMutate for MemoryStorage { fn take( &mut self, key: &::Key, - ) -> Result, Infallible> { + ) -> Result, Self::Error> { Ok(self.memory.state_transition_bytecodes.remove(key)) } } impl StorageInspect for MemoryStorage { - type Error = Infallible; + type Error = MemoryStorageError; fn get( &self, key: &::Key, - ) -> Result>, Infallible> { + ) -> Result>, Self::Error> { Ok(self.memory.balances.get(key).map(Cow::Borrowed)) } fn contains_key( &self, key: &::Key, - ) -> Result { + ) -> Result { Ok(self.memory.balances.contains_key(key)) } } @@ -343,25 +389,25 @@ impl StorageMutate for MemoryStorage { &mut self, key: &::Key, value: &Word, - ) -> Result, Infallible> { + ) -> Result, Self::Error> { Ok(self.memory.balances.insert(*key, *value)) } fn take( &mut self, key: &::Key, - ) -> Result, Infallible> { + ) -> Result, Self::Error> { Ok(self.memory.balances.remove(key)) } } impl StorageInspect for MemoryStorage { - type Error = Infallible; + type Error = MemoryStorageError; fn get( &self, key: &::Key, - ) -> Result::OwnedValue>>, Infallible> + ) -> Result::OwnedValue>>, Self::Error> { Ok(self.memory.contract_state.get(key).map(Cow::Borrowed)) } @@ -369,7 +415,7 @@ impl StorageInspect for MemoryStorage { fn contains_key( &self, key: &::Key, - ) -> Result { + ) -> Result { Ok(self.memory.contract_state.contains_key(key)) } } @@ -379,14 +425,14 @@ impl StorageMutate for MemoryStorage { &mut self, key: &::Key, value: &::Value, - ) -> Result::OwnedValue>, Infallible> { + ) -> Result::OwnedValue>, Self::Error> { Ok(self.memory.contract_state.insert(*key, value.into())) } fn take( &mut self, key: &::Key, - ) -> Result, Infallible> { + ) -> Result, Self::Error> { Ok(self.memory.contract_state.remove(key)) } } @@ -396,7 +442,7 @@ impl StorageWrite for MemoryStorage { &mut self, key: &::Key, buf: &[u8], - ) -> Result { + ) -> Result { let size = buf.len(); self.memory .contract_state @@ -434,7 +480,7 @@ impl StorageSize for MemoryStorage { fn size_of_value( &self, key: &::Key, - ) -> Result, Infallible> { + ) -> Result, Self::Error> { Ok(self .memory .contract_state @@ -447,13 +493,31 @@ impl StorageRead for MemoryStorage { fn read( &self, key: &::Key, + offset: usize, buf: &mut [u8], ) -> Result, Self::Error> { - Ok(self.memory.contract_state.get(key).map(|data| { - let len = buf.len().min(data.as_ref().len()); - buf.copy_from_slice(&data.as_ref()[..len]); - len - })) + self.memory + .contract_state + .get(key) + .map(|data| { + let contract_state_len = data.as_ref().len(); + // We need to handle the case where the offset is greater than the length + // of the serialized ContractState. In this case we follow + // the same approach as `copy_from_slice_zero_fill` and + // fill the input buffer with zeros. + if offset > contract_state_len { + return Err(MemoryStorageError::OffsetOutOfBounds( + offset, + contract_state_len, + )); + } + let starting_from_offset = &data.as_ref()[offset..]; + let len = buf.len().min(starting_from_offset.len()); + buf[..len].copy_from_slice(&starting_from_offset[..len]); + buf[len..].fill(0); + Ok(len) + }) + .transpose() } fn read_alloc( @@ -472,7 +536,7 @@ impl StorageSize for MemoryStorage { fn size_of_value( &self, key: &::Key, - ) -> Result, Infallible> { + ) -> Result, Self::Error> { Ok(self.memory.blobs.get(key).map(|c| c.as_ref().len())) } } @@ -481,13 +545,28 @@ impl StorageRead for MemoryStorage { fn read( &self, key: &::Key, + offset: usize, buf: &mut [u8], ) -> Result, Self::Error> { - Ok(self.memory.blobs.get(key).map(|data| { - let len = buf.len().min(data.as_ref().len()); - buf.copy_from_slice(&data.as_ref()[..len]); - len - })) + self.memory + .blobs + .get(key) + .map(|data| { + let blob_len = data.as_ref().len(); + // We need to handle the case where the offset is greater than the length + // of the serialized ContractState. In this case we follow + // the same approach as `copy_from_slice_zero_fill` and + // fill the input buffer with zeros. + if offset > blob_len { + return Err(MemoryStorageError::OffsetOutOfBounds(offset, blob_len)); + } + let starting_from_offset = &data.as_ref()[offset..]; + let len = buf.len().min(starting_from_offset.len()); + buf[..len].copy_from_slice(&starting_from_offset[..len]); + buf[len..].fill(0); + Ok(len) + }) + .transpose() } fn read_alloc( @@ -499,19 +578,19 @@ impl StorageRead for MemoryStorage { } impl StorageInspect for MemoryStorage { - type Error = Infallible; + type Error = MemoryStorageError; fn get( &self, key: &::Key, - ) -> Result::OwnedValue>>, Infallible> { + ) -> Result::OwnedValue>>, Self::Error> { Ok(self.memory.blobs.get(key).map(Cow::Borrowed)) } fn contains_key( &self, key: &::Key, - ) -> Result { + ) -> Result { Ok(self.memory.blobs.contains_key(key)) } } @@ -521,14 +600,14 @@ impl StorageMutate for MemoryStorage { &mut self, key: &::Key, value: &::Value, - ) -> Result::OwnedValue>, Infallible> { + ) -> Result::OwnedValue>, Self::Error> { Ok(self.memory.blobs.insert(*key, value.into())) } fn take( &mut self, key: &::Key, - ) -> Result, Infallible> { + ) -> Result, Self::Error> { Ok(self.memory.blobs.remove(key)) } } @@ -538,7 +617,7 @@ impl StorageWrite for MemoryStorage { &mut self, key: &::Key, buf: &[u8], - ) -> Result { + ) -> Result { let size = buf.len(); self.memory.blobs.insert(*key, BlobBytes::from(buf)); Ok(size) @@ -573,9 +652,9 @@ impl StorageWrite for MemoryStorage { impl ContractsAssetsStorage for MemoryStorage {} impl InterpreterStorage for MemoryStorage { - type DataError = Infallible; + type DataError = MemoryStorageError; - fn block_height(&self) -> Result { + fn block_height(&self) -> Result { Ok(self.block_height) } @@ -595,11 +674,11 @@ impl InterpreterStorage for MemoryStorage { Ok((GENESIS + (*height as Word * INTERVAL)).0) } - fn block_hash(&self, block_height: BlockHeight) -> Result { + fn block_hash(&self, block_height: BlockHeight) -> Result { Ok(Hasher::hash(block_height.to_be_bytes())) } - fn coinbase(&self) -> Result { + fn coinbase(&self) -> Result { Ok(self.coinbase) } @@ -782,4 +861,52 @@ mod tests { .map(|v| v.map(|v| v.into_owned())) .collect() } + + #[test_case(0, 32 => Ok(Some(32)))] + #[test_case(4, 32 => Ok(Some(28)))] + #[test_case(8, 32 => Ok(Some(24)))] + #[test_case(0, 28 => Ok(Some(28)))] + #[test_case(4, 28 => Ok(Some(28)))] + #[test_case(8, 28 => Ok(Some(24)))] + #[test_case(28, 0 => Ok(Some(0)))] + #[test_case(28, 4 => Ok(Some(4)))] + #[test_case(28, 8 => Ok(Some(4)))] + #[test_case(32, 0 => Ok(Some(0)))] + #[test_case(32, 4 => Ok(Some(0)))] + #[test_case(32, 8 => Ok(Some(0)))] + #[test_case(33, 0 => Err(MemoryStorageError::OffsetOutOfBounds(33,32)))] + #[test_case(33, 4 => Err(MemoryStorageError::OffsetOutOfBounds(33,32)))] + #[test_case(33, 8 => Err(MemoryStorageError::OffsetOutOfBounds(33,32)))] + fn test_contract_read( + offset: usize, + load_buf_size: usize, + ) -> Result, MemoryStorageError> { + // Given + let raw_contract = [1u8; 32]; + let mut mem = MemoryStorage::default(); + let contract = Contract::from(raw_contract.as_ref()); + mem.memory + .contracts + .insert(ContractId::default(), contract.clone()); + let mut buf: Vec = Vec::with_capacity(load_buf_size); + (0..load_buf_size).for_each(|_| buf.push(0)); + + // When + let bytes_read = StorageRead::::read( + &mem, + &ContractId::default(), + offset, + &mut buf, + ); + + // Then + + if let Ok(bytes_read) = bytes_read { + let contract_bytes_in_buffer = bytes_read.unwrap_or(0); + assert!(buf[0..contract_bytes_in_buffer].iter().all(|&v| v == 1)); + assert!(buf[contract_bytes_in_buffer..].iter().all(|&v| v == 0)); + } + + bytes_read + } } diff --git a/fuel-vm/src/storage/predicate.rs b/fuel-vm/src/storage/predicate.rs index 81373e8ed5..042521e090 100644 --- a/fuel-vm/src/storage/predicate.rs +++ b/fuel-vm/src/storage/predicate.rs @@ -139,7 +139,12 @@ impl StorageSize for EmptyStorage { } impl StorageRead for EmptyStorage { - fn read(&self, _: &BlobId, _: &mut [u8]) -> Result, Self::Error> { + fn read( + &self, + _: &BlobId, + _: usize, + _: &mut [u8], + ) -> Result, Self::Error> { Err(Self::Error::UnsupportedStorageOperation) } @@ -241,6 +246,7 @@ impl StorageRead for PredicateStorage { fn read( &self, _key: &::Key, + _offset: usize, _buf: &mut [u8], ) -> Result, Self::Error> { Err(Self::Error::UnsupportedStorageOperation) @@ -292,6 +298,7 @@ impl StorageRead for PredicateStorage { fn read( &self, _key: &::Key, + _offset: usize, _buf: &mut [u8], ) -> Result, Self::Error> { Err(Self::Error::UnsupportedStorageOperation) @@ -350,9 +357,10 @@ where fn read( &self, key: &::Key, + offset: usize, buf: &mut [u8], ) -> Result, Self::Error> { - StorageRead::::read(&self.storage, key, buf) + StorageRead::::read(&self.storage, key, offset, buf) .map_err(|e| Self::Error::StorageError(D::storage_error_to_string(e))) }