rpc: Exclude descriptor when address is excluded #24636

1. 
   MarcoFalke commented at 11:00 am on March 22, 2022: member

   I don’t think output descriptors should be used to describe redeem scripts and witness scripts.

   Fix this by excluding them when it doesn’t make sense.

   This should only affect the decodepsbt RPC.

   Found by https://github.com/bitcoin/bitcoin/pull/23083

2. MarcoFalke added the label Needs backport (23.x) on Mar 22, 2022
3. MarcoFalke added the label Docs on Mar 22, 2022
4. MarcoFalke added the label RPC/REST/ZMQ on Mar 22, 2022
5. MarcoFalke added this to the milestone 23.0 on Mar 22, 2022
6. rpc: Exclude descriptor when address is excluded faf37c217a
7. MarcoFalke force-pushed on Mar 23, 2022
8. MarcoFalke renamed this:
   doc: Fix decodepsbt docs
   rpc: Exclude descriptor when address is excluded
   on Mar 23, 2022
9. MarcoFalke removed the label Docs on Mar 23, 2022
10. MarcoFalke marked this as ready for review on Mar 23, 2022
11. 
    MarcoFalke commented at 10:18 am on March 23, 2022: member

    Completely reworked the pull
12. 
    achow101 commented at 4:59 pm on March 23, 2022: member

    ACK faf37c217a408114224f91b7ada3fb6ff29b0c0a
13. 
    fanquake commented at 8:12 am on March 24, 2022: member

    cc @meshcollider @theStack @instagibbs @sipa
14. 
    jonatack commented at 12:04 pm on March 24, 2022: member

    ACK faf37c217a408114224f91b7ada3fb6ff29b0c0a

    This change adds a “desc” field that was missing from the “witness_utxo” decodepsbt inputs help and IIUC removes undocumented “desc” descriptor fields from the decodepsbt inputs and outputs “redeem_script” and “witness_script” result objects.

    Code review:

    ScriptPubKeyToUniv() is declared with include_address = true by default

    0src/core_io.h:56:void ScriptPubKeyToUniv(const CScript& scriptPubKey, UniValue& out, bool include_hex, bool include_address = true);
    

    and is invoked once in decodepsbt in the witness inputs code

     0    // inputs
     1    CAmount total_in = 0;
     2    bool have_all_utxos = true;
     3    UniValue inputs(UniValue::VARR);
     4    for (unsigned int i = 0; i < psbtx.inputs.size(); ++i) {
     5        const PSBTInput& input = psbtx.inputs[i];
     6        UniValue in(UniValue::VOBJ);
     7        // UTXOs
     8        bool have_a_utxo = false;
     9        CTxOut txout;
    10        if (!input.witness_utxo.IsNull()) {
    11            txout = input.witness_utxo;
    12
    13            UniValue o(UniValue::VOBJ);
    14            ScriptPubKeyToUniv(txout.scriptPubKey, o, /* include_hex */ true);
    15
    16            UniValue out(UniValue::VOBJ);
    17            out.pushKV("amount", ValueFromAmount(txout.nValue));
    18            out.pushKV("scriptPubKey", o);
    19
    20            in.pushKV("witness_utxo", out);
    21
    22            have_a_utxo = true;
    23        }
    

    relevant updated decodepsbt help

     0  "inputs" : [                             (json array)
     1    {                                      (json object)
     2      "non_witness_utxo" : {               (json object, optional) Decoded network transaction for non-witness UTXOs
     3        ...
     4      },
     5      "witness_utxo" : {                   (json object, optional) Transaction output for witness UTXOs
     6        "amount" : n,                      (numeric) The value in BTC
     7        "scriptPubKey" : {                 (json object)
     8          "asm" : "str",                   (string) The asm
     9          "desc" : "str",                  (string) Inferred descriptor for the output
    10          "hex" : "hex",                   (string) The hex
    11          "type" : "str",                  (string) The type, eg 'pubkeyhash'
    12          "address" : "str"                (string, optional) The Bitcoin address (only if a well-defined address exists)
    13        }
    

    OTOH ScriptToUniv() is the only caller to invoke ScriptPubKeyToUniv() with include_address = false, and outside of a fuzz test is only called in decodepsbt inputs and outputs “redeem_script” and “witness_script” result code and so with this pull the descriptor will be excluded

    0void ScriptToUniv(const CScript& script, UniValue& out)
    1{
    2    ScriptPubKeyToUniv(script, out, /* include_hex */ true, /* include_address */ false);
    3}
    

15. fanquake merged this on Mar 24, 2022
16. fanquake closed this on Mar 24, 2022

    ---

17. MarcoFalke deleted the branch on Mar 24, 2022
18. 
    jonatack commented at 11:13 am on March 28, 2022: member

    Backported to v23.0 in #24512.
19. fanquake removed the label Needs backport (23.x) on Mar 28, 2022
20. jonatack referenced this in commit f712b523fa on Mar 28, 2022
21. hebasto referenced this in commit 92da8a3cc4 on Mar 31, 2022
22. jonatack referenced this in commit 235b042594 on Mar 31, 2022
23. fanquake referenced this in commit c243e08351 on Mar 31, 2022
24. sidhujag referenced this in commit d66d0d3aeb on Apr 2, 2022
25. DrahtBot locked this on Mar 28, 2023

Contributors
MarcoFalke achow101 fanquake jonatack

Labels
RPC/REST/ZMQ

Milestone
23.0