final fixes after arbo refactor

The main issue was that on update, if the key and value are the same,
the clean up routine for removing old intermediate nodes, was removing
the current valid intermediate nodes up to the root.

In addition adapt some tests to become compatible with the new impl.
And remove some that are not longer valid.

dockerfiles/testsuite/env.gateway0

@@ -3,7 +3,7 @@ VOCDONI_LOGLEVEL=debug
 VOCDONI_DEV=True
 VOCDONI_ENABLEAPI=True
 VOCDONI_ENABLERPC=True
-VOCDONI_ENABLEFAUCETWITHAMOUNT=1000
+VOCDONI_ENABLEFAUCETWITHAMOUNT=100000
 VOCDONI_VOCHAIN_SEEDS=3c3765494e758ae7baccb1f5b0661755302ddc47@seed:26656
 VOCDONI_VOCHAIN_GENESIS=/app/misc/genesis.json
 VOCDONI_VOCHAIN_NOWAITSYNC=True

dockerfiles/testsuite/genesis.json

@@ -23,8 +23,8 @@
   },
   "app_hash": "",
   "app_state": {
-    "max_election_size": 100000,
-    "network_capacity": 4000, 
+    "max_election_size": 1000000,
+    "network_capacity": 10000, 
     "validators": [
       {
         "signer_address": "6bcc92be71d48cfe3f2f5042f157ad978f3e8117",
@@ -58,19 +58,19 @@
     "accounts":[
         {
            "address":"0xccEc2c2D658261Fbdc40b04FEc06d49057242D39",
-           "balance":100000
+           "balance":10000000
          },
          {
            "address":"0x776d858D17C8018F07899dB535866EBf805a32E0",
-           "balance":100000
+           "balance":10000000
          },
          {
            "address":"0x074fcAacb8B01850539eaE7E9fEE8dc94549db96",
-           "balance":100000
+           "balance":10000000
          },
 	       {
            "address":"0x88a499cEf9D1330111b41360173967c9C1bf703f",
-           "balance":100000
+           "balance":10000000
          }	
       ],
     "treasurer": "0xfe10DAB06D636647f4E40dFd56599da9eF66Db1c",

statedb/treeupdate.go

@@ -2,6 +2,7 @@ package statedb
 
 import (
 	"bytes"
+	"fmt"
 	"io"
 	"path"
 	"sync"
@@ -136,6 +137,11 @@ func (u *TreeUpdate) Del(key []byte) error {
 	return u.tree.Del(u.tree.tx, key)
 }
 
+// PrintGraphviz prints the tree in Graphviz format.
+func (u *TreeUpdate) PrintGraphviz() error {
+	return u.tree.PrintGraphviz(u.tree.tx)
+}
+
 // SubTree is used to open the subTree (singleton and non-singleton) as a
 // TreeUpdate.  The treeUpdate.tx is created from u.tx appending the prefix
 // `subKeySubTree | cfg.prefix`.  In turn the treeUpdate.tree uses the
@@ -164,9 +170,12 @@ func (u *TreeUpdate) SubTree(cfg TreeConfig) (treeUpdate *TreeUpdate, err error)
 		return nil, err
 	}
 	if !bytes.Equal(root, lastRoot) {
-		if err := tree.SetRoot(txTree, root); err != nil {
-			return nil, err
-		}
+		panic(fmt.Sprintf("root for %s mismatch: %x != %x", cfg.kindID, root, lastRoot))
+		// Note (Pau): since we modified arbo to remove all unecessary intermediate nodes,
+		// we cannot set a past root.We should probably remove this code.
+		//if err := tree.SetRoot(txTree, root); err != nil {
+		//	return nil, err
+		//}
 	}
 	treeUpdate = &TreeUpdate{
 		tx: tx,
@@ -296,16 +305,16 @@ func propagateRoot(treeUpdate *TreeUpdate) ([]byte, error) {
 	for key, updates := range updatesByKey {
 		leaf, err := treeUpdate.tree.Get(treeUpdate.tree.tx, []byte(key))
 		if err != nil {
-			return nil, err
+			return nil, fmt.Errorf("failed to get leaf %x: %w", key, err)
 		}
 		for _, update := range updates {
 			leaf, err = update.setRoot(leaf, update.root)
 			if err != nil {
-				return nil, err
+				return nil, fmt.Errorf("failed to set root for leaf %x: %w", key, err)
 			}
 		}
 		if err := treeUpdate.tree.Set(treeUpdate.tree.tx, []byte(key), leaf); err != nil {
-			return nil, err
+			return nil, fmt.Errorf("failed to set leaf %x: %w", key, err)
 		}
 	}
 	// We either updated leaves here, or treeUpdate.tree was dirty, so we
@@ -324,22 +333,22 @@ func propagateRoot(treeUpdate *TreeUpdate) ([]byte, error) {
 func (t *TreeTx) Commit(version uint32) error {
 	root, err := propagateRoot(&t.TreeUpdate)
 	if err != nil {
-		return err
+		return fmt.Errorf("could not propagate root: %w", err)
 	}
 	t.openSubs = sync.Map{}
 	curVersion, err := getVersion(t.tx)
 	if err != nil {
-		return err
+		return fmt.Errorf("could not get current version: %w", err)
 	}
 	// If root is nil, it means that there were no updates to the StateDB,
 	// so the next version root is the current version root.
 	if root == nil {
 		if root, err = t.sdb.getVersionRoot(t.tx, curVersion); err != nil {
-			return err
+			return fmt.Errorf("could not get current version root: %w", err)
 		}
 	}
 	if err := setVersionRoot(t.tx, version, root); err != nil {
-		return err
+		return fmt.Errorf("could not set version root: %w", err)
 	}
 	return t.tx.Commit()
 }
@@ -407,7 +416,7 @@ func (v *treeUpdateView) Import(r io.Reader) error {
 }
 
 func (v *treeUpdateView) PrintGraphviz() error {
-	return v.tree.PrintGraphviz()
+	return v.tree.PrintGraphviz(v.tx)
 }
 
 // verify that treeUpdateView fulfills the TreeViewer interface.

statedb/treeview.go

@@ -83,7 +83,7 @@ func (v *TreeView) IterateNodes(callback func(key, value []byte) bool) error {
 }
 
 func (v *TreeView) PrintGraphviz() error {
-	return v.tree.PrintGraphviz()
+	return v.tree.PrintGraphviz(v.db)
 }
 
 // Iterate iterates over all leafs of this tree.  When callback returns true,

tree/arbo/addbatch_test.go

@@ -1,7 +1,6 @@
 package arbo
 
 import (
-	"bytes"
 	"crypto/rand"
 	"encoding/hex"
 	"fmt"
@@ -975,176 +974,3 @@ func TestAddKeysWithEmptyValues(t *testing.T) {
 	c.Assert(err, qt.IsNil)
 	c.Check(verif, qt.IsFalse)
 }
-
-/*
-func TestAddBatchThresholdInDisk(t *testing.T) {
-	c := qt.New(t)
-
-	// customize thresholdNLeafs for the test
-	testThresholdNLeafs := 1024
-
-	database1 := metadb.NewTest(t)
-	tree1, err := NewTree(Config{database1, 256, testThresholdNLeafs,
-		HashFunctionBlake2b})
-	c.Assert(err, qt.IsNil)
-
-	database2, err := pebbledb.New(db.Options{Path: c.TempDir()})
-	c.Assert(err, qt.IsNil)
-	tree2, err := NewTree(Config{database2, 256, testThresholdNLeafs,
-		HashFunctionBlake2b})
-	c.Assert(err, qt.IsNil)
-
-	database3, err := pebbledb.New(db.Options{Path: c.TempDir()})
-	c.Assert(err, qt.IsNil)
-	tree3, err := NewTree(Config{database3, 256, testThresholdNLeafs,
-		HashFunctionBlake2b})
-	c.Assert(err, qt.IsNil)
-
-	var keys, values [][]byte
-	for i := 0; i < 3*testThresholdNLeafs; i++ {
-		k := randomBytes(32)
-		v := randomBytes(32)
-		if err := tree1.Add(k, v); err != nil {
-			t.Fatal(err)
-		}
-		if i < testThresholdNLeafs+1 {
-			if err := tree2.Add(k, v); err != nil {
-				t.Fatal(err)
-			}
-		}
-		// store for later addition through AddBatch
-		keys = append(keys, k)
-		values = append(values, v)
-	}
-
-	invalids, err := tree2.AddBatch(keys[testThresholdNLeafs+1:], values[testThresholdNLeafs+1:])
-	c.Assert(err, qt.IsNil)
-	c.Check(len(invalids), qt.Equals, 0)
-	// check that both trees roots are equal
-	checkRoots(c, tree1, tree2)
-
-	// call directly the tree3.addBatchInDisk to ensure that is tested
-	wTx := tree3.db.WriteTx()
-	defer wTx.Discard()
-	invalids, err = tree3.addBatchInDisk(wTx, keys, values)
-	c.Assert(err, qt.IsNil)
-	err = wTx.Commit()
-	c.Assert(err, qt.IsNil)
-	c.Check(len(invalids), qt.Equals, 0)
-	// check that both trees roots are equal
-	checkRoots(c, tree1, tree3)
-
-	// now add one leaf more to the trees to ensure that the previous
-	// actions did not left the tree in an invalid state
-	k := randomBytes(32)
-	v := randomBytes(32)
-	err = tree1.Add(k, v)
-	c.Assert(err, qt.IsNil)
-	err = tree2.Add(k, v)
-	c.Assert(err, qt.IsNil)
-	err = tree3.Add(k, v)
-	c.Assert(err, qt.IsNil)
-}
-*/
-
-func initTestUpFromSubRoots(c *qt.C) (*Tree, *Tree) {
-	database1 := metadb.NewTest(c)
-	tree1, err := NewTree(Config{database1, 256, DefaultThresholdNLeafs,
-		HashFunctionBlake2b})
-	c.Assert(err, qt.IsNil)
-
-	database2 := metadb.NewTest(c)
-	tree2, err := NewTree(Config{database2, 256, DefaultThresholdNLeafs,
-		HashFunctionBlake2b})
-	c.Assert(err, qt.IsNil)
-	return tree1, tree2
-}
-
-func testUpFromSubRoots(c *qt.C, tree1, tree2 *Tree, preSubRoots [][]byte) {
-	// add the preSubRoots to the tree1
-	for i := 0; i < len(preSubRoots); i++ {
-		if bytes.Equal(preSubRoots[i], tree1.emptyHash) {
-			continue
-		}
-		err := tree1.Add(preSubRoots[i], nil)
-		c.Assert(err, qt.IsNil)
-	}
-	root1, err := tree1.Root()
-	c.Assert(err, qt.IsNil)
-
-	wTx := tree2.db.WriteTx()
-	subRoots := make([][]byte, len(preSubRoots))
-	for i := 0; i < len(preSubRoots); i++ {
-		if preSubRoots[i] == nil || bytes.Equal(preSubRoots[i], tree1.emptyHash) {
-			subRoots[i] = tree1.emptyHash
-			continue
-		}
-		leafKey, leafValue, err := tree2.newLeafValue(preSubRoots[i], nil)
-		c.Assert(err, qt.IsNil)
-		subRoots[i] = leafKey
-
-		err = wTx.Set(leafKey, leafValue)
-		c.Assert(err, qt.IsNil)
-	}
-	// first fill the leaf nodes
-	// then call upFromSubRoots
-	root2FromUp, err := tree2.upFromSubRoots(wTx, subRoots)
-	c.Assert(err, qt.IsNil)
-
-	err = tree2.SetRootWithTx(wTx, root2FromUp)
-	c.Assert(err, qt.IsNil)
-	err = wTx.Commit()
-	c.Assert(err, qt.IsNil)
-
-	root2, err := tree2.Root()
-	c.Assert(err, qt.IsNil)
-
-	c.Assert(root1, qt.DeepEquals, root2)
-}
-
-func testUpFromSubRootsWithEmpties(c *qt.C, preSubRoots [][]byte, indexEmpties []int) {
-	tree1, tree2 := initTestUpFromSubRoots(c)
-
-	testPreSubRoots := make([][]byte, len(preSubRoots))
-	copy(testPreSubRoots, preSubRoots)
-	for i := 0; i < len(indexEmpties); i++ {
-		testPreSubRoots[indexEmpties[i]] = tree1.emptyHash
-	}
-	testUpFromSubRoots(c, tree1, tree2, testPreSubRoots)
-}
-
-func TestUpFromSubRoots(t *testing.T) {
-	c := qt.New(t)
-
-	// prepare preSubRoots
-	preSubRoots := [][]byte{
-		BigIntToBytes(32, big.NewInt(4)),
-		BigIntToBytes(32, big.NewInt(2)),
-		BigIntToBytes(32, big.NewInt(1)),
-		BigIntToBytes(32, big.NewInt(3)),
-	}
-
-	// test using the full 4 leafs as subRoots
-	testUpFromSubRootsWithEmpties(c, preSubRoots, nil)
-	// 1st subRoot empty
-	testUpFromSubRootsWithEmpties(c, preSubRoots, []int{0})
-	// 2nd subRoot empty
-	testUpFromSubRootsWithEmpties(c, preSubRoots, []int{1})
-	// 3rd subRoot empty
-	testUpFromSubRootsWithEmpties(c, preSubRoots, []int{2})
-	// 4th subRoot empty
-	testUpFromSubRootsWithEmpties(c, preSubRoots, []int{3})
-
-	// other combinations of empty SubRoots
-	testUpFromSubRootsWithEmpties(c, preSubRoots, []int{0, 1, 2, 3})
-	testUpFromSubRootsWithEmpties(c, preSubRoots, []int{0, 1})
-	testUpFromSubRootsWithEmpties(c, preSubRoots, []int{1, 2})
-	testUpFromSubRootsWithEmpties(c, preSubRoots, []int{1, 3})
-	testUpFromSubRootsWithEmpties(c, preSubRoots, []int{2, 3})
-	testUpFromSubRootsWithEmpties(c, preSubRoots, []int{0, 2, 3})
-}
-
-// TODO test adding batch with multiple invalid keys
-// TODO for tests of AddBatch, if the root does not match the Add root, bulk
-// all the leafs of both trees into a log file to later be able to debug and
-// recreate the case

tree/arbo/navigate.go

@@ -11,6 +11,7 @@ import (
 // down goes down to the leaf recursively
 func (t *Tree) down(rTx db.Reader, newKey, currKey []byte, siblings [][]byte, intermediates *[][]byte,
 	path []bool, currLvl int, getLeaf bool) ([]byte, []byte, [][]byte, error) {
+
 	if currLvl > t.maxLevels {
 		return nil, nil, nil, ErrMaxLevel
 	}
@@ -23,7 +24,7 @@ func (t *Tree) down(rTx db.Reader, newKey, currKey []byte, siblings [][]byte, in
 	}
 	currValue, err = rTx.Get(currKey)
 	if err != nil {
-		return nil, nil, nil, fmt.Errorf("could not get value for key %x: %w", currKey, err)
+		return nil, nil, nil, fmt.Errorf("while down, could not get value for key %x: %w", currKey, err)
 	}
 
 	switch currValue[0] {
@@ -65,7 +66,9 @@ func (t *Tree) down(rTx db.Reader, newKey, currKey []byte, siblings [][]byte, in
 				fmt.Errorf("intermediate value invalid length (expected: %d, actual: %d)",
 					PrefixValueLen+t.hashFunction.Len()*2, len(currValue))
 		}
-		*intermediates = append(*intermediates, currKey)
+		if intermediates != nil {
+			*intermediates = append(*intermediates, currKey)
+		}
 
 		// collect siblings while going down
 		if path[currLvl] {