Silent Payments: receiving #28202

pull josibake wants to merge 16 commits into bitcoin:master from josibake:implement-bip352-receiving changing 38 files +3096 −53
  1. josibake commented at 4:42 pm on August 2, 2023: member

    This PR is a child of #27827 and only implements the receiving logic. For the rest:

    • #28122 - implement the silent payments scheme
    • #28201 - implements sending
    • #27827 - all three PRs together - this PR is meant for tracking progress and is useful for reviewers who want to compile all three PRs at once for testing

    This PR depends on #28122 and is marked as a draft until it is merged. Commits up to https://github.com/bitcoin/bitcoin/commit/8ee791e7b34a354ce6835b968fe12924b8908d7c belong to #28122; please review those commits on #28122

    Receiving

    Description coming soon

  2. conf: add EDCH module 8283ca76e4
  3. 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
  4. 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
  5. Add "sp" HRP 13e6c7b544
  6. 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
  7. Add a function for encoding an SP address 328951d056
  8. 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
  9. 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
  10. wallet: create new type OutputType::SILENT_PAYMENT 8d994890ef
  11. script: add `sp()` descriptor b15cbcb47a
  12. wallet: add Silent Payment flag 4f3784431b
  13. wallet: get wallet descriptor by output type 03ebe23243
  14. DrahtBot commented at 4:42 pm on August 2, 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
    Concept ACK BrandonOdiwuor

    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)
    • #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)
    • #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)
    • #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.

  15. DrahtBot added the label CI failed on Aug 2, 2023
  16. josibake renamed this:
    Silent Payments: implement receiving
    Silent Payments: receiving
    on Aug 3, 2023
  17. wallet: enable SP option in the `createwallet` 
    Co-authored-by: Aurèle Oulès <aurele@oules.com>
    795792674e
  18. receiver: encode silent payment address d947eda6f2
  19. wallet: add Silent Payment verification 
    Co-authored-by: josibake <josibake@protonmail.com>
    703b41615e
  20. josibake force-pushed on Aug 3, 2023
  21. tests: add receiving functional test 3f1b810e5d
  22. josibake force-pushed on Aug 3, 2023
  23. DrahtBot removed the label CI failed on Aug 3, 2023
  24. in src/wallet/wallet.cpp:1676 in 703b41615e outdated 
    1671+                    CPubKey pubkey{stack.back()};
1672+                    assert(pubkey.IsFullyValid());
1673+                    pubkeys_for_shared_secret.push_back(pubkey);
1674+                    continue;
1675+                }
1676+            case TxoutType::SCRIPTHASH:
    achow101 commented at 12:19 pm on August 9, 2023:
    Should recurse on SCRIPTHASH instead of assuming.
  25. in src/wallet/wallet.cpp:2136 in 3f1b810e5d 
    2127@@ -1852,6 +2128,24 @@ int64_t CWallet::RescanFromTime(int64_t startTime, const WalletRescanReserver& r
2128     return startTime;
2129 }
2130 
2131+Coin CWallet::FindPreviousCoin(const CTxIn& txin) const
2132+{
2133+    // Find the UTXO being spent in UTXO Set to learn the transaction type
2134+    std::map<COutPoint, Coin> coins;
2135+    coins[txin.prevout];
2136+    chain().findCoins(coins);
    achow101 commented at 12:22 pm on August 9, 2023:
    This looks in the UTXO set but is called on a transaction that has already been verified and so therefore it’s inputs are no longer in the UTXO set. This results in the Coin being uninitialized which causes later Solver calls to fail unexpectedly.
    achow101 commented at 12:54 pm on August 9, 2023:
    As discussed offline, we shouldn’t use any view of the UTXO set. Rather we should use the undo data when receiving a blockConnected, and modify TransactionAddedToMempool to include the previous outputs. This lets us avoid any race conditions with the state of the UTXO set and guarantees that the previous output data will always be available for the silent payments calculations.
  26. BrandonOdiwuor commented at 2:57 pm on August 16, 2023: contributor

    Tested ACK with some comments at the end

    Was able to successfully run unit and functional tests on ‘pr/28202’ branch and verified that the tests fail against the master branch

    Manually tested on regtest as described below:

    0$ ./src/bitcoin-cli -regtest -named createwallet wallet_name="sp-wallet" silent_payment=true

     0$ ./src/bitcoin-cli -regtest  getwalletinfo # showed that a wallet with silent payment enabled was created successfully
 1
 2{
 3    "walletname": "sp-wallet",
 4    "walletversion": 169900,
 5    "format": "sqlite",
 6    "balance": 0.00000000,
 7    "unconfirmed_balance": 0.00000000,
 8    "immature_balance": 0.00000000,
 9    "txcount": 0,
10    "keypoolsize": 4001,
11    "keypoolsize_hd_internal": 4000,
12    "paytxfee": 0.00000000,
13    "private_keys_enabled": true,
14    "avoid_reuse": false,
15    "scanning": false,
16    "descriptors": true,
17    "external_signer": false,
18    "blank": false,
19    "silent_payment": true,
20    "lastprocessedblock": {
21        "hash": "2fbd2f41ecd9e553cc8d129cff71846e0f26da7584eb8362e7cc70c0913b8e98",
22        "height": 212
23    }
24}

     0$ ./src/bitcoin-cli -regtest listdescriptors # was able to list a silent payment descriptor
 1{
 2…
 3    {                                                                                                                                                                                                              
 4        "desc": "sp([f3a37e3d/352h/1h/0h/1h]tpubDFMFwrp8wbV1voiEY3dy6ba5mv3nzbzRQmBHYfu6c3gGnf49QM9PB6Uy4P3Rpb5YcgXHT4vcXCpWiDt8rSoQXceJenU2WkvxcV6rYAxVxm6/0,[f3a37e3d/352h/1h/0h/0h]tpubDFMFwrp8wbV1tkGrdjqb8VSRp7srJQGvtqxmBSU3LVySKFPzCH1sP9cuLtUHA2Cz6qFSXdi2F52gAGNzqMrZrGrNwmf7EzbxebVEzBKugvm/0)#qzhxnezt",
 5        "timestamp": 1692181466,
 6        "active": true,
 7        "internal": false,
 8        "next": 0
 9    },
10…
11}

    0$ ./src/bitcoin-cli -regtest  getsilentaddress # was able to generate a silent payment address
1{
2    "address": "sprt1qq0yr8yv2dzrrvkyde8lex6qjy2jklgpxud260yvepgdfh5rh2ldj2qket9cwqj9xt5j7mseqk4appaqzung4xhwknaxcchlf54vutvv4mcper7ca"
3}

    0$ ./src/bitcoin-cli -regtest  decodesilentaddress sprt1qq0yr8yv2dzrrvkyde8lex6qjy2jklgpxud260yvepgdfh5rh2ldj2qket9cwqj9xt5j7mseqk4appaqzung4xhwknaxcchlf54vutvv4mcper7ca 
1# was able to correctly decode the silent payment address above into scan and spen pubkeys
2{
3    "address": "sprt1qq0yr8yv2dzrrvkyde8lex6qjy2jklgpxud260yvepgdfh5rh2ldj2qket9cwqj9xt5j7mseqk4appaqzung4xhwknaxcchlf54vutvv4mcper7ca",
4    "scan_pubkey": "03c833918a688636588dc9ff93681222a56fa026e355a791990a1a9bd07757db25",
5    "spend_pubkey": "02d95970e048a65d25edc320b57a10f402e4d1535dd69f4d8c5fe9a559c5b195de"
6}

    comments

    • I noticed that ./src/bitcoin-cli -regtest listdescriptors only listed one descriptor for silent payments
    • Also that ./src/bitcoin-cli -regtest getsilentaddress always return the same one address every time
  27. DrahtBot added the label CI failed on Sep 15, 2023
  28. DrahtBot commented at 6:10 pm on September 19, 2023: contributor

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

  29. DrahtBot added the label Needs rebase on Sep 19, 2023
  30. josibake commented at 11:58 am on September 26, 2023: member

    Closing in favor of #28453

    Originally, the idea was to implement sending and receiving separately, but it makes more sense to have them together so the receiving PR can use silent payments for generating change.

  31. josibake closed this on Sep 26, 2023
  32. josibake deleted the branch on Jan 26, 2024


josibake DrahtBot achow101 BrandonOdiwuor

Labels
Needs rebase CI failed

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-06-29 07:13 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me