Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chain: use neutrino filters to speed up bitcoind seed recovery #889

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

chain: use neutrino filters to speed up bitcoind seed recovery #889

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b131910
chain: refactor btcd filter rescan into maybeShouldFetchBlock
Roasbeef Sep 22, 2023
2cb1df2
chain: use neutrino filters to speed up bitcoind seed recovery
Roasbeef Sep 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 114 additions & 5 deletions chain/bitcoind_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package chain
import (
"container/list"
"encoding/hex"
"encoding/json"
"errors"
"fmt"
"sync"
Expand All @@ -11,6 +12,8 @@ import (

"github.com/btcsuite/btcd/btcjson"
"github.com/btcsuite/btcd/btcutil"
"github.com/btcsuite/btcd/btcutil/gcs"
"github.com/btcsuite/btcd/btcutil/gcs/builder"
"github.com/btcsuite/btcd/chaincfg/chainhash"
"github.com/btcsuite/btcd/txscript"
"github.com/btcsuite/btcd/wire"
Expand Down Expand Up @@ -956,6 +959,74 @@ func (c *BitcoindClient) reorg(currentBlock waddrmgr.BlockStamp,
return nil
}

// blockFilterReq is a request to fetch a neutrino filter for a block from the
// target bitocind node.
type blockFilterReq struct {
BlockHash string `json:"blockhash"`
FilterType string `json:"filtertype"`
}

// blockFilterResp is a response containing a neutrino filter for a block from
// bitcond node.
type blockFilterResp struct {
Filter string `json:"filter"`
FilterHeader string `json:"header"`
}

const (
// bitcoindFilterRPC is the name of the RPC used to fetch a block
// filter from bitcoind.
bitcoindFilterRPC = "getblockfilter"

// bitcoindFilterType is the type of filter to fetch from bitcoind.
bitcoindFilterType = "basic"
)

// fetchBlockFilter fetches the GCS filter for a block from the remote node.
func (c *BitcoindClient) fetchBlockFilter(blkHash chainhash.Hash,
) (*gcs.Filter, error) {

filterReq := blockFilterReq{
BlockHash: blkHash.String(),
FilterType: bitcoindFilterType,
}
jsonFilterReq, err := json.Marshal(filterReq)
if err != nil {
return nil, err
}

resp, err := c.chainConn.client.RawRequest(
bitcoindFilterRPC, []json.RawMessage{jsonFilterReq},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This threw an error for me, looks like we need to JSON encode each field individually, not the whole message. This worked for me:

	hash, err := json.Marshal(blkHash.String())
	if err != nil {
		return nil, fmt.Errorf("cannot marshal hash: %w", err)
	}
	filterType, err := json.Marshal(bitcoindFilterType)
	if err != nil {
		return nil, fmt.Errorf("cannot marshal hash: %w", err)
	}

	resp, err := c.chainConn.client.RawRequest(
		bitcoindFilterRPC, []json.RawMessage{hash, filterType},
	)
	if err != nil {
		return nil, fmt.Errorf("cannot send request: %w", err)
	}

)
if err != nil {
return nil, err
}

var filterResp blockFilterResp
if err := json.Unmarshal(resp, &filterResp); err != nil {
return nil, err
}

rawFilter, err := hex.DecodeString(filterResp.Filter)
if err != nil {
return nil, err
}

filter, err := gcs.FromNBytes(
builder.DefaultP, builder.DefaultM, rawFilter,
)
if err != nil {
return nil, err
}

// Skip any empty filters.
if filter.N() == 0 {
return nil, nil
}

return filter, nil
}

// FilterBlocks scans the blocks contained in the FilterBlocksRequest for any
// addresses of interest. Each block will be fetched and filtered sequentially,
// returning a FilterBlocksReponse for the first block containing a matching
Expand All @@ -968,12 +1039,50 @@ func (c *BitcoindClient) FilterBlocks(

blockFilterer := NewBlockFilterer(c.chainConn.cfg.ChainParams, req)

// Iterate over the requested blocks, fetching each from the rpc client.
// Each block will scanned using the reverse addresses indexes generated
// above, breaking out early if any addresses are found.
// Construct the watchlist using the addresses and outpoints contained
// in the filter blocks request.
watchList, err := buildFilterBlocksWatchList(req)
if err != nil {
return nil, err
}

// Iterate over the requested blocks, fetching each from the rpc
// client. Each block will scanned using the reverse addresses indexes
// generated above, breaking out early if any addresses are found.
for i, block := range req.Blocks {
// TODO(conner): add prefetching, since we already know we'll be
// fetching *every* block
var shouldFetchBlock bool

switch {
// If we don't have filters, then we always need to fetch the
// block.
case !c.chainConn.hasNeutrinoFilters:
shouldFetchBlock = true

// If we have the filters, then we'll actually query the
// bitcoind node.
default:
shouldFetchBlock, err = maybeShouldFetchBlock(
c.fetchBlockFilter, block, watchList,
)
if err != nil {
return nil, err
}
}

// If the filter concluded that there're no matches in this
// block, then we don't need to fetch it, as there're no false
// negatives.
if !shouldFetchBlock {
log.Infof("Skipping block height=%d hash=%v, no "+
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very spammy. Maybe demote to debug?

"filter match", block.Height, block.Hash)
continue
}

log.Infof("Fetching block height=%d hash=%v",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is quite useful to debug! I synced my mainnet node and found it very interesting that we seem to fetch around 90 blocks per 2000 block scanning batch. Is it possible that the filters have that high of a false positive rate? Because I used an xprv I didn't have a birthday block encoded and started at height 481596 (segwit activation), where my wallet definitely didn't have any transactions yet...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is a property of the filters themselves or whether we can optimize something with how we construct the filter matcher on our side? Or maybe we have a weird value (e.g. all zero) that skews the matcher into more false positives?

block.Height, block.Hash)

// TODO(conner): add prefetching, since we already know we'll
// be fetching *every* block
rawBlock, err := c.GetBlock(&block.Hash)
if err != nil {
return nil, err
Expand Down
39 changes: 39 additions & 0 deletions chain/bitcoind_conn.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
"github.com/btcsuite/btcd/rpcclient"
"github.com/btcsuite/btcd/wire"
"github.com/lightningnetwork/lnd/ticker"

"encoding/json"
)

const (
Expand Down Expand Up @@ -108,6 +110,10 @@ type BitcoindConn struct {
rescanClientsMtx sync.Mutex
rescanClients map[uint64]*BitcoindClient

// hasNeutrinoFilters indicates if the bitcoind connection can serve
// neutrino filters over RPC.
hasNeutrinoFilters bool

quit chan struct{}
wg sync.WaitGroup
}
Expand All @@ -116,6 +122,32 @@ type BitcoindConn struct {
// running over Tor, this must support dialing peers over Tor as well.
type Dialer = func(string) (net.Conn, error)

// hasNeutrinoFilters returns whether or not the bitcoind node is able to serve
// neutrino filters based on its version.
func hasNeutrinoFilters(client *rpcclient.Client) (bool, error) {
// Fetch the bitcoind version.
resp, err := client.RawRequest("getnetworkinfo", nil)
if err != nil {
return false, err
}

info := struct {
Version int64 `json:"version"`
}{}

if err := json.Unmarshal(resp, &info); err != nil {
return false, err
}

// Bitcoind returns a single value representing the semantic version:
// 10000 * CLIENT_VERSION_MAJOR + 100 * CLIENT_VERSION_MINOR + 1 *
// CLIENT_VERSION_BUILD
//
// The getblockfilter call was added in version 19.0.0, so we return
// for versions >= 190000.
return info.Version >= 190000, nil
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This just gives us whether bitcoind is new enough to have filters. But don't we also need to detect whether they are enabled? Since on bitcoind they aren't turned on by default (are they in btcd?) as far as I know.
I just tried this locally with the index disabled and got: -1: Index is not enabled for filtertype basic

}

// NewBitcoindConn creates a client connection to the node described by the host
// string. The ZMQ connections are established immediately to ensure liveness.
// If the remote node does not operate on the same bitcoin network as described
Expand Down Expand Up @@ -174,11 +206,18 @@ func NewBitcoindConn(cfg *BitcoindConfig) (*BitcoindConn, error) {
}
}

hasNeutrinoFilters, err := hasNeutrinoFilters(client)
if err != nil {
return nil, fmt.Errorf("unable to determine if bitcoind "+
"has neutrino filters: %w", err)
}

bc := &BitcoindConn{
cfg: *cfg,
client: client,
prunedBlockDispatcher: prunedBlockDispatcher,
rescanClients: make(map[uint64]*BitcoindClient),
hasNeutrinoFilters: hasNeutrinoFilters,
quit: make(chan struct{}),
}

Expand Down
89 changes: 67 additions & 22 deletions chain/btcd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,65 @@ func (c *RPCClient) BlockStamp() (*waddrmgr.BlockStamp, error) {
}
}

// fetchBlockFilter fetches the GCS filter for a block from the remote node.
func (c *RPCClient) fetchBlockFilter(blkHash chainhash.Hash,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I still can't get over this formatting tbh... Do you really prefer this over:

// fetchBlockFilter fetches the GCS filter for a block from the remote node.
func (c *RPCClient) fetchBlockFilter(
	blkHash chainhash.Hash(*gcs.Filter, error) {

?

) (*gcs.Filter, error) {

rawFilter, err := c.GetCFilter(&blkHash, wire.GCSFilterRegular)
if err != nil {
return nil, err
}

// Ensure the filter is large enough to be deserialized.
if len(rawFilter.Data) < 4 {
return nil, nil
}

filter, err := gcs.FromNBytes(
builder.DefaultP, builder.DefaultM, rawFilter.Data,
)
if err != nil {
return nil, err
}

// Skip any empty filters.
if filter.N() == 0 {
return nil, nil
}

return filter, nil
}

// blockFilterFetcher is a function that fetches a block filter for a given
// block hash. A nil filter is returned if a valid filter doesn't exist for a
// given block.
type blockFilterFetcher func(blkHash chainhash.Hash) (*gcs.Filter, error)

// maybeShouldFetchBlock returns true if the contents of a block *might* have
// some items that match our watch list. This uses the neutrino filters to do a
// quick loop up to see if the block has relevant items to avoid downloading
// the full block.
func maybeShouldFetchBlock(blockFilterFetcher blockFilterFetcher,
blk wtxmgr.BlockMeta, watchList [][]byte) (bool, error) {

filter, err := blockFilterFetcher(blk.Hash)
if err != nil {
return false, err
}

if filter == nil {
return false, nil
}

key := builder.DeriveKey(&blk.Hash)
matched, err := filter.MatchAny(key, watchList)
if err != nil {
return false, err
}

return matched, nil
}

// FilterBlocks scans the blocks contained in the FilterBlocksRequest for any
// addresses of interest. For each requested block, the corresponding compact
// filter will first be checked for matches, skipping those that do not report
Expand All @@ -221,33 +280,19 @@ func (c *RPCClient) FilterBlocks(
// the filter returns a positive match, the full block is then requested
// and scanned for addresses using the block filterer.
for i, blk := range req.Blocks {
rawFilter, err := c.GetCFilter(&blk.Hash, wire.GCSFilterRegular)
if err != nil {
return nil, err
}

// Ensure the filter is large enough to be deserialized.
if len(rawFilter.Data) < 4 {
continue
}

filter, err := gcs.FromNBytes(
builder.DefaultP, builder.DefaultM, rawFilter.Data,
shouldFetchBlock, err := maybeShouldFetchBlock(
c.fetchBlockFilter, blk, watchList,
)
if err != nil {
return nil, err
}

// Skip any empty filters.
if filter.N() == 0 {
continue
}

key := builder.DeriveKey(&blk.Hash)
matched, err := filter.MatchAny(key, watchList)
if err != nil {
return nil, err
} else if !matched {
// If the filter concluded that there're no matches in this
// block, then we don't need to fetch it, as there're no false
// negatives.
if !shouldFetchBlock {
log.Infof("Skipping block height=%d hash=%v, no "+
"filter match", blk.Height, blk.Hash)
continue
}

Expand Down
Loading