Change read behavior on the InMemoryTransaction to use offset and allow not equal buf size
#2673
Conversation
AurelienFT
requested review from
xgreenx,
Dentosal,
MitchTurner,
netrome,
acerone85,
rafal-ch and
rymnc
as code owners
| @@ -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) | |||
There was a problem hiding this comment.
technically the bug was introduced in #2438, the change in fuel-vm seems to be working as expected from what I can tell
There was a problem hiding this comment.
For me it's FuelLabs/fuel-vm#847 that broke CCP and LDC opcode if this PR hasn't land on master, the read function would have been wrong since #2438 but the opcode would have been fine.
There was a problem hiding this comment.
yep, but the error is triggered in the if branch of an if else statement that does not invoke any of the code in fuel-vm.
There was a problem hiding this comment.
yes I talked about the error just before in the sentence I just added this parenthesis with the opcode in case we want to find later to have beteer search results
crates/storage/src/transactional.rs
Outdated
| let storage = InMemoryStorage::<Column>::default(); | ||
| let mut view = storage.read_transaction(); | ||
| let key = vec![0xA, 0xB, 0xC]; | ||
| let expected = Value::from([1, 2, 3]); |
There was a problem hiding this comment.
nit: we probably shouldn't use expected here (or on the other tests). Copy pasta.
Perhaps source_data makes more sense.
crates/storage/src/transactional.rs
Outdated
| #[test] | ||
| fn read_returns_from_view_buf_smaller() { | ||
| // setup | ||
| let storage = InMemoryStorage::<Column>::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]); | ||
| } |
There was a problem hiding this comment.
Maybe add another test with offset and passing. Could just be the same as this but
assert_eq!(buf, [2, 3]);
There was a problem hiding this comment.
LGTM. I'd recommend getting @xgreenx 's eyes on it just in case.
Linked Issues/PRs
closes #2672
Description
Change code of
readofInMemoryTransactionto match the otherreadfunctionsTests had been added and the error in
readfunction has been improved to be more clear.Checklist
Before requesting review