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.

Sign up for GitHub

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

Jump to bottom

Replace old storage API with the new one explained in the StorageKey RFC #4464

Merged
merged 5 commits into from
Apr 21, 2023
Merged

Replace old storage API with the new one explained in the StorageKey RFC #4464

merged 5 commits into from
Apr 21, 2023

Conversation

mohammadfawaz
Copy link
Contributor

@mohammadfawaz mohammadfawaz commented Apr 18, 2023 •
edited
Loading

Description

Implement FuelLabs/sway-rfcs#23

Closes #3419 (technically via the RFC)
Closes #3202
Closes #3043
Closes #2639
Closes #2465
Closes #4304

This is a big one but mostly removes stuff. It's hard to review but the summary of changes below should be sufficient. Most of the changes here are things that I just had to do to make CI pass.

  • Removed the --experimental-storage flag and made its behavior the default. Also removed everything that was required for the previous storage APIs in favour of the new ones.
  • Break down the std::storage into multiple submodules:
  • storage_key implements the API for std::core::StorageKey
  • storage_api implements the free functions read (previously get), write (previously store), and clear.
  • 4 more modules for the dynamic storage types which now use the new StorageKey API.
  • #[storage(write)] now allows reading from storage as well. This is needed because the way we pack structs in storage now requires that we sometimes read from storage first if we were to write a portion of a slot.
  • Removed the "storage only types" checks and the corresponding tests.
  • Removed the __get_storage_key intrinsic and the get_storage_key IR instruction and all corresponding tests. Also removed the state_index metadata as it is no longer required.
  • Added tests and example to showcase nested storage maps and nested storage vectors.

Checklist

  • I have linked to any relevant issues.
  • 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).
  • 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.
  • I have done my best to ensure that my PR adheres to the Fuel Labs Code Review Standards.
  • I have requested a review from the relevant team or maintainers.

@mohammadfawaz mohammadfawaz changed the title Replace old storage API with the new one explained in https://github.com/FuelLabs/sway-rfcs/pull/23 Replace old storage API with the new one explained in the StorageKey RFC Apr 18, 2023
@mohammadfawaz mohammadfawaz force-pushed the mohammadfawaz/new_storage branch 4 times, most recently from 8aa8c48 to b84161a Compare April 19, 2023 01:22
@mohammadfawaz mohammadfawaz self-assigned this Apr 19, 2023
@mohammadfawaz mohammadfawaz added language server LSP server lib: std Standard library compiler: ir IRgen and sway-ir including optimization passes language feature Core language features visible to end users breaking May cause existing user code to break. Requires a minor or major release. compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen compiler: parser Everything to do with the parser labels Apr 19, 2023
@mohammadfawaz mohammadfawaz force-pushed the mohammadfawaz/new_storage branch 17 times, most recently from a2165ab to c246e05 Compare April 20, 2023 02:03
@mohammadfawaz mohammadfawaz requested a review from a team April 20, 2023 02:30
@mohammadfawaz mohammadfawaz marked this pull request as ready for review April 20, 2023 02:30
This was referenced Apr 20, 2023
Open
Merged
@JoshuaBatty JoshuaBatty mentioned this pull request Apr 21, 2023
Closed
@JoshuaBatty JoshuaBatty requested review from a team April 21, 2023 00:22
@JoshuaBatty JoshuaBatty requested review from a team April 21, 2023 03:08
mitchmindtree
mitchmindtree approved these changes Apr 21, 2023
View reviewed changes
Copy link
Contributor

@mitchmindtree mitchmindtree left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one @mohammadfawaz, great to see how much this simplifies! Loving these detailed docs with examples too - good motivation to address #3176. I just left a couple Qs/suggestions but no blockers.

sway-lib-std/src/storage.sw Show resolved Hide resolved
sway-lib-std/src/storage/storage_api.sw Show resolved Hide resolved
test/src/e2e_vm_tests/test_programs/should_fail/generics_in_contract/src/main.sw Show resolved Hide resolved
@JoshuaBatty JoshuaBatty requested review from a team April 21, 2023 04:48
JoshuaBatty
JoshuaBatty approved these changes Apr 21, 2023
View reviewed changes
Copy link
Member

@JoshuaBatty JoshuaBatty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super impressive work, thanks boss :)

@JoshuaBatty JoshuaBatty requested review from a team April 21, 2023 05:06
@mohammadfawaz mohammadfawaz merged commit 1db8385 into master Apr 21, 2023
25 checks passed
@mohammadfawaz mohammadfawaz deleted the mohammadfawaz/new_storage branch April 21, 2023 11:36
@iqdecay iqdecay mentioned this pull request May 3, 2023
Merged
iqdecay added a commit that referenced this pull request May 3, 2023
@iqdecay
chore: remove leftover build.sh (#4529)
befca87 
This file was removed in #4476  but reintroduced in #4464.
@Dhaiwat10 Dhaiwat10 mentioned this pull request May 25, 2023
Merged
7 tasks
@JoshuaBatty JoshuaBatty mentioned this pull request Jul 18, 2023
Merged
7 tasks
JoshuaBatty added a commit that referenced this pull request Jul 18, 2023
@JoshuaBatty
Collect storage keyword and assign type_def to storage fields (#4810)
62f9857 
## Description
The storage tests in the language server were removed in #4464. That PR
also removed the ability for the language server to access the `storage`
token and assign correct type definition to storage fields. This PR adds
all that functionality back in.

closes #4641
closes #4482
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JoshuaBatty JoshuaBatty JoshuaBatty approved these changes

@mitchmindtree mitchmindtree mitchmindtree approved these changes

Assignees

@mohammadfawaz mohammadfawaz

Labels
breaking May cause existing user code to break. Requires a minor or major release. compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen compiler: ir IRgen and sway-ir including optimization passes compiler: parser Everything to do with the parser language feature Core language features visible to end users language server LSP server lib: std Standard library
Projects
None yet
Milestone
No milestone
3 participants
@mohammadfawaz @JoshuaBatty @mitchmindtree