Page MenuHomePhabricator

Store the UTXO set on a per output basis rather than a per transaction basis
ClosedPublic

Authored by deadalnix on Aug 30 2017, 21:59.

Details

Summary

This is the main chunk of the work started with D342 .

It come with an update on the database format and an upgrade routine to do the migration at startup if it is required.

Test Plan
make check
../qa/pull-tester/rpc-tests.py -extended

Also run a node on testnet from a folder that has been sync before this patch to test it upgrades properly.

Diff Detail

Repository
rABC Bitcoin ABC
Branch
pertxodb
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 844
Build 844: arc lint + arc unit

Event Timeline

deadalnix retitled this revision from Store the ITXO set on a per output basis rather than a per transaction basis to Store the UTXO set on a per output basis rather than a per transaction basis.Aug 30 2017, 22:21

Fix disk space requirement estimation.

Honestly, some of this is over my head. Put in some comments, hope they are useful. Going to take a look at the other PR's now.

src/rpc/blockchain.cpp
884

Should we use sizeof rather than hard coding sizes for each item?

903

Can we use COutPoint instead of uint32 here?

1056

+1

src/test/coins_tests.cpp
677

Should there be equivalent asserts included?

src/txdb.cpp
90

NIT: preferred Committing

315

Seems the batch gets written even if the loop above aborts early. Is this intended?

schancel added a subscriber: schancel.

This looks good based on the review we did.

src/coins.cpp
48 ↗(On Diff #1299)

Is this used in testing, or should this assert that it's unimplemented?

This revision is now accepted and ready to land.Sep 6 2017, 18:01
hanchon added inline comments.
src/txdb.cpp
97

This change is breaking the blockchain test (python3 rpc-tests.py blockchain). It makes the estimate size return 0. Reverting the change make the test pass with a size of 16780.
I'll keep checking the code to see if the test is wrong or if some values need to be modified. I also need to apply D515 to see if It was solved there

src/rpc/blockchain.cpp
884

I'm good with using sizeof. Maybe a bit out of the scope of this diff.

903

This would just duplicate the outpoint many times and make hashing more expensive.

src/test/coins_tests.cpp
677

A coin is only one output, so the assert doesn't make sense anymore.

src/txdb.cpp
90

I do not care much either way. I changed it because sipa did as well.

315

The loop abort when we get at the last item, so that's expected. When there is an error, it returns so this doesn't run.

src/coins.cpp
48 ↗(On Diff #1299)

Probably, but kind out of the scope of this change.

src/txdb.cpp
97

Yes this is fixed by D516

This revision was automatically updated to reflect the committed changes.