General Merkle Follow Up (#2510)

* tmlibs -> libs

* update changelog

* address some comments from review of #2298

CHANGELOG_PENDING.md

@@ -5,21 +5,29 @@ Special thanks to external contributors on this release:
 BREAKING CHANGES:
 
 * CLI/RPC/Config
-- [config] `mempool.wal` is disabled by default
+  * [config] `mempool.wal` is disabled by default
+  * [rpc] \#2298 `/abci_query` takes `prove` argument instead of `trusted` and switches the default
+    behaviour to `prove=false`
+  * [privval] \#2459 Split `SocketPVMsg`s implementations into Request and Response, where the Response may contain a error message (returned by the remote signer).
 
 * Apps
+  * [abci] \#2298 ResponseQuery.Proof is now a structured merkle.Proof, not just
+    arbitrary bytes
 
 * Go API
-- [node] Remove node.RunForever
-- [config] \#2232 timeouts as time.Duration, not ints
+  * [node] Remove node.RunForever
+  * [config] \#2232 timeouts as time.Duration, not ints
+  * [rpc/client] \#2298 `ABCIQueryOptions.Trusted` -> `ABCIQueryOptions.Prove`
+  * [types] \#2298 Remove `Index` and `Total` fields from `TxProof`.
+  * [crypto/merkle & lite] \#2298 Various changes to accomodate General Merkle trees
 
 * Blockchain Protocol
   * [types] \#2459 `Vote`/`Proposal`/`Heartbeat` use amino encoding instead of JSON in `SignBytes`.
-  * [privval] \#2459 Split `SocketPVMsg`s implementations into Request and Response, where the Response may contain a error message (returned by the remote signer). 
 
 * P2P Protocol
 
 FEATURES:
+- [crypto/merkle] \#2298 General Merkle Proof scheme for chaining various types of Merkle trees together
 
 IMPROVEMENTS:
 - [consensus] [\#2169](https://github.com/cosmos/cosmos-sdk/issues/2169) add additional metrics

README.md

@@ -118,11 +118,12 @@ CHANGELOG even if they don't lead to MINOR version bumps:
 - rpc/client
 - config
 - node
-- libs/bech32
-- libs/common
-- libs/db
-- libs/errors
-- libs/log
+- libs
+  - bech32
+  - common
+  - db
+  - errors
+  - log
 
 Exported objects in these packages that are not covered by the versioning scheme
 are explicitly marked by `// UNSTABLE` in their go doc comment and may change at any

config/config.go

@@ -20,7 +20,7 @@ const (
 // generate the config.toml. Please reflect any changes
 // made here in the defaultConfigTemplate constant in
 // config/toml.go
-// NOTE: tmlibs/cli must know to look in the config dir!
+// NOTE: libs/cli must know to look in the config dir!
 var (
 	DefaultTendermintDir = ".tendermint"
 	defaultConfigDir     = "config"

crypto/merkle/proof.go

@@ -3,17 +3,19 @@ package merkle
 import (
 	"bytes"
 
-	cmn "github.com/tendermint/tmlibs/common"
+	cmn "github.com/tendermint/tendermint/libs/common"
 )
 
 //----------------------------------------
 // ProofOp gets converted to an instance of ProofOperator:
 
-// ProofOperator is a layer for calculating intermediate Merkle root
-// Run() takes a list of bytes because it can be more than one
-// for example in range proofs
-// ProofOp() defines custom encoding which can be decoded later with
-// OpDecoder
+// ProofOperator is a layer for calculating intermediate Merkle roots
+// when a series of Merkle trees are chained together.
+// Run() takes leaf values from a tree and returns the Merkle
+// root for the corresponding tree. It takes and returns a list of bytes
+// to allow multiple leaves to be part of a single proof, for instance in a range proof.
+// ProofOp() encodes the ProofOperator in a generic way so it can later be
+// decoded with OpDecoder.
 type ProofOperator interface {
 	Run([][]byte) ([][]byte, error)
 	GetKey() []byte
@@ -23,8 +25,8 @@ type ProofOperator interface {
 //----------------------------------------
 // Operations on a list of ProofOperators
 
-// ProofOperators is a slice of ProofOperator(s)
-// Each operator will be applied to the input value sequencially
+// ProofOperators is a slice of ProofOperator(s).
+// Each operator will be applied to the input value sequentially
 // and the last Merkle root will be verified with already known data
 type ProofOperators []ProofOperator
 
@@ -91,8 +93,8 @@ func (prt *ProofRuntime) Decode(pop ProofOp) (ProofOperator, error) {
 	return decoder(pop)
 }
 
-func (prt *ProofRuntime) DecodeProof(proof *Proof) (poz ProofOperators, err error) {
-	poz = ProofOperators(nil)
+func (prt *ProofRuntime) DecodeProof(proof *Proof) (ProofOperators, error) {
+	var poz ProofOperators
 	for _, pop := range proof.Ops {
 		operator, err := prt.Decode(pop)
 		if err != nil {
@@ -100,7 +102,7 @@ func (prt *ProofRuntime) DecodeProof(proof *Proof) (poz ProofOperators, err erro
 		}
 		poz = append(poz, operator)
 	}
-	return
+	return poz, nil
 }
 
 func (prt *ProofRuntime) VerifyValue(proof *Proof, root []byte, keypath string, value []byte) (err error) {

crypto/merkle/proof_key_path.go

@@ -35,6 +35,8 @@ import (
 	kp.AppendKey([]byte{0x01, 0x02, 0x03}, KeyEncodingURL)
 	kp.String() // Should return "/App/IBC/x:010203"
 
+	NOTE: Key paths must begin with a `/`.
+
 	NOTE: All encodings *MUST* work compatibly, such that you can choose to use
 	whatever encoding, and the decoded keys will always be the same.  In other
 	words, it's just as good to encode all three keys using URL encoding or HEX
@@ -52,7 +54,7 @@ type keyEncoding int
 const (
 	KeyEncodingURL keyEncoding = iota
 	KeyEncodingHex
-	KeyEncodingMax
+	KeyEncodingMax // Number of known encodings. Used for testing
 )
 
 type Key struct {
@@ -81,6 +83,8 @@ func (pth KeyPath) String() string {
 	return res
 }
 
+// Decode a path to a list of keys. Path must begin with `/`.
+// Each key must use a known encoding.
 func KeyPathToKeys(path string) (keys [][]byte, err error) {
 	if path == "" || path[0] != '/' {
 		return nil, cmn.NewError("key path string must start with a forward slash '/'")

crypto/merkle/proof_simple_value.go

@@ -25,7 +25,7 @@ type SimpleValueOp struct {
 	key []byte
 
 	// To encode in ProofOp.Data
-	Proof *SimpleProof `json:"simple-proof"`
+	Proof *SimpleProof `json:"simple_proof"`
 }
 
 var _ ProofOperator = SimpleValueOp{}

libs/common/types.pb.go

@@ -26,7 +26,7 @@ var _ = math.Inf
 // proto package needs to be updated.
 const _ = proto.GoGoProtoPackageIsVersion2 // please upgrade the proto package
 
-// Define these here for compatibility but use tmlibs/common.KVPair.
+// Define these here for compatibility but use libs/common.KVPair.
 type KVPair struct {
 	Key                  []byte   `protobuf:"bytes,1,opt,name=key,proto3" json:"key,omitempty"`
 	Value                []byte   `protobuf:"bytes,2,opt,name=value,proto3" json:"value,omitempty"`
@@ -82,7 +82,7 @@ func (m *KVPair) GetValue() []byte {
 	return nil
 }
 
-// Define these here for compatibility but use tmlibs/common.KI64Pair.
+// Define these here for compatibility but use libs/common.KI64Pair.
 type KI64Pair struct {
 	Key                  []byte   `protobuf:"bytes,1,opt,name=key,proto3" json:"key,omitempty"`
 	Value                int64    `protobuf:"varint,2,opt,name=value,proto3" json:"value,omitempty"`

lite/doc.go

@@ -88,7 +88,7 @@ type PersistentProvider interface {
 }
 ```
 
-* DBProvider - persistence provider for use with any tmlibs/DB.
+* DBProvider - persistence provider for use with any libs/DB.
 * MultiProvider - combine multiple providers.
 
 The suggested use for local light clients is client.NewHTTPProvider(...) for

lite/dynamic_verifier.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"fmt"
 	"sync"
+
 	log "github.com/tendermint/tendermint/libs/log"
 	lerr "github.com/tendermint/tendermint/lite/errors"
 	"github.com/tendermint/tendermint/types"
@@ -25,10 +26,9 @@ type DynamicVerifier struct {
 	// This is a source of new info, like a node rpc, or other import method.
 	source Provider
 
-	// pending map for synchronize concurrent verification requests
+	// pending map to synchronize concurrent verification requests
+	mtx                  sync.Mutex
 	pendingVerifications map[int64]chan struct{}
-
-	mtx sync.Mutex
 }
 
 // NewDynamicVerifier returns a new DynamicVerifier. It uses the