feat: Use Vec of Bytes for Contract State #671

bvrooman · 2024-02-06T01:12:29Z

Related issues:

Use Vec<u8> as a value for the ContractsState table #655

Voxelot · 2024-02-08T21:59:15Z

@xgreenx would this negate the performance benefit of being able to directly copy the contract bytecode into vm memory out of the db? I.e. seems like this would require copying the whole bytecode onto the heap first?

Dentosal · 2024-02-11T19:16:00Z

fuel-tx/src/transaction/types/storage.rs

@@ -69,7 +58,7 @@ impl Distribution<StorageSlot> for Standard {
    fn sample<R: Rng + ?Sized>(&self, rng: &mut R) -> StorageSlot {


Note: now sample doesn't generate from all possible values anymore, which might affect test coverage. Seems to be a non-issue for now.

xgreenx · 2024-02-11T19:32:39Z

would this negate the performance benefit of being able to directly copy the contract bytecode into vm memory out of the db? I.e. seems like this would require copying the whole bytecode onto the heap first?

@Voxelot The change affects the ContractsState table, not ContractsRawCode table=) So everything works as before

xgreenx · 2024-02-11T20:36:20Z

fuel-tx/src/transaction/types/storage.rs

-    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 {


We don't add support for dynamic storage in the current PR. We only want to use Vec<u8> in the bytes. This change affects the layout of the serialized transaction.

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.

xgreenx · 2024-02-12T13:34:36Z

fuel-vm/src/interpreter/blockchain.rs

@@ -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()))


Instead, we can use StorageWrite::write or StorageWrite::replace. In this case, we don't need to spend 1 byte to store the size of the vector. As we did for ContractsRawCode table.

Thanks to @Voxelot for pointing of this idea=)

xgreenx · 2024-02-12T13:36:25Z

fuel-vm/src/storage.rs

    type OwnedValue = Self::Value;
    /// The table value is hash of the value.
-    type Value = Bytes32;
+    type Value = StorageData;


I think we can use [u8] here and for OwnedValue = StorageData=)

bvrooman · 2024-02-12T19:11:36Z

Additional note from today's Sync: We will not make any breaking changes to transaction serialization as part of this task. We will do that when we work directly on dynamic storage.

MitchTurner · 2024-02-14T18:58:56Z

fuel-tx/src/transaction/types/storage.rs

@@ -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() {


Could you include given/when/then sections for these tests?

MitchTurner · 2024-02-14T19:17:58Z

fuel-tx/src/transaction/types/storage.rs

@@ -18,49 +18,38 @@ use rand::{

 use core::cmp::Ordering;

+pub type StorageData = Vec<u8>;


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.

This is just for the ContractsState table storage. This can be named better - something like ContractsStateData could be more congruent with the table, where the key is ContractsStateKey. I will make that change, but let me know if you have a suggestion you prefer.

I renamed it to ContractsStateData to illustrate that this is part of the ContractsState table alongside the ContractsStateKey.

Dentosal · 2024-02-19T19:21:33Z

fuel-vm/src/storage/interpreter.rs

+        // StorageWrite::<ContractsState>::write(self, &(contract, key).into(),
+        // value.into())


Maybe these comments should be removed?

This part specifically is unclear in my mind. We want to use a higher level of abstraction for the ContractsState table, on top of StorageMutate and StorageInspect by implementing StorageWrite and StorageRead. This should consolidate the read and write patterns for vector based storage. I'm not sure I understand how/why that's used in this specific context of Merkle storage.

Summarizing from the call earlier today - it sounds like we need to implement StorageRead and StorageWrite for the contracts state table. This will also allow us to structure the table using Vec<u8> or [u8] under the hood and avoid the need for the StorageData newtype wrapper.

We will still use the new wrapper type to provide some convenience functions for converting between types, as well as to control the canonical serialization implementation while we need a fixed size.
Edit: We will use it only in the table definition

xgreenx · 2024-02-26T10:34:51Z

fuel-tx/src/transaction/types/storage.rs

+    }
+}
+
+// TODO: Remove manual serialization when implementing dynamic storage


We don't need to use this type in the transaction. It should be used only inside of the ConstractsState table. In this case you don't need to have custom serialization and deserialziation

Ok, I can revert the changes to StorageSlot and keep the value type as Bytes32.

xgreenx · 2024-02-26T10:35:39Z

fuel-tx/src/transaction/types/storage.rs


 #[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: ContractsStateData,


Let's not change the type for the Transaction to avoid breaking changes and to not affect the serialziation

….com/FuelLabs/fuel-vm into bvrooman/feat/dynamic-contract-state

xgreenx · 2024-02-27T13:53:23Z

fuel-vm/src/interpreter/blockchain.rs

@@ -1306,7 +1306,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(Some)


Suggested change

.flat_map(Some)

bvrooman added 5 commits January 31, 2024 20:27

Change ContractsState::Value to Vec<u8>

739f5eb

WIP

29c5a48

WIP

b174897

WIP

6566120

WIP

c2a7644

bvrooman mentioned this pull request Feb 6, 2024

Use Vec<u8> as a value for the ContractsState table #655

Closed

2 tasks

bvrooman added 5 commits February 5, 2024 22:57

Update

f9fe1cf

Update CHANGELOG.md

46388b3

Fix test

0775951

Update blockchain.rs

4fc8288

no_std vec

9f6e7bb

bvrooman changed the title ~~feat: Dynamic Contract State~~ feat: Use Vec of Bytes for Contract State Feb 7, 2024

bvrooman self-assigned this Feb 7, 2024

bvrooman marked this pull request as ready for review February 9, 2024 00:47

bvrooman requested a review from a team February 9, 2024 00:47

bvrooman mentioned this pull request Feb 10, 2024

Versioning of the protocol-required database types FuelLabs/fuel-core#1552

Closed

17 tasks

Dentosal reviewed Feb 11, 2024

View reviewed changes

Merge branch 'master' into bvrooman/feat/dynamic-contract-state

6150b1d

xgreenx reviewed Feb 12, 2024

View reviewed changes

MitchTurner reviewed Feb 14, 2024

View reviewed changes

bvrooman added 2 commits February 16, 2024 14:17

WIP

db04050

Test Fix

99c5541

bvrooman marked this pull request as draft February 16, 2024 19:36

bvrooman added 2 commits February 16, 2024 16:47

WIP

e5cc6f4

Maintain serialization/deserialization format

ce7bbd8

xgreenx self-requested a review February 19, 2024 18:05

Fix includes for no_std

8ff5159

Dentosal reviewed Feb 19, 2024

View reviewed changes

bvrooman added 7 commits February 22, 2024 17:39

Merge branch 'master' into bvrooman/feat/dynamic-contract-state

1c0439d

Use StorageRead/StorageWrite

ecfcdc3

Clippeehee

a9149f1

Uncomment take

8eda94e

Update tests

873b518

Fix test

87b707f

Rename StorageData to ContractsStateData for clarity

df88364

xgreenx reviewed Feb 26, 2024

View reviewed changes

bvrooman and others added 9 commits February 26, 2024 12:16

Revert changes to StorageSlot

b7c1f90

Merge branch 'master' into bvrooman/feat/dynamic-contract-state

59bb82b

Refactor ContractsState into separate file

15a069a

Minor refactor

65ac84f

Merge branch 'bvrooman/feat/dynamic-contract-state' of https://github…

6d7d036

….com/FuelLabs/fuel-vm into bvrooman/feat/dynamic-contract-state

Update use statements

f6f3dfd

Remove commented out code

d444d20

Fix feature flag

bc0a256

Merge branch 'master' into bvrooman/feat/dynamic-contract-state

6dde649

xgreenx reviewed Feb 27, 2024

View reviewed changes

Use iterator instead of the vector

d784d40

xgreenx approved these changes Feb 27, 2024

View reviewed changes

xgreenx enabled auto-merge February 27, 2024 14:17

xgreenx added this pull request to the merge queue Feb 27, 2024

Merged via the queue into master with commit ecf5b24 Feb 27, 2024
37 checks passed

xgreenx deleted the bvrooman/feat/dynamic-contract-state branch February 27, 2024 14:37

Dentosal mentioned this pull request Feb 28, 2024

Regenesis support FuelLabs/fuel-core#1693

Merged

xgreenx mentioned this pull request Feb 29, 2024

Release 0.47.0 #687

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Use Vec of Bytes for Contract State #671

feat: Use Vec of Bytes for Contract State #671

bvrooman commented Feb 6, 2024 •

edited

Loading

Voxelot commented Feb 8, 2024

Dentosal Feb 11, 2024

xgreenx commented Feb 11, 2024 •

edited

Loading

xgreenx Feb 11, 2024

bvrooman Feb 19, 2024

xgreenx Feb 12, 2024

xgreenx Feb 12, 2024

bvrooman commented Feb 12, 2024

MitchTurner Feb 14, 2024 •

edited

Loading

MitchTurner Feb 14, 2024

bvrooman Feb 19, 2024 •

edited

Loading

bvrooman Feb 26, 2024 •

edited

Loading

Dentosal Feb 19, 2024

bvrooman Feb 19, 2024

Voxelot Feb 21, 2024 •

edited

Loading

bvrooman Feb 26, 2024 •

edited

Loading

xgreenx Feb 26, 2024

bvrooman Feb 26, 2024

xgreenx Feb 26, 2024

xgreenx Feb 27, 2024

		@@ -69,7 +58,7 @@ impl Distribution<StorageSlot> for Standard {
		fn sample<R: Rng + ?Sized>(&self, rng: &mut R) -> StorageSlot {

		@@ -18,49 +18,38 @@ use rand::{

		use core::cmp::Ordering;

		pub type StorageData = Vec<u8>;

		// StorageWrite::<ContractsState>::write(self, &(contract, key).into(),
		// value.into())

feat: Use Vec of Bytes for Contract State #671

feat: Use Vec of Bytes for Contract State #671

Conversation

bvrooman commented Feb 6, 2024 • edited Loading

Voxelot commented Feb 8, 2024

Choose a reason for hiding this comment

xgreenx commented Feb 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bvrooman commented Feb 12, 2024

MitchTurner Feb 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bvrooman Feb 19, 2024 • edited Loading

Choose a reason for hiding this comment

bvrooman Feb 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Voxelot Feb 21, 2024 • edited Loading

Choose a reason for hiding this comment

bvrooman Feb 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bvrooman commented Feb 6, 2024 •

edited

Loading

xgreenx commented Feb 11, 2024 •

edited

Loading

MitchTurner Feb 14, 2024 •

edited

Loading

bvrooman Feb 19, 2024 •

edited

Loading

bvrooman Feb 26, 2024 •

edited

Loading

Voxelot Feb 21, 2024 •

edited

Loading

bvrooman Feb 26, 2024 •

edited

Loading