Skip to content

Clear storage cell / trie without reading cell's value first fails#9239

Merged
damian-orzechowski merged 4 commits intomasterfrom
fix/clear_storage_no_read
Sep 9, 2025
Merged

Clear storage cell / trie without reading cell's value first fails#9239
damian-orzechowski merged 4 commits intomasterfrom
fix/clear_storage_no_read

Conversation

@damian-orzechowski
Copy link
Contributor

@damian-orzechowski damian-orzechowski commented Sep 3, 2025

Fixes Closes Resolves #

Fixes an issue when you set empty value (clear) for a storage cell that was not previously read (using Get). In such case, new value (empty) would not be set as code assumes non existent values as empty, which in turn fails on value comparison (new vs old) when commiting changes.

Changes

  • PerContractState will load value from tree if not found in block cache when saving a new change

Types of changes

What types of changes does your code introduce?

  • Bugfix (a non-breaking change that fixes an issue)
  • New feature (a non-breaking change that adds functionality)
  • Breaking change (a change that causes existing functionality not to work as expected)
  • Optimization
  • Refactoring
  • Documentation update
  • Build-related changes
  • Other: Description

Testing

Requires testing

  • Yes
  • No

If yes, did you write tests?

  • Yes
  • No

Notes on testing

Optional. Remove if not applicable.

Documentation

Requires documentation update

  • Yes
  • No

If yes, link the PR to the docs update or the issue with the details labeled docs. Remove if not applicable.

Requires explanation in Release Notes

  • Yes
  • No

@damian-orzechowski damian-orzechowski marked this pull request as ready for review September 3, 2025 16:20
worldState.Set(new StorageCell(TestItem.AddressA, 1), _values[11]);
worldState.Set(new StorageCell(TestItem.AddressA, 2), _values[12]);
worldState.Commit(Prague.Instance);
worldState.CommitTree(1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like it will also fail previously if Reset() is called after the commit. Previously the blockchange and the storage tree is separated, now its the same causing the CommitTree to have an implicit reset.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's correct. Recreated the same behaviour with calling Reset() - though with calling Reset(false) works fine.

@damian-orzechowski damian-orzechowski merged commit eee5ac2 into master Sep 9, 2025
137 of 139 checks passed
@damian-orzechowski damian-orzechowski deleted the fix/clear_storage_no_read branch September 9, 2025 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments