Allow txindex in prune mode #13014

pull jonasschnelli wants to merge 4 commits into bitcoin:master from jonasschnelli:2018/04/txindex_prune changing 6 files +96 −28

jonasschnelli commented at 7:49 am on April 18, 2018: contributor
Idea is outlined here: #12651

This allows to enable the txindex when pruning is enabled. During pruning a block file, the CDiskTxPos objects in the txindex do get migrated to a “per-blockheight” approach allowing to get the blockhash and tx-position of every txid.

Once #10794 (requestblocks, or similar) is merged, getrawtransaction could wait until the block is fetched via the connected peers and return the transaction. Retrieving a transaction would still be fast enough for non-high-load usage (less then a second to a couple of seconds).

This would allow to run a txindex with ~20GB of diskspace for non high-load usage like personal block explorers.

ToDo:
- Tests
- Release-Notes
Allow txindex in prune mode eff1f2cd44
jonasschnelli added the label UTXO Db and Indexes on Apr 18, 2018
Migrate txindex data of pruned blocks to blockheight/tx-pos model ccc190e374
Return blockhash if block is pruned in GetTransaction() 5ccf1c0145
Return blockhash if getrawtransaction found tx via index in pruned block ce86c1aa6f
jonasschnelli force-pushed on Apr 18, 2018

in src/validation.h:283 in ce86c1aa6f

277@@ -278,7 +278,14 @@ void ThreadScriptCheck();
278 /** Check whether we are doing an initial block download (synchronizing from disk or network) */
279 bool IsInitialBlockDownload();
280 /** Retrieve a transaction (from memory pool, or from disk, if possible) */
281-bool GetTransaction(const uint256& hash, CTransactionRef& tx, const Consensus::Params& params, uint256& hashBlock, bool fAllowSlow = false, CBlockIndex* blockIndex = nullptr);
282+enum class GetTransactionResult
283+{
284+    LOAD_OK,

Empact commented at 12:27 pm on April 18, 2018:

OK seems sufficient

Sjors commented at 11:04 am on April 19, 2018: member

Does this work on top of #11857?
jonasschnelli commented at 11:56 am on April 19, 2018: contributor

@sjors: I originally developed this on top of 11857… it’s a trivial offset. So yes. It works on top of that.

in src/validation.cpp:1030 in ce86c1aa6f

1026+                if (postx.nFile == std::numeric_limits<int>::max()) {
1027+                    // blockfile is not available (pruned), CDiskTxPos.nPos points to the blockheight
1028+                    // lookup blockindex via height from txindex and set hashBlock
1029+                    CBlockIndex *p_index_possible = chainActive[postx.nPos];
1030+                    if (p_index_possible) {
1031+                        hashBlock = *p_index_possible->phashBlock;

promag commented at 2:29 pm on April 19, 2018:

hashBlock is only updated if block index is found. Either assert(hashBlock.IsNull()) before or just call hashBlock.SetNull().

in src/rpc/rawtransaction.cpp:185 in ce86c1aa6f

181+        } else if (res == GetTransactionResult::BLOCK_PRUNED) {
182+            //TODO: fetch block via peers, wait for download and return transaction
183+            //see https://github.com/bitcoin/bitcoin/pull/10794
184+            UniValue result(UniValue::VOBJ);
185+            result.pushKV("status", "pruned");
186+            result.pushKV("inblock", hash_block.GetHex());

promag commented at 2:29 pm on April 19, 2018:

hash_block can be null, should display it?

luke-jr commented at 8:07 pm on June 12, 2018:

Should be "blockhash"

in src/validation.cpp:3585 in ce86c1aa6f

3581+                    AbortNode(state, "Failed to read block");
3582+                    return;
3583+                }
3584+                std::vector<std::pair<uint256, CDiskTxPos>> vPos;
3585+                unsigned int txpos = 0;
3586+                for(const auto& tx : block.vtx)

promag commented at 2:31 pm on April 19, 2018:

Missing space after for. { in this line.

Empact commented at 9:09 pm on April 23, 2018:

clang-format will fix this and maybe more.

promag commented at 2:40 pm on April 19, 2018: member

Concept ACK.
TheBlueMatt commented at 11:37 pm on April 27, 2018: member

Seems there’s lots of debate on the utility here. Concept ACK/NACK discussion is at #12651, so should probably hold off review here so as to avoid splitting the discussion.
MarcoFalke added the label Needs rebase on Jun 6, 2018
Sjors commented at 1:54 pm on September 8, 2018: member
There should be tests to cover the scenario where:
1. an index exists and a block gets pruned
2. an already pruned node is launched with -txindex (this should probably fail)
The documentation should make it more clear that all transaction hashes since genesis are kept. Normally the index is about 17 GB on mainnet, does this PR make it smaller?

An alternative to this PR could be to drop old transactions from the index, though that reduces utility (perhaps better discussed in #12651).

Needs (not super difficult) rebase to work with the new txindex structure from #13033.
DrahtBot commented at 7:23 pm on December 12, 2018: member
DrahtBot added the label Up for grabs on Dec 12, 2018
DrahtBot closed this on Dec 12, 2018
laanwj removed the label Needs rebase on Oct 24, 2019
DrahtBot locked this on Dec 16, 2021

Contributors
jonasschnelli Empact Sjors promag luke-jr TheBlueMatt DrahtBot

Labels
UTXO Db and Indexes Up for grabs