Silent Payments: send and receive

josibake commented at 12:12 pm on June 5, 2023: member

For reviewers

In an attempt to make reviewing a bit more sane, I’m breaking this up into a few smaller PRs, but will keep this one open as the parent PR and keep it rebased on the child PRs. The main purpose of having this PR is to track progress on child PRs and also have an easy way to compile bitcoind with both send and receive support for testing. Additionally, I’ll be adding more functional tests to this PR since it’s much easier to test when bitcoind can both send and receive.

PRs

#28122
- Implements the logic from BIP352 without any wallet code. This PR adds the necessary cryptographic functions and implements the logic needed for sending and scanning. This PR also includes the test vectors from the BIP as unit tests. Both the send and receive PRs have this as a dependency. In terms of priority, this PR should be reviewed first
#28201
- Implements sending in the Bitcoin Core wallet. This PR allows a wallet to send to a silent payment address, regardless of whether or not the wallet can receive silent payments
- Ready for review, but marked as a draft until dependencies are merged
#28202
- Implements receiving in the Bitcoin Core wallet. This PR allows a wallet to generate silent payment addresses and scan for silent payments, regardless of whether or not the wallet can send to a silent payment address
- Ready for review but marked as a draft until dependencies are merged

For the silent payments specification, see https://github.com/bitcoin/bips/pull/1458

Overall

This PR implements the full silent payments scheme: sending and receiving. The following items are not covered in this PR and are intended for follow-up PRs:

Adding labels for the receiver wallet
Full RPC coverage (only sendtoaddress and sendmany are covered in this PR)
Light client support (vending the tweak data per block, either in an index or to serve to indexer, such as electrum server)
Add benchmarks to validate that there are no DoS concerns for doing silent payment verification for transactions in the mempool
External signer support (dependent on hardware wallets supporting silent payments)
More unit / functional test coverage

Major changes

This PR is a continuation of the work done in #24897. Below is a summary of the major changes:

Remove labels
- The original draft included labels, but this has been deferred for a later PR. Labels are not necessary for sending and receiving and there are still some open questions on how best to implement them in Bitcoin Core. Labels can also be added at any point by the receiver without requiring any changes from the sender
Remove indexes
- In the original draft, indexes were used when scanning for silent payments and when doing wallet rescans. This has been removed in favor of using rev*.dat files for rescanning. It may make sense to add an index in the future, but for the purpose of vending tweak data to light clients, which is still an open question
Update to implement the most current version of BIP352
- Since the original draft, there have been a few changes in the BIP which are reflected in the current PR. Most notably, using 33-byte compressed keys for the silent payment address (as opposed to X-only keys in the original draft)

It may be helpful for context to read through the discussions on #24897 , but ongoing review should happen in the relevant child PRs listed above.

DrahtBot commented at 12:12 pm on June 5, 2023: contributor

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
Approach ACK	w0xlt

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:

#28453 (wallet: Receive silent payment transactions by achow101)
#28333 (wallet: Construct ScriptPubKeyMans with all data rather than loaded progressively by achow101)
#28246 (wallet: Use CTxDestination in CRecipient instead of just scriptPubKey by achow101)
#28201 (Silent Payments: sending by josibake)
#28142 (wallet: Allow users to create a wallet that encrypts all database records by achow101)
#28122 (Silent Payments: Implement BIP352 by josibake)
#27865 (wallet: Track no-longer-spendable TXOs separately by achow101)
#27788 (rpc: Be able to access RPC parameters by name by achow101)
#27596 (assumeutxo (2) by jamesob)
#27351 (wallet: add seeds argument to importdescriptors by apoelstra)
#27307 (wallet: track mempool conflicts with wallet transactions by ishaanam)
#27286 (wallet: Keep track of the wallet’s own transaction outputs in memory by achow101)
#26840 (refactor: importpubkey, importprivkey, importaddress, importmulti, and importdescriptors rpc by KolbyML)
#26728 (wallet: Have the wallet store the key for automatically generated descriptors by achow101)
#26008 (wallet: cache IsMine scriptPubKeys to improve performance of descriptor wallets by achow101)
#25907 (wallet: rpc to add automatically generated descriptors by achow101)
#25665 (refactor: Add util::Result failure values, multiple error and warning messages by ryanofsky)
#25273 (wallet: Pass through transaction locktime and preset input sequences and scripts to CreateTransaction by achow101)
#22838 (descriptors: Be able to specify change and receiving in a single descriptor string by achow101)
#22341 (rpc: add getxpub by Sjors)

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.

josibake renamed this:
~~Silent Payments~~
[DRAFT] Silent Payments
on Jun 5, 2023

DrahtBot added the label CI failed on Jun 5, 2023

josibake marked this as ready for review on Jun 5, 2023

josibake renamed this:
~~[DRAFT] Silent Payments~~
[Silent Payments]: Base functionality
on Jun 5, 2023

DrahtBot renamed this:
~~[Silent Payments]: Base functionality~~
[Silent Payments]: Base functionality
on Jun 5, 2023

josibake force-pushed on Jun 5, 2023

josibake force-pushed on Jun 6, 2023

DrahtBot removed the label CI failed on Jun 6, 2023

in src/wallet/test/silentpayment_tests.cpp:130 in b1ce82495f outdated

125+    auto silent_recipient = TestRecipient(recipient_spend_seckey);
126+
127+    BOOST_CHECK(EncodeSecret(silent_recipient.GetNegatedScanSecKey()) == "L2HBwB2tkUdGeb2KWx2EaUWD1YjW8u4eZ7uXJfJA8u3ibkrPtvh5");
128+    BOOST_CHECK(HexStr(silent_recipient.GetScanPubkey()) == "bfa2fb9b2d094a039d4b5bf76a159d028f15a8350d5c957651d6dc00af4ccf41");
129+
130+    // const std::vector<std::pair<std::string, std::string>> spend_keys = {

w0xlt commented at 0:30 am on June 11, 2023:

These comments can be removed.

in src/wallet/test/silentpayment_tests.cpp:174 in b1ce82495f outdated

182+    CKey silent_shared_secret;
183+    silent_shared_secret.Set(std::begin(_silent_shared_secret), std::end(_silent_shared_secret), true);
184+
185+    BOOST_CHECK(recipient_shared_secret == silent_shared_secret);
186+
187+    // const std::vector<std::pair<std::string, std::string>> results = {

w0xlt commented at 0:31 am on June 11, 2023:

These comments can be removed.

w0xlt commented at 0:35 am on June 11, 2023: contributor

Approach ACK naturally, Will review in detail soon.

Thanks for moving this project forward and splitting the draft PR in smaller ones.

josibake force-pushed on Jun 13, 2023

DrahtBot added the label CI failed on Jun 13, 2023

josibake force-pushed on Jun 13, 2023

DrahtBot removed the label CI failed on Jun 13, 2023

DrahtBot added the label Needs rebase on Jun 14, 2023

josibake force-pushed on Jun 14, 2023

DrahtBot removed the label Needs rebase on Jun 14, 2023

josibake force-pushed on Jun 16, 2023

luke-jr commented at 2:02 am on June 24, 2023: member

Adds support to the Bitcoin Core wallet for receiving silent payments

Prefer to leave this for a second PR

DrahtBot added the label Needs rebase on Jun 26, 2023

josibake commented at 12:33 pm on July 6, 2023: member

Adds support to the Bitcoin Core wallet for receiving silent payments

Prefer to leave this for a second PR

Can you explain your reasoning for wanting to split it up? I’d prefer to keep them together only because I see no reason to merge sending without receiving support in Bitcoin Core. In terms of work, if the goal is to merge both sending and receiving support, the amount of review doesn’t change, and splitting into two PRs seems like unnecessary shuffling of commits

in src/primitives/transaction.h:162 in 96f7a3a0c0 outdated

158@@ -159,20 +159,22 @@ class CTxOut
159 public:
160     CAmount nValue;
161     CScript scriptPubKey;
162+    bool m_silentpayment;

dergoegge commented at 12:24 pm on July 13, 2023:

In my opinion, this flag doesn’t belong here because only the receiver/sender of a silent payment would care about it, which would make it a piece of wallet meta-data.

Without knowing much about the wallet my guess would be that it should be stored on CWalletTx?

in src/pubkey.h:285 in 96f7a3a0c0 outdated

280@@ -271,6 +281,9 @@ class XOnlyPubKey
281     /** Construct a Taproot tweaked output point with this point as internal key. */
282     std::optional<std::pair<XOnlyPubKey, bool>> CreateTapTweak(const uint256* merkle_root) const;
283 
284+    /** Tweak an x-only public key by adding the generator multiplied with tweak32 to it. */
285+    XOnlyPubKey AddTweak(const unsigned char *tweak32) const;

dergoegge commented at 12:34 pm on July 13, 2023:

Maybe make this a uint256&? Passing a raw pointer without a size (expecting it to be 32 byte) seems a little bug prone.

0    XOnlyPubKey AddTweak(const uint256& tweak32) const;

in src/key.h:116 in 96f7a3a0c0 outdated

100@@ -101,6 +101,20 @@ class CKey
101     //! Negate private key
102     bool Negate();
103 
104+    //! Tweak a secret key by adding a scalar value to it.
105+    //
106+    CKey AddTweak(const unsigned char *tweak32) const;
107+
108+    //! Tweak a secret key by multiplying it by a scalar value.
109+    CKey MultiplyTweak(const unsigned char *tweak32) const;

dergoegge commented at 12:36 pm on July 13, 2023:

Same as other suggestion

0    CKey AddTweak(const uint256& tweak32) const;
1
2    //! Tweak a secret key by multiplying it by a scalar value.
3    CKey MultiplyTweak(const uint256& tweak32) const;

dergoegge changes_requested

in test/functional/wallet_silentpayment_blockfilterindex.py:78 in 96f7a3a0c0 outdated

73+
74+        recv_addr_01 = recipient_wallet_04.getsilentaddress()['address']
75+
76+        self.nodes[0].unloadwallet(f'recipient_wallet_04')
77+
78+        for i in range(50):

brunoerg commented at 4:48 pm on July 13, 2023:

In 96f7a3a0c0ccf81269a587dd2aa52aaf895e0046: if i has not been used, so it could be for _ in range(50)

in test/functional/wallet_silentpayment_blockfilterindex.py:93 in 96f7a3a0c0 outdated

88+
89+    def run_test(self):
90+        self.nodes[0].createwallet(wallet_name=f'sender_wallet', descriptors=True)
91+        sender_wallet = self.nodes[0].get_wallet_rpc(f'sender_wallet')
92+
93+        self.generatetoaddress(self.nodes[0], 1, sender_wallet.getnewaddress('', 'bech32'))

brunoerg commented at 4:50 pm on July 13, 2023:

In 96f7a3a0c0ccf81269a587dd2aa52aaf895e0046: you could simplify it by doing:

 0diff --git a/test/functional/wallet_silentpayment_blockfilterindex.py b/test/functional/wallet_silentpayment_blockfilterindex.py
 1index 16d467c18..8f5127ae1 100755
 2--- a/test/functional/wallet_silentpayment_blockfilterindex.py
 3+++ b/test/functional/wallet_silentpayment_blockfilterindex.py
 4@@ -90,10 +90,8 @@ class SilentIdentifierSimple(BitcoinTestFramework):
 5         self.nodes[0].createwallet(wallet_name=f'sender_wallet', descriptors=True)
 6         sender_wallet = self.nodes[0].get_wallet_rpc(f'sender_wallet')
 7 
 8-        self.generatetoaddress(self.nodes[0], 1, sender_wallet.getnewaddress('', 'bech32'))
 9-        self.generatetoaddress(self.nodes[0], 1, sender_wallet.getnewaddress('', 'bech32'))
10-        self.generatetoaddress(self.nodes[0], 1, sender_wallet.getnewaddress('', 'bech32m'))
11-        self.generatetoaddress(self.nodes[0], 1, sender_wallet.getnewaddress('', 'legacy'))
12+        for address_type in ['bech32', 'bech32', 'bech32m', 'legacy']:
13+            self.generatetoaddress(self.nodes[0], 1, sender_wallet.getnewaddress('', address_type))
14         self.generatetoaddress(self.nodes[0], COINBASE_MATURITY + 10, "bcrt1qjqmxmkpmxt80xz4y3746zgt0q3u3ferr34acd5")
15 
16         self.one_wallet(sender_wallet)

in src/key_io.cpp:333 in c4d64fadae outdated

328+std::vector<unsigned char> DecodeSilentAddress(const std::string& str)
329+{
330+    const auto& params{Params()};
331+
332+    const auto& silent_payment_hrp = params.SilentPaymentHRP();
333+    auto dest_silent_payment_hrp = ToLower(std::string_view(str).substr(0, params.SilentPaymentHRP().size()));

brunoerg commented at 4:56 pm on July 13, 2023:

In c4d64fadae0d77da22e0eec91a97c8f3290ff8c3: You could use silent_payment_hrp instead of calling params.SilentPaymentHRP() again.

 0diff --git a/src/key_io.cpp b/src/key_io.cpp
 1index c6ad8dc1a..105772aab 100644
 2--- a/src/key_io.cpp
 3+++ b/src/key_io.cpp
 4@@ -347,7 +347,7 @@ std::vector<unsigned char> DecodeSilentAddress(const std::string& str)
 5     const auto& params{Params()};
 6 
 7     const auto& silent_payment_hrp = params.SilentPaymentHRP();
 8-    auto dest_silent_payment_hrp = ToLower(std::string_view(str).substr(0, params.SilentPaymentHRP().size()));
 9+    auto dest_silent_payment_hrp = ToLower(std::string_view(str).substr(0, silent_payment_hrp.size()));
10 
11     if (dest_silent_payment_hrp != silent_payment_hrp) {
12         return {};
13@@ -356,7 +356,7 @@ std::vector<unsigned char> DecodeSilentAddress(const std::string& str)
14     std::vector<unsigned char> data;
15     data.clear();
16     const auto dec = bech32::Decode(str, /*silent=*/true);
17-    auto dec_silent_payment_hrp = dec.hrp.substr(0, params.SilentPaymentHRP().size());
18+    auto dec_silent_payment_hrp = dec.hrp.substr(0, silent_payment_hrp.size());

in src/key_io.cpp:340 in c4d64fadae outdated

335+    if (dest_silent_payment_hrp != silent_payment_hrp) {
336+        return {};
337+    }
338+
339+    std::vector<unsigned char> data;
340+    data.clear();

brunoerg commented at 4:59 pm on July 13, 2023:

In c4d64fadae0d77da22e0eec91a97c8f3290ff8c3: This clear call is unnecessary here.

in src/silentpayment.cpp:210 in ccb4c1d23d outdated

205@@ -206,4 +206,53 @@ bool ExtractPubkeyFromInput(const Coin& prevOut, const CTxIn& txin, std::vector<
206     // something is wrong.
207     return false;
208 }
209+
210+std::map<uint256, CPubKey> GetSilentPaymentKeysPerBlock(const uint256& block_hash, const CBlockUndo& blockUndo, const std::vector<CTransactionRef> vtx)

brunoerg commented at 5:11 pm on July 13, 2023:

in ccb4c1d23db7bf33f2914270d354f0e5a2f93a48: It seems like block_hash has not been used in GetSilentPaymentKeysPerBlock.

in src/key_io.cpp:344 in c4d64fadae outdated

339+    std::vector<unsigned char> data;
340+    data.clear();
341+    const auto dec = bech32::Decode(str, /*silent=*/true);
342+    auto dec_silent_payment_hrp = dec.hrp.substr(0, params.SilentPaymentHRP().size());
343+
344+    auto version = dec.data[0];

brunoerg commented at 5:27 pm on July 13, 2023:

In c4d64fadae0d77da22e0eec91a97c8f3290ff8c3: dec.data[0] may cause a segmentation fault.

You are trying to access data[0] right before checking if data is empty.

0auto version = dec.data[0];
1if (dec.encoding != bech32::Encoding::BECH32M || dec.data.empty() || dec.hrp != silent_payment_hrp || version != 0) {
2    return {};
3}

Note, a simple fuzzer would catch it, e.g.:

 0diff --git a/src/test/fuzz/key_io.cpp b/src/test/fuzz/key_io.cpp
 1index a1c587a75..9b5a4b278 100644
 2--- a/src/test/fuzz/key_io.cpp
 3+++ b/src/test/fuzz/key_io.cpp
 4@@ -4,7 +4,9 @@
 5 
 6 #include <chainparams.h>
 7 #include <key_io.h>
 8+#include <test/fuzz/FuzzedDataProvider.h>
 9 #include <test/fuzz/fuzz.h>
10+#include <test/fuzz/util.h>
11 #include <util/chaintype.h>
12 
13 #include <cassert>
14@@ -37,3 +39,13 @@ FUZZ_TARGET_INIT(key_io, initialize_key_io)
15         assert(ext_pub_key == DecodeExtPubKey(EncodeExtPubKey(ext_pub_key)));
16     }
17 }
18+
19+FUZZ_TARGET_INIT(key_io_silent_payments, initialize_key_io)
20+{
21+    FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
22+    const std::vector<unsigned char> data{ConsumeFixedLengthByteVector(fuzzed_data_provider, 64)};
23+    (void)DecodeSilentData(data);
24+
25+    const std::string silent_addr{fuzzed_data_provider.ConsumeRandomLengthString()};
26+    (void)DecodeSilentAddress(silent_addr);
27+}

in src/wallet/rpc/addresses.cpp:143 in 3a0d081ee0 outdated

138+    UniValue ret(UniValue::VOBJ);
139+    ret.pushKV("address", request.params[0].get_str());
140+    ret.pushKV("scan_pubkey", HexStr(scan_pubkey));
141+    ret.pushKV("spend_pubkey", HexStr(spend_pubkey));
142+
143+    const std::shared_ptr<const CWallet> pwallet = GetWalletForJSONRPCRequest(request);

brunoerg commented at 5:36 pm on July 13, 2023:

In 3a0d081ee07f3c65220a45855c96bf35ac2643fd: pwallet seems unecessary here? perhaps it’s a leftover?

 0UniValue ret(UniValue::VOBJ);
 1ret.pushKV("address", request.params[0].get_str());
 2ret.pushKV("scan_pubkey", HexStr(scan_pubkey));
 3ret.pushKV("spend_pubkey", HexStr(spend_pubkey));
 4
 5const std::shared_ptr<const CWallet> pwallet = GetWalletForJSONRPCRequest(request);
 6if (!pwallet) return ret;
 7
 8LOCK(pwallet->cs_wallet);
 9
10return ret;

in src/wallet/rpc/addresses.cpp:72 in 96f7a3a0c0 outdated

65@@ -66,6 +66,90 @@ RPCHelpMan getnewaddress()
66     };
67 }
68 
69+RPCHelpMan getsilentaddress()
70+{
71+    return RPCHelpMan{"getsilentaddress",
72+                "\nReturns a silent paymet address if it is enabled in wallet.\n",

brunoerg commented at 5:50 pm on July 13, 2023:

In 3a0d081ee07f3c65220a45855c96bf35ac2643fd: s/paymet/payment

in src/wallet/scriptpubkeyman.cpp:2704 in 12731a823d outdated

2640@@ -2641,6 +2641,49 @@ bool DescriptorScriptPubKeyMan::AddCryptedKey(const CKeyID& key_id, const CPubKe
2641     return true;
2642 }
2643 
2644+std::tuple<CKey,bool> DescriptorScriptPubKeyMan::GetPrivKeyForSilentPayment(const CScript& scriptPubKey, const bool onlyTaproot) const
2645+{
2646+    std::vector<std::vector<unsigned char>> solutions;
2647+    TxoutType whichType = Solver(scriptPubKey, solutions);

brunoerg commented at 1:39 pm on July 14, 2023:

In 12731a823d9ed3fde5dd19521da5d7544fcb07dd: Perhaps we could add a check/assert to ensure that solutions is not empty?

josibake renamed this:
~~[Silent Payments]: Base functionality~~
Silent Payments: Base functionality
on Jul 21, 2023

josibake renamed this:
~~Silent Payments: Base functionality~~
Silent Payments: wallet implementation
on Jul 21, 2023

josibake renamed this:
~~Silent Payments: wallet implementation~~
Silent Payments: send and receive
on Jul 21, 2023

in src/wallet/rpc/wallet.cpp:76 in 96f7a3a0c0 outdated

72@@ -69,6 +73,7 @@ static RPCHelpMan getwalletinfo()
73                         {RPCResult::Type::BOOL, "descriptors", "whether this wallet uses descriptors for scriptPubKey management"},
74                         {RPCResult::Type::BOOL, "external_signer", "whether this wallet is configured to use an external signer such as a hardware wallet"},
75                         {RPCResult::Type::BOOL, "blank", "Whether this wallet intentionally does not contain any keys, scripts, or descriptors"},
76+                        {RPCResult::Type::BOOL, "silent_payment", "whether this supports silent payments"},

brunoerg commented at 5:57 pm on July 25, 2023:

suggestion:

0                        {RPCResult::Type::BOOL, "silent_payment", "whether this wallet supports silent payments"},

in src/wallet/rpc/wallet.cpp:356 in 96f7a3a0c0 outdated

356@@ -351,6 +357,7 @@ static RPCHelpMan createwallet()
357                                                                        " support for creating and opening legacy wallets will be removed in the future."},
358             {"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."},
359             {"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."},
360+            {"silent_payment", RPCArg::Type::BOOL, RPCArg::Default{false}, "Experimental. Indicates that the wallet supports Silent Payments. This means a more complex scanning logic"},

brunoerg commented at 5:58 pm on July 25, 2023:

0            {"silent_payment", RPCArg::Type::BOOL, RPCArg::Default{false}, "Experimental. Indicates that the wallet supports Silent Payments. This means a more complex scanning logic."},

luke-jr commented at 10:42 pm on July 27, 2023: member

Can you explain your reasoning for wanting to split it up? I’d prefer to keep them together only because I see no reason to merge sending without receiving support in Bitcoin Core.

The sooner sending is supported everywhere, the sooner it is practical for people to use it for receiving. Might as well get that ball rolling ASAP.
Sending support means the BIP isn’t going to change under me (and break compatibility / risk coin loss) if I merge things in Knots first.
Core refuses to care about compatibility with other software, so I can’t merge anything that changes the wallet format in Knots until after it’s in a Core release.

conf: add EDCH module 8283ca76e4

crypto: update CKey, CPubkey for silent payments

* Add methods tweaking a CKey by adding a scalar value or by multiplying
* Add a method for combining multiple CPubKeys via addition
* Add a method for doing ECDH with a CPubKey and scalar

These are the CKey,CPubkey primitives we need to implement
the silent payments protocol.

376a9b9ad4

Increase the Bech32m limit if decoding SP address

Bech32m imposes a 90 character limit when decoding strings. Since a
silent payment address is the concatenation of two public keys, raise
this limit to 1024 when decoding a silent payment address.

The new limit is much higher than needed to account for forward
compatibility: new silent payment versions may add new data to the data
part of the address.

a469cb1e55

Add "sp" HRP 13e6c7b544

Add functions for decoding SP addresses

Add a function for decoding the string address and a second
function for decoding the data part of the silent payment address.

37b0ca1c3d

Add a function for encoding an SP address 328951d056

Implement BIP352: Silent Payments

Implement sending and receiving, per BIP352. This is done without
requiring a full wallet, in order to simplify unit testing and to create
a more clear boundary as to what pertains to the BIP and what is left to
the wallet to decided as an implementation.

To correctly hash the public keys, per the BIP, update the HashWriter.write
method to accept a Span<unsigned char>. This allows us to easily pass a
public key in for hashing, where only the data from the public key is hashed.

This commit also moves the `CRecipient` struct to wallet/types.h and
adds a new struct, `V0SilentPaymentDestination`. These are added here to
simplify implementing sending and receiving in the future.

8be9116939

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.

Also adds a comparator to `CRecipient`, to make testing easier.

8ee791e7b3

josibake force-pushed on Aug 2, 2023

DrahtBot removed the label Needs rebase on Aug 2, 2023

DrahtBot added the label CI failed on Aug 2, 2023

josibake marked this as a draft on Aug 2, 2023

refactor, wallet: use std::variant<CRecipient>

Instead of using CRecipient in the transaction creation workflow, use
std::variant<CRecipient>.

This sets us up to add V0SilentPaymentDestination as a new destination in a future commit.

90ff3dcacc

wallet: extend Destination variant

Add V0SilentPaymentDestination to the Destination variant. This commit
does not add any logic for what to do with the new destination type, so
this commit shouldn't introduce any behavior changes.

bc23aa6c25

refactor, move-only: move GetSerializeSize call

Move the `GetSerializeSizeFromDestination` call up to the first time
loop over vecSend.

89e67e8b30

move-only: add vouts after coin selection

Add the TxOuts to the transaction after coin selection. This gives us a
change to generate the silent payment outputs right after coin selection
and then add all the `CRecipients` to the transaction in one go.

There is a slight behavior change here: we are now checking if the vout
amounts are dust AFTER coin selection, whereas previously we were
checking this before coin selection.

14aa9b0fb8

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

2dd29c68c9

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.

c9a373b484

wallet: add `IsInputForSharedSecretDerivation` function a6addec821

wallet: add `CreateSilentPaymentOutputs` function 181e329a1d

wallet: add `SeparateDestinations` function 5510291734

wallet: create silent payment outputs

Using the new functions from the last few commits, put it all together:

* In `CreateTransactionInternal`, separate vecSend into a `CRecipient` vector and `V0SilentPaymentDestination` vector
* If there are silent payment destinations, call `CreateSilentPaymentOutputs` and add the newly created outputs to the `CRecipients` vector
* Convert the `CRecipients` to `CTxOut`s and add them to the transaction, same as before

126df9c463

wallet: make coin selection silent payment aware

Add a flag to the `CoinControl` object if silent payment destinations
are provided.

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.

4f5ff5a3e4

wallet: check if silent payments are possible

Add function to see if making a silent payments is possible:

* Check that the wallet has private keys
* Check that the wallet is unlocked

24642316a6

wallet: update TransactionChangeType

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

19065cf238

tests: add sending functional tests e6f7458324

wallet: create new type OutputType::SILENT_PAYMENT 99657441eb

script: add `sp()` descriptor 5aa9658a60

wallet: add Silent Payment flag 889e9c02d4

wallet: get wallet descriptor by output type e041c09e2e

wallet: enable SP option in the `createwallet`

Co-authored-by: Aurèle Oulès <aurele@oules.com>

1a57821e2c

receiver: encode silent payment address 98c3030284

wallet: add Silent Payment verification

Co-authored-by: josibake <josibake@protonmail.com>

51188520cd

tests: add receiving functional test a04d6356f1

josibake force-pushed on Aug 3, 2023

Sjors commented at 9:11 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!

You should also link from the child PRs back to this one.

DrahtBot removed the label CI failed on Aug 3, 2023

DrahtBot added the label CI failed on Sep 16, 2023

DrahtBot commented at 6:12 pm on September 19, 2023: contributor

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

DrahtBot added the label Needs rebase on Sep 19, 2023

josibake commented at 12:02 pm on September 26, 2023: member

Closing in favor of #28536, where on-going tracking will be done.

To test, compile #28453 (has both send and receive support)

josibake closed this on Sep 26, 2023

josibake deleted the branch on Jan 26, 2024

Silent Payments: send and receive #27827

For reviewers

PRs

Overall

Major changes

Reviews

Conflicts