Skip to content

fix(trie): differentiate []byte(nil) and []byte{} for storage values#2964

Merged
timwu20 merged 2 commits intoqdm12/trie/nodevaluenamefrom
qdm12/trie/nil2empty
Nov 21, 2022
Merged

fix(trie): differentiate []byte(nil) and []byte{} for storage values#2964
timwu20 merged 2 commits intoqdm12/trie/nodevaluenamefrom
qdm12/trie/nil2empty

Conversation

@qdm12
Copy link
Copy Markdown
Contributor

@qdm12 qdm12 commented Nov 20, 2022

Changes

Differentiate empty storage values []byte{} with no storage value []byte(nil)

  • Leaves can have an empty storage value ([]byte{}), but cannot have no storage value
  • Branch with value can have an empty storage value ([]byte{}), but cannot have no storage value
  • Branch without value cannot have a value at all ([]byte(nil))

Side fix: do not further read from scale decoder buffer when the slice length is 0, to avoid returning an error (io.EOF)

Tests

go test ./lib/trie/... ./internal/trie/... ./dot/state

Issues

Primary Reviewer

@kishansagathiya

@qdm12 qdm12 changed the base branch from development to qdm12/trie/nodevaluename November 20, 2022 19:27
@qdm12 qdm12 marked this pull request as ready for review November 20, 2022 19:48
@codecov
Copy link
Copy Markdown

codecov bot commented Nov 20, 2022

Codecov Report

Merging #2964 (a3d2fe3) into qdm12/trie/nodevaluename (9598892) will increase coverage by 0.11%.
The diff coverage is 76.92%.

Additional details and impacted files
@@                     Coverage Diff                      @@
##           qdm12/trie/nodevaluename    #2964      +/-   ##
============================================================
+ Coverage                     63.26%   63.37%   +0.11%     
============================================================
  Files                           218      218              
  Lines                         27505    27503       -2     
============================================================
+ Hits                          17400    17430      +30     
+ Misses                         8503     8469      -34     
- Partials                       1602     1604       +2     

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.

4 participants