[Draft / POC] Silent Payments #24897

This PR proposes an early version of Silent Payment (author:@RubenSomsen). In this scheme, the recipient generates a public address, but the sender tweaks the address and the recipient detects the payment by verifying all transactions on the blockchain. An example use case would be private donations.

The purpose of this PR is not a final version, but to start the discussion and get benchmarks based on a real implementation.

This version is built on top of #994 (bitcoin-core/secp256k1) for x-only ECDH support and #23480 (bitcoin/bitcoin) for rawtr(). Each new silent transaction detected is stored in wallet as a rawtr() descriptor.

In this implementation, the sender can tweak the recipient address by passing the silent_payment option to send RPC. The transaction output will be different from the address entered.

For example ./src/bitcoin-cli -regtest -named send outputs="[{\"bcrt1pwlh5xuyrpgfunwyww8cfu78yfs2yqyevl7yturavahh5kgxwdd2q5hzgfu\": 1.1}]" fee_rate=1 options="{ \"silent_payment\": true}".

will generate vout with completely unrelated outputs:

 0"vout": [
 1    {
 2      "value": 1.10000000,
 3      "n": 0,
 4      "scriptPubKey": {
 5        "desc": "rawtr(65b19890c5ca40edb816d26f5f48cd9f3ed51121613b1c2405adc1a6dbbc824a)#8myx9tcu",
 6        "address": "bcrt1pvkce3yx9efqwmwqk6fh47jxdnuld2yfpvya3cfq94hq6dkausf9qrfjkgz",
 7
 8      }
 9    },
10    {
11      "value": 2.02499835,
12      "n": 1,
13      "scriptPubKey": {
14        "desc": "rawtr(c45cb3d500bbf8f0c8841e8e011b008781d826c16ee348edb822c0f97419bc4d)#26hcce63",
15        "address": "bcrt1pc3wt84gqh0u0pjyyr68qzxcqs7qasfkpdm353mdcytq0jaqeh3xsuvlykg",
16      }
17    }
18  ]

Any wallet, as long as it has access to private keys, can send silent payments. Thus, this excludes watch-only wallets or wallets with external signers .

But the recipient’s wallet needs a new flag called SILENT_PAYMENT. This flag allows an additional scan that verifies that the wallet keys match the silent payment scheme. When it detects a silent payment that belongs to the wallet, it is stored in a rawtr() descriptor.

./src/bitcoin-cli -regtest -named createwallet wallet_name="recipient" silent_payment=true

Therefore, scanning each address for each transaction is potentially prohibitive overhead, so the node can be initialized with keypool=1 or a descriptor with range [0,1] can be imported into a blank wallet. Until there is more benchmark data, it is the safest option. The proposal recommends one static address.

I’ve been running some silent payments on signet using wallets with default keypool and default range, I haven’t noticed any relevant performance drops on the signet node. Apparently this implementation is working as expected but I can’t guarantee that the scheme is implemented correctly or safely, so I’m opening this PR for reviews, modifications and improvements.

There is a new functional test (test/functional/wallet_silentpayment.py) that can help to better understand the implementation.

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

Reviews

See the guideline for information on the review process.

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #bitcoin-core/gui/650 (Add Import to Wallet GUI by KolbyML)
• #27215 (wallet: Turn destdata entries into CAddressBookData fields by achow101)
• #26951 (wallet: improve rescan performance 640% by pstratem)
• #26858 (wallet: Use defined purposes instead of inlining by aureleoules)
• #26840 (refactor: importpubkey, importprivkey, importaddress, importmulti, and importdescriptors rpc by KolbyML)
• #26836 (wallet: finish addressbook encapsulation by furszy)
• #26728 (wallet: Have the wallet store the key for automatically generated descriptors by achow101)
• #26177 (refactor / kernel: Move non-gArgs chainparams functionality to kernel by TheCharlatan)
• #26129 (wallet, refactor: FundTransaction(): return out-params as util::Result structure by theStack)
• #26094 (rpc: Return block hash & height in getbalances, gettransaction and getwalletinfo by aureleoules)
• #26076 (Switch hardened derivation marker to h (in normalized descriptors and new wallets) by Sjors)
• #26008 (wallet: cache IsMine scriptPubKeys to improve performance of descriptor wallets by achow101)
• #25991 (Wallet: Add foreign_outputs metadata to support CoinJoin transactions by luke-jr)
• #25907 (wallet: rpc to add automatically generated descriptors by achow101)
• #25806 (wallet: group outputs only once, decouple it from Coin Selection by furszy)
• #25766 (wallet: Include a signature with encrypted keys to mitigate a wallet scam by achow101)
• #25722 (refactor: Use util::Result class for wallet loading by ryanofsky)
• #25634 (wallet, tests: Expand and test when the blank wallet flag should be un/set by achow101)
• #25620 (wallet: Introduce AddressBookManager by furszy)
• #25297 (wallet: group independent db writes on single batched db transaction by furszy)
• #25273 (wallet: Pass through transaction locktime and preset input sequences and scripts to CreateTransaction by achow101)
• #24914 (wallet: Load database records in a particular order by achow101)
• #23561 (BIP324: Handshake prerequisites by dhruv)
• #23432 (BIP324: CKey encode/decode to elligator-swift by dhruv)
• #22838 (descriptors: Be able to specify change and receiving in a single descriptor string by achow101)
• #22341 (rpc: add getxpub by Sjors)
• #19690 (util: improve FindByte() performance by LarryRuane)
• #19461 (multiprocess: Add bitcoin-gui -ipcconnect option by ryanofsky)
• #19460 (multiprocess: Add bitcoin-wallet -ipcconnect option by ryanofsky)
• #10102 (Multiprocess bitcoin by ryanofsky)

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.

@luke-jr Yes, it will need to be further discussed if this concept proves to be valid.

In this PR, all descriptor wallets support silent transactions as the SILENT_PAYMENT flag can be enabled or disabled at any time. The rescanblockchain RPC can be used to retrieve previous silent payments.

As only Taproot addresses can be used for silent payments and only descriptor wallets support Taproot, this may suffice, but it certainly needs further discussion.

Need some way to avoid users trying to send a silent payment to a wallet that doesn’t support it @luke-jr When the recipient publishes their silent payment address, it needs to be clear that it is not a regular taproot address. The address format should be distinct. The tweaked silent payment address that is generated by the sender and is ultimately committed on-chain should be indistinguishable from a regular taproot output.

only Taproot addresses can be used for silent payments @w0xlt Note that nothing about the silent payment scheme is exclusive to taproot, but regardless I do think it’s good to limit the specifications to creating taproot outputs only as it encourages taproot use and means transactions without taproot outputs (albeit hopefully rare in the eventual future) don’t need to be scanned.

When the recipient publishes their silent payment address, it needs to be clear that it is not a regular taproot address. The address format should be distinct.

But I don’t see that in this current spec/code?

Need some way to avoid users trying to send a silent payment to a wallet that doesn’t support it… am I missing something?

it’s worse than that, AFAIU the address needs to support it not just the wallet – E.g., what if the Tr key is a NUMS point?

Worth making a new address type / format so that you never mix the two.

@luke-jr @JeremyRubin Thanks for the comments. The current version deliberately does not implement a new address/descriptor format. This requires a separate discussion and changes in wallet behavior. And it doesn’t make sense to start a discussion about it before the scheme proves itself valid in a viable time.

Currently the main focus is to validate the scheme and verify that the performance is viable.

@prusnak It s not well documented as well as being discussed.

In the current implementation, for simplicity, only the first input is used.

There is a suggestion by @RubenSomsen to use a hash of all inputs. This would make silent payments compatible with coinjoins.

This is a possibility of change if the current version proves viable, since there shouldn’t be much difference in performance between the two versions.

I posted some estimates here.

ACK https://github.com/bitcoin/bitcoin/pull/24897/commits/8f616acf868bbc75b31bf5170c40fd9957c08e91

I have tested the code by following the steps shared in https://gist.github.com/w0xlt/72390ded95dd797594f80baba5d2e6ee

Silent payment address: dig -t txt alice.silentbitco.in +short

Tx: https://explorer.bc-2.jp/tx/296d4a6e4383886d3ccca9c103c6216fd1cdebb2915b890bf6c04304c04aae8b?expand

Huge Concept ACK..

This is amazing.. 🔥🔥🔥🔥

Very cool! It’s nice to be able to test this concept with “real” demo software. I’ll share some thoughts about the concept on the mailinglist or gist. Will focus on the implementation here.

I was able to send to signet coins to myself. Here’s the public key: tb1p0ykcmjxv26utyntns8ryn70at9t38zkdta9fj3zrrnpnczuddxvqf8qwac

My first thought was that it may be better (initially) to implement silent payments as a standalone tool that uses the Bitcoin Core RPC.

However, given that this implementation requires a new index, I don’t know how practical that is. Getting a new index added to Bitcoin Core however is also not easy (https://bitcoincore.reviews/14053). The light at the end the tunnel here would be a plugin system for indexes, which should be possible once process separation is complete. See #24230. That will take a while though.

Another reason for building a separate tool that uses the RPC is that this proposal has a pretty big resource need, something that the project may not want to commit to anytime soon. The sending side is not resource intensive though, so perhaps that is easier to get in.

Limiting the scope of this implementation to taproot makes sense. Limiting it to the first input seems brittle however, hopefully it’s not too difficult to scan them all. Other implementations might order the input coins differently. Also, if you go for the approach of building an external tool that uses the RPC, then you can could use send with 1 fixed input and have the wallet automatically select the other coins. But in that case the RPC gives no guarantee which position the input will end up (it might be the first in practice atm).

In case the proposal is modified to consider all inputs, keep in mind that this makes coin selection (slightly) more complicated. The wallet has to decide, before it knows the output address, which inputs to use. That would involve some surgery in the way coin selection is integrated in the wallet. When implemented as an external tool, you would probably make two calls to send: one with a dummy destination and automatic coin selection to get the coins, and then one with manual coin selection and the real destination. But that’s not pretty, and won’t work if the wallet has a combination of taproot and non-taproot coins, because you currently can’t tell coin selection to only use taproot inputs.

Importing a descriptor with a shorter range than the keypool size has no effect afaik, it’s just treated as a descriptor with range 0-1000. We could change that behavior, so that a descriptor imported with a fixed range is not expanded unless you re-import it. Not sure if that’s ideal, cc @achow101. Another option might be to make the keypool configurable per wallet, though that also feels a bit meh. A user may have multiple wallets, not all of which use this feature, so relying on a global keypool=1 setting is not ideal. And a factor 1000 slower scanning seems suboptimal.

Another approach could be that a silent_payment wallet only ever generates 1 key, perhaps with a new silent(pk) descriptor type. That might also make it easier for getnewaddress to spit out something that’s not actually an address. See https://gist.github.com/RubenSomsen/c43b79517e7cb701ebf77eec6dbb46b8?permalink_comment_id=4177027#gistcomment-4177027 for discussion around what format the silent payment public key should be communicated in.

Ideally the scriptPubKeys are stored in the silent(pk) descriptor. This would make getaddressinfo return the right descriptor. In that case getnewaddress should always fail, because the addresses are not known in advance. You’d need another way to communicate the public key. The easiest hack might be to return it as public-key version of the silent(pk) descriptor, so the user can find it with either listdescriptors or getaddressinfo on any received payment.

@Sjors @kristapsk @1440000bytes Thanks for reviews and tests. I addressed your suggestions in the last force-push. @Sjors I agree that using a separate wallet/tool ​​for silent payments can be a good solution. I initially considered this option, but it would imply re-implementing the scan logic. The tool must be able to request and de-serialize blocks and the UTXO Set to scan coins. So a new branch appeared to be the simplest way to implement a proof of concept.

In case the proposal is modified to consider all inputs, keep in mind that this makes coin selection (slightly) more complicated.

Yes. There are some tradeoffs involving how to use inputs for ECDH. I don’t think there’s a way to accept every type of input, as seen in https://github.com/bitcoin/bitcoin/pull/24897/commits/178e39d804a092016abe9650a1cde085173d6795.

Example:

0if (whichType == TxoutType::NONSTANDARD ||
1    whichType == TxoutType::MULTISIG ||
2    whichType == TxoutType::WITNESS_UNKNOWN ) {
3        return false;
4    }

given that this implementation requires a new index,

This index just speeds up the transaction verification. This is not necessary for Silent Payments, although in this particular implementation it is required for UX purposes. I’m getting some metrics to see if it can be removed and still have a good user experience.

relying on a global keypool=1 setting is not ideal.

Yes. This is just for testing and should not be required in an eventual final version.

Another approach could be that a silent_payment wallet only ever generates 1 key, perhaps with a new silent(pk) descriptor type.

I think this is the best solution. This would imply changing the getnewaddress RPC to always return the same address for this silent descriptor and allowing a non-ranged descriptor to be active.

Rebased.

why is CI failing?

@1440000bytes this PR is built on top of #23480 and https://github.com/bitcoin-core/secp256k1/pull/994, which have not yet been merged.

One of the error is the same as #23480, which occurs in wallet_taproot.py. This is failing almost every CI check, but not on my machine (Ubuntu 21.10). Regarding the error inwallet_taproot.py, I suggested the solution in #23480 (comment), but had forgotten to commit it here. This error has now been fixed.

The other error is a lint error about manually merging #994. I’m still not sure how to fix this lint error.

EDIT: The only error now is the lint error mentioned above. From what I understand from the file test/lint/git-subtree-check.sh, this is because the src/secp256k1 subtree is compared with the master branch of https://github.com/bitcoin-core/secp256k1 and as it is different, it results in an error.

If so, this error will remain until https://github.com/bitcoin-core/secp256k1/pull/994 is merged.

Anyway, it’s a lint error (formatting basically). Does not affect functionality.

Another approach could be that a silent_payment wallet only ever generates 1 key, perhaps with a new silent(pk) descriptor type. That might also make it easier for getnewaddress to spit out something that’s not actually an address.

How would you apply a label and determine who the sender is?

Also, being non-reversible from the UTXO doesn’t make it any less of an address…

@luke-jr Not sure if I understand your question, but this scheme doesn’t change anything regarding the sender/recipient relationship. The sender will know the tweaked address and the recipient will know where the coins are coming from.

What this proposal prevents is external observers from knowing or tracking how many coins or transactions a public address has. And it also prevents reuse of a public address (as it happens in on-chain donations, for example).

Not sure if I understand your question, but this scheme doesn’t change anything regarding the sender/recipient relationship. The sender will know the tweaked address and the recipient will know where the coins are coming from.

The recipient knows who the sender is and the purpose of the transaction, based on the address the bitcoins were sent to. If getnewaddress gave the same address every call, it would be impossible to tell the difference between them.

And it also prevents reuse of a public address (as it happens in on-chain donations, for example).

I thought the whole point was to make reuse of such an address safe?

If getnewaddress gave the same address every call

You can label the tweaked address. It is part of the wallet. The rawtr(pk) descriptor is normally added to wallet.

I thought the whole point was to make reuse of such an address safe?

Yes. That’s the point. The same address will be tweaked on every transaction

As I understand it, the purpose of SP is primarily to increase privacy in a situation where you need to publicly post an address and anyone can send coins to it.

If you need more control to categorize each address by payer, an HD wallet is better.

You can label the tweaked address. It is part of the wallet.

That seems like a bad design… it should show the silent address.

But more importantly, for the recipient to set a label, he has to know what it’s for first….

it should show the silent address.

This can be done. The wallet knows that a certain transaction / address was found by verifying SP.

This PR has been updated with a new silent payment version, which eliminates some manual steps from the previous version (such as the need to set the keypool to avoid costly multi-key scan).

This is achieved by using a new descriptor type (sp()) that has no range and contains exactly one key.

Example: sp(cQq73sG9....JD51uaRD)#9llg6xjm

This descriptor introduces a new type of output: silent-payment. This output type returns a standard Taproot script (Segwit V1), but with HRP changed from bc to sp on the mainnet (or tsp on testnet and signet).

This output type will always generate the same address (unless another sp descriptor is enabled on the same wallet).

0$ ./src/bitcoin-cli -signet getnewaddress '' 'silent-payment'
1tsp1pfmjyl7ecpmx8yf8cu6g3ez36jy7s9mzuh5pdnal3k0n588uzgmfs4s4fws

To create a silent transaction, simply use the silent payment address as one of the outputs. The send RPC will automatically identify and tweak it.

The transaction can contain multiple outputs, combining silent and standard addresses.

I have written a step by step signet tutorial so reviewers can test this new version easily. https://gist.github.com/w0xlt/a7b498ac1ff14b8c292a22be789bd93f

Many people don’t receive only donations. If you have multiple clients, you need a different address for each so you know who is paying you. @luke-jr client could share txid in which he sent the payment

@1440000bytes yes, those hacks are available but I’d prefer to avoid such hacks because they significantly degrade UX.

Not sure I would consider it hack because it makes things simple. Even BIP 47 has one reusable code per wallet so I would find it confusing to have multiple silent payment address in the same wallet. Although no strong opinion and won’t mind anything as long as silent payment address is available in core.

• May make spending funds together impossible/difficult
• Managing multiple wallets is PITA
• Sending txid is just more work for humans that computers could do instead

I think these are enough reasons to at least support multiple addresses in descriptors and leaving it up to wallets to decide whether to provide support for more than one in GUI/CLI/…

client could share txid in which he sent the payment

Anyone could claim to have sent it. There is still no proof-of-sender in Bitcoin, and it doesn’t look like anyone is working on it.

Even if it were possible, it’d still be a pain - this kind of thing should “just work” and label transactions, without extra steps from the users.

Even BIP 47 has one reusable code per wallet

BIP 47 was a bad design that shouldn’t be implemented nor imitated…

The default behavior for most descriptors is to be ranged and therefore support multiple addresses. If the idea is to have multiple sp addresses, a ranged sp version is sufficient (and is easier to implement than the current version).

The decision to use a rangeless descriptor is for validation purposes. Verification of silent payments is costly. And if the user inadvertently generates multiple sp addresses, the overhead can be prohibitive. Until there are more reliable statistics on how many sp addresses a node (even on a raspberrey) can process (from mempool and when loading wallets), I think it’s prudent to keep only one active address per wallet.

The label issue shouldn’t be complicated, I think. UTXO can inherit the same label assigned to the sp address or descriptor. I can work on that in the next iteration.

I think there is a use case for multiple SP addresses @Kixunil good point about wanting to identify the reason for the payment. Naively, the issue is that two keys means twice the scanning, but an interesting alternative would be to simply use the same key (assuming you’re OK with using the same identity) but add a public identifier f to it when tweaking. So instead of hash(i*X)*G + X you get hash(i*X)*G + X + f*G . This means every additional “address” only costs one additional ECC addition when scanning (relatively cheap compared to doing ECC multiplications).

The main downside with this is that f becomes crucial for recovering from backup. If we set f as an index (0, 1, 2, 3…) then you’d only have to remember how many “addresses” you issued (and perhaps overshoot when unsure) to ensure recovery of funds, though of course you’d rather also remember which index is associated with what payment reason.

Absolute worst case scenario you could even do something similar to the gap limit where you scan the full history (not just the UTXO set so you don’t miss spent outputs) with a default max index of e.g. 100, and then if you find out most of them are in use, you scan the next 100, etc. Costly, but thorough, and only needed as a last resort.

This PR has been updated with a new silent payment version, which eliminates some manual steps from the previous version (such as the need to set the keypool to avoid costly multi-key scan).

This is achieved by using a new descriptor type (sp()) that has no range and contains exactly one key.

Example: sp(cQq73sG9....JD51uaRD)#9llg6xjm

This descriptor introduces a new type of output: silent-payment. This output type returns a standard Taproot script (Segwit V1), but with HRP changed from bc to sp on the mainnet (or tsp on testnet and signet).

This output type will always generate the same address (unless another sp descriptor is enabled on the same wallet).

0$ ./src/bitcoin-cli -signet getnewaddress '' 'silent-payment'
1tsp1pfmjyl7ecpmx8yf8cu6g3ez36jy7s9mzuh5pdnal3k0n588uzgmfs4s4fws

To create a silent transaction, simply use the silent payment address as one of the outputs. The send RPC will automatically identify and tweak it.

The transaction can contain multiple outputs, combining silent and standard addresses.

I have written a step by step signet tutorial so reviewers can test this new version easily. https://gist.github.com/w0xlt/a7b498ac1ff14b8c292a22be789bd93f

Tested the new version and everything worked fine except scanning UTXO set: https://gist.github.com/w0xlt/a7b498ac1ff14b8c292a22be789bd93f?permalink_comment_id=4286510#gistcomment-4286510

I have updated the steps I followed here in tutorial section: https://silentbitco.in/

Updated the new silent payment address in DNS as TXT record: tsp1pkr8e083j7vp0w82ry5s53rz3t26yf0p8dzg7gqs53r6y399kyw4qa8z52y

Suggestion: We could add a new RPC command to create silent payment address instead of using getnewaddress. So the CLI command would look like this:

0$ bitcoin-cli getspaddress

@w0xlt can you rebase?

The purpose of this PR is not a final version, but to start the discussion and get benchmarks based on a real implementation.

T> his version is built on top of https://github.com/bitcoin-core/secp256k1/pull/994 (bitcoin-core/secp256k1) for x-only ECDH support and #23480 (bitcoin/bitcoin) for rawtr(). Each new silent transaction detected is stored in wallet as a rawtr() descriptor.

Can you confirm this is one of the version that can be used for bitcoin?

@1440000bytes Done.

I am not sure I understand your question. The idea of current implementation is to be used with Bitcoin. I’m evaluating the possibility of including more than one sp address per wallet, but that would be a future change .

@1440000bytes This isn’t even close to ready for review yet… It still needs at least a BIP and support for generating multiple addresses. And the secp256k1 changes also haven’t been merged yet, much less included in the version Bitcoin Core uses.

Suggestion: We could add a new RPC command to create silent payment address instead of using getnewaddress.

Why? getnewaddress is the correct thing to use here…

This new version addresses most (or all) requests made in PR so far:

. Implements the new scheme suggested by @RubenSomsen that allows multiple silent addresses per wallet with minimal overhead. . Implements a new RPC to retrieve silent addresses (suggested by @1440000bytes), which allows users to assign different labels to different addresses. That way, the user knows which silent address the UTXO came from (as suggested by @luke-jr).

Example:

0./src/bitcoin-cli -signet -rpcwallet="receiver" getspaddress
1tsp001pjgcwd9p6f2rcgf35dlgvj77h2afylg6lp5cdn0cztrk4k54w99kqxn48tq
2
3# This will return the same address as above (both have no label)
4./src/bitcoin-cli -signet -rpcwallet="receiver" getspaddress
5tsp001pjgcwd9p6f2rcgf35dlgvj77h2afylg6lp5cdn0cztrk4k54w99kqxn48tq
6
7# New label, new address
8./src/bitcoin-cli -signet -rpcwallet="receiver" getspaddress 'donation'
9tsp011pjgcwd9p6f2rcgf35dlgvj77h2afylg6lp5cdn0cztrk4k54w99kq80t7lt 

In this new scheme, the address has a new field called identifier, which tells the receiver and sender how to tweak the address correctly.

If the receiver, for whatever reason, doesn’t know which identifiers have been used, there is no problem. The wallet can scan all identifiers from 0 to 99. Currently, only 100 different identifiers per wallet are allowed. This limit, however, can be increased at any time in the future.

Unlike address formats so far, sp addresses are not script-related and may eventually include any additional information needed, such as an expiration timestamp (or block height). That way, users don’t have to track the address indefinitely.

As usual I wrote a step by step tutorial: https://gist.github.com/w0xlt/c81277ae8677b6c0d3dd073893210875

@1440000bytes This isn’t even close to ready for review yet… It still needs at least a BIP and support for generating multiple addresses. And the secp256k1 changes also haven’t been merged yet, much less included in the version Bitcoin Core uses.

Suggestion: We could add a new RPC command to create silent payment address instead of using getnewaddress.

Why? getnewaddress is the correct thing to use here…

I respect for your review. At least we know whats needed for bitcoin core/knots to use this.

@1440000bytes This new push addresses all requests made so far, but as @luke-jr said, there is still a long way to go before this PR is merged (if at all). https://github.com/bitcoin-core/secp256k1/pull/994 needs to be merged first.

Why? getnewaddress is the correct thing to use here…

I accepted the suggestion of @1440000bytes because getspaddress will not necessarily return a new address. If the user repeats the label, the address will be the same, so getnewaddress does not exactly represent the expected behavior.

@1440000bytes This new push addresses all requests made so far, but as @luke-jr said, there is still a long way to go before this PR is merged (if at all). bitcoin-core/secp256k1#994 needs to be merged first.

Why? getnewaddress is the correct thing to use here…

I accepted the suggestion of @1440000bytes because getspaddress will not necessarily return a new address. If the user repeats the label, the address will be the same, so getnewaddress does not exactly represent the expected behavior.

I dont want this as another “proof of concept”

Users need privacy right now and it will be an inspiration when core wallet does it with something like silent payment. We don’t need to share bitcoin address on social media.

Ross https://twitter.com/raw_avocado/status/1575110869809143808 got it wrong with opsec by sharing email address on bitcointalk. Lot of users were tracked because they shared bitcoin address on social media. Lets fix this partially by allowing the users to share a silent payment address.

This new version addresses most (or all) requests made in PR so far:

I just finished going over the BIPs PRs and didn’t see one for this yet.

In this new scheme, the address has a new field called identifier, which tells the receiver and sender how to set the address correctly.

Wouldn’t this allow someone who obtains two addresses, to determine they are both for the same wallet? Seems like a major privacy problem.

Currently, only 100 different identifiers per wallet are allowed. This limit, however, can be increased at any time in the future.

Any limit seems problematic, but 100 is definitely too few.

Why? getnewaddress is the correct thing to use here…

I accepted the suggestion of @1440000bytes because getspaddress will not necessarily return a new address. If the user repeats the label, the address will be the same, so getnewaddress does not exactly represent the expected behavior.

I see, that makes sense. getaccountaddress used to do something similar. We no longer have accounts, but maybe a more generic name getreusableaddress would be better. (I assume getnewaddress can also be used still?)

Currently, only 100 different identifiers per wallet are allowed. This limit, however, can be increased at any time in the future.

This identifier is encoded in the sp address, isn’t it sensitive information to leak? Any limit seems too low, especially 100. Can’t the identifier be encoded using a CompactSize?

This identifier is encoded in the sp address, isn’t it sensitive information to leak?

A common feature of these stealth address schemes (BIP47, BIP351) is that a static identifier may be safely associated with your identity without loss of privacy.

There seems to be some confusion about the function of the identifier. Its function is not to create more than one identity, but to be able to distinguish why someone paid you. For example, when the same entity is raising money on behalf of two different charities and wants to know for which of the two charities the sender intended their donation.

To be absolutely clear, the payments that appear on-chain are still absolutely unlinkable by a third party observer, nor are they able to tell which identifier was used.

In cases where you don’t want people to know that you’re the same entity, the identifier is insufficient – you’ll need a completely separate Silent Payment address which (roughly) doubles your scanning efforts, so it’s much more costly in terms of performance.

@luke-jr I suspect we’re not yet on the same page. Could you be specific about what you think is being leaked? Essentially all that is happening is that the person who is getting paid is publicly saying “you could send me money for purpose A or purpose B, so please tweak the final address a bit so I know for which purpose you sent me money”. The resulting on-chain address is still completely unrecognizable by outside parties.

So essentially all that is happening is that a few extra secret bits of information are being transmitted to the recipient along with the payment. There is no impact on privacy other than the fact that the recipient voluntarily made a statement about how it would interpret those bits.

Of course this is not equivalent to having a completely separate identity on which you could get paid, but that is not its purpose. I.e. the difference is A says pay me for X, B says pay me for Y, and A might be the same person as B but nobody knows, or A saying pay me for X or Y so we know the same person will receive the funds no matter if you pay for X or Y. Both are useful. One is more flexible than the other, but also more computationally expensive.

@luke-jr I agree it is a communication challenge to explain such a feature to users, but I hope you see now that it can be of benefit when used correctly. I also agree that this idea is not adequate in cases where you do not want people to know that the same entity is receiving money for separate purposes.

Another example which may be more clear: maybe you’re a known open source developer who is openly working on multiple projects and you want to distinguish for which of these projects your supporters are donating.

@aureleoules Thanks for the detailed review. I addressed all the suggestions.

the user asks for a new address, they should get a new address @luke-jr as mentioned earlier, that’s why a new RPC was created instead of using getnewaddress, just so the user doesn’t think a new address will be generated.

Silent Payment v4 (coinjoin support added)

Changes:

. Silent payments now use all inputs to create transactions. Previously, they only used the first input. This change increases privacy and makes silent payments compatible with coinjoin.

. getspaddress RPC renamed to getsilentaddress for clarity

. Added support for silent payment in PSBT via walletcreatefundedpsbt RPC.

. Added a new index scheme (which stores the sum of input public keys for each transaction). The previous index bitcoin/signet/indexes/silentpaymentindex should be removed as it is no longer compatible with this new version.

For reviewers:

Now, silent payments use the scheme hash(i1*X + i2*X + i3*X + ...)*G + X == hash(x*(I1+I2+I3+...))*G + X, as described here: https://gist.github.com/RubenSomsen/c43b79517e7cb701ebf77eec6dbb46b8#variant-using-all-inputs

As inputs can be Taproot, this introduced a new issue as bitcoin-core/secp256k1 does not support x-only public key sum (perhaps due to missing prefix byte).

I opened a new PR https://github.com/bitcoin-core/secp256k1/pull/1143 to add a function to convert from x-only to compressed public key with even y. This is the solution being used by the current silent payment implementation.

Tutorial updated: https://gist.github.com/w0xlt/c81277ae8677b6c0d3dd073893210875

How about returning a JSON object with some warnings for new labels to explicit that new labels are not new identities?

 0./src/bitcoin-cli -testnet -rpcwallet=sp1 getsilentaddress client1 
 1{
 2  "address": "tsp071pp809z56vwvyqhwc4fsjq8vhjw6yyguzv2u9hnwt3jvyl2ha49nwqtsc8w9",
 3  "index": 7,
 4  "label": "client1",
 5  "warnings": "This address is not a new identity. It is a re-use of an existing identity with a different label."
 6}
 7./src/bitcoin-cli -testnet -rpcwallet=sp1 getsilentaddress client1
 8{
 9  "address": "tsp071pp809z56vwvyqhwc4fsjq8vhjw6yyguzv2u9hnwt3jvyl2ha49nwqtsc8w9",
10  "label": "client1",
11  "index": 7
12}
13./src/bitcoin-cli -testnet -rpcwallet=sp1 getsilentaddress client2
14{
15  "address": "tsp081pp809z56vwvyqhwc4fsjq8vhjw6yyguzv2u9hnwt3jvyl2ha49nwqyqkdah",
16  "index": 8,
17  "label": "client2",
18  "warnings": "This address is not a new identity. It is a re-use of an existing identity with a different label."
19}
20./src/bitcoin-cli -testnet -rpcwallet=sp1 getsilentaddress client2
21{
22  "address": "tsp081pp809z56vwvyqhwc4fsjq8vhjw6yyguzv2u9hnwt3jvyl2ha49nwqyqkdah",
23  "label": "client2",
24  "index": 8
25}

 0diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp
 1index 1f8621d48..cd6165cef 100644
 2--- a/src/wallet/rpc/addresses.cpp
 3+++ b/src/wallet/rpc/addresses.cpp
 4@@ -110,10 +110,14 @@ RPCHelpMan getsilentaddress()
 5     if (!request.params[0].isNull())
 6         label = LabelFromValue(request.params[0]);
 7 
 8+    UniValue obj(UniValue::VOBJ);
 9     for (auto const& [key, val] : pwallet->m_silent_address_book)
10     {
11         if (val.m_label == label) {
12-            return val.m_address;
13+            obj.pushKV("address", val.m_address);
14+            obj.pushKV("label", val.m_label);
15+            obj.pushKV("index", key);
16+            return obj;
17         }
18     }
19 
20@@ -128,7 +132,16 @@ RPCHelpMan getsilentaddress()
21         throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, util::ErrorString(op_dest).original);
22     }
23 
24-    return EncodeDestination(*op_dest, silent_payment, current_index);
25+    const auto address{EncodeDestination(*op_dest, silent_payment, current_index)};
26+
27+    obj.pushKV("address", address);
28+    obj.pushKV("index", current_index);
29+    obj.pushKV("label", label);
30+    if (current_index > 0) {
31+        obj.pushKV("warnings", "This address is not a new identity. It is a re-use of an existing identity with a different label.");
32+    }
33+
34+    return obj;
35 },
36     };
37 }

Left some more code-review comments.

I also tested the following scenario:

• Create regular wallet
• Generate some coins to it
• Create SP wallet
• Send payment from regular wallet to SP address
• SP wallet correctly shows the coins received
• Send back from SP wallet to regular wallet
• Regular wallet shows the coins received

I also tested the same scenario from an SP wallet to another SP wallet and it worked successfully.

Thanks for the co-author! @w0xlt

I was playing around with the code and tried benchmarking, here’s a benchmark for SumXOnlyPublicKeys if you want to include it in this PR:

 0diff --git a/src/Makefile.bench.include b/src/Makefile.bench.include
 1index e1e206687..91650b621 100644
 2--- a/src/Makefile.bench.include
 3+++ b/src/Makefile.bench.include
 4@@ -45,6 +45,7 @@ bench_bench_bitcoin_SOURCES = \
 5   bench/rollingbloom.cpp \
 6   bench/rpc_blockchain.cpp \
 7   bench/rpc_mempool.cpp \
 8+  bench/silentpayment.cpp \
 9   bench/strencodings.cpp \
10   bench/util_time.cpp \
11   bench/verify_script.cpp
12diff --git a/src/bench/silentpayment.cpp b/src/bench/silentpayment.cpp
13new file mode 100644
14index 000000000..1cd5455b9
15--- /dev/null
16+++ b/src/bench/silentpayment.cpp
17@@ -0,0 +1,68 @@
18+// Copyright (c) 2022 The Bitcoin Core developers
19+// Distributed under the MIT software license, see the accompanying
20+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
21+
22+#include <bench/bench.h>
23+
24+#include <random.h>
25+#include <secp256k1.h>
26+#include <secp256k1_ecdh.h>
27+#include <secp256k1_extrakeys.h>
28+#include <silentpayment.h>
29+#include <test/util/setup_common.h>
30+
31+static XOnlyPubKey gen_key(secp256k1_context* ctx) {
32+    unsigned char seckey[32];
33+    GetRandBytes(seckey);
34+
35+    secp256k1_pubkey pubkey;
36+    int return_val = secp256k1_ec_pubkey_create(ctx, &pubkey, seckey);
37+    assert(return_val);
38+
39+    secp256k1_xonly_pubkey xonly_pubkey;
40+    return_val = secp256k1_xonly_pubkey_from_pubkey(ctx, &xonly_pubkey, nullptr, &pubkey);
41+    assert(return_val);
42+
43+    unsigned char xonly_pubkey_bytes[32];
44+    return_val = secp256k1_xonly_pubkey_serialize(ctx, xonly_pubkey_bytes, &xonly_pubkey);
45+    assert(return_val);
46+
47+    return XOnlyPubKey(xonly_pubkey_bytes);
48+}
49+
50+static void SumXOnlyPublicKeys(benchmark::Bench& bench, int key_count)
51+{
52+    auto ctx = secp256k1_context_create(SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY);
53+    std::vector<XOnlyPubKey> sender_x_only_public_keys;
54+    for (int i = 0; i < key_count; i++) {
55+        sender_x_only_public_keys.push_back(gen_key(ctx));
56+    }
57+
58+    bench.run([&] {
59+        silentpayment::Recipient::SumXOnlyPublicKeys(sender_x_only_public_keys);
60+    });
61+
62+    secp256k1_context_destroy(ctx);
63+}
64+
65+static void SumXOnlyPublicKeys_1(benchmark::Bench& bench)
66+{
67+    SumXOnlyPublicKeys(bench, 1);
68+}
69+static void SumXOnlyPublicKeys_10(benchmark::Bench& bench)
70+{
71+    SumXOnlyPublicKeys(bench, 10);
72+}
73+static void SumXOnlyPublicKeys_100(benchmark::Bench& bench)
74+{
75+    SumXOnlyPublicKeys(bench, 100);
76+}
77+static void SumXOnlyPublicKeys_1000(benchmark::Bench& bench)
78+{
79+    SumXOnlyPublicKeys(bench, 1000);
80+}
81+
82+BENCHMARK(SumXOnlyPublicKeys_1);
83+BENCHMARK(SumXOnlyPublicKeys_10);
84+BENCHMARK(SumXOnlyPublicKeys_100);
85+BENCHMARK(SumXOnlyPublicKeys_1000);