Prune provably-unspendable outputs

sipa commented at 8:25 pm on June 24, 2013: member

Automatically prune outputs (at creation) which are provably unspendable.

sipa commented at 8:25 pm on June 24, 2013: member

Tested by running a -reindex on mainnet.

Mazo commented at 10:22 am on June 26, 2013: none

What kind of effect does this have on the blockchain size?

jgarzik commented at 11:56 am on June 26, 2013: contributor

@Mazo None. The blockchain continues to store every single transaction, from 2009 through eternity.

mikehearn commented at 10:36 am on July 5, 2013: contributor

It looks good to me. Are there any unit tests for blocks and UTXO set changes? If so it’d be good to add a test for this.

TheBlueMatt commented at 1:02 pm on July 7, 2013: member

/me votes new tests be added to https://code.google.com/p/bitcoinj/source/browse/core/src/test/java/com/google/bitcoin/core/FullBlockTestGenerator.java

sipa commented at 1:05 pm on July 7, 2013: member

How would you test it? There is no observable difference.

You can try to create OP_RETURN outputs and try to spend them, and see that fails, but that’s true before and after this PR (which doesn’t mean it’s a useless test).

TheBlueMatt commented at 1:09 pm on July 7, 2013: member

Well, yes, testing this properly probably cant be done out-of-process (maybe over rpc, though), but adding some OP_RETURN scripts to the block test-set (and some that look semi-unspendable, eg OP_RETURN in an IF) would be really nice.

petertodd commented at 3:35 pm on July 8, 2013: contributor

@sipa @TheBlueMatt I added tests for attempting to spend OP_RETURNS, including with IF’s and similar, to the unit tests actually. I agree there should be tests to ensure they don’t end up in the UTXO set though.

Prune provably-unspendable outputs ec84e81e83

BitcoinPullTester commented at 10:48 pm on July 9, 2013: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/ec84e81e8383b3b1e1ef4a6dbcb088193d8de5d7 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

sipa commented at 10:51 pm on July 9, 2013: member

@petertodd @TheBlueMatt I agree we need tests to verify that such unspendable outputs don’t end up in the UTXO set, but I disagree it should be part of pulltester. This does not affect network interaction, but is a client-side optimization.

TheBlueMatt commented at 10:12 am on July 10, 2013: member

The point of adding it to pull-tester is that if it is done wrong, it is network interaction.

Pieter Wuille notifications@github.com wrote:

@petertodd @TheBlueMatt I agree we need tests to verify that such unspendable outputs don’t end up in the UTXO set, but I disagree it should be part of pulltester. This does not affect network interaction, but is a client-side optimization.

Reply to this email directly or view it on GitHub: #2791 (comment)

sipa commented at 9:28 pm on July 10, 2013: member

@TheBlueMatt Just to be clear: I’m all for adding tests to pulltester intended to trigger edge cases related to handling of provably-unspendable outputs. I’m just not in favor of making pulltester use more than the P2P interface for testing (i.e., making it a whitebox rather than blackbox test) - tests for checking that unspendable outputs do not end up in the UTXO set are for unit tests, not for network interaction testing.

petertodd commented at 7:16 am on July 11, 2013: contributor

Over the network how would you know if a peer wasn’t doing OP_RETURN pruning properly anyway if their script implementation was correct?

sipa commented at 11:59 am on July 14, 2013: member

@petertodd That’s my point - anyone in the network only cares whether your script/verification implementation is correct. Only you care about whether it’s not using more storage than necessary.

I prefer the block-acceptance tests to remain implementation-independent, so it can remain as generic as possible, and be used to find bugs that could lead to non-convergence - not only for bitcoind/bitcoin-qt. If another implementation chooses not to implement pruning, that’s their choice. Sure, it would change the economics of the system as a whole, but it’s not a network rule violation to do so.

Of course, if a some form of committed-UTXO-set-in-coinbase is ever added, the actual UTXO set becomes observable to the network rules, and this changes of course.

We need unit tests for verifying that provably-unspendable outputs do not end up in the UTXO set though.

jgarzik commented at 3:16 pm on July 19, 2013: contributor

ACK

gmaxwell commented at 3:21 pm on July 19, 2013: contributor

So this will make the gettxoutsetinfo on existing nodes diverge until everyone does a -reindex. Likewise, gettxout will return different results. Is this a problem?

One possibility would be to write a small piece of code that checks at startup to see if a particular OP_RETURN output is in the UTXO set, and if it is, traverses the set to remove all of them. At a minimum there should likely be a comment stuck someplace in the code to remind us if we ever make the utxo set normative that we need traverse it and prune these transactions first.

petertodd commented at 3:36 pm on July 19, 2013: contributor

@gmaxwell sounds like a good idea to me. The canary txout should be the first op_return in the chain of course.

petertodd commented at 3:36 pm on July 19, 2013: contributor

Oh, and don’t forget test net…

sipa commented at 11:27 am on July 20, 2013: member

How about only enabling this when a “prune_op_return” flag is set in the database, which can only be set/changed at initial creation/reindex, and outputting this flag as part of gettxoutsetinfo?

jgarzik commented at 3:14 pm on July 20, 2013: contributor

@sipa ACK. Definitely output the flag via gettxsetinfo.

petertodd commented at 12:24 pm on July 22, 2013: contributor

@sipa A flag sounds like less work, and few people would be affected, so go ahead and do it that way.

The important thing is to give users an understanding of why two different UTXO hashes don’t match. Maybe add a UTXO version number, and just increment it every time we change something? IMO it’s fine to have a meta version -1 that gets set if you run some dev code that puts the UTXO database in a bad state or have a flag called “utxo-consistent” that is unset in that case.

jgarzik commented at 2:22 pm on July 22, 2013: contributor

Either a version number or simply list of flags that might permute the output.

gmaxwell commented at 9:35 pm on August 23, 2013: contributor

Perfect is the enemy of good. I think the inconsistent values here can be resolved by anyone who wants to create UTXO tree hashes for proofs, and otherwise by reindexing. I think we should just take this as is.

jgarzik commented at 2:59 am on August 25, 2013: contributor

Let’s get this merged.

gavinandresen referenced this in commit fb8724ee6c on Sep 23, 2013

gavinandresen merged this on Sep 23, 2013

gavinandresen closed this on Sep 23, 2013

Bushstar referenced this in commit 071b60deda on Apr 5, 2019

DrahtBot locked this on Sep 8, 2021

Prune provably-unspendable outputs #2791