From 9840f5f81697ee79a449c432b0a251608d44e73b Mon Sep 17 00:00:00 2001
From: hanish gogada <hanishgogadahcu@gmail.com>
Date: Sat, 21 Dec 2024 23:48:15 -0800
Subject: [PATCH 01/21] added tree configuration and testcases

---
 trees/treeconfig.go      | 194 +++++++++++++++++++++++++++++++++++++++
 trees/treeconfig_test.go | 138 ++++++++++++++++++++++++++++
 2 files changed, 332 insertions(+)
 create mode 100644 trees/treeconfig.go
 create mode 100644 trees/treeconfig_test.go

diff --git a/trees/treeconfig.go b/trees/treeconfig.go
new file mode 100644
index 00000000..9ec94ab7
--- /dev/null
+++ b/trees/treeconfig.go
@@ -0,0 +1,194 @@
+package trees
+
+import (
+	"fmt"
+	"math"
+
+	"github.com/relab/hotstuff"
+)
+
+// TreeConfiguration is an abstraction for a tree communication model.
+type TreeConfiguration interface {
+	InitializeWithPIDs(treePositions []hotstuff.ID) error
+	GetTreeHeight() int
+	GetChildren() []hotstuff.ID
+	GetSubTreeNodes() []hotstuff.ID
+	GetParent() (hotstuff.ID, bool)
+}
+
+// Tree implements a fault free tree configuration.
+type Tree struct {
+	id                hotstuff.ID
+	configurationSize int
+	height            int
+	idToPosMapping    map[hotstuff.ID]int
+	posToIDMapping    []hotstuff.ID
+	branchFactor      int
+}
+
+// CreateTree Creates the tree configuration, currently only fault free tree configuration is supported.
+func CreateTree(configurationSize int, myID hotstuff.ID, bf int) TreeConfiguration {
+
+	if configurationSize <= 0 {
+		return nil
+	}
+	temp := configurationSize
+	temp = temp - 1 //root
+	height := 1
+	for i := 1; temp > 0; i++ {
+		temp = temp - int(math.Pow(float64(bf), float64(i)))
+		height++
+	}
+	return &Tree{
+		id:                myID,
+		configurationSize: configurationSize,
+		height:            height,
+		branchFactor:      bf,
+	}
+}
+
+// InitializeWithPIDs uses the map to initialize the position of replicas.
+func (t *Tree) InitializeWithPIDs(ids []hotstuff.ID) error {
+	if len(ids) != t.configurationSize {
+		return fmt.Errorf("Invalid number of replicas")
+	}
+	t.posToIDMapping = ids
+	t.idToPosMapping = make(map[hotstuff.ID]int)
+	for index, ID := range ids {
+		if _, ok := t.idToPosMapping[ID]; !ok {
+			t.idToPosMapping[ID] = index
+		} else {
+			return fmt.Errorf("Duplicate replica ID")
+		}
+	}
+	return nil
+}
+
+func (t *Tree) GetTreeHeight() int {
+	return t.height
+}
+
+func (t *Tree) getPosition() (int, error) {
+	pos, ok := t.idToPosMapping[t.id]
+	if !ok {
+		return 0, fmt.Errorf("Replica not found")
+	}
+	return pos, nil
+}
+
+func (t *Tree) getReplicaPosition(replicaId hotstuff.ID) (int, error) {
+	pos, ok := t.idToPosMapping[replicaId]
+	if !ok {
+		return 0, fmt.Errorf("Replica not found")
+	}
+	return pos, nil
+}
+
+// GetParent fetches the ID of the parent, if root, returns itself.
+func (t *Tree) GetParent() (hotstuff.ID, bool) {
+	myPos, err := t.getPosition()
+	if err != nil {
+		return hotstuff.ID(0), false
+	}
+	if myPos == 0 {
+		return t.id, false
+	}
+	return t.posToIDMapping[(myPos-1)/t.branchFactor], true
+}
+
+// GetChildren returns the children of the replicas, if any.
+func (t *Tree) GetChildren() []hotstuff.ID {
+	return t.GetChildrenOfNode(t.id)
+}
+
+func (t *Tree) isWithInIndex(position int) bool {
+	return position < t.configurationSize
+}
+
+// IsRoot return true if the replica is at root of the tree.
+func (t *Tree) IsRoot(nodeID hotstuff.ID) bool {
+	pos, err := t.getReplicaPosition(nodeID)
+	if err != nil {
+		return false
+	}
+	return pos == 0
+}
+
+// GetChildrenOfNode returns the children of a specific replica.
+func (t *Tree) GetChildrenOfNode(nodeID hotstuff.ID) []hotstuff.ID {
+	children := make([]hotstuff.ID, 0)
+	nodePos, err := t.getReplicaPosition(nodeID)
+	if err != nil {
+		return children
+	}
+	for i := 1; i <= t.branchFactor; i++ {
+		childPos := (t.branchFactor * nodePos) + i
+		if t.isWithInIndex(childPos) {
+			children = append(children, t.posToIDMapping[childPos])
+		} else {
+			break
+		}
+	}
+	return children
+}
+
+// getHeight returns the height of a given replica.
+func (t *Tree) getHeight(nodeID hotstuff.ID) int {
+	if t.IsRoot(nodeID) {
+		return t.height
+	}
+	nodePos, err := t.getReplicaPosition(nodeID)
+	if err != nil {
+		return 0
+	}
+	startLimit := 0
+	endLimit := 0
+	for i := 1; i < t.height; i++ {
+		startLimit = startLimit + int(math.Pow(float64(t.branchFactor), float64(i-1)))
+		endLimit = endLimit + int(math.Pow(float64(t.branchFactor), float64(i)))
+		if nodePos >= startLimit && nodePos <= endLimit {
+			return t.height - i
+		}
+	}
+	return 0
+}
+
+// GetHeight returns the height of the replica
+func (t *Tree) GetHeight() int {
+	return t.getHeight(t.id)
+}
+
+// GetPeers returns the peers of given ID, if any.
+func (t *Tree) GetPeers(nodeID hotstuff.ID) []hotstuff.ID {
+	peers := make([]hotstuff.ID, 0)
+	if t.IsRoot(nodeID) {
+		return peers
+	}
+	parent, ok := t.GetParent()
+	if !ok {
+		return peers
+	}
+	parentChildren := t.GetChildrenOfNode(parent)
+	return parentChildren
+}
+
+// GetSubTreeNodes returns all the nodes of its subtree.
+func (t *Tree) GetSubTreeNodes() []hotstuff.ID {
+	nodeID := t.id
+	subTreeNodes := make([]hotstuff.ID, 0)
+	children := t.GetChildrenOfNode(nodeID)
+	queue := make([]hotstuff.ID, 0)
+	queue = append(queue, children...)
+	subTreeNodes = append(subTreeNodes, children...)
+	if len(children) == 0 {
+		return subTreeNodes
+	}
+	for len(queue) > 0 {
+		child := queue[0]
+		queue = queue[1:]
+		children := t.GetChildrenOfNode(child)
+		subTreeNodes = append(subTreeNodes, children...)
+		queue = append(queue, children...)
+	}
+	return subTreeNodes
+}
diff --git a/trees/treeconfig_test.go b/trees/treeconfig_test.go
new file mode 100644
index 00000000..b4fca5db
--- /dev/null
+++ b/trees/treeconfig_test.go
@@ -0,0 +1,138 @@
+package trees
+
+import (
+	"slices"
+	"sort"
+	"testing"
+
+	"github.com/relab/hotstuff"
+)
+
+type createTreeTest struct {
+	configurationSize int
+	id                hotstuff.ID
+	branchFactor      int
+	height            int
+}
+
+func TestCreateTree(t *testing.T) {
+	createTreeTestData := []createTreeTest{
+		{10, 1, 2, 4},
+		{21, 1, 4, 3},
+		{21, 1, 3, 4},
+		{111, 1, 10, 3},
+		{111, 1, 3, 5},
+	}
+	for _, test := range createTreeTestData {
+		tree := CreateTree(test.configurationSize, test.id, test.branchFactor)
+		if tree.GetTreeHeight() != test.height {
+			t.Errorf("Expected height %d, got %d", test.height, tree.GetTreeHeight())
+		}
+	}
+}
+
+func TestTreeWithNegativeCases(t *testing.T) {
+	tree := CreateTree(10, 1, 2)
+	if tree.GetTreeHeight() != 4 {
+		t.Errorf("Expected height 4, got %d", tree.GetTreeHeight())
+	}
+	if len(tree.GetChildren()) != 0 {
+		t.Errorf("Expected nil, got %v", tree.GetChildren())
+	}
+	if len(tree.GetSubTreeNodes()) != 0 {
+		t.Errorf("Expected nil, got %v", tree.GetSubTreeNodes())
+	}
+	if _, ok := tree.GetParent(); ok {
+		t.Errorf("Expected false, got true")
+	}
+	tree = CreateTree(-1, 1, 2)
+	if tree != nil {
+		t.Errorf("Expected nil, got %v", tree)
+	}
+	ids := []hotstuff.ID{1, 2, 3, 3, 4, 5, 6, 7, 8, 9}
+	tree = CreateTree(10, 1, 2)
+	err := tree.InitializeWithPIDs(ids)
+	if err == nil {
+		t.Errorf("Expected error, got nil")
+	}
+	ids = []hotstuff.ID{1, 2, 3, 4, 5, 6, 7, 8, 9}
+	err = tree.InitializeWithPIDs(ids)
+	if err == nil {
+		t.Errorf("Expected error, got nil")
+	}
+}
+
+type treeConfigTest struct {
+	configurationSize int
+	id                hotstuff.ID
+	branchFactor      int
+	height            int
+	children          []hotstuff.ID
+	subTreeNodes      []hotstuff.ID
+	parent            hotstuff.ID
+	isRoot            bool
+	replicaHeight     int
+	peers             []hotstuff.ID
+}
+
+func TestTreeAPIWithInitializeWithPIDs(t *testing.T) {
+	treeConfigTestData := []treeConfigTest{
+		{10, 1, 2, 4, []hotstuff.ID{2, 3}, []hotstuff.ID{2, 3, 4, 5, 6, 7, 8, 9, 10}, 1, true, 4, []hotstuff.ID{}},
+		{10, 5, 2, 4, []hotstuff.ID{10}, []hotstuff.ID{10}, 2, false, 2, []hotstuff.ID{4, 5}},
+		{10, 2, 2, 4, []hotstuff.ID{4, 5}, []hotstuff.ID{4, 5, 8, 9, 10}, 1, false, 3, []hotstuff.ID{2, 3}},
+		{10, 3, 2, 4, []hotstuff.ID{6, 7}, []hotstuff.ID{6, 7}, 1, false, 3, []hotstuff.ID{2, 3}},
+		{10, 4, 2, 4, []hotstuff.ID{8, 9}, []hotstuff.ID{8, 9}, 2, false, 2, []hotstuff.ID{4, 5}},
+		{21, 1, 4, 3, []hotstuff.ID{2, 3, 4, 5}, []hotstuff.ID{2, 3, 4, 5, 6, 7, 8, 9, 10,
+			11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21}, 1, true, 3, []hotstuff.ID{}},
+		{21, 1, 3, 4, []hotstuff.ID{2, 3, 4}, []hotstuff.ID{2, 3, 4, 5, 6, 7, 8, 9, 10,
+			11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21}, 1, true, 4, []hotstuff.ID{}},
+		{21, 2, 4, 3, []hotstuff.ID{6, 7, 8, 9}, []hotstuff.ID{6, 7, 8, 9}, 1, false, 2, []hotstuff.ID{2, 3, 4, 5}},
+		{21, 2, 3, 4, []hotstuff.ID{5, 6, 7}, []hotstuff.ID{5, 6, 7, 14, 15, 16, 17, 18, 19, 20, 21}, 1, false, 3, []hotstuff.ID{2, 3, 4}},
+		{21, 9, 3, 4, []hotstuff.ID{}, []hotstuff.ID{}, 3, false, 2, []hotstuff.ID{8, 9, 10}},
+		{21, 7, 3, 4, []hotstuff.ID{20, 21}, []hotstuff.ID{20, 21}, 2, false, 2, []hotstuff.ID{5, 6, 7}},
+		{21, 3, 4, 3, []hotstuff.ID{10, 11, 12, 13}, []hotstuff.ID{10, 11, 12, 13}, 1, false, 2, []hotstuff.ID{2, 3, 4, 5}},
+		{21, 10, 4, 3, []hotstuff.ID{}, []hotstuff.ID{}, 3, false, 1, []hotstuff.ID{10, 11, 12, 13}},
+		{21, 15, 4, 3, []hotstuff.ID{}, []hotstuff.ID{}, 4, false, 1, []hotstuff.ID{14, 15, 16, 17}},
+		{21, 20, 4, 3, []hotstuff.ID{}, []hotstuff.ID{}, 5, false, 1, []hotstuff.ID{18, 19, 20, 21}},
+		{21, 5, 4, 3, []hotstuff.ID{18, 19, 20, 21}, []hotstuff.ID{18, 19, 20, 21}, 1, false, 2, []hotstuff.ID{2, 3, 4, 5}},
+	}
+	for _, test := range treeConfigTestData {
+		tree := CreateTree(test.configurationSize, test.id, test.branchFactor)
+		ids := make([]hotstuff.ID, test.configurationSize)
+		for i := 0; i < test.configurationSize; i++ {
+			ids[i] = hotstuff.ID(i + 1)
+		}
+		tree.InitializeWithPIDs(ids)
+		if tree.GetTreeHeight() != test.height {
+			t.Errorf("Expected height %d, got %d", test.height, tree.GetTreeHeight())
+		}
+		gotChildren := tree.GetChildren()
+		sort.Slice(gotChildren, func(i, j int) bool { return gotChildren[i] < gotChildren[j] })
+		if len(gotChildren) != len(test.children) || !slices.Equal(gotChildren, test.children) {
+			t.Errorf("Expected %v, got %v", test.children, tree.GetChildren())
+		}
+		subTree := tree.GetSubTreeNodes()
+		sort.Slice(subTree, func(i, j int) bool { return subTree[i] < subTree[j] })
+		if len(subTree) != len(test.subTreeNodes) ||
+			!slices.Equal(subTree, test.subTreeNodes) {
+			t.Errorf("Expected %v, got %v", test.subTreeNodes, tree.GetSubTreeNodes())
+		}
+		if parent, ok := tree.GetParent(); ok {
+			if parent != test.parent {
+				t.Errorf("Expected %d, got %d", test.parent, parent)
+			}
+		}
+		treeConfig := tree.(*Tree)
+		if treeConfig.IsRoot(test.id) != test.isRoot {
+			t.Errorf("Expected %t, got %t", test.isRoot, treeConfig.IsRoot(test.id))
+		}
+		if treeConfig.GetHeight() != test.replicaHeight {
+			t.Errorf("Expected %d, got %d", test.replicaHeight, treeConfig.GetHeight())
+		}
+		gotPeers := treeConfig.GetPeers(test.id)
+		sort.Slice(gotPeers, func(i, j int) bool { return gotPeers[i] < gotPeers[j] })
+		if len(gotPeers) != len(test.peers) || !slices.Equal(gotPeers, test.peers) {
+			t.Errorf("Expected %v, got %v", test.peers, treeConfig.GetPeers(test.id))
+		}
+	}
+}

From 0131823e38b87d0cde82cac92b556bb930bc377b Mon Sep 17 00:00:00 2001
From: Hein Meling <hein.meling@uis.no>
Date: Sun, 22 Dec 2024 11:42:41 +0100
Subject: [PATCH 02/21] chore(trees): gofmt and fixed lint issues

---
 trees/treeconfig.go      | 11 +++++------
 trees/treeconfig_test.go | 14 +++++++++-----
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/trees/treeconfig.go b/trees/treeconfig.go
index 9ec94ab7..e1f613cf 100644
--- a/trees/treeconfig.go
+++ b/trees/treeconfig.go
@@ -28,12 +28,11 @@ type Tree struct {
 
 // CreateTree Creates the tree configuration, currently only fault free tree configuration is supported.
 func CreateTree(configurationSize int, myID hotstuff.ID, bf int) TreeConfiguration {
-
 	if configurationSize <= 0 {
 		return nil
 	}
 	temp := configurationSize
-	temp = temp - 1 //root
+	temp = temp - 1 // root
 	height := 1
 	for i := 1; temp > 0; i++ {
 		temp = temp - int(math.Pow(float64(bf), float64(i)))
@@ -50,7 +49,7 @@ func CreateTree(configurationSize int, myID hotstuff.ID, bf int) TreeConfigurati
 // InitializeWithPIDs uses the map to initialize the position of replicas.
 func (t *Tree) InitializeWithPIDs(ids []hotstuff.ID) error {
 	if len(ids) != t.configurationSize {
-		return fmt.Errorf("Invalid number of replicas")
+		return fmt.Errorf("invalid number of replicas")
 	}
 	t.posToIDMapping = ids
 	t.idToPosMapping = make(map[hotstuff.ID]int)
@@ -58,7 +57,7 @@ func (t *Tree) InitializeWithPIDs(ids []hotstuff.ID) error {
 		if _, ok := t.idToPosMapping[ID]; !ok {
 			t.idToPosMapping[ID] = index
 		} else {
-			return fmt.Errorf("Duplicate replica ID")
+			return fmt.Errorf("duplicate replica ID")
 		}
 	}
 	return nil
@@ -71,7 +70,7 @@ func (t *Tree) GetTreeHeight() int {
 func (t *Tree) getPosition() (int, error) {
 	pos, ok := t.idToPosMapping[t.id]
 	if !ok {
-		return 0, fmt.Errorf("Replica not found")
+		return 0, fmt.Errorf("replica not found")
 	}
 	return pos, nil
 }
@@ -79,7 +78,7 @@ func (t *Tree) getPosition() (int, error) {
 func (t *Tree) getReplicaPosition(replicaId hotstuff.ID) (int, error) {
 	pos, ok := t.idToPosMapping[replicaId]
 	if !ok {
-		return 0, fmt.Errorf("Replica not found")
+		return 0, fmt.Errorf("replica not found")
 	}
 	return pos, nil
 }
diff --git a/trees/treeconfig_test.go b/trees/treeconfig_test.go
index b4fca5db..1463985b 100644
--- a/trees/treeconfig_test.go
+++ b/trees/treeconfig_test.go
@@ -82,10 +82,12 @@ func TestTreeAPIWithInitializeWithPIDs(t *testing.T) {
 		{10, 2, 2, 4, []hotstuff.ID{4, 5}, []hotstuff.ID{4, 5, 8, 9, 10}, 1, false, 3, []hotstuff.ID{2, 3}},
 		{10, 3, 2, 4, []hotstuff.ID{6, 7}, []hotstuff.ID{6, 7}, 1, false, 3, []hotstuff.ID{2, 3}},
 		{10, 4, 2, 4, []hotstuff.ID{8, 9}, []hotstuff.ID{8, 9}, 2, false, 2, []hotstuff.ID{4, 5}},
-		{21, 1, 4, 3, []hotstuff.ID{2, 3, 4, 5}, []hotstuff.ID{2, 3, 4, 5, 6, 7, 8, 9, 10,
-			11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21}, 1, true, 3, []hotstuff.ID{}},
-		{21, 1, 3, 4, []hotstuff.ID{2, 3, 4}, []hotstuff.ID{2, 3, 4, 5, 6, 7, 8, 9, 10,
-			11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21}, 1, true, 4, []hotstuff.ID{}},
+		{21, 1, 4, 3, []hotstuff.ID{2, 3, 4, 5}, []hotstuff.ID{
+			2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21,
+		}, 1, true, 3, []hotstuff.ID{}},
+		{21, 1, 3, 4, []hotstuff.ID{2, 3, 4}, []hotstuff.ID{
+			2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21,
+		}, 1, true, 4, []hotstuff.ID{}},
 		{21, 2, 4, 3, []hotstuff.ID{6, 7, 8, 9}, []hotstuff.ID{6, 7, 8, 9}, 1, false, 2, []hotstuff.ID{2, 3, 4, 5}},
 		{21, 2, 3, 4, []hotstuff.ID{5, 6, 7}, []hotstuff.ID{5, 6, 7, 14, 15, 16, 17, 18, 19, 20, 21}, 1, false, 3, []hotstuff.ID{2, 3, 4}},
 		{21, 9, 3, 4, []hotstuff.ID{}, []hotstuff.ID{}, 3, false, 2, []hotstuff.ID{8, 9, 10}},
@@ -102,7 +104,9 @@ func TestTreeAPIWithInitializeWithPIDs(t *testing.T) {
 		for i := 0; i < test.configurationSize; i++ {
 			ids[i] = hotstuff.ID(i + 1)
 		}
-		tree.InitializeWithPIDs(ids)
+		if err := tree.InitializeWithPIDs(ids); err != nil {
+			t.Errorf("Expected nil, got %v", err)
+		}
 		if tree.GetTreeHeight() != test.height {
 			t.Errorf("Expected height %d, got %d", test.height, tree.GetTreeHeight())
 		}

From 5fe6e96ebc2561214065bd44bd8dcbc07fdb6512 Mon Sep 17 00:00:00 2001
From: Hein Meling <hein.meling@uis.no>
Date: Sun, 22 Dec 2024 13:25:25 +0100
Subject: [PATCH 03/21] chore(trees): removed TreeConfiguration interface

We don't need an interface (yet), since there is only one
implementation. A client (user) of the package can easily add its
own interface as necessary.
---
 trees/treeconfig.go      | 13 ++-----------
 trees/treeconfig_test.go | 13 ++++++-------
 2 files changed, 8 insertions(+), 18 deletions(-)

diff --git a/trees/treeconfig.go b/trees/treeconfig.go
index e1f613cf..4f7deeaa 100644
--- a/trees/treeconfig.go
+++ b/trees/treeconfig.go
@@ -7,15 +7,6 @@ import (
 	"github.com/relab/hotstuff"
 )
 
-// TreeConfiguration is an abstraction for a tree communication model.
-type TreeConfiguration interface {
-	InitializeWithPIDs(treePositions []hotstuff.ID) error
-	GetTreeHeight() int
-	GetChildren() []hotstuff.ID
-	GetSubTreeNodes() []hotstuff.ID
-	GetParent() (hotstuff.ID, bool)
-}
-
 // Tree implements a fault free tree configuration.
 type Tree struct {
 	id                hotstuff.ID
@@ -26,8 +17,8 @@ type Tree struct {
 	branchFactor      int
 }
 
-// CreateTree Creates the tree configuration, currently only fault free tree configuration is supported.
-func CreateTree(configurationSize int, myID hotstuff.ID, bf int) TreeConfiguration {
+// CreateTree creates the tree configuration, currently only fault free tree configuration is supported.
+func CreateTree(configurationSize int, myID hotstuff.ID, bf int) *Tree {
 	if configurationSize <= 0 {
 		return nil
 	}
diff --git a/trees/treeconfig_test.go b/trees/treeconfig_test.go
index 1463985b..4ed23643 100644
--- a/trees/treeconfig_test.go
+++ b/trees/treeconfig_test.go
@@ -126,17 +126,16 @@ func TestTreeAPIWithInitializeWithPIDs(t *testing.T) {
 				t.Errorf("Expected %d, got %d", test.parent, parent)
 			}
 		}
-		treeConfig := tree.(*Tree)
-		if treeConfig.IsRoot(test.id) != test.isRoot {
-			t.Errorf("Expected %t, got %t", test.isRoot, treeConfig.IsRoot(test.id))
+		if tree.IsRoot(test.id) != test.isRoot {
+			t.Errorf("Expected %t, got %t", test.isRoot, tree.IsRoot(test.id))
 		}
-		if treeConfig.GetHeight() != test.replicaHeight {
-			t.Errorf("Expected %d, got %d", test.replicaHeight, treeConfig.GetHeight())
+		if tree.GetHeight() != test.replicaHeight {
+			t.Errorf("Expected %d, got %d", test.replicaHeight, tree.GetHeight())
 		}
-		gotPeers := treeConfig.GetPeers(test.id)
+		gotPeers := tree.GetPeers(test.id)
 		sort.Slice(gotPeers, func(i, j int) bool { return gotPeers[i] < gotPeers[j] })
 		if len(gotPeers) != len(test.peers) || !slices.Equal(gotPeers, test.peers) {
-			t.Errorf("Expected %v, got %v", test.peers, treeConfig.GetPeers(test.id))
+			t.Errorf("Expected %v, got %v", test.peers, tree.GetPeers(test.id))
 		}
 	}
 }

From 2bd3535fa3ebe2e505285bcc0d7ddad54ad2fb61 Mon Sep 17 00:00:00 2001
From: Hein Meling <hein.meling@uis.no>
Date: Sun, 22 Dec 2024 13:36:51 +0100
Subject: [PATCH 04/21] test(trees): make the TestCreateTree table use labeled
 entries

This makes the test data specify the label: number to make it more
readable; it is hard to map the four numbers to the field name of
original createTreeTest type.
---
 trees/treeconfig_test.go | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/trees/treeconfig_test.go b/trees/treeconfig_test.go
index 4ed23643..f4a708a1 100644
--- a/trees/treeconfig_test.go
+++ b/trees/treeconfig_test.go
@@ -8,25 +8,24 @@ import (
 	"github.com/relab/hotstuff"
 )
 
-type createTreeTest struct {
-	configurationSize int
-	id                hotstuff.ID
-	branchFactor      int
-	height            int
-}
-
 func TestCreateTree(t *testing.T) {
-	createTreeTestData := []createTreeTest{
-		{10, 1, 2, 4},
-		{21, 1, 4, 3},
-		{21, 1, 3, 4},
-		{111, 1, 10, 3},
-		{111, 1, 3, 5},
+	tests := []struct {
+		configurationSize int
+		id                hotstuff.ID
+		branchFactor      int
+		wantHeight        int
+	}{
+		{configurationSize: 10, id: 1, branchFactor: 2, wantHeight: 4},
+		{configurationSize: 21, id: 1, branchFactor: 4, wantHeight: 3},
+		{configurationSize: 21, id: 1, branchFactor: 3, wantHeight: 4},
+		{configurationSize: 111, id: 1, branchFactor: 10, wantHeight: 3},
+		{configurationSize: 111, id: 1, branchFactor: 3, wantHeight: 5},
 	}
-	for _, test := range createTreeTestData {
+	for _, test := range tests {
 		tree := CreateTree(test.configurationSize, test.id, test.branchFactor)
-		if tree.GetTreeHeight() != test.height {
-			t.Errorf("Expected height %d, got %d", test.height, tree.GetTreeHeight())
+		if tree.GetTreeHeight() != test.wantHeight {
+			t.Errorf("CreateTree(%d, %d, %d).GetTreeHeight() = %d, want %d",
+				test.configurationSize, test.id, test.branchFactor, tree.GetTreeHeight(), test.wantHeight)
 		}
 	}
 }

From 376c42c64ac6540b957ac49931c169e4f8b8b9ef Mon Sep 17 00:00:00 2001
From: Hein Meling <hein.meling@uis.no>
Date: Sun, 22 Dec 2024 14:18:45 +0100
Subject: [PATCH 05/21] refactor(trees): remove double storage of tree position
 to ID map

We only need to store the IDs once; the index into the ID slice
represents that ID's position in the tree. I think it will be faster
for small trees to do this approach; we may consider to add a
benchmark to find the breaking point where it is faster to use a
map for this. If a map is needed for larger trees for speed, we
could consider to revert this change, or allocate the map only if
the tree is actually large.
---
 trees/treeconfig.go | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/trees/treeconfig.go b/trees/treeconfig.go
index 4f7deeaa..c44d3060 100644
--- a/trees/treeconfig.go
+++ b/trees/treeconfig.go
@@ -3,6 +3,7 @@ package trees
 import (
 	"fmt"
 	"math"
+	"slices"
 
 	"github.com/relab/hotstuff"
 )
@@ -12,7 +13,6 @@ type Tree struct {
 	id                hotstuff.ID
 	configurationSize int
 	height            int
-	idToPosMapping    map[hotstuff.ID]int
 	posToIDMapping    []hotstuff.ID
 	branchFactor      int
 }
@@ -42,15 +42,16 @@ func (t *Tree) InitializeWithPIDs(ids []hotstuff.ID) error {
 	if len(ids) != t.configurationSize {
 		return fmt.Errorf("invalid number of replicas")
 	}
-	t.posToIDMapping = ids
-	t.idToPosMapping = make(map[hotstuff.ID]int)
-	for index, ID := range ids {
-		if _, ok := t.idToPosMapping[ID]; !ok {
-			t.idToPosMapping[ID] = index
+	// check for duplicate IDs
+	idToIndexMap := make(map[hotstuff.ID]int)
+	for index, id := range ids {
+		if _, ok := idToIndexMap[id]; !ok {
+			idToIndexMap[id] = index
 		} else {
-			return fmt.Errorf("duplicate replica ID")
+			return fmt.Errorf("duplicate replica ID: %d", id)
 		}
 	}
+	t.posToIDMapping = ids
 	return nil
 }
 
@@ -59,16 +60,16 @@ func (t *Tree) GetTreeHeight() int {
 }
 
 func (t *Tree) getPosition() (int, error) {
-	pos, ok := t.idToPosMapping[t.id]
-	if !ok {
+	pos := slices.Index(t.posToIDMapping, t.id)
+	if pos == -1 {
 		return 0, fmt.Errorf("replica not found")
 	}
 	return pos, nil
 }
 
 func (t *Tree) getReplicaPosition(replicaId hotstuff.ID) (int, error) {
-	pos, ok := t.idToPosMapping[replicaId]
-	if !ok {
+	pos := slices.Index(t.posToIDMapping, replicaId)
+	if pos == -1 {
 		return 0, fmt.Errorf("replica not found")
 	}
 	return pos, nil

From 7266f1fe49820ecf30887a32c7660a3c8c670c68 Mon Sep 17 00:00:00 2001
From: hanish gogada <hanishgogadahcu@gmail.com>
Date: Mon, 23 Dec 2024 00:21:27 -0800
Subject: [PATCH 06/21] Addressed review comments and added benchmarks for
 ChildrenOf api

---
 internal/tree/treeconfig.go                 | 155 ++++++++++++++++
 {trees => internal/tree}/treeconfig_test.go | 100 ++++++-----
 trees/treeconfig.go                         | 185 --------------------
 3 files changed, 213 insertions(+), 227 deletions(-)
 create mode 100644 internal/tree/treeconfig.go
 rename {trees => internal/tree}/treeconfig_test.go (71%)
 delete mode 100644 trees/treeconfig.go

diff --git a/internal/tree/treeconfig.go b/internal/tree/treeconfig.go
new file mode 100644
index 00000000..a204a558
--- /dev/null
+++ b/internal/tree/treeconfig.go
@@ -0,0 +1,155 @@
+package tree
+
+import (
+	"math"
+	"slices"
+
+	"github.com/relab/hotstuff"
+)
+
+// Tree implements a fault free tree configuration.
+type Tree struct {
+	id             hotstuff.ID
+	height         int
+	posToIDMapping []hotstuff.ID
+	branchFactor   int
+}
+
+// CreateTree creates the tree configuration, currently only fault free tree configuration is supported.
+func CreateTree(myID hotstuff.ID, bf int, ids []hotstuff.ID) *Tree {
+
+	temp := len(ids)
+	temp = temp - 1 // root
+	height := 1
+	if bf < 2 {
+		panic("branch factor should be greater than 1")
+	}
+	index := slices.Index(ids, myID)
+	if index == -1 {
+		panic("replica ID not found in tree configuration")
+	}
+	for i := 1; temp > 0; i++ {
+		temp = temp - int(math.Pow(float64(bf), float64(i)))
+		height++
+	}
+	return &Tree{
+		id:             myID,
+		height:         height,
+		branchFactor:   bf,
+		posToIDMapping: ids,
+	}
+}
+
+func (t *Tree) GetTreeHeight() int {
+	return t.height
+}
+
+func (t *Tree) replicaPosition(replicaId hotstuff.ID) int {
+	return slices.Index(t.posToIDMapping, replicaId)
+}
+
+// Parent fetches the ID of the parent, if root, returns itself
+// bool value indicates there is no parent for this id
+func (t *Tree) Parent() (hotstuff.ID, bool) {
+	myPos := t.replicaPosition(t.id)
+	if myPos == -1 {
+		return hotstuff.ID(0), false
+	}
+	if myPos == 0 {
+		return t.id, false
+	}
+	parentPos := (myPos - 1) / t.branchFactor
+	return t.posToIDMapping[parentPos], true
+}
+
+// ChildrenOf returns the children of the replica, if any.
+func (t *Tree) ChildrenOf() []hotstuff.ID {
+	return t.ChildrenOfNode(t.id)
+}
+
+func (t *Tree) isWithInIndex(position int) bool {
+	return position < len(t.posToIDMapping)
+}
+
+// IsRoot return true if the replica is at root of the tree.
+func (t *Tree) IsRoot(nodeID hotstuff.ID) bool {
+	return t.replicaPosition(nodeID) == 0
+}
+
+// ChildrenOfNode returns the children of a specific replica.
+func (t *Tree) ChildrenOfNode(nodeID hotstuff.ID) []hotstuff.ID {
+	children := make([]hotstuff.ID, 0)
+	nodePos := t.replicaPosition(nodeID)
+	if nodePos == -1 {
+		return children
+	}
+	for i := 1; i <= t.branchFactor; i++ {
+		childPos := (t.branchFactor * nodePos) + i
+		if t.isWithInIndex(childPos) {
+			children = append(children, t.posToIDMapping[childPos])
+		} else {
+			break
+		}
+	}
+	return children
+}
+
+// getHeight returns the height from the given replica's vantage point.
+func (t *Tree) getHeight(nodeID hotstuff.ID) int {
+	if t.IsRoot(nodeID) {
+		return t.height
+	}
+	nodePos := t.replicaPosition(nodeID)
+	if nodePos == -1 {
+		return 0
+	}
+	startLimit := 0
+	endLimit := 0
+	for i := 1; i < t.height; i++ {
+		startLimit = startLimit + int(math.Pow(float64(t.branchFactor), float64(i-1)))
+		endLimit = endLimit + int(math.Pow(float64(t.branchFactor), float64(i)))
+		if nodePos >= startLimit && nodePos <= endLimit {
+			return t.height - i
+		}
+	}
+	return 0
+}
+
+// GetHeight returns the height of the replica
+func (t *Tree) GetHeight() int {
+	return t.getHeight(t.id)
+}
+
+// PeersOf returns the peers of given ID, if any.
+func (t *Tree) PeersOf(nodeID hotstuff.ID) []hotstuff.ID {
+	peers := make([]hotstuff.ID, 0)
+	if t.IsRoot(nodeID) {
+		return peers
+	}
+	parent, ok := t.Parent()
+	if !ok {
+		return peers
+	}
+	return t.ChildrenOfNode(parent)
+}
+
+// SubTree returns all the nodes of its subtree.
+func (t *Tree) SubTree() []hotstuff.ID {
+	nodeID := t.id
+	subTreeNodes := make([]hotstuff.ID, 0)
+	children := t.ChildrenOfNode(nodeID)
+	queue := make([]hotstuff.ID, 0)
+	queue = append(queue, children...)
+	subTreeNodes = append(subTreeNodes, children...)
+	if len(children) == 0 {
+		return subTreeNodes
+	}
+	for len(queue) > 0 {
+		child := queue[0]
+		queue = queue[1:]
+		children := t.ChildrenOfNode(child)
+		subTreeNodes = append(subTreeNodes, children...)
+		queue = append(queue, children...)
+	}
+	return subTreeNodes
+}
diff --git a/trees/treeconfig_test.go b/internal/tree/treeconfig_test.go
similarity index 71%
rename from trees/treeconfig_test.go
rename to internal/tree/treeconfig_test.go
index f4a708a1..0750d48e 100644
--- a/trees/treeconfig_test.go
+++ b/internal/tree/treeconfig_test.go
@@ -1,4 +1,4 @@
-package trees
+package tree
 
 import (
 	"slices"
@@ -22,7 +22,11 @@ func TestCreateTree(t *testing.T) {
 		{configurationSize: 111, id: 1, branchFactor: 3, wantHeight: 5},
 	}
 	for _, test := range tests {
-		tree := CreateTree(test.configurationSize, test.id, test.branchFactor)
+		ids := make([]hotstuff.ID, test.configurationSize)
+		for i := 0; i < test.configurationSize; i++ {
+			ids[i] = hotstuff.ID(i + 1)
+		}
+		tree := CreateTree(test.id, test.branchFactor, ids)
 		if tree.GetTreeHeight() != test.wantHeight {
 			t.Errorf("CreateTree(%d, %d, %d).GetTreeHeight() = %d, want %d",
 				test.configurationSize, test.id, test.branchFactor, tree.GetTreeHeight(), test.wantHeight)
@@ -30,35 +34,18 @@ func TestCreateTree(t *testing.T) {
 	}
 }
 
-func TestTreeWithNegativeCases(t *testing.T) {
-	tree := CreateTree(10, 1, 2)
-	if tree.GetTreeHeight() != 4 {
-		t.Errorf("Expected height 4, got %d", tree.GetTreeHeight())
-	}
-	if len(tree.GetChildren()) != 0 {
-		t.Errorf("Expected nil, got %v", tree.GetChildren())
-	}
-	if len(tree.GetSubTreeNodes()) != 0 {
-		t.Errorf("Expected nil, got %v", tree.GetSubTreeNodes())
-	}
-	if _, ok := tree.GetParent(); ok {
-		t.Errorf("Expected false, got true")
-	}
-	tree = CreateTree(-1, 1, 2)
-	if tree != nil {
-		t.Errorf("Expected nil, got %v", tree)
-	}
-	ids := []hotstuff.ID{1, 2, 3, 3, 4, 5, 6, 7, 8, 9}
-	tree = CreateTree(10, 1, 2)
-	err := tree.InitializeWithPIDs(ids)
-	if err == nil {
-		t.Errorf("Expected error, got nil")
-	}
-	ids = []hotstuff.ID{1, 2, 3, 4, 5, 6, 7, 8, 9}
-	err = tree.InitializeWithPIDs(ids)
-	if err == nil {
-		t.Errorf("Expected error, got nil")
-	}
+func TestCreateTreeNegativeBF(t *testing.T) {
+	defer func() { _ = recover() }()
+	ids := []hotstuff.ID{1, 2, 3, 4, 5}
+	tree := CreateTree(1, -1, ids)
+	t.Errorf("CreateTree should panic, got %v", tree)
+}
+
+func TestCreateTreeInvalidId(t *testing.T) {
+	defer func() { _ = recover() }()
+	ids := []hotstuff.ID{1, 2, 3, 4, 5}
+	tree := CreateTree(10, 2, ids)
+	t.Errorf("CreateTree should panic, got %v", tree)
 }
 
 type treeConfigTest struct {
@@ -98,29 +85,27 @@ func TestTreeAPIWithInitializeWithPIDs(t *testing.T) {
 		{21, 5, 4, 3, []hotstuff.ID{18, 19, 20, 21}, []hotstuff.ID{18, 19, 20, 21}, 1, false, 2, []hotstuff.ID{2, 3, 4, 5}},
 	}
 	for _, test := range treeConfigTestData {
-		tree := CreateTree(test.configurationSize, test.id, test.branchFactor)
+
 		ids := make([]hotstuff.ID, test.configurationSize)
 		for i := 0; i < test.configurationSize; i++ {
 			ids[i] = hotstuff.ID(i + 1)
 		}
-		if err := tree.InitializeWithPIDs(ids); err != nil {
-			t.Errorf("Expected nil, got %v", err)
-		}
+		tree := CreateTree(test.id, test.branchFactor, ids)
 		if tree.GetTreeHeight() != test.height {
 			t.Errorf("Expected height %d, got %d", test.height, tree.GetTreeHeight())
 		}
-		gotChildren := tree.GetChildren()
+		gotChildren := tree.ChildrenOf()
 		sort.Slice(gotChildren, func(i, j int) bool { return gotChildren[i] < gotChildren[j] })
 		if len(gotChildren) != len(test.children) || !slices.Equal(gotChildren, test.children) {
-			t.Errorf("Expected %v, got %v", test.children, tree.GetChildren())
+			t.Errorf("Expected %v, got %v", test.children, tree.ChildrenOf())
 		}
-		subTree := tree.GetSubTreeNodes()
+		subTree := tree.SubTree()
 		sort.Slice(subTree, func(i, j int) bool { return subTree[i] < subTree[j] })
 		if len(subTree) != len(test.subTreeNodes) ||
 			!slices.Equal(subTree, test.subTreeNodes) {
-			t.Errorf("Expected %v, got %v", test.subTreeNodes, tree.GetSubTreeNodes())
+			t.Errorf("Expected %v, got %v", test.subTreeNodes, tree.SubTree())
 		}
-		if parent, ok := tree.GetParent(); ok {
+		if parent, ok := tree.Parent(); ok {
 			if parent != test.parent {
 				t.Errorf("Expected %d, got %d", test.parent, parent)
 			}
@@ -131,10 +116,41 @@ func TestTreeAPIWithInitializeWithPIDs(t *testing.T) {
 		if tree.GetHeight() != test.replicaHeight {
 			t.Errorf("Expected %d, got %d", test.replicaHeight, tree.GetHeight())
 		}
-		gotPeers := tree.GetPeers(test.id)
+		gotPeers := tree.PeersOf(test.id)
 		sort.Slice(gotPeers, func(i, j int) bool { return gotPeers[i] < gotPeers[j] })
 		if len(gotPeers) != len(test.peers) || !slices.Equal(gotPeers, test.peers) {
-			t.Errorf("Expected %v, got %v", test.peers, tree.GetPeers(test.id))
+			t.Errorf("Expected %v, got %v", test.peers, tree.PeersOf(test.id))
 		}
 	}
 }
+
+func benchmarkGetChildren(size int, bf int, b *testing.B) {
+	ids := make([]hotstuff.ID, size)
+	for i := 0; i < size; i++ {
+		ids[i] = hotstuff.ID(i + 1)
+	}
+	tree := CreateTree(1, bf, ids)
+	for i := 0; i < b.N; i++ {
+		tree.ChildrenOf()
+	}
+}
+
+func BenchmarkGetChildren10(b *testing.B) {
+	benchmarkGetChildren(10, 2, b)
+}
+
+func BenchmarkGetChildren21(b *testing.B) {
+	benchmarkGetChildren(21, 4, b)
+}
+
+func BenchmarkGetChildren111(b *testing.B) {
+	benchmarkGetChildren(111, 10, b)
+}
+
+func BenchmarkGetChildren211(b *testing.B) {
+	benchmarkGetChildren(211, 14, b)
+}
+
+func BenchmarkGetChildren421(b *testing.B) {
+	benchmarkGetChildren(421, 20, b)
+}
diff --git a/trees/treeconfig.go b/trees/treeconfig.go
deleted file mode 100644
index c44d3060..00000000
--- a/trees/treeconfig.go
+++ /dev/null
@@ -1,185 +0,0 @@
-package trees
-
-import (
-	"fmt"
-	"math"
-	"slices"
-
-	"github.com/relab/hotstuff"
-)
-
-// Tree implements a fault free tree configuration.
-type Tree struct {
-	id                hotstuff.ID
-	configurationSize int
-	height            int
-	posToIDMapping    []hotstuff.ID
-	branchFactor      int
-}
-
-// CreateTree creates the tree configuration, currently only fault free tree configuration is supported.
-func CreateTree(configurationSize int, myID hotstuff.ID, bf int) *Tree {
-	if configurationSize <= 0 {
-		return nil
-	}
-	temp := configurationSize
-	temp = temp - 1 // root
-	height := 1
-	for i := 1; temp > 0; i++ {
-		temp = temp - int(math.Pow(float64(bf), float64(i)))
-		height++
-	}
-	return &Tree{
-		id:                myID,
-		configurationSize: configurationSize,
-		height:            height,
-		branchFactor:      bf,
-	}
-}
-
-// InitializeWithPIDs uses the map to initialize the position of replicas.
-func (t *Tree) InitializeWithPIDs(ids []hotstuff.ID) error {
-	if len(ids) != t.configurationSize {
-		return fmt.Errorf("invalid number of replicas")
-	}
-	// check for duplicate IDs
-	idToIndexMap := make(map[hotstuff.ID]int)
-	for index, id := range ids {
-		if _, ok := idToIndexMap[id]; !ok {
-			idToIndexMap[id] = index
-		} else {
-			return fmt.Errorf("duplicate replica ID: %d", id)
-		}
-	}
-	t.posToIDMapping = ids
-	return nil
-}
-
-func (t *Tree) GetTreeHeight() int {
-	return t.height
-}
-
-func (t *Tree) getPosition() (int, error) {
-	pos := slices.Index(t.posToIDMapping, t.id)
-	if pos == -1 {
-		return 0, fmt.Errorf("replica not found")
-	}
-	return pos, nil
-}
-
-func (t *Tree) getReplicaPosition(replicaId hotstuff.ID) (int, error) {
-	pos := slices.Index(t.posToIDMapping, replicaId)
-	if pos == -1 {
-		return 0, fmt.Errorf("replica not found")
-	}
-	return pos, nil
-}
-
-// GetParent fetches the ID of the parent, if root, returns itself.
-func (t *Tree) GetParent() (hotstuff.ID, bool) {
-	myPos, err := t.getPosition()
-	if err != nil {
-		return hotstuff.ID(0), false
-	}
-	if myPos == 0 {
-		return t.id, false
-	}
-	return t.posToIDMapping[(myPos-1)/t.branchFactor], true
-}
-
-// GetChildren returns the children of the replicas, if any.
-func (t *Tree) GetChildren() []hotstuff.ID {
-	return t.GetChildrenOfNode(t.id)
-}
-
-func (t *Tree) isWithInIndex(position int) bool {
-	return position < t.configurationSize
-}
-
-// IsRoot return true if the replica is at root of the tree.
-func (t *Tree) IsRoot(nodeID hotstuff.ID) bool {
-	pos, err := t.getReplicaPosition(nodeID)
-	if err != nil {
-		return false
-	}
-	return pos == 0
-}
-
-// GetChildrenOfNode returns the children of a specific replica.
-func (t *Tree) GetChildrenOfNode(nodeID hotstuff.ID) []hotstuff.ID {
-	children := make([]hotstuff.ID, 0)
-	nodePos, err := t.getReplicaPosition(nodeID)
-	if err != nil {
-		return children
-	}
-	for i := 1; i <= t.branchFactor; i++ {
-		childPos := (t.branchFactor * nodePos) + i
-		if t.isWithInIndex(childPos) {
-			children = append(children, t.posToIDMapping[childPos])
-		} else {
-			break
-		}
-	}
-	return children
-}
-
-// getHeight returns the height of a given replica.
-func (t *Tree) getHeight(nodeID hotstuff.ID) int {
-	if t.IsRoot(nodeID) {
-		return t.height
-	}
-	nodePos, err := t.getReplicaPosition(nodeID)
-	if err != nil {
-		return 0
-	}
-	startLimit := 0
-	endLimit := 0
-	for i := 1; i < t.height; i++ {
-		startLimit = startLimit + int(math.Pow(float64(t.branchFactor), float64(i-1)))
-		endLimit = endLimit + int(math.Pow(float64(t.branchFactor), float64(i)))
-		if nodePos >= startLimit && nodePos <= endLimit {
-			return t.height - i
-		}
-	}
-	return 0
-}
-
-// GetHeight returns the height of the replica
-func (t *Tree) GetHeight() int {
-	return t.getHeight(t.id)
-}
-
-// GetPeers returns the peers of given ID, if any.
-func (t *Tree) GetPeers(nodeID hotstuff.ID) []hotstuff.ID {
-	peers := make([]hotstuff.ID, 0)
-	if t.IsRoot(nodeID) {
-		return peers
-	}
-	parent, ok := t.GetParent()
-	if !ok {
-		return peers
-	}
-	parentChildren := t.GetChildrenOfNode(parent)
-	return parentChildren
-}
-
-// GetSubTreeNodes returns all the nodes of its subtree.
-func (t *Tree) GetSubTreeNodes() []hotstuff.ID {
-	nodeID := t.id
-	subTreeNodes := make([]hotstuff.ID, 0)
-	children := t.GetChildrenOfNode(nodeID)
-	queue := make([]hotstuff.ID, 0)
-	queue = append(queue, children...)
-	subTreeNodes = append(subTreeNodes, children...)
-	if len(children) == 0 {
-		return subTreeNodes
-	}
-	for len(queue) > 0 {
-		child := queue[0]
-		queue = queue[1:]
-		children := t.GetChildrenOfNode(child)
-		subTreeNodes = append(subTreeNodes, children...)
-		queue = append(queue, children...)
-	}
-	return subTreeNodes
-}

From 30a1ae560b8bdf5d42fce726c90bfc9f5b163ca2 Mon Sep 17 00:00:00 2001
From: Hein Meling <hein.meling@uis.no>
Date: Tue, 24 Dec 2024 21:59:59 +0100
Subject: [PATCH 07/21] fix(tree): do the panicking first on bad input then
 compute height

This moves the code to compute the height together rather than
splitting it with code for argument checks.

This makes it easier to see that computing the height could be moved
to a external function if it was ever necessary to do separately.
---
 internal/tree/treeconfig.go | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/internal/tree/treeconfig.go b/internal/tree/treeconfig.go
index a204a558..4b17421c 100644
--- a/internal/tree/treeconfig.go
+++ b/internal/tree/treeconfig.go
@@ -17,17 +17,15 @@ type Tree struct {
 
 // CreateTree creates the tree configuration, currently only fault free tree configuration is supported.
 func CreateTree(myID hotstuff.ID, bf int, ids []hotstuff.ID) *Tree {
-
-	temp := len(ids)
-	temp = temp - 1 // root
-	height := 1
 	if bf < 2 {
-		panic("branch factor should be greater than 1")
+		panic("Branch factor must be greater than 1")
 	}
-	index := slices.Index(ids, myID)
-	if index == -1 {
-		panic("replica ID not found in tree configuration")
+	if slices.Index(ids, myID) == -1 {
+		panic("Replica ID not found in tree configuration")
 	}
+
+	temp := len(ids) - 1 // root
+	height := 1
 	for i := 1; temp > 0; i++ {
 		temp = temp - int(math.Pow(float64(bf), float64(i)))
 		height++

From 1871d1dcc2b870f0e594733636eea164d9bc4e17 Mon Sep 17 00:00:00 2001
From: Hein Meling <hein.meling@uis.no>
Date: Tue, 24 Dec 2024 22:06:04 +0100
Subject: [PATCH 08/21] refactor(tree): rename GetTreeHeight to TreeHeight

In Go we try to avoid using the Get prefix.
---
 internal/tree/treeconfig.go      |  7 ++++---
 internal/tree/treeconfig_test.go | 11 +++++------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/internal/tree/treeconfig.go b/internal/tree/treeconfig.go
index 4b17421c..d20c555a 100644
--- a/internal/tree/treeconfig.go
+++ b/internal/tree/treeconfig.go
@@ -38,12 +38,13 @@ func CreateTree(myID hotstuff.ID, bf int, ids []hotstuff.ID) *Tree {
 	}
 }
 
-func (t *Tree) GetTreeHeight() int {
+// TreeHeight returns the height of the full tree.
+func (t *Tree) TreeHeight() int {
 	return t.height
 }
 
-func (t *Tree) replicaPosition(replicaId hotstuff.ID) int {
-	return slices.Index(t.posToIDMapping, replicaId)
+func (t *Tree) replicaPosition(id hotstuff.ID) int {
+	return slices.Index(t.posToIDMapping, id)
 }
 
 // Parent fetches the ID of the parent, if root, returns itself
diff --git a/internal/tree/treeconfig_test.go b/internal/tree/treeconfig_test.go
index 0750d48e..7a949535 100644
--- a/internal/tree/treeconfig_test.go
+++ b/internal/tree/treeconfig_test.go
@@ -27,9 +27,9 @@ func TestCreateTree(t *testing.T) {
 			ids[i] = hotstuff.ID(i + 1)
 		}
 		tree := CreateTree(test.id, test.branchFactor, ids)
-		if tree.GetTreeHeight() != test.wantHeight {
+		if tree.TreeHeight() != test.wantHeight {
 			t.Errorf("CreateTree(%d, %d, %d).GetTreeHeight() = %d, want %d",
-				test.configurationSize, test.id, test.branchFactor, tree.GetTreeHeight(), test.wantHeight)
+				test.configurationSize, test.id, test.branchFactor, tree.TreeHeight(), test.wantHeight)
 		}
 	}
 }
@@ -41,7 +41,7 @@ func TestCreateTreeNegativeBF(t *testing.T) {
 	t.Errorf("CreateTree should panic, got %v", tree)
 }
 
-func TestCreateTreeInvalidId(t *testing.T) {
+func TestCreateTreeInvalidID(t *testing.T) {
 	defer func() { _ = recover() }()
 	ids := []hotstuff.ID{1, 2, 3, 4, 5}
 	tree := CreateTree(10, 2, ids)
@@ -85,14 +85,13 @@ func TestTreeAPIWithInitializeWithPIDs(t *testing.T) {
 		{21, 5, 4, 3, []hotstuff.ID{18, 19, 20, 21}, []hotstuff.ID{18, 19, 20, 21}, 1, false, 2, []hotstuff.ID{2, 3, 4, 5}},
 	}
 	for _, test := range treeConfigTestData {
-
 		ids := make([]hotstuff.ID, test.configurationSize)
 		for i := 0; i < test.configurationSize; i++ {
 			ids[i] = hotstuff.ID(i + 1)
 		}
 		tree := CreateTree(test.id, test.branchFactor, ids)
-		if tree.GetTreeHeight() != test.height {
-			t.Errorf("Expected height %d, got %d", test.height, tree.GetTreeHeight())
+		if tree.TreeHeight() != test.height {
+			t.Errorf("Expected height %d, got %d", test.height, tree.TreeHeight())
 		}
 		gotChildren := tree.ChildrenOf()
 		sort.Slice(gotChildren, func(i, j int) bool { return gotChildren[i] < gotChildren[j] })

From 64b3d234651ec0f5136d52c5081473db71d212ae Mon Sep 17 00:00:00 2001
From: Hein Meling <hein.meling@uis.no>
Date: Tue, 24 Dec 2024 22:13:47 +0100
Subject: [PATCH 09/21] fix(tree): removed unnecessary check for -1

We don't need to check that t.id's position isn't -1 since we do
that check in CreateTree, so we would never get -1 here.
---
 internal/tree/treeconfig.go | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/internal/tree/treeconfig.go b/internal/tree/treeconfig.go
index d20c555a..9f07eb11 100644
--- a/internal/tree/treeconfig.go
+++ b/internal/tree/treeconfig.go
@@ -47,13 +47,10 @@ func (t *Tree) replicaPosition(id hotstuff.ID) int {
 	return slices.Index(t.posToIDMapping, id)
 }
 
-// Parent fetches the ID of the parent, if root, returns itself
-// bool value indicates there is no parent for this id
+// Parent returns the ID of the parent and true. If this tree's node ID is the root,
+// the root's ID is returned and false to indicate it does not have a parent.
 func (t *Tree) Parent() (hotstuff.ID, bool) {
 	myPos := t.replicaPosition(t.id)
-	if myPos == -1 {
-		return hotstuff.ID(0), false
-	}
 	if myPos == 0 {
 		return t.id, false
 	}

From c29ed18f92f7b2926912b9eda909ad343a719292 Mon Sep 17 00:00:00 2001
From: Hein Meling <hein.meling@uis.no>
Date: Tue, 24 Dec 2024 22:18:35 +0100
Subject: [PATCH 10/21] refactor(tree): rename to ChildrenOf(node) and
 NodeChildren()

NodeChildren returns this tree's replica's children, while the
ChildrenOf(node) returns the node's children.

This also removes an unnecessary allocation; we can return nil
instead of an empty slice of IDs.
---
 internal/tree/treeconfig.go | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/internal/tree/treeconfig.go b/internal/tree/treeconfig.go
index 9f07eb11..badab818 100644
--- a/internal/tree/treeconfig.go
+++ b/internal/tree/treeconfig.go
@@ -58,11 +58,6 @@ func (t *Tree) Parent() (hotstuff.ID, bool) {
 	return t.posToIDMapping[parentPos], true
 }
 
-// ChildrenOf returns the children of the replica, if any.
-func (t *Tree) ChildrenOf() []hotstuff.ID {
-	return t.ChildrenOfNode(t.id)
-}
-
 func (t *Tree) isWithInIndex(position int) bool {
 	return position < len(t.posToIDMapping)
 }
@@ -72,13 +67,18 @@ func (t *Tree) IsRoot(nodeID hotstuff.ID) bool {
 	return t.replicaPosition(nodeID) == 0
 }
 
-// ChildrenOfNode returns the children of a specific replica.
-func (t *Tree) ChildrenOfNode(nodeID hotstuff.ID) []hotstuff.ID {
-	children := make([]hotstuff.ID, 0)
+// NodeChildren returns the children of this tree's replica, if any.
+func (t *Tree) NodeChildren() []hotstuff.ID {
+	return t.ChildrenOf(t.id)
+}
+
+// ChildrenOf returns the children of a specific replica.
+func (t *Tree) ChildrenOf(nodeID hotstuff.ID) []hotstuff.ID {
 	nodePos := t.replicaPosition(nodeID)
 	if nodePos == -1 {
-		return children
+		return nil
 	}
+	children := make([]hotstuff.ID, 0)
 	for i := 1; i <= t.branchFactor; i++ {
 		childPos := (t.branchFactor * nodePos) + i
 		if t.isWithInIndex(childPos) {

From 5f92d281f07cc1a80ea7892cbd2c37376fa451b3 Mon Sep 17 00:00:00 2001
From: Hein Meling <hein.meling@uis.no>
Date: Tue, 24 Dec 2024 22:23:05 +0100
Subject: [PATCH 11/21] refactor(tree): rename GetHeight to NodeHeight

NodeHeight returns the height of this tree's replica's height.
---
 internal/tree/treeconfig.go | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/internal/tree/treeconfig.go b/internal/tree/treeconfig.go
index badab818..d4594ab7 100644
--- a/internal/tree/treeconfig.go
+++ b/internal/tree/treeconfig.go
@@ -90,8 +90,8 @@ func (t *Tree) ChildrenOf(nodeID hotstuff.ID) []hotstuff.ID {
 	return children
 }
 
-// getHeight returns the height from the given replica's vantage point.
-func (t *Tree) getHeight(nodeID hotstuff.ID) int {
+// heightOf returns the height from the given replica's vantage point.
+func (t *Tree) heightOf(nodeID hotstuff.ID) int {
 	if t.IsRoot(nodeID) {
 		return t.height
 	}
@@ -111,9 +111,9 @@ func (t *Tree) getHeight(nodeID hotstuff.ID) int {
 	return 0
 }
 
-// GetHeight returns the height of the replica
-func (t *Tree) GetHeight() int {
-	return t.getHeight(t.id)
+// NodeHeight returns the height of this tree's replica.
+func (t *Tree) NodeHeight() int {
+	return t.heightOf(t.id)
 }
 
 // PeersOf returns the peers of given ID, if any.

From ad4f140ad378c1bd97d4d3cf07ca88892abc2430 Mon Sep 17 00:00:00 2001
From: Hein Meling <hein.meling@uis.no>
Date: Tue, 24 Dec 2024 22:27:52 +0100
Subject: [PATCH 12/21] refactor(tree): avoid allocation of empty slice of IDs

---
 internal/tree/treeconfig.go | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/internal/tree/treeconfig.go b/internal/tree/treeconfig.go
index d4594ab7..b458ff5c 100644
--- a/internal/tree/treeconfig.go
+++ b/internal/tree/treeconfig.go
@@ -116,17 +116,16 @@ func (t *Tree) NodeHeight() int {
 	return t.heightOf(t.id)
 }
 
-// PeersOf returns the peers of given ID, if any.
+// PeersOf returns the sibling peers of given ID, if any.
 func (t *Tree) PeersOf(nodeID hotstuff.ID) []hotstuff.ID {
-	peers := make([]hotstuff.ID, 0)
 	if t.IsRoot(nodeID) {
-		return peers
+		return nil
 	}
 	parent, ok := t.Parent()
 	if !ok {
-		return peers
+		return nil
 	}
-	return t.ChildrenOfNode(parent)
+	return t.ChildrenOf(parent)
 }
 
 // SubTree returns all the nodes of its subtree.

From 8001f10759944d700e785ee9228594da25895ab9 Mon Sep 17 00:00:00 2001
From: Hein Meling <hein.meling@uis.no>
Date: Tue, 24 Dec 2024 22:29:23 +0100
Subject: [PATCH 13/21] refactor(tree): simplified SubTree implementation

---
 internal/tree/treeconfig.go      | 15 ++++++---------
 internal/tree/treeconfig_test.go | 10 +++++-----
 2 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/internal/tree/treeconfig.go b/internal/tree/treeconfig.go
index b458ff5c..616796e2 100644
--- a/internal/tree/treeconfig.go
+++ b/internal/tree/treeconfig.go
@@ -128,21 +128,18 @@ func (t *Tree) PeersOf(nodeID hotstuff.ID) []hotstuff.ID {
 	return t.ChildrenOf(parent)
 }
 
-// SubTree returns all the nodes of its subtree.
+// SubTree returns all subtree nodes of this tree's replica.
 func (t *Tree) SubTree() []hotstuff.ID {
-	nodeID := t.id
-	subTreeNodes := make([]hotstuff.ID, 0)
-	children := t.ChildrenOfNode(nodeID)
-	queue := make([]hotstuff.ID, 0)
-	queue = append(queue, children...)
-	subTreeNodes = append(subTreeNodes, children...)
+	children := t.ChildrenOf(t.id)
 	if len(children) == 0 {
-		return subTreeNodes
+		return nil
 	}
+	subTreeNodes := slices.Clone(children)
+	queue := slices.Clone(children)
 	for len(queue) > 0 {
 		child := queue[0]
 		queue = queue[1:]
-		children := t.ChildrenOfNode(child)
+		children := t.ChildrenOf(child)
 		subTreeNodes = append(subTreeNodes, children...)
 		queue = append(queue, children...)
 	}
diff --git a/internal/tree/treeconfig_test.go b/internal/tree/treeconfig_test.go
index 7a949535..86aff96a 100644
--- a/internal/tree/treeconfig_test.go
+++ b/internal/tree/treeconfig_test.go
@@ -93,10 +93,10 @@ func TestTreeAPIWithInitializeWithPIDs(t *testing.T) {
 		if tree.TreeHeight() != test.height {
 			t.Errorf("Expected height %d, got %d", test.height, tree.TreeHeight())
 		}
-		gotChildren := tree.ChildrenOf()
+		gotChildren := tree.NodeChildren()
 		sort.Slice(gotChildren, func(i, j int) bool { return gotChildren[i] < gotChildren[j] })
 		if len(gotChildren) != len(test.children) || !slices.Equal(gotChildren, test.children) {
-			t.Errorf("Expected %v, got %v", test.children, tree.ChildrenOf())
+			t.Errorf("Expected %v, got %v", test.children, tree.NodeChildren())
 		}
 		subTree := tree.SubTree()
 		sort.Slice(subTree, func(i, j int) bool { return subTree[i] < subTree[j] })
@@ -112,8 +112,8 @@ func TestTreeAPIWithInitializeWithPIDs(t *testing.T) {
 		if tree.IsRoot(test.id) != test.isRoot {
 			t.Errorf("Expected %t, got %t", test.isRoot, tree.IsRoot(test.id))
 		}
-		if tree.GetHeight() != test.replicaHeight {
-			t.Errorf("Expected %d, got %d", test.replicaHeight, tree.GetHeight())
+		if tree.NodeHeight() != test.replicaHeight {
+			t.Errorf("Expected %d, got %d", test.replicaHeight, tree.NodeHeight())
 		}
 		gotPeers := tree.PeersOf(test.id)
 		sort.Slice(gotPeers, func(i, j int) bool { return gotPeers[i] < gotPeers[j] })
@@ -130,7 +130,7 @@ func benchmarkGetChildren(size int, bf int, b *testing.B) {
 	}
 	tree := CreateTree(1, bf, ids)
 	for i := 0; i < b.N; i++ {
-		tree.ChildrenOf()
+		tree.NodeChildren()
 	}
 }
 

From 8f780f9c07c83bf054c647f506f7fa10c8cb9bbe Mon Sep 17 00:00:00 2001
From: Hein Meling <hein.meling@uis.no>
Date: Tue, 24 Dec 2024 22:35:02 +0100
Subject: [PATCH 14/21] doc(tree): added a comment about computing height of
 the tree

---
 internal/tree/treeconfig.go | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/internal/tree/treeconfig.go b/internal/tree/treeconfig.go
index 616796e2..04a19a7d 100644
--- a/internal/tree/treeconfig.go
+++ b/internal/tree/treeconfig.go
@@ -15,7 +15,8 @@ type Tree struct {
 	branchFactor   int
 }
 
-// CreateTree creates the tree configuration, currently only fault free tree configuration is supported.
+// CreateTree creates the tree configuration.
+// Currently only fault free tree configuration is supported.
 func CreateTree(myID hotstuff.ID, bf int, ids []hotstuff.ID) *Tree {
 	if bf < 2 {
 		panic("Branch factor must be greater than 1")
@@ -24,6 +25,7 @@ func CreateTree(myID hotstuff.ID, bf int, ids []hotstuff.ID) *Tree {
 		panic("Replica ID not found in tree configuration")
 	}
 
+	// compute height of the tree
 	temp := len(ids) - 1 // root
 	height := 1
 	for i := 1; temp > 0; i++ {

From 6e02d5f2f29ab69c83c9ed820abcde2ec3ca077d Mon Sep 17 00:00:00 2001
From: Hein Meling <hein.meling@uis.no>
Date: Tue, 24 Dec 2024 22:57:41 +0100
Subject: [PATCH 15/21] refactor(tree): rename Node/node methods/vars to
 Replica/replica

---
 internal/tree/treeconfig.go      | 47 ++++++++++++++++----------------
 internal/tree/treeconfig_test.go | 18 ++++++------
 2 files changed, 33 insertions(+), 32 deletions(-)

diff --git a/internal/tree/treeconfig.go b/internal/tree/treeconfig.go
index 04a19a7d..9a3e06ee 100644
--- a/internal/tree/treeconfig.go
+++ b/internal/tree/treeconfig.go
@@ -49,8 +49,9 @@ func (t *Tree) replicaPosition(id hotstuff.ID) int {
 	return slices.Index(t.posToIDMapping, id)
 }
 
-// Parent returns the ID of the parent and true. If this tree's node ID is the root,
-// the root's ID is returned and false to indicate it does not have a parent.
+// Parent returns the ID of the parent of this tree's replica and true.
+// If this tree's replica is the root, the root's ID is returned
+// and false to indicate it does not have a parent.
 func (t *Tree) Parent() (hotstuff.ID, bool) {
 	myPos := t.replicaPosition(t.id)
 	if myPos == 0 {
@@ -65,24 +66,24 @@ func (t *Tree) isWithInIndex(position int) bool {
 }
 
 // IsRoot return true if the replica is at root of the tree.
-func (t *Tree) IsRoot(nodeID hotstuff.ID) bool {
-	return t.replicaPosition(nodeID) == 0
+func (t *Tree) IsRoot(replicaID hotstuff.ID) bool {
+	return t.replicaPosition(replicaID) == 0
 }
 
-// NodeChildren returns the children of this tree's replica, if any.
-func (t *Tree) NodeChildren() []hotstuff.ID {
+// ReplicaChildren returns the children of this tree's replica, if any.
+func (t *Tree) ReplicaChildren() []hotstuff.ID {
 	return t.ChildrenOf(t.id)
 }
 
 // ChildrenOf returns the children of a specific replica.
-func (t *Tree) ChildrenOf(nodeID hotstuff.ID) []hotstuff.ID {
-	nodePos := t.replicaPosition(nodeID)
-	if nodePos == -1 {
+func (t *Tree) ChildrenOf(replicaID hotstuff.ID) []hotstuff.ID {
+	replicaPos := t.replicaPosition(replicaID)
+	if replicaPos == -1 {
 		return nil
 	}
 	children := make([]hotstuff.ID, 0)
 	for i := 1; i <= t.branchFactor; i++ {
-		childPos := (t.branchFactor * nodePos) + i
+		childPos := (t.branchFactor * replicaPos) + i
 		if t.isWithInIndex(childPos) {
 			children = append(children, t.posToIDMapping[childPos])
 		} else {
@@ -93,12 +94,12 @@ func (t *Tree) ChildrenOf(nodeID hotstuff.ID) []hotstuff.ID {
 }
 
 // heightOf returns the height from the given replica's vantage point.
-func (t *Tree) heightOf(nodeID hotstuff.ID) int {
-	if t.IsRoot(nodeID) {
+func (t *Tree) heightOf(replicaID hotstuff.ID) int {
+	if t.IsRoot(replicaID) {
 		return t.height
 	}
-	nodePos := t.replicaPosition(nodeID)
-	if nodePos == -1 {
+	replicaPos := t.replicaPosition(replicaID)
+	if replicaPos == -1 {
 		return 0
 	}
 	startLimit := 0
@@ -106,21 +107,21 @@ func (t *Tree) heightOf(nodeID hotstuff.ID) int {
 	for i := 1; i < t.height; i++ {
 		startLimit = startLimit + int(math.Pow(float64(t.branchFactor), float64(i-1)))
 		endLimit = endLimit + int(math.Pow(float64(t.branchFactor), float64(i)))
-		if nodePos >= startLimit && nodePos <= endLimit {
+		if replicaPos >= startLimit && replicaPos <= endLimit {
 			return t.height - i
 		}
 	}
 	return 0
 }
 
-// NodeHeight returns the height of this tree's replica.
-func (t *Tree) NodeHeight() int {
+// ReplicaHeight returns the height of this tree's replica.
+func (t *Tree) ReplicaHeight() int {
 	return t.heightOf(t.id)
 }
 
 // PeersOf returns the sibling peers of given ID, if any.
-func (t *Tree) PeersOf(nodeID hotstuff.ID) []hotstuff.ID {
-	if t.IsRoot(nodeID) {
+func (t *Tree) PeersOf(replicaID hotstuff.ID) []hotstuff.ID {
+	if t.IsRoot(replicaID) {
 		return nil
 	}
 	parent, ok := t.Parent()
@@ -130,20 +131,20 @@ func (t *Tree) PeersOf(nodeID hotstuff.ID) []hotstuff.ID {
 	return t.ChildrenOf(parent)
 }
 
-// SubTree returns all subtree nodes of this tree's replica.
+// SubTree returns all subtree replicas of this tree's replica.
 func (t *Tree) SubTree() []hotstuff.ID {
 	children := t.ChildrenOf(t.id)
 	if len(children) == 0 {
 		return nil
 	}
-	subTreeNodes := slices.Clone(children)
+	subTreeReplicas := slices.Clone(children)
 	queue := slices.Clone(children)
 	for len(queue) > 0 {
 		child := queue[0]
 		queue = queue[1:]
 		children := t.ChildrenOf(child)
-		subTreeNodes = append(subTreeNodes, children...)
+		subTreeReplicas = append(subTreeReplicas, children...)
 		queue = append(queue, children...)
 	}
-	return subTreeNodes
+	return subTreeReplicas
 }
diff --git a/internal/tree/treeconfig_test.go b/internal/tree/treeconfig_test.go
index 86aff96a..d783b593 100644
--- a/internal/tree/treeconfig_test.go
+++ b/internal/tree/treeconfig_test.go
@@ -54,7 +54,7 @@ type treeConfigTest struct {
 	branchFactor      int
 	height            int
 	children          []hotstuff.ID
-	subTreeNodes      []hotstuff.ID
+	subTreeReplicas   []hotstuff.ID
 	parent            hotstuff.ID
 	isRoot            bool
 	replicaHeight     int
@@ -93,16 +93,16 @@ func TestTreeAPIWithInitializeWithPIDs(t *testing.T) {
 		if tree.TreeHeight() != test.height {
 			t.Errorf("Expected height %d, got %d", test.height, tree.TreeHeight())
 		}
-		gotChildren := tree.NodeChildren()
+		gotChildren := tree.ReplicaChildren()
 		sort.Slice(gotChildren, func(i, j int) bool { return gotChildren[i] < gotChildren[j] })
 		if len(gotChildren) != len(test.children) || !slices.Equal(gotChildren, test.children) {
-			t.Errorf("Expected %v, got %v", test.children, tree.NodeChildren())
+			t.Errorf("Expected %v, got %v", test.children, tree.ReplicaChildren())
 		}
 		subTree := tree.SubTree()
 		sort.Slice(subTree, func(i, j int) bool { return subTree[i] < subTree[j] })
-		if len(subTree) != len(test.subTreeNodes) ||
-			!slices.Equal(subTree, test.subTreeNodes) {
-			t.Errorf("Expected %v, got %v", test.subTreeNodes, tree.SubTree())
+		if len(subTree) != len(test.subTreeReplicas) ||
+			!slices.Equal(subTree, test.subTreeReplicas) {
+			t.Errorf("Expected %v, got %v", test.subTreeReplicas, tree.SubTree())
 		}
 		if parent, ok := tree.Parent(); ok {
 			if parent != test.parent {
@@ -112,8 +112,8 @@ func TestTreeAPIWithInitializeWithPIDs(t *testing.T) {
 		if tree.IsRoot(test.id) != test.isRoot {
 			t.Errorf("Expected %t, got %t", test.isRoot, tree.IsRoot(test.id))
 		}
-		if tree.NodeHeight() != test.replicaHeight {
-			t.Errorf("Expected %d, got %d", test.replicaHeight, tree.NodeHeight())
+		if tree.ReplicaHeight() != test.replicaHeight {
+			t.Errorf("Expected %d, got %d", test.replicaHeight, tree.ReplicaHeight())
 		}
 		gotPeers := tree.PeersOf(test.id)
 		sort.Slice(gotPeers, func(i, j int) bool { return gotPeers[i] < gotPeers[j] })
@@ -130,7 +130,7 @@ func benchmarkGetChildren(size int, bf int, b *testing.B) {
 	}
 	tree := CreateTree(1, bf, ids)
 	for i := 0; i < b.N; i++ {
-		tree.NodeChildren()
+		tree.ReplicaChildren()
 	}
 }
 

From 89457180dc4ba74cbf1247754c4cc83c11b155c9 Mon Sep 17 00:00:00 2001
From: Hein Meling <hein.meling@uis.no>
Date: Tue, 24 Dec 2024 22:59:45 +0100
Subject: [PATCH 16/21] chore(tree): moved private helper funcs to bottom of
 file

---
 internal/tree/treeconfig.go | 58 ++++++++++++++++++-------------------
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/internal/tree/treeconfig.go b/internal/tree/treeconfig.go
index 9a3e06ee..82c1b077 100644
--- a/internal/tree/treeconfig.go
+++ b/internal/tree/treeconfig.go
@@ -45,10 +45,6 @@ func (t *Tree) TreeHeight() int {
 	return t.height
 }
 
-func (t *Tree) replicaPosition(id hotstuff.ID) int {
-	return slices.Index(t.posToIDMapping, id)
-}
-
 // Parent returns the ID of the parent of this tree's replica and true.
 // If this tree's replica is the root, the root's ID is returned
 // and false to indicate it does not have a parent.
@@ -61,10 +57,6 @@ func (t *Tree) Parent() (hotstuff.ID, bool) {
 	return t.posToIDMapping[parentPos], true
 }
 
-func (t *Tree) isWithInIndex(position int) bool {
-	return position < len(t.posToIDMapping)
-}
-
 // IsRoot return true if the replica is at root of the tree.
 func (t *Tree) IsRoot(replicaID hotstuff.ID) bool {
 	return t.replicaPosition(replicaID) == 0
@@ -93,27 +85,6 @@ func (t *Tree) ChildrenOf(replicaID hotstuff.ID) []hotstuff.ID {
 	return children
 }
 
-// heightOf returns the height from the given replica's vantage point.
-func (t *Tree) heightOf(replicaID hotstuff.ID) int {
-	if t.IsRoot(replicaID) {
-		return t.height
-	}
-	replicaPos := t.replicaPosition(replicaID)
-	if replicaPos == -1 {
-		return 0
-	}
-	startLimit := 0
-	endLimit := 0
-	for i := 1; i < t.height; i++ {
-		startLimit = startLimit + int(math.Pow(float64(t.branchFactor), float64(i-1)))
-		endLimit = endLimit + int(math.Pow(float64(t.branchFactor), float64(i)))
-		if replicaPos >= startLimit && replicaPos <= endLimit {
-			return t.height - i
-		}
-	}
-	return 0
-}
-
 // ReplicaHeight returns the height of this tree's replica.
 func (t *Tree) ReplicaHeight() int {
 	return t.heightOf(t.id)
@@ -148,3 +119,32 @@ func (t *Tree) SubTree() []hotstuff.ID {
 	}
 	return subTreeReplicas
 }
+
+// heightOf returns the height from the given replica's vantage point.
+func (t *Tree) heightOf(replicaID hotstuff.ID) int {
+	if t.IsRoot(replicaID) {
+		return t.height
+	}
+	replicaPos := t.replicaPosition(replicaID)
+	if replicaPos == -1 {
+		return 0
+	}
+	startLimit := 0
+	endLimit := 0
+	for i := 1; i < t.height; i++ {
+		startLimit = startLimit + int(math.Pow(float64(t.branchFactor), float64(i-1)))
+		endLimit = endLimit + int(math.Pow(float64(t.branchFactor), float64(i)))
+		if replicaPos >= startLimit && replicaPos <= endLimit {
+			return t.height - i
+		}
+	}
+	return 0
+}
+
+func (t *Tree) replicaPosition(id hotstuff.ID) int {
+	return slices.Index(t.posToIDMapping, id)
+}
+
+func (t *Tree) isWithInIndex(position int) bool {
+	return position < len(t.posToIDMapping)
+}

From e470824b834d5d92d5965b8303bf6528f80dcf60 Mon Sep 17 00:00:00 2001
From: Hein Meling <hein.meling@uis.no>
Date: Tue, 24 Dec 2024 23:26:21 +0100
Subject: [PATCH 17/21] refactor(tree): allocate less in SubTree() method

---
 internal/tree/treeconfig.go | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/internal/tree/treeconfig.go b/internal/tree/treeconfig.go
index 82c1b077..714cfcaa 100644
--- a/internal/tree/treeconfig.go
+++ b/internal/tree/treeconfig.go
@@ -108,14 +108,12 @@ func (t *Tree) SubTree() []hotstuff.ID {
 	if len(children) == 0 {
 		return nil
 	}
-	subTreeReplicas := slices.Clone(children)
-	queue := slices.Clone(children)
-	for len(queue) > 0 {
-		child := queue[0]
-		queue = queue[1:]
-		children := t.ChildrenOf(child)
-		subTreeReplicas = append(subTreeReplicas, children...)
-		queue = append(queue, children...)
+	subTreeReplicas := make([]hotstuff.ID, len(children))
+	copy(subTreeReplicas, children)
+	for i := 0; i < len(subTreeReplicas); i++ {
+		node := subTreeReplicas[i]
+		newChildren := t.ChildrenOf(node)
+		subTreeReplicas = append(subTreeReplicas, newChildren...)
 	}
 	return subTreeReplicas
 }

From 6f94b07949909ed3aeaed6c2e253eb1d7ffbac77 Mon Sep 17 00:00:00 2001
From: Hein Meling <hein.meling@uis.no>
Date: Tue, 24 Dec 2024 23:35:28 +0100
Subject: [PATCH 18/21] refactor(tree): avoid allocation in ChildrenOf()

We can return a subsclice of the original to avoid allocating.
---
 internal/tree/treeconfig.go | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/internal/tree/treeconfig.go b/internal/tree/treeconfig.go
index 714cfcaa..0be66dac 100644
--- a/internal/tree/treeconfig.go
+++ b/internal/tree/treeconfig.go
@@ -73,16 +73,17 @@ func (t *Tree) ChildrenOf(replicaID hotstuff.ID) []hotstuff.ID {
 	if replicaPos == -1 {
 		return nil
 	}
-	children := make([]hotstuff.ID, 0)
-	for i := 1; i <= t.branchFactor; i++ {
-		childPos := (t.branchFactor * replicaPos) + i
-		if t.isWithInIndex(childPos) {
-			children = append(children, t.posToIDMapping[childPos])
-		} else {
-			break
-		}
+	childStart := replicaPos*t.branchFactor + 1
+	if childStart >= len(t.posToIDMapping) {
+		// no children since start is beyond slice length
+		return nil
+	}
+	childEnd := childStart + t.branchFactor
+	if childEnd > len(t.posToIDMapping) {
+		// clamp to the slice length
+		childEnd = len(t.posToIDMapping)
 	}
-	return children
+	return t.posToIDMapping[childStart:childEnd]
 }
 
 // ReplicaHeight returns the height of this tree's replica.

From 7dddede38a73c8be347d5c96a7ebb649b6351ec1 Mon Sep 17 00:00:00 2001
From: Hein Meling <hein.meling@uis.no>
Date: Tue, 24 Dec 2024 23:57:15 +0100
Subject: [PATCH 19/21] refactor(tree): avoid math.Pow in heightOf()

The math.Pow calls in a loop can be slow; this should be faster.
---
 internal/tree/treeconfig.go | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/internal/tree/treeconfig.go b/internal/tree/treeconfig.go
index 0be66dac..8ec3eac9 100644
--- a/internal/tree/treeconfig.go
+++ b/internal/tree/treeconfig.go
@@ -128,14 +128,28 @@ func (t *Tree) heightOf(replicaID hotstuff.ID) int {
 	if replicaPos == -1 {
 		return 0
 	}
-	startLimit := 0
-	endLimit := 0
-	for i := 1; i < t.height; i++ {
-		startLimit = startLimit + int(math.Pow(float64(t.branchFactor), float64(i-1)))
-		endLimit = endLimit + int(math.Pow(float64(t.branchFactor), float64(i)))
-		if replicaPos >= startLimit && replicaPos <= endLimit {
-			return t.height - i
+
+	// startLvl is the "first index" in the current level,
+	// levelCount is how many nodes are in this level.
+	//
+	// With branchFactor = n, the level sizes grow as:
+	// Level 0: 1 node (the root)
+	// Level 1: n nodes
+	// Level 2: n^2 nodes
+	// ...
+	// We start at level 1, since the root returns early.
+	startLvl := 1              // index of the first node at level 1
+	lvlCount := t.branchFactor // number of nodes at level 1
+
+	for lvl := 1; lvl < t.height; lvl++ {
+		endLvl := startLvl + lvlCount // one-past the last node at this level
+		if replicaPos >= startLvl && replicaPos < endLvl {
+			// replicaPos is in [startLvl, endLvl): t.height-lvl is the height.
+			return t.height - lvl
 		}
+		// Move to the next level:
+		startLvl = endLvl
+		lvlCount *= t.branchFactor
 	}
 	return 0
 }

From 8fa760c058ce58023c30ab9ac57c5ba620c79397 Mon Sep 17 00:00:00 2001
From: Hein Meling <hein.meling@uis.no>
Date: Wed, 25 Dec 2024 00:25:00 +0100
Subject: [PATCH 20/21] bench(tree): use modern table-driven benchmarks

---
 internal/tree/treeconfig_test.go | 51 +++++++++++++++-----------------
 1 file changed, 24 insertions(+), 27 deletions(-)

diff --git a/internal/tree/treeconfig_test.go b/internal/tree/treeconfig_test.go
index d783b593..26249e37 100644
--- a/internal/tree/treeconfig_test.go
+++ b/internal/tree/treeconfig_test.go
@@ -1,6 +1,7 @@
 package tree
 
 import (
+	"fmt"
 	"slices"
 	"sort"
 	"testing"
@@ -123,33 +124,29 @@ func TestTreeAPIWithInitializeWithPIDs(t *testing.T) {
 	}
 }
 
-func benchmarkGetChildren(size int, bf int, b *testing.B) {
-	ids := make([]hotstuff.ID, size)
-	for i := 0; i < size; i++ {
-		ids[i] = hotstuff.ID(i + 1)
+func BenchmarkReplicaChildren(b *testing.B) {
+	benchmarks := []struct {
+		size int
+		bf   int
+	}{
+		{size: 10, bf: 2},
+		{size: 21, bf: 4},
+		{size: 111, bf: 10},
+		{size: 211, bf: 14},
+		{size: 421, bf: 20},
 	}
-	tree := CreateTree(1, bf, ids)
-	for i := 0; i < b.N; i++ {
-		tree.ReplicaChildren()
+	for _, bm := range benchmarks {
+		b.Run(fmt.Sprintf("size=%d/bf=%d", bm.size, bm.bf), func(b *testing.B) {
+			ids := make([]hotstuff.ID, bm.size)
+			for i := 0; i < bm.size; i++ {
+				ids[i] = hotstuff.ID(i + 1)
+			}
+			tree := CreateTree(1, bm.bf, ids)
+			// replace `for range b.N` with `for b.Loop()` when go 1.24 released (in release candidate as of writing)
+			// for b.Loop() {
+			for range b.N {
+				tree.ReplicaChildren()
+			}
+		})
 	}
 }
-
-func BenchmarkGetChildren10(b *testing.B) {
-	benchmarkGetChildren(10, 2, b)
-}
-
-func BenchmarkGetChildren21(b *testing.B) {
-	benchmarkGetChildren(21, 4, b)
-}
-
-func BenchmarkGetChildren111(b *testing.B) {
-	benchmarkGetChildren(111, 10, b)
-}
-
-func BenchmarkGetChildren211(b *testing.B) {
-	benchmarkGetChildren(211, 14, b)
-}
-
-func BenchmarkGetChildren421(b *testing.B) {
-	benchmarkGetChildren(421, 20, b)
-}

From c3f8a5a964caa1d39b0f531c82833863907c5d34 Mon Sep 17 00:00:00 2001
From: Hein Meling <hein.meling@uis.no>
Date: Wed, 25 Dec 2024 00:52:37 +0100
Subject: [PATCH 21/21] fix(tree): removed unused isWithInIndex()

---
 internal/tree/treeconfig.go | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/internal/tree/treeconfig.go b/internal/tree/treeconfig.go
index 8ec3eac9..e50f954d 100644
--- a/internal/tree/treeconfig.go
+++ b/internal/tree/treeconfig.go
@@ -157,7 +157,3 @@ func (t *Tree) heightOf(replicaID hotstuff.ID) int {
 func (t *Tree) replicaPosition(id hotstuff.ID) int {
 	return slices.Index(t.posToIDMapping, id)
 }
-
-func (t *Tree) isWithInIndex(position int) bool {
-	return position < len(t.posToIDMapping)
-}