Skip to content

Conversation

@achow101
Copy link
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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Travis is complaining about whitespace here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@gmaxwell
Copy link
Contributor

ACK

@sipa
Copy link
Member

sipa commented Sep 16, 2018

utACK 52d9adc. I'll test this soon.

if (sigdata.witness) {
// Convert the non-witness utxo to witness
if (input.witness_utxo.IsNull() && input.non_witness_utxo) {
input.witness_utxo = input.non_witness_utxo->vout[tx.vin[index].prevout.n];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC this could be

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. Done.

@sipa
Copy link
Member

sipa commented Sep 20, 2018

Here is a test that fails before and succeeds after this PR: sipa@ce238f2

@sipa
Copy link
Member

sipa commented Sep 22, 2018

@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.

@achow101
Copy link
Member Author

I've added @sipa's test

achow101 and others added 2 commits September 22, 2018 15:27
If a witness signature was created when a non-witness UTXO is used,
convert the non-witness UTXO to a witness one.
@DrahtBot
Copy link
Contributor

DrahtBot commented Sep 22, 2018

Reviewers, this pull request conflicts with the following ones:

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.

@sipa
Copy link
Member

sipa commented Sep 23, 2018

ACK 862d159

Copy link
Contributor

@promag promag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK 862d159.


from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import assert_equal, assert_raises_rpc_error, find_output
from test_framework.util import assert_equal, assert_raises_rpc_error, find_output, disconnect_nodes, connect_nodes_bi, sync_blocks
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, nit, nit, sort.

@DrahtBot
Copy link
Contributor

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

@laanwj
Copy link
Member

laanwj commented Nov 1, 2018

utACK 862d159

@laanwj laanwj merged commit 862d159 into bitcoin:master Nov 1, 2018
laanwj added a commit that referenced this pull request Nov 1, 2018
…sig created

862d159 Add test for conversion from non-witness to witness UTXO (Pieter Wuille)
f8c1714 Convert non-witness UTXOs to witness if witness sig created (Andrew Chow)

Pull request description:

  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.

Tree-SHA512: 2235eeb008ffa48e821628032d689e4a83bff6c29b93fa050ab2ee492b0e67b3a30f29a680d4a0e574e05c3a2f9edf0005e161fbe25b7aef2acd034a2424e2f2
@sipa sipa mentioned this pull request Nov 22, 2018
@fanquake
Copy link
Member

862d159 will be backported in #14780.

sipa added a commit to sipa/bitcoin that referenced this pull request Dec 3, 2018
maflcko pushed a commit that referenced this pull request Dec 5, 2018
7bee414 Add test for conversion from non-witness to witness UTXO (Pieter Wuille)
ff56bb9 Add regression test for PSBT signing bug #14473 (Glenn Willen)
db445d4 Refactor PSBTInput signing to enforce invariant (Glenn Willen)
ad94165 Simplify arguments to SignPSBTInput (Glenn Willen)
39ece4f Add bool PSBTInputSigned (Glenn Willen)
70ee1f8 New PartiallySignedTransaction constructor from CTransction (Glenn Willen)
a9eab08 Remove redundant txConst parameter to FillPSBT (Glenn Willen)
cfdd6b2 More concise conversion of CDataStream to string (Glenn Willen)
a3fe125 check that a separator is found for psbt inputs, outputs, and global map (Andrew Chow)

Pull request description:

  This is a backport of #14588, #14377, and #14197's test to 0.17.

Tree-SHA512: 07535ec69a878a63b549e5e463345e233f34662dff805202614cf2ffc896c6d1981363e6d06d02db2e02d815075ad8ebdc5f93f637052cff8c8cbe6c8dfa096a
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants