Introduce Role-based interfaces for wallet

.gemini/config.yaml

@@ -33,4 +33,4 @@ code_review:
 
 # List of glob patterns to ignore (files and directories).
 # Type: array of string, default: [].
-ignore_patterns: []
+ignore_patterns: ["deprecated.go"]

.gemini/styleguide.md

@@ -1,4 +1,4 @@
-# LND Style Guide
+# Btcwallet Style Guide
 
 ## Code Documentation and Commenting
 
@@ -9,7 +9,7 @@
 - Unit tests must always use the `require` library. Either table driven unit 
   tests or tests using the `rapid` library are preferred. 
 - The line length MUST NOT exceed 80 characters, this is very important. 
-  You must count the Golang indentation (tabulator character) as 8 spaces when 
+  You must count the Golang indentation (tabulator character) as 4 spaces when 
   determining the line length. Use creative approaches or the wrapping rules 
   specified below to make sure the line length isn't exceeded.
 - Every function must be commented with its purpose and assumptions.
@@ -151,7 +151,7 @@ if amt < 546 {
 ### 80 character line length
 
 - Wrap columns at 80 characters.
-- Tabs are 8 spaces.
+- Tabs are 4 spaces.
 
 **WRONG**
 ```go

.gitignore

@@ -6,3 +6,7 @@ coverage.txt
 .vscode
 .DS_Store
 .aider*
+coverage.out
+*.prof
+*.test
+*cpu.out

.golangci.yml

@@ -10,6 +10,9 @@ run:
 linters:
   default: all
   disable:
+    # TODO(yy): Re-enable this linter once the refactoring series is done.
+    - ireturn
+
     # Global variables are used in many places throughout the code base.
     - gochecknoglobals
 
@@ -91,6 +94,19 @@ linters:
       multi-func: true
       multi-if: true
 
+    wrapcheck:
+      ignore-sig-regexps:
+        # Allow returning .Err() from context.Context without wrapping it.
+        - context\.Context.*\.Err\(\)
+
+    gomoddirectives:
+      replace-local: true
+      replace-allow-list:
+        # This package will be downgrade to internal so we will import it
+        # directly here.
+        - github.com/btcsuite/btcwallet/wtxmgr
+
+
   # Defines a set of rules to ignore issues.
   # It does not skip the analysis, and so does not ignore "typecheck" errors.
   exclusions:
@@ -112,6 +128,7 @@ linters:
     paths:
       - rpc/legacyrpc/
       - wallet/deprecated.go
+      - wallet/deprecated_test.go
 
     rules:
       # Exclude gosec from running for tests so that tests with weak randomness

btcwallet.go

@@ -237,10 +237,16 @@ func rpcClientConnectLoop(legacyRPCServer *legacyrpc.Server, loader *wallet.Load
 			loadedWallet.SetChainSynced(false)
 
 			// TODO: Rework the wallet so changing the RPC client
-			// does not require stopping and restarting everything.
-			loadedWallet.Stop()
+			//nolint:staticcheck // This should be fixed once
+			// the interface refactor is finished, and new wallet
+			// RPC is built.
+			loadedWallet.StopDeprecated()
 			loadedWallet.WaitForShutdown()
-			loadedWallet.Start()
+
+			//nolint:staticcheck // This should be fixed once
+			// the interface refactor is finished, and new wallet
+			// RPC is built.
+			loadedWallet.StartDeprecated()
 		}
 	}
 }

chain/bitcoind_client.go

@@ -4,6 +4,7 @@ import (
 	"container/list"
 	"context"
 	"encoding/hex"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"sync"
@@ -13,7 +14,10 @@ import (
 	"github.com/btcsuite/btcd/address/v2"
 	"github.com/btcsuite/btcd/btcjson"
 	"github.com/btcsuite/btcd/btcutil/v2"
+	"github.com/btcsuite/btcd/btcutil/v2/gcs"
+	"github.com/btcsuite/btcd/btcutil/v2/gcs/builder"
 	"github.com/btcsuite/btcd/chainhash/v2"
+	"github.com/btcsuite/btcd/rpcclient"
 	"github.com/btcsuite/btcd/txscript/v2"
 	"github.com/btcsuite/btcd/wire/v2"
 	"github.com/btcsuite/btcwallet/waddrmgr"
@@ -26,6 +30,10 @@ var (
 	// to receive a notification for a specific item and the bitcoind client
 	// is in the middle of shutting down.
 	ErrBitcoindClientShuttingDown = errors.New("client is shutting down")
+
+	// ErrOnlyBasicFilters is an error returned when a filter type other
+	// than basic is requested.
+	ErrOnlyBasicFilters = errors.New("only basic filters are supported")
 )
 
 // BitcoindClient represents a persistent client connection to a bitcoind server
@@ -50,6 +58,15 @@ type BitcoindClient struct {
 	// the RPC and ZMQ connections to a bitcoind node.
 	chainConn *BitcoindConn
 
+	// batchClient is a secondary RPC client dedicated for batch requests.
+	// This client is created specifically for batch operations because the
+	// rpcclient.Client in batch mode is stateful, accumulating requests
+	// until `Send()` is called. Using a dedicated instance avoids race
+	// conditions and ensures isolation from other concurrent RPC calls
+	// made by the main `chainConn.client` or other `BitcoindClient`
+	// instances.
+	batchClient *rpcclient.Client
+
 	// bestBlock keeps track of the tip of the current best chain.
 	bestBlockMtx sync.RWMutex
 	bestBlock    waddrmgr.BlockStamp
@@ -114,6 +131,56 @@ func (c *BitcoindClient) BackEnd() string {
 	return "bitcoind"
 }
 
+// GetCFilter returns a compact filter for the given block hash and filter
+// type.
+//
+// NOTE: This is part of the chain.Interface interface.
+func (c *BitcoindClient) GetCFilter(hash *chainhash.Hash,
+	filterType wire.FilterType) (*gcs.Filter, error) {
+
+	if filterType != wire.GCSFilterRegular {
+		return nil, ErrOnlyBasicFilters
+	}
+
+	// The getblockfilter RPC takes the block hash and the filter type.
+	// Filter type defaults to "basic" if omitted, but we specify it for
+	// clarity.
+	params := []json.RawMessage{
+		json.RawMessage(fmt.Sprintf("%q", hash.String())),
+		json.RawMessage(fmt.Sprintf("%q", "basic")),
+	}
+
+	resp, err := c.chainConn.client.RawRequest("getblockfilter", params)
+	if err != nil {
+		return nil, c.MapRPCErr(err)
+	}
+
+	var res struct {
+		Filter string `json:"filter"`
+		Header string `json:"header"`
+	}
+
+	err = json.Unmarshal(resp, &res)
+	if err != nil {
+		return nil, fmt.Errorf("failed to unmarshal filter: %w", err)
+	}
+
+	filterBytes, err := hex.DecodeString(res.Filter)
+	if err != nil {
+		return nil, fmt.Errorf("failed to decode filter: %w", err)
+	}
+
+	filter, err := gcs.FromNBytes(
+		builder.DefaultP, builder.DefaultM, filterBytes,
+	)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create filter from bytes: %w",
+			err)
+	}
+
+	return filter, nil
+}
+
 // GetBestBlock returns the highest block known to bitcoind.
 func (c *BitcoindClient) GetBestBlock() (*chainhash.Hash, int32, error) {
 	bcinfo, err := c.chainConn.client.GetBlockChainInfo()
@@ -573,6 +640,9 @@ func (c *BitcoindClient) Stop() {
 	// prevent sending notifications to it after it's been stopped.
 	c.chainConn.RemoveClient(c.id)
 
+	c.batchClient.Shutdown()
+	c.batchClient.WaitForShutdown()
+
 	c.notificationQueue.Stop()
 }
 
@@ -1185,8 +1255,13 @@ func (c *BitcoindClient) filterBlock(block *wire.MsgBlock, height int32,
 		// transaction.
 		blockDetails.Index = i
 		txDetails := btcutil.NewTx(tx)
+
+		// We disable individual transaction notifications here because
+		// the full set of relevant transactions will be dispatched
+		// atomically via FilteredBlockConnected at the end of block
+		// processing.
 		isRelevant, rec, err := c.filterTx(
-			txDetails, blockDetails, notify,
+			txDetails, blockDetails, false,
 		)
 		if err != nil {
 			log.Warnf("Unable to filter transaction %v: %v",
@@ -1349,8 +1424,9 @@ func (c *BitcoindClient) filterTx(txDetails *btcutil.Tx,
 		c.mempool[*txDetails.Hash()] = struct{}{}
 	}
 
-	c.onRelevantTx(rec, blockDetails)
-
+	if notify {
+		c.onRelevantTx(rec, blockDetails)
+	}
 	return true, rec, nil
 }
 
@@ -1416,3 +1492,157 @@ func (c *BitcoindClient) updateWatchedFilters(update any) {
 		}
 	}
 }
+
+// GetBlockHashes returns a slice of block hashes for the given height range.
+func (c *BitcoindClient) GetBlockHashes(startHeight,
+	endHeight int64) ([]chainhash.Hash, error) {
+
+	if startHeight > endHeight {
+		return nil, fmt.Errorf("%w: start height %d, end height %d",
+			ErrInvalidParam, startHeight, endHeight)
+	}
+
+	client := c.batchClient
+	count := endHeight - startHeight + 1
+	hashes := make([]chainhash.Hash, 0, count)
+	futures := make([]rpcclient.FutureGetBlockHashResult, 0, count)
+
+	for h := startHeight; h <= endHeight; h++ {
+		futures = append(futures, client.GetBlockHashAsync(h))
+	}
+
+	err := client.Send()
+	if err != nil {
+		return nil, fmt.Errorf("batch send: %w", err)
+	}
+
+	for _, f := range futures {
+		hash, err := f.Receive()
+		if err != nil {
+			return nil, fmt.Errorf("receive block hash: %w", err)
+		}
+
+		hashes = append(hashes, *hash)
+	}
+
+	return hashes, nil
+}
+
+// GetCFilters returns a slice of filters for the given block hashes.
+func (c *BitcoindClient) GetCFilters(hashes []chainhash.Hash,
+	filterType wire.FilterType) ([]*gcs.Filter, error) {
+
+	if filterType != wire.GCSFilterRegular {
+		return nil, ErrOnlyBasicFilters
+	}
+
+	client := c.batchClient
+	filters := make([]*gcs.Filter, 0, len(hashes))
+	futures := make([]rpcclient.FutureRawResult, 0, len(hashes))
+
+	for _, hash := range hashes {
+		params := []json.RawMessage{
+			json.RawMessage(fmt.Sprintf("%q", hash.String())),
+			json.RawMessage(fmt.Sprintf("%q", "basic")),
+		}
+		futures = append(futures, client.RawRequestAsync(
+			"getblockfilter", params,
+		))
+	}
+
+	err := client.Send()
+	if err != nil {
+		return nil, fmt.Errorf("batch send: %w", err)
+	}
+
+	for _, f := range futures {
+		resp, err := f.Receive()
+		if err != nil {
+			return nil, fmt.Errorf("receive cfilter: %w", err)
+		}
+
+		var res struct {
+			Filter string `json:"filter"`
+			Header string `json:"header"`
+		}
+
+		err = json.Unmarshal(resp, &res)
+		if err != nil {
+			return nil, fmt.Errorf("unmarshal cfilter: %w", err)
+		}
+
+		filterBytes, err := hex.DecodeString(res.Filter)
+		if err != nil {
+			return nil, fmt.Errorf("decode cfilter: %w", err)
+		}
+
+		filter, err := gcs.FromNBytes(
+			builder.DefaultP, builder.DefaultM, filterBytes,
+		)
+		if err != nil {
+			return nil, fmt.Errorf("parse cfilter: %w", err)
+		}
+
+		filters = append(filters, filter)
+	}
+
+	return filters, nil
+}
+
+// GetBlocks returns a slice of full blocks for the given block hashes.
+func (c *BitcoindClient) GetBlocks(hashes []chainhash.Hash) (
+	[]*wire.MsgBlock, error) {
+
+	client := c.batchClient
+	blocks := make([]*wire.MsgBlock, 0, len(hashes))
+	futures := make([]rpcclient.FutureGetBlockResult, 0, len(hashes))
+
+	for _, hash := range hashes {
+		futures = append(futures, client.GetBlockAsync(&hash))
+	}
+
+	err := client.Send()
+	if err != nil {
+		return nil, fmt.Errorf("batch send: %w", err)
+	}
+
+	for _, f := range futures {
+		block, err := f.Receive()
+		if err != nil {
+			return nil, fmt.Errorf("receive block: %w", err)
+		}
+
+		blocks = append(blocks, block)
+	}
+
+	return blocks, nil
+}
+
+// GetBlockHeaders returns a slice of block headers for the given block hashes.
+func (c *BitcoindClient) GetBlockHeaders(hashes []chainhash.Hash) (
+	[]*wire.BlockHeader, error) {
+
+	client := c.batchClient
+	headers := make([]*wire.BlockHeader, 0, len(hashes))
+	futures := make([]rpcclient.FutureGetBlockHeaderResult, 0, len(hashes))
+
+	for _, hash := range hashes {
+		futures = append(futures, client.GetBlockHeaderAsync(&hash))
+	}
+
+	err := client.Send()
+	if err != nil {
+		return nil, fmt.Errorf("batch send: %w", err)
+	}
+
+	for _, f := range futures {
+		header, err := f.Receive()
+		if err != nil {
+			return nil, fmt.Errorf("receive header: %w", err)
+		}
+
+		headers = append(headers, header)
+	}
+
+	return headers, nil
+}