feat: fix linter issues, naming issues, clean up

Author: mrz1836

README.md

@@ -1,3 +1,5 @@
+// Package subtree provides utilities for working with Bitcoin SV merkle tree subtrees,
+// including coinbase placeholder handling and subtree data structures.
 package subtree
 
 import (
@@ -14,18 +16,23 @@ var (
 		0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
 		0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
 	}
+	// CoinbasePlaceholderHashValue is the hash representation of the coinbase placeholder
 	CoinbasePlaceholderHashValue = chainhash.Hash(CoinbasePlaceholder)
-	CoinbasePlaceholderHash      = &CoinbasePlaceholderHashValue
+	// CoinbasePlaceholderHash is a pointer to the coinbase placeholder hash value
+	CoinbasePlaceholderHash = &CoinbasePlaceholderHashValue
 
+	// FrozenBytes represents the frozen transaction bytes used for placeholder transactions
 	FrozenBytes = [36]byte{
 		0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
 		0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
 		0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
 		0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
 		0xFF, 0xFF, 0xFF, 0xFF,
 	}
+	// FrozenBytesTxBytes represents the transaction bytes portion of FrozenBytes
 	FrozenBytesTxBytes = FrozenBytes[0:32]
-	FrozenBytesTxHash  = chainhash.Hash(FrozenBytesTxBytes)
+	// FrozenBytesTxHash is the hash of the frozen transaction bytes
+	FrozenBytesTxHash = chainhash.Hash(FrozenBytesTxBytes)
 )
 
 func generateCoinbasePlaceholderTx() *bt.Tx {

coinbase_placeholder.go

@@ -12,8 +12,8 @@ func TestCoinbasePlaceholderTx(t *testing.T) {
 	coinbasePlaceholderTxHash := coinbasePlaceholderTx.TxIDChainHash()
 	assert.True(t, IsCoinbasePlaceHolderTx(coinbasePlaceholderTx))
 	assert.Equal(t, uint32(0xFFFFFFFF), coinbasePlaceholderTx.Version)
-	assert.Equal(t, coinbasePlaceholderTx.LockTime, uint32(0xFFFFFFFF))
-	assert.Equal(t, coinbasePlaceholderTx.TxIDChainHash(), coinbasePlaceholderTxHash)
+	assert.Equal(t, uint32(0xFFFFFFFF), coinbasePlaceholderTx.LockTime)
+	assert.Equal(t, coinbasePlaceholderTxHash, coinbasePlaceholderTx.TxIDChainHash())
 	assert.False(t, IsCoinbasePlaceHolderTx(bt.NewTx()))
 	assert.Equal(t, "a8502e9c08b3c851201a71d25bf29fd38a664baedb777318b12d19242f0e46ab", coinbasePlaceholderTx.TxIDChainHash().String())
 }

coinbase_placeholder_test.go

@@ -2,6 +2,7 @@ package subtree
 
 import "golang.org/x/exp/constraints"
 
+// Min returns the minimum of two ordered values
 func Min[T constraints.Ordered](a, b T) T {
 	if a < b {
 		return a
@@ -10,6 +11,7 @@ func Min[T constraints.Ordered](a, b T) T {
 	return b
 }
 
+// Max returns the maximum of two ordered values
 func Max[T constraints.Ordered](a, b T) T {
 	if a > b {
 		return a

compare.go

@@ -0,0 +1,68 @@
+package subtree
+
+import "errors"
+
+// Sentinel errors for the subtree package
+
+// Range and bounds errors
+var (
+	// ErrIndexOutOfRange is returned when an index is out of range
+	ErrIndexOutOfRange = errors.New("index out of range")
+
+	// ErrTxIndexOutOfBounds is returned when a transaction index is out of bounds
+	ErrTxIndexOutOfBounds = errors.New("transaction index out of bounds")
+)
+
+// Validation errors
+var (
+	// ErrHeightNegative is returned when height is negative
+	ErrHeightNegative = errors.New("height must be at least 0")
+
+	// ErrNotPowerOfTwo is returned when the number of leaves must be a power of two
+	ErrNotPowerOfTwo = errors.New("numberOfLeaves must be a power of two")
+
+	// ErrSubtreeFull is returned when trying to add a node to a full subtree
+	ErrSubtreeFull = errors.New("subtree is full")
+
+	// ErrSubtreeNil is returned when the subtree is nil
+	ErrSubtreeNil = errors.New("subtree is nil")
+
+	// ErrSubtreeNotEmpty is returned when subtree should be empty before adding a coinbase node
+	ErrSubtreeNotEmpty = errors.New("subtree should be empty before adding a coinbase node")
+
+	// ErrSubtreeNodesEmpty is returned when subtree nodes slice is empty
+	ErrSubtreeNodesEmpty = errors.New("subtree nodes slice is empty")
+
+	// ErrNoSubtreesAvailable is returned when no subtrees are available
+	ErrNoSubtreesAvailable = errors.New("no subtrees available")
+
+	// ErrCoinbasePlaceholderMisuse is returned when coinbase placeholder node should be added with AddCoinbaseNode
+	ErrCoinbasePlaceholderMisuse = errors.New("coinbase placeholder node should be added with AddCoinbaseNode")
+
+	// ErrConflictingNodeNotInSubtree is returned when conflicting node is not in the subtree
+	ErrConflictingNodeNotInSubtree = errors.New("conflicting node is not in the subtree")
+
+	// ErrNodeNotFound is returned when a node is not found
+	ErrNodeNotFound = errors.New("node not found")
+)
+
+// Data mismatch errors
+var (
+	// ErrParentTxHashesMismatch is returned when parent tx hashes and indexes length mismatch
+	ErrParentTxHashesMismatch = errors.New("parent tx hashes and indexes length mismatch")
+
+	// ErrTxHashMismatch is returned when transaction hash does not match subtree node hash
+	ErrTxHashMismatch = errors.New("transaction hash does not match subtree node hash")
+
+	// ErrSubtreeLengthMismatch is returned when subtree length does not match tx data length
+	ErrSubtreeLengthMismatch = errors.New("subtree length does not match tx data length")
+)
+
+// Serialization errors
+var (
+	// ErrCannotSerializeSubtreeNotSet is returned when cannot serialize because subtree is not set
+	ErrCannotSerializeSubtreeNotSet = errors.New("cannot serialize, subtree is not set")
+
+	// ErrReadError is a generic read error for testing
+	ErrReadError = errors.New("read error")
+)

errors.go

@@ -90,7 +90,7 @@ func (p *TxInpoints) GetParentTxHashes() []chainhash.Hash {
 // GetParentTxHashAtIndex returns the parent transaction hash at the specified index.
 func (p *TxInpoints) GetParentTxHashAtIndex(index int) (chainhash.Hash, error) {
 	if index >= len(p.ParentTxHashes) {
-		return chainhash.Hash{}, fmt.Errorf("index out of range")
+		return chainhash.Hash{}, ErrIndexOutOfRange
 	}
 
 	return p.ParentTxHashes[index], nil
@@ -115,7 +115,7 @@ func (p *TxInpoints) GetTxInpoints() []Inpoint {
 // GetParentVoutsAtIndex returns the parent transaction output indexes at the specified index.
 func (p *TxInpoints) GetParentVoutsAtIndex(index int) ([]uint32, error) {
 	if index >= len(p.ParentTxHashes) {
-		return nil, fmt.Errorf("index out of range")
+		return nil, ErrIndexOutOfRange
 	}
 
 	return p.Idxs[index], nil
@@ -124,7 +124,7 @@ func (p *TxInpoints) GetParentVoutsAtIndex(index int) ([]uint32, error) {
 // Serialize serializes the TxInpoints object into a byte slice.
 func (p *TxInpoints) Serialize() ([]byte, error) {
 	if len(p.ParentTxHashes) != len(p.Idxs) {
-		return nil, fmt.Errorf("parent tx hashes and indexes length mismatch")
+		return nil, ErrParentTxHashesMismatch
 	}
 
 	bufBytes := make([]byte, 0, 1024) // 1KB (arbitrary size, should be enough for most cases)

inpoints.go

@@ -13,8 +13,8 @@ func TestTxInpoints(t *testing.T) {
 		p, err := NewTxInpointsFromTx(tx)
 		require.NoError(t, err)
 
-		assert.Equal(t, 1, len(p.ParentTxHashes))
-		assert.Equal(t, 1, len(p.Idxs[0]))
+		assert.Len(t, p.ParentTxHashes, 1)
+		assert.Len(t, p.Idxs[0], 1)
 	})
 
 	t.Run("serialize", func(t *testing.T) {
@@ -23,13 +23,13 @@ func TestTxInpoints(t *testing.T) {
 
 		b, err := p.Serialize()
 		require.NoError(t, err)
-		assert.Equal(t, 44, len(b))
+		assert.Len(t, b, 44)
 
 		p2, err := NewTxInpointsFromBytes(b)
 		require.NoError(t, err)
 
-		assert.Equal(t, 1, len(p2.ParentTxHashes))
-		assert.Equal(t, 1, len(p2.Idxs[0]))
+		assert.Len(t, p2.ParentTxHashes, 1)
+		assert.Len(t, p2.Idxs[0], 1)
 
 		assert.Equal(t, p.ParentTxHashes[0], p2.ParentTxHashes[0])
 		assert.Equal(t, p.Idxs[0][0], p2.Idxs[0][0])
@@ -52,8 +52,8 @@ func TestTxInpoints(t *testing.T) {
 		require.NoError(t, err)
 
 		// make sure they are the same
-		assert.Equal(t, len(p.ParentTxHashes), len(p2.ParentTxHashes))
-		assert.Equal(t, len(p.Idxs), len(p2.Idxs))
+		assert.Len(t, p2.ParentTxHashes, len(p.ParentTxHashes))
+		assert.Len(t, p2.Idxs, len(p.Idxs))
 		assert.Equal(t, p.ParentTxHashes[0], p2.ParentTxHashes[0])
 		assert.Equal(t, p.Idxs[0][0], p2.Idxs[0][0])
 	})
@@ -65,7 +65,7 @@ func TestGetTxInpoints(t *testing.T) {
 
 	// Test getting inpoints
 	inpoints := p.GetTxInpoints()
-	assert.Equal(t, 1, len(inpoints))
+	assert.Len(t, inpoints, 1)
 	assert.Equal(t, uint32(5), inpoints[0].Index)
 	assert.Equal(t, *tx.Inputs[0].PreviousTxIDChainHash(), inpoints[0].Hash)
 }
@@ -103,7 +103,7 @@ func TestGetParentVoutsAtIndex(t *testing.T) {
 		vouts, err := p.GetParentVoutsAtIndex(0)
 		require.NoError(t, err)
 
-		assert.Equal(t, 1, len(vouts))
+		assert.Len(t, vouts, 1)
 		assert.Equal(t, uint32(5), vouts[0])
 	})
 

inpoints_test.go

@@ -12,7 +12,7 @@ import (
 // GetMerkleProofForCoinbase returns a merkle proof for the coinbase transaction
 func GetMerkleProofForCoinbase(subtrees []*Subtree) ([]*chainhash.Hash, error) {
 	if len(subtrees) == 0 {
-		return nil, fmt.Errorf("no subtrees available")
+		return nil, ErrNoSubtreesAvailable
 	}
 
 	merkleProof, err := subtrees[0].GetMerkleProof(0)
@@ -42,7 +42,7 @@ func GetMerkleProofForCoinbase(subtrees []*Subtree) ([]*chainhash.Hash, error) {
 }
 
 // BuildMerkleTreeStoreFromBytes builds a merkle tree from the given nodes.
-func BuildMerkleTreeStoreFromBytes(nodes []SubtreeNode) (*[]chainhash.Hash, error) {
+func BuildMerkleTreeStoreFromBytes(nodes []Node) (*[]chainhash.Hash, error) {
 	if len(nodes) == 0 {
 		return &[]chainhash.Hash{}, nil
 	}
@@ -94,7 +94,7 @@ func BuildMerkleTreeStoreFromBytes(nodes []SubtreeNode) (*[]chainhash.Hash, erro
 }
 
 // calcMerkles calculates the merkle hashes for the given nodes in the range
-func calcMerkles(nodes []SubtreeNode, merkleFrom, merkleTo, nextPoT, length int, merkles []chainhash.Hash) {
+func calcMerkles(nodes []Node, merkleFrom, merkleTo, nextPoT, length int, merkles []chainhash.Hash) {
 	var offset int
 
 	var currentMerkle chainhash.Hash
@@ -103,30 +103,37 @@ func calcMerkles(nodes []SubtreeNode, merkleFrom, merkleTo, nextPoT, length int,
 
 	for i := merkleFrom; i < merkleTo; i += 2 {
 		offset = i / 2
+		currentMerkle, currentMerkle1 = getMerklePair(nodes, merkles, i, nextPoT, length)
+		merkles[offset] = calcMerkle(currentMerkle, currentMerkle1)
+	}
+}
 
-		if i < nextPoT {
-			if i >= length {
-				currentMerkle = chainhash.Hash{}
-			} else {
-				currentMerkle = nodes[i].Hash
-			}
+// getMerklePair returns a pair of merkle hashes at the given index
+func getMerklePair(nodes []Node, merkles []chainhash.Hash, i, nextPoT, length int) (chainhash.Hash, chainhash.Hash) {
+	var currentMerkle, currentMerkle1 chainhash.Hash
 
-			if i+1 >= length {
-				currentMerkle1 = chainhash.Hash{}
-			} else {
-				currentMerkle1 = nodes[i+1].Hash
-			}
-		} else {
-			currentMerkle = merkles[i-nextPoT]
-			currentMerkle1 = merkles[i-nextPoT+1]
-		}
+	if i < nextPoT {
+		currentMerkle = getNodeHashAt(nodes, i, length)
+		currentMerkle1 = getNodeHashAt(nodes, i+1, length)
+	} else {
+		currentMerkle = merkles[i-nextPoT]
+		currentMerkle1 = merkles[i-nextPoT+1]
+	}
 
-		merkles[offset] = calcMerkle(currentMerkle, currentMerkle1)
+	return currentMerkle, currentMerkle1
+}
+
+// getNodeHashAt returns the hash at the given index, or an empty hash if out of bounds
+func getNodeHashAt(nodes []Node, index, length int) chainhash.Hash {
+	if index >= length {
+		return chainhash.Hash{}
 	}
+
+	return nodes[index].Hash
 }
 
 // calcMerkle calculates the parent node hash from the left and right child nodes
-func calcMerkle(currentMerkle chainhash.Hash, currentMerkle1 chainhash.Hash) [32]byte {
+func calcMerkle(currentMerkle, currentMerkle1 chainhash.Hash) [32]byte {
 	switch {
 	// When there is no left child node, the parent is nil ("") too.
 	case currentMerkle.Equal(chainhash.Hash{}):

merkle_tree.go

@@ -5,6 +5,7 @@ import (
 	"math/bits"
 )
 
+// CeilPowerOfTwo returns the smallest power of two that is greater than or equal to the given number
 func CeilPowerOfTwo(num int) int {
 	if num <= 0 {
 		return 1
@@ -19,6 +20,7 @@ func CeilPowerOfTwo(num int) int {
 	return ceilValue
 }
 
+// IsPowerOfTwo returns true if the given number is a power of two
 func IsPowerOfTwo(num int) bool {
 	if num <= 0 {
 		return false

power_of_two.go

@@ -1,7 +1,6 @@
 package subtree
 
 import (
-	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -11,12 +10,12 @@ func TestIsPowerOf2(t *testing.T) {
 	// Testing the function
 	numbers := []int{1, 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024, 1048576, 70368744177664}
 	for _, num := range numbers {
-		assert.True(t, IsPowerOfTwo(num), fmt.Sprintf("%d should be a power of 2", num))
+		assert.True(t, IsPowerOfTwo(num), "%d should be a power of 2", num)
 	}
 
 	numbers = []int{-1, 0, 41, 13}
 	for _, num := range numbers {
-		assert.False(t, IsPowerOfTwo(num), fmt.Sprintf("%d should be a power of 2", num))
+		assert.False(t, IsPowerOfTwo(num), "%d should be a power of 2", num)
 	}
 }
 
@@ -26,6 +25,6 @@ func TestNextLowerPowerOf2(t *testing.T) {
 	expected := []uint{16, 32, 64, 128, 0, 131072}
 
 	for i, num := range numbers {
-		assert.Equal(t, expected[i], NextLowerPowerOfTwo(num), fmt.Sprintf("%d should be a power of 2", num))
+		assert.Equal(t, expected[i], NextLowerPowerOfTwo(num), "%d should be a power of 2", num)
 	}
 }