Fixes from review

lite/client/provider.go

@@ -106,28 +106,23 @@ func (p *provider) getValidatorSet(chainID string, height int64) (valset *types.
 		return nil, lerr.ErrMissingValidators(chainID, height)
 	}
 	valset = types.NewValidatorSet(res.Validators)
-	valset.TotalVotingPower() // to test deep equality.
 	return
 }
 
 // This does no validation.
 func (p *provider) fillFullCommit(signedHeader types.SignedHeader) (fc lite.FullCommit, err error) {
-	fc.SignedHeader = signedHeader
 
 	// Get the validators.
 	valset, err := p.getValidatorSet(signedHeader.ChainID, signedHeader.Height)
 	if err != nil {
 		return lite.FullCommit{}, err
 	}
-	fc.Validators = valset
 
 	// Get the next validators.
 	nextValset, err := p.getValidatorSet(signedHeader.ChainID, signedHeader.Height+1)
 	if err != nil {
 		return lite.FullCommit{}, err
-	} else {
-		fc.NextValidators = nextValset
 	}
 
-	return fc, nil
+	return lite.NewFullCommit(signedHeader, valset, nextValset), nil
 }

lite/client/provider_test.go

@@ -51,7 +51,7 @@ func TestProvider(t *testing.T) {
 	assert.True(sh < 5000)
 
 	// let's check this is valid somehow
-	assert.Nil(fc.ValidateBasic(chainID))
+	assert.Nil(fc.ValidateFull(chainID))
 
 	// historical queries now work :)
 	lower := sh - 5

lite/commit.go

@@ -12,7 +12,7 @@ import (
 // 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,
-// even in the face of arbitrarily power changes.
+// even in the face of arbitrarily large power changes.
 type FullCommit struct {
 	SignedHeader   types.SignedHeader  `json:"signed_header"`
 	Validators     *types.ValidatorSet `json:"validator_set"`
@@ -33,7 +33,7 @@ func NewFullCommit(signedHeader types.SignedHeader, valset, nextValset *types.Va
 // signed the SignedHeader.Commit.
 // If > 2/3 did not sign the Commit from fc.Validators, it
 // is not a valid commit!
-func (fc FullCommit) ValidateBasic(chainID string) error {
+func (fc FullCommit) ValidateFull(chainID string) error {
 	// Ensure that Validators exists and matches the header.
 	if fc.Validators.Size() == 0 {
 		return errors.New("need FullCommit.Validators")

lite/inquiring_certifier.go

@@ -101,21 +101,21 @@ func (ic *InquiringCertifier) Certify(shdr types.SignedHeader) error {
 		return nil
 	} else if err != nil {
 		return err
-	} else {
-		// Create filled FullCommit.
-		nfc := FullCommit{
-			SignedHeader:   shdr,
-			Validators:     tfc.NextValidators,
-			NextValidators: nextValset,
-		}
-		// Validate the full commit.  This checks the cryptographic
-		// signatures of Commit against Validators.
-		if err := nfc.ValidateBasic(ic.chainID); err != nil {
-			return err
-		}
-		// Trust it.
-		return ic.trusted.SaveFullCommit(nfc)
 	}
+
+	// Create filled FullCommit.
+	nfc := FullCommit{
+		SignedHeader:   shdr,
+		Validators:     tfc.NextValidators,
+		NextValidators: nextValset,
+	}
+	// Validate the full commit.  This checks the cryptographic
+	// signatures of Commit against Validators.
+	if err := nfc.ValidateFull(ic.chainID); err != nil {
+		return err
+	}
+	// Trust it.
+	return ic.trusted.SaveFullCommit(nfc)
 }
 
 // verifyAndSave will verify if this is a valid source full commit given the
@@ -139,7 +139,7 @@ func (ic *InquiringCertifier) verifyAndSave(tfc, sfc FullCommit) error {
 }
 
 // updateToHeight will use divide-and-conquer to find a path to h.
-// Returns nil iff we successfully verify and persist a full commit
+// Returns nil error iff we successfully verify and persist a full commit
 // for height h, using repeated applications of bisection if necessary.
 //
 // Returns ErrCommitNotFound if source provider doesn't have the commit for h.
@@ -153,7 +153,7 @@ func (ic *InquiringCertifier) updateToHeight(h int64) (FullCommit, error) {
 
 	// Validate the full commit.  This checks the cryptographic
 	// signatures of Commit against Validators.
-	if err := sfc.ValidateBasic(ic.chainID); err != nil {
+	if err := sfc.ValidateFull(ic.chainID); err != nil {
 		return FullCommit{}, err
 	}
 
@@ -169,9 +169,9 @@ FOR_LOOP:
 		if err != nil {
 			return FullCommit{}, err
 		}
-		// Maybe we have nothing to do.
+		// We have nothing to do.
 		if tfc.Height() == h {
-			return FullCommit{}, nil
+			return tfc, nil
 		}
 
 		// Try to update to full commit with checks.
@@ -179,24 +179,24 @@ FOR_LOOP:
 		if err == nil {
 			// All good!
 			return sfc, nil
-		} else {
-			// Handle special case when err is ErrTooMuchChange.
-			if lerr.IsErrTooMuchChange(err) {
-				// Divide and conquer.
-				start, end := tfc.Height(), sfc.Height()
-				if !(start < end) {
-					panic("should not happen")
-				}
-				mid := (start + end) / 2
-				_, err = ic.updateToHeight(mid)
-				if err != nil {
-					return FullCommit{}, err
-				}
-				// If we made it to mid, we retry.
-				continue FOR_LOOP
-			}
-			return FullCommit{}, err
 		}
+
+		// Handle special case when err is ErrTooMuchChange.
+		if lerr.IsErrTooMuchChange(err) {
+			// Divide and conquer.
+			start, end := tfc.Height(), sfc.Height()
+			if !(start < end) {
+				panic("should not happen")
+			}
+			mid := (start + end) / 2
+			_, err = ic.updateToHeight(mid)
+			if err != nil {
+				return FullCommit{}, err
+			}
+			// If we made it to mid, we retry.
+			continue FOR_LOOP
+		}
+		return FullCommit{}, err
 	}
 }