Skip to content

Commit

Permalink
Introduce unique field_id in StorageKey (FuelLabs#4537)
Browse files Browse the repository at this point in the history
## Description

This adds a field to `StorageKey` that is unique among storage fields.
It aims to alleviate the problem created by having multiple zero sized
fields inside of a storage struct, which are, by necessity, living in
the same slot.

It allows `StorageMap` (which itself is zero-sized) to address its own
values on a unique address map instead of colliding with adjacent
`StorageMap`s.

This is a breaking change because it will generate different IR
for the same code.

Fixes FuelLabs#4524
Fixes FuelLabs#4523


## 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: Sophie Dankel <[email protected]>
  • Loading branch information
IGI-111 and sdankel authored May 5, 2023
1 parent 9f59f53 commit 97f3c5f
Show file tree
Hide file tree
Showing 14 changed files with 177 additions and 16 deletions.
17 changes: 15 additions & 2 deletions sway-core/src/ir_generation/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2531,10 +2531,10 @@ impl<'eng> FnCompiler<'eng> {
.add_metadatum(context, span_md_idx);

// The type of a storage access is `StorageKey` which is a struct containing
// a `b256` and ` u64`.
// a `b256`, `u64` and `b256`.
let b256_ty = Type::get_b256(context);
let uint64_ty = Type::get_uint64(context);
let storage_key_aggregate = Type::new_struct(context, vec![b256_ty, uint64_ty]);
let storage_key_aggregate = Type::new_struct(context, vec![b256_ty, uint64_ty, b256_ty]);

// Local variable holding the `StorageKey` struct
let storage_key_local_name = self.lexical_map.insert_anon();
Expand Down Expand Up @@ -2575,6 +2575,19 @@ impl<'eng> FnCompiler<'eng> {
.store(gep_1_val, offset_within_slot_val)
.add_metadatum(context, span_md_idx);

// Store the field identifier as the third field in the `StorageKey` struct
let unique_field_id = get_storage_key(ix, indices); // use the indices to get a field id that is unique even for zero-sized values that live in the same slot
let field_id = convert_literal_to_value(context, &Literal::B256(unique_field_id.into()))
.add_metadatum(context, span_md_idx);
let gep_2_val =
self.current_block
.ins(context)
.get_elem_ptr_with_idx(storage_key, b256_ty, 2);
self.current_block
.ins(context)
.store(gep_2_val, field_id)
.add_metadatum(context, span_md_idx);

Ok(storage_key)
}
}
4 changes: 4 additions & 0 deletions sway-lib-core/src/storage.sw
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ library;
/// Describes a location in storage specified by the `b256` key of a particular storage slot and an
/// offset, in words, from the start of the storage slot at `key`. The parameter `T` is the type of
/// the data to be read from or written to at `offset`.
/// `field_id` is a unique identifier for the storage field being referred to, it is different even
/// for multiple zero sized fields that might live at the same location but
/// represent different storage constructs.
pub struct StorageKey<T> {
slot: b256,
offset: u64,
field_id: b256,
}
3 changes: 3 additions & 0 deletions sway-lib-std/src/storage/storage_key.sw
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ impl<T> StorageKey<T> {
/// let r: StorageKey<u64> = StorageKey {
/// slot: 0x0000000000000000000000000000000000000000000000000000000000000000,
/// offset: 2,
/// field_id: 0x0000000000000000000000000000000000000000000000000000000000000000,
/// };
///
/// // Reads the third word from storage slot with key 0x000...0
Expand Down Expand Up @@ -49,6 +50,7 @@ impl<T> StorageKey<T> {
/// let r: StorageKey<u64> = StorageKey {
/// slot: 0x0000000000000000000000000000000000000000000000000000000000000000,
/// offset: 2,
/// field_id: 0x0000000000000000000000000000000000000000000000000000000000000000,
/// };
///
/// // Reads the third word from storage slot with key 0x000...0
Expand All @@ -74,6 +76,7 @@ impl<T> StorageKey<T> {
/// let r: StorageKey<u64> = StorageKey {
/// slot: 0x0000000000000000000000000000000000000000000000000000000000000000,
/// offset: 2,
/// field_id: 0x0000000000000000000000000000000000000000000000000000000000000000,
/// };
/// let x = r.write(42); // Writes 42 at the third word of storage slot with key 0x000...0
/// }
Expand Down
5 changes: 3 additions & 2 deletions sway-lib-std/src/storage/storage_map.sw
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ impl<K, V> StorageKey<StorageMap<K, V>> {
/// ```
#[storage(read, write)]
pub fn insert(self, key: K, value: V) {
let key = sha256((key, self.slot));
let key = sha256((key, self.field_id));
write::<V>(key, 0, value);
}

Expand Down Expand Up @@ -60,8 +60,9 @@ impl<K, V> StorageKey<StorageMap<K, V>> {
/// ```
pub fn get(self, key: K) -> StorageKey<V> {
StorageKey {
slot: sha256((key, self.slot)),
slot: sha256((key, self.field_id)),
offset: 0,
field_id: sha256((key, self.field_id)),
}
}

Expand Down
21 changes: 12 additions & 9 deletions sway-lib-std/src/storage/storage_vec.sw
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ impl<V> StorageKey<StorageVec<V>> {
let len = read::<u64>(self.slot, 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.slot));
let key = sha256((len, self.field_id));
write::<V>(key, 0, value);

// Incrementing the length
Expand Down Expand Up @@ -86,7 +86,7 @@ impl<V> StorageKey<StorageVec<V>> {
// reduces len by 1, effectively removing the last item in the vec
write(self.slot, 0, len - 1);

let key = sha256((len - 1, self.slot));
let key = sha256((len - 1, self.field_id));
read::<V>(key, 0)
}

Expand Down Expand Up @@ -126,8 +126,9 @@ impl<V> StorageKey<StorageVec<V>> {
}

Some(StorageKey {
slot: sha256((index, self.slot)),
slot: sha256((index, self.field_id)),
offset: 0,
field_id: sha256((index, self.field_id)),
})
}

Expand Down Expand Up @@ -182,7 +183,7 @@ impl<V> StorageKey<StorageVec<V>> {
let mut count = index + 1;
while count < len {
// gets the storage location for the previous index
let key = sha256((count - 1, self.slot));
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());

Expand Down Expand Up @@ -292,7 +293,7 @@ impl<V> StorageKey<StorageVec<V>> {
// if the index is higher than or equal len, there is no element to set
assert(index < len);

let key = sha256((index, self.slot));
let key = sha256((index, self.field_id));
write::<V>(key, 0, value);
}

Expand Down Expand Up @@ -344,7 +345,7 @@ impl<V> StorageKey<StorageVec<V>> {

// if len is 0, index must also be 0 due to above check
if len == index {
let key = sha256((index, self.slot));
let key = sha256((index, self.field_id));
write::<V>(key, 0, value);

// increments len by 1
Expand All @@ -358,7 +359,7 @@ impl<V> StorageKey<StorageVec<V>> {
// performed in reverse to prevent data overwriting
let mut count = len - 1;
while count >= index {
let key = sha256((count + 1, self.slot));
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());

Expand All @@ -367,7 +368,7 @@ impl<V> StorageKey<StorageVec<V>> {
}

// inserts the value into the now unused index
let key = sha256((index, self.slot));
let key = sha256((index, self.field_id));
write::<V>(key, 0, value);

// increments len by 1
Expand Down Expand Up @@ -541,8 +542,9 @@ impl<V> StorageKey<StorageVec<V>> {
match read::<u64>(self.slot, 0).unwrap_or(0) {
0 => None,
_ => Some(StorageKey {
slot: sha256((0, self.slot)),
slot: sha256((0, self.field_id)),
offset: 0,
field_id: sha256((0, self.field_id)),
}),
}
}
Expand Down Expand Up @@ -576,6 +578,7 @@ impl<V> StorageKey<StorageVec<V>> {
len => Some(StorageKey {
slot: sha256((len - 1, self.slot)),
offset: 0,
field_id: sha256((0, self.field_id)),
}),
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[[package]]
name = 'core'
source = 'path+from-root-C02956062CE96F8B'

[[package]]
name = 'empty_fields_in_storage_struct'
source = 'member'
dependencies = ['std']

[[package]]
name = 'std'
source = 'path+from-root-C02956062CE96F8B'
dependencies = ['core']
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[project]
authors = ["Fuel Labs <[email protected]>"]
license = "Apache-2.0"
name = "empty_fields_in_storage_struct"
entry = "main.sw"

[dependencies]
std = { path = "../../../../../../sway-lib-std" }
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
contract;

use std::storage::storage_map::*;

abi ReproAttempt {
#[storage(read, write)]
fn foo_insert(key: u64, value: u64);

#[storage(read)]
fn foo_get(key: u64) -> Option<u64>;

#[storage(read, write)]
fn bar_insert(key: u64, value: u64);

#[storage(read)]
fn bar_get(key: u64) -> Option<u64>;
}

struct StructOfStorageMaps {
foo: StorageMap<u64, u64>,
bar: StorageMap<u64, u64>,
}

storage {
struct_of_maps: StructOfStorageMaps = StructOfStorageMaps {
foo: StorageMap {},
bar: StorageMap {},
},
}

impl ReproAttempt for Contract {
#[storage(read, write)]
fn foo_insert(key: u64, value: u64) {
storage.struct_of_maps.foo.insert(key, value);
}

#[storage(read)]
fn foo_get(key: u64) -> Option<u64> {
storage.struct_of_maps.foo.get(key).try_read()
}

#[storage(read, write)]
fn bar_insert(key: u64, value: u64) {
storage.struct_of_maps.bar.insert(key, value);
}

#[storage(read)]
fn bar_get(key: u64) -> Option<u64> {
storage.struct_of_maps.bar.get(key).try_read()
}
}

#[test()]
fn test_read_write() {
let repro = abi(ReproAttempt, CONTRACT_ID);

assert(repro.foo_get(1).is_none());
repro.foo_insert(1, 2);
assert(repro.foo_get(1).unwrap() == 2);

assert(repro.bar_get(1).is_none());
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
contract;

mod wallet_abi;

use std::{
auth::AuthError,
call_frames::msg_asset_id,
constants::BASE_ASSET_ID,
context::msg_amount,
token::transfer_to_address,
};

use wallet_abi::Wallet;
const OWNER_ADDRESS = Address::from(0x8900c5bec4ca97d4febf9ceb4754a60d782abbf3cd815836c1872116f203f861);

storage {
balance: u64 = 0,
}

impl Wallet for Contract {
#[payable, storage(read, write)]
fn receive_funds() {
if msg_asset_id() == BASE_ASSET_ID {
storage.balance.write(storage.balance.read() + msg_amount());
}
}

#[storage(read, write)]
fn send_funds(amount_to_send: u64, recipient_address: Address) {
let sender: Result<Identity, AuthError> = msg_sender();
match sender.unwrap() {
Identity::Address(addr) => assert(addr == OWNER_ADDRESS),
_ => revert(0),
};

let current_balance = storage.balance.read();
assert(current_balance >= amount_to_send);

storage.balance.write(current_balance - amount_to_send);

transfer_to_address(amount_to_send, BASE_ASSET_ID, recipient_address);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
library;

abi Wallet {
#[payable, storage(read, write)]
fn receive_funds();

// non-payable method
#[storage(read, write)]
fn send_funds(amount_to_send: u64, recipient_address: Address);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
category = "unit_tests_pass"
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ script;
use basic_storage_abi::{BasicStorage, Quad};

fn main() -> u64 {
let addr = abi(BasicStorage, 0xb58bd85320e71f5d4f079ee969c7eeda8bf6284d0604b8ff70806355ab90aef4);
let addr = abi(BasicStorage, 0x8ad1c620170e6706427ab0f94feac233947a7f6d61a14e57788c257fcef62360);
let key = 0x0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff;
let value = 4242;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ script;
use increment_abi::Incrementor;

fn main() -> bool {
let the_abi = abi(Incrementor, 0xabc8916cc1d37a762308deab3fc46e2fbe2ce3b9c918002c28368e0e026aa418);
let the_abi = abi(Incrementor, 0xabaff9d9d138a89c5d1235e982981f206cf09f0078049c4f1f657005eb5d7484);
the_abi.increment(5);
the_abi.increment(5);
let result = the_abi.get();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use storage_access_abi::*;
use std::hash::sha256;

fn main() -> bool {
let contract_id = 0x0d2dcfa21906257722b0a983429c70ebfb543a4905e4913bd1f844bf7ae8f9ed;
let contract_id = 0xb1e7bac0bb97571cd18c26900c58421669f4f7fbe3bc5a47a8b768571d1c7974;
let caller = abi(StorageAccess, contract_id);

// Test initializers
Expand Down

0 comments on commit 97f3c5f

Please sign in to comment.