From 5d5829f6ef437cb784fec0dc14c480c4fc32a572 Mon Sep 17 00:00:00 2001 From: Erik Grinaker Date: Tue, 30 Jun 2020 11:51:39 +0200 Subject: [PATCH] remove Amino (#265) Removes Amino and adds varint utility-functions instead, keeping the same serialized format. Partial fix for #242. The last vestige of Amino is in the proof handling, namely `AbsenceOp` and `ValueOp`, which are structs encoded/decoded with Amino. --- docs/node/node.md | 28 ++++++------- encoding.go | 104 ++++++++++++++++++++++++++++++++++++++++++++++ export_test.go | 1 + node.go | 69 ++++++++++++++++-------------- node_test.go | 63 +++++++++++++++++++++++++--- nodedb.go | 2 +- proof.go | 25 ++++++----- testutils_test.go | 8 ++-- tree_test.go | 69 ++++++++++++++++++++++++++++++ 9 files changed, 300 insertions(+), 69 deletions(-) create mode 100644 encoding.go diff --git a/docs/node/node.md b/docs/node/node.md index 64d19614c..24647fe1b 100644 --- a/docs/node/node.md +++ b/docs/node/node.md @@ -34,27 +34,27 @@ Every node is persisted by encoding the key, version, height, size and hash. If ```golang // Writes the node as a serialized byte slice to the supplied io.Writer. func (node *Node) writeBytes(w io.Writer) error { - cause := amino.EncodeInt8(w, node.height) + cause := encodeVarint(w, node.height) if cause != nil { return errors.Wrap(cause, "writing height") } - cause = amino.EncodeVarint(w, node.size) + cause = encodeVarint(w, node.size) if cause != nil { return errors.Wrap(cause, "writing size") } - cause = amino.EncodeVarint(w, node.version) + cause = encodeVarint(w, node.version) if cause != nil { return errors.Wrap(cause, "writing version") } // Unlike writeHashBytes, key is written for inner nodes. - cause = amino.EncodeByteSlice(w, node.key) + cause = encodeBytes(w, node.key) if cause != nil { return errors.Wrap(cause, "writing key") } if node.isLeaf() { - cause = amino.EncodeByteSlice(w, node.value) + cause = encodeBytes(w, node.value) if cause != nil { return errors.Wrap(cause, "writing value") } @@ -62,7 +62,7 @@ func (node *Node) writeBytes(w io.Writer) error { if node.leftHash == nil { panic("node.leftHash was nil in writeBytes") } - cause = amino.EncodeByteSlice(w, node.leftHash) + cause = encodeBytes(w, node.leftHash) if cause != nil { return errors.Wrap(cause, "writing left hash") } @@ -70,7 +70,7 @@ func (node *Node) writeBytes(w io.Writer) error { if node.rightHash == nil { panic("node.rightHash was nil in writeBytes") } - cause = amino.EncodeByteSlice(w, node.rightHash) + cause = encodeBytes(w, node.rightHash) if cause != nil { return errors.Wrap(cause, "writing right hash") } @@ -87,15 +87,15 @@ A node's hash is calculated by hashing the height, size, and version of the node // Writes the node's hash to the given io.Writer. This function expects // child hashes to be already set. func (node *Node) writeHashBytes(w io.Writer) error { - err := amino.EncodeInt8(w, node.height) + err := encodeVarint(w, node.height) if err != nil { return errors.Wrap(err, "writing height") } - err = amino.EncodeVarint(w, node.size) + err = encodeVarint(w, node.size) if err != nil { return errors.Wrap(err, "writing size") } - err = amino.EncodeVarint(w, node.version) + err = encodeVarint(w, node.version) if err != nil { return errors.Wrap(err, "writing version") } @@ -103,14 +103,14 @@ func (node *Node) writeHashBytes(w io.Writer) error { // Key is not written for inner nodes, unlike writeBytes. if node.isLeaf() { - err = amino.EncodeByteSlice(w, node.key) + err = encodeBytes(w, node.key) if err != nil { return errors.Wrap(err, "writing key") } // Indirection needed to provide proofs without values. // (e.g. proofLeafNode.ValueHash) valueHash := tmhash.Sum(node.value) - err = amino.EncodeByteSlice(w, valueHash) + err = encodeBytes(w, valueHash) if err != nil { return errors.Wrap(err, "writing value") } @@ -118,11 +118,11 @@ func (node *Node) writeHashBytes(w io.Writer) error { if node.leftHash == nil || node.rightHash == nil { panic("Found an empty child hash") } - err = amino.EncodeByteSlice(w, node.leftHash) + err = encodeBytes(w, node.leftHash) if err != nil { return errors.Wrap(err, "writing left hash") } - err = amino.EncodeByteSlice(w, node.rightHash) + err = encodeBytes(w, node.rightHash) if err != nil { return errors.Wrap(err, "writing right hash") } diff --git a/encoding.go b/encoding.go new file mode 100644 index 000000000..1f2f153aa --- /dev/null +++ b/encoding.go @@ -0,0 +1,104 @@ +package iavl + +import ( + "encoding/binary" + "errors" + "fmt" + "io" + "math/bits" +) + +// decodeBytes decodes a varint length-prefixed byte slice, returning it along with the number +// of input bytes read. +func decodeBytes(bz []byte) ([]byte, int, error) { + size, n, err := decodeUvarint(bz) + if err != nil { + return nil, n, err + } + if int(size) < 0 { + return nil, n, fmt.Errorf("invalid negative length %v decoding []byte", size) + } + if len(bz) < n+int(size) { + return nil, n, fmt.Errorf("insufficient bytes decoding []byte of length %v", size) + } + bz2 := make([]byte, size) + copy(bz2, bz[n:n+int(size)]) + n += int(size) + return bz2, n, nil +} + +// decodeUvarint decodes a varint-encoded unsigned integer from a byte slice, returning it and the +// number of bytes decoded. +func decodeUvarint(bz []byte) (uint64, int, error) { + u, n := binary.Uvarint(bz) + if n == 0 { + // buf too small + return u, n, errors.New("buffer too small") + } else if n < 0 { + // value larger than 64 bits (overflow) + // and -n is the number of bytes read + n = -n + return u, n, errors.New("EOF decoding uvarint") + } + return u, n, nil +} + +// decodeVarint decodes a varint-encoded integer from a byte slice, returning it and the number of +// bytes decoded. +func decodeVarint(bz []byte) (int64, int, error) { + i, n := binary.Varint(bz) + if n == 0 { + return i, n, errors.New("buffer too small") + } else if n < 0 { + // value larger than 64 bits (overflow) + // and -n is the number of bytes read + n = -n + return i, n, errors.New("EOF decoding varint") + } + return i, n, nil +} + +// encodeBytes writes a varint length-prefixed byte slice to the writer. +func encodeBytes(w io.Writer, bz []byte) error { + err := encodeUvarint(w, uint64(len(bz))) + if err != nil { + return err + } + _, err = w.Write(bz) + return err +} + +// encodeBytesSize returns the byte size of the given slice including length-prefixing. +func encodeBytesSize(bz []byte) int { + return encodeUvarintSize(uint64(len(bz))) + len(bz) +} + +// encodeUvarint writes a varint-encoded unsigned integer to an io.Writer. +func encodeUvarint(w io.Writer, u uint64) error { + var buf [binary.MaxVarintLen64]byte + n := binary.PutUvarint(buf[:], u) + _, err := w.Write(buf[0:n]) + return err +} + +// encodeUvarintSize returns the byte size of the given integer as a varint. +func encodeUvarintSize(u uint64) int { + if u == 0 { + return 1 + } + return (bits.Len64(u) + 6) / 7 +} + +// encodeVarint writes a varint-encoded integer to an io.Writer. +func encodeVarint(w io.Writer, i int64) error { + var buf [binary.MaxVarintLen64]byte + n := binary.PutVarint(buf[:], i) + _, err := w.Write(buf[0:n]) + return err +} + +// encodeVarintSize returns the byte size of the given integer as a varint. +func encodeVarintSize(i int64) int { + var buf [binary.MaxVarintLen64]byte + return binary.PutVarint(buf[:], i) +} diff --git a/export_test.go b/export_test.go index c20663019..45bdc4b81 100644 --- a/export_test.go +++ b/export_test.go @@ -45,6 +45,7 @@ func setupExportTreeBasic(t require.TestingT) *ImmutableTree { } // setupExportTreeRandom sets up a randomly generated tree. +// nolint: dupl func setupExportTreeRandom(t *testing.T) *ImmutableTree { const ( randSeed = 49872768940 // For deterministic tests diff --git a/node.go b/node.go index 6993b37aa..c399353a4 100644 --- a/node.go +++ b/node.go @@ -8,9 +8,9 @@ import ( "crypto/sha256" "fmt" "io" + "math" "github.com/pkg/errors" - amino "github.com/tendermint/go-amino" ) // Node represents a node in a Tree. @@ -46,32 +46,35 @@ func NewNode(key []byte, value []byte, version int64) *Node { func MakeNode(buf []byte) (*Node, error) { // Read node header (height, size, version, key). - height, n, cause := amino.DecodeInt8(buf) + height, n, cause := decodeVarint(buf) if cause != nil { return nil, errors.Wrap(cause, "decoding node.height") } buf = buf[n:] + if height < int64(math.MinInt8) || height > int64(math.MaxInt8) { + return nil, errors.New("invalid height, must be int8") + } - size, n, cause := amino.DecodeVarint(buf) + size, n, cause := decodeVarint(buf) if cause != nil { return nil, errors.Wrap(cause, "decoding node.size") } buf = buf[n:] - ver, n, cause := amino.DecodeVarint(buf) + ver, n, cause := decodeVarint(buf) if cause != nil { return nil, errors.Wrap(cause, "decoding node.version") } buf = buf[n:] - key, n, cause := amino.DecodeByteSlice(buf) + key, n, cause := decodeBytes(buf) if cause != nil { return nil, errors.Wrap(cause, "decoding node.key") } buf = buf[n:] node := &Node{ - height: height, + height: int8(height), size: size, version: ver, key: key, @@ -80,19 +83,19 @@ func MakeNode(buf []byte) (*Node, error) { // Read node body. if node.isLeaf() { - val, _, cause := amino.DecodeByteSlice(buf) + val, _, cause := decodeBytes(buf) if cause != nil { return nil, errors.Wrap(cause, "decoding node.value") } node.value = val } else { // Read children. - leftHash, n, cause := amino.DecodeByteSlice(buf) + leftHash, n, cause := decodeBytes(buf) if cause != nil { return nil, errors.Wrap(cause, "deocding node.leftHash") } buf = buf[n:] - rightHash, _, cause := amino.DecodeByteSlice(buf) + rightHash, _, cause := decodeBytes(buf) if cause != nil { return nil, errors.Wrap(cause, "decoding node.rightHash") } @@ -279,15 +282,15 @@ func (node *Node) validate() error { // Writes the node's hash to the given io.Writer. This function expects // child hashes to be already set. func (node *Node) writeHashBytes(w io.Writer) error { - err := amino.EncodeInt8(w, node.height) + err := encodeVarint(w, int64(node.height)) if err != nil { return errors.Wrap(err, "writing height") } - err = amino.EncodeVarint(w, node.size) + err = encodeVarint(w, node.size) if err != nil { return errors.Wrap(err, "writing size") } - err = amino.EncodeVarint(w, node.version) + err = encodeVarint(w, node.version) if err != nil { return errors.Wrap(err, "writing version") } @@ -295,17 +298,16 @@ func (node *Node) writeHashBytes(w io.Writer) error { // Key is not written for inner nodes, unlike writeBytes. if node.isLeaf() { - err = amino.EncodeByteSlice(w, node.key) + err = encodeBytes(w, node.key) if err != nil { return errors.Wrap(err, "writing key") } // Indirection needed to provide proofs without values. // (e.g. ProofLeafNode.ValueHash) - h := sha256.Sum256(node.value) - valueHash := h[:] + valueHash := sha256.Sum256(node.value) - err = amino.EncodeByteSlice(w, valueHash) + err = encodeBytes(w, valueHash[:]) if err != nil { return errors.Wrap(err, "writing value") } @@ -313,11 +315,11 @@ func (node *Node) writeHashBytes(w io.Writer) error { if node.leftHash == nil || node.rightHash == nil { panic("Found an empty child hash") } - err = amino.EncodeByteSlice(w, node.leftHash) + err = encodeBytes(w, node.leftHash) if err != nil { return errors.Wrap(err, "writing left hash") } - err = amino.EncodeByteSlice(w, node.rightHash) + err = encodeBytes(w, node.rightHash) if err != nil { return errors.Wrap(err, "writing right hash") } @@ -344,43 +346,46 @@ func (node *Node) writeHashBytesRecursively(w io.Writer) (hashCount int64, err e return } -func (node *Node) aminoSize() int { +func (node *Node) encodedSize() int { n := 1 + - amino.VarintSize(node.size) + - amino.VarintSize(node.version) + - amino.ByteSliceSize(node.key) + encodeVarintSize(node.size) + + encodeVarintSize(node.version) + + encodeBytesSize(node.key) if node.isLeaf() { - n += amino.ByteSliceSize(node.value) + n += encodeBytesSize(node.value) } else { - n += amino.ByteSliceSize(node.leftHash) + - amino.ByteSliceSize(node.rightHash) + n += encodeBytesSize(node.leftHash) + + encodeBytesSize(node.rightHash) } return n } // Writes the node as a serialized byte slice to the supplied io.Writer. func (node *Node) writeBytes(w io.Writer) error { - cause := amino.EncodeInt8(w, node.height) + if node == nil { + return errors.New("cannot write nil node") + } + cause := encodeVarint(w, int64(node.height)) if cause != nil { return errors.Wrap(cause, "writing height") } - cause = amino.EncodeVarint(w, node.size) + cause = encodeVarint(w, node.size) if cause != nil { return errors.Wrap(cause, "writing size") } - cause = amino.EncodeVarint(w, node.version) + cause = encodeVarint(w, node.version) if cause != nil { return errors.Wrap(cause, "writing version") } // Unlike writeHashBytes, key is written for inner nodes. - cause = amino.EncodeByteSlice(w, node.key) + cause = encodeBytes(w, node.key) if cause != nil { return errors.Wrap(cause, "writing key") } if node.isLeaf() { - cause = amino.EncodeByteSlice(w, node.value) + cause = encodeBytes(w, node.value) if cause != nil { return errors.Wrap(cause, "writing value") } @@ -388,7 +393,7 @@ func (node *Node) writeBytes(w io.Writer) error { if node.leftHash == nil { panic("node.leftHash was nil in writeBytes") } - cause = amino.EncodeByteSlice(w, node.leftHash) + cause = encodeBytes(w, node.leftHash) if cause != nil { return errors.Wrap(cause, "writing left hash") } @@ -396,7 +401,7 @@ func (node *Node) writeBytes(w io.Writer) error { if node.rightHash == nil { panic("node.rightHash was nil in writeBytes") } - cause = amino.EncodeByteSlice(w, node.rightHash) + cause = encodeBytes(w, node.rightHash) if cause != nil { return errors.Wrap(cause, "writing right hash") } diff --git a/node_test.go b/node_test.go index 6ae8e39eb..e9de0029e 100644 --- a/node_test.go +++ b/node_test.go @@ -2,6 +2,7 @@ package iavl import ( "bytes" + "encoding/hex" "math/rand" "testing" @@ -9,7 +10,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestNode_aminoSize(t *testing.T) { +func TestNode_encodedSize(t *testing.T) { node := &Node{ key: randBytes(10), value: randBytes(10), @@ -25,11 +26,61 @@ func TestNode_aminoSize(t *testing.T) { } // leaf node - require.Equal(t, 26, node.aminoSize()) + require.Equal(t, 26, node.encodedSize()) // non-leaf node node.height = 1 - require.Equal(t, 57, node.aminoSize()) + require.Equal(t, 57, node.encodedSize()) +} + +func TestNode_encode_decode(t *testing.T) { + testcases := map[string]struct { + node *Node + expectHex string + expectError bool + }{ + "nil": {nil, "", true}, + "empty": {&Node{}, "0000000000", false}, + "inner": {&Node{ + height: 3, + version: 2, + size: 7, + key: []byte("key"), + leftHash: []byte{0x70, 0x80, 0x90, 0xa0}, + rightHash: []byte{0x10, 0x20, 0x30, 0x40}, + }, "060e04036b657904708090a00410203040", false}, + "leaf": {&Node{ + height: 0, + version: 3, + size: 1, + key: []byte("key"), + value: []byte("value"), + }, "000206036b65790576616c7565", false}, + } + for name, tc := range testcases { + tc := tc + t.Run(name, func(t *testing.T) { + var buf bytes.Buffer + err := tc.node.writeBytes(&buf) + if tc.expectError { + require.Error(t, err) + return + } + require.NoError(t, err) + require.Equal(t, tc.expectHex, hex.EncodeToString(buf.Bytes())) + + node, err := MakeNode(buf.Bytes()) + require.NoError(t, err) + // since key and value is always decoded to []byte{} we augment the expected struct here + if tc.node.key == nil { + tc.node.key = []byte{} + } + if tc.node.value == nil && tc.node.height == 0 { + tc.node.value = []byte{} + } + require.Equal(t, tc.node, node) + }) + } } func TestNode_validate(t *testing.T) { @@ -80,7 +131,7 @@ func TestNode_validate(t *testing.T) { } } -func BenchmarkNode_aminoSize(b *testing.B) { +func BenchmarkNode_encodedSize(b *testing.B) { node := &Node{ key: randBytes(25), value: randBytes(100), @@ -93,7 +144,7 @@ func BenchmarkNode_aminoSize(b *testing.B) { b.ReportAllocs() b.ResetTimer() for i := 0; i < b.N; i++ { - node.aminoSize() + node.encodedSize() } } @@ -120,7 +171,7 @@ func BenchmarkNode_WriteBytes(b *testing.B) { sub.ReportAllocs() for i := 0; i < sub.N; i++ { var buf bytes.Buffer - buf.Grow(node.aminoSize()) + buf.Grow(node.encodedSize()) _ = node.writeBytes(&buf) } }) diff --git a/nodedb.go b/nodedb.go index 9c228013f..b51fabedb 100644 --- a/nodedb.go +++ b/nodedb.go @@ -115,7 +115,7 @@ func (ndb *nodeDB) SaveNode(node *Node) { // Save node bytes to db. var buf bytes.Buffer - buf.Grow(node.aminoSize()) + buf.Grow(node.encodedSize()) if err := node.writeBytes(&buf); err != nil { panic(err) diff --git a/proof.go b/proof.go index c369429dd..c2607bb09 100644 --- a/proof.go +++ b/proof.go @@ -8,7 +8,6 @@ import ( "github.com/pkg/errors" cmn "github.com/cosmos/iavl/common" - amino "github.com/tendermint/go-amino" ) var ( @@ -56,27 +55,27 @@ func (pin ProofInnerNode) Hash(childHash []byte) []byte { hasher := sha256.New() buf := new(bytes.Buffer) - err := amino.EncodeInt8(buf, pin.Height) + err := encodeVarint(buf, int64(pin.Height)) if err == nil { - err = amino.EncodeVarint(buf, pin.Size) + err = encodeVarint(buf, pin.Size) } if err == nil { - err = amino.EncodeVarint(buf, pin.Version) + err = encodeVarint(buf, pin.Version) } if len(pin.Left) == 0 { if err == nil { - err = amino.EncodeByteSlice(buf, childHash) + err = encodeBytes(buf, childHash) } if err == nil { - err = amino.EncodeByteSlice(buf, pin.Right) + err = encodeBytes(buf, pin.Right) } } else { if err == nil { - err = amino.EncodeByteSlice(buf, pin.Left) + err = encodeBytes(buf, pin.Left) } if err == nil { - err = amino.EncodeByteSlice(buf, childHash) + err = encodeBytes(buf, childHash) } } if err != nil { @@ -118,18 +117,18 @@ func (pln ProofLeafNode) Hash() []byte { hasher := sha256.New() buf := new(bytes.Buffer) - err := amino.EncodeInt8(buf, 0) + err := encodeVarint(buf, 0) if err == nil { - err = amino.EncodeVarint(buf, 1) + err = encodeVarint(buf, 1) } if err == nil { - err = amino.EncodeVarint(buf, pln.Version) + err = encodeVarint(buf, pln.Version) } if err == nil { - err = amino.EncodeByteSlice(buf, pln.Key) + err = encodeBytes(buf, pln.Key) } if err == nil { - err = amino.EncodeByteSlice(buf, pln.ValueHash) + err = encodeBytes(buf, pln.ValueHash) } if err != nil { panic(fmt.Sprintf("Failed to hash ProofLeafNode: %v", err)) diff --git a/testutils_test.go b/testutils_test.go index 4c6cc7a6c..33a84936e 100644 --- a/testutils_test.go +++ b/testutils_test.go @@ -11,7 +11,6 @@ import ( cmn "github.com/cosmos/iavl/common" "github.com/stretchr/testify/require" - amino "github.com/tendermint/go-amino" db "github.com/tendermint/tm-db" ) @@ -21,12 +20,15 @@ func randstr(length int) string { func i2b(i int) []byte { buf := new(bytes.Buffer) - amino.EncodeInt32(buf, int32(i)) + encodeVarint(buf, int64(i)) return buf.Bytes() } func b2i(bz []byte) int { - i, _, _ := amino.DecodeInt32(bz) + i, _, err := decodeVarint(bz) + if err != nil { + panic(err) + } return int(i) } diff --git a/tree_test.go b/tree_test.go index 7b40542b7..fd123fef0 100644 --- a/tree_test.go +++ b/tree_test.go @@ -3,8 +3,10 @@ package iavl import ( "bytes" + "encoding/hex" "flag" "fmt" + "math/rand" "os" "runtime" "strconv" @@ -97,6 +99,73 @@ func TestVersionedRandomTree(t *testing.T) { require.Equal(tree.nodeSize(), len(tree.ndb.nodes())) } +// nolint: dupl +func TestTreeHash(t *testing.T) { + const ( + randSeed = 49872768940 // For deterministic tests + keySize = 16 + valueSize = 16 + + versions = 4 // number of versions to generate + versionOps = 4096 // number of operations (create/update/delete) per version + updateRatio = 0.4 // ratio of updates out of all operations + deleteRatio = 0.2 // ratio of deletes out of all operations + ) + + // expected hashes for each version + expectHashes := []string{ + "58ec30fa27f338057e5964ed9ec3367e59b2b54bec4c194f10fde7fed16c2a1c", + "91ad3ace227372f0064b2d63e8493ce8f4bdcbd16c7a8e4f4d54029c9db9570c", + "92c25dce822c5968c228cfe7e686129ea281f79273d4a8fcf6f9130a47aa5421", + "e44d170925554f42e00263155c19574837a38e3efed8910daccc7fa12f560fa0", + } + require.Len(t, expectHashes, versions, "must have expected hashes for all versions") + + r := rand.New(rand.NewSource(randSeed)) + tree, err := NewMutableTree(db.NewMemDB(), 0) + require.NoError(t, err) + + keys := make([][]byte, 0, versionOps) + for i := 0; i < versions; i++ { + for j := 0; j < versionOps; j++ { + key := make([]byte, keySize) + value := make([]byte, valueSize) + + // The performance of this is likely to be terrible, but that's fine for small tests + switch { + case len(keys) > 0 && r.Float64() <= deleteRatio: + index := r.Intn(len(keys)) + key = keys[index] + keys = append(keys[:index], keys[index+1:]...) + _, removed := tree.Remove(key) + require.True(t, removed) + + case len(keys) > 0 && r.Float64() <= updateRatio: + key = keys[r.Intn(len(keys))] + r.Read(value) + updated := tree.Set(key, value) + require.True(t, updated) + + default: + r.Read(key) + r.Read(value) + // If we get an update, set again + for tree.Set(key, value) { + key = make([]byte, keySize) + r.Read(key) + } + keys = append(keys, key) + } + } + hash, version, err := tree.SaveVersion() + require.NoError(t, err) + require.EqualValues(t, i+1, version) + require.Equal(t, expectHashes[i], hex.EncodeToString(hash)) + } + + require.EqualValues(t, versions, tree.Version()) +} + func TestVersionedRandomTreeSmallKeys(t *testing.T) { require := require.New(t) d, closeDB := getTestDB()