Change hash representation from slice to 32-byte array

This change is non-functional, just a clean-up, reducing tech debt.

This codebase manipulates many 32-byte hashes, such as block hashes,
transaction IDs, and merkle roots. These should always have been
represented as fixed-size 32-byte arrays rather than variable-length
slices. This prevents bugs (a slice that's supposed to be of length
32 bytes could be a different length) and makes assigning, comparing,
function argument passing, and function return value passing simpler
and less fragile.

The new hash type, hash32.T, which is defined as [32]byte (32-byte
array of bytes), can be treated like any simple variable, such as an
integer. A slice, in contrast, is like a string; it's really a structure
that includes a length, capacity, and pointer to separately-allocated
memory to hold the elements of the slice.

The only point of friction is that protobuf doesn't support fixed-sized
arrays, only (in effect) slices. So conversion must be done in each
direction.

Author: Larry Ruane

common/cache.go

@@ -14,6 +14,7 @@ import (
 	"path/filepath"
 	"sync"
 
+	"github.com/zcash/lightwalletd/hash32"
 	"github.com/zcash/lightwalletd/walletrpc"
 	"google.golang.org/protobuf/proto"
 )
@@ -22,10 +23,10 @@ import (
 type BlockCache struct {
 	lengthsName, blocksName string // pathnames
 	lengthsFile, blocksFile *os.File
-	starts                  []int64 // Starting offset of each block within blocksFile
-	firstBlock              int     // height of the first block in the cache (usually Sapling activation)
-	nextBlock               int     // height of the first block not in the cache
-	latestHash              []byte  // hash of the most recent (highest height) block, for detecting reorgs.
+	starts                  []int64  // Starting offset of each block within blocksFile
+	firstBlock              int      // height of the first block in the cache (usually Sapling activation)
+	nextBlock               int      // height of the first block not in the cache
+	latestHash              hash32.T // hash of the most recent (highest height) block, for detecting reorgs.
 	mutex                   sync.RWMutex
 }
 
@@ -44,18 +45,18 @@ func (c *BlockCache) GetFirstHeight() int {
 }
 
 // GetLatestHash returns the hash (block ID) of the most recent (highest) known block.
-func (c *BlockCache) GetLatestHash() []byte {
+func (c *BlockCache) GetLatestHash() hash32.T {
 	c.mutex.RLock()
 	defer c.mutex.RUnlock()
 	return c.latestHash
 }
 
 // HashMatch indicates if the given prev-hash matches the most recent block's hash
 // so reorgs can be detected.
-func (c *BlockCache) HashMatch(prevhash []byte) bool {
+func (c *BlockCache) HashMatch(prevhash hash32.T) bool {
 	c.mutex.RLock()
 	defer c.mutex.RUnlock()
-	return c.latestHash == nil || bytes.Equal(c.latestHash, prevhash)
+	return c.latestHash == hash32.Nil || c.latestHash == prevhash
 }
 
 // Make the block at the given height the lowest height that we don't have.
@@ -167,7 +168,7 @@ func (c *BlockCache) readBlock(height int) *walletrpc.CompactBlock {
 
 // Caller should hold c.mutex.Lock().
 func (c *BlockCache) setLatestHash() {
-	c.latestHash = nil
+	c.latestHash = hash32.Nil
 	// There is at least one block; get the last block's hash
 	if c.nextBlock > c.firstBlock {
 		// At least one block remains; get the last block's hash
@@ -176,8 +177,7 @@ func (c *BlockCache) setLatestHash() {
 			c.recoverFromCorruption(c.nextBlock - 10000)
 			return
 		}
-		c.latestHash = make([]byte, len(block.Hash))
-		copy(c.latestHash, block.Hash)
+		c.latestHash = hash32.T(block.Hash)
 	}
 }
 
@@ -312,10 +312,7 @@ func (c *BlockCache) Add(height int, block *walletrpc.CompactBlock) error {
 	offset := c.starts[len(c.starts)-1]
 	c.starts = append(c.starts, offset+int64(len(data)+8))
 
-	if c.latestHash == nil {
-		c.latestHash = make([]byte, len(block.Hash))
-	}
-	copy(c.latestHash, block.Hash)
+	c.latestHash = hash32.T(block.Hash)
 	c.nextBlock++
 	// Invariant: m[firstBlock..nextBlock) are valid.
 	return nil

common/cache_test.go

@@ -9,6 +9,7 @@ import (
 	"os"
 	"testing"
 
+	"github.com/zcash/lightwalletd/hash32"
 	"github.com/zcash/lightwalletd/parser"
 	"github.com/zcash/lightwalletd/walletrpc"
 )
@@ -84,7 +85,7 @@ func TestCache(t *testing.T) {
 
 	// Reorg to before the first block moves back to only the first block
 	cache.Reorg(289459)
-	if cache.latestHash != nil {
+	if cache.latestHash != hash32.Nil {
 		t.Fatal("unexpected latestHash, should be nil")
 	}
 	if cache.nextBlock != 289460 {

common/common.go

@@ -14,6 +14,7 @@ import (
 	"time"
 
 	"github.com/sirupsen/logrus"
+	"github.com/zcash/lightwalletd/hash32"
 	"github.com/zcash/lightwalletd/parser"
 	"github.com/zcash/lightwalletd/walletrpc"
 )
@@ -369,12 +370,12 @@ func getBlockFromRPC(height int) (*walletrpc.CompactBlock, error) {
 		return nil, errors.New("received unexpected height block")
 	}
 	for i, t := range block.Transactions() {
-		txid, err := hex.DecodeString(block1.Tx[i])
+		txid, err := hash32.Decode(block1.Tx[i])
 		if err != nil {
 			return nil, fmt.Errorf("error decoding getblock txid: %w", err)
 		}
 		// convert from big-endian
-		t.SetTxID(parser.Reverse(txid))
+		t.SetTxID(hash32.Reverse(txid))
 	}
 	r := block.ToCompact()
 	r.ChainMetadata.SaplingCommitmentTreeSize = block1.Trees.Sapling.Size
@@ -426,13 +427,13 @@ func BlockIngestor(c *BlockCache, rep int) {
 		if err != nil {
 			Log.Fatal("bad getbestblockhash return:", err, result)
 		}
-		lastBestBlockHash, err := hex.DecodeString(hashHex)
+		lastBestBlockHash, err := hash32.Decode(hashHex)
 		if err != nil {
 			Log.Fatal("error decoding getbestblockhash", err, hashHex)
 		}
 
 		height := c.GetNextHeight()
-		if string(lastBestBlockHash) == string(parser.Reverse(c.GetLatestHash())) {
+		if lastBestBlockHash == hash32.Reverse(c.GetLatestHash()) {
 			// Synced
 			c.Sync()
 			if lastHeightLogged != height-1 {
@@ -450,14 +451,14 @@ func BlockIngestor(c *BlockCache, rep int) {
 			Time.Sleep(8 * time.Second)
 			continue
 		}
-		if block != nil && c.HashMatch(block.PrevHash) {
+		if block != nil && c.HashMatch(hash32.T(block.PrevHash)) {
 			if err = c.Add(height, block); err != nil {
 				Log.Fatal("Cache add failed:", err)
 			}
 			// Don't log these too often.
 			if DarksideEnabled || Time.Now().Sub(lastLog).Seconds() >= 4 {
 				lastLog = Time.Now()
-				Log.Info("Adding block to cache ", height, " ", displayHash(block.Hash))
+				Log.Info("Adding block to cache ", height, " ", displayHash(hash32.T(block.Hash)))
 			}
 			continue
 		}
@@ -537,29 +538,29 @@ func GetBlockRange(cache *BlockCache, blockOut chan<- *walletrpc.CompactBlock, e
 // the meanings of the `Height` field of the `RawTransaction` type are as
 // follows:
 //
-// * height 0: the transaction is in the mempool
-// * height 0xffffffffffffffff: the transaction has been mined on a fork that
-//   is not currently the main chain
-// * any other height: the transaction has been mined in the main chain at the
-//   given height
+//   - height 0: the transaction is in the mempool
+//   - height 0xffffffffffffffff: the transaction has been mined on a fork that
+//     is not currently the main chain
+//   - any other height: the transaction has been mined in the main chain at the
+//     given height
 func ParseRawTransaction(message json.RawMessage) (*walletrpc.RawTransaction, error) {
-		// Many other fields are returned, but we need only these two.
-		var txinfo ZcashdRpcReplyGetrawtransaction
-		err := json.Unmarshal(message, &txinfo)
-		if err != nil {
-			return nil, err
-		}
-		txBytes, err := hex.DecodeString(txinfo.Hex)
-		if err != nil {
-			return nil, err
-		}
+	// Many other fields are returned, but we need only these two.
+	var txinfo ZcashdRpcReplyGetrawtransaction
+	err := json.Unmarshal(message, &txinfo)
+	if err != nil {
+		return nil, err
+	}
+	txBytes, err := hex.DecodeString(txinfo.Hex)
+	if err != nil {
+		return nil, err
+	}
 
-		return &walletrpc.RawTransaction{
-			Data:   txBytes,
-			Height: uint64(txinfo.Height),
-		}, nil
+	return &walletrpc.RawTransaction{
+		Data:   txBytes,
+		Height: uint64(txinfo.Height),
+	}, nil
 }
 
-func displayHash(hash []byte) string {
-	return hex.EncodeToString(parser.Reverse(hash))
+func displayHash(hash hash32.T) string {
+	return hash32.Encode(hash32.Reverse(hash))
 }