Skip to content

Commit

Permalink
Adjust minor comments and error wrappings
Browse files Browse the repository at this point in the history
  • Loading branch information
qdm12 committed Nov 18, 2022
1 parent 39c7043 commit 9598892
Show file tree
Hide file tree
Showing 10 changed files with 62 additions and 62 deletions.
4 changes: 2 additions & 2 deletions internal/trie/node/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ The header is then concatenated with the partial key of the node, encoded as Lit

The remaining bytes appended depend on the node variant.

- For leaves, the SCALE-encoded leaf value is appended.
- For leaves, the SCALE-encoded leaf storage value is appended.
- For branches, the following elements are concatenated in this order and appended to the previous header+partial key:
- Children bitmap (2 bytes)
- SCALE-encoded node value
- SCALE-encoded node storage value
- Hash(Encoding(Child[0]))
- Hash(Encoding(Child[1]))
- ...
Expand Down
12 changes: 6 additions & 6 deletions internal/trie/node/copy.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ var (
// - the HashDigest field is left empty on the copy
// - the Encoding field is left empty on the copy
// - the key field is deep copied
// - the value field is deep copied
// - the storage value field is deep copied
DefaultCopySettings = CopySettings{
CopyKey: true,
CopyStorageValue: true,
Expand All @@ -20,7 +20,7 @@ var (
// - the HashDigest field is deep copied
// - the Encoding field is deep copied
// - the key field is deep copied
// - the value field is deep copied
// - the storage value field is deep copied
DeepCopySettings = CopySettings{
CopyChildren: true,
CopyCached: true,
Expand All @@ -46,8 +46,8 @@ type CopySettings struct {
// the node. This is useful when false if the key is about to
// be assigned after the Copy operation, to save a memory operation.
CopyKey bool
// CopyStorageValue can be set to true to deep copy the value field of
// the node. This is useful when false if the value is about to
// CopyStorageValue can be set to true to deep copy the storage value field of
// the node. This is useful when false if the storage value is about to
// be assigned after the Copy operation, to save a memory operation.
CopyStorageValue bool
}
Expand Down Expand Up @@ -87,8 +87,8 @@ func (n *Node) Copy(settings CopySettings) *Node {
copy(cpy.PartialKey, n.PartialKey)
}

// nil and []byte{} values for branches result in a different node encoding,
// so we ensure to keep the `nil` value.
// nil and []byte{} storage values for branches result in a different node encoding,
// so we ensure to keep the `nil` storage value.
if settings.CopyStorageValue && n.StorageValue != nil {
cpy.StorageValue = make([]byte, len(n.StorageValue))
copy(cpy.StorageValue, n.StorageValue)
Expand Down
12 changes: 6 additions & 6 deletions internal/trie/node/decode.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ var (
// in the scale package.
// TODO remove once the following issue is done:
// https://github.com/ChainSafe/gossamer/issues/2631 .
ErrDecodeStorageValue = errors.New("cannot decode value")
ErrDecodeStorageValue = errors.New("cannot decode storage value")
ErrReadChildrenBitmap = errors.New("cannot read children bitmap")
// ErrDecodeChildHash is defined since no sentinel error is defined
// in the scale package.
Expand Down Expand Up @@ -62,7 +62,7 @@ func Decode(reader io.Reader) (n *Node, err error) {
// Note that since the encoded branch stores the hash of the children nodes, we are not
// reconstructing the child nodes from the encoding. This function instead stubs where the
// children are known to be with an empty leaf. The children nodes hashes are then used to
// find other values using the persistent database.
// find other storage values using the persistent database.
func decodeBranch(reader io.Reader, variant byte, partialKeyLength uint16) (
node *Node, err error) {
node = &Node{
Expand Down Expand Up @@ -132,14 +132,14 @@ func decodeLeaf(reader io.Reader, partialKeyLength uint16) (node *Node, err erro
}

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

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

return node, nil
Expand Down
26 changes: 13 additions & 13 deletions internal/trie/node/decode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,19 +193,19 @@ func Test_decodeBranch(t *testing.T) {
concatByteSlices([][]byte{
{9}, // key data
{0, 4}, // children bitmap
// missing encoded branch value
// missing encoded branch storage value
}),
),
variant: branchWithValueVariant.bits,
partialKeyLength: 1,
errWrapped: ErrDecodeStorageValue,
errMessage: "cannot decode value: reading byte: EOF",
errMessage: "cannot decode storage value: reading byte: EOF",
},
"success for branch with value": {
reader: bytes.NewBuffer(concatByteSlices([][]byte{
{9}, // key data
{0, 4}, // children bitmap
scaleEncodeBytes(t, 7, 8, 9), // branch value
scaleEncodeBytes(t, 7, 8, 9), // branch storage value
scaleEncodedChildHash,
})),
variant: branchWithValueVariant.bits,
Expand All @@ -227,7 +227,7 @@ func Test_decodeBranch(t *testing.T) {
reader: bytes.NewBuffer(concatByteSlices([][]byte{
{1}, // key data
{0b0000_0001, 0b0000_0000}, // children bitmap
scaleEncodeBytes(t, 1), // branch value
scaleEncodeBytes(t, 1), // branch storage value
{0}, // garbage inlined node
})),
variant: branchWithValueVariant.bits,
Expand All @@ -244,19 +244,19 @@ func Test_decodeBranch(t *testing.T) {
scaleEncodeByteSlice(t, concatByteSlices([][]byte{
{leafVariant.bits | 1}, // partial key length of 1
{2}, // key data
scaleEncodeBytes(t, 2), // value data
scaleEncodeBytes(t, 2), // storage value data
})),
// top level inlined branch less than 32 bytes
scaleEncodeByteSlice(t, concatByteSlices([][]byte{
{branchWithValueVariant.bits | 1}, // partial key length of 1
{3}, // key data
{0b0000_0001, 0b0000_0000}, // children bitmap
scaleEncodeBytes(t, 3), // branch value
scaleEncodeBytes(t, 3), // branch storage value
// bottom level leaf
scaleEncodeByteSlice(t, concatByteSlices([][]byte{
{leafVariant.bits | 1}, // partial key length of 1
{4}, // key data
scaleEncodeBytes(t, 4), // value data
scaleEncodeBytes(t, 4), // storage value data
})),
})),
})),
Expand Down Expand Up @@ -320,25 +320,25 @@ func Test_decodeLeaf(t *testing.T) {
"value decoding error": {
reader: bytes.NewBuffer([]byte{
9, // key data
255, 255, // bad value data
255, 255, // bad storage value data
}),
variant: leafVariant.bits,
partialKeyLength: 1,
errWrapped: ErrDecodeStorageValue,
errMessage: "cannot decode value: unknown prefix for compact uint: 255",
errMessage: "cannot decode storage value: unknown prefix for compact uint: 255",
},
"missing value data": {
"missing storage value data": {
reader: bytes.NewBuffer([]byte{
9, // key data
// missing value data
// missing storage value data
}),
variant: leafVariant.bits,
partialKeyLength: 1,
leaf: &Node{
PartialKey: []byte{9},
},
},
"empty value data": {
"empty storage value data": {
reader: bytes.NewBuffer(concatByteSlices([][]byte{
{9}, // key data
scaleEncodeByteSlice(t, nil),
Expand All @@ -353,7 +353,7 @@ func Test_decodeLeaf(t *testing.T) {
reader: bytes.NewBuffer(
concatByteSlices([][]byte{
{9}, // key data
scaleEncodeBytes(t, 1, 2, 3, 4, 5), // value data
scaleEncodeBytes(t, 1, 2, 3, 4, 5), // storage value data
}),
),
variant: leafVariant.bits,
Expand Down
2 changes: 1 addition & 1 deletion internal/trie/node/dirty.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ package node
// SetDirty sets the dirty status to true for the node.
func (n *Node) SetDirty() {
n.Dirty = true
// A node is marked dirty if its key or value is modified.
// A node is marked dirty if its partial key or storage value is modified.
// This means its Merkle value field is no longer valid.
n.MerkleValue = nil
}
Expand Down
6 changes: 3 additions & 3 deletions internal/trie/node/encode.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,13 @@ func (n *Node) Encode(buffer Buffer) (err error) {
}
}

// Only encode node value if the node is a leaf or
// the node is a branch with a non empty value.
// 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) {
encoder := scale.NewEncoder(buffer)
err = encoder.Encode(n.StorageValue)
if err != nil {
return fmt.Errorf("scale encoding value: %w", err)
return fmt.Errorf("scale encoding storage value: %w", err)
}
}

Expand Down
20 changes: 10 additions & 10 deletions internal/trie/node/encode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func Test_Node_Encode(t *testing.T) {
wrappedErr: errTest,
errMessage: "cannot write LE key to buffer: test error",
},
"leaf buffer write error for encoded value": {
"leaf buffer write error for encoded storage value": {
node: &Node{
PartialKey: []byte{1, 2, 3},
StorageValue: []byte{4, 5, 6},
Expand All @@ -78,7 +78,7 @@ func Test_Node_Encode(t *testing.T) {
},
},
wrappedErr: errTest,
errMessage: "scale encoding value: test error",
errMessage: "scale encoding storage value: test error",
},
"leaf success": {
node: &Node{
Expand All @@ -95,15 +95,15 @@ func Test_Node_Encode(t *testing.T) {
},
expectedEncoding: []byte{1, 2, 3},
},
"leaf with empty value success": {
"leaf with empty storage value success": {
node: &Node{
PartialKey: []byte{1, 2, 3},
},
writes: []writeCall{
{written: []byte{leafVariant.bits | 3}}, // partial key length 3
{written: []byte{0x01, 0x23}}, // partial key
{written: []byte{0}}, // node value encoded length
{written: nil}, // node value
{written: []byte{0}}, // node storage value encoded length
{written: nil}, // node storage value
},
expectedEncoding: []byte{1, 2, 3},
},
Expand Down Expand Up @@ -163,7 +163,7 @@ func Test_Node_Encode(t *testing.T) {
wrappedErr: errTest,
errMessage: "cannot write children bitmap to buffer: test error",
},
"buffer write error for value": {
"buffer write error for storage value": {
node: &Node{
PartialKey: []byte{1, 2, 3},
StorageValue: []byte{100},
Expand All @@ -182,13 +182,13 @@ func Test_Node_Encode(t *testing.T) {
{ // children bitmap
written: []byte{136, 0},
},
{ // value
{ // storage value
written: []byte{4},
err: errTest,
},
},
wrappedErr: errTest,
errMessage: "scale encoding value: test error",
errMessage: "scale encoding storage value: test error",
},
"buffer write error for children encoding": {
node: &Node{
Expand All @@ -209,7 +209,7 @@ func Test_Node_Encode(t *testing.T) {
{ // children bitmap
written: []byte{136, 0},
},
// value
// storage value
{written: []byte{4}},
{written: []byte{100}},
{ // children
Expand Down Expand Up @@ -241,7 +241,7 @@ func Test_Node_Encode(t *testing.T) {
{ // children bitmap
written: []byte{136, 0},
},
// value
// storage value
{written: []byte{4}},
{written: []byte{100}},
{ // first children
Expand Down
2 changes: 1 addition & 1 deletion internal/trie/node/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (n Node) StringNode() (stringNode *gotree.Node) {
stringNode.Appendf("Generation: %d", n.Generation)
stringNode.Appendf("Dirty: %t", n.Dirty)
stringNode.Appendf("Key: " + bytesToString(n.PartialKey))
stringNode.Appendf("Value: " + bytesToString(n.StorageValue))
stringNode.Appendf("Storage value: " + bytesToString(n.StorageValue))
if n.Descendants > 0 { // must be a branch
stringNode.Appendf("Descendants: %d", n.Descendants)
}
Expand Down
Loading

0 comments on commit 9598892

Please sign in to comment.