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. MarcoFalke cross-referenced this on Mar 22, 2022 from issue rpc: have raw transaction decoding infer output descriptors by instagibbs
7. rpc: Exclude descriptor when address is excluded faf37c217a
8. MarcoFalke force-pushed on Mar 23, 2022
9. MarcoFalke renamed this:
   doc: Fix decodepsbt docs
   rpc: Exclude descriptor when address is excluded
   on Mar 23, 2022
10. MarcoFalke removed the label Docs on Mar 23, 2022
11. MarcoFalke marked this as ready for review on Mar 23, 2022
12. 
    MarcoFalke commented at 10:18 AM on March 23, 2022: member

    Completely reworked the pull

13. 
    achow101 commented at 4:59 PM on March 23, 2022: member

    ACK faf37c217a408114224f91b7ada3fb6ff29b0c0a

14. 
    fanquake commented at 8:12 AM on March 24, 2022: member

    cc @meshcollider @theStack @instagibbs @sipa

15. 
    jonatack commented at 12:04 PM on March 24, 2022: contributor

    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

    src/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

        // inputs
        CAmount total_in = 0;
        bool have_all_utxos = true;
        UniValue inputs(UniValue::VARR);
        for (unsigned int i = 0; i < psbtx.inputs.size(); ++i) {
            const PSBTInput& input = psbtx.inputs[i];
            UniValue in(UniValue::VOBJ);
            // UTXOs
            bool have_a_utxo = false;
            CTxOut txout;
            if (!input.witness_utxo.IsNull()) {
                txout = input.witness_utxo;
    
                UniValue o(UniValue::VOBJ);
                ScriptPubKeyToUniv(txout.scriptPubKey, o, /* include_hex */ true);
    
                UniValue out(UniValue::VOBJ);
                out.pushKV("amount", ValueFromAmount(txout.nValue));
                out.pushKV("scriptPubKey", o);
    
                in.pushKV("witness_utxo", out);
    
                have_a_utxo = true;
            }
    

    relevant updated decodepsbt help

      "inputs" : [                             (json array)
        {                                      (json object)
          "non_witness_utxo" : {               (json object, optional) Decoded network transaction for non-witness UTXOs
            ...
          },
          "witness_utxo" : {                   (json object, optional) Transaction output for witness UTXOs
            "amount" : n,                      (numeric) The value in BTC
            "scriptPubKey" : {                 (json object)
              "asm" : "str",                   (string) The asm
              "desc" : "str",                  (string) Inferred descriptor for the output
              "hex" : "hex",                   (string) The hex
              "type" : "str",                  (string) The type, eg 'pubkeyhash'
              "address" : "str"                (string, optional) The Bitcoin address (only if a well-defined address exists)
            }
    

    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

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

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

    ---

18. MarcoFalke deleted the branch on Mar 24, 2022
19. 
    jonatack commented at 11:13 AM on March 28, 2022: contributor

    Backported to v23.0 in #24512.

20. fanquake removed the label Needs backport (23.x) on Mar 28, 2022
21. jonatack referenced this in commit f712b523fa on Mar 28, 2022
22. jonatack cross-referenced this on Mar 28, 2022 from issue 23.x backports by jonatack
23. hebasto referenced this in commit 92da8a3cc4 on Mar 31, 2022
24. jonatack referenced this in commit 235b042594 on Mar 31, 2022
25. fanquake referenced this in commit c243e08351 on Mar 31, 2022
26. sidhujag referenced this in commit d66d0d3aeb on Apr 2, 2022
27. bitcoin locked this on Mar 28, 2023

Contributors

MarcoFalkeachow101fanquakejonatack

Milestone
23.0