From f78d45ba8dd5a14342a286813d9a58585619cc83 Mon Sep 17 00:00:00 2001 From: Quentin McGaw Date: Mon, 29 Nov 2021 15:46:26 +0000 Subject: [PATCH] Address TODOs --- lib/trie/branch/children.go | 11 +++---- lib/trie/branch/decode.go | 9 +----- lib/trie/branch/decode_test.go | 36 +++++++---------------- lib/trie/branch/encode.go | 4 --- lib/trie/encodedecode_test/branch_test.go | 1 - lib/trie/leaf/decode.go | 11 ++----- lib/trie/leaf/decode_test.go | 28 +++++------------- lib/trie/leaf/encode.go | 13 -------- 8 files changed, 26 insertions(+), 87 deletions(-) diff --git a/lib/trie/branch/children.go b/lib/trie/branch/children.go index 54395458da4..ff911dc5138 100644 --- a/lib/trie/branch/children.go +++ b/lib/trie/branch/children.go @@ -5,10 +5,8 @@ package branch // ChildrenBitmap returns the 16 bit bitmap // of the children in the branch. -func (b *Branch) ChildrenBitmap() uint16 { - var bitmap uint16 - var i uint - for i = 0; i < 16; i++ { +func (b *Branch) ChildrenBitmap() (bitmap uint16) { + for i := uint(0); i < 16; i++ { if b.Children[i] != nil { bitmap = bitmap | 1<> 6 if nodeType != 2 && nodeType != 3 { return nil, fmt.Errorf("%w: %d", ErrNodeTypeIsNotABranch, nodeType) @@ -81,7 +74,7 @@ func Decode(reader io.Reader, header byte) (branch *Branch, err error) { } } - branch.Dirty = true // TODO move as soon as it gets modified? + branch.Dirty = true return branch, nil } diff --git a/lib/trie/branch/decode_test.go b/lib/trie/branch/decode_test.go index 9d89ea2d7ae..db2ce1f43f5 100644 --- a/lib/trie/branch/decode_test.go +++ b/lib/trie/branch/decode_test.go @@ -44,46 +44,36 @@ func Test_Decode(t *testing.T) { errWrapped error errMessage string }{ - "no data with header 0": { - reader: bytes.NewBuffer(nil), - errWrapped: ErrReadHeaderByte, - errMessage: "cannot read header byte: EOF", - }, "no data with header 1": { reader: bytes.NewBuffer(nil), - header: 1, - errWrapped: ErrNodeTypeIsNotABranch, - errMessage: "node type is not a branch: 0", - }, - "first byte as 0 header 0": { - reader: bytes.NewBuffer([]byte{0}), + header: 65, errWrapped: ErrNodeTypeIsNotABranch, - errMessage: "node type is not a branch: 0", + errMessage: "node type is not a branch: 1", }, "key decoding error": { reader: bytes.NewBuffer([]byte{ - 129, // node type 2 and key length 1 // missing key data byte }), + header: 129, // node type 2 and key length 1 errWrapped: decode.ErrReadKeyData, errMessage: "cannot decode key: cannot read key data: EOF", }, "children bitmap read error": { reader: bytes.NewBuffer([]byte{ - 129, // node type 2 and key length 1 - 9, // key data + 9, // key data // missing children bitmap 2 bytes }), + header: 129, // node type 2 and key length 1 errWrapped: ErrReadChildrenBitmap, errMessage: "cannot read children bitmap: EOF", }, "children decoding error": { reader: bytes.NewBuffer([]byte{ - 129, // node type 2 and key length 1 9, // key data 0, 4, // children bitmap // missing children scale encoded data }), + header: 129, // node type 2 and key length 1 errWrapped: ErrDecodeChildHash, errMessage: "cannot decode child hash: at index 10: EOF", }, @@ -91,13 +81,13 @@ func Test_Decode(t *testing.T) { reader: bytes.NewBuffer( concatByteSlices([][]byte{ { - 129, // node type 2 and key length 1 9, // key data 0, 4, // children bitmap }, scaleEncodeBytes(t, 1, 2, 3, 4, 5), // child hash }), ), + header: 129, // node type 2 and key length 1 branch: &Branch{ Key: []byte{9}, Children: [16]node.Node{ @@ -113,29 +103,25 @@ func Test_Decode(t *testing.T) { "value decoding error for node type 3": { reader: bytes.NewBuffer( concatByteSlices([][]byte{ - { - 193, // node type 3 and key length 1 - 9, // key data - }, + {9}, // key data {0, 4}, // children bitmap // missing encoded branch value }), ), + header: 193, // node type 3 and key length 1 errWrapped: ErrDecodeValue, errMessage: "cannot decode value: EOF", }, "success node type 3": { reader: bytes.NewBuffer( concatByteSlices([][]byte{ - { - 193, // node type 3 and key length 1 - 9, // key data - }, + {9}, // key data {0, 4}, // children bitmap scaleEncodeBytes(t, 7, 8, 9), // branch value scaleEncodeBytes(t, 1, 2, 3, 4, 5), // child hash }), ), + header: 193, // node type 3 and key length 1 branch: &Branch{ Key: []byte{9}, Value: []byte{7, 8, 9}, diff --git a/lib/trie/branch/encode.go b/lib/trie/branch/encode.go index 3a1bfa4e74f..04d7bc9cf96 100644 --- a/lib/trie/branch/encode.go +++ b/lib/trie/branch/encode.go @@ -21,10 +21,6 @@ import ( // and then SCALE encodes it. This is used to encode children // nodes of branches. func (b *Branch) ScaleEncodeHash() (encoding []byte, err error) { - // if b == nil { // TODO remove - // panic("Should write 0 to buffer") - // } - buffer := pools.DigestBuffers.Get().(*bytes.Buffer) buffer.Reset() defer pools.DigestBuffers.Put(buffer) diff --git a/lib/trie/encodedecode_test/branch_test.go b/lib/trie/encodedecode_test/branch_test.go index 41b5d68dccd..4cecf6893c3 100644 --- a/lib/trie/encodedecode_test/branch_test.go +++ b/lib/trie/encodedecode_test/branch_test.go @@ -60,7 +60,6 @@ func Test_Branch_Encode_Decode(t *testing.T) { Key: []byte{5}, Children: [16]node.Node{ &leaf.Leaf{ - // TODO key and value are nil here?? Why? Hash: []byte{0x41, 0x9, 0x4, 0xa}, }, }, diff --git a/lib/trie/leaf/decode.go b/lib/trie/leaf/decode.go index a01afb4a1cc..6c20fffbccf 100644 --- a/lib/trie/leaf/decode.go +++ b/lib/trie/leaf/decode.go @@ -19,14 +19,7 @@ var ( ) // Decode reads and decodes from a reader with the encoding specified in lib/trie/encode/doc.go. -func Decode(r io.Reader, header byte) (leaf *Leaf, err error) { // TODO return leaf - if header == 0 { // TODO remove this is taken care of by the caller - header, err = decode.ReadNextByte(r) - if err != nil { - return nil, fmt.Errorf("%w: %s", ErrReadHeaderByte, err) - } - } - +func Decode(r io.Reader, header byte) (leaf *Leaf, err error) { nodeType := header >> 6 if nodeType != 1 { return nil, fmt.Errorf("%w: %d", ErrNodeTypeIsNotALeaf, nodeType) @@ -51,7 +44,7 @@ func Decode(r io.Reader, header byte) (leaf *Leaf, err error) { // TODO return l leaf.Value = value } - leaf.Dirty = true // TODO move this as soon as it gets modified + leaf.Dirty = true return leaf, nil } diff --git a/lib/trie/leaf/decode_test.go b/lib/trie/leaf/decode_test.go index 5e98eb71d5e..e702b666e24 100644 --- a/lib/trie/leaf/decode_test.go +++ b/lib/trie/leaf/decode_test.go @@ -42,45 +42,35 @@ func Test_Decode(t *testing.T) { errWrapped error errMessage string }{ - "no data with header 0": { - reader: bytes.NewBuffer(nil), - errWrapped: ErrReadHeaderByte, - errMessage: "cannot read header byte: EOF", - }, "no data with header 1": { reader: bytes.NewBuffer(nil), header: 1, errWrapped: ErrNodeTypeIsNotALeaf, errMessage: "node type is not a leaf: 0", }, - "first byte as 0 header 0": { - reader: bytes.NewBuffer([]byte{0}), - errWrapped: ErrNodeTypeIsNotALeaf, - errMessage: "node type is not a leaf: 0", - }, "key decoding error": { reader: bytes.NewBuffer([]byte{ - 65, // node type 1 and key length 1 // missing key data byte }), + header: 65, // node type 1 and key length 1 errWrapped: decode.ErrReadKeyData, errMessage: "cannot decode key: cannot read key data: EOF", }, "value decoding error": { reader: bytes.NewBuffer([]byte{ - 65, // node type 1 and key length 1 - 9, // key data + 9, // key data // missing value data }), + header: 65, // node type 1 and key length 1 errWrapped: ErrDecodeValue, errMessage: "cannot decode value: EOF", }, "zero value": { reader: bytes.NewBuffer([]byte{ - 65, // node type 1 and key length 1 - 9, // key data - 0, // missing value data + 9, // key data + 0, // missing value data }), + header: 65, // node type 1 and key length 1 leaf: &Leaf{ Key: []byte{9}, Dirty: true, @@ -89,13 +79,11 @@ func Test_Decode(t *testing.T) { "success": { reader: bytes.NewBuffer( concatByteSlices([][]byte{ - { - 65, // node type 1 and key length 1 - 9, // key data - }, + {9}, // key data scaleEncodeBytes(t, 1, 2, 3, 4, 5), // value data }), ), + header: 65, // node type 1 and key length 1 leaf: &Leaf{ Key: []byte{9}, Value: []byte{1, 2, 3, 4, 5}, diff --git a/lib/trie/leaf/encode.go b/lib/trie/leaf/encode.go index 8467d204f7f..ca1dd62cc49 100644 --- a/lib/trie/leaf/encode.go +++ b/lib/trie/leaf/encode.go @@ -86,15 +86,6 @@ func (l *Leaf) EncodeAndHash() (encoding, hash []byte, err error) { // The encoding has the following format: // NodeHeader | Extra partial key length | Partial Key | Value func (l *Leaf) Encode(buffer encode.Buffer) (err error) { - // if l == nil { - // // TODO remove if not needed - // _, err := buffer.Write([]byte{0}) - // if err != nil { - // return fmt.Errorf("cannot write nil encoding to buffer: %w", err) - // } - // return nil - // } - l.encodingMu.RLock() if !l.Dirty && l.Encoding != nil { _, err = buffer.Write(l.Encoding) @@ -145,10 +136,6 @@ func (l *Leaf) Encode(buffer encode.Buffer) (err error) { // and then SCALE encodes it. This is used to encode children // nodes of branches. func (l *Leaf) ScaleEncodeHash() (b []byte, err error) { - // if l == nil { // TODO remove - // panic("Should write 0 to buffer") - // } - buffer := pools.DigestBuffers.Get().(*bytes.Buffer) buffer.Reset() defer pools.DigestBuffers.Put(buffer)