Silent Payments: sending

josibake commented at 4:37 pm on August 2, 2023: member

This PR is part of integrating silent payments into Bitcoin Core. Status and tracking for the project is managed in #28536

This PR depends on #28122 and is marked as a draft until it is merged. If interested in those commits, please review on #28122

Pre-work / Refactors

The first three commits are pre-work / refactors needed for this PR. They are broken out into the following PRS:

Sending

Silent Payments logic

The main focus of this PR is:

Applying the Taptweak to a taproot internal private key (this is a copy-paste of the code for applying the taptweak in the signing process)
Getting a private key from a given scriptPubKey
Creating silent payment outputs
Applying the created scriptPubKeys back to the vector of CRecipients

The functions are then used together to create silent payment outputs during CreateTransactionInternal.

Final steps

The last commits ensure that:

Coin selection is silent payments aware and knows to exclude taproot script path spends and inputs with unknown witness when funding a transaction which pays to a silent payment address
The change output type is correctly chosen when paying to a silent payment address
Functional tests

DrahtBot commented at 4:37 pm on August 2, 2023: contributor

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

Code Coverage

For detailed information about the code coverage, see the test coverage report.

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:

#30050 (refactor, wallet: get serialized size of CRecipients directly by josibake)
#30048 (crypto: add NUMS_H const by josibake)
#30047 (refactor: Model the bech32 charlimit as an Enum by josibake)
#29325 (consensus: Store transaction nVersion as uint32_t 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)
#27286 (wallet: Keep track of the wallet’s own transaction outputs in memory by achow101)
#27260 (Enhanced error messages for invalid network prefix during address parsing. by russeree)

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 added the label CI failed on Aug 2, 2023

josibake renamed this:
~~Silent Payments: implement sending~~
Silent Payments: sending
on Aug 3, 2023

josibake force-pushed on Aug 3, 2023

DrahtBot removed the label CI failed on Aug 3, 2023

josibake commented at 9:50 am on August 3, 2023: member

Maybe add a quick summary in the description with the main implementation differences relative to #24897. It seems a big one is that this doesn’t require an index!

updated! I added the summary in #27827 and added links back to the parent PR in each of the child PRs.

in src/wallet/spend.cpp:942 in 181e329a1d outdated

899+    assert(tx_outpoints.size() > 0);
900+    CKey scalar_ecdh_input = PrepareScalarECDHInput(input_private_keys, tx_outpoints);
901+    std::vector<SilentPaymentRecipient> silent_payment_recipients = GroupSilentPaymentAddresses(silent_payment_destinations);
902+    std::vector<CRecipient> outputs;
903+    for (const auto& recipient : silent_payment_recipients) {
904+        Sender sender{scalar_ecdh_input, recipient};

josibake commented at 10:29 am on August 8, 2023:

This does not need to be a struct, it can just be a function.

in test/functional/wallet_silentpayments_sending.py:91 in e6f7458324 outdated

86+            address=SILENT_PAYMENT_ADDRESS,
87+            amount=21,
88+        )
89+        assert txid
90+
91+    def test_deterministic_send(self):

BrandonOdiwuor commented at 5:32 pm on August 9, 2023:

Incorporating logs within the test is crucial for offering transparent insight into the test’s progression, simplifying the identification of problems, and enhancing comprehension of the test’s overall behavior.

in test/functional/wallet_silentpayments_sending.py:127 in e6f7458324 outdated

122+                assert output["scriptPubKey"]["address"] == "bcrt1p5num3dvry0ffusg3s7v2j5fy025p95jtn6js65jlruwc69r9je2s6qfsj7"
123+                break
124+        else:
125+            assert False
126+
127+    def test_address_reuse(self):

BrandonOdiwuor commented at 5:34 pm on August 9, 2023:

Also consider adding logs to this test as the ones above

josibake force-pushed on Aug 30, 2023

josibake force-pushed on Aug 31, 2023

josibake force-pushed on Sep 8, 2023

josibake force-pushed on Sep 11, 2023

DrahtBot added the label CI failed on Sep 11, 2023

josibake force-pushed on Sep 11, 2023

josibake force-pushed on Sep 12, 2023

josibake force-pushed on Sep 14, 2023

DrahtBot removed the label CI failed on Sep 14, 2023

DrahtBot added the label Needs rebase on Sep 19, 2023

josibake force-pushed on Sep 21, 2023

DrahtBot added the label CI failed on Sep 21, 2023

DrahtBot removed the label Needs rebase on Sep 21, 2023

DrahtBot removed the label CI failed on Sep 21, 2023

josibake commented at 4:30 pm on September 26, 2023: member

Note: send does not work, but sendall, sendtoaddress does

josibake force-pushed on Oct 2, 2023

DrahtBot added the label CI failed on Oct 2, 2023

josibake force-pushed on Oct 3, 2023

DrahtBot removed the label CI failed on Oct 4, 2023

DrahtBot added the label Needs rebase on Oct 16, 2023

josibake force-pushed on Jan 15, 2024

DrahtBot removed the label Needs rebase on Jan 15, 2024

DrahtBot commented at 9:36 pm on January 17, 2024: contributor

🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the documentation.

Possibly this is 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.

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

Debug: https://github.com/bitcoin/bitcoin/runs/20505330863

DrahtBot added the label CI failed on Jan 17, 2024

josibake force-pushed on Jan 19, 2024

josibake commented at 2:30 pm on January 19, 2024: member

Rebased https://github.com/bitcoin/bitcoin/commit/3f4b0f77c7f2ff52b9c84bf1c5a70281a066cfe2 -> https://github.com/bitcoin/bitcoin/commit/3b0dd79cacaf4e97489746ace5c1c8d6fe8db38d (implement-bip352-sending-01 -> implement-bip352-01-rebase, compare)

Fixed silent merge conflict with CKey now returning std::byte*

DrahtBot removed the label CI failed on Jan 19, 2024

willcl-ark added the label Wallet on Jan 24, 2024

willcl-ark added the label Privacy on Jan 24, 2024

DrahtBot added the label Needs rebase on Jan 26, 2024

josibake force-pushed on Jan 26, 2024

DrahtBot removed the label Needs rebase on Jan 26, 2024

DrahtBot added the label CI failed on Feb 2, 2024

DrahtBot removed the label CI failed on Feb 7, 2024

DrahtBot added the label Needs rebase on Feb 20, 2024

Squashed 'src/secp256k1/' changes from d8311688bd..3d08027789

3d08027789 ci: enable silentpayments module
85946762a5 tests: add BIP-352 test vectors
bf349c2a08 silentpayments: add examples/silentpayments.c
9a7106e19c silentpayments: add recipient light client support
f113564298 silentpayments: add recipient scanning routine
4fb8716f4f silentpayments: add opaque data type `public_data`
987d829e8f silentpayments: add recipient label support
14ca754578 silentpayments: add sender routine
9b965927da silentpayments: implement output pubkey creation
a0fcc2c780 silentpayments: implement shared secret creation
13f203dacd silentpayments: add sortable recipient struct
a9326bdd7a doc: add module description for silentpayments
15d3e71cc1 build: add skeleton for new silentpayments (BIP352) module
cc7d18a8a8 extrakeys: add secp256k1_pubkey_sort

git-subtree-dir: src/secp256k1
git-subtree-split: 3d080277895655e8274ee73aacd154c4ead143e3

785ef3d558

Merge commit '785ef3d5588dd6975bd4e8e067e6a62697867fe8' into refresh-secp256k1 bfe9876348

Squashed 'src/secp256k1/' changes from 3d08027789..0270b14309

0270b14309 labels: actually set the label

git-subtree-dir: src/secp256k1
git-subtree-split: 0270b1430981584582645161a04a4df67cd187bb

7607d3cfbe

Merge commit '7607d3cfbe3c86ad9d01f56437dd0a38fbb13045' into refresh-secp256k1 03a2a2d78a

josibake force-pushed on Apr 22, 2024

DrahtBot removed the label Needs rebase on Apr 22, 2024

DrahtBot added the label CI failed on Apr 22, 2024

DrahtBot commented at 9:52 pm on April 22, 2024: contributor

🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the documentation.

Possibly this is 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.

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

Debug: https://github.com/bitcoin/bitcoin/runs/24114299414

josibake force-pushed on Apr 27, 2024

Squashed 'src/secp256k1/' changes from 0270b14309..92f592023f

92f592023f ci: enable silentpayments module
8ddc4574c9 tests: add BIP-352 test vectors
8315abd830 silentpayments: add benchmark for `scan_outputs`
f3a9516ec8 silentpayments: add examples/silentpayments.c
7e11e7613b silentpayments: add recipient light client support
3321771b0e silentpayments: add recipient scanning routine
766567f099 silentpayments: add opaque data type `public_data`
8d0bb06ce7 silentpayments: add recipient label support
9c9bd057bc silentpayments: add sender routine
036e688fd0 silentpayments: implement output pubkey creation
1ffee123d6 silentpayments: implement shared secret creation
7a5683260c silentpayments: add sortable recipient struct
a8d6f4b8e1 doc: add module description for silentpayments
1121a4d376 build: add skeleton for new silentpayments (BIP352) module
7d2591ce12 Add secp256k1_pubkey_sort
da515074e3 Merge bitcoin-core/secp256k1#1058: Signed-digit multi-comb ecmult_gen algorithm
4c341f89ab Add changelog entry for SDMC
a043940253 Permit COMB_BITS < 256 for exhaustive tests
39b2f2a321 Add test case for ecmult_gen recoded = {-1,0,1}
644e86de9a Reintroduce projective blinding
07810d9abb Reduce side channels from single-bit reads
a0d32b597d Optimization: use Nx32 representation for recoded bits
e03dcc44b5 Make secp256k1_scalar_get_bits support 32-bit reads
5005abee60 Rename scalar_get_bits -> scalar_get_bits_limb32; return uint32_t
6247f485b6 Optimization: avoid unnecessary doublings in precomputation
15d0cca2a6 Optimization: first table lookup needs no point addition
7a33db35cd Optimization: move (2^COMB_BITS-1)/2 term into ctx->scalar_offset
ed2a056f3d Provide 3 configurations accessible through ./configure
5f7be9f6a5 Always generate tables for current (blocks,teeth) config
fde1dfcd8d Signed-digit multi-comb ecmult_gen algorithm
486518b350 Make exhaustive tests's scalar_inverse(&x,&x) work
ab45c3e089 Initial gej blinding -> final ge blinding
aa00a6b892 Introduce CEIL_DIV macro and use it
REVERT: 0270b14309 labels: actually set the label
REVERT: 3d08027789 ci: enable silentpayments module
REVERT: 85946762a5 tests: add BIP-352 test vectors
REVERT: bf349c2a08 silentpayments: add examples/silentpayments.c
REVERT: 9a7106e19c silentpayments: add recipient light client support
REVERT: f113564298 silentpayments: add recipient scanning routine
REVERT: 4fb8716f4f silentpayments: add opaque data type `public_data`
REVERT: 987d829e8f silentpayments: add recipient label support
REVERT: 14ca754578 silentpayments: add sender routine
REVERT: 9b965927da silentpayments: implement output pubkey creation
REVERT: a0fcc2c780 silentpayments: implement shared secret creation
REVERT: 13f203dacd silentpayments: add sortable recipient struct
REVERT: a9326bdd7a doc: add module description for silentpayments
REVERT: 15d3e71cc1 build: add skeleton for new silentpayments (BIP352) module
REVERT: cc7d18a8a8 extrakeys: add secp256k1_pubkey_sort

git-subtree-dir: src/secp256k1
git-subtree-split: 92f592023f3f4d6a66724772349fbdc4967ab50f

2bfd600177

Merge commit '2bfd6001775c258f81e48b84624286fb9d898800' into refresh-secp256k1 41cf626708

crypto: add NUMS_H const b7724e9523

Compare COutPoints lexicographically cb0b68e9ca

Model the bech32 charlimit as an Enum

Bech32(m) was defined with a 90 character limit so that certain
guarantees for error detection could be made for segwit addresses.
However, there is nothing about the encoding scheme itself that requires
a limit and in practice bech32(m) has been used without the 90 char
limit (e.g. lightning invoices).

Further, increasing the character limit doesn't do away with error
detection, it simply lessons the guarantees.

Model charlimit as an Enum, so that if a different address scheme is
using bech32(m), the character limit for that address scheme can be
used, rather than always using the 90 charlimit defined for segwit
addresses.

145978049e

conf: add ECDH,SILENTPAYMENTS secp256k1 modules 9fec70a2d4

Add "sp" HRP 41f426aa22

Add V0SilentPaymentDestination address type 622c7a98b9

common: add bip352.{h,cpp} secp256k1 module

Wrap the silentpayments module from libsecp256k1. This is placed in
common as it is intended to be used by:

  * RPCs: for parsing addresses
  * Wallet: for sending, receiving, spending silent payment outputs
  * Node: for creating silent payment indexes for light clients

5b06ccf1dc

wallet: disable sending to silent payment address

Have `IsValidDestination` return false for silent payment destinations
and set an error string when decoding a silent payment address.

This prevents anyone from sending to a silent payment address before
sending is implemented in the wallet, but also allows the functions to
be used in the unit testing famework.

e565824a47

tests: add BIP352 test vectors as unit tests

Use the test vectors to test sending and receiving. A few cases are not
covered here, namely anything that requires testing specific to the
wallet. For example:

* Taproot script path spending is not tested, as that is better tested in
  a wallets coin selection / signing logic
* Re-computing outputs during RBF is not tested, as that is better
  tested in a wallets RBF logic

The unit tests are written in such a way that adding new test cases is
as easy as updating the JSON file

246e2a24c5

wallet: get serialized size from CRecipient

Now that a CRecipient holds a CTxDestination, we can get the serialized
size using a visitor. This doesnt change any current behavior, but provides
a nice generalization that can be used to apply special logic to a CTxDestination
serialization in the future.

a5a7e09f3a

move-only: refactor CreateTransactionInternal

Move the output serialization size calculation into the loop where the
outputs are iterated over to calculate the total sum.

Move the code for adding the the txoutputs to the transaction to after
coin selection.

While this code structure generally follows a more logical flow,
the primary motivation for moving the code for adding outputs to the
transaction sets us up nicely for silent payments (in a future PR):
we need to know the input set before generating the final output scriptPubKeys.

f1668c83e1

crypto: add method for applying the taptweak

The wallet returns an untweaked internal key for taproot outputs. If the
output commits to a tree of scripts, this key needs to be tweaked with
the merkle root. Even if the output does not commit to a tree of
scripts, BIP341/342 recommend commiting to a hash of the public key.

Depending how the taproot output was created, we need to apply the merkle
root tweak or hash of the public key tweak before doing ECDH

a1c6d22f3e

wallet: get serialized size for `V0SilentPayments`

BIP352 v0 specifies that a silent payment output is a taproot output.
Taproot scriptPubKeys are a fixed size, so when calculating the
serialized size for a CRecipient with a V0SilentPayments destination,
use WitnessV1Taproot for the serialized txout size.

797e21c8c1

wallet: add method for retreiving a private key

Add a method for retreiving a private key for a given scriptPubKey.
If the scriptPubKey is a taproot output, tweak the private key with the
merkle root or hash of the public key, if applicable.

9c0de0f580

wallet: make coin selection silent payment aware

Add a flag to the `CoinControl` object if silent payment destinations
are provided. Before adding the flag, call a function which checks if:

* The wallet has private keys
* The wallet is unlocked

Without both of the above being true, we cannot send to a silent payment
address.

During coin selection, if this flag is set, skip taproot inputs when
script spend data is available. This is based on the assumption that if
a user provides script spend data, they don't have access to the key
path spend. As future improvement, we could instead check to see if we
have access to the key path spend, and only exclude the output when we
don't regardless of whether or not the user provides script spend data.

Also skip UTXOs of type `WITNESS_UNKNOWN`, although it is very unlikely
our wallet would ever try to spend a witness unknown output.

26fd679d83

wallet: add `IsInputForSharedSecretDerivation` function 36c8b7ca11

wallet: add `CreateSilentPaymentOutputs` function

`CreateSilentPaymentsOutputs` gets the correct private keys, adds them
together, groups the silent payment destinations and then generates the
taproot script pubkeys. These are then passed back to
CreateTransactionInternal, which uses these scriptPubKeys to update
vecSend before adding them to the transaction outputs.

27d1fe8178

wallet: update TransactionChangeType

If sending to a silent payment destination, the change type should be taproot

aaa9353f53

wallet: enable sending to silent payment address 94cae4d175

tests: add sending functional tests a4edd78570

josibake force-pushed on May 5, 2024

DrahtBot removed the label CI failed on May 5, 2024

DrahtBot added the label Needs rebase on May 20, 2024

DrahtBot commented at 11:55 am on May 20, 2024: contributor

🐙 This pull request conflicts with the target branch and needs rebase.