-
Notifications
You must be signed in to change notification settings - Fork 91
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Use Vec of Bytes for Contract State #671
Changes from 11 commits
739f5eb
29c5a48
b174897
6566120
c2a7644
f9fe1cf
46388b3
0775951
4fc8288
9f6e7bb
6150b1d
db04050
99c5541
e5cc6f4
ce7bbd8
0d814ed
2450866
8ff5159
1c0439d
ecfcdc3
a9149f1
8eda94e
873b518
87b707f
df88364
b7c1f90
59bb82b
15a069a
65ac84f
6d7d036
f6f3dfd
d444d20
bc0a256
6dde649
d784d40
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,10 @@ | ||
use alloc::vec::Vec; | ||
use fuel_types::{ | ||
canonical::{ | ||
Deserialize, | ||
Serialize, | ||
}, | ||
Bytes32, | ||
Bytes64, | ||
}; | ||
|
||
#[cfg(feature = "random")] | ||
|
@@ -18,49 +18,38 @@ use rand::{ | |
|
||
use core::cmp::Ordering; | ||
|
||
pub type StorageData = Vec<u8>; | ||
|
||
#[derive(Debug, Default, Clone, PartialEq, Eq, Hash)] | ||
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] | ||
#[cfg_attr(feature = "typescript", wasm_bindgen::prelude::wasm_bindgen)] | ||
#[derive(Deserialize, Serialize)] | ||
pub struct StorageSlot { | ||
key: Bytes32, | ||
value: Bytes32, | ||
value: StorageData, | ||
} | ||
|
||
impl StorageSlot { | ||
pub const SLOT_SIZE: usize = Bytes32::LEN + Bytes32::LEN; | ||
|
||
pub const fn new(key: Bytes32, value: Bytes32) -> Self { | ||
pub const fn new(key: Bytes32, value: StorageData) -> Self { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't add support for dynamic storage in the current PR. We only want to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This PR now includes manual implementations of serialization/deserialization to maintain a fixed size for storage slots. These implementations will be removed when fully supporting dynamic storage. |
||
StorageSlot { key, value } | ||
} | ||
|
||
pub const fn key(&self) -> &Bytes32 { | ||
&self.key | ||
} | ||
|
||
pub const fn value(&self) -> &Bytes32 { | ||
pub const fn value(&self) -> &StorageData { | ||
&self.value | ||
} | ||
} | ||
|
||
impl From<&StorageSlot> for Bytes64 { | ||
fn from(s: &StorageSlot) -> Self { | ||
let mut buf = [0u8; StorageSlot::SLOT_SIZE]; | ||
|
||
buf[..Bytes32::LEN].copy_from_slice(s.key.as_ref()); | ||
buf[Bytes32::LEN..].copy_from_slice(s.value.as_ref()); | ||
|
||
buf.into() | ||
pub fn size(&self) -> usize { | ||
Serialize::size(self) | ||
} | ||
} | ||
|
||
impl From<&Bytes64> for StorageSlot { | ||
fn from(b: &Bytes64) -> Self { | ||
let key = <Bytes32 as Deserialize>::from_bytes(&b[..Bytes32::LEN]) | ||
.expect("Infallible deserialization"); | ||
let value = <Bytes32 as Deserialize>::from_bytes(&b[Bytes32::LEN..]) | ||
.expect("Infallible deserialization"); | ||
Self::new(key, value) | ||
impl From<&(Bytes32, StorageData)> for StorageSlot { | ||
fn from((key, value): &(Bytes32, StorageData)) -> Self { | ||
Self::new(*key, value.clone()) | ||
} | ||
} | ||
|
||
|
@@ -69,7 +58,7 @@ impl Distribution<StorageSlot> for Standard { | |
fn sample<R: Rng + ?Sized>(&self, rng: &mut R) -> StorageSlot { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note: now |
||
StorageSlot { | ||
key: rng.gen(), | ||
value: rng.gen(), | ||
value: rng.gen::<Bytes32>().to_vec(), | ||
} | ||
} | ||
} | ||
|
@@ -89,7 +78,10 @@ impl Ord for StorageSlot { | |
#[cfg(test)] | ||
mod tests { | ||
use super::*; | ||
use rand::SeedableRng; | ||
use rand::{ | ||
RngCore, | ||
SeedableRng, | ||
}; | ||
use std::{ | ||
fs::File, | ||
path::PathBuf, | ||
|
@@ -101,7 +93,7 @@ mod tests { | |
fn test_storage_slot_serialization() { | ||
let rng = &mut rand::rngs::StdRng::seed_from_u64(8586); | ||
let key: Bytes32 = rng.gen(); | ||
let value: Bytes32 = rng.gen(); | ||
let value = rng.gen::<Bytes32>().to_vec(); | ||
|
||
let slot = StorageSlot::new(key, value); | ||
let slots = vec![slot.clone()]; | ||
|
@@ -124,4 +116,42 @@ mod tests { | |
serde_json::from_reader(storage_slots_file).expect("read file"); | ||
assert_eq!(storage_slots.len(), 1); | ||
} | ||
|
||
#[test] | ||
fn test_storage_slot_canonical_serialization() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you include given/when/then sections for these tests? |
||
let rng = &mut rand::rngs::StdRng::seed_from_u64(8586); | ||
let key: Bytes32 = rng.gen(); | ||
let mut value = [0u8; 128]; | ||
rng.fill_bytes(&mut value); | ||
|
||
let slot = StorageSlot::new(key, value.to_vec()); | ||
|
||
let slot_bytes = slot.to_bytes(); | ||
|
||
let (slot_key, slot_data) = slot_bytes.split_at(32); | ||
|
||
assert_eq!(slot_key, key.as_ref()); | ||
|
||
let slot_data_num_bytes = | ||
u64::from_bytes(&slot_data[..8]).expect("read from bytes"); | ||
assert_eq!(slot_data_num_bytes, 128); | ||
|
||
// `from_bytes` works | ||
let recreated_slot = | ||
StorageSlot::from_bytes(&slot_bytes).expect("read from bytes"); | ||
assert_eq!(recreated_slot, slot); | ||
} | ||
|
||
#[test] | ||
fn test_storage_slot_size() { | ||
let rng = &mut rand::rngs::StdRng::seed_from_u64(8586); | ||
let key: Bytes32 = rng.gen(); | ||
let mut value = [0u8; 128]; | ||
rng.fill_bytes(&mut value); | ||
|
||
let slot = StorageSlot::new(key, value.to_vec()); | ||
let size = slot.size(); | ||
let expected_size = 32 + 8 + 128; // Key + u64 (data size) + Data | ||
assert_eq!(size, expected_size); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -54,7 +54,10 @@ use crate::{ | |
InterpreterStorage, | ||
}, | ||
}; | ||
use alloc::vec::Vec; | ||
use alloc::{ | ||
vec, | ||
vec::Vec, | ||
}; | ||
use fuel_asm::PanicReason; | ||
use fuel_storage::StorageSize; | ||
use fuel_tx::{ | ||
|
@@ -1041,7 +1044,7 @@ pub(crate) fn state_write_word<S: InterpreterStorage>( | |
let contract = ContractId::from_bytes_ref(contract.read(memory)); | ||
let key = Bytes32::from_bytes_ref(key.read(memory)); | ||
|
||
let mut value = Bytes32::default(); | ||
let mut value = vec![0; 32]; | ||
|
||
value[..WORD_SIZE].copy_from_slice(&c.to_be_bytes()); | ||
|
||
|
@@ -1228,10 +1231,10 @@ fn state_read_qword<S: InterpreterStorage>( | |
.map_err(RuntimeError::Storage)? | ||
.into_iter() | ||
.flat_map(|bytes| match bytes { | ||
Some(bytes) => **bytes, | ||
Some(bytes) => bytes.into_owned(), | ||
None => { | ||
all_set = false; | ||
*Bytes32::zeroed() | ||
vec![0; 32] | ||
} | ||
}) | ||
.collect(); | ||
|
@@ -1295,7 +1298,7 @@ fn state_write_qword<'vm, S: InterpreterStorage>( | |
|
||
let values: Vec<_> = memory[input.source_address_memory_range.usizes()] | ||
.chunks_exact(Bytes32::LEN) | ||
.flat_map(|chunk| Some(Bytes32::from(<[u8; 32]>::try_from(chunk).ok()?))) | ||
.flat_map(|chunk| Some(chunk.to_vec())) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead, we can use Thanks to @Voxelot for pointing of this idea=) |
||
.collect(); | ||
|
||
let unset_count = storage | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious about the naming of this. Are we intending all storage to move to
Vec
or just for contract state? Or is there no distinction between storage and contract storage?Just trying to understand the scope of all this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just for the
ContractsState
table storage. This can be named better - something likeContractsStateData
could be more congruent with the table, where the key isContractsStateKey
. I will make that change, but let me know if you have a suggestion you prefer.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I renamed it to
ContractsStateData
to illustrate that this is part of theContractsState
table alongside theContractsStateKey
.