diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 392b61da..af1c5566 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -22,7 +22,7 @@ Special thanks to external contributors on this release: BlockSize.MaxGas * P2P Protocol - - [consensus] \#2960 normalize priorities to not exceed `2*TotalVotingPower` to mitigate unfair proposer selection + - [consensus] \#2960 normalize priorities to not exceed `2*TotalVotingPower` to mitigate unfair proposer selection heavily preferring earlier joined validators in the case of an early bonded large validator unbonding ### FEATURES: @@ -32,5 +32,7 @@ Special thanks to external contributors on this release: - [instrumentation] \#3082 Add `chain_id` label for all metrics ### BUG FIXES: -- [log] \#3060 Fix year format - [crypto] \#3164 Update `btcd` fork for rare signRFC6979 bug +- [lite] \#3171 Fix verifying large validator set changes +- [log] \#3060 Fix year format +- [mempool] \#3168 Limit tx size to fit in the max reactor msg size diff --git a/lite/base_verifier.go b/lite/base_verifier.go index fcde01c0..9eb880bb 100644 --- a/lite/base_verifier.go +++ b/lite/base_verifier.go @@ -35,34 +35,40 @@ func NewBaseVerifier(chainID string, height int64, valset *types.ValidatorSet) * } // Implements Verifier. -func (bc *BaseVerifier) ChainID() string { - return bc.chainID +func (bv *BaseVerifier) ChainID() string { + return bv.chainID } // Implements Verifier. -func (bc *BaseVerifier) Verify(signedHeader types.SignedHeader) error { +func (bv *BaseVerifier) Verify(signedHeader types.SignedHeader) error { - // We can't verify commits older than bc.height. - if signedHeader.Height < bc.height { + // We can't verify commits for a different chain. + if signedHeader.ChainID != bv.chainID { + return cmn.NewError("BaseVerifier chainID is %v, cannot verify chainID %v", + bv.chainID, signedHeader.ChainID) + } + + // We can't verify commits older than bv.height. + if signedHeader.Height < bv.height { return cmn.NewError("BaseVerifier height is %v, cannot verify height %v", - bc.height, signedHeader.Height) + bv.height, signedHeader.Height) } // We can't verify with the wrong validator set. if !bytes.Equal(signedHeader.ValidatorsHash, - bc.valset.Hash()) { - return lerr.ErrUnexpectedValidators(signedHeader.ValidatorsHash, bc.valset.Hash()) + bv.valset.Hash()) { + return lerr.ErrUnexpectedValidators(signedHeader.ValidatorsHash, bv.valset.Hash()) } // Do basic sanity checks. - err := signedHeader.ValidateBasic(bc.chainID) + err := signedHeader.ValidateBasic(bv.chainID) if err != nil { return cmn.ErrorWrap(err, "in verify") } // Check commit signatures. - err = bc.valset.VerifyCommit( - bc.chainID, signedHeader.Commit.BlockID, + err = bv.valset.VerifyCommit( + bv.chainID, signedHeader.Commit.BlockID, signedHeader.Height, signedHeader.Commit) if err != nil { return cmn.ErrorWrap(err, "in verify") diff --git a/lite/commit.go b/lite/commit.go index 25efb8dc..6cd35417 100644 --- a/lite/commit.go +++ b/lite/commit.go @@ -8,7 +8,7 @@ import ( "github.com/tendermint/tendermint/types" ) -// FullCommit is a signed header (the block header and a commit that signs it), +// FullCommit contains a SignedHeader (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 Verifier's latest validator set, diff --git a/lite/dbprovider.go b/lite/dbprovider.go index 9f4b264f..ef1b2a59 100644 --- a/lite/dbprovider.go +++ b/lite/dbprovider.go @@ -13,6 +13,9 @@ import ( "github.com/tendermint/tendermint/types" ) +var _ PersistentProvider = (*DBProvider)(nil) + +// DBProvider stores commits and validator sets in a DB. type DBProvider struct { logger log.Logger label string diff --git a/lite/doc.go b/lite/doc.go index f68798dc..429b096e 100644 --- a/lite/doc.go +++ b/lite/doc.go @@ -53,10 +53,6 @@ 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 -verified with a BaseVerifier as long as sufficient signatures from the -previous validator set are present in the commit. - DynamicVerifier - this Verifier implements an auto-update and persistence strategy to verify any SignedHeader of the blockchain. diff --git a/lite/dynamic_verifier.go b/lite/dynamic_verifier.go index 6a772091..8b69d2d7 100644 --- a/lite/dynamic_verifier.go +++ b/lite/dynamic_verifier.go @@ -18,12 +18,17 @@ var _ Verifier = (*DynamicVerifier)(nil) // "source" provider to obtain the needed FullCommits to securely sync with // validator set changes. It stores properly validated data on the // "trusted" local system. +// TODO: make this single threaded and create a new +// ConcurrentDynamicVerifier that wraps it with concurrency. +// see https://github.com/tendermint/tendermint/issues/3170 type DynamicVerifier struct { - logger log.Logger chainID string - // These are only properly validated data, from local system. + logger log.Logger + + // Already validated, stored locally trusted PersistentProvider - // This is a source of new info, like a node rpc, or other import method. + + // New info, like a node rpc, or other import method. source Provider // pending map to synchronize concurrent verification requests @@ -35,8 +40,8 @@ type DynamicVerifier struct { // trusted provider to store validated data and the source provider to // obtain missing data (e.g. FullCommits). // -// The trusted provider should a CacheProvider, MemProvider or -// files.Provider. The source provider should be a client.HTTPProvider. +// The trusted provider should be a DBProvider. +// The source provider should be a client.HTTPProvider. func NewDynamicVerifier(chainID string, trusted PersistentProvider, source Provider) *DynamicVerifier { return &DynamicVerifier{ logger: log.NewNopLogger(), @@ -47,68 +52,71 @@ func NewDynamicVerifier(chainID string, trusted PersistentProvider, source Provi } } -func (ic *DynamicVerifier) SetLogger(logger log.Logger) { +func (dv *DynamicVerifier) SetLogger(logger log.Logger) { logger = logger.With("module", "lite") - ic.logger = logger - ic.trusted.SetLogger(logger) - ic.source.SetLogger(logger) + dv.logger = logger + dv.trusted.SetLogger(logger) + dv.source.SetLogger(logger) } // Implements Verifier. -func (ic *DynamicVerifier) ChainID() string { - return ic.chainID +func (dv *DynamicVerifier) ChainID() string { + return dv.chainID } // Implements Verifier. // // If the validators have changed since the last known time, it looks to -// ic.trusted and ic.source to prove the new validators. On success, it will -// try to store the SignedHeader in ic.trusted if the next +// dv.trusted and dv.source to prove the new validators. On success, it will +// try to store the SignedHeader in dv.trusted if the next // validator can be sourced. -func (ic *DynamicVerifier) Verify(shdr types.SignedHeader) error { +func (dv *DynamicVerifier) Verify(shdr types.SignedHeader) error { // Performs synchronization for multi-threads verification at the same height. - ic.mtx.Lock() - if pending := ic.pendingVerifications[shdr.Height]; pending != nil { - ic.mtx.Unlock() + dv.mtx.Lock() + if pending := dv.pendingVerifications[shdr.Height]; pending != nil { + dv.mtx.Unlock() <-pending // pending is chan struct{} } else { pending := make(chan struct{}) - ic.pendingVerifications[shdr.Height] = pending + dv.pendingVerifications[shdr.Height] = pending defer func() { close(pending) - ic.mtx.Lock() - delete(ic.pendingVerifications, shdr.Height) - ic.mtx.Unlock() + dv.mtx.Lock() + delete(dv.pendingVerifications, shdr.Height) + dv.mtx.Unlock() }() - ic.mtx.Unlock() + dv.mtx.Unlock() } + //Get the exact trusted commit for h, and if it is - // equal to shdr, then don't even verify it, - // and just return nil. - trustedFCSameHeight, err := ic.trusted.LatestFullCommit(ic.chainID, shdr.Height, shdr.Height) + // equal to shdr, then it's already trusted, so + // just return nil. + trustedFCSameHeight, err := dv.trusted.LatestFullCommit(dv.chainID, shdr.Height, shdr.Height) if err == nil { // If loading trust commit successfully, and trust commit equal to shdr, then don't verify it, // just return nil. if bytes.Equal(trustedFCSameHeight.SignedHeader.Hash(), shdr.Hash()) { - ic.logger.Info(fmt.Sprintf("Load full commit at height %d from cache, there is not need to verify.", shdr.Height)) + dv.logger.Info(fmt.Sprintf("Load full commit at height %d from cache, there is not need to verify.", shdr.Height)) return nil } } else if !lerr.IsErrCommitNotFound(err) { // Return error if it is not CommitNotFound error - ic.logger.Info(fmt.Sprintf("Encountered unknown error in loading full commit at height %d.", shdr.Height)) + dv.logger.Info(fmt.Sprintf("Encountered unknown error in loading full commit at height %d.", shdr.Height)) return err } // Get the latest known full commit <= h-1 from our trusted providers. // The full commit at h-1 contains the valset to sign for h. - h := shdr.Height - 1 - trustedFC, err := ic.trusted.LatestFullCommit(ic.chainID, 1, h) + prevHeight := shdr.Height - 1 + trustedFC, err := dv.trusted.LatestFullCommit(dv.chainID, 1, prevHeight) if err != nil { return err } - if trustedFC.Height() == h { + // sync up to the prevHeight and assert our latest NextValidatorSet + // is the ValidatorSet for the SignedHeader + if trustedFC.Height() == prevHeight { // Return error if valset doesn't match. if !bytes.Equal( trustedFC.NextValidators.Hash(), @@ -118,11 +126,12 @@ func (ic *DynamicVerifier) Verify(shdr types.SignedHeader) error { shdr.Header.ValidatorsHash) } } else { - // If valset doesn't match... - if !bytes.Equal(trustedFC.NextValidators.Hash(), + // If valset doesn't match, try to update + if !bytes.Equal( + trustedFC.NextValidators.Hash(), shdr.Header.ValidatorsHash) { // ... update. - trustedFC, err = ic.updateToHeight(h) + trustedFC, err = dv.updateToHeight(prevHeight) if err != nil { return err } @@ -137,14 +146,21 @@ func (ic *DynamicVerifier) Verify(shdr types.SignedHeader) error { } // Verify the signed header using the matching valset. - cert := NewBaseVerifier(ic.chainID, trustedFC.Height()+1, trustedFC.NextValidators) + cert := NewBaseVerifier(dv.chainID, trustedFC.Height()+1, trustedFC.NextValidators) err = cert.Verify(shdr) if err != nil { return err } + // By now, the SignedHeader is fully validated and we're synced up to + // SignedHeader.Height - 1. To sync to SignedHeader.Height, we need + // the validator set at SignedHeader.Height + 1 so we can verify the + // SignedHeader.NextValidatorSet. + // TODO: is the ValidateFull below mostly redundant with the BaseVerifier.Verify above? + // See https://github.com/tendermint/tendermint/issues/3174. + // Get the next validator set. - nextValset, err := ic.source.ValidatorSet(ic.chainID, shdr.Height+1) + nextValset, err := dv.source.ValidatorSet(dv.chainID, shdr.Height+1) if lerr.IsErrUnknownValidators(err) { // Ignore this error. return nil @@ -160,31 +176,31 @@ func (ic *DynamicVerifier) Verify(shdr types.SignedHeader) error { } // Validate the full commit. This checks the cryptographic // signatures of Commit against Validators. - if err := nfc.ValidateFull(ic.chainID); err != nil { + if err := nfc.ValidateFull(dv.chainID); err != nil { return err } // Trust it. - return ic.trusted.SaveFullCommit(nfc) + return dv.trusted.SaveFullCommit(nfc) } // verifyAndSave will verify if this is a valid source full commit given the -// best match trusted full commit, and if good, persist to ic.trusted. +// best match trusted full commit, and if good, persist to dv.trusted. // Returns ErrTooMuchChange when >2/3 of trustedFC did not sign sourceFC. // Panics if trustedFC.Height() >= sourceFC.Height(). -func (ic *DynamicVerifier) verifyAndSave(trustedFC, sourceFC FullCommit) error { +func (dv *DynamicVerifier) verifyAndSave(trustedFC, sourceFC FullCommit) error { if trustedFC.Height() >= sourceFC.Height() { panic("should not happen") } err := trustedFC.NextValidators.VerifyFutureCommit( sourceFC.Validators, - ic.chainID, sourceFC.SignedHeader.Commit.BlockID, + dv.chainID, sourceFC.SignedHeader.Commit.BlockID, sourceFC.SignedHeader.Height, sourceFC.SignedHeader.Commit, ) if err != nil { return err } - return ic.trusted.SaveFullCommit(sourceFC) + return dv.trusted.SaveFullCommit(sourceFC) } // updateToHeight will use divide-and-conquer to find a path to h. @@ -192,29 +208,30 @@ func (ic *DynamicVerifier) verifyAndSave(trustedFC, sourceFC FullCommit) error { // for height h, using repeated applications of bisection if necessary. // // Returns ErrCommitNotFound if source provider doesn't have the commit for h. -func (ic *DynamicVerifier) updateToHeight(h int64) (FullCommit, error) { +func (dv *DynamicVerifier) updateToHeight(h int64) (FullCommit, error) { // Fetch latest full commit from source. - sourceFC, err := ic.source.LatestFullCommit(ic.chainID, h, h) + sourceFC, err := dv.source.LatestFullCommit(dv.chainID, h, h) if err != nil { return FullCommit{}, err } - // Validate the full commit. This checks the cryptographic - // signatures of Commit against Validators. - if err := sourceFC.ValidateFull(ic.chainID); err != nil { - return FullCommit{}, err - } - // If sourceFC.Height() != h, we can't do it. if sourceFC.Height() != h { return FullCommit{}, lerr.ErrCommitNotFound() } + // Validate the full commit. This checks the cryptographic + // signatures of Commit against Validators. + if err := sourceFC.ValidateFull(dv.chainID); err != nil { + return FullCommit{}, err + } + + // Verify latest FullCommit against trusted FullCommits FOR_LOOP: for { // Fetch latest full commit from trusted. - trustedFC, err := ic.trusted.LatestFullCommit(ic.chainID, 1, h) + trustedFC, err := dv.trusted.LatestFullCommit(dv.chainID, 1, h) if err != nil { return FullCommit{}, err } @@ -224,21 +241,21 @@ FOR_LOOP: } // Try to update to full commit with checks. - err = ic.verifyAndSave(trustedFC, sourceFC) + err = dv.verifyAndSave(trustedFC, sourceFC) if err == nil { // All good! return sourceFC, nil } // Handle special case when err is ErrTooMuchChange. - if lerr.IsErrTooMuchChange(err) { + if types.IsErrTooMuchChange(err) { // Divide and conquer. start, end := trustedFC.Height(), sourceFC.Height() if !(start < end) { panic("should not happen") } mid := (start + end) / 2 - _, err = ic.updateToHeight(mid) + _, err = dv.updateToHeight(mid) if err != nil { return FullCommit{}, err } @@ -249,8 +266,8 @@ FOR_LOOP: } } -func (ic *DynamicVerifier) LastTrustedHeight() int64 { - fc, err := ic.trusted.LatestFullCommit(ic.chainID, 1, 1<<63-1) +func (dv *DynamicVerifier) LastTrustedHeight() int64 { + fc, err := dv.trusted.LatestFullCommit(dv.chainID, 1, 1<<63-1) if err != nil { panic("should not happen") } diff --git a/lite/dynamic_verifier_test.go b/lite/dynamic_verifier_test.go index 9ff8ed81..386de513 100644 --- a/lite/dynamic_verifier_test.go +++ b/lite/dynamic_verifier_test.go @@ -10,6 +10,7 @@ import ( dbm "github.com/tendermint/tendermint/libs/db" log "github.com/tendermint/tendermint/libs/log" + "github.com/tendermint/tendermint/types" ) func TestInquirerValidPath(t *testing.T) { @@ -70,6 +71,70 @@ func TestInquirerValidPath(t *testing.T) { assert.Nil(err, "%+v", err) } +func TestDynamicVerify(t *testing.T) { + trust := NewDBProvider("trust", dbm.NewMemDB()) + source := NewDBProvider("source", dbm.NewMemDB()) + + // 10 commits with one valset, 1 to change, + // 10 commits with the next one + n1, n2 := 10, 10 + nCommits := n1 + n2 + 1 + maxHeight := int64(nCommits) + fcz := make([]FullCommit, nCommits) + + // gen the 2 val sets + chainID := "dynamic-verifier" + power := int64(10) + keys1 := genPrivKeys(5) + vals1 := keys1.ToValidators(power, 0) + keys2 := genPrivKeys(5) + vals2 := keys2.ToValidators(power, 0) + + // make some commits with the first + for i := 0; i < n1; i++ { + fcz[i] = makeFullCommit(int64(i), keys1, vals1, vals1, chainID) + } + + // update the val set + fcz[n1] = makeFullCommit(int64(n1), keys1, vals1, vals2, chainID) + + // make some commits with the new one + for i := n1 + 1; i < nCommits; i++ { + fcz[i] = makeFullCommit(int64(i), keys2, vals2, vals2, chainID) + } + + // Save everything in the source + for _, fc := range fcz { + source.SaveFullCommit(fc) + } + + // Initialize a Verifier with the initial state. + err := trust.SaveFullCommit(fcz[0]) + require.Nil(t, err) + ver := NewDynamicVerifier(chainID, trust, source) + ver.SetLogger(log.TestingLogger()) + + // fetch the latest from the source + latestFC, err := source.LatestFullCommit(chainID, 1, maxHeight) + require.NoError(t, err) + + // try to update to the latest + err = ver.Verify(latestFC.SignedHeader) + require.NoError(t, err) + +} + +func makeFullCommit(height int64, keys privKeys, vals, nextVals *types.ValidatorSet, chainID string) FullCommit { + height += 1 + consHash := []byte("special-params") + appHash := []byte(fmt.Sprintf("h=%d", height)) + resHash := []byte(fmt.Sprintf("res=%d", height)) + return keys.GenFullCommit( + chainID, height, nil, + vals, nextVals, + appHash, consHash, resHash, 0, len(keys)) +} + func TestInquirerVerifyHistorical(t *testing.T) { assert, require := assert.New(t), require.New(t) trust := NewDBProvider("trust", dbm.NewMemDB()) diff --git a/lite/errors/errors.go b/lite/errors/errors.go index 59b6380d..75442c72 100644 --- a/lite/errors/errors.go +++ b/lite/errors/errors.go @@ -25,12 +25,6 @@ func (e errUnexpectedValidators) Error() string { e.got, e.want) } -type errTooMuchChange struct{} - -func (e errTooMuchChange) Error() string { - return "Insufficient signatures to validate due to valset changes" -} - type errUnknownValidators struct { chainID string height int64 @@ -85,22 +79,6 @@ func IsErrUnexpectedValidators(err error) bool { return false } -//----------------- -// ErrTooMuchChange - -// ErrTooMuchChange indicates that the underlying validator set was changed by >1/3. -func ErrTooMuchChange() error { - return cmn.ErrorWrap(errTooMuchChange{}, "") -} - -func IsErrTooMuchChange(err error) bool { - if err_, ok := err.(cmn.Error); ok { - _, ok := err_.Data().(errTooMuchChange) - return ok - } - return false -} - //----------------- // ErrUnknownValidators diff --git a/lite/multiprovider.go b/lite/multiprovider.go index 734d042c..a05e19b1 100644 --- a/lite/multiprovider.go +++ b/lite/multiprovider.go @@ -6,6 +6,8 @@ import ( "github.com/tendermint/tendermint/types" ) +var _ PersistentProvider = (*multiProvider)(nil) + // multiProvider allows you to place one or more caches in front of a source // Provider. It runs through them in order until a match is found. type multiProvider struct { diff --git a/lite/provider.go b/lite/provider.go index 97e06a06..ebab1626 100644 --- a/lite/provider.go +++ b/lite/provider.go @@ -1,7 +1,7 @@ package lite import ( - log "github.com/tendermint/tendermint/libs/log" + "github.com/tendermint/tendermint/libs/log" "github.com/tendermint/tendermint/types" ) diff --git a/types/block.go b/types/block.go index 5872a680..99ee3f8e 100644 --- a/types/block.go +++ b/types/block.go @@ -638,6 +638,7 @@ func (commit *Commit) StringIndented(indent string) string { //----------------------------------------------------------------------------- // SignedHeader is a header along with the commits that prove it. +// It is the basis of the lite client. type SignedHeader struct { *Header `json:"header"` Commit *Commit `json:"commit"` diff --git a/types/validator_set.go b/types/validator_set.go index 4040810f..38b9260a 100644 --- a/types/validator_set.go +++ b/types/validator_set.go @@ -413,8 +413,7 @@ func (vals *ValidatorSet) VerifyCommit(chainID string, blockID BlockID, height i if talliedVotingPower > vals.TotalVotingPower()*2/3 { return nil } - return fmt.Errorf("Invalid commit -- insufficient voting power: got %v, needed %v", - talliedVotingPower, vals.TotalVotingPower()*2/3+1) + return errTooMuchChange{talliedVotingPower, vals.TotalVotingPower()*2/3 + 1} } // VerifyFutureCommit will check to see if the set would be valid with a different @@ -496,12 +495,37 @@ func (vals *ValidatorSet) VerifyFutureCommit(newSet *ValidatorSet, chainID strin } if oldVotingPower <= oldVals.TotalVotingPower()*2/3 { - return cmn.NewError("Invalid commit -- insufficient old voting power: got %v, needed %v", - oldVotingPower, oldVals.TotalVotingPower()*2/3+1) + return errTooMuchChange{oldVotingPower, oldVals.TotalVotingPower()*2/3 + 1} } return nil } +//----------------- +// ErrTooMuchChange + +func IsErrTooMuchChange(err error) bool { + switch err_ := err.(type) { + case cmn.Error: + _, ok := err_.Data().(errTooMuchChange) + return ok + case errTooMuchChange: + return true + default: + return false + } +} + +type errTooMuchChange struct { + got int64 + needed int64 +} + +func (e errTooMuchChange) Error() string { + return fmt.Sprintf("Invalid commit -- insufficient old voting power: got %v, needed %v", e.got, e.needed) +} + +//---------------- + func (vals *ValidatorSet) String() string { return vals.StringIndented("") }