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 tree differentiate between default and non-default #395

Closed
adlerjohn opened this issue Aug 3, 2022 · 8 comments
Closed

Storage tree differentiate between default and non-default #395

adlerjohn opened this issue Aug 3, 2022 · 8 comments
Labels
comp:FVM Component: FuelVM enhancement New feature or request

Comments

@adlerjohn
Copy link
Contributor

adlerjohn commented Aug 3, 2022

In light of the most recent exploit on Nomad (https://twitter.com/samczsun/status/1554260106107179010), there's a need to differentiate between default and non-default leaves when querying the contract storage tree.

Current Design

The default value of a leaf in the SMT is empty (i.e. zero bytes). However, when reading from storage, 0 is returned if the leaf is empty. This is lossy since it's discarding information.

| Notes | Returns zero if the state element does not exist. |

Moreover, there's no way to reset a storage slot; writing 0 is not the same thing as writing an empty value.

Proposed Changes

  1. Add a new instruction, to clear a storage slot. The instruction resets the given storage slot to the default value (i.e. zero bytes).
  2. Add an additional register parameter to SRW and SRWQ, that will be set to a Boolean for whether the given storage slot was empty or not.
@adlerjohn adlerjohn added enhancement New feature or request comp:FVM Component: FuelVM labels Aug 3, 2022
@adlerjohn adlerjohn moved this to Todo in Fuel Network Aug 3, 2022
@sezna
Copy link
Contributor

sezna commented Aug 3, 2022

Add an additional register parameter to SRW and SRWQ, that will hold a flag for whether the given storage slot was empty or not.

Does "empty" here mean zeroed out? Because that is also lossy. Or will the VM keep track of initialized (occupied) vs uninitialized (cleared) slots in a separate bitfield?

@adlerjohn
Copy link
Contributor Author

adlerjohn commented Aug 3, 2022

The SMT already differentiates between "empty, i.e. zero bytes" and "32 bytes of zeroes," and therefore so does the VM implicitly. The proposed changes simply expose this in the instruction set.

@SwayStar123
Copy link
Collaborator

this is quite poggers if i say so myself

@SwayStar123
Copy link
Collaborator

SwayStar123 commented Aug 3, 2022

Add an additional register parameter to SRW and SRWQ, that will hold a flag for whether the given storage slot was empty or not.

Not familiar with what SRW and SRWQ are, I thought of a flag type system before to solve this problem (like the sign on a signed int) but then I realized that would tack on a 1 bit cost to every single slot used, might add up quickly? Im assuming thats how SRW works aswell, do you not see this as a potential problem?

@adlerjohn
Copy link
Contributor Author

There's actually no need for any changes to the SMT because the SMT actually already differentiates between a default leaf and a zero-value non-default leaf. The extra register parameter to SRW and SRWQ would simply be set to zero or one depending on whether the slot was default or non-default.

@sezna
Copy link
Contributor

sezna commented Aug 3, 2022

If we already have this differentiation we should absolutely surface it in SRW and SRWQ. It's free money.

@ghost ghost self-assigned this Oct 10, 2022
@ghost ghost moved this from Todo to In Progress in Fuel Network Oct 10, 2022
ghost pushed a commit that referenced this issue Oct 12, 2022
As part of #395, this PR updates the SRW and SRWQ to allow for
sequential reads for quad words and populates a provided register with
whether the word was set/unset in state.
ghost pushed a commit that referenced this issue Oct 20, 2022
Part of Issue #395.

Add clear sequential word command.

Co-authored-by: Brandon Kite <brandonkite92@gmail.com>
Co-authored-by: John Adler <adlerjohn@users.noreply.github.com>
ghost pushed a commit that referenced this issue Oct 31, 2022
Part of issue #395.

Will allow for writing of sequential storage slots and will populate rB
with whether the first slot was previously set.

Co-authored-by: Brandon Kite <brandonkite92@gmail.com>
Co-authored-by: John Adler <adlerjohn@users.noreply.github.com>
@Voxelot
Copy link
Member

Voxelot commented Dec 6, 2022

@adlerjohn should we close this issue now? The spec changes are in and the impl is done (pending sequential storage slot access).

@adlerjohn
Copy link
Contributor Author

Yep!

Repository owner moved this from In Progress to Done in Fuel Network Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:FVM Component: FuelVM enhancement New feature or request
Projects
Status: Done
Development

No branches or pull requests

4 participants