`WalletSync` issues with `RpcBlockchain` and `CompactFiltersBlockchain`

[evanlinjin]

The bug

After looking through the codebase for RpcBlockchain, it looks as if importdescriptors would only be called on the first sync, or not at all if there were already more than 100 scriptPubKeys cached before the first sync attempt. In other words, the Bitcoin Core wallet would only be aware of 100 scriptPubKeys or none at all.

The CompactFiltersBlockchain implementation may have similar issues to the electrum-based Blockchain implementations (fixed in #672), where owned outputs can be missed when compiling data from raw transactions (when scriptPubKeys are under-cached), and subsequent syncs will not rediscover these missed outputs.

Note that these deductions are not tested, but mere assumptions from scanning over the codebase.

Tests to write

I recommend writing the following tests (to confirm the existence of these bugs):

1. After caching greater-than-100 scriptPubKeys before doing initial Wallet::sync, we should be able to detect correct balance and owned UTXO set (assuming that all txs interact with owned scriptPubKeys with derivation index less-than-100).

2. Given a positive integer that is greater-than-0 (x), after doing the initial Wallet::sync, create a self-send transaction, sending from scriptPubKey at derivation index 99 to derivation index 99+x. Cache at-least x more scriptPubKeys and re-sync. Wallet should have the correct balance and output-set.

Additional context

Feel free to look at PR #672 (which fixes #521 and #451 for electrum-based blockchains). However, note that the sync logic differs from RPC and Compact Block Filters, so the fix will be different.

If I have understood the codebase correctly, and these bugs exist, it will be good to have a fix ASAP since these are critical problems.

[evanlinjin]

Looking at how importdescriptors is called...

bdk/src/blockchain/rpc.rs

Lines 201 to 204 in 3644a45

	json!({
	"timestamp": "now",
	"desc": format!("{}#{}", desc, get_checksum(&desc).unwrap()),
	})

And the documentation for importdescriptors...

We see that old wallets cannot recover funds using BDK which syncs with RpcBlockchain.

[evanlinjin]

Looking at how importdescriptors is called...

bdk/src/blockchain/rpc.rs

Lines 201 to 204 in 3644a45

	json!({
	"timestamp": "now",
	"desc": format!("{}#{}", desc, get_checksum(&desc).unwrap()),
	})

And the documentation for importdescriptors...

We see that old wallets cannot recover funds using BDK which syncs with RpcBlockchain.

I was wrong. Transactions are fetched later via multiple calls with rescanblockchain.