From 6fffc349f16af86d9d9b6efb7dc591cb345304a9 Mon Sep 17 00:00:00 2001 From: AurelienFT Date: Wed, 5 Feb 2025 17:26:32 +0100 Subject: [PATCH 1/4] Change code of `read` of `InMemoryTransaction` to match the other `read` functions --- crates/storage/src/transactional.rs | 85 ++++++++++++++++++++++++++--- 1 file changed, 78 insertions(+), 7 deletions(-) diff --git a/crates/storage/src/transactional.rs b/crates/storage/src/transactional.rs index e5e568dd3ea..1dcf2dde5a6 100644 --- a/crates/storage/src/transactional.rs +++ b/crates/storage/src/transactional.rs @@ -398,14 +398,21 @@ where if let Some(operation) = self.get_from_changes(key, column) { match operation { WriteOperation::Insert(value) => { - let read = value.len(); - if read != buf.len() { - return Err(crate::Error::Other(anyhow::anyhow!( - "Buffer size is not equal to the value size" - ))); + let bytes_len = value.as_ref().len(); + let start = offset; + let end = offset.saturating_add(buf.len()); + + if end > bytes_len { + return Err(anyhow::anyhow!( + "Offset `{offset}` is out of bounds `{bytes_len}` for key `{:?}`", + key + ) + .into()); } - buf.copy_from_slice(value.as_ref()); - Ok(Some(read)) + + let starting_from_offset = &value.as_ref()[start..end]; + buf[..].copy_from_slice(starting_from_offset); + Ok(Some(buf.len())) } WriteOperation::Remove => Ok(None), } @@ -656,6 +663,70 @@ mod test { use super::*; use crate::column::Column; + #[test] + fn read_returns_from_view_exact_size() { + // setup + let storage = InMemoryStorage::::default(); + let mut view = storage.read_transaction(); + let key = vec![0xA, 0xB, 0xC]; + let expected = Value::from([1, 2, 3]); + view.put(&key, Column::Metadata, expected.clone()).unwrap(); + // test + let mut buf = [0; 3]; + let ret = view.read(&key, Column::Metadata, 0, &mut buf).unwrap(); + // verify + assert_eq!(ret, Some(3)); + assert_eq!(buf, [1, 2, 3]); + } + + #[test] + fn read_returns_from_view_buf_smaller() { + // setup + let storage = InMemoryStorage::::default(); + let mut view = storage.read_transaction(); + let key = vec![0xA, 0xB, 0xC]; + let expected = Value::from([1, 2, 3]); + view.put(&key, Column::Metadata, expected.clone()).unwrap(); + // test + let mut buf = [0; 2]; + let ret = view.read(&key, Column::Metadata, 0, &mut buf).unwrap(); + // verify + assert_eq!(ret, Some(2)); + assert_eq!(buf, [1, 2]); + } + + #[test] + fn read_returns_from_view_buf_bigger() { + // setup + let storage = InMemoryStorage::::default(); + let mut view = storage.read_transaction(); + let key = vec![0xA, 0xB, 0xC]; + let expected = Value::from([1, 2, 3]); + view.put(&key, Column::Metadata, expected.clone()).unwrap(); + // test + let mut buf = [0; 4]; + let ret = view.read(&key, Column::Metadata, 0, &mut buf).unwrap(); + // verify + assert_eq!(ret, Some(3)); + assert_eq!(buf, [1, 2, 3, 0]); + } + + #[test] + fn read_returns_from_view_buf_bigger_because_offset() { + // setup + let storage = InMemoryStorage::::default(); + let mut view = storage.read_transaction(); + let key = vec![0xA, 0xB, 0xC]; + let expected = Value::from([1, 2, 3]); + view.put(&key, Column::Metadata, expected.clone()).unwrap(); + // test + let mut buf = [0; 3]; + let ret = view.read(&key, Column::Metadata, 1, &mut buf).unwrap(); + // verify + assert_eq!(ret, Some(2)); + assert_eq!(buf, [2, 3, 0]); + } + #[test] fn get_returns_from_view() { // setup From f42cc4b703f0ce9318234c0c6378617d95bb34cf Mon Sep 17 00:00:00 2001 From: AurelienFT Date: Wed, 5 Feb 2025 17:29:56 +0100 Subject: [PATCH 2/4] Changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 50a6a3cc073..ad84d2ed97b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - [2667](https://github.com/FuelLabs/fuel-core/pull/2667): Populate `Blobs` table in global merkle root storage. - [2652](https://github.com/FuelLabs/fuel-core/pull/2652): Global Merkle Root storage crate: Add Raw contract bytecode to global merkle root storage when processing Create transactions. +### Fixed +- [2673](https://github.com/FuelLabs/fuel-core/pull/2673): Change read behavior on the InMemoryTransaction to use offset and allow not equal buf size (fix CCP and LDC broken from https://github.com/FuelLabs/fuel-vm/pull/847) + ## [Version 0.41.5] ### Changed From 2a96effcd2b6c621ea9841d60ca69136d084c5ed Mon Sep 17 00:00:00 2001 From: AurelienFT Date: Wed, 5 Feb 2025 17:44:33 +0100 Subject: [PATCH 3/4] Fix test and improves error messages --- crates/fuel-core/src/state/rocks_db.rs | 8 ++++---- crates/storage/src/kv_store.rs | 7 ++++--- crates/storage/src/transactional.rs | 15 +++++++-------- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/crates/fuel-core/src/state/rocks_db.rs b/crates/fuel-core/src/state/rocks_db.rs index a41547976ac..697fb3deb09 100644 --- a/crates/fuel-core/src/state/rocks_db.rs +++ b/crates/fuel-core/src/state/rocks_db.rs @@ -915,19 +915,19 @@ where .map(|value| { let bytes_len = value.len(); let start = offset; - let end = offset.saturating_add(buf.len()); + let buf_len = buf.len(); + let end = offset.saturating_add(buf_len); if end > bytes_len { return Err(StorageError::Other(anyhow::anyhow!( - "Offset `{offset}` is out of bounds `{bytes_len}` \ - for key `{:?}` and column `{column:?}`", + "Offset `{offset}` + buf_len `{buf_len}` read until {end} which is out of bounds `{bytes_len}` for key `{:?}` and column `{column:?}`", key ))); } let starting_from_offset = &value[start..end]; buf[..].copy_from_slice(starting_from_offset); - Ok(buf.len()) + Ok(buf_len) }) .transpose()?; diff --git a/crates/storage/src/kv_store.rs b/crates/storage/src/kv_store.rs index a7029db8d20..b9753947ed3 100644 --- a/crates/storage/src/kv_store.rs +++ b/crates/storage/src/kv_store.rs @@ -71,11 +71,12 @@ pub trait KeyValueInspect { .map(|value| { let bytes_len = value.as_ref().len(); let start = offset; - let end = offset.saturating_add(buf.len()); + let buf_len = buf.len(); + let end = offset.saturating_add(buf_len); if end > bytes_len { return Err(anyhow::anyhow!( - "Offset `{offset}` is out of bounds `{bytes_len}` for key `{:?}`", + "Offset `{offset}` + buf_len `{buf_len}` read until {end} which is out of bounds `{bytes_len}` for key `{:?}`", key ) .into()); @@ -83,7 +84,7 @@ pub trait KeyValueInspect { let starting_from_offset = &value.as_ref()[start..end]; buf[..].copy_from_slice(starting_from_offset); - Ok(buf.len()) + Ok(buf_len) }) .transpose() } diff --git a/crates/storage/src/transactional.rs b/crates/storage/src/transactional.rs index 1dcf2dde5a6..ef87cc1b35f 100644 --- a/crates/storage/src/transactional.rs +++ b/crates/storage/src/transactional.rs @@ -400,11 +400,12 @@ where WriteOperation::Insert(value) => { let bytes_len = value.as_ref().len(); let start = offset; + let buf_len = buf.len(); let end = offset.saturating_add(buf.len()); if end > bytes_len { return Err(anyhow::anyhow!( - "Offset `{offset}` is out of bounds `{bytes_len}` for key `{:?}`", + "Offset `{offset}` + buf_len `{buf_len}` read until {end} which is out of bounds `{bytes_len}` for key `{:?}`", key ) .into()); @@ -412,7 +413,7 @@ where let starting_from_offset = &value.as_ref()[start..end]; buf[..].copy_from_slice(starting_from_offset); - Ok(Some(buf.len())) + Ok(Some(buf_len)) } WriteOperation::Remove => Ok(None), } @@ -705,10 +706,9 @@ mod test { view.put(&key, Column::Metadata, expected.clone()).unwrap(); // test let mut buf = [0; 4]; - let ret = view.read(&key, Column::Metadata, 0, &mut buf).unwrap(); + let ret = view.read(&key, Column::Metadata, 0, &mut buf).unwrap_err(); // verify - assert_eq!(ret, Some(3)); - assert_eq!(buf, [1, 2, 3, 0]); + assert_eq!(ret, crate::Error::Other(anyhow::anyhow!("Offset `0` + buf_len `4` read until 4 which is out of bounds `3` for key `[10, 11, 12]`".to_string()))); } #[test] @@ -721,10 +721,9 @@ mod test { view.put(&key, Column::Metadata, expected.clone()).unwrap(); // test let mut buf = [0; 3]; - let ret = view.read(&key, Column::Metadata, 1, &mut buf).unwrap(); + let ret = view.read(&key, Column::Metadata, 1, &mut buf).unwrap_err(); // verify - assert_eq!(ret, Some(2)); - assert_eq!(buf, [2, 3, 0]); + assert_eq!(ret, crate::Error::Other(anyhow::anyhow!("Offset `1` + buf_len `3` read until 4 which is out of bounds `3` for key `[10, 11, 12]`".to_string()))); } #[test] From 95ee36250ebd617451bba11e3cbfedfd0fdb7e00 Mon Sep 17 00:00:00 2001 From: AurelienFT Date: Wed, 5 Feb 2025 20:00:54 +0100 Subject: [PATCH 4/4] add one more test and change variable name in tests --- crates/storage/src/transactional.rs | 32 +++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/crates/storage/src/transactional.rs b/crates/storage/src/transactional.rs index ef87cc1b35f..abe5e4dedf0 100644 --- a/crates/storage/src/transactional.rs +++ b/crates/storage/src/transactional.rs @@ -670,8 +670,8 @@ mod test { let storage = InMemoryStorage::::default(); let mut view = storage.read_transaction(); let key = vec![0xA, 0xB, 0xC]; - let expected = Value::from([1, 2, 3]); - view.put(&key, Column::Metadata, expected.clone()).unwrap(); + let value = Value::from([1, 2, 3]); + view.put(&key, Column::Metadata, value).unwrap(); // test let mut buf = [0; 3]; let ret = view.read(&key, Column::Metadata, 0, &mut buf).unwrap(); @@ -686,8 +686,8 @@ mod test { let storage = InMemoryStorage::::default(); let mut view = storage.read_transaction(); let key = vec![0xA, 0xB, 0xC]; - let expected = Value::from([1, 2, 3]); - view.put(&key, Column::Metadata, expected.clone()).unwrap(); + let value = Value::from([1, 2, 3]); + view.put(&key, Column::Metadata, value).unwrap(); // test let mut buf = [0; 2]; let ret = view.read(&key, Column::Metadata, 0, &mut buf).unwrap(); @@ -696,14 +696,30 @@ mod test { assert_eq!(buf, [1, 2]); } + #[test] + fn read_returns_from_view_with_offset() { + // setup + let storage = InMemoryStorage::::default(); + let mut view = storage.read_transaction(); + let key = vec![0xA, 0xB, 0xC]; + let value = Value::from([1, 2, 3]); + view.put(&key, Column::Metadata, value).unwrap(); + // test + let mut buf = [0; 2]; + let ret = view.read(&key, Column::Metadata, 1, &mut buf).unwrap(); + // verify + assert_eq!(ret, Some(2)); + assert_eq!(buf, [2, 3]); + } + #[test] fn read_returns_from_view_buf_bigger() { // setup let storage = InMemoryStorage::::default(); let mut view = storage.read_transaction(); let key = vec![0xA, 0xB, 0xC]; - let expected = Value::from([1, 2, 3]); - view.put(&key, Column::Metadata, expected.clone()).unwrap(); + let value = Value::from([1, 2, 3]); + view.put(&key, Column::Metadata, value).unwrap(); // test let mut buf = [0; 4]; let ret = view.read(&key, Column::Metadata, 0, &mut buf).unwrap_err(); @@ -717,8 +733,8 @@ mod test { let storage = InMemoryStorage::::default(); let mut view = storage.read_transaction(); let key = vec![0xA, 0xB, 0xC]; - let expected = Value::from([1, 2, 3]); - view.put(&key, Column::Metadata, expected.clone()).unwrap(); + let value = Value::from([1, 2, 3]); + view.put(&key, Column::Metadata, value).unwrap(); // test let mut buf = [0; 3]; let ret = view.read(&key, Column::Metadata, 1, &mut buf).unwrap_err();