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

Storage domains #6701

Open
1 task done
ironcev opened this issue Nov 6, 2024 · 0 comments
Open
1 task done

Storage domains #6701

ironcev opened this issue Nov 6, 2024 · 0 comments
Assignees
Labels
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 General compiler. Should eventually become more specific as the issue is triaged tracking-issue Tracking issue for experimental Sway features

Comments

@ironcev
Copy link
Member

ironcev commented Nov 6, 2024

This is a tracking issue for Storage domains.
The experimental feature flag for the issue is storage_domains.

Description

Certain storage types, like, e.g., StorageMap allow storage slots of their contained elements to be defined based on developer's input. E.g., the key in a StorageMap used to calculate the storage slot is a developer input.

To ensure that pre-images of such storage slots can never be the same as a pre-image of compiler generated key of a storage field, we will prefix the pre-images of storage fields with a single byte that denotes the storage field domain. Storage types like StorageMap must have a different domain prefix than this STORAGE_DOMAIN which will be set to 0u8.

For detailed elaboration see #6317.

Breaking Changes

Storage domains changes how storage keys are generated for storage fields. The previous formula for calculating storage fields' keys was:

sha256("storage::<optional namespace 1>::<optional namespace 2>.<field name>")

The new formula prefixes the field path with 0u8:

sha256((0u8, "storage::<optional namespace 1>::<optional namespace 2>.<field name>"))

To calculate old and new slot keys for storage field paths, you can run this Rust code.

Review explicitly defined slot keys in storage declarations (in keywords)

If you've used the in keyword in storage declarations to manually assign slot keys, and if the assigned keys were representing the keys of storage fields of contracts that will be newly compiled, you might want to recalculate the storage keys.

For illustration, in this example, we have three storage fields and three fields declared using the in keyword that hard-code the slot keys of the other three fields. The example shows the old and the new keys, calculated with the old and the new formula, respectively.

storage {
    // Old slot key: c979570128d5f52725e9a343a7f4992d8ed386d7c8cfd25f1c646c51c2ac6b4b
    // New slot key: 419b1120ea993203d7e223dfbe76184322453d6f8de946e827a8669102ab395b
    x: u64 = 0,
    namespace_1 {
        // Old slot key: 2f055029200cd7fa6751421635c722fcda6ed2261de0f1e0d19d7f257e760589
        // New slot key: 50a62a537821cd5e77fd4093bb0febc39a0253b2bf65ffcdffb7297d9556afac
        y: u64 = 0,
        namespace_2 {
            // Old slot key: 03d2ee7fb8f3f5e1084e86b02d9d742ede96559e44875c6210c7008e2d184694
            // New slot key: e6967a9e92c754da39a64baaaec3d8a56194ac400f5dd66cc2b509a77ea98a64
            z: u64 = 0,
        }
    }

    // Review if these keys should be replaced with the new keys.
    same_as_x in 0xc979570128d5f52725e9a343a7f4992d8ed386d7c8cfd25f1c646c51c2ac6b4b: u64 = 0,
    same_as_y in 0x2f055029200cd7fa6751421635c722fcda6ed2261de0f1e0d19d7f257e760589: u64 = 0,
    same_as_z in 0x03d2ee7fb8f3f5e1084e86b02d9d742ede96559e44875c6210c7008e2d184694: u64 = 0,
}

Explicitly define storage slot keys if they need to be backward compatible

If the contract owning the storage is behind a proxy, its storage field keys must be backward compatible and the same as the old keys. In this, and any other case where the backward compatibility of the storage slot keys is needed, the old keys must be explicitly defined for storage fields, by using the in keyword and the old keys.

E.g., assume we have a contract with the following storage behind a proxy:

storage {
    x: u64 = 0,
    namespace_1 {
        y: u64 = 0,
        namespace_2 {
            z: u64 = 0,
        }
    }
}

To keep the fields' storage slots backward compatible, we must give them slot keys calculated using the old formula:

storage {
    x in 0xc979570128d5f52725e9a343a7f4992d8ed386d7c8cfd25f1c646c51c2ac6b4b: u64 = 0,
    namespace_1 {
        y in 0x2f055029200cd7fa6751421635c722fcda6ed2261de0f1e0d19d7f257e760589: u64 = 0,
        namespace_2 {
            z in 0x03d2ee7fb8f3f5e1084e86b02d9d742ede96559e44875c6210c7008e2d184694: u64 = 0,
        }
    }
}

Steps

@ironcev ironcev self-assigned this Nov 6, 2024
@ironcev ironcev added compiler General compiler. Should eventually become more specific as the issue is triaged compiler: ir IRgen and sway-ir including optimization passes compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen tracking-issue Tracking issue for experimental Sway features labels Nov 6, 2024
@ironcev ironcev mentioned this issue Nov 6, 2024
8 tasks
IGI-111 added a commit that referenced this issue Nov 15, 2024
## Description

This PR fixes #6317 by implementing `storage_domains` experimental
feature.

This feature introduces two _storage domains_, one for the storage
fields defined inside of the `storage` declaration, and a different one
for storage slots generated inside of the `StorageMap`.

The PR strictly fixes the issue described in #6317 by applying the
recommendation proposed in the issue.
A general approach to storage key domains will be discussed as a part of
the [Configurable and composable storage
RFC](FuelLabs/sway-rfcs#40).

Additionally, the PR:
- adds expressive diagnostics for the duplicated storage keys warning.
- removes the misleading internal compiler error that always followed
the storage key type mismatch error (see demo below).

Closes #6317, #6701.

## Breaking Changes

The PR changes the way how storage keys are generated for `storage`
fields. Instead of `sha256("storage::ns1::ns2.field_name")` we now use
`sha256((0u8, "storage::ns1::ns2.field_name"))`.

This is a breaking change for those who relied on the storage key
calculation.

## Demo

Before, every type-mismatch was always followed by an ICE, because the
compilation continued despite the type-mismatch.

![Mismatched types and ICE -
Before](https://github.com/user-attachments/assets/ac7915f7-3458-409e-a2bb-118dd4925234)

This is solved now, and the type-mismatch has a dedicated help message.

![Mismatched types and ICE -
After](https://github.com/user-attachments/assets/570aedd3-4c9c-4945-bfd0-5f12d68dbead)

## 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.
- [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]>
Co-authored-by: IGI-111 <[email protected]>
@IGI-111 IGI-111 mentioned this issue Jan 6, 2025
8 tasks
IGI-111 pushed a commit that referenced this issue Jan 20, 2025
## Description

This PR introduces `forc-migrate`, a Forc tool for migrating Sway
projects to the next breaking change version of Sway.

The tool addresses two points crucial for code updates caused by
breaking changes:
- it informs developers about the breaking changes and **assists in
planing and executing** the migration.
- it **automatically changes source code** where possible, reducing the
manual effort needed for code changes.

Besides adding the `forc-migrate` tool, the PR:
- extends `Diagnostic` to support migration diagnostics aside with
errors and warnings.
- changes `swayfmt` to support generating source code from arbitrary
lexed trees. The change is a minimal one done only in the parts of
`swayfmt` that are executed by migration steps written in this PR.
Adapting `swayfmt` to fully support arbitrary lexed trees will be done
in #6779.

The migration for the `references` feature, migrating `ref mut` to
`&mut`, is developed only partially, to demonstrate the development and
usage of automatic migrations that alter the original source code.

The intended usage of the tool is documented in detail in the "forc
migrate" chapter of The Sway Book: _Forc reference > Plugins >
forc_migrate_. (The generated documentation has issues that are caused
by the documentation generation bug explained in #6792. These issues
will be fixed in a separate PR that will fix it for all the plugins.)

We expect the `forc-migrate` to evolve based on the developer's
feedback. Some of the possible extensions of the tool are:
- adding additional CLI options, e.g., for executing only specific
migration steps, or ignoring them.
- passing parameters to migration steps from the CLI.
- not allowing updates by default, if the repository contains modified
or untracked files.
- migrating workspaces.
- migrating other artifacts, e.g., Forc.toml files or contract IDs.
- migrating between arbitrary versions of Sway.
- migrating SDK code.
- etc.
 
`forc-migrate` also showed a clear need for better infrastructure for
writing static analyzers and transforming Sway code. The approach used
in the implementation of this PR should be seen as a pragmatic
beginning, based on the reuse of what we currently have. Some future
options are discussed in #6836.

## Demo

### `forc migrate show`

Shows the breaking change features and related migration steps. This
command can be run anywhere and does not require a Sway project.

```
Breaking change features:
  - storage_domains    (#6701)
  - references         (#5063)

Migration steps (1 manual and 1 semiautomatic):
storage_domains
  [M] Review explicitly defined slot keys in storage declarations (`in` keywords)

references
  [S] Replace `ref mut` function parameters with `&mut`

Experimental feature flags:
- for Forc.toml:  experimental = { storage_domains = true, references = true }
- for CLI:        --experimental storage_domains,references
```

### `forc migrate check`

Performs a dry-run of the migration on a concrete Sway project. It
outputs all the occurrences in code that need to be reviewed or changed,
as well as the migration time effort:

```
info: [storage_domains] Review explicitly defined slot keys in storage declarations (`in` keywords)
  --> /home/kebradalaonda/Desktop/M Forc migrate tool/src/main.sw:19:10
   |
...
19 |     y in b256::zero(): u64 = 0,
   |          ------------
20 |     z: u64 = 0,
21 |     a in calculate_slot_address(): u64 = 0,
   |          ------------------------
22 |     b in 0x0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f20: u64 = 0,
   |          ------------------------------------------------------------------
   |
   = help: If the slot keys used in `in` keywords represent keys generated for `storage` fields
   = help: by the Sway compiler, those keys might need to be recalculated.
   = help:  
   = help: The previous formula for calculating storage field keys was: `sha256("storage.<field name>")`.
   = help: The new formula is:                                          `sha256((0u8, "storage.<field name>"))`.
   = help:  
   = help: For a detailed migration guide see: #6701
____

Migration effort:

storage_domains
  [M] Review explicitly defined slot keys in storage declarations (`in` keywords)
      Occurrences:     3    Migration effort (hh::mm): ~00:06

references
  [S] Replace `ref mut` function parameters with `&mut`
      Occurrences:     0    Migration effort (hh::mm): ~00:00

Total migration effort (hh::mm): ~00:06
``` 

### `forc migrate run`

Runs the migration steps and guides developers through the migration
process.

## 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] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [ ] 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
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 General compiler. Should eventually become more specific as the issue is triaged tracking-issue Tracking issue for experimental Sway features
Projects
None yet
Development

No branches or pull requests

1 participant