Silent Payments: Receiving #32966

Eunovo · 2025-07-14T08:30:35Z

This PR is part of integrating silent payments into Bitcoin Core. Status and tracking for the project is managed in #28536
This PR is based on #28201 and will remain in draft until #28201 is merged.

This PR:

Adds a Silent Payments descriptor implemenation
Adds a SilentPaymentsDescriptorScriptPubKeyMan Impl that is a subclass of DescriptorScriptPubKeyMan
Implements Silent Payments scanning for the wallet
Updates sending logic to use silent-payments destination for change when sending to silent-payments destination
Adds unit and functional tests for silent payments-related functionality

Follow-ups

Silent Payments Label functionality is incomplete in this PR and will be added as a follow-up PR.

DrahtBot · 2025-07-14T08:30:40Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32966.

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#33169 (interfaces, chain, refactor: Remove unused getTipLocator and incaccurate getActiveChainLocator by pablomartin4btc)
#33163 (BIP360: Includes the following: by jbride)
#33116 (refactor: Convert uint256 to Txid by marcofleon)
#33112 (wallet: relax external_signer flag constraints by Sjors)
#33043 ([POC] wallet: Enable non-electronic (paper-based) wallet backup with codex32 by w0xlt)
#33034 (wallet: Store transactions in a separate sqlite table by achow101)
#32977 (wallet: Remove wallet version and several legacy related functions by w0xlt)
#32896 (wallet, rpc: add v3 transaction creation and wallet support by ishaanam)
#32861 (Have createwalletdescriptor auto-detect an unused(KEY) by Sjors)
#32784 (wallet: derivehdkey RPC to get xpub at arbitrary path by Sjors)
#32763 (wallet: Replace CWalletTx::mapValue and vOrderForm with explicit class members by achow101)
#32652 (wallet: add codex32 argument to addhdkey by roconnor-blockstream)
#32579 (headerssync: Preempt unrealistic unit test behavior by hodlinator)
#32523 (wallet: Remove isminetypes by achow101)
#32489 (wallet: Add exportwatchonlywallet RPC to export a watchonly version of a wallet by achow101)
#32471 (wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key by Eunovo)
#31974 (Drop testnet3 by Sjors)
#31514 (wallet: allow label for non-ranged external descriptor (if internal=false) & disallow label for ranged descriptors by scgbckbone)
#30243 (descriptors: taproot partial descriptors by Eunovo)
#30157 (Fee Estimation via Fee rate Forecasters by ismaelsadeeq)
#29136 (wallet: addhdkey RPC to add just keys to wallets via new unused(KEY) descriptor by achow101)
#28333 (wallet: Construct ScriptPubKeyMans with all data rather than loaded progressively by achow101)
#27865 (wallet: Track no-longer-spendable TXOs separately by achow101)
#27260 (Enhanced error messages for invalid network prefix during address parsing. by portlandhodl)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

DrahtBot · 2025-07-14T10:32:42Z

🚧 At least one of the CI tasks failed.
_{Task tidy: https://github.com/bitcoin/bitcoin/runs/45909076324}
_{LLM reason (✨ experimental): The CI failure was caused by errors in clang-tidy checks, specifically in descriptor.cpp and walletutil.cpp, which are treated as failures due to warnings-as-errors settings.}

Hints

Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

Sjors · 2025-07-14T11:23:42Z

src/wallet/rpc/wallet.cpp

@@ -358,6 +358,7 @@ static RPCHelpMan createwallet()
            {"descriptors", RPCArg::Type::BOOL, RPCArg::Default{true}, "If set, must be \"true\""},
            {"load_on_startup", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."},
            {"external_signer", RPCArg::Type::BOOL, RPCArg::Default{false}, "Use an external signer such as a hardware wallet. Requires -signer to be configured. Wallet creation will fail if keys cannot be fetched. Requires disable_private_keys and descriptors set to true."},
+            {"silent_payments", RPCArg::Type::BOOL, RPCArg::Default{false}, "Enable creating and receiving with a silent payments address. Will disable fast rescans with block filters"},


In 89831da wallet/rpc: add create silent-payments wallet option: while testing I found that it doesn't disable block filters.

Thanks @Sjors. I'll fix it

We can disable the fast rescan for now just to keep this PR focused, but in theory we should be able to use the fast rescan regardless of whether or not the wallet is a silent payments wallet. Perhaps we can leave a comment to enable fast rescans in a follow up?

IIUC The silent payments wallet needs to scan entire blocks since it doesn't know ahead of time which scriptPubKeys to use in a GCS filter. Since we can't skip blocks, fast rescans shouldn't provide any advantages here.

EDIT
It might also be possible to use fast rescan when importing descriptors if none of the descriptors are silent payment descriptors. Is this what you were referring to @josibake?

The silent payments wallet needs to scan entire blocks since it doesn't know ahead of time which scriptPubKeys to use in a GCS filter.

This makes sense. Is it documented anywhere so we don't forget by the next review round? :-)

It might also be possible to use fast rescan when importing descriptors if none of the descriptors are silent payment descriptors.

It wasn't referring to that and I wouldn't worry about this. Somewhere in the RPC documentation (createwallet and/or importdescriptors) we should warn that silent payments disables fast rescan. But -blockfilterindex isn't on by default and so it's probably only advanced users that will run into this.

This makes sense. Is it documented anywhere so we don't forget by the next review round? :-)

I added a separate commit that disables fast rescan and I added an explanation to the commit message. Will push soon.

EDIT
see 82db7cc

The silent payments wallet needs to scan entire blocks since it doesn't know ahead of time which scriptPubKeys to use in a GCS filter

This is correct. I was imagining a world where we can pre-compute the scriptPubKeys, but realised we have to iterate the blocks anyways to get the tweaks so its likely faster to just check directly. In the future, if the wallet has some sort of silent payments index, then I think fast rescans would be possible (and beneficial).

Sjors · 2025-07-14T20:13:21Z

Another thing I noticed while testing using the send RPC, in a wallet with only an sp() descriptor, is that it insists on having a bech32 descriptor for change:

Transaction needs a change address, but we can't generate it. Error: No bech32 addresses available.

It seems safer, for backups, to send change to the same silent payment address.

Wallet created by:

bitcoin rpc createwallet Silent blank=true silent_payments=true
bitcoin rpc importdescriptors '[{"desc": "sp(xprv...,xprv...)",  "timestamp": "now", "active": true]

Also looking forward to have labels back :-)

josibake

Did a big overview with @Sjors , leaving the notes from our discussion as a review. In general, I think we should investigate using a custom type for the scan key since a lot of the changes seem to be hacking around the fact we represent the scan key as a private key, but then often need to use it as not a private key.

Still in the process of reviewing, but leaving the initial notes for now

josibake · 2025-07-15T14:14:18Z

src/script/descriptor.cpp

@@ -206,6 +206,8 @@ struct PubkeyProvider
    /** Get the descriptor string form including private data (if available in arg). */
    virtual bool ToPrivateString(const SigningProvider& arg, std::string& out) const = 0;

+    virtual std::optional<std::string> ToPrivateString(const CKey& key) const = 0;


While reviewing the PR, it seems a lot of things could be simplified by if we just treated the scan key as something that isn't a CKey, i.e., we introduce a completely new object for holding the scan key. I think that would allow us to drop this commit and simplify other places where we are trying to work around the fact that a scan key isnt really a private key.

I haven't thought this all the way through yet, but in my initial pass on the PR it seems like it should be possible.

src/outputtype.cpp

josibake · 2025-07-15T14:25:43Z

src/script/descriptor.cpp

@@ -776,7 +776,7 @@ class DescriptorImpl : public Descriptor
    }

    // NOLINTNEXTLINE(misc-no-recursion)
-    void ExpandPrivate(int pos, const SigningProvider& provider, FlatSigningProvider& out) const final
+    void ExpandPrivate(int pos, const SigningProvider& provider, FlatSigningProvider& out) const override


Potentially another area we could simplify if we end up not treating a scan key as a CKey.

Confirmed, considering we don't need ExpandPrivate anymore, this can remain const final.

josibake · 2025-07-15T14:29:22Z

src/script/descriptor.cpp

+        out.sp_keys = std::make_pair(m_scan_key, *spend_pubkey);
+
+        auto it{tmp.keys.find(spend_pubkey->GetID())};
+        if (it != tmp.keys.end()) out.keys.emplace(spend_pubkey->GetID(), it->second);


AFAICT, we only use the sp_keys member of the FlatSigningProvider as a way of passing around the scan key and the spend pubkey, and then also for the spend private key (for signing). It makes sense to me that we would later add the spend private key + tweaks to the FlatSigningProvider, but ideally thats the only thing we use it for and can get rid of sp_keys.

Without sp_keys, the wallet can't identify which keys are scan and spend keys from the signing provider. Do you have an alternative approach in mind?

src/script/descriptor.cpp

josibake · 2025-07-15T15:09:46Z

src/wallet/scriptpubkeyman.cpp

+    return CTxDestination{op_dest};
+}
+
+bool SilentPaymentDescriptorScriptPubKeyMan::TopUp(const uint256& tweak)


The name TopUp here makes sense in that we are trying to match how the rest of the wallet does it, but @Sjors pointed out this can be confusing for people who think of TopUp in terms of the gap limit / key pool refresh context, which we aren't really doing here. We could just have one function called AddTweak and then use a boolean for writing to the database, e.g., AddTweak(uint256& tweak, store_in_db=false)

josibake · 2025-07-15T15:11:00Z

src/wallet/scriptpubkeyman.cpp

+    return res;
+}
+
+bool SilentPaymentDescriptorScriptPubKeyMan::TopUpWithDB(WalletBatch& batch, const uint256& tweak)


Could be AddTweakWithDB (based on my previous comment).

Worth mentioning, the only reason we wouldn't be able to do this is if this function (and TopUp) are called generically on SPKMans, e.g., in a loop. I don't think this is the case, but wanted to mention it.

josibake · 2025-07-15T15:11:54Z

src/wallet/scriptpubkeyman.cpp

+FlatSigningProvider SilentPaymentDescriptorScriptPubKeyMan::GetSPProvider() const
+{
+    AssertLockHeld(cs_desc_man);
+    FlatSigningProvider provider;
+    provider.keys = GetKeys();
+    FlatSigningProvider tmp;
+    m_wallet_descriptor.descriptor->ExpandPrivate(0, provider, tmp);
+    return tmp;
+}


Can likely be removed if we have custom methods for storing scan/spend pubkey data, or custom methods for getting them.

src/interfaces/chain.h

josibake · 2025-07-15T15:15:54Z

src/wallet/wallet.cpp

    // Scan block
    bool wallet_updated = false;
    for (size_t index = 0; index < block.data->vtx.size(); index++) {
+        CTransactionRef tx{block.data->vtx.at(index)};


Do we need the CTransactionRef here, as opposed to just operating on block.data->vtx directly?

We only use the reference once, so it's not needed. I'll take this out as I retouch.

I ended up leaving the reference and used it in 2 other locations. The code looks slightly cleaner this way.

macgyver13 · 2025-08-18T12:17:45Z

@Eunovo Nice work on addressing the issues identified in previous tests. All of those are working as expected now.

I am including a patch that adds 2 more failing tests when adding silent-payments address in the transaction output:
self.test_sendrawtransaction()
self.test_mixed_output_types()

test_sp_receive.patch

Eunovo · 2025-08-19T13:09:04Z

@Eunovo Nice work on addressing the issues identified in previous tests. All of those are working as expected now.

I am including a patch that adds 2 more failing tests when adding silent-payments address in the transaction output: self.test_sendrawtransaction() self.test_mixed_output_types()

test_sp_receive.patch

I think these might be more appropriate for the sending PR #28201 cc @josibake

While the scankey looks like a private key, and is represented as a `CKey` in the code, it is actually much closer to a public key and it needs to be treated as a public key, so it can be added to watch-only wallets.

sp() expects a scan private key and a spend key which could be a private key or a public key. The scan key is really a "scan entropy" and will not be treated as an actual private key.

Create an SPDescriptor interface that will be used by the `SilentPaymentDescriptorScriptPubKeyMan`. The interface only needs to provide a way to get the scan key, the spend key can be obtained through the normal Descriptor methods. While the scan key is represented as a `CKey`, it will not be used in the signing process. This creates an argument against putting this in a signing provider.

This method makes it easier to tweak silent payment spend keys in later commits

The spent_coins data will be used to scan for silent payments in later commits Co-authored-by: Ava Chow <github@achow101.com>

This undo data will be used to scan for silent payments in later commits Co-authored-by: Ava Chow <github@achow101.com>

The scan key will be derived while the descriptor is parsed, so we must pass the master xpriv into `GenerateWalletDescriptor`. The spend key is expanded later so it is specified as an xpub and path instead.

Co-authored-by: Ava Chow <github@achow101.com>

silent_payments wallet needs to scan entire blocks since it doesn't know ahead of time which scriptPubKeys to use in it's fast rescan filter.

Co-authored-by: Ava Chow <github@achow101.com>

traditionally, the receiving scripts are known to the core wallet because they are added to the addressbook at the time they are requested. in silent payments, the outputs are not known until found. its important, however, to have the found scripts in the addressbook so all of the change accounting can be down properly. this commits adds found outputs to the address book if they are not change. the way change is determined is a bit hacky in that we just check if the found output was found with a label (since this is the only label currently implemented). in the future, we should specifically check for a change specific label.

The wallet tries to match change OutputTpe to other outputs to preserve privacy. Use silent payment change address when sending coins to: - any silent payment address - any taproot address

`CreateTransaction` fails if change_type is set to OutputType::SILENT_PAYMENTS and there are no eligible inputs for the transaction. This commit modifies `CreateTransaction` logic to make a second attempt to create a tx without OutputType::SILENT_PAYMENTS change_type

This allows wallets with only sp descs to use sp change destination for transactions.

Co-authored-by: macgyver13 <4712150+macgyver13@users.noreply.github.com>

Eunovo · 2025-08-19T14:28:12Z

Some Major Updates:

The SilentPaymentDescriptorScriptPubKeyMan now obtains the scan key through a GetScanKey method, only available on Silent Payment descriptors
The spend key is no longer derived during descriptor parsing; it now has to be "expanded" the same way other descriptors are expanded
LoadKeysAndChangeLabel has been replaced with an overridden TopUpWithDB method. The wallet calls TopUp when it wants the SPKMans to derive and load new keys and scripts.

josibake mentioned this pull request Jul 10, 2025

BIP352 tracking issue #28536

Open

15 tasks

DrahtBot added the CI failed label Jul 14, 2025

Sjors reviewed Jul 14, 2025

View reviewed changes

DrahtBot mentioned this pull request Jul 15, 2025

wallet: Remove wallet version and several legacy related functions #32977

Merged

josibake reviewed Jul 15, 2025

View reviewed changes

Add OutputType::SILENT_PAYMENTS

768f20b

Eunovo force-pushed the 2025-implement-bip352-receiving branch 2 times, most recently from 4b49704 to f86b9d2 Compare August 18, 2025 08:35

Eunovo and others added 23 commits August 19, 2025 16:07

descriptors: impl ScankeyPubkeyProvider

f60836f

While the scankey looks like a private key, and is represented as a `CKey` in the code, it is actually much closer to a public key and it needs to be treated as a public key, so it can be added to watch-only wallets.

descriptor: implement sp(scan, spend) desc

3868244

sp() expects a scan private key and a spend key which could be a private key or a public key. The scan key is really a "scan entropy" and will not be treated as an actual private key.

pubkey: add TweakAdd() method

958c134

This method makes it easier to tweak silent payment spend keys in later commits

notifications: Add spent_coins to tx added notification

b9f3aa3

The spent_coins data will be used to scan for silent payments in later commits Co-authored-by: Ava Chow <github@achow101.com>

interfaces: Load block undo data

95420df

This undo data will be used to scan for silent payments in later commits Co-authored-by: Ava Chow <github@achow101.com>

wallet: generate sp() descriptor

6ca21d1

The scan key will be derived while the descriptor is parsed, so we must pass the master xpriv into `GenerateWalletDescriptor`. The spend key is expanded later so it is specified as an xpub and path instead.

wallet: create SilentPaymentDescriptorScriptPubKeyMan

15ab99d

Co-authored-by: Ava Chow <github@achow101.com>

wallet: scan for silent payments

826202a

Co-authored-by: Ava Chow <github@achow101.com>

wallet/rpc: add create silent-payments wallet option

b7acc7d

Co-authored-by: Ava Chow <github@achow101.com>

wallet: disable fast rescan for silent_payments wallet

d96ef13

silent_payments wallet needs to scan entire blocks since it doesn't know ahead of time which scriptPubKeys to use in it's fast rescan filter.

wallet: import silent payments descriptor

c3600e2

tests: add silent payments receiving functional tests

d861f6c

Co-authored-by: Ava Chow <github@achow101.com>

wallet/rpc: add sp-address support to getaddressinfo

157f48b

use silent payments change when sending

bdee098

The wallet tries to match change OutputTpe to other outputs to preserve privacy. Use silent payment change address when sending coins to: - any silent payment address - any taproot address

wallet/spend: Filter out coins not eligible to pay sp change dest

5971a06

wallet: prefer silent_payments over Bech32m as fallback change type

bf96935

This allows wallets with only sp descs to use sp change destination for transactions.

wallet/tests: Enable Silent Payments in wallet unit tests

1957fd6

wallet/gui: add silent_payments option to createwalletdialog

a7eb82c

wallet/gui: add silent-payments to address type dropdown

dca0bbd

tests: silent-payments wallet coin control

7150948

Co-authored-by: macgyver13 <4712150+macgyver13@users.noreply.github.com>

Eunovo force-pushed the 2025-implement-bip352-receiving branch from f86b9d2 to 7150948 Compare August 19, 2025 14:08

Silent Payments: Receiving #32966

Are you sure you want to change the base?

Silent Payments: Receiving #32966

Uh oh!

Conversation

Eunovo commented Jul 14, 2025

Uh oh!

DrahtBot commented Jul 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

Uh oh!

DrahtBot commented Jul 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Eunovo Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Sjors Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Eunovo Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Sjors commented Jul 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

josibake left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Eunovo Jul 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

macgyver13 commented Aug 18, 2025

Uh oh!

Eunovo commented Aug 19, 2025

Uh oh!

Eunovo commented Aug 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

DrahtBot commented Jul 14, 2025 •

edited

Loading

Eunovo Jul 21, 2025 •

edited

Loading

Sjors Jul 21, 2025 •

edited

Loading

Eunovo Jul 21, 2025 •

edited

Loading

Sjors commented Jul 14, 2025 •

edited

Loading

Eunovo Jul 20, 2025 •

edited

Loading

Eunovo commented Aug 19, 2025 •

edited

Loading