Skip to content

Commit

Permalink
index, rpc: Coinstatsindex follow-ups
Browse files Browse the repository at this point in the history 
Summary:
> Index: Return early from failed coinstatsindex init

bitcoin/bitcoin@01386bf

----

> rpc: Add missing gettxoutsetinfo help docs

bitcoin/bitcoin@a5f6791

----

> rpc: Block until synced if coinstatsindex is used in gettxoutsetinfo
>
> During initial sync after startup the gettxoutsetinfo RPC will still return an error while catching up. However, after the initial sync the index will not error immediately anymore when it's in the process of syncing to the tip while being called. Instead it will block until synced and then return the response.

bitcoin/bitcoin@d4356d4

----

> Index: Improve logging in coinstatsindex
>
> More accurate logging of a warning should make clear if the recovery condition was hit while catching the results of the previous block.

bitcoin/bitcoin@5b3d4e7

----

> coinstats: Add comments for new coinstatsindex values

bitcoin/bitcoin@779e638

----

This concludes backport of [[bitcoin/bitcoin#22047 | core#22047]]

Some commits were already backported as dependencies of other backports in D11597 and D11605

Test Plan: `ninja all check-all`

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D13906
  • Loading branch information
@fjahr @PiRK
fjahr authored and PiRK committed May 17, 2023
1 parent c70fb42 commit f7270ea
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 108 deletions.
76 changes: 41 additions & 35 deletions src/index/coinstatsindex.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,11 +130,14 @@ bool CoinStatsIndex::WriteBlock(const CBlock &block,

BlockHash expected_block_hash{pindex->pprev->GetBlockHash()};
if (read_out.first != expected_block_hash) {
LogPrintf("WARNING: previous block header belongs to unexpected "
"block %s; expected %s\n",
read_out.first.ToString(),
expected_block_hash.ToString());

if (!m_db->Read(DBHashKey(expected_block_hash), read_out)) {
return error("%s: previous block header belongs to unexpected "
"block %s; expected %s",
__func__, read_out.first.ToString(),
expected_block_hash.ToString());
return error("%s: previous block header not found; expected %s",
__func__, expected_block_hash.ToString());
}
}

Expand Down Expand Up @@ -376,37 +379,37 @@ bool CoinStatsIndex::Init() {
}
}

if (BaseIndex::Init()) {
const CBlockIndex *pindex{CurrentIndex()};
if (!BaseIndex::Init()) {
return false;
}

if (pindex) {
DBVal entry;
if (!LookUpOne(*m_db, pindex, entry)) {
return error(
"%s: Cannot read current %s state; index may be corrupted",
__func__, GetName());
}
m_transaction_output_count = entry.transaction_output_count;
m_bogo_size = entry.bogo_size;
m_total_amount = entry.total_amount;
m_total_subsidy = entry.total_subsidy;
m_total_unspendable_amount = entry.total_unspendable_amount;
m_total_prevout_spent_amount = entry.total_prevout_spent_amount;
m_total_new_outputs_ex_coinbase_amount =
entry.total_new_outputs_ex_coinbase_amount;
m_total_coinbase_amount = entry.total_coinbase_amount;
m_total_unspendables_genesis_block =
entry.total_unspendables_genesis_block;
m_total_unspendables_bip30 = entry.total_unspendables_bip30;
m_total_unspendables_scripts = entry.total_unspendables_scripts;
m_total_unspendables_unclaimed_rewards =
entry.total_unspendables_unclaimed_rewards;
}
const CBlockIndex *pindex{CurrentIndex()};

return true;
if (pindex) {
DBVal entry;
if (!LookUpOne(*m_db, pindex, entry)) {
return error(
"%s: Cannot read current %s state; index may be corrupted",
__func__, GetName());
}
m_transaction_output_count = entry.transaction_output_count;
m_bogo_size = entry.bogo_size;
m_total_amount = entry.total_amount;
m_total_subsidy = entry.total_subsidy;
m_total_unspendable_amount = entry.total_unspendable_amount;
m_total_prevout_spent_amount = entry.total_prevout_spent_amount;
m_total_new_outputs_ex_coinbase_amount =
entry.total_new_outputs_ex_coinbase_amount;
m_total_coinbase_amount = entry.total_coinbase_amount;
m_total_unspendables_genesis_block =
entry.total_unspendables_genesis_block;
m_total_unspendables_bip30 = entry.total_unspendables_bip30;
m_total_unspendables_scripts = entry.total_unspendables_scripts;
m_total_unspendables_unclaimed_rewards =
entry.total_unspendables_unclaimed_rewards;
}

return false;
return true;
}

// Reverse a single block as part of a reorg
Expand All @@ -431,11 +434,14 @@ bool CoinStatsIndex::ReverseBlock(const CBlock &block,

BlockHash expected_block_hash{pindex->pprev->GetBlockHash()};
if (read_out.first != expected_block_hash) {
LogPrintf("WARNING: previous block header belongs to unexpected "
"block %s; expected %s\n",
read_out.first.ToString(),
expected_block_hash.ToString());

if (!m_db->Read(DBHashKey(expected_block_hash), read_out)) {
return error("%s: previous block header belongs to unexpected "
"block %s; expected %s",
__func__, read_out.first.ToString(),
expected_block_hash.ToString());
return error("%s: previous block header not found; expected %s",
__func__, expected_block_hash.ToString());
}
}
}
Expand Down
16 changes: 16 additions & 0 deletions src/node/coinstats.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,30 @@ struct CCoinsStats {
bool index_used{false};

// Following values are only available from coinstats index

//! Total cumulative amount of block subsidies up to and including this
//! block
Amount total_subsidy{Amount::zero()};
//! Total cumulative amount of unspendable coins up to and including this
//! block
Amount total_unspendable_amount{Amount::zero()};
//! Total cumulative amount of prevouts spent up to and including this block
Amount total_prevout_spent_amount{Amount::zero()};
//! Total cumulative amount of outputs created up to and including this
//! block
Amount total_new_outputs_ex_coinbase_amount{Amount::zero()};
//! Total cumulative amount of coinbase outputs up to and including this
//! block
Amount total_coinbase_amount{Amount::zero()};
//! The unspendable coinbase amount from the genesis block
Amount total_unspendables_genesis_block{Amount::zero()};
//! The two unspendable coinbase outputs total amount caused by BIP30
Amount total_unspendables_bip30{Amount::zero()};
//! Total cumulative amount of outputs sent to unspendable scripts
//! (OP_RETURN for example) up to and including this block
Amount total_unspendables_scripts{Amount::zero()};
//! Total cumulative amount of coins lost due to unclaimed miner rewards up
//! to and including this block
Amount total_unspendables_unclaimed_rewards{Amount::zero()};

CCoinsStats(CoinStatsHashType hash_type) : m_hash_type(hash_type) {}
Expand Down
47 changes: 29 additions & 18 deletions src/rpc/blockchain.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1414,15 +1414,20 @@ static RPCHelpMan gettxoutsetinfo() {
"block_info",
"Info on amounts in the block at this block height (only "
"available if coinstatsindex is used)",
{{RPCResult::Type::STR_AMOUNT, "prevout_spent", ""},
{RPCResult::Type::STR_AMOUNT, "coinbase", ""},
{RPCResult::Type::STR_AMOUNT, "new_outputs_ex_coinbase", ""},
{RPCResult::Type::STR_AMOUNT, "unspendable", ""},
{{RPCResult::Type::STR_AMOUNT, "prevout_spent",
"Total amount of all prevouts spent in this block"},
{RPCResult::Type::STR_AMOUNT, "coinbase",
"Coinbase subsidy amount of this block"},
{RPCResult::Type::STR_AMOUNT, "new_outputs_ex_coinbase",
"Total amount of new outputs created by this block"},
{RPCResult::Type::STR_AMOUNT, "unspendable",
"Total amount of unspendable outputs created in this block"},
{RPCResult::Type::OBJ,
"unspendables",
"Detailed view of the unspendable categories",
{
{RPCResult::Type::STR_AMOUNT, "genesis_block", ""},
{RPCResult::Type::STR_AMOUNT, "genesis_block",
"The unspendable amount of the Genesis block subsidy"},
{RPCResult::Type::STR_AMOUNT, "bip30",
"Transactions overridden by duplicates (no longer "
"possible with BIP30)"},
Expand Down Expand Up @@ -1490,6 +1495,25 @@ static RPCHelpMan gettxoutsetinfo() {
pindex = ParseHashOrHeight(request.params[1], chainman);
}

if (stats.index_requested && g_coin_stats_index) {
if (!g_coin_stats_index->BlockUntilSyncedToCurrentChain()) {
const IndexSummary summary{
g_coin_stats_index->GetSummary()};

// If a specific block was requested and the index has
// already synced past that height, we can return the data
// already even though the index is not fully synced yet.
if (pindex->nHeight > summary.best_block_height) {
throw JSONRPCError(
RPC_INTERNAL_ERROR,
strprintf(
"Unable to get data because coinstatsindex is "
"still syncing. Current height: %d",
summary.best_block_height));
}
}
}

if (GetUTXOStats(coins_view, *blockman, stats,
node.rpc_interruption_point, pindex)) {
ret.pushKV("height", int64_t(stats.nHeight));
Expand Down Expand Up @@ -1556,19 +1580,6 @@ static RPCHelpMan gettxoutsetinfo() {
ret.pushKV("block_info", block_info);
}
} else {
if (g_coin_stats_index) {
const IndexSummary summary{
g_coin_stats_index->GetSummary()};

if (!summary.synced) {
throw JSONRPCError(
RPC_INTERNAL_ERROR,
strprintf("Unable to read UTXO set because "
"coinstatsindex is still syncing. "
"Current height: %d",
summary.best_block_height));
}
}
throw JSONRPCError(RPC_INTERNAL_ERROR,
"Unable to read UTXO set");
}
Expand Down
57 changes: 2 additions & 55 deletions test/functional/feature_coinstatsindex.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from test_framework.messages import XEC, CTxOut, ToHex
from test_framework.script import OP_FALSE, OP_RETURN, CScript
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import assert_equal, assert_raises_rpc_error, try_rpc
from test_framework.util import assert_equal, assert_raises_rpc_error
from test_framework.wallet import MiniWallet, getnewdestination


Expand Down Expand Up @@ -64,19 +64,11 @@ def _test_coin_stats_index(self):
"Test that gettxoutsetinfo() output is consistent with or without"
" coinstatsindex option"
)
self.wait_until(
lambda: not try_rpc(-32603, "Unable to read UTXO set", node.gettxoutsetinfo)
)
res0 = node.gettxoutsetinfo("none")

# The fields 'disk_size' and 'transactions' do not exist on the index
del res0["disk_size"], res0["transactions"]

self.wait_until(
lambda: not try_rpc(
-32603, "Unable to read UTXO set", index_node.gettxoutsetinfo, "muhash"
)
)
for hash_option in index_hash_options:
res1 = index_node.gettxoutsetinfo(hash_option)
# The fields 'block_info' and 'total_unspendable_amount' only exist
Expand All @@ -95,11 +87,6 @@ def _test_coin_stats_index(self):
# Generate a new tip
self.generate(node, 5)

self.wait_until(
lambda: not try_rpc(
-32603, "Unable to read UTXO set", index_node.gettxoutsetinfo, "muhash"
)
)
for hash_option in index_hash_options:
# Fetch old stats by height
res2 = index_node.gettxoutsetinfo(hash_option, 102)
Expand Down Expand Up @@ -189,11 +176,6 @@ def _test_coin_stats_index(self):
# Include both txs in a block
self.generate(self.nodes[0], 1)

self.wait_until(
lambda: not try_rpc(
-32603, "Unable to read UTXO set", index_node.gettxoutsetinfo, "muhash"
)
)
for hash_option in index_hash_options:
# Check all amounts were registered correctly
res6 = index_node.gettxoutsetinfo(hash_option, 108)
Expand Down Expand Up @@ -229,11 +211,6 @@ def _test_coin_stats_index(self):
self.nodes[0].submitblock(ToHex(block))
self.sync_all()

self.wait_until(
lambda: not try_rpc(
-32603, "Unable to read UTXO set", index_node.gettxoutsetinfo, "muhash"
)
)
for hash_option in index_hash_options:
res7 = index_node.gettxoutsetinfo(hash_option, 109)
assert_equal(res7["total_unspendable_amount"], Decimal("80990000.00"))
Expand Down Expand Up @@ -262,11 +239,6 @@ def _test_coin_stats_index(self):
assert_equal(res8, res9)

self.generate(index_node, 1, sync_fun=self.no_op)
self.wait_until(
lambda: not try_rpc(
-32603, "Unable to read UTXO set", index_node.gettxoutsetinfo, "muhash"
)
)
res10 = index_node.gettxoutsetinfo("muhash")
assert res8["txouts"] < res10["txouts"]

Expand All @@ -290,22 +262,12 @@ def _test_reorg_index(self):
index_node = self.nodes[1]
reorg_blocks = self.generatetoaddress(index_node, 2, getnewdestination()[2])
reorg_block = reorg_blocks[1]
self.wait_until(
lambda: not try_rpc(
-32603, "Unable to read UTXO set", index_node.gettxoutsetinfo, "muhash"
)
)
res_invalid = index_node.gettxoutsetinfo("muhash")
index_node.invalidateblock(reorg_blocks[0])
assert_equal(index_node.gettxoutsetinfo("muhash")["height"], 110)

# Add two new blocks
block = self.generate(index_node, 2, sync_fun=self.no_op)[1]
self.wait_until(
lambda: not try_rpc(
-32603, "Unable to read UTXO set", index_node.gettxoutsetinfo, "muhash"
)
)
res = index_node.gettxoutsetinfo(
hash_type="muhash", hash_or_height=None, use_index=False
)
Expand All @@ -332,17 +294,7 @@ def _test_reorg_index(self):
# Ensure that removing and re-adding blocks yields consistent results
block = index_node.getblockhash(99)
index_node.invalidateblock(block)
self.wait_until(
lambda: not try_rpc(
-32603, "Unable to read UTXO set", index_node.gettxoutsetinfo, "muhash"
)
)
index_node.reconsiderblock(block)
self.wait_until(
lambda: not try_rpc(
-32603, "Unable to read UTXO set", index_node.gettxoutsetinfo, "muhash"
)
)
res3 = index_node.gettxoutsetinfo(hash_type="muhash", hash_or_height=112)
assert_equal(res2, res3)

Expand All @@ -352,14 +304,9 @@ def _test_reorg_index(self):
node.getblock(reorg_block)

self.restart_node(0, ["-coinstatsindex"])
self.wait_until(
lambda: not try_rpc(
-32603, "Unable to read UTXO set", node.gettxoutsetinfo, "muhash"
)
)
assert_raises_rpc_error(
-32603,
"Unable to read UTXO set",
"Unable to get data because coinstatsindex is still syncing.",
node.gettxoutsetinfo,
"muhash",
reorg_block,
Expand Down

0 comments on commit f7270ea

Please sign in to comment.