fixup disallowing 0-value spendable outputs #143
Conversation
src/main.cpp
Outdated
There was a problem hiding this comment.
Just to clarify, does secp256k1_pedersen_commit require val.GetAmount() > 0 or is this an emergency assert?
There was a problem hiding this comment.
An input should never happen like this if the logic is correct on the
output side. I thought an assert made the most sense here since it shouldn't be violated.
There was a problem hiding this comment.
If you try to make a commitment of value 0 with no blinder, I think it hits a libsecp assert.
|
It could be really useful to have some tests for this. |
|
Updated with tests and a fix making sure that unspendable 0-value outputs are valid in coinbase. |
| # Split the network and build two chains of different lengths. | ||
| self.split_network () | ||
| self.nodes[0].generate(10) | ||
| self.nodes[2].sendtoaddress(self.nodes[2].getnewaddress(), 1) |
There was a problem hiding this comment.
this splits the two chains to ensure two tips, since 0-value coinbase must be unspendable(and OP_RETURN by default, meaning the two chains would be otherwise identical).
| # node1, and make sure both node0 and node2 pick them up properly: | ||
| node0utxos = self.nodes[0].listunspent(1, 9999999, [], "bitcoin") | ||
| assert_equal(len(node0utxos), 104) | ||
| assert_equal(len(node0utxos), 3) |
There was a problem hiding this comment.
wallet only has 3 coins now that 101 of them are unspendable
| coinbaseTx.vin[0].prevout.SetNull(); | ||
| coinbaseTx.vout.resize(1); | ||
| coinbaseTx.vout[0].scriptPubKey = scriptPubKeyIn; | ||
| coinbaseTx.vout[0].scriptPubKey = nFees ? scriptPubKeyIn : CScript() << OP_RETURN; |
There was a problem hiding this comment.
miners will no longer create 0-value spendable outputs that are illegal
|
tACK e1ea5a1 although I'd add the RPC related comments you made in the PR to the code. |
2 participants
Also fixed the coinbase value checking for values out of range.