refactor: Use slices instead of maps

accumulator/batchproof.go

@@ -112,122 +112,145 @@ func FromBytesBatchProof(b []byte) (BatchProof, error) {
 // TODO OH WAIT -- this is not how to to it!  Don't hash all the way up to the
 // roots to verify -- just hash up to any populated node!  Saves a ton of CPU!
 
-// verifyBatchProof takes a block proof and reconstructs / verifies it.
-// takes a blockproof to verify, and the known correct roots to check against.
-// also takes the number of leaves and forest rows (those are redundant
-// if we don't do weird stuff with overly-high forests, which we might)
-// it returns a bool of whether the proof worked, and a map of the sparse
-// forest in the blockproof
-func verifyBatchProof(
-	bp BatchProof, roots []Hash,
-	numLeaves uint64, rows uint8) (bool, map[uint64]Hash) {
-
-	// if nothing to prove, it worked
+// verifyBatchProof verifies a batchproof by checking against the set of known correct roots.
+// Takes a BatchProof, the accumulator roots, and the number of leaves in the forest.
+// Returns wether or not the proof verified correctly, the partial proof tree,
+// and the subset of roots that was computed.
+func verifyBatchProof(bp BatchProof, roots []Hash, numLeaves uint64,
+	// 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, [][3]node, []node) {
 	if len(bp.Targets) == 0 {
-		return true, nil
+		return true, nil, nil
 	}
 
-	// Construct a map with positions to hashes
-	proofmap, err := bp.Reconstruct(numLeaves, rows)
-	if err != nil {
-		fmt.Printf("VerifyBlockProof Reconstruct ERROR %s\n", err.Error())
-		return false, proofmap
+	if cached == nil {
+		cached = func(_ uint64) (bool, Hash) { return false, empty }
 	}
 
-	rootPositions, rootRows := getRootsReverse(numLeaves, rows)
-
-	// partial forest is built, go through and hash everything to make sure
-	// you get the right roots
+	rows := treeRows(numLeaves)
+	proofPositions, computablePositions := ProofPositions(bp.Targets, numLeaves, rows)
+	// targetNodes holds nodes that are known, on the bottom row those are the targets,
+	// on the upper rows it holds computed nodes.
+	// rootCandidates holds the roots that where computed, and have to be compared to the actual roots
+	// at the end.
+	targetNodes := make([]node, 0, len(bp.Targets)*int(rows))
+	rootCandidates := make([]node, 0, len(roots))
+	// trees is a slice of 3-Tuples, each tuple represents a parent and its children.
+	// tuple[0] is the parent, tuple[1] is the left child and tuple[2] is the right child.
+	// trees holds the entire proof tree of the batchproof in this way, sorted by the tuple[0].
+	trees := make([][3]node, 0, len(computablePositions))
+	// initialise the targetNodes for row 0.
+	// TODO: this would be more straight forward if bp.Proofs wouldn't contain the targets
+	proofHashes := make([]Hash, 0, len(proofPositions))
+	targets := bp.Targets
+	var targetsMatched uint64
+	for len(targets) > 0 {
+		// check if the target is the row 0 root.
+		// this is the case if its the last leaf (pos==numLeaves-1)
+		// AND the tree has a root at row 0 (numLeaves&1==1)
+		if targets[0] == numLeaves-1 && numLeaves&1 == 1 {
+			// target is the row 0 root, append it to the root candidates.
+			rootCandidates = append(rootCandidates, node{Val: roots[0], Pos: targets[0]})
+			bp.Proof = bp.Proof[1:]
+			break
+		}
 
-	tagRow := bp.Targets
-	nextRow := []uint64{}
-	sortUint64s(tagRow) // probably don't need to sort
+		// `targets` might contain a target and its sibling or just the target, if
+		// only the target is present the sibling will be in `proofPositions`.
+		if uint64(len(proofPositions)) > targetsMatched &&
+			targets[0]^1 == proofPositions[targetsMatched] {
+			// the sibling of the target is included in the proof positions.
+			lr := targets[0] & 1
+			targetNodes = append(targetNodes, node{Pos: targets[0], Val: bp.Proof[lr]})
+			proofHashes = append(proofHashes, bp.Proof[lr^1])
+			targetsMatched++
+			bp.Proof = bp.Proof[2:]
+			targets = targets[1:]
+			continue
+		}
 
-	// TODO it's ugly that I keep treating the 0-row as a special case,
-	// and has led to a number of bugs.  It *is* special in a way, in that
-	// the bottom row is the only thing you actually prove and add/delete,
-	// but it'd be nice if it could all be treated uniformly.
+		// the sibling is not included in the proof positions, therefore it has to be included in `targets.
+		// if there are less than 2 proof hashes or less than 2 targets left the proof is invalid
+		// because there is a target without matching proof.
+		if len(bp.Proof) < 2 || len(targets) < 2 {
+			return false, nil, nil
+		}
 
-	if verbose {
-		fmt.Printf("tagrow len %d\n", len(tagRow))
+		targetNodes = append(targetNodes,
+			node{Pos: targets[0], Val: bp.Proof[0]},
+			node{Pos: targets[1], Val: bp.Proof[1]})
+		bp.Proof = bp.Proof[2:]
+		targets = targets[2:]
 	}
 
-	var left, right uint64
-
-	// iterate through rows
-	for row := uint8(0); row <= rows; row++ {
-		// iterate through tagged positions in this row
-		for len(tagRow) > 0 {
-			// Efficiency gains here. If there are two or more things to verify,
-			// check if the next thing to verify is the sibling of the current leaf
-			// we're on. Siblingness can be checked with bitwise XOR but since targets are
-			// sorted, we can do bitwise OR instead.
-			if len(tagRow) > 1 && tagRow[0]|1 == tagRow[1] {
-				left = tagRow[0]
-				right = tagRow[1]
-				tagRow = tagRow[2:]
-			} else { // if not only use one tagged position
-				right = tagRow[0] | 1
-				left = right ^ 1
-				tagRow = tagRow[1:]
+	proofHashes = append(proofHashes, bp.Proof...)
+	bp.Proof = proofHashes
+
+	// hash every target node with its sibling (which either is contained in the proof or also a target)
+	for len(targetNodes) > 0 {
+		var target, proof node
+		target = targetNodes[0]
+		if len(proofPositions) > 0 && target.Pos^1 == proofPositions[0] {
+			// target has a sibling in the proof positions, fetch proof
+			proof = node{Pos: proofPositions[0], Val: bp.Proof[0]}
+			proofPositions = proofPositions[1:]
+			bp.Proof = bp.Proof[1:]
+			targetNodes = targetNodes[1:]
+		} else {
+			// target should have its sibling in targetNodes
+			if len(targetNodes) == 1 {
+				// sibling not found
+				return false, nil, nil
 			}
 
-			if verbose {
-				fmt.Printf("left %d rootPoss %d\n", left, rootPositions[0])
-			}
-			// check for roots
-			if left == rootPositions[0] {
-				if verbose {
-					fmt.Printf("one left in tagrow; should be root\n")
-				}
-				// Grab the hash of this position from the map
-				computedRootHash, ok := proofmap[left]
-				if !ok {
-					fmt.Printf("ERR no proofmap for root at %d\n", left)
-					return false, nil
-				}
-				// Verify that this root hash matches the one we stored
-				if computedRootHash != roots[0] {
-					fmt.Printf("row %d root, pos %d expect %04x got %04x\n",
-						row, left, roots[0][:4], computedRootHash[:4])
-					return false, nil
-				}
-				// otherwise OK and pop of the root
-				roots = roots[1:]
-				rootPositions = rootPositions[1:]
-				rootRows = rootRows[1:]
-				break
-			}
+			proof = targetNodes[1]
+			targetNodes = targetNodes[2:]
+		}
 
-			// Grab the parent position of the leaf we've verified
-			parentPos := parent(left, rows)
-			if verbose {
-				fmt.Printf("%d %04x %d %04x -> %d\n",
-					left, proofmap[left], right, proofmap[right], parentPos)
-			}
+		// figure out which node is left and which is right
+		left := target
+		right := proof
+		if target.Pos&1 == 1 {
+			right, left = left, right
+		}
 
-			// this will crash if either is 0000
-			// reconstruct the next row and add the parent to the map
-			parhash := parentHash(proofmap[left], proofmap[right])
-			nextRow = append(nextRow, parentPos)
-			proofmap[parentPos] = parhash
+		// get the hash of the parent from the cache or compute it
+		parentPos := parent(target.Pos, rows)
+		isParentCached, hash := cached(parentPos)
+		if !isParentCached {
+			hash = parentHash(left.Val, right.Val)
 		}
+		trees = append(trees, [3]node{{Val: hash, Pos: parentPos}, left, right})
 
-		// Make the nextRow the tagRow so we'll be iterating over it
-		// reset th nextRow
-		tagRow = nextRow
-		nextRow = []uint64{}
+		row := detectRow(parentPos, rows)
+		if numLeaves&(1<<row) > 0 && parentPos == rootPosition(numLeaves, row, rows) {
+			// the parent is a root -> store as candidate, to check against actual roots later.
+			rootCandidates = append(rootCandidates, node{Val: hash, Pos: parentPos})
+			continue
+		}
+		targetNodes = append(targetNodes, node{Val: hash, Pos: parentPos})
+	}
 
-		// if done with row and there's a root left on this row, remove it
-		if len(rootRows) > 0 && rootRows[0] == row {
-			// bit ugly to do these all separately eh
-			roots = roots[1:]
-			rootPositions = rootPositions[1:]
-			rootRows = rootRows[1:]
+	if len(rootCandidates) == 0 {
+		// no roots to verify
+		return false, nil, nil
+	}
+
+	// `roots` is ordered, therefore to verify that `rootCandidates` holds a subset of the roots
+	// we count the roots that match in order.
+	rootMatches := 0
+	for _, root := range roots {
+		if len(rootCandidates) > rootMatches && root == rootCandidates[rootMatches].Val {
+			rootMatches++
 		}
 	}
+	if len(rootCandidates) != rootMatches {
+		// the proof is invalid because some root candidates were not included in `roots`.
+		return false, nil, nil
+	}
 
-	return true, proofmap
+	return true, trees, rootCandidates
 }
 
 // Reconstruct takes a number of leaves and rows, and turns a block proof back

accumulator/forest_test.go

@@ -188,7 +188,7 @@ func addDelFullBatchProof(nAdds, nDels int) error {
 	}
 	bp.SortTargets()
 	// check block proof.  Note this doesn't delete anything, just proves inclusion
-	worked, _ := verifyBatchProof(bp, f.getRoots(), f.numLeaves, f.rows)
+	worked, _, _ := verifyBatchProof(bp, f.getRoots(), f.numLeaves, nil)
 	//	worked := f.VerifyBatchProof(bp)
 
 	if !worked {

accumulator/forestproofs.go

@@ -177,84 +177,13 @@ func (f *Forest) ProveBatch(hs []Hash) (BatchProof, error) {
 	copy(sortedTargets, bp.Targets)
 	sortUint64s(sortedTargets)
 
-	// TODO feels like you could do all this with just slices and no maps...
-	// that would be better
-	// proofTree is the partially populated tree of everything needed for the
-	// proofs
-	proofTree := make(map[uint64]Hash)
-
-	// go through each target and add a proof for it up to the intersection
-	for _, pos := range sortedTargets {
-		// add hash for the deletion itself and its sibling
-		// if they already exist, skip the whole thing
-		_, alreadyThere := proofTree[pos]
-		if alreadyThere {
-			//			fmt.Printf("%d omit already there\n", pos)
-			continue
-		}
-		// TODO change this for the real thing; no need to prove 0-tree root.
-		// but we still need to verify it and tag it as a target.
-		if pos == f.numLeaves-1 && pos&1 == 0 {
-			proofTree[pos] = f.data.read(pos)
-			// fmt.Printf("%d add as root\n", pos)
-			continue
-		}
-
-		// always put in both siblings when on the bottom row
-		// this can be out of order but it will be sorted later
-		proofTree[pos] = f.data.read(pos)
-		proofTree[pos^1] = f.data.read(pos ^ 1)
-		// fmt.Printf("added leaves %d, %d\n", pos, pos^1)
-
-		treeTop := detectSubTreeRows(pos, f.numLeaves, f.rows)
-		pos = parent(pos, f.rows)
-		// go bottom to top and add siblings into the partial tree
-		// start at row 1 though; we always populate the bottom leaf and sibling
-		// This either gets to the top, or intersects before that and deletes
-		// something
-		for h := uint8(1); h < treeTop; h++ {
-			// check if the sibling is already there, in which case we're done
-			// also check if the parent itself is there, in which case we delete it!
-			// I think this with the early ignore at the bottom make it optimal
-			_, selfThere := proofTree[pos]
-			_, sibThere := proofTree[pos^1]
-			if sibThere {
-				// sibling position already exists in partial tree; done
-				// with this branch
-
-				// TODO seems that this never happens and can be removed
-				panic("this never happens...?")
-			}
-			if selfThere {
-				// self position already there; remove as children are known
-				//				fmt.Printf("remove proof from pos %d\n", pos)
-
-				delete(proofTree, pos)
-				delete(proofTree, pos^1) // right? can delete both..?
-				break
-			}
-			// fmt.Printf("add proof from pos %d\n", pos^1)
-			proofTree[pos^1] = f.data.read(pos ^ 1)
-			pos = parent(pos, f.rows)
-		}
-	}
-
-	var nodeSlice []node
-
-	// run through partial tree to turn it into a slice
-	for pos, hash := range proofTree {
-		nodeSlice = append(nodeSlice, node{pos, hash})
+	proofPositions, _ := ProofPositions(sortedTargets, f.numLeaves, f.rows)
+	targetsAndProof := mergeSortedSlices(proofPositions, sortedTargets)
+	bp.Proof = make([]Hash, len(targetsAndProof))
+	for i, proofPos := range targetsAndProof {
+		bp.Proof[i] = f.data.read(proofPos)
 	}
-	// fmt.Printf("made nodeSlice %d nodes\n", len(nodeSlice))
 
-	// sort the slice of nodes (even though we only want the hashes)
-	sortNodeSlice(nodeSlice)
-	// copy the sorted / in-order hashes into a hash slice
-	bp.Proof = make([]Hash, len(nodeSlice))
-
-	for i, n := range nodeSlice {
-		bp.Proof[i] = n.Val
-	}
 	if verbose {
 		fmt.Printf("blockproof targets: %v\n", bp.Targets)
 	}
@@ -266,6 +195,6 @@ func (f *Forest) ProveBatch(hs []Hash) (BatchProof, error) {
 
 // VerifyBatchProof :
 func (f *Forest) VerifyBatchProof(bp BatchProof) bool {
-	ok, _ := verifyBatchProof(bp, f.getRoots(), f.numLeaves, f.rows)
+	ok, _, _ := verifyBatchProof(bp, f.getRoots(), f.numLeaves, nil)
 	return ok
 }