Repository navigation
rpc: match Core header, address, and mempool JSON - #864
Conversation
There was a problem hiding this comment.
Review findings:
-
The new inactive-header paths set
mediantimeto the block's own timestamp in bothoff_chain_header_valueanddisconnected_block_value. Core reports the median time past over the preceding 11 headers on that branch, so this diverges for inactive chains with more than one header. Please calculate branch MTP from the stored parent links (or omit the field if it cannot be computed). -
off_chain_headerperforms a parent-header lookup, an active-chain hash lookup, and then another parent-header lookup on each iteration. A long stored side branch can therefore turn a singlegetblockheader/getblockrequest into hundreds of thousands of random store lookups (the walk permits 100,000 steps). Please avoid the duplicate parent read and use a bounded/indexed ancestry walk so stale branches cannot make RPC requests scan the whole branch.
A header that has left the active chain was missing from getblockheader, and getblock could not rebuild its Class A body. Genesis also emitted an empty previousblockhash. The same response builders now include nextblockhash and target, tip bits, 16-significant-digit difficulty, P2TR/P2A validateaddress, getmempoolinfo fullrbf and cluster caps, and 8-decimal BTC amounts. getblock verbosity 1 size, strippedsize, and weight are already on master.
getblockheader and getblock reported an inactive block's own timestamp as mediantime. Core uses the median of that header and up to 10 ancestors. The ancestry walk also read each parent twice and would follow a stale branch for 100,000 steps. Read each parent once, stop at the active-chain height index, and cap the walk at the header-sync limit.
7d67ee8 to
2466703
Compare
The health gauge printed the raw difficulty float. getblockchaininfo rounds to 16 significant digits, and the cross-surface pin compares them as f64. Regtest genesis differs by one ULP unless both sides use that rounding.
Summary
getblockheaderreturns a stored header that is no longer on the active chain (confirmations: -1, its ownpreviousblockhash). Genesis omitspreviousblockhash. Active headers includenextblockhashandtarget. Fixes rpc: getblockheader returns -5 "Block not found" for a header that is no longer on the active chain, though getchaintips lists it #853.getblockrebuilds a Class A body for that same hash, and still serves a held never-confirmed body.getblockchaininfoaddsbitsandtarget. Difficulty uses 16 significant digits.validateaddresssetsisscriptfor P2TR and P2A and omits witness fields on pay-to-anchor.getmempoolinfoaddsfullrbf(true),maxdatacarriersize(null), and the live cluster limits. BTC amounts print as 8-decimal numbers, includingscantxoutsettotal_amount. Fixes rpc: output differences from Bitcoin Core in getblockheader, getblock, getblockchaininfo, getdifficulty, validateaddress and getmempoolinfo #854.getblockverbosity 1size/strippedsize/weightis already on master (rpc: getblock verbosity 1 carries size, strippedsize, and weight #847). This pull request does not redo it.Test plan
cargo test -p rbitcoin-rpc --lib -- --exact methods::chain::core_double_tests::sixteen_significant_digitscargo test -p rbitcoin-rpc --lib -- --exact methods::tests::getblock_named_verbose_genesis_and_hexcargo test -p rbitcoin-rpc --lib -- --exact methods::tests::getmempoolinfo_permitbaremultisig_is_always_truecargo test -p rbitcoin-rpc --lib -- --exact methods::tests::decode_rpc_subsetcargo test -p rbitcoin-rpc --lib -- --exact methods::tests::rpc_regtest_chain_opscargo clippy -p rbitcoin-rpc -p rbitcoin-net -p rbitcoin-mempool --all-targets -- -D warnings