Skip to content
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

StorageVec::insert() does not work with nested storage types #6040

Open
ironcev opened this issue May 21, 2024 · 0 comments
Open

StorageVec::insert() does not work with nested storage types #6040

ironcev opened this issue May 21, 2024 · 0 comments
Labels
bug Something isn't working lib: std Standard library

Comments

@ironcev
Copy link
Member

ironcev commented May 21, 2024

E.g., having:

storage {
    nested_vec: StorageVec<StorageVec<u64>> = StorageVec {},
}

and inserting:

storage.nested_vec.insert(0, StorageVec {});
storage.nested_vec.insert(0, StorageVec {});

will revert at runtime at the second insert. At the same time:

storage.nested_vec.push(StorageVec {});
storage.nested_vec.push(StorageVec {});

works.

The same is with e.g. StorageVec<StorageString>.

For additional examples, see commented out tests in #6043 that are marked with TODOs that relate to this issue.

The bug might be in the fact that in the StorageVec::insert() these lines:

let write_offset = offset_calculator::<V>(count + 1);
let read_offset = offset_calculator::<V>(count);
write::<V>(key, write_offset, read::<V>(key, read_offset).unwrap());

will be calculating the same write_offset and read_offset for zero-size types.

To that, this issue is very likely an issue with all zero-sized types and not only storage types. It's just that nobody usually stores zero-size types in the storage.

As a general remark, I am not sure if fixing this issue makes sense before Storage v3.0 is implemented.

@ironcev ironcev added bug Something isn't working lib: std Standard library labels May 21, 2024
ironcev added a commit that referenced this issue May 22, 2024
## Description

This PR fixes #6036 by using `field_id()` in the `StorageString` instead
of `slot()`.

Additionally, the PR:
- fixes documentation glitches in the `StorageVec`.
- optimizes `StorageMap::get` by removing double calculation of the
`key`.
- cleans up most of the Sway and all Rust warnings in the SDK harness
tests.

The PR is not solving all the issues related to composing dynamic
storage types. E.g, #6040 is (very likely just one) of them.

Closes #6036.

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [ ] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [ ] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [ ] 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working lib: std Standard library
Projects
None yet
Development

No branches or pull requests

1 participant