Skip to content

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

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

Merged
merged 2 commits into from
Nov 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions dot/state/storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,8 @@ func TestStorage_LoadFromDB(t *testing.T) {
trieKV := []struct {
key []byte
value []byte
}{{},
}{
{value: []byte{}},
{[]byte("key1"), []byte("value1")},
{[]byte("key2"), []byte("value2")},
{[]byte("xyzKey1"), []byte("xyzValue1")},
Expand Down Expand Up @@ -147,7 +148,7 @@ func TestStorage_LoadFromDB(t *testing.T) {

entries, err := storage.Entries(&root)
require.NoError(t, err)
require.Equal(t, 3, len(entries))
require.Equal(t, 4, len(entries))
}

func TestStorage_StoreTrie_NotSyncing(t *testing.T) {
Expand Down
9 changes: 2 additions & 7 deletions internal/trie/node/decode.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,15 +132,10 @@ func decodeLeaf(reader io.Reader, partialKeyLength uint16) (node *Node, err erro
}

sd := scale.NewDecoder(reader)
var storageValue []byte
err = sd.Decode(&storageValue)
if err != nil && !errors.Is(err, io.EOF) {
err = sd.Decode(&node.StorageValue)
if err != nil {
return nil, fmt.Errorf("%w: %s", ErrDecodeStorageValue, err)
}

if len(storageValue) > 0 {
node.StorageValue = storageValue
}

return node, nil
}
12 changes: 6 additions & 6 deletions internal/trie/node/decode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -334,19 +334,19 @@ func Test_decodeLeaf(t *testing.T) {
}),
variant: leafVariant.bits,
partialKeyLength: 1,
leaf: &Node{
PartialKey: []byte{9},
},
errWrapped: ErrDecodeStorageValue,
errMessage: "cannot decode storage value: reading byte: EOF",
},
"empty storage value data": {
reader: bytes.NewBuffer(concatByteSlices([][]byte{
{9}, // key data
scaleEncodeByteSlice(t, nil),
{9}, // key data
scaleEncodeByteSlice(t, []byte{}), // results to []byte{0}
})),
variant: leafVariant.bits,
partialKeyLength: 1,
leaf: &Node{
PartialKey: []byte{9},
PartialKey: []byte{9},
StorageValue: []byte{},
},
},
"success": {
Expand Down
7 changes: 4 additions & 3 deletions internal/trie/node/encode.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,10 @@ func (n *Node) Encode(buffer Buffer) (err error) {
}
}

// Only encode node storage value if the node is a leaf or
// the node is a branch with a non empty storage value.
if !nodeIsBranch || (nodeIsBranch && n.StorageValue != nil) {
// Only encode node storage value if the node has a storage value,
// even if it is empty. Do not encode if the branch is without value.
// Note leaves and branches with value cannot have a `nil` storage value.
if n.StorageValue != nil {
encoder := scale.NewEncoder(buffer)
err = encoder.Encode(n.StorageValue)
if err != nil {
Expand Down
5 changes: 3 additions & 2 deletions internal/trie/node/encode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,14 @@ func Test_Node_Encode(t *testing.T) {
},
"leaf with empty storage value success": {
node: &Node{
PartialKey: []byte{1, 2, 3},
PartialKey: []byte{1, 2, 3},
StorageValue: []byte{},
},
writes: []writeCall{
{written: []byte{leafVariant.bits | 3}}, // partial key length 3
{written: []byte{0x01, 0x23}}, // partial key
{written: []byte{0}}, // node storage value encoded length
{written: nil}, // node storage value
{written: []byte{}}, // node storage value
},
expectedEncoding: []byte{1, 2, 3},
},
Expand Down
8 changes: 4 additions & 4 deletions lib/trie/trie.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,10 +333,10 @@ func (t *Trie) Put(keyLE, value []byte) {

func (t *Trie) insertKeyLE(keyLE, value []byte, deletedMerkleValues map[string]struct{}) {
nibblesKey := codec.KeyLEToNibbles(keyLE)
if len(value) == 0 {
// Force value to be inserted to nil since we don't
// differentiate between nil and empty values.
value = nil
if value == nil {
// Force nil value to be inserted to []byte{} since `nil` means there
// is no value.
value = []byte{}
}
t.root, _, _ = t.insert(t.root, nibblesKey, value, deletedMerkleValues)
}
Expand Down
6 changes: 1 addition & 5 deletions lib/trie/trie_endtoend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,7 @@ func Fuzz_Trie_PutAndGet_Single(f *testing.F) {
trie := NewEmptyTrie()
trie.Put(key, value)
retrievedValue := trie.Get(key)
if retrievedValue == nil {
assert.Empty(t, value)
} else {
assert.Equal(t, value, retrievedValue)
}
assert.Equal(t, value, retrievedValue)
})
}

Expand Down
9 changes: 6 additions & 3 deletions pkg/scale/decode.go
Original file line number Diff line number Diff line change
Expand Up @@ -592,9 +592,12 @@ func (ds *decodeState) decodeBytes(dstv reflect.Value) (err error) {
}

b := make([]byte, length)
_, err = ds.Read(b)
if err != nil {
return

if length > 0 {
_, err = ds.Read(b)
if err != nil {
return
}
}

in := dstv.Interface()
Expand Down