accumulator/batchproof: Error instead of bool for verifyBatchProof

Now the verifyBatchProof function returns an error instead of a bool to
denote that a proof has failed to validate. If the proof validates, a
nil is returned. If it doesn't, an error with a helpful error message
will be returned.

There may be various reasons for verifyBatchProof to fail but the
message that the caller got was just a bool. Returning an error when the
proof fails allows more information to be tranferred to the caller.

accumulator/batchproof.go

@@ -260,15 +260,17 @@ func verifyBatchProof(targetHashes []Hash, bp BatchProof, roots []Hash, numLeave
 	// cached should be a function that fetches nodes from the pollard and
 	// indicates whether they exist or not, this is only useful for the pollard
 	// and nil should be passed for the forest.
-	cached func(pos uint64) (bool, Hash)) (bool, [][]miniTree, []node) {
+	cached func(pos uint64) (bool, Hash)) ([][]miniTree, []node, error) {
 
 	// If there is nothing to prove, return true
 	if len(bp.Targets) == 0 {
-		return true, nil, nil
+		return nil, nil, nil
 	}
 	// There should be a hash for each of the targets being proven
 	if len(bp.Targets) != len(targetHashes) {
-		return false, nil, nil
+		err := fmt.Errorf("verifyBatchProof: %d BatchProof.Targets but have %d targetHashes."+
+			" Should have same amount for each", len(bp.Targets), len(targetHashes))
+		return nil, nil, err
 	}
 
 	tPos := make([]targPos, len(bp.Targets))
@@ -302,7 +304,9 @@ func verifyBatchProof(targetHashes []Hash, bp BatchProof, roots []Hash, numLeave
 
 	// The proof should have as many hashes as there are proof positions.
 	if len(proofPositions.list) != len(bp.Proof) {
-		return false, nil, nil
+		err := fmt.Errorf("verifyBatchProof: %d bp.Proofs but calculated %d proof positions."+
+			" Should have same amount for each", len(bp.Proof), len(proofPositions.list))
+		return nil, nil, err
 	}
 
 	// targetNodes holds nodes that are known, on the bottom row those
@@ -350,7 +354,9 @@ func verifyBatchProof(targetHashes []Hash, bp BatchProof, roots []Hash, numLeave
 		// hashes or less than 2 targets left the proof is invalid because
 		// there is a target without matching proof.
 		if len(targetHashes) < 2 || len(targets) < 2 {
-			return false, nil, nil
+			err := fmt.Errorf("verifyBatchProof: target to prove is without its sibling." +
+				" Cannot verify proof")
+			return nil, nil, err
 		}
 
 		targetNodes = append(targetNodes,
@@ -380,7 +386,9 @@ func verifyBatchProof(targetHashes []Hash, bp BatchProof, roots []Hash, numLeave
 			// target should have its sibling in targetNodes
 			if len(targetNodes) == 1 {
 				// sibling not found
-				return false, nil, nil
+				err := fmt.Errorf("verifyBatchProof: target to prove is without its sibling." +
+					" Verify failed")
+				return nil, nil, err
 			}
 
 			proof = targetNodes[1]
@@ -412,13 +420,20 @@ func verifyBatchProof(targetHashes []Hash, bp BatchProof, roots []Hash, numLeave
 				} else {
 					// The left and right did not match the cached
 					// left and right.
-					return false, nil, nil
+					err := fmt.Errorf("verifyBatchProof: cached hash doesn't match with the"+
+						" calculated hash. Left calculated %x, left cached %x. Right calculated"+
+						" %x, right cached %x",
+						left.Val, cachedLeft, right.Val, cachedRight)
+					return nil, nil, err
 				}
 			} else {
 				hash = parentHash(left.Val, right.Val)
 				if hash != cachedParent {
 					// The calculated hash did not match the cached parent.
-					return false, nil, nil
+					err := fmt.Errorf("verifyBatchProof: calculated parent hash of %x doesn't"+
+						" match with the cached hash of %x.",
+						hash, cachedParent)
+					return nil, nil, err
 				}
 			}
 		} else {
@@ -447,7 +462,9 @@ func verifyBatchProof(targetHashes []Hash, bp BatchProof, roots []Hash, numLeave
 
 	if len(rootCandidates) == 0 {
 		// no roots to verify
-		return false, nil, nil
+		err := fmt.Errorf("verifyBatchProof: no roots were calculated to" +
+			"match with the stored roots")
+		return nil, nil, err
 	}
 
 	// `roots` is ordered, therefore to verify that `rootCandidates`
@@ -463,10 +480,12 @@ func verifyBatchProof(targetHashes []Hash, bp BatchProof, roots []Hash, numLeave
 	if len(rootCandidates) != rootMatches {
 		// the proof is invalid because some root candidates were not
 		// included in `roots`.
-		return false, nil, nil
+		err := fmt.Errorf("verifyBatchProof: generated %d roots but only"+
+			"matched %d roots", len(rootCandidates), rootMatches)
+		return nil, nil, err
 	}
 
-	return true, trees, rootCandidates
+	return trees, rootCandidates, nil
 }
 
 // Reconstruct takes a number of leaves and rows, and turns a block proof back

accumulator/batchproof_test.go

@@ -34,14 +34,13 @@ func TestIncompleteBatchProof(t *testing.T) {
 
 	// create blockProof based on the last add in the slice
 	blockProof, err := f.ProveBatch(leavesToProve)
-
 	if err != nil {
 		t.Fatal(err)
 	}
 
 	blockProof.Proof = blockProof.Proof[:len(blockProof.Proof)-1]
-	shouldBeFalse := f.VerifyBatchProof(leavesToProve, blockProof)
-	if shouldBeFalse != false {
+	err = f.VerifyBatchProof(leavesToProve, blockProof)
+	if err == nil {
 		t.Fail()
 		t.Logf("Incomplete proof passes verification")
 	}
@@ -77,16 +76,15 @@ func TestVerifyBatchProof(t *testing.T) {
 
 	// create blockProof based on the last add in the slice
 	blockProof, err := f.ProveBatch(leavesToProve)
-
 	if err != nil {
 		t.Fatal(err)
 	}
 
 	// Confirm that verify block proof works
-	shouldBetrue := f.VerifyBatchProof(leavesToProve, blockProof)
-	if shouldBetrue != true {
+	err = f.VerifyBatchProof(leavesToProve, blockProof)
+	if err != nil {
 		t.Fail()
-		t.Logf("Block failed to verify")
+		t.Logf("Block failed to verify. Error: %s", err.Error())
 	}
 
 	// delete last leaf and add a new leaf
@@ -98,8 +96,8 @@ func TestVerifyBatchProof(t *testing.T) {
 	}
 
 	// Attempt to verify block proof with deleted element
-	shouldBeFalse := f.VerifyBatchProof(leavesToProve, blockProof)
-	if shouldBeFalse != false {
+	err = f.VerifyBatchProof(leavesToProve, blockProof)
+	if err == nil {
 		t.Fail()
 		t.Logf("Block verified with old proof. Double spending allowed.")
 	}
@@ -130,22 +128,20 @@ func TestProofShouldNotValidateAfterNodeDeleted(t *testing.T) {
 		t.Fatal(fmt.Errorf("ProveBlock of existing values: %v", err))
 	}
 
-	if !f.VerifyBatchProof([]Hash{adds[proofIndex].Hash}, batchProof) {
-		t.Fatal(
-			fmt.Errorf(
-				"proof of %d didn't verify (before deletion)",
-				proofIndex))
+	err = f.VerifyBatchProof([]Hash{adds[proofIndex].Hash}, batchProof)
+	if err != nil {
+		t.Fatal(fmt.Errorf("proof of %d didn't verify (before deletion)",
+			proofIndex))
 	}
 
 	_, err = f.Modify(nil, []uint64{0})
 	if err != nil {
 		t.Fatal(fmt.Errorf("Modify with deletions: %v", err))
 	}
 
-	if f.VerifyBatchProof([]Hash{adds[proofIndex].Hash}, batchProof) {
-		t.Fatal(
-			fmt.Errorf(
-				"proof of %d is still valid (after deletion)",
-				proofIndex))
+	err = f.VerifyBatchProof([]Hash{adds[proofIndex].Hash}, batchProof)
+	if err == nil {
+		t.Fatal(fmt.Errorf("proof of %d is still valid (after deletion)",
+			proofIndex))
 	}
 }

accumulator/forest_test.go

@@ -333,11 +333,9 @@ func addDelFullBatchProof(nAdds, nDels int) error {
 		return err
 	}
 	// check block proof.  Note this doesn't delete anything, just proves inclusion
-	worked, _, _ := verifyBatchProof(leavesToProve, bp, f.getRoots(), f.numLeaves, nil)
-	//	worked := f.VerifyBatchProof(bp)
-
-	if !worked {
-		return fmt.Errorf("VerifyBatchProof failed")
+	_, _, err = verifyBatchProof(leavesToProve, bp, f.getRoots(), f.numLeaves, nil)
+	if err != nil {
+		return fmt.Errorf("VerifyBatchProof failed. Error: %s", err.Error())
 	}
 	fmt.Printf("VerifyBatchProof worked\n")
 	return nil
@@ -442,8 +440,11 @@ func TestSmallRandomForests(t *testing.T) {
 				t.Fatalf("proveblock failed proving existing leaf: %v", err)
 			}
 
-			if !(f.VerifyBatchProof([]Hash{chosenUndeletedLeaf.Hash}, blockProof)) {
-				t.Fatal("verifyblockproof failed verifying proof for existing leaf")
+			err = f.VerifyBatchProof([]Hash{chosenUndeletedLeaf.Hash}, blockProof)
+			if err != nil {
+				retErr := fmt.Errorf("verifyblockproof failed verifying proof for existing leaf."+
+					" Error: %s", err.Error())
+				t.Fatal(retErr)
 			}
 		}
 	}

accumulator/forestproofs.go

@@ -198,7 +198,7 @@ func (f *Forest) ProveBatch(hs []Hash) (BatchProof, error) {
 }
 
 // VerifyBatchProof is just a wrapper around verifyBatchProof
-func (f *Forest) VerifyBatchProof(toProve []Hash, bp BatchProof) bool {
-	ok, _, _ := verifyBatchProof(toProve, bp, f.getRoots(), f.numLeaves, nil)
-	return ok
+func (f *Forest) VerifyBatchProof(toProve []Hash, bp BatchProof) error {
+	_, _, err := verifyBatchProof(toProve, bp, f.getRoots(), f.numLeaves, nil)
+	return err
 }

accumulator/pollardproof.go

@@ -9,7 +9,7 @@ import (
 func (p *Pollard) IngestBatchProof(toProve []Hash, bp BatchProof) error {
 	// verify the batch proof.
 	rootHashes := p.rootHashesForward()
-	ok, trees, roots := verifyBatchProof(toProve, bp, rootHashes, p.numLeaves,
+	trees, roots, err := verifyBatchProof(toProve, bp, rootHashes, p.numLeaves,
 		// pass a closure that checks the pollard for cached nodes.
 		// returns true and the hash value of the node if it exists.
 		// returns false if the node does not exist or the hash value is empty.
@@ -24,8 +24,10 @@ func (p *Pollard) IngestBatchProof(toProve []Hash, bp BatchProof) error {
 
 			return false, empty
 		})
-	if !ok {
-		return fmt.Errorf("block proof mismatch")
+	if err != nil {
+		retErr := fmt.Errorf("Pollard.IngestBatchProof: BatchProof verify failed. %s",
+			err.Error())
+		return retErr
 	}
 
 	// rootIdx and rootIdxBackwards is needed because p.populate()