Skip to content

Conversation

sdaftuar
Copy link
Member

@sdaftuar sdaftuar commented Jan 9, 2018

Avoid creating very small utxos that would violate an assumption in
test_non_standard_witness.

Fixes #11953

tx.wit.vtxinwit.append(CTxInWitness())
sign_P2PK_witness_input(witness_program, tx, index, SIGHASH_SINGLE|SIGHASH_ANYONECANPAY, i.nValue, key)
sign_P2PK_witness_input(witness_program, tx, index, SIGHASH_ALL|SIGHASH_ANYONECANPAY, i.nValue, key)
Copy link
Member

Choose a reason for hiding this comment

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

why this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

It felt wrong to use SIGHASH_SINGLE with an out-of-bounds index.

Copy link
Member

Choose a reason for hiding this comment

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

ah, ok then comment could use an update a few lines up.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops! Thanks, will fix.

@sdaftuar sdaftuar force-pushed the 2018-01-fix-p2p-segwit branch from 7fe5b20 to d18c310 Compare January 9, 2018 19:11
Avoid creating very small utxos that would violate an assumption in
test_non_standard_witness.
@sdaftuar sdaftuar force-pushed the 2018-01-fix-p2p-segwit branch from d18c310 to 35c2b1f Compare January 9, 2018 19:13
@jnewbery
Copy link
Contributor

jnewbery commented Jan 9, 2018

Tested ACk 35c2b1f

@maflcko
Copy link
Member

maflcko commented Jan 10, 2018

Thanks!

utACK 35c2b1f. Created a similar fix last year, but somehow forgot to submit it.

Copy link
Contributor

@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

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

utACK 35c2b1f. Change makes sense, but I couldn't figure out what problem the previous outputs caused in test_non_standard_witness.

@maflcko
Copy link
Member

maflcko commented Jan 10, 2018

In test_non_standard_witness, you subtract the fee at some point. Having really small outputs, this could lead to negative outputs, provoking the "bad-txns-vout-negative". The smallest observed value depends on the test run and the exception is hit very rarely non-deterministically.

@instagibbs
Copy link
Member

Yes the PR comment makes it sound like it's a non-standard witness, rather than non-standard output.

@fanquake fanquake added the Tests label Jan 11, 2018
@laanwj
Copy link
Member

laanwj commented Jan 11, 2018

utACK 35c2b1f

@laanwj laanwj merged commit 35c2b1f into bitcoin:master Jan 11, 2018
laanwj added a commit that referenced this pull request Jan 11, 2018
35c2b1f Fix rare failure in p2p-segwit.py (Suhas Daftuar)

Pull request description:

  Avoid creating very small utxos that would violate an assumption in
  test_non_standard_witness.

  Fixes #11953

Tree-SHA512: 5fb7ae68f8731df819bab365923a84568b57227e4112f711fc2574767d15be83acd3e99d0d5bac94a42411a958b13a2119468babefed14efcfdda180004d4166
@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.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

travis failure in p2p-segwit.py not(b'bad-txns-vout-negative' == b'bad-witness-nonstandard')
7 participants