Skip to content

Commit

Permalink
Fix StorageVec bugs with use of field_id (FuelLabs#4543)
Browse files Browse the repository at this point in the history 
## Description

Only some of the read/writes in `StorageVec` were updated to use
`field_id` in FuelLabs#4537. For example,
there are two writes when inserting, the length and value itself. Only
the value was updated meaning the lengths would still be overwriting one
another.
 
Tests to ensure both values and lengths are updated separately have been
added.

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.

---------

Co-authored-by: bitzoic <[email protected]>
Co-authored-by: IGI-111 <[email protected]>
  • Loading branch information
@bitzoic @IGI-111
3 people authored May 7, 2023
1 parent 657a10b commit 1d6ab3b
Show file tree
Hide file tree
Showing 3 changed files with 138 additions and 50 deletions.
70 changes: 35 additions & 35 deletions sway-lib-std/src/storage/storage_vec.sw
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,14 @@ impl<V> StorageKey<StorageVec<V>> {
/// ```
#[storage(read, write)]
pub fn push(self, value: V) {
let len = read::<u64>(self.slot, 0).unwrap_or(0);
let len = read::<u64>(self.field_id, 0).unwrap_or(0);

// Storing the value at the current length index (if this is the first item, starts off at 0)
let key = sha256((len, self.field_id));
write::<V>(key, 0, value);

// Incrementing the length
write(self.slot, 0, len + 1);
write(self.field_id, 0, len + 1);
}

/// Removes the last element of the vector and returns it, `None` if empty.
Expand Down Expand Up @@ -76,15 +76,15 @@ impl<V> StorageKey<StorageVec<V>> {
/// ```
#[storage(read, write)]
pub fn pop(self) -> Option<V> {
let len = read::<u64>(self.slot, 0).unwrap_or(0);
let len = read::<u64>(self.field_id, 0).unwrap_or(0);

// if the length is 0, there is no item to pop from the vec
if len == 0 {
return None;
}

// reduces len by 1, effectively removing the last item in the vec
write(self.slot, 0, len - 1);
write(self.field_id, 0, len - 1);

let key = sha256((len - 1, self.field_id));
read::<V>(key, 0)
Expand Down Expand Up @@ -118,7 +118,7 @@ impl<V> StorageKey<StorageVec<V>> {
/// ```
#[storage(read)]
pub fn get(self, index: u64) -> Option<StorageKey<V>> {
let len = read::<u64>(self.slot, 0).unwrap_or(0);
let len = read::<u64>(self.field_id, 0).unwrap_or(0);

// if the index is larger or equal to len, there is no item to return
if len <= index {
Expand Down Expand Up @@ -170,13 +170,13 @@ impl<V> StorageKey<StorageVec<V>> {
/// ```
#[storage(read, write)]
pub fn remove(self, index: u64) -> V {
let len = read::<u64>(self.slot, 0).unwrap_or(0);
let len = read::<u64>(self.field_id, 0).unwrap_or(0);

// if the index is larger or equal to len, there is no item to remove
assert(index < len);

// gets the element before removing it, so it can be returned
let removed_element = read::<V>(sha256((index, self.slot)), 0).unwrap();
let removed_element = read::<V>(sha256((index, self.field_id)), 0).unwrap();

// for every element in the vec with an index greater than the input index,
// shifts the index for that element down one
Expand All @@ -185,13 +185,13 @@ impl<V> StorageKey<StorageVec<V>> {
// gets the storage location for the previous index
let key = sha256((count - 1, self.field_id));
// moves the element of the current index into the previous index
write::<V>(key, 0, read::<V>(sha256((count, self.slot)), 0).unwrap());
write::<V>(key, 0, read::<V>(sha256((count, self.field_id)), 0).unwrap());

count += 1;
}

// decrements len by 1
write(self.slot, 0, len - 1);
write(self.field_id, 0, len - 1);

removed_element
}
Expand Down Expand Up @@ -233,20 +233,20 @@ impl<V> StorageKey<StorageVec<V>> {
/// ```
#[storage(read, write)]
pub fn swap_remove(self, index: u64) -> V {
let len = read::<u64>(self.slot, 0).unwrap_or(0);
let len = read::<u64>(self.field_id, 0).unwrap_or(0);

// if the index is larger or equal to len, there is no item to remove
assert(index < len);

let hash_of_to_be_removed = sha256((index, self.slot));
let hash_of_to_be_removed = sha256((index, self.field_id));
// gets the element before removing it, so it can be returned
let element_to_be_removed = read::<V>(hash_of_to_be_removed, 0).unwrap();

let last_element = read::<V>(sha256((len - 1, self.slot)), 0).unwrap();
let last_element = read::<V>(sha256((len - 1, self.field_id)), 0).unwrap();
write::<V>(hash_of_to_be_removed, 0, last_element);

// decrements len by 1
write(self.slot, 0, len - 1);
write(self.field_id, 0, len - 1);

element_to_be_removed
}
Expand Down Expand Up @@ -288,7 +288,7 @@ impl<V> StorageKey<StorageVec<V>> {
/// ```
#[storage(read, write)]
pub fn set(self, index: u64, value: V) {
let len = read::<u64>(self.slot, 0).unwrap_or(0);
let len = read::<u64>(self.field_id, 0).unwrap_or(0);

// if the index is higher than or equal len, there is no element to set
assert(index < len);
Expand Down Expand Up @@ -338,7 +338,7 @@ impl<V> StorageKey<StorageVec<V>> {
/// ```
#[storage(read, write)]
pub fn insert(self, index: u64, value: V) {
let len = read::<u64>(self.slot, 0).unwrap_or(0);
let len = read::<u64>(self.field_id, 0).unwrap_or(0);

// if the index is larger than len, there is no space to insert
assert(index <= len);
Expand All @@ -349,7 +349,7 @@ impl<V> StorageKey<StorageVec<V>> {
write::<V>(key, 0, value);

// increments len by 1
write(self.slot, 0, len + 1);
write(self.field_id, 0, len + 1);

return;
}
Expand All @@ -361,7 +361,7 @@ impl<V> StorageKey<StorageVec<V>> {
while count >= index {
let key = sha256((count + 1, self.field_id));
// shifts all the values up one index
write::<V>(key, 0, read::<V>(sha256((count, self.slot)), 0).unwrap());
write::<V>(key, 0, read::<V>(sha256((count, self.field_id)), 0).unwrap());

if count == 0 { break; }
count -= 1;
Expand All @@ -372,7 +372,7 @@ impl<V> StorageKey<StorageVec<V>> {
write::<V>(key, 0, value);

// increments len by 1
write(self.slot, 0, len + 1);
write(self.field_id, 0, len + 1);
}

/// Returns the length of the vector.
Expand Down Expand Up @@ -400,7 +400,7 @@ impl<V> StorageKey<StorageVec<V>> {
/// ```
#[storage(read)]
pub fn len(self) -> u64 {
read::<u64>(self.slot, 0).unwrap_or(0)
read::<u64>(self.field_id, 0).unwrap_or(0)
}

/// Checks whether the len is zero or not.
Expand Down Expand Up @@ -432,7 +432,7 @@ impl<V> StorageKey<StorageVec<V>> {
/// ```
#[storage(read)]
pub fn is_empty(self) -> bool {
read::<u64>(self.slot, 0).unwrap_or(0) == 0
read::<u64>(self.field_id, 0).unwrap_or(0) == 0
}

/// Sets the len to zero.
Expand Down Expand Up @@ -460,7 +460,7 @@ impl<V> StorageKey<StorageVec<V>> {
/// ```
#[storage(write)]
pub fn clear(self) {
let _ = clear::<u64>(self.slot);
let _ = clear::<u64>(self.field_id);
}

/// Swaps two elements.
Expand Down Expand Up @@ -500,16 +500,16 @@ impl<V> StorageKey<StorageVec<V>> {
/// ```
#[storage(read, write)]
pub fn swap(self, element1_index: u64, element2_index: u64) {
let len = read::<u64>(self.slot, 0).unwrap_or(0);
let len = read::<u64>(self.field_id, 0).unwrap_or(0);
assert(element1_index < len);
assert(element2_index < len);

if element1_index == element2_index {
return;
}

let element1_key = sha256((element1_index, self.slot));
let element2_key = sha256((element2_index, self.slot));
let element1_key = sha256((element1_index, self.field_id));
let element2_key = sha256((element2_index, self.field_id));

let element1_value = read::<V>(element1_key, 0).unwrap();
write::<V>(element1_key, 0, read::<V>(element2_key, 0).unwrap());
Expand Down Expand Up @@ -539,7 +539,7 @@ impl<V> StorageKey<StorageVec<V>> {
/// ```
#[storage(read)]
pub fn first(self) -> Option<StorageKey<V>> {
match read::<u64>(self.slot, 0).unwrap_or(0) {
match read::<u64>(self.field_id, 0).unwrap_or(0) {
0 => None,
_ => Some(StorageKey {
slot: sha256((0, self.field_id)),
Expand Down Expand Up @@ -573,10 +573,10 @@ impl<V> StorageKey<StorageVec<V>> {
/// ```
#[storage(read)]
pub fn last(self) -> Option<StorageKey<V>> {
match read::<u64>(self.slot, 0).unwrap_or(0) {
match read::<u64>(self.field_id, 0).unwrap_or(0) {
0 => None,
len => Some(StorageKey {
slot: sha256((len - 1, self.slot)),
slot: sha256((len - 1, self.field_id)),
offset: 0,
field_id: sha256((0, self.field_id)),
}),
Expand Down Expand Up @@ -610,7 +610,7 @@ impl<V> StorageKey<StorageVec<V>> {
/// ```
#[storage(read, write)]
pub fn reverse(self) {
let len = read::<u64>(self.slot, 0).unwrap_or(0);
let len = read::<u64>(self.field_id, 0).unwrap_or(0);

if len < 2 {
return;
Expand All @@ -619,8 +619,8 @@ impl<V> StorageKey<StorageVec<V>> {
let mid = len / 2;
let mut i = 0;
while i < mid {
let element1_key = sha256((i, self.slot));
let element2_key = sha256((len - i - 1, self.slot));
let element1_key = sha256((i, self.field_id));
let element2_key = sha256((len - i - 1, self.field_id));

let element1_value = read::<V>(element1_key, 0).unwrap();
write::<V>(element1_key, 0, read::<V>(element2_key, 0).unwrap());
Expand Down Expand Up @@ -661,11 +661,11 @@ impl<V> StorageKey<StorageVec<V>> {
/// ```
#[storage(read, write)]
pub fn fill(self, value: V) {
let len = read::<u64>(self.slot, 0).unwrap_or(0);
let len = read::<u64>(self.field_id, 0).unwrap_or(0);

let mut i = 0;
while i < len {
write::<V>(sha256((i, self.slot)), 0, value);
write::<V>(sha256((i, self.field_id)), 0, value);
i += 1;
}
}
Expand Down Expand Up @@ -713,11 +713,11 @@ impl<V> StorageKey<StorageVec<V>> {
/// ```
#[storage(read, write)]
pub fn resize(self, new_len: u64, value: V) {
let mut len = read::<u64>(self.slot, 0).unwrap_or(0);
let mut len = read::<u64>(self.field_id, 0).unwrap_or(0);
while len < new_len {
write::<V>(sha256((len, self.slot)), 0, value);
write::<V>(sha256((len, self.field_id)), 0, value);
len += 1;
}
write::<u64>(self.slot, 0, new_len);
write::<u64>(self.field_id, 0, new_len);
}
}
2 changes: 1 addition & 1 deletion test/src/e2e_vm_tests/test_programs/should_pass/empty_fields_in_storage_struct/Forc.toml
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
[project]
authors = ["Fuel Labs <[email protected]>"]
entry = "main.sw"
license = "Apache-2.0"
name = "empty_fields_in_storage_struct"
entry = "main.sw"

[dependencies]
std = { path = "../../../../../../sway-lib-std" }
Loading

0 comments on commit 1d6ab3b

Please sign in to comment.