proof: migrate ProofOp encoding to Protobuf (#287)

Fixes #242. Switches ProofOp encoding from Amino to Protobuf, but retains the same byte layout for backwards compatibility. This is not strictly Protobuf since the proofs are length-prefixed, but that's what Amino did so that's what we'll keep doing.

The Protobuf types need to be moved to a separate package and cleaned up: they conflict with the native types, and we're making the same mistake as before by leaking Protobuf types through public APIs. I'll submit a separate PR for this.

CHANGELOG.md

@@ -10,6 +10,10 @@ https://github.com/cosmos/iavl. This also affects the module import path, which
 
 - The module path has changed from `github.com/tendermint/iavl` to `github.com/cosmos/iavl`.
 
+### Improvements
+
+- Proofs are now encoded using Protobuf instead of Amino. The binary encoding is identical.
+
 ## 0.14.0 (July 2, 2020)
 
 **Important information:** the pruning functionality introduced with IAVL 0.13.0 via the options 

encoding.go

@@ -1,6 +1,7 @@
 package iavl
 
 import (
+	"bytes"
 	"encoding/binary"
 	"errors"
 	"fmt"
@@ -68,6 +69,13 @@ func encodeBytes(w io.Writer, bz []byte) error {
 	return err
 }
 
+// encodeBytesSlice length-prefixes the byte slice and returns it.
+func encodeBytesSlice(bz []byte) ([]byte, error) {
+	var buf bytes.Buffer
+	err := encodeBytes(&buf, bz)
+	return buf.Bytes(), 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)

go.mod

@@ -5,11 +5,9 @@ go 1.13
 require (
 	github.com/gogo/protobuf v1.3.1
 	github.com/golang/protobuf v1.4.2 // indirect
-	github.com/google/gofuzz v1.0.0 // indirect
 	github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e // indirect
 	github.com/pkg/errors v0.9.1
 	github.com/stretchr/testify v1.6.1
-	github.com/tendermint/go-amino v0.14.1
 	github.com/tendermint/tm-db v0.6.0
 	golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9
 	golang.org/x/text v0.3.2 // indirect

go.sum

@@ -46,8 +46,6 @@ github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMyw
 github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
 github.com/google/go-cmp v0.4.0 h1:xsAVV57WRhGj6kEIi8ReJzQlHHqcBYCElAvkovg3B/4=
 github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
-github.com/google/gofuzz v1.0.0 h1:A8PeW59pxE9IoFRqBp37U+mSNaQoZ46F1f0f863XSXw=
-github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
 github.com/hpcloud/tail v1.0.0 h1:nfCOvKYfkgYP8hkirhJocXT2+zOD8yUNjXaWfTlyFKI=
 github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU=
 github.com/jmhodges/levigo v1.0.0 h1:q5EC36kV79HWeTBWsod3mG11EgStG3qArTKcvlksN1U=
@@ -78,8 +76,6 @@ github.com/syndtr/goleveldb v1.0.1-0.20190923125748-758128399b1d h1:gZZadD8H+fF+
 github.com/syndtr/goleveldb v1.0.1-0.20190923125748-758128399b1d/go.mod h1:9OrXJhf154huy1nPWmuSrkgjPUtUNhA+Zmy+6AESzuA=
 github.com/tecbot/gorocksdb v0.0.0-20191217155057-f0fad39f321c h1:g+WoO5jjkqGAzHWCjJB1zZfXPIAaDpzXIEJ0eS6B5Ok=
 github.com/tecbot/gorocksdb v0.0.0-20191217155057-f0fad39f321c/go.mod h1:ahpPrc7HpcfEWDQRZEmnXMzHY03mLDYMCxeDzy46i+8=
-github.com/tendermint/go-amino v0.14.1 h1:o2WudxNfdLNBwMyl2dqOJxiro5rfrEaU0Ugs6offJMk=
-github.com/tendermint/go-amino v0.14.1/go.mod h1:i/UKE5Uocn+argJJBb12qTZsCDBcAYMbR92AaJVmKso=
 github.com/tendermint/tm-db v0.6.0 h1:Us30k7H1UDcdqoSPhmP8ztAW/SWV6c6OfsfeCiboTC4=
 github.com/tendermint/tm-db v0.6.0/go.mod h1:xj3AWJ08kBDlCHKijnhJ7mTcDMOikT1r8Poxy2pJn7Q=
 go.etcd.io/bbolt v1.3.5 h1:XAzx9gjCb0Rxj7EoqcClPD1d5ZBxZJk0jbuoPHenBt0=

proof.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"crypto/sha256"
 	"fmt"
+	"math"
 
 	"github.com/pkg/errors"
 
@@ -89,6 +90,34 @@ func (pin ProofInnerNode) Hash(childHash []byte) []byte {
 	return hasher.Sum(nil)
 }
 
+// toProto converts the inner node proof to Protobuf, for use in ProofOps.
+func (pin ProofInnerNode) toProto() *ProofOpInner {
+	return &ProofOpInner{
+		Height:  int32(pin.Height),
+		Size_:   pin.Size,
+		Version: pin.Version,
+		Left:    pin.Left,
+		Right:   pin.Right,
+	}
+}
+
+// proofInnerNodeFromProto converts a Protobuf ProofOpInner to a ProofInnerNode.
+func proofInnerNodeFromProto(pbInner *ProofOpInner) (ProofInnerNode, error) {
+	if pbInner == nil {
+		return ProofInnerNode{}, errors.New("inner node cannot be nil")
+	}
+	if pbInner.Height > math.MaxInt8 || pbInner.Height < math.MinInt8 {
+		return ProofInnerNode{}, fmt.Errorf("height must fit inside an int8, got %v", pbInner.Height)
+	}
+	return ProofInnerNode{
+		Height:  int8(pbInner.Height),
+		Size:    pbInner.Size_,
+		Version: pbInner.Version,
+		Left:    pbInner.Left,
+		Right:   pbInner.Right,
+	}, nil
+}
+
 //----------------------------------------
 
 type ProofLeafNode struct {
@@ -142,6 +171,27 @@ func (pln ProofLeafNode) Hash() []byte {
 	return hasher.Sum(nil)
 }
 
+// toProto converts the leaf node proof to Protobuf, for use in ProofOps.
+func (pln ProofLeafNode) toProto() *ProofOpLeaf {
+	return &ProofOpLeaf{
+		Key:       pln.Key,
+		ValueHash: pln.ValueHash,
+		Version:   pln.Version,
+	}
+}
+
+// proofLeafNodeFromProto converts a Protobuf ProofOpInner to a ProofLeafNode.
+func proofLeafNodeFromProto(pbLeaf *ProofOpLeaf) (ProofLeafNode, error) {
+	if pbLeaf == nil {
+		return ProofLeafNode{}, errors.New("leaf node cannot be nil")
+	}
+	return ProofLeafNode{
+		Key:       pbLeaf.Key,
+		ValueHash: pbLeaf.ValueHash,
+		Version:   pbLeaf.Version,
+	}, nil
+}
+
 //----------------------------------------
 
 // If the key does not exist, returns the path to the next leaf left of key (w/

proof_iavl_absence.go

@@ -3,6 +3,7 @@ package iavl
 import (
 	"fmt"
 
+	proto "github.com/gogo/protobuf/proto"
 	"github.com/pkg/errors"
 )
 
@@ -35,16 +36,37 @@ func AbsenceOpDecoder(pop ProofOp) (ProofOperator, error) {
 	if pop.Type != ProofOpIAVLAbsence {
 		return nil, errors.Errorf("unexpected ProofOp.Type; got %v, want %v", pop.Type, ProofOpIAVLAbsence)
 	}
-	var op AbsenceOp // a bit strange as we'll discard this, but it works.
-	err := cdc.UnmarshalBinaryLengthPrefixed(pop.Data, &op)
+	// Strip the varint length prefix, used for backwards compatibility with Amino.
+	bz, n, err := decodeBytes(pop.Data)
 	if err != nil {
-		return nil, errors.Wrap(err, "decoding ProofOp.Data into IAVLAbsenceOp")
+		return nil, err
 	}
-	return NewAbsenceOp(pop.Key, op.Proof), nil
+	if n != len(pop.Data) {
+		return nil, fmt.Errorf("unexpected bytes, expected %v got %v", n, len(pop.Data))
+	}
+	pbProofOp := &ProofOpAbsence{}
+	err = proto.Unmarshal(bz, pbProofOp)
+	if err != nil {
+		return nil, err
+	}
+	proof, err := rangeProofFromProto(pbProofOp.Proof)
+	if err != nil {
+		return nil, err
+	}
+	return NewAbsenceOp(pop.Key, &proof), nil
 }
 
 func (op AbsenceOp) ProofOp() ProofOp {
-	bz := cdc.MustMarshalBinaryLengthPrefixed(op)
+	pbProof := ProofOpAbsence{Proof: op.Proof.toProto()}
+	bz, err := pbProof.Marshal()
+	if err != nil {
+		panic(err)
+	}
+	// We length-prefix the byte slice to retain backwards compatibility with the Amino proofs.
+	bz, err = encodeBytesSlice(bz)
+	if err != nil {
+		panic(err)
+	}
 	return ProofOp{
 		Type: ProofOpIAVLAbsence,
 		Key:  op.key,

proof_iavl_test.go

@@ -0,0 +1,102 @@
+package iavl
+
+import (
+	"encoding/hex"
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	db "github.com/tendermint/tm-db"
+)
+
+func TestProofOp(t *testing.T) {
+	tree, err := NewMutableTreeWithOpts(db.NewMemDB(), 0, nil)
+	require.NoError(t, err)
+	keys := []byte{0x0a, 0x11, 0x2e, 0x32, 0x50, 0x72, 0x99, 0xa1, 0xe4, 0xf7} // 10 total.
+	for _, ikey := range keys {
+		key := []byte{ikey}
+		tree.Set(key, key)
+	}
+	root := tree.WorkingHash()
+
+	testcases := []struct {
+		key           byte
+		expectPresent bool
+		expectProofOp string
+	}{
+		{0x00, false, "aa010aa7010a280808100a18012a2022b4e34a1778d6a03aac39f00d89deb886e0cc37454e300b7aebeb4f4939c0790a280804100418012a20734fad809673ab2b9672453a8b2bc8c9591e2d1d97933df5b4c3b0531bf82e720a280802100218012a20154b101a72acffe0f5e65d1e144a57dc6f97758d2049821231f02b6a5b44fe811a270a010a122001ba4719c80b6fe911b091a7c05124b64eeece964e09c058ef8f9805daca546b1801"},
+		{0x0a, true, "aa010aa7010a280808100a18012a2022b4e34a1778d6a03aac39f00d89deb886e0cc37454e300b7aebeb4f4939c0790a280804100418012a20734fad809673ab2b9672453a8b2bc8c9591e2d1d97933df5b4c3b0531bf82e720a280802100218012a20154b101a72acffe0f5e65d1e144a57dc6f97758d2049821231f02b6a5b44fe811a270a010a122001ba4719c80b6fe911b091a7c05124b64eeece964e09c058ef8f9805daca546b1801"},
+		{0x0b, false, "d5010ad2010a280808100a18012a2022b4e34a1778d6a03aac39f00d89deb886e0cc37454e300b7aebeb4f4939c0790a280804100418012a20734fad809673ab2b9672453a8b2bc8c9591e2d1d97933df5b4c3b0531bf82e720a280802100218012a20154b101a72acffe0f5e65d1e144a57dc6f97758d2049821231f02b6a5b44fe8112001a270a010a122001ba4719c80b6fe911b091a7c05124b64eeece964e09c058ef8f9805daca546b18011a270a011112204a64a107f0cb32536e5bce6c98c393db21cca7f4ea187ba8c4dca8b51d4ea80a1801"},
+		{0x11, true, "aa010aa7010a280808100a18012a2022b4e34a1778d6a03aac39f00d89deb886e0cc37454e300b7aebeb4f4939c0790a280804100418012a20734fad809673ab2b9672453a8b2bc8c9591e2d1d97933df5b4c3b0531bf82e720a28080210021801222053d2828f35e33aecab8e411a40afb0475288973b96aed2220e9894f43a5375ad1a270a011112204a64a107f0cb32536e5bce6c98c393db21cca7f4ea187ba8c4dca8b51d4ea80a1801"},
+		{0x60, false, "d5010ad2010a280808100a18012220e39776faa9ef2b83ae828860d24f807efab321d02b78081c0e68e1bf801b0e220a280806100618012a20631b10ce49ece4cc9130befac927865742fb11caf2e8fc08fc00a4a25e4bc7940a280802100218012a207a4a97f565ae0b3ea8abf175208f176ac8301665ac2d26c89be3664f90e23da612001a270a015012205c62e091b8c0565f1bafad0dad5934276143ae2ccef7a5381e8ada5b1a8d26d218011a270a01721220454349e422f05297191ead13e21d3db520e5abef52055e4964b82fb213f593a11801"},
+		{0x72, true, "aa010aa7010a280808100a18012220e39776faa9ef2b83ae828860d24f807efab321d02b78081c0e68e1bf801b0e220a280806100618012a20631b10ce49ece4cc9130befac927865742fb11caf2e8fc08fc00a4a25e4bc7940a28080210021801222035f8ea805390e084854f399b42ccdeaea33a1dedc115638ac48d0600637dba1f1a270a01721220454349e422f05297191ead13e21d3db520e5abef52055e4964b82fb213f593a11801"},
+		{0x99, true, "d4010ad1010a280808100a18012220e39776faa9ef2b83ae828860d24f807efab321d02b78081c0e68e1bf801b0e220a2808061006180122201d6b29f2c439fc9f15703eb7031e4a216002ea36ee9496583f97b20302b6a74e0a280804100418012a2043b83a6acefd4fd33970d1bc8fc47bed81220c752b8de7053e8ee082a2c7c1290a280802100218012a208f69a1db006c0ee9fad3c7c624b92acc88e9ed00771976ea24a64796c236fef01a270a01991220fd9528b920d6d3956e9e16114523e1889c751e8c1e040182116d4c906b43f5581801"},
+		{0xaa, false, "a9020aa6020a280808100a18012220e39776faa9ef2b83ae828860d24f807efab321d02b78081c0e68e1bf801b0e220a2808061006180122201d6b29f2c439fc9f15703eb7031e4a216002ea36ee9496583f97b20302b6a74e0a280804100418012a2043b83a6acefd4fd33970d1bc8fc47bed81220c752b8de7053e8ee082a2c7c1290a280802100218012220a303930ca8831618ac7e4ddd10546cfc366fb730d6630c030a97226bbefc6935122a0a280802100218012a2077ad141b2010cf7107de941aac5b46f44fa4f41251076656a72308263a964fb91a270a01a112208a8950f7623663222542c9469c73be3c4c81bbdf019e2c577590a61f2ce9a15718011a270a01e412205e1effe9b7bab73dce628ccd9f0cbbb16c1e6efc6c4f311e59992a467bc119fd1801"},
+		{0xe4, true, "d4010ad1010a280808100a18012220e39776faa9ef2b83ae828860d24f807efab321d02b78081c0e68e1bf801b0e220a2808061006180122201d6b29f2c439fc9f15703eb7031e4a216002ea36ee9496583f97b20302b6a74e0a2808041004180122208bc4764843fdd745dc853fa62f2fac0001feae9e46136192f466c09773e2ed050a280802100218012a2077ad141b2010cf7107de941aac5b46f44fa4f41251076656a72308263a964fb91a270a01e412205e1effe9b7bab73dce628ccd9f0cbbb16c1e6efc6c4f311e59992a467bc119fd1801"},
+		{0xf7, true, "d4010ad1010a280808100a18012220e39776faa9ef2b83ae828860d24f807efab321d02b78081c0e68e1bf801b0e220a2808061006180122201d6b29f2c439fc9f15703eb7031e4a216002ea36ee9496583f97b20302b6a74e0a2808041004180122208bc4764843fdd745dc853fa62f2fac0001feae9e46136192f466c09773e2ed050a28080210021801222032af6e3eec2b63d5fe1bd992a89ef3467b3cee639c068cace942f01326098f171a270a01f7122050868f20258bbc9cce0da2719e8654c108733dd2f663b8737c574ec0ead93eb31801"},
+		{0xfe, false, "d4010ad1010a280808100a18012220e39776faa9ef2b83ae828860d24f807efab321d02b78081c0e68e1bf801b0e220a2808061006180122201d6b29f2c439fc9f15703eb7031e4a216002ea36ee9496583f97b20302b6a74e0a2808041004180122208bc4764843fdd745dc853fa62f2fac0001feae9e46136192f466c09773e2ed050a28080210021801222032af6e3eec2b63d5fe1bd992a89ef3467b3cee639c068cace942f01326098f171a270a01f7122050868f20258bbc9cce0da2719e8654c108733dd2f663b8737c574ec0ead93eb31801"},
+		//{0xff, false, ""}, // FIXME This panics, see https://github.com/cosmos/iavl/issues/286
+	}
+
+	for _, tc := range testcases {
+		tc := tc
+		t.Run(fmt.Sprintf("%02x", tc.key), func(t *testing.T) {
+			key := []byte{tc.key}
+			value, proof, err := tree.GetWithProof(key)
+			require.NoError(t, err)
+
+			// Verify that proof is valid.
+			err = proof.Verify(root)
+			require.NoError(t, err)
+
+			// Encode and decode proof, either ValueOp or AbsentOp depending on key existence.
+			expectBytes, err := hex.DecodeString(tc.expectProofOp)
+			require.NoError(t, err)
+
+			if tc.expectPresent {
+				require.NotNil(t, value)
+				err = proof.VerifyItem(key, value)
+				require.NoError(t, err)
+
+				valueOp := NewValueOp(key, proof)
+				proofOp := valueOp.ProofOp()
+				assert.Equal(t, ProofOp{
+					Type: ProofOpIAVLValue,
+					Key:  key,
+					Data: expectBytes,
+				}, proofOp)
+
+				//t.Logf("Expect: %x", expectBytes)
+				//t.Logf("Actual: %x", proofOp.Data)
+
+				d, e := ValueOpDecoder(proofOp)
+				require.NoError(t, e)
+				decoded := d.(ValueOp)
+				err = decoded.Proof.Verify(root)
+				require.NoError(t, err)
+				assert.Equal(t, valueOp, decoded)
+
+			} else {
+				require.Nil(t, value)
+				err = proof.VerifyAbsence(key)
+				require.NoError(t, err)
+
+				absenceOp := NewAbsenceOp(key, proof)
+				proofOp := absenceOp.ProofOp()
+				assert.Equal(t, ProofOp{
+					Type: ProofOpIAVLAbsence,
+					Key:  key,
+					Data: expectBytes,
+				}, proofOp)
+
+				d, e := AbsenceOpDecoder(proofOp)
+				require.NoError(t, e)
+				decoded := d.(AbsenceOp)
+				err = decoded.Proof.Verify(root)
+				require.NoError(t, err)
+				assert.Equal(t, absenceOp, decoded)
+			}
+		})
+	}
+}

proof_iavl_value.go

@@ -3,6 +3,7 @@ package iavl
 import (
 	"fmt"
 
+	proto "github.com/gogo/protobuf/proto"
 	"github.com/pkg/errors"
 )
 
@@ -36,16 +37,37 @@ func ValueOpDecoder(pop ProofOp) (ProofOperator, error) {
 	if pop.Type != ProofOpIAVLValue {
 		return nil, errors.Errorf("unexpected ProofOp.Type; got %v, want %v", pop.Type, ProofOpIAVLValue)
 	}
-	var op ValueOp // a bit strange as we'll discard this, but it works.
-	err := cdc.UnmarshalBinaryLengthPrefixed(pop.Data, &op)
+	// Strip the varint length prefix, used for backwards compatibility with Amino.
+	bz, n, err := decodeBytes(pop.Data)
 	if err != nil {
-		return nil, errors.Wrap(err, "decoding ProofOp.Data into IAVLValueOp")
+		return nil, err
 	}
-	return NewValueOp(pop.Key, op.Proof), nil
+	if n != len(pop.Data) {
+		return nil, fmt.Errorf("unexpected bytes, expected %v got %v", n, len(pop.Data))
+	}
+	pbProofOp := &ProofOpValue{}
+	err = proto.Unmarshal(bz, pbProofOp)
+	if err != nil {
+		return nil, err
+	}
+	proof, err := rangeProofFromProto(pbProofOp.Proof)
+	if err != nil {
+		return nil, err
+	}
+	return NewValueOp(pop.Key, &proof), nil
 }
 
 func (op ValueOp) ProofOp() ProofOp {
-	bz := cdc.MustMarshalBinaryLengthPrefixed(op)
+	pbProof := ProofOpValue{Proof: op.Proof.toProto()}
+	bz, err := pbProof.Marshal()
+	if err != nil {
+		panic(err)
+	}
+	// We length-prefix the byte slice to retain backwards compatibility with the Amino proofs.
+	bz, err = encodeBytesSlice(bz)
+	if err != nil {
+		panic(err)
+	}
 	return ProofOp{
 		Type: ProofOpIAVLValue,
 		Key:  op.key,