UTXO snapshot creation (dumptxoutset) #16899

jamesob · 2019-09-17T17:12:11Z

Parent PR: #15606
Issue: #15605
Specification: https://github.com/jamesob/assumeutxo-docs/tree/master/proposal

This changeset defines the serialization format for UTXO snapshots and adds an RPC command for creating them, dumptxoutset. It also adds a convenience script for generating and verifying snapshots at a certain height, since that requires doing a hacky rewind of the chain via invalidateblock.

All of this is unused at the moment.

src/validation.h

maflcko

.

src/node/coinstats.h

src/rpc/blockchain.cpp

src/validation.h

maflcko · 2019-09-17T17:41:47Z

What was the conclusion on the question whether the dump should include all headers?

DrahtBot · 2019-09-17T18:49:39Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#16523 (Add removemempoolentry RPC to evict transactions from the mempool by metalicjames)
#16365 (Log RPC parameters (arguments) if -debug=rpcparams by LarryRuane)

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.

jamesob

What was the conclusion on the question whether the dump should include all headers?

We need to connect to peers to make the node usable anyway, and headers take very little time to sync, so I don't see much benefit to bundling the headers with the snapshot. Though if anyone thinks differently I'd be curious to hear why.

src/validation.h

src/rpc/blockchain.cpp

maflcko · 2019-09-24T15:27:18Z

test/functional/rpc_dumptxoutset.py

+
+        # TODO I'd like to make assertions about the contents of the file
+        # (ideally its hash), but because we can't get a deterministic block
+        # header (we'd need to mock time) and we have no way of deserializing


Why can't you use mock time here?

Also, the test isn't running (see travis)

To get a deterministic blockheader, we'd need to use GetTime<...>() here: https://github.com/bitcoin/bitcoin/blob/master/src/miner.cpp#L93

Huh, that nTimeStart is only used for logging benchmarks

Ah oops, you're totally right. Will try with mock.

ryanofsky

utACK 52bba3c

src/validation.h

ryanofsky · 2019-09-24T16:12:29Z

src/rpc/blockchain.cpp

+{
+    RPCHelpMan{
+        "dumptxoutset",
+        "\nWrite the serialized UTXO set to disk.\n"


In commit "rpc: add dumptxoutset" (b6af8f5)

Up to you, but I think documentation could say more about

Data format

What you'd use this function for, or it's mostly meant to be useful for developers

Status of the implementation. If this is experimental, whether the data format might change, if there's going to be a corresponding load rpc method

src/rpc/blockchain.cpp

ryanofsky · 2019-09-24T16:33:31Z

src/rpc/blockchain.cpp

+        pcursor->Next();
+    }
+
+    afile.fclose();


In commit "rpc: add dumptxoutset" (b6af8f5)

Implementation seems fine for now. Unclear if in the future we might want to:

Put some magic bytes in snapshot header so to make it easier to recognize a snapshot file

Add version or length fields to make it possible to extend the format and include additional data

Write to temporary file and then rename in case theres a shutdown or crash in the middle of writing so you can distinguish an incomplete snapshot from a complete one

src/rpc/blockchain.cpp

Sjors · 2019-09-30T11:30:30Z

A progress indicator would be nice. If it wasn't for the system activity monitor showing CPU and disk write activity I would have interrupted the script somewhere during the ~ half an hour dumptxoutset takes.

Maybe the script should call setnetworkactive false. E.g. I'm seeing Warning: not punishing manually-connected peer, as well as a P2P timeout error.

Checksum on macOS 10.14.6 for block 597000: 3fd987a705a9b70f3197ebbe782ff457b1ea6ca373ac37b5c1a3764f7c414fe4 (produced twice, but on the same datadir)

{
  "coins_written": 62228899,
  "base_hash": "0000000000000000000eadb01d27f7b6ca1c17e71f0ff33853de9d3b25c57e55",
}

src/node/coinstats.cpp

ryanofsky

utACK c2830a9. Changes since last review: new utxo_snapshot.h file, removed hash and validation_complete snapshot metadata fields, fixed dumptxoutset base_height documentation, dumped to temporary file and renamed instead of writing directly, extended rpc_dumptxoutset test removing todo

2a4e60b Fix block index inconsistency in InvalidateBlock() (Suhas Daftuar) Pull request description: Previously, we could release `cs_main` while leaving the block index in a state that would fail `CheckBlockIndex()`, because `setBlockIndexCandidates` was not being fully populated before releasing `cs_main`. ACKs for top commit: TheBlueMatt: utACK 2a4e60b. I also discovered another issue in InvalidateBlock while reviewing, see #16856. Sjors: ACK 2a4e60b. Tested on top of #16899. Also tested `invalidateblock` with `-checkblockindex=1`. fjahr: ACK 2a4e60b. Ran tests, reviewed code, inspected behavior while manually testing `invalidateblock`. Tree-SHA512: ced12f9dfff0d413258c709921543fb154789898165590b30d1ee0cdc72863382f189744f7669a7c924d3689a1cc623efdf4e5ae3efc60054572c1e6826de612

laanwj · 2019-10-30T11:59:51Z

src/rpc/blockchain.cpp

+    Coin coin;
+
+    while (pcursor->Valid()) {
+        boost::this_thread::interruption_point();


There's no point in adding a boost interruption point here. RPC threads aren't managed by a boost thread group.
Check IsRPCRunning() instead.

Ah if that's the case then we should fix FindScriptPubKey at some point too. https://github.com/bitcoin/bitcoin/blob/master/src/rpc/blockchain.cpp#L1970-L1978

I wouldn't mind fixing it in this pull in a new commit. It seem like you copy-pasted the code from there, so it wouldn't hurt review if people saw where the code came from

95c7f5e test: add dumptxoutset RPC test (James O'Beirne) ddc90a8 devtools: add utxo_snapshot.sh (James O'Beirne) 49f281b rpc: add dumptxoutset (James O'Beirne) 644c7e7 coinstats: add coins_count (James O'Beirne) 707fde7 add unused SnapshotMetadata class (James O'Beirne) Pull request description: This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11): Parent PR: bitcoin#15606 Issue: bitcoin#15605 Specification: https://github.com/jamesob/assumeutxo-docs/tree/master/proposal --- This changeset defines the serialization format for UTXO snapshots and adds an RPC command for creating them, `dumptxoutset`. It also adds a convenience script for generating and verifying snapshots at a certain height, since that requires doing a hacky rewind of the chain via `invalidateblock`. All of this is unused at the moment.

ryanofsky

Code review ACK 95c7f5e. Changes since last review: rebase, more stable setmocktime call

jamesob · 2019-11-04T19:29:00Z

au.dumptxoutset.13 -> au.dumptxoutset.14

Removed used of boost::interruption_point on @laanwj's advice.

Also changes existing CCoinsStats attributes to be C++11 initialized.

Allows the creation of a UTXO snapshot to disk.

to allow easy (if not time-consuming) generation and verification of snapshots.

laanwj · 2019-11-05T18:39:35Z

We should remove all uses of boost::interruption_point in RPC code, but not in this PR.

ACK 92b2f53

92b2f53 test: add dumptxoutset RPC test (James O'Beirne) c1ccbc3 devtools: add utxo_snapshot.sh (James O'Beirne) 57cf74c rpc: add dumptxoutset (James O'Beirne) 92fafb3 coinstats: add coins_count (James O'Beirne) 707fde7 add unused SnapshotMetadata class (James O'Beirne) Pull request description: This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11): Parent PR: #15606 Issue: #15605 Specification: https://github.com/jamesob/assumeutxo-docs/tree/master/proposal --- This changeset defines the serialization format for UTXO snapshots and adds an RPC command for creating them, `dumptxoutset`. It also adds a convenience script for generating and verifying snapshots at a certain height, since that requires doing a hacky rewind of the chain via `invalidateblock`. All of this is unused at the moment. ACKs for top commit: laanwj: ACK 92b2f53 Tree-SHA512: 200dff87767f157d627e99506ec543465d9329860a6cd49363081619c437163a640a46d008faa92b1f44fd403bfc7a7c9e851c658b5a4849efa9a34ca976bf31

Sjors · 2019-11-05T18:48:15Z

contrib/devtools/utxo_snapshot.sh

+  echo
+  echo 'Examples:'
+  echo
+  echo "  ./contrib/devtools/utxo_snapshot.sh 570000 utxo.dat ./src/bitcoin-cli -datadir=\$(pwd)/testdata"


I'm making a snapshot at height 600000, will share a torrent link later.

maflcko · 2019-11-05T18:49:25Z

Post merge ACK. Good stuff @jamesob

Sjors · 2019-11-05T18:51:33Z

contrib/devtools/utxo_snapshot.sh

+PIVOT_BLOCKHASH=$($BITCOIN_CLI_CALL getblockhash $(( GENERATE_AT_HEIGHT + 1 )) )
+
+(>&2 echo "Rewinding chain back to height ${GENERATE_AT_HEIGHT} (by invalidating ${PIVOT_BLOCKHASH}); this may take a while")
+${BITCOIN_CLI_CALL} invalidateblock "${PIVOT_BLOCKHASH}"


For others trying this: it's best to call setnetworkactive false first

92b2f53 test: add dumptxoutset RPC test (James O'Beirne) c1ccbc3 devtools: add utxo_snapshot.sh (James O'Beirne) 57cf74c rpc: add dumptxoutset (James O'Beirne) 92fafb3 coinstats: add coins_count (James O'Beirne) 707fde7 add unused SnapshotMetadata class (James O'Beirne) Pull request description: This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11): Parent PR: bitcoin#15606 Issue: bitcoin#15605 Specification: https://github.com/jamesob/assumeutxo-docs/tree/master/proposal --- This changeset defines the serialization format for UTXO snapshots and adds an RPC command for creating them, `dumptxoutset`. It also adds a convenience script for generating and verifying snapshots at a certain height, since that requires doing a hacky rewind of the chain via `invalidateblock`. All of this is unused at the moment. ACKs for top commit: laanwj: ACK 92b2f53 Tree-SHA512: 200dff87767f157d627e99506ec543465d9329860a6cd49363081619c437163a640a46d008faa92b1f44fd403bfc7a7c9e851c658b5a4849efa9a34ca976bf31

Summary: * add unused SnapshotMetadata class * coinstats: add coins_count Also changes existing CCoinsStats attributes to be C++11 initialized. * rpc: add dumptxoutset Allows the creation of a UTXO snapshot to disk. * devtools: add utxo_snapshot.sh to allow easy (if not time-consuming) generation and verification of snapshots. * test: add dumptxoutset RPC test This is a backport of Core [[bitcoin/bitcoin#16899 | PR16899]] Test Plan: ninja all check-all Reviewers: #bitcoin_abc, jasonbcox Reviewed By: #bitcoin_abc, jasonbcox Differential Revision: https://reviews.bitcoinabc.org/D7945

92b2f53 test: add dumptxoutset RPC test (James O'Beirne) c1ccbc3 devtools: add utxo_snapshot.sh (James O'Beirne) 57cf74c rpc: add dumptxoutset (James O'Beirne) 92fafb3 coinstats: add coins_count (James O'Beirne) 707fde7 add unused SnapshotMetadata class (James O'Beirne) Pull request description: This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11): Parent PR: bitcoin#15606 Issue: bitcoin#15605 Specification: https://github.com/jamesob/assumeutxo-docs/tree/master/proposal --- This changeset defines the serialization format for UTXO snapshots and adds an RPC command for creating them, `dumptxoutset`. It also adds a convenience script for generating and verifying snapshots at a certain height, since that requires doing a hacky rewind of the chain via `invalidateblock`. All of this is unused at the moment. ACKs for top commit: laanwj: ACK 92b2f53 Tree-SHA512: 200dff87767f157d627e99506ec543465d9329860a6cd49363081619c437163a640a46d008faa92b1f44fd403bfc7a7c9e851c658b5a4849efa9a34ca976bf31

2a4e60b Fix block index inconsistency in InvalidateBlock() (Suhas Daftuar) Pull request description: Previously, we could release `cs_main` while leaving the block index in a state that would fail `CheckBlockIndex()`, because `setBlockIndexCandidates` was not being fully populated before releasing `cs_main`. ACKs for top commit: TheBlueMatt: utACK 2a4e60b. I also discovered another issue in InvalidateBlock while reviewing, see bitcoin#16856. Sjors: ACK 2a4e60b. Tested on top of bitcoin#16899. Also tested `invalidateblock` with `-checkblockindex=1`. fjahr: ACK 2a4e60b. Ran tests, reviewed code, inspected behavior while manually testing `invalidateblock`. Tree-SHA512: ced12f9dfff0d413258c709921543fb154789898165590b30d1ee0cdc72863382f189744f7669a7c924d3689a1cc623efdf4e5ae3efc60054572c1e6826de612

jamesob force-pushed the 2019-09-au-dumptxoutset branch 2 times, most recently from fd56894 to e30e3a4 Compare September 17, 2019 17:27

jamesob commented Sep 17, 2019

View reviewed changes

src/validation.h Outdated Show resolved Hide resolved

maflcko reviewed Sep 17, 2019

View reviewed changes

src/node/coinstats.h Outdated Show resolved Hide resolved

src/rpc/blockchain.cpp Outdated Show resolved Hide resolved

src/rpc/blockchain.cpp Show resolved Hide resolved

src/validation.h Outdated Show resolved Hide resolved

jamesob force-pushed the 2019-09-au-dumptxoutset branch from e30e3a4 to 0705b18 Compare September 17, 2019 17:49

DrahtBot added RPC/REST/ZMQ Scripts and tools Tests Validation labels Sep 17, 2019

fanquake removed the Tests label Sep 17, 2019

jamesob force-pushed the 2019-09-au-dumptxoutset branch 3 times, most recently from a71a387 to 6452a87 Compare September 18, 2019 17:30

jamesob commented Sep 18, 2019

View reviewed changes

jamesob force-pushed the 2019-09-au-dumptxoutset branch from 6452a87 to 52bba3c Compare September 18, 2019 18:14

ariard reviewed Sep 22, 2019

View reviewed changes

src/validation.h Outdated Show resolved Hide resolved

src/validation.h Outdated Show resolved Hide resolved

src/validation.h Outdated Show resolved Hide resolved

src/rpc/blockchain.cpp Outdated Show resolved Hide resolved

maflcko reviewed Sep 24, 2019

View reviewed changes

ryanofsky approved these changes Sep 24, 2019

View reviewed changes

jamesob force-pushed the 2019-09-au-dumptxoutset branch 3 times, most recently from 2894d3e to c2830a9 Compare September 26, 2019 16:54

This was referenced Sep 30, 2019

rpc: gettxoutsetinfo locks m_cs_chainstate #16993

Closed

Assertion `setBlockIndexCandidates.count(pindex)' failed #16444

Closed

maflcko reviewed Sep 30, 2019

View reviewed changes

src/node/coinstats.cpp Outdated Show resolved Hide resolved

Sjors mentioned this pull request Sep 30, 2019

Fix block index inconsistency in InvalidateBlock() #16849

Merged

ryanofsky approved these changes Sep 30, 2019

View reviewed changes

add unused SnapshotMetadata class

707fde7

jamesob force-pushed the 2019-09-au-dumptxoutset branch from 0efd43d to 95c7f5e Compare October 29, 2019 17:59

DrahtBot removed the Needs rebase label Oct 29, 2019

laanwj reviewed Oct 30, 2019

View reviewed changes

ryanofsky approved these changes Oct 31, 2019

View reviewed changes

jamesob force-pushed the 2019-09-au-dumptxoutset branch from 95c7f5e to 53bae65 Compare November 4, 2019 19:27

coinstats: add coins_count

92fafb3

Also changes existing CCoinsStats attributes to be C++11 initialized.

jamesob force-pushed the 2019-09-au-dumptxoutset branch from 53bae65 to 4ebc0c1 Compare November 5, 2019 15:54

jamesob added 3 commits November 5, 2019 13:35

rpc: add dumptxoutset

57cf74c

Allows the creation of a UTXO snapshot to disk.

devtools: add utxo_snapshot.sh

c1ccbc3

to allow easy (if not time-consuming) generation and verification of snapshots.

test: add dumptxoutset RPC test

92b2f53

jamesob force-pushed the 2019-09-au-dumptxoutset branch from 4ebc0c1 to 92b2f53 Compare November 5, 2019 18:36

laanwj merged commit 92b2f53 into bitcoin:master Nov 5, 2019

Sjors reviewed Nov 5, 2019

View reviewed changes

jamesob mentioned this pull request Nov 18, 2019

assumeutxo #15606

Closed

18 tasks

MarkLTZ mentioned this pull request Apr 4, 2020

Bitcoin PR tracking litecoinz-core/litecoinz#14

Closed

pierreN mentioned this pull request Apr 17, 2020

rpc: allow dumptxoutset to dump human-readable data #18689

Closed

kwvg mentioned this pull request Oct 9, 2021

merge bitcoin#15948, #15976, #16194...: assumeutxo project backports (part 1) dashpay/dash#4498

Merged

bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021

UTXO snapshot creation (dumptxoutset) #16899

UTXO snapshot creation (dumptxoutset) #16899

Uh oh!

Conversation

jamesob commented Sep 17, 2019

Uh oh!

Uh oh!

maflcko left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maflcko commented Sep 17, 2019

Uh oh!

DrahtBot commented Sep 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

jamesob left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Sjors commented Sep 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

laanwj Oct 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

jamesob commented Nov 4, 2019

Uh oh!

laanwj commented Nov 5, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maflcko commented Nov 5, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

maflcko left a comment •

edited

Loading

DrahtBot commented Sep 17, 2019 •

edited

Loading

Sjors commented Sep 30, 2019 •

edited

Loading

laanwj Oct 30, 2019 •

edited

Loading