Bez/tm0.26 update pt 2 redux (cosmos#2684)

* Update to TM v0.26.0
* Update TODOs
* Proof and verification updates
* Fix linting
* Fix key path creation
* Temporarily fix tendermint revision to make tests pass

Gopkg.toml

@@ -36,7 +36,7 @@
 
 [[override]]
   name = "github.com/tendermint/tendermint"
-  version = "v0.26.0"
+  revision = "ebee4377b15f2958b08994485375dd2ee8a649ac" # TODO replace w/ 0.26.1
 
 ## deps without releases:
 

client/context/query.go

@@ -12,6 +12,7 @@ import (
 
 	"github.com/cosmos/cosmos-sdk/store"
 	abci "github.com/tendermint/tendermint/abci/types"
+	"github.com/tendermint/tendermint/crypto/merkle"
 	cmn "github.com/tendermint/tendermint/libs/common"
 	tmliteErr "github.com/tendermint/tendermint/lite/errors"
 	tmliteProxy "github.com/tendermint/tendermint/lite/proxy"
@@ -197,38 +198,34 @@ func (ctx CLIContext) Verify(height int64) (tmtypes.SignedHeader, error) {
 }
 
 // verifyProof perform response proof verification.
-func (ctx CLIContext) verifyProof(_ string, resp abci.ResponseQuery) error {
+func (ctx CLIContext) verifyProof(queryPath string, resp abci.ResponseQuery) error {
 	if ctx.Verifier == nil {
 		return fmt.Errorf("missing valid certifier to verify data from distrusted node")
 	}
 
-	// TODO: handle in another TM v0.26 update PR
-	// // the AppHash for height H is in header H+1
-	// commit, err := ctx.Verify(resp.Height + 1)
-	// if err != nil {
-	// 	return err
-	// }
-
-	// var multiStoreProof store.MultiStoreProof
-	// cdc := codec.New()
-
-	// err = cdc.UnmarshalBinaryLengthPrefixed(resp.Proof, &multiStoreProof)
-	// if err != nil {
-	// 	return errors.Wrap(err, "failed to unmarshalBinary rangeProof")
-	// }
-
-	// // verify the substore commit hash against trusted appHash
-	// substoreCommitHash, err := store.VerifyMultiStoreCommitInfo(
-	// 	multiStoreProof.StoreName, multiStoreProof.StoreInfos, commit.Header.AppHash,
-	// )
-	// if err != nil {
-	// 	return errors.Wrap(err, "failed in verifying the proof against appHash")
-	// }
-
-	// err = store.VerifyRangeProof(resp.Key, resp.Value, substoreCommitHash, &multiStoreProof.RangeProof)
-	// if err != nil {
-	// 	return errors.Wrap(err, "failed in the range proof verification")
-	// }
+	// the AppHash for height H is in header H+1
+	commit, err := ctx.Verify(resp.Height + 1)
+	if err != nil {
+		return err
+	}
+
+	// TODO: Instead of reconstructing, stash on CLIContext field?
+	prt := store.DefaultProofRuntime()
+
+	// TODO: Better convention for path?
+	storeName, err := parseQueryStorePath(queryPath)
+	if err != nil {
+		return err
+	}
+
+	kp := merkle.KeyPath{}
+	kp = kp.AppendKey([]byte(storeName), merkle.KeyEncodingURL)
+	kp = kp.AppendKey(resp.Key, merkle.KeyEncodingURL)
+
+	err = prt.VerifyValue(resp.Proof, commit.Header.AppHash, kp.String(), resp.Value)
+	if err != nil {
+		return errors.Wrap(err, "failed to prove merkle proof")
+	}
 
 	return nil
 }
@@ -241,20 +238,40 @@ func (ctx CLIContext) queryStore(key cmn.HexBytes, storeName, endPath string) ([
 }
 
 // isQueryStoreWithProof expects a format like /<queryType>/<storeName>/<subpath>
-// queryType can be app or store.
+// queryType must be "store" and subpath must be "key" to require a proof.
 func isQueryStoreWithProof(path string) bool {
 	if !strings.HasPrefix(path, "/") {
 		return false
 	}
 
 	paths := strings.SplitN(path[1:], "/", 3)
-	if len(paths) != 3 {
+	switch {
+	case len(paths) != 3:
 		return false
-	}
-
-	if store.RequireProof("/" + paths[2]) {
+	case paths[0] != "store":
+		return false
+	case store.RequireProof("/" + paths[2]):
 		return true
 	}
 
 	return false
 }
+
+// parseQueryStorePath expects a format like /store/<storeName>/key.
+func parseQueryStorePath(path string) (storeName string, err error) {
+	if !strings.HasPrefix(path, "/") {
+		return "", errors.New("expected path to start with /")
+	}
+
+	paths := strings.SplitN(path[1:], "/", 3)
+	switch {
+	case len(paths) != 3:
+		return "", errors.New("expected format like /store/<storeName>/key")
+	case paths[0] != "store":
+		return "", errors.New("expected format like /store/<storeName>/key")
+	case paths[2] != "key":
+		return "", errors.New("expected format like /store/<storeName>/key")
+	}
+
+	return paths[1], nil
+}

store/iavlstore.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/tendermint/iavl"
 	abci "github.com/tendermint/tendermint/abci/types"
+	"github.com/tendermint/tendermint/crypto/merkle"
 	cmn "github.com/tendermint/tendermint/libs/common"
 	dbm "github.com/tendermint/tendermint/libs/db"
 
@@ -209,8 +210,8 @@ func (st *iavlStore) Query(req abci.RequestQuery) (res abci.ResponseQuery) {
 	res.Height = getHeight(tree, req)
 
 	switch req.Path {
-	case "/key":
-		key := req.Data // Data holds the key bytes
+	case "/key": // get by key
+		key := req.Data // data holds the key bytes
 
 		res.Key = key
 		if !st.VersionExists(res.Height) {
@@ -224,18 +225,8 @@ func (st *iavlStore) Query(req abci.RequestQuery) (res abci.ResponseQuery) {
 				res.Log = err.Error()
 				break
 			}
-
 			res.Value = value
-			// cdc := amino.NewCodec()
-
-			// p, err := cdc.MarshalBinaryLengthPrefixed(proof)
-			// if err != nil {
-			// 	res.Log = err.Error()
-			// 	break
-			// }
-
-			// TODO: handle in another TM v0.26 update PR
-			// res.Proof = p
+			res.Proof = &merkle.Proof{Ops: []merkle.ProofOp{iavl.NewIAVLValueOp(key, proof).ProofOp()}}
 		} else {
 			_, res.Value = tree.GetVersioned(key, res.Height)
 		}

store/multistoreproof.go

@@ -2,90 +2,139 @@ package store
 
 import (
 	"bytes"
+	"fmt"
 
-	"github.com/pkg/errors"
 	"github.com/tendermint/iavl"
+	"github.com/tendermint/tendermint/crypto/merkle"
 	cmn "github.com/tendermint/tendermint/libs/common"
 )
 
 // MultiStoreProof defines a collection of store proofs in a multi-store
 type MultiStoreProof struct {
 	StoreInfos []storeInfo
-	StoreName  string
-	RangeProof iavl.RangeProof
 }
 
-// buildMultiStoreProof build MultiStoreProof based on iavl proof and storeInfos
-func buildMultiStoreProof(iavlProof []byte, storeName string, storeInfos []storeInfo) []byte {
-	var rangeProof iavl.RangeProof
-	cdc.MustUnmarshalBinaryLengthPrefixed(iavlProof, &rangeProof)
+func NewMultiStoreProof(storeInfos []storeInfo) *MultiStoreProof {
+	return &MultiStoreProof{StoreInfos: storeInfos}
+}
 
-	msp := MultiStoreProof{
-		StoreInfos: storeInfos,
-		StoreName:  storeName,
-		RangeProof: rangeProof,
+// ComputeRootHash returns the root hash for a given multi-store proof.
+func (proof *MultiStoreProof) ComputeRootHash() []byte {
+	ci := commitInfo{
+		Version:    -1, // TODO: Not needed; improve code.
+		StoreInfos: proof.StoreInfos,
 	}
+	return ci.Hash()
+}
 
-	proof := cdc.MustMarshalBinaryLengthPrefixed(msp)
-	return proof
+// RequireProof returns whether proof is required for the subpath.
+func RequireProof(subpath string) bool {
+	// XXX: create a better convention.
+	// Currently, only when query subpath is "/key", will proof be included in
+	// response. If there are some changes about proof building in iavlstore.go,
+	// we must change code here to keep consistency with iavlStore#Query.
+	if subpath == "/key" {
+		return true
+	}
+
+	return false
 }
 
-// VerifyMultiStoreCommitInfo verify multiStoreCommitInfo against appHash
-func VerifyMultiStoreCommitInfo(storeName string, storeInfos []storeInfo, appHash []byte) ([]byte, error) {
-	var substoreCommitHash []byte
-	var height int64
-	for _, storeInfo := range storeInfos {
-		if storeInfo.Name == storeName {
-			substoreCommitHash = storeInfo.Core.CommitID.Hash
-			height = storeInfo.Core.CommitID.Version
-		}
+//-----------------------------------------------------------------------------
+
+var _ merkle.ProofOperator = MultiStoreProofOp{}
+
+// the multi-store proof operation constant value
+const ProofOpMultiStore = "multistore"
+
+// TODO: document
+type MultiStoreProofOp struct {
+	// Encoded in ProofOp.Key
+	key []byte
+
+	// To encode in ProofOp.Data.
+	Proof *MultiStoreProof `json:"proof"`
+}
+
+func NewMultiStoreProofOp(key []byte, proof *MultiStoreProof) MultiStoreProofOp {
+	return MultiStoreProofOp{
+		key:   key,
+		Proof: proof,
 	}
-	if len(substoreCommitHash) == 0 {
-		return nil, cmn.NewError("failed to get substore root commit hash by store name")
+}
+
+// MultiStoreProofOpDecoder returns a multi-store merkle proof operator from a
+// given proof operation.
+func MultiStoreProofOpDecoder(pop merkle.ProofOp) (merkle.ProofOperator, error) {
+	if pop.Type != ProofOpMultiStore {
+		return nil, cmn.NewError("unexpected ProofOp.Type; got %v, want %v", pop.Type, ProofOpMultiStore)
 	}
 
-	ci := commitInfo{
-		Version:    height,
-		StoreInfos: storeInfos,
+	// XXX: a bit strange as we'll discard this, but it works
+	var op MultiStoreProofOp
+
+	err := cdc.UnmarshalBinaryLengthPrefixed(pop.Data, &op)
+	if err != nil {
+		return nil, cmn.ErrorWrap(err, "decoding ProofOp.Data into MultiStoreProofOp")
 	}
-	if !bytes.Equal(appHash, ci.Hash()) {
-		return nil, cmn.NewError("the merkle root of multiStoreCommitInfo doesn't equal to appHash")
+
+	return NewMultiStoreProofOp(pop.Key, op.Proof), nil
+}
+
+// ProofOp return a merkle proof operation from a given multi-store proof
+// operation.
+func (op MultiStoreProofOp) ProofOp() merkle.ProofOp {
+	bz := cdc.MustMarshalBinaryLengthPrefixed(op)
+	return merkle.ProofOp{
+		Type: ProofOpMultiStore,
+		Key:  op.key,
+		Data: bz,
 	}
-	return substoreCommitHash, nil
 }
 
-// VerifyRangeProof verify iavl RangeProof
-func VerifyRangeProof(key, value []byte, substoreCommitHash []byte, rangeProof *iavl.RangeProof) error {
+// String implements the Stringer interface for a mult-store proof operation.
+func (op MultiStoreProofOp) String() string {
+	return fmt.Sprintf("MultiStoreProofOp{%v}", op.GetKey())
+}
 
-	// verify the proof to ensure data integrity.
-	err := rangeProof.Verify(substoreCommitHash)
-	if err != nil {
-		return errors.Wrap(err, "proof root hash doesn't equal to substore commit root hash")
+// GetKey returns the key for a multi-store proof operation.
+func (op MultiStoreProofOp) GetKey() []byte {
+	return op.key
+}
+
+// Run executes a multi-store proof operation for a given value. It returns
+// the root hash if the value matches all the store's commitID's hash or an
+// error otherwise.
+func (op MultiStoreProofOp) Run(args [][]byte) ([][]byte, error) {
+	if len(args) != 1 {
+		return nil, cmn.NewError("Value size is not 1")
 	}
 
-	if len(value) != 0 {
-		// verify existence proof
-		err = rangeProof.VerifyItem(key, value)
-		if err != nil {
-			return errors.Wrap(err, "failed in existence verification")
-		}
-	} else {
-		// verify absence proof
-		err = rangeProof.VerifyAbsence(key)
-		if err != nil {
-			return errors.Wrap(err, "failed in absence verification")
+	value := args[0]
+	root := op.Proof.ComputeRootHash()
+
+	for _, si := range op.Proof.StoreInfos {
+		if si.Name == string(op.key) {
+			if bytes.Equal(value, si.Core.CommitID.Hash) {
+				return [][]byte{root}, nil
+			}
+
+			return nil, cmn.NewError("hash mismatch for substore %v: %X vs %X", si.Name, si.Core.CommitID.Hash, value)
 		}
 	}
 
-	return nil
+	return nil, cmn.NewError("key %v not found in multistore proof", op.key)
 }
 
-// RequireProof return whether proof is require for the subpath
-func RequireProof(subpath string) bool {
-	// Currently, only when query subpath is "/store" or "/key", will proof be included in response.
-	// If there are some changes about proof building in iavlstore.go, we must change code here to keep consistency with iavlstore.go:212
-	if subpath == "/store" || subpath == "/key" {
-		return true
-	}
-	return false
+//-----------------------------------------------------------------------------
+
+// XXX: This should be managed by the rootMultiStore which may want to register
+// more proof ops?
+func DefaultProofRuntime() (prt *merkle.ProofRuntime) {
+	prt = merkle.NewProofRuntime()
+	prt.RegisterOpDecoder(merkle.ProofOpSimpleValue, merkle.SimpleValueOpDecoder)
+	prt.RegisterOpDecoder(iavl.ProofOpIAVLValue, iavl.IAVLValueOpDecoder)
+	prt.RegisterOpDecoder(iavl.ProofOpIAVLAbsence, iavl.IAVLAbsenceOpDecoder)
+	prt.RegisterOpDecoder(ProofOpMultiStore, MultiStoreProofOpDecoder)
+	return
 }