mirror of
https://github.com/bitcoin/bitcoin.git
synced 2024-11-20 10:38:42 +01:00
Merge bitcoin/bitcoin#29720: rpc: Avoid getchaintxstats invalid results
2342b46c45
test: Add coverage for getchaintxstats in assumeutxo context (Fabian Jahr)faf2a6750b
rpc: Reorder getchaintxstats output (MarcoFalke)fa2dada0c9
rpc: Avoid getchaintxstats invalid results (MarcoFalke) Pull request description: The `getchaintxstats` RPC reply during AU background download may return non-zero, but invalid, values for `window_tx_count` and `txrate`. For example, `txcount` may be zero for a to-be-downloaded block, but may be non-zero for an ancestor block which is already downloaded. Thus, the values returned may be negative (and cause intermediate integer sanitizer violations). Also, `txcount` may be accurate for the snapshot base block, or a descendant of it. However it may be zero for an ancestor block that still needs to be downloaded. Thus, the values returned may be positive, but wrong. Fix all issues by skipping the returned value if either `txcount` is unset (equal to zero). Also, skip `txcount` in the returned value, if it is unset (equal to zero). Fixes https://github.com/bitcoin/bitcoin/issues/29328 ACKs for top commit: fjahr: re-ACK2342b46c45
achow101: ACK2342b46c45
mzumsande: ACK2342b46c45
Tree-SHA512: 931cecc40ee5dc0f96be728db7eb297155f8343076cd29c8b8c050c99fd1d568b80f54c9459a34ca7a9489c2474c729796d00eeb1934d6a9f7b4d6a53e3ec430
This commit is contained in:
commit
173ab0ccf2
@ -1643,13 +1643,19 @@ static RPCHelpMan getchaintxstats()
|
||||
RPCResult::Type::OBJ, "", "",
|
||||
{
|
||||
{RPCResult::Type::NUM_TIME, "time", "The timestamp for the final block in the window, expressed in " + UNIX_EPOCH_TIME},
|
||||
{RPCResult::Type::NUM, "txcount", "The total number of transactions in the chain up to that point"},
|
||||
{RPCResult::Type::NUM, "txcount", /*optional=*/true,
|
||||
"The total number of transactions in the chain up to that point, if known. "
|
||||
"It may be unknown when using assumeutxo."},
|
||||
{RPCResult::Type::STR_HEX, "window_final_block_hash", "The hash of the final block in the window"},
|
||||
{RPCResult::Type::NUM, "window_final_block_height", "The height of the final block in the window."},
|
||||
{RPCResult::Type::NUM, "window_block_count", "Size of the window in number of blocks"},
|
||||
{RPCResult::Type::NUM, "window_tx_count", /*optional=*/true, "The number of transactions in the window. Only returned if \"window_block_count\" is > 0"},
|
||||
{RPCResult::Type::NUM, "window_interval", /*optional=*/true, "The elapsed time in the window in seconds. Only returned if \"window_block_count\" is > 0"},
|
||||
{RPCResult::Type::NUM, "txrate", /*optional=*/true, "The average rate of transactions per second in the window. Only returned if \"window_interval\" is > 0"},
|
||||
{RPCResult::Type::NUM, "window_tx_count", /*optional=*/true,
|
||||
"The number of transactions in the window. "
|
||||
"Only returned if \"window_block_count\" is > 0 and if txcount exists for the start and end of the window."},
|
||||
{RPCResult::Type::NUM, "txrate", /*optional=*/true,
|
||||
"The average rate of transactions per second in the window. "
|
||||
"Only returned if \"window_interval\" is > 0 and if window_tx_count exists."},
|
||||
}},
|
||||
RPCExamples{
|
||||
HelpExampleCli("getchaintxstats", "")
|
||||
@ -1690,19 +1696,25 @@ static RPCHelpMan getchaintxstats()
|
||||
|
||||
const CBlockIndex& past_block{*CHECK_NONFATAL(pindex->GetAncestor(pindex->nHeight - blockcount))};
|
||||
const int64_t nTimeDiff{pindex->GetMedianTimePast() - past_block.GetMedianTimePast()};
|
||||
const int nTxDiff = pindex->nChainTx - past_block.nChainTx;
|
||||
const auto window_tx_count{
|
||||
(pindex->nChainTx != 0 && past_block.nChainTx != 0) ? std::optional{pindex->nChainTx - past_block.nChainTx} : std::nullopt,
|
||||
};
|
||||
|
||||
UniValue ret(UniValue::VOBJ);
|
||||
ret.pushKV("time", (int64_t)pindex->nTime);
|
||||
ret.pushKV("txcount", (int64_t)pindex->nChainTx);
|
||||
if (pindex->nChainTx) {
|
||||
ret.pushKV("txcount", pindex->nChainTx);
|
||||
}
|
||||
ret.pushKV("window_final_block_hash", pindex->GetBlockHash().GetHex());
|
||||
ret.pushKV("window_final_block_height", pindex->nHeight);
|
||||
ret.pushKV("window_block_count", blockcount);
|
||||
if (blockcount > 0) {
|
||||
ret.pushKV("window_tx_count", nTxDiff);
|
||||
ret.pushKV("window_interval", nTimeDiff);
|
||||
if (window_tx_count) {
|
||||
ret.pushKV("window_tx_count", *window_tx_count);
|
||||
if (nTimeDiff > 0) {
|
||||
ret.pushKV("txrate", ((double)nTxDiff) / nTimeDiff);
|
||||
ret.pushKV("txrate", double(*window_tx_count) / nTimeDiff);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -33,6 +33,7 @@ from dataclasses import dataclass
|
||||
from test_framework.messages import tx_from_hex
|
||||
from test_framework.test_framework import BitcoinTestFramework
|
||||
from test_framework.util import (
|
||||
assert_approx,
|
||||
assert_equal,
|
||||
assert_raises_rpc_error,
|
||||
)
|
||||
@ -315,21 +316,35 @@ class AssumeutxoTest(BitcoinTestFramework):
|
||||
the snapshot, and final values after the snapshot is validated."""
|
||||
for height, block in blocks.items():
|
||||
tx = n1.getblockheader(block.hash)["nTx"]
|
||||
chain_tx = n1.getchaintxstats(nblocks=1, blockhash=block.hash)["txcount"]
|
||||
stats = n1.getchaintxstats(nblocks=1, blockhash=block.hash)
|
||||
chain_tx = stats.get("txcount", None)
|
||||
window_tx_count = stats.get("window_tx_count", None)
|
||||
tx_rate = stats.get("txrate", None)
|
||||
window_interval = stats.get("window_interval")
|
||||
|
||||
# Intermediate nTx of the starting block should be set, but nTx of
|
||||
# later blocks should be 0 before they are downloaded.
|
||||
# The window_tx_count of one block is equal to the blocks tx count.
|
||||
# If the window tx count is unknown, the value is missing.
|
||||
# The tx_rate is calculated from window_tx_count and window_interval
|
||||
# when possible.
|
||||
if final or height == START_HEIGHT:
|
||||
assert_equal(tx, block.tx)
|
||||
assert_equal(window_tx_count, tx)
|
||||
if window_interval > 0:
|
||||
assert_approx(tx_rate, window_tx_count / window_interval, vspan=0.1)
|
||||
else:
|
||||
assert_equal(tx_rate, None)
|
||||
else:
|
||||
assert_equal(tx, 0)
|
||||
assert_equal(window_tx_count, None)
|
||||
|
||||
# Intermediate nChainTx of the starting block and snapshot block
|
||||
# should be set, but others should be 0 until they are downloaded.
|
||||
# should be set, but others should be None until they are downloaded.
|
||||
if final or height in (START_HEIGHT, SNAPSHOT_BASE_HEIGHT):
|
||||
assert_equal(chain_tx, block.chain_tx)
|
||||
else:
|
||||
assert_equal(chain_tx, 0)
|
||||
assert_equal(chain_tx, None)
|
||||
|
||||
check_tx_counts(final=False)
|
||||
|
||||
|
@ -51,7 +51,6 @@ unsigned-integer-overflow:CCoinsViewCache::Uncache
|
||||
unsigned-integer-overflow:CompressAmount
|
||||
unsigned-integer-overflow:DecompressAmount
|
||||
unsigned-integer-overflow:crypto/
|
||||
unsigned-integer-overflow:getchaintxstats*
|
||||
unsigned-integer-overflow:MurmurHash3
|
||||
unsigned-integer-overflow:CBlockPolicyEstimator::processBlockTx
|
||||
unsigned-integer-overflow:TxConfirmStats::EstimateMedianVal
|
||||
@ -63,7 +62,6 @@ implicit-integer-sign-change:CBlockPolicyEstimator::processBlockTx
|
||||
implicit-integer-sign-change:SetStdinEcho
|
||||
implicit-integer-sign-change:compressor.h
|
||||
implicit-integer-sign-change:crypto/
|
||||
implicit-integer-sign-change:getchaintxstats*
|
||||
implicit-integer-sign-change:TxConfirmStats::removeTx
|
||||
implicit-integer-sign-change:prevector.h
|
||||
implicit-integer-sign-change:verify_flags
|
||||
|
Loading…
Reference in New Issue
Block a user