Implement storage access via storage keyword #646

sezna · 2022-01-15T01:27:24Z

This closes #645. This is a WIP currently.

Remaining:

parse storage accesses
type check storage accesses
build out some storage data structures and rules around what works and what doesn't in a storage block (tl;dr: implement mapping)
correctness tests

This enables storage declarations, accesses, and reassignment via the storage keyword.

Syntax

Storage Declaration

storage {
    <id>: <type>,
    ....
}

Storage Accesses

    ... = storage.<var>;
    ... = storage.<struct_var>;
    ... = storage.<struct_var>.<field>....

Storage Reassignment

    storage.<var> = ...
    storage.<struct_var> = ...
    storage.<struct_var>.<field> = ....

Notes

Each subfield is stored in its own storage slot (i.e. gets its own key). By subfield, I mean the most primitive type of an aggregate, though not all aggregates are fully supported yet. For example for the following storage:

struct T { x: u64, y: b256 } 
struct S { t: T };
storage {
    s: S,    
}

x and y will get their own storage slots.

This only works with IR so the test I added won't pass for now, though I did test it manually.
Functionally, this PR also depends on Contracts and contract calls in IR #976.

This example should be updated after FuelLabs#646 lands.

… mdbook preprocessor. (#766) * Extract `hello_world` example from the book into a standalone project Related to #544. Follows the example set in #731. Tested with `mdbook serve`. Also adds the `hello_world` and `subcurrency` examples to the Sway workspace so that we can test both projects under CI. * Fix hello_world example project name in forc manifest * Move fizzbuzz and wallet_smart_contract examples to standalone projects * Add `build-all-examples` script and associated CI passes This script walks the examples directory and, for every directory containing a `forc.toml` file, runs `forc build` for that directory. Examples that encounter a failure of some kind have their `stdout` and `stderr` piped through so we can easily identify cause of failure in CI. This also removes the examples from the workspace. Currently unsure whether or not the added CI pass to install `forc` will result in `forc` actually being available on the PATH in the CI worker, but will find out soon. Still need to work out how to run `forc test` for each project without having cargo complain about building within the parent sway workspace. * Add a clear build summary at the end of `build-all-examples` Currently, it's tricky to get an idea of how many successes/failures there were without combing through the stdout/stderr. This adds a little summary to the end that looks something like this: ``` Build all examples summary: [✓]: /home/mindtree/programming/rust/fuel/sway/examples/fizzbuzz succeeded! [✓]: /home/mindtree/programming/rust/fuel/sway/examples/hello_world succeeded! [x]: /home/mindtree/programming/rust/fuel/sway/examples/wallet_smart_contract failed! [✓]: /home/mindtree/programming/rust/fuel/sway/examples/subcurrency succeeded! 3 successes, 1 failure ``` * Set build-all-examples script version to 0.0.0 * Exclude examples from workspace, Fix hello_world test harness path * Temporarily comment out storage related code in wallet example This example should be updated after #646 lands. * Install forc with `--debug` to share build artifacts Currently it takes CI another 7 mins to build and install forc from scratch. This should allow `cargo` to re-use the artifacts it has already built during the sway building and testing steps earlier in the CI job. * Change author field in examples from mindtree -> Fuel Labs

mohammadfawaz · 2022-03-21T19:36:09Z

I will do all the purity/impurity checks and warnings in a separate PR.

Also, had to force push because consolidating my branch and this one would've been a nightmare.

..._tests/test_programs/should_pass/require_contract_deployment/storage_access_caller/Forc.toml

adlerjohn · 2022-03-21T19:59:30Z

...ests/test_programs/should_pass/require_contract_deployment/storage_access_caller/src/main.sw

+
+    // Test 1
+    caller.set_x {
+        gas: 10000


Is there a reason to specify gas here instead of default? My concern is that if we set the gas cost to a storage write to e.g. 20k like on Ethereum, then all these tests will fail. Why not just forward all gas?

All of our contract tests seem to be overriding gas.
When I remove this, I get an OutOfGas panic.

I get the same behavior with this test: https://github.com/FuelLabs/sway/blob/master/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/call_basic_storage/src/main.sw and the assembly for it looks correct. Here's the relevant snippet

... lw $r4 $cgas i0 ; loading $cgas (gas) into abi function lw $r3 data_3 ; loading the default coins value for call lw $r2 data_4 ; loading the default asset_id value for call lw $r1 data_5 ; literal instantiation move $r7 $sp cfei i48 mcpi $r7 $r1 i32 ; copy contract address for call sw $r7 $r0 i4 ; write fn selector to rA + 32 for call sw $r7 $r5 i5 ; move user param for call call $r7 $r3 $r2 $r4 ; call external contract ...

isn't $cgas meant to start with a tx.gasLimit?

$cgas starts at tx.gasLimit, yes. But with each contract call, it's initially the gas forwarded. Note that if you try to forward more than $cgas, then only $cgas is forwarded. It should not be a panic in the VM.

test/src/e2e_vm_tests/mod.rs

adlerjohn · 2022-03-21T20:01:58Z

Can't seem to add @sezna as a reviewer (maybe because he's the PR author?)

sezna · 2022-03-21T23:28:17Z

Can't seem to add @sezna as a reviewer (maybe because he's the PR author?)

taking a look!

sway-core/src/semantic_analysis/namespace.rs

sway-core/src/sway.pest

...ests/test_programs/should_pass/require_contract_deployment/storage_access_caller/src/main.sw

Co-authored-by: Alex Hansen <alex@alex-hansen.com>

sezna · 2022-03-22T17:42:15Z

I would approve but I'm the author, so I can't. Cc @otrho perhaps since he has some context?

Cargo.lock

mohammadfawaz · 2022-03-23T15:43:30Z

@emilyaherbert merging with master was slightly non-trivial due to the new ParserLifter that you introduced. Will you please make sure I didn't mess anything up?

emilyaherbert · 2022-03-23T22:54:26Z

@emilyaherbert merging with master was slightly non-trivial due to the new ParserLifter that you introduced. Will you please make sure I didn't mess anything up?

LGTM

mohammadfawaz · 2022-03-24T01:16:39Z

I'm merging this even though the test stopped working now due some changes that Toby had to (rightfully) make to the contract call stuff in IR to make the verifier work as it needs. Toby and I will be dealing with that asap.

The AST generated for storage has not changed and so, this change continues to do the right thing as it was tested before Toby made the verifier changes.

sezna added enhancement New feature or request compiler General compiler. Should eventually become more specific as the issue is triaged labels Jan 15, 2022

sezna self-assigned this Jan 15, 2022

mitchmindtree added a commit to mitchmindtree/sway that referenced this pull request Feb 11, 2022

Temporarily comment out storage related code in wallet example

4da1bee

This example should be updated after FuelLabs#646 lands.

otrho mentioned this pull request Mar 3, 2022

Provision IR for future storage API. #866

Merged

Mohammad and others added 15 commits March 10, 2022 10:01

StorageDeclaration and StorageAccess

1065c1e

clippy and fmt

125ba7c

More decl stuff

7a391f1

ir codegen for srwq

f9293e6

new IR instruction

bbfa042

many ir changes

e72c4a9

start of reassignment

c457bb5

merge with master

71f2939

updated test

de39112

wip

12be058

wip

9d81ec7

wip

cbb7e0b

merge with master

6d83509

multi-access

6392294

merging some functions

2d68e2e

mohammadfawaz mentioned this pull request Mar 17, 2022

IR generation for storage accesses #959

Merged

mohammadfawaz added 8 commits March 17, 2022 20:46

merge with master

9d5c1bd

unnecessary changes

284416e

test

1703041

Initial commit

337da5e

fixing a few things

1e1e9fc

additional fixes

4c91105

adding tests

dbd0e80

fixing default value for cgas

e02ab43

mohammadfawaz force-pushed the sezna/storage-access branch from f2629ba to c7e93d6 Compare March 21, 2022 19:26

fixing tests

c3ed975

mohammadfawaz requested review from emilyaherbert, otrho and adlerjohn March 21, 2022 19:35

mohammadfawaz marked this pull request as ready for review March 21, 2022 19:36

mohammadfawaz self-assigned this Mar 21, 2022

adlerjohn reviewed Mar 21, 2022

View reviewed changes

adlerjohn unassigned sezna Mar 21, 2022

mohammadfawaz mentioned this pull request Mar 21, 2022

Enable the test added in https://github.com/FuelLabs/sway/pull/646 once IR becomes the default pipeline #981

Closed

Update Forc.toml format

21ad7ce

sezna commented Mar 22, 2022

View reviewed changes

Update sway-core/src/semantic_analysis/namespace.rs

1105f47

Co-authored-by: Alex Hansen <alex@alex-hansen.com>

adlerjohn suggested changes Mar 22, 2022

View reviewed changes

Cargo.lock Outdated Show resolved Hide resolved

mohammadfawaz mentioned this pull request Mar 23, 2022

Remaining tasks for storage #1006

Open

7 tasks

mohammadfawaz added 3 commits March 23, 2022 11:38

merge with master

d2eb5fd

cargo.lock

40dd4e0

revert Cargo.lock

07b5804

emilyaherbert approved these changes Mar 23, 2022

View reviewed changes

Merge branch 'master' into sezna/storage-access

30fa5ed

adlerjohn approved these changes Mar 24, 2022

View reviewed changes

mohammadfawaz merged commit 85032fb into master Mar 24, 2022

mohammadfawaz deleted the sezna/storage-access branch March 24, 2022 01:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement storage access via storage keyword #646

Implement storage access via storage keyword #646

sezna commented Jan 15, 2022 •

edited by mohammadfawaz

Loading

mohammadfawaz commented Mar 21, 2022 •

edited

Loading

adlerjohn Mar 21, 2022

mohammadfawaz Mar 21, 2022

adlerjohn Mar 22, 2022

adlerjohn commented Mar 21, 2022

sezna commented Mar 21, 2022 •

edited

Loading

sezna commented Mar 22, 2022

mohammadfawaz commented Mar 23, 2022 •

edited by adlerjohn

Loading

emilyaherbert commented Mar 23, 2022

mohammadfawaz commented Mar 24, 2022

Implement storage access via storage keyword #646

Implement storage access via storage keyword #646

Conversation

sezna commented Jan 15, 2022 • edited by mohammadfawaz Loading

Syntax

Storage Declaration

Storage Accesses

Storage Reassignment

Notes

mohammadfawaz commented Mar 21, 2022 • edited Loading

adlerjohn Mar 21, 2022

Choose a reason for hiding this comment

mohammadfawaz Mar 21, 2022

Choose a reason for hiding this comment

adlerjohn Mar 22, 2022

Choose a reason for hiding this comment

adlerjohn commented Mar 21, 2022

sezna commented Mar 21, 2022 • edited Loading

sezna commented Mar 22, 2022

mohammadfawaz commented Mar 23, 2022 • edited by adlerjohn Loading

emilyaherbert commented Mar 23, 2022

mohammadfawaz commented Mar 24, 2022

sezna commented Jan 15, 2022 •

edited by mohammadfawaz

Loading

mohammadfawaz commented Mar 21, 2022 •

edited

Loading

sezna commented Mar 21, 2022 •

edited

Loading

mohammadfawaz commented Mar 23, 2022 •

edited by adlerjohn

Loading