Addressed review for #1815 except those marked as 'TODO make issue'

CHANGELOG.md

@@ -556,7 +556,7 @@ BREAKING CHANGES:
     - use scripts/wal2json to convert to json for debugging
 
 FEATURES:
- - new `certifiers` pkg contains the tendermint light-client library (name subject to change)!
+ - new `Verifiers` pkg contains the tendermint light-client library (name subject to change)!
  - rpc: `/genesis` includes the `app_options` .
  - rpc: `/abci_query` takes an additional `height` parameter to support historical queries.
  - rpc/client: new ABCIQueryWithOptions supports options like `trusted` (set false to get a proof) and `height` to query a historical height.

Gopkg.lock

@@ -11,7 +11,7 @@
   branch = "master"
   name = "github.com/btcsuite/btcd"
   packages = ["btcec"]
-  revision = "9a2f9524024889e129a5422aca2cff73cb3eabf6"
+  revision = "f5e261fc9ec3437697fb31d8b38453c293204b29"
 
 [[projects]]
   name = "github.com/btcsuite/btcutil"
@@ -342,7 +342,7 @@
     "cpu",
     "unix"
   ]
-  revision = "bd9dbc187b6e1dacfdd2722a87e83093c2d7bd6e"
+  revision = "3dc4335d56c789b04b0ba99b7a37249d9b614314"
 
 [[projects]]
   name = "golang.org/x/text"

cmd/tendermint/commands/lite.go

@@ -68,10 +68,10 @@ func runProxy(cmd *cobra.Command, args []string) error {
 	logger.Info("Connecting to source HTTP client...")
 	node := rpcclient.NewHTTP(nodeAddr, "/websocket")
 
-	logger.Info("Constructing certifier...")
-	cert, err := proxy.NewCertifier(chainID, home, node, logger)
+	logger.Info("Constructing Verifier...")
+	cert, err := proxy.NewVerifier(chainID, home, node, logger)
 	if err != nil {
-		return cmn.ErrorWrap(err, "constructing certifier")
+		return cmn.ErrorWrap(err, "constructing Verifier")
 	}
 	cert.SetLogger(logger)
 	sc := proxy.SecureClient(node, cert)

lite/client/provider.go

@@ -8,12 +8,12 @@ package client
 import (
 	"fmt"
 
+	log "github.com/tendermint/tendermint/libs/log"
 	"github.com/tendermint/tendermint/lite"
 	lerr "github.com/tendermint/tendermint/lite/errors"
 	rpcclient "github.com/tendermint/tendermint/rpc/client"
 	ctypes "github.com/tendermint/tendermint/rpc/core/types"
 	"github.com/tendermint/tendermint/types"
-	log "github.com/tendermint/tendermint/libs/log"
 )
 
 // SignStatusClient combines a SignClient and StatusClient.
@@ -106,12 +106,10 @@ func (p *provider) getValidatorSet(chainID string, height int64) (valset *types.
 		err = fmt.Errorf("expected height >= 1, got height %v", height)
 		return
 	}
-	heightPtr := new(int64)
-	*heightPtr = height
-	res, err := p.client.Validators(heightPtr)
+	res, err := p.client.Validators(&height)
 	if err != nil {
 		// TODO pass through other types of errors.
-		return nil, lerr.ErrMissingValidators(chainID, height)
+		return nil, lerr.ErrUnknownValidators(chainID, height)
 	}
 	valset = types.NewValidatorSet(res.Validators)
 	return

lite/client/provider_test.go

@@ -13,7 +13,6 @@ import (
 	"github.com/tendermint/tendermint/types"
 )
 
-// TODO fix tests!!
 func TestMain(m *testing.M) {
 	app := kvstore.NewKVStoreApplication()
 	node := rpctest.StartTendermint(app)
@@ -59,15 +58,4 @@ func TestProvider(t *testing.T) {
 	assert.Nil(err, "%+v", err)
 	assert.Equal(lower, fc.Height())
 
-	/*
-		// also get by hash (given the match)
-		fc, err = p.GetByHash(vhash)
-		require.Nil(err, "%+v", err)
-		require.Equal(vhash, fc.Header.ValidatorsHash)
-
-		// get by hash fails without match
-		fc, err = p.GetByHash([]byte("foobar"))
-		assert.NotNil(err)
-		assert.True(liteErr.IsCommitNotFoundErr(err))
-	*/
 }

lite/commit.go

@@ -11,7 +11,7 @@ import (
 // FullCommit is a signed header (the block header and a commit that signs it),
 // the validator set which signed the commit, and the next validator set. The
 // next validator set (which is proven from the block header) allows us to
-// revert to block-by-block updating of lite certifier's latest validator set,
+// revert to block-by-block updating of lite Verifier's latest validator set,
 // even in the face of arbitrarily large power changes.
 type FullCommit struct {
 	SignedHeader   types.SignedHeader  `json:"signed_header"`

lite/dbprovider.go

@@ -22,7 +22,10 @@ type DBProvider struct {
 }
 
 func NewDBProvider(label string, db dbm.DB) *DBProvider {
+
+	// NOTE: when debugging, this type of construction might be useful.
 	//db = dbm.NewDebugDB("db provider "+cmn.RandStr(4), db)
+
 	cdc := amino.NewCodec()
 	cryptoAmino.RegisterAmino(cdc)
 	dbp := &DBProvider{
@@ -127,8 +130,8 @@ func (dbp *DBProvider) LatestFullCommit(chainID string, minHeight, maxHeight int
 					dbp.logger.Info("DBProvider.LatestFullCommit() found latest.", "height", lfc.Height())
 					return lfc, nil
 				} else {
-					dbp.logger.Info("DBProvider.LatestFullCommit() got error", "lfc", lfc)
-					dbp.logger.Info(fmt.Sprintf("%+v", err))
+					dbp.logger.Error("DBProvider.LatestFullCommit() got error", "lfc", lfc)
+					dbp.logger.Error(fmt.Sprintf("%+v", err))
 					return lfc, err
 				}
 			}
@@ -144,14 +147,19 @@ func (dbp *DBProvider) ValidatorSet(chainID string, height int64) (valset *types
 func (dbp *DBProvider) getValidatorSet(chainID string, height int64) (valset *types.ValidatorSet, err error) {
 	vsBz := dbp.db.Get(validatorSetKey(chainID, height))
 	if vsBz == nil {
-		err = lerr.ErrMissingValidators(chainID, height)
+		err = lerr.ErrUnknownValidators(chainID, height)
 		return
 	}
 	err = dbp.cdc.UnmarshalBinary(vsBz, &valset)
 	if err != nil {
 		return
 	}
-	valset.TotalVotingPower() // to test deep equality.
+
+	// To test deep equality.  This makes it easier to test for e.g. valset
+	// equivalence using assert.Equal (tests for deep equality) in our tests,
+	// which also tests for unexported/private field equivalence.
+	valset.TotalVotingPower()
+
 	return
 }
 
@@ -209,52 +217,52 @@ func (dbp *DBProvider) deleteAfterN(chainID string, after int) error {
 		itr.Next()
 	}
 
-	dbp.logger.Info(fmt.Sprintf("DBProvider.deleteAfterN() deleted %v items\n", numDeleted))
+	dbp.logger.Info(fmt.Sprintf("DBProvider.deleteAfterN() deleted %v items", numDeleted))
 	return nil
 }
 
 //----------------------------------------
+// key encoding
 
 func signedHeaderKey(chainID string, height int64) []byte {
 	return []byte(fmt.Sprintf("%s/%010d/sh", chainID, height))
 }
 
-var signedHeaderKeyPattern = regexp.MustCompile(`([^/]+)/([0-9]*)/sh`)
+func validatorSetKey(chainID string, height int64) []byte {
+	return []byte(fmt.Sprintf("%s/%010d/vs", chainID, height))
+}
 
-func parseSignedHeaderKey(key []byte) (chainID string, height int64, ok bool) {
-	submatch := signedHeaderKeyPattern.FindSubmatch(key)
+//----------------------------------------
+// key parsing
+
+var keyPattern = regexp.MustCompile(`^([^/]+)/([0-9]*)/(.*)$`)
+
+func parseKey(key []byte) (chainID string, height int64, part string, ok bool) {
+	submatch := keyPattern.FindSubmatch(key)
 	if submatch == nil {
-		return "", 0, false
+		return "", 0, "", false
 	}
 	chainID = string(submatch[1])
 	heightStr := string(submatch[2])
 	heightInt, err := strconv.Atoi(heightStr)
 	if err != nil {
-		return "", 0, false
+		return "", 0, "", false
 	}
 	height = int64(heightInt)
+	part = string(submatch[3])
 	ok = true // good!
 	return
 }
 
-func validatorSetKey(chainID string, height int64) []byte {
-	return []byte(fmt.Sprintf("%s/%010d/vs", chainID, height))
+func parseSignedHeaderKey(key []byte) (chainID string, height int64, ok bool) {
+	chainID, height, part, ok := parseKey(key)
+	if part != "sh" {
+		return "", 0, false
+	}
+	return chainID, height, true
 }
 
-var chainKeyPrefixPattern = regexp.MustCompile(`([^/]+)/([0-9]*)/`)
-
 func parseChainKeyPrefix(key []byte) (chainID string, height int64, ok bool) {
-	submatch := chainKeyPrefixPattern.FindSubmatch(key)
-	if submatch == nil {
-		return "", 0, false
-	}
-	chainID = string(submatch[1])
-	heightStr := string(submatch[2])
-	heightInt, err := strconv.Atoi(heightStr)
-	if err != nil {
-		return "", 0, false
-	}
-	height = int64(heightInt)
-	ok = true // good!
-	return
+	chainID, height, _, ok = parseKey(key)
+	return chainID, height, true
 }

lite/doc.go

@@ -35,29 +35,29 @@ change on the chain. In practice, most applications will not have frequent
 drastic updates to the validator set, so the logic defined in this package for
 lite client syncing is optimized to use intelligent bisection and
 block-skipping for efficient sourcing and verification of these data structures
-and updates to the validator set (see the InquiringCertifier for more
+and updates to the validator set (see the DynamicVerifier for more
 information).
 
 The FullCommit is also declared in this package as a convenience structure,
 which includes the SignedHeader along with the full current and next
 ValidatorSets.
 
-## Certifier
+## Verifier
 
-A Certifier validates a new SignedHeader given the currently known state. There
-are two different types of Certifiers provided.
+A Verifier validates a new SignedHeader given the currently known state. There
+are two different types of Verifiers provided.
 
-BaseCertifier - given a validator set and a height, this Certifier verifies
+BaseVerifier - given a validator set and a height, this Verifier verifies
 that > 2/3 of the voting power of the given validator set had signed the
 SignedHeader, and that the SignedHeader was to be signed by the exact given
 validator set, and that the height of the commit is at least height (or
 greater).
 
 SignedHeader.Commit may be signed by a different validator set, it can get
-certified with a BaseCertifier as long as sufficient signatures from the
+certified with a BaseVerifier as long as sufficient signatures from the
 previous validator set are present in the commit.
 
-InquiringCertifier - this certifier implements an auto-update and persistence
+DynamicVerifier - this Verifier implements an auto-update and persistence
 strategy to certify any SignedHeader of the blockchain.
 
 ## Provider and PersistentProvider
@@ -77,7 +77,7 @@ type Provider interface {
 * client.NewHTTPProvider - query Tendermint rpc.
 
 A PersistentProvider is a Provider that also allows for saving state.  This is
-used by the InquiringCertifier for persistence.
+used by the DynamicVerifier for persistence.
 
 ```go
 type PersistentProvider interface {
@@ -131,7 +131,7 @@ important to verify that you have the proper validator set when initializing
 the client, as that is the root of all trust.
 
 The software currently assumes that the unbonding period is infinite in
-duration.  If the InquiringCertifier hasn't been updated in a while, you should
+duration.  If the DynamicVerifier hasn't been updated in a while, you should
 manually verify the block headers using other sources.
 
 TODO: Update the software to handle cases around the unbonding period.

lite/errors/errors.go

@@ -31,12 +31,12 @@ func (e errTooMuchChange) Error() string {
 	return "Insufficient signatures to validate due to valset changes"
 }
 
-type errMissingValidators struct {
+type errUnknownValidators struct {
 	chainID string
 	height  int64
 }
 
-func (e errMissingValidators) Error() string {
+func (e errUnknownValidators) Error() string {
 	return fmt.Sprintf("Validators are unknown or missing for chain %s and height %d",
 		e.chainID, e.height)
 }
@@ -96,16 +96,16 @@ func IsErrTooMuchChange(err error) bool {
 }
 
 //-----------------
-// ErrMissingValidators
+// ErrUnknownValidators
 
-// ErrMissingValidators indicates that some validator set was missing or unknown.
-func ErrMissingValidators(chainID string, height int64) error {
-	return cmn.ErrorWrap(errMissingValidators{chainID, height}, "")
+// ErrUnknownValidators indicates that some validator set was missing or unknown.
+func ErrUnknownValidators(chainID string, height int64) error {
+	return cmn.ErrorWrap(errUnknownValidators{chainID, height}, "")
 }
 
-func IsErrMissingValidators(err error) bool {
+func IsErrUnknownValidators(err error) bool {
 	if err_, ok := err.(cmn.Error); ok {
-		_, ok := err_.Data().(errMissingValidators)
+		_, ok := err_.Data().(errUnknownValidators)
 		return ok
 	}
 	return false

lite/multiprovider.go

@@ -1,9 +1,9 @@
 package lite
 
 import (
+	log "github.com/tendermint/tendermint/libs/log"
 	lerr "github.com/tendermint/tendermint/lite/errors"
 	"github.com/tendermint/tendermint/types"
-	log "github.com/tendermint/tendermint/libs/log"
 )
 
 // multiProvider allows you to place one or more caches in front of a source
@@ -79,5 +79,5 @@ func (mc *multiProvider) ValidatorSet(chainID string, height int64) (valset *typ
 			return valset, nil
 		}
 	}
-	return nil, lerr.ErrMissingValidators(chainID, height)
+	return nil, lerr.ErrUnknownValidators(chainID, height)
 }

lite/proxy/query.go

@@ -28,13 +28,13 @@ type KeyProof interface {
 }
 
 // GetWithProof will query the key on the given node, and verify it has
-// a valid proof, as defined by the certifier.
+// a valid proof, as defined by the Verifier.
 //
 // If there is any error in checking, returns an error.
 // If val is non-empty, proof should be KeyExistsProof
 // If val is empty, proof should be KeyMissingProof
 func GetWithProof(key []byte, reqHeight int64, node rpcclient.Client,
-	cert lite.Certifier) (
+	cert lite.Verifier) (
 	val cmn.HexBytes, height int64, proof KeyProof, err error) {
 
 	if reqHeight < 0 {
@@ -54,7 +54,7 @@ func GetWithProof(key []byte, reqHeight int64, node rpcclient.Client,
 
 // GetWithProofOptions is useful if you want full access to the ABCIQueryOptions
 func GetWithProofOptions(path string, key []byte, opts rpcclient.ABCIQueryOptions,
-	node rpcclient.Client, cert lite.Certifier) (
+	node rpcclient.Client, cert lite.Verifier) (
 	*ctypes.ResultABCIQuery, KeyProof, error) {
 
 	_resp, err := node.ABCIQueryWithOptions(path, key, opts)
@@ -128,7 +128,7 @@ func GetWithProofOptions(path string, key []byte, opts rpcclient.ABCIQueryOption
 
 // GetCertifiedCommit gets the signed header for a given height and certifies
 // it. Returns error if unable to get a proven header.
-func GetCertifiedCommit(h int64, client rpcclient.Client, cert lite.Certifier) (types.SignedHeader, error) {
+func GetCertifiedCommit(h int64, client rpcclient.Client, cert lite.Verifier) (types.SignedHeader, error) {
 
 	// FIXME: cannot use cert.GetByHeight for now, as it also requires
 	// Validators and will fail on querying tendermint for non-current height.

lite/proxy/query_test.go

@@ -58,7 +58,7 @@ func _TestAppProofs(t *testing.T) {
 	source := certclient.NewProvider(chainID, cl)
 	seed, err := source.LatestFullCommit(chainID, brh-2, brh-2)
 	require.NoError(err, "%+v", err)
-	cert := lite.NewBaseCertifier("my-chain", seed.Height(), seed.Validators)
+	cert := lite.NewBaseVerifier("my-chain", seed.Height(), seed.Validators)
 
 	client.WaitForHeight(cl, 3, nil)
 	latest, err := source.LatestFullCommit(chainID, 1, 1<<63-1)
@@ -117,7 +117,7 @@ func _TestTxProofs(t *testing.T) {
 	source := certclient.NewProvider(chainID, cl)
 	seed, err := source.LatestFullCommit(chainID, brh-2, brh-2)
 	require.NoError(err, "%+v", err)
-	cert := lite.NewBaseCertifier("my-chain", seed.Height(), seed.Validators)
+	cert := lite.NewBaseVerifier("my-chain", seed.Height(), seed.Validators)
 
 	// First let's make sure a bogus transaction hash returns a valid non-existence proof.
 	key := types.Tx([]byte("bogus")).Hash()

lite/proxy/wrapper.go

@@ -10,18 +10,18 @@ import (
 
 var _ rpcclient.Client = Wrapper{}
 
-// Wrapper wraps a rpcclient with a Certifier and double-checks any input that is
+// Wrapper wraps a rpcclient with a Verifier and double-checks any input that is
 // provable before passing it along. Allows you to make any rpcclient fully secure.
 type Wrapper struct {
 	rpcclient.Client
-	cert *lite.InquiringCertifier
+	cert *lite.DynamicVerifier
 }
 
-// SecureClient uses a given certifier to wrap an connection to an untrusted
+// SecureClient uses a given Verifier to wrap an connection to an untrusted
 // host and return a cryptographically secure rpc client.
 //
 // If it is wrapping an HTTP rpcclient, it will also wrap the websocket interface
-func SecureClient(c rpcclient.Client, cert *lite.InquiringCertifier) Wrapper {
+func SecureClient(c rpcclient.Client, cert *lite.DynamicVerifier) Wrapper {
 	wrap := Wrapper{c, cert}
 	// TODO: no longer possible as no more such interface exposed....
 	// if we wrap http client, then we can swap out the event switch to filter

lite/types.go

@@ -4,10 +4,10 @@ import (
 	"github.com/tendermint/tendermint/types"
 )
 
-// Certifier checks the votes to make sure the block really is signed properly.
-// Certifier must know the current or recent set of validitors by some other
+// Verifier checks the votes to make sure the block really is signed properly.
+// Verifier must know the current or recent set of validitors by some other
 // means.
-type Certifier interface {
+type Verifier interface {
 	Certify(sheader types.SignedHeader) error
 	ChainID() string
 }

types/block.go

@@ -446,7 +446,7 @@ type SignedHeader struct {
 // and commit are consistent.
 //
 // NOTE: This does not actually check the cryptographic signatures.  Make
-// sure to use a Certifier to validate the signatures actually provide a
+// sure to use a Verifier to validate the signatures actually provide a
 // significantly strong proof for this header's validity.
 func (sh SignedHeader) ValidateBasic(chainID string) error {
 

types/validator_set.go

@@ -48,6 +48,11 @@ func NewValidatorSet(valz []*Validator) *ValidatorSet {
 	return vals
 }
 
+// Nil or empty validator sets are invalid.
+func (vals *ValidatorSet) IsNilOrEmpty() bool {
+	return vals == nil || len(vals.Validators) == 0
+}
+
 // Increment Accum and update the proposer on a copy, and return it.
 func (vals *ValidatorSet) CopyIncrementAccum(times int) *ValidatorSet {
 	copy := vals.Copy()