[psbt] Convert non-witness UTXOs to witness if witness sig created #14197

1. 
   achow101 commented at 4:15 am on September 11, 2018: member

   If a witness signature was created when a non-witness UTXO is used, convert the non-witness UTXO to a witness one.

   Port of #14196 to master.

2. achow101 force-pushed on Sep 11, 2018
3. in src/script/sign.cpp:279 in f03eb813df outdated

   275@@ -276,6 +276,13 @@ bool SignPSBTInput(const SigningProvider& provider, const CMutableTransaction& t
   276     // Verify that a witness signature was produced in case one was required.
   277     if (require_witness_sig && !sigdata.witness) return false;
   278     input.FromSignatureData(sigdata);
   279+    
   

   ---

   ---

   

   fanquake commented at 5:20 am on September 11, 2018:

   Travis is complaining about whitespace here.

   ---

   

   achow101 commented at 6:54 pm on September 11, 2018:

   Fixed
4. achow101 force-pushed on Sep 11, 2018
5. laanwj added this to the "Blockers" column in a project

   ---

6. 
   gmaxwell commented at 4:48 pm on September 14, 2018: contributor

   ACK
7. 
   sipa commented at 1:39 am on September 16, 2018: member

   utACK 52d9adcdfbf545cb9a9facb482cb3673578abfa0. I’ll test this soon.
8. in src/script/sign.cpp:283 in 52d9adcdfb outdated

   276@@ -277,6 +277,13 @@ bool SignPSBTInput(const SigningProvider& provider, const CMutableTransaction& t
   277     if (require_witness_sig && !sigdata.witness) return false;
   278     input.FromSignatureData(sigdata);
   279 
   280+    if (sigdata.witness) {
   281+        // Convert the non-witness utxo to witness
   282+        if (input.witness_utxo.IsNull() && input.non_witness_utxo) {
   283+            input.witness_utxo = input.non_witness_utxo->vout[tx.vin[index].prevout.n];
   

   ---

   ---

   

   promag commented at 2:08 pm on September 17, 2018:

   IIUC this could be

   0assert(!utxo.IsNull());
   1input.witness_utxo = utxo;
   

   ---

   

   achow101 commented at 7:26 pm on September 22, 2018:

   Indeed. Done.
9. 
   sipa commented at 10:59 pm on September 20, 2018: member

   Here is a test that fails before and succeeds after this PR: https://github.com/sipa/bitcoin/commit/ce238f2aff3253cdde88d0f9b995b7e0016cb52b
10. 
    sipa commented at 5:53 pm on September 22, 2018: member

    @achow101 Feel free to cherry-pick the test into this PR.

    That RPC test is a reproduction of what I did to notice the issue in the first place, so Tested ACK from me.

11. achow101 force-pushed on Sep 22, 2018
12. 
    achow101 commented at 7:26 pm on September 22, 2018: member

    I’ve added @sipa’s test
13. achow101 force-pushed on Sep 22, 2018
14. Convert non-witness UTXOs to witness if witness sig created

    If a witness signature was created when a non-witness UTXO is used,
    convert the non-witness UTXO to a witness one.

    f8c1714634
15. Add test for conversion from non-witness to witness UTXO 862d159d63
16. achow101 force-pushed on Sep 22, 2018
17. 
    DrahtBot commented at 8:04 pm on September 22, 2018: member

    • #14588 (Refactor PSBT signing logic to enforce invariant and fix signing bug by gwillen)
    • #13932 (Additional utility RPCs for PSBT by achow101)

    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.

18. 
    sipa commented at 8:26 pm on September 23, 2018: member

    ACK 862d159d635c1de219d94e030b186a745fe28eb9
19. in test/functional/rpc_psbt.py:9 in 862d159d63

     5@@ -6,7 +6,7 @@
     6 """
     7 
     8 from test_framework.test_framework import BitcoinTestFramework
     9-from test_framework.util import assert_equal, assert_raises_rpc_error, find_output
    10+from test_framework.util import assert_equal, assert_raises_rpc_error, find_output, disconnect_nodes, connect_nodes_bi, sync_blocks
    

    ---

    ---

    

    promag commented at 4:02 pm on September 26, 2018:

    nit, nit, nit, sort.
20. 
    promag commented at 4:09 pm on September 26, 2018: member

    utACK 862d159.
21. 
    DrahtBot commented at 11:23 am on September 28, 2018: member

    Coverage	Change (pull 14197)	Reference (master)
    Lines	+0.0163 %	87.0361 %
    Functions	+0.1081 %	84.1130 %
    Branches	-0.0057 %	51.5451 %

22. AdamISZ referenced this in commit fcb0c6b68b on Oct 18, 2018
23. 
    laanwj commented at 3:10 pm on November 1, 2018: member

    utACK 862d159d635c1de219d94e030b186a745fe28eb9
24. laanwj added the label RPC/REST/ZMQ on Nov 1, 2018
25. laanwj merged this on Nov 1, 2018
26. laanwj closed this on Nov 1, 2018

    ---

27. laanwj referenced this in commit f6df989842 on Nov 1, 2018
28. laanwj removed this from the "Blockers" column in a project

    ---

29. 
    fanquake commented at 2:30 pm on November 30, 2018: member

    862d159 will be backported in #14780.
30. sipa referenced this in commit 7bee41452b on Dec 3, 2018
31. MarcoFalke referenced this in commit 5d12143c73 on Dec 5, 2018
32. DrahtBot locked this on Sep 8, 2021

Contributors
achow101 fanquake gmaxwell sipa promag DrahtBot laanwj

Labels
RPC/REST/ZMQ