Make IsSegWitOutput return true for taproot outputs #22421

1. 
   sipa commented at 5:00 PM on July 9, 2021: member

   This fixes a bug: currently utxoupdatepsbt will not fill in UTXO data for PSBTs spending taproot outputs.

2. 
   sipa commented at 5:00 PM on July 9, 2021: member

   Noticed by @roconnor-blockstream.

3. 
   Xekyo commented at 5:11 PM on July 9, 2021: member

   Concept ACK

4. in src/script/sign.cpp:617 in e3e05948c1 outdated

    613 | @@ -614,7 +614,10 @@ bool IsSegWitOutput(const SigningProvider& provider, const CScript& script)
    614 |  {
    615 |      std::vector<valtype> solutions;
    616 |      auto whichtype = Solver(script, solutions);
    617 | -    if (whichtype == TxoutType::WITNESS_V0_SCRIPTHASH || whichtype == TxoutType::WITNESS_V0_KEYHASH || whichtype == TxoutType::WITNESS_UNKNOWN) return true;
    618 | +    if (whichtype == TxoutType::WITNESS_V0_SCRIPTHASH ||
   

   ---

   ---

   

   MarcoFalke commented at 5:28 AM on July 10, 2021:

   nit: Maybe turn this into a switch-case to get compiler output when v2 is added, but this isn't updated?

   ---

   

   MarcoFalke commented at 8:51 AM on July 15, 2021:

   @sipa Are you still working on this? Happy to take over if not.

5. MarcoFalke approved
6. MarcoFalke added this to the milestone 22.0 on Jul 10, 2021
7. 
   laanwj commented at 8:13 AM on July 12, 2021: member

   ACK, but I like @MarcoFalke's suggestion to make the compiler complain about an uncovered case in case more enum items are added in the future, and this function is not updated.

8. 
   GeneFerneau commented at 7:07 PM on July 15, 2021: none

   Concept + review ACK e3e0594

9. Make IsSegWitOutput return true for taproot outputs 8465978f23
10. sipa force-pushed on Jul 16, 2021
11. 
    sipa commented at 12:09 AM on July 16, 2021: member

    I took another approach, and made it us IsWitnessProgram instead of trying to match against all variations that now exist as output from Solver.

12. in src/script/sign.cpp:625 in 8465978f23

     629 | +        auto whichtype = Solver(script, solutions);
     630 | +        if (whichtype == TxoutType::SCRIPTHASH) {
     631 | +            auto h160 = uint160(solutions[0]);
     632 | +            CScript subscript;
     633 | +            if (provider.GetCScript(CScriptID{h160}, subscript)) {
     634 | +                if (subscript.IsWitnessProgram(version, program)) return true;
    

    ---

    ---

    

    achow101 commented at 12:15 AM on July 16, 2021:

    This would return true for Taproot scripts that are in the P2SH even though such scripts cannot be spent. Do we want to do that?

    ---

    

    sipa commented at 12:19 AM on July 16, 2021:

    Yes, I noticed that, but I think my answer is yes. It also used to return true for unknown witness versions, so no reason to exclude v1 p2sh. It's still segwit; just not taproot.

13. fanquake deleted a comment on Jul 16, 2021
14. 
    achow101 commented at 6:32 PM on July 16, 2021: member

    Code Review ACK 8465978f235e2e43feb5dabe2a4d61026343b6ab

15. 
    jonatack commented at 7:32 PM on July 16, 2021: member

    ACK 8465978f235e2e43feb5dabe2a4d61026343b6ab

    Could this use test coverage?

16. meshcollider added the label Wallet on Jul 18, 2021
17. 
    meshcollider commented at 7:35 AM on July 18, 2021: contributor

    utACK 8465978f235e2e43feb5dabe2a4d61026343b6ab

18. meshcollider merged this on Jul 18, 2021
19. meshcollider closed this on Jul 18, 2021

    ---

20. sidhujag referenced this in commit ff3a8f5415 on Jul 23, 2021
21. roconnor-blockstream referenced this in commit 3c2c27ddf0 on Jul 25, 2021
22. gwillen referenced this in commit b387aac530 on Jun 1, 2022
23. DrahtBot locked this on Aug 18, 2022

Contributors

sipaXekyoMarcoFalkelaanwjGeneFerneauachow101jonatackmeshcollider

Milestone
22.0