Storage operations refactor.

[g-r-a-n-t]

What was wrong?

The operations sstore and sload were being used incorrectly. We were passing in byte pointers, when we should have been using words.

How was it fixed?

Despite the fact that these operations take word pointers, I think we should continue using byte pointers to express storage values. This allows us to store composite/array types more gas efficiently, since we can reference multiple values inside of a single word. We also get to keep the same internal encoding of values in storage and memory.

To deal with this, the storage pointers expressed in bytes are simply converted to word and byte offset pairs when it comes time to load or manipulate storage. One tricky part of this is that values cannot span multiple words in storage. In memory, you can load/store a word at any byte address, meaning you can have a packed array of 20 byte values, without having to work with multiple words. This is not the case in storage, we can only address words. So for example, the 2nd address in an address array would occupy two words. Trying to deal with values spanning multiple words would be too impractical, so we should store values in such a way that no single value spans multiple words. For now, this is only an issue with address arrays, but it is something that we will also need to consider when storing structs more efficiently.

To-Do

• Update storage operations and add thorough testing

• Update everything that uses these storage operations

• Fix encoding of address arrays

• Add entry to the release notes (may forgo for trivial changes)

• Clean up commit history

[codecov-io]

Codecov Report

Merging #251 (91bad63) into master (ce2834f) will increase coverage by 0.02%.
The diff coverage is 97.91%.

@@            Coverage Diff             @@
##           master     #251      +/-   ##
==========================================
+ Coverage   93.99%   94.02%   +0.02%     
==========================================
  Files          54       54              
  Lines        3765     3783      +18     
==========================================
+ Hits         3539     3557      +18     
  Misses        226      226              

Impacted Files	Coverage Δ
compiler/src/yul/mappers/assignments.rs	97.82% <ø> (ø)
compiler/src/yul/mappers/declarations.rs	100.00% <ø> (ø)
analyzer/src/namespace/types.rs	82.50% <50.00%> (ø)
analyzer/src/namespace/events.rs	100.00% <100.00%> (ø)
common/src/utils/keccak.rs	100.00% <100.00%> (ø)
compiler/src/abi/utils.rs	71.42% <100.00%> (ø)
compiler/src/yul/mappers/expressions.rs	97.24% <100.00%> (-0.02%)	⬇️
compiler/src/yul/operations/data.rs	100.00% <100.00%> (ø)
compiler/src/yul/runtime/functions/data.rs	100.00% <100.00%> (ø)
compiler/src/yul/runtime/functions/mod.rs	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ce2834f...6232252. Read the comment docs.

[cburgdorf]

I think it would be good to discuss this PR in our meeting today. I think I understand the general rational and also this part.

In memory, you can load/store a word at any byte address, meaning you can have a packed array of 20 byte values, without having to work with multiple words. This is not the case in storage, we can only address words. So for example, the 2nd address in an address array would occupy two words.

However, this PR doesn't seem to only affect the size in storage but the size in memory, too.

If I have the following Fe code:

    pub def bar(someone: address):
        folks: address[1]
        folks[0] = someone

Then on current master it will compile to this:

function $$bar($someone) {
    let $folks := alloc(20)
    mstoren(add($folks, mul(0, 20)), $someone, 20) 
}

But with this PR it will compile to this:

function $$bar($someone) {
    let $folks := alloc(32)
    mstoren(add($folks, mul(0, 32)), 32, $someone) 
}

So, it seems that we would be wasting space in memory even if we don't need to.