Skip to content

Address bitcoind RPC review: gettxout/getrawtransaction fixes, compatibility nits, and refactors #822

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dsbaars wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Address bitcoind RPC review: gettxout/getrawtransaction fixes, compatibility nits, and refactors #822

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e9adddc
Address bitcoind rpc review questions (gettxout, getrawtransaction).
dsbaars Jul 9, 2026
a450368
Fix bitcoind rpc nits (chainwork, confirmations, filtertype).
dsbaars Jul 9, 2026
9029652
Address bitcoind rpc refactor TODOs (verbosity enum, utility, config).
dsbaars Jul 9, 2026
dffd9ee
Reference upstream issues in blocked bitcoind rpc TODOs.
dsbaars Jul 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions include/bitcoin/server/protocols/protocol_bitcoind_rpc.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ class BCS_API protocol_bitcoind_rpc
double maxfeerate) NOEXCEPT;

/// Json context helpers (shared with rest, defined in *_json.cpp).
static double verification_progress(size_t blocks, size_t headers) NOEXCEPT;
static uint32_t median_time_past(const node::query& query,
const database::header_link& link) NOEXCEPT;
static void inject_block_context(boost::json::object& out,
Expand Down
115 changes: 66 additions & 49 deletions src/protocols/bitcoind/protocol_bitcoind_rpc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,16 @@ using namespace network::messages;
using namespace std::placeholders;
using namespace boost::json;

// bitcoind getblock verbosity levels (doc/JSON-RPC-interface.md).
namespace {
enum class block_verbosity : size_t
{
hex = 0, // serialized block, hex-encoded

@evoskuil evoskuil Jul 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style, generally avoid trailing comments.

hashed = 1, // block object listing txids
verbose = 2 // block object embedding full tx objects
};
} // namespace

BC_PUSH_WARNING(NO_THROW_IN_NOEXCEPT)
BC_PUSH_WARNING(SMART_PTR_NOT_NEEDED)
BC_PUSH_WARNING(NO_VALUE_OR_CONST_REF_SHARED_PTR)
Expand Down Expand Up @@ -197,7 +207,8 @@ bool protocol_bitcoind_rpc::handle_get_block(const code& ec,
}

size_t level{};
if (!to_integer(level, verbosity))
if (!to_integer(level, verbosity) ||
level > static_cast<size_t>(block_verbosity::verbose))
{
send_error(error::invalid_argument);
return true;
Expand All @@ -206,41 +217,27 @@ bool protocol_bitcoind_rpc::handle_get_block(const code& ec,
constexpr auto witness = true;
const auto& query = archive();
const auto link = query.to_header(hash);

if (level == zero)
const auto block = query.get_block(link, witness);
if (!block)
{
const auto block = query.get_block(link, witness);
if (!block)
{
send_error(error::not_found, blockhash, blockhash.size());
return true;
}

send_text(to_text(*block, block->serialized_size(witness), witness));
send_error(error::not_found, blockhash, blockhash.size());
return true;
}

if (level == one || level == two)
const auto detail = static_cast<block_verbosity>(level);

@evoskuil evoskuil Jul 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Converting to integral and then casting for each usage isn't making for the most readable code. I would recommend providing to/from methods for the enum that provide an invalid value upon float conversion, internally handles invalid float, and then exclusively use the enum values.

if (detail == block_verbosity::hex)
{
const auto block = query.get_block(link, witness);
if (!block)
{
send_error(error::not_found, blockhash, blockhash.size());
return true;
}

// TODO: map "level/verbosity" to enumeration and remove comments.
// verbosity 1 lists txids; verbosity 2 embeds full tx objects.
auto model = is_one(level) ?
value_from(bitcoind_hashed(*block)) :
value_from(bitcoind_verbose(*block));

inject_block_context(model.as_object(), query, link, block->header());
send_result(std::move(model), two * block->serialized_size(witness));
send_text(to_text(*block, block->serialized_size(witness), witness));
return true;
}

send_error(error::invalid_argument);
// hashed lists txids; verbose embeds full tx objects.
auto model = detail == block_verbosity::hashed ?
value_from(bitcoind_hashed(*block)) :
value_from(bitcoind_verbose(*block));

inject_block_context(model.as_object(), query, link, block->header());
send_result(std::move(model), two * block->serialized_size(witness));
return true;
}

Expand All @@ -261,15 +258,16 @@ bool protocol_bitcoind_rpc::handle_get_block_chain_info(const code& ec,
return true;
}

// TODO: make utility method and move explanation there.
// verificationprogress is approximated as confirmed/candidate height, the
// best available estimate of the chain tip during sync (1.0 once current).
const auto progress = is_zero(headers) ? 1.0 :
std::min(1.0, static_cast<double>(blocks) /
static_cast<double>(headers));
const auto progress = verification_progress(blocks, headers);

// TODO: blocks/headers is a misnomer (off-by-one), intended?
// blocks/headers are heights (not counts) per bitcoind convention: blocks is
// the confirmed tip height, headers the candidate (best-header) height.
using namespace chain;

// Cumulative work to the tip, big-endian per bitcoind chainwork.
uint256_t work{};
query.get_work(work, link);

send_result(object_t
{
{ "chain", chain_name(query) },
Expand All @@ -283,6 +281,7 @@ bool protocol_bitcoind_rpc::handle_get_block_chain_info(const code& ec,
{ "mediantime", median_time_past(query, link) },
{ "verificationprogress", progress },
{ "initialblockdownload", !is_current_chain(true) },
{ "chainwork", encode_hash(from_uintx(work)) },
{ "pruned", false },
{ "warnings", std::string{} }
}, 512);
Expand All @@ -302,11 +301,18 @@ bool protocol_bitcoind_rpc::handle_get_block_count(const code& ec,

bool protocol_bitcoind_rpc::handle_get_block_filter(const code& ec,
rpc_interface::get_block_filter, const std::string& blockhash,
const std::string&) NOEXCEPT
const std::string& filtertype) NOEXCEPT
{
if (stopped(ec))
return false;

// bitcoind defines only the "basic" (neutrino) filter type.
if (filtertype != "basic")
{
send_error(error::invalid_argument);
return true;
}

hash_digest hash{};
if (!decode_hash(hash, blockhash))
{
Expand Down Expand Up @@ -442,9 +448,10 @@ bool protocol_bitcoind_rpc::handle_get_tx_out(const code& ec,
const auto& query = archive();
const auto output_link = query.to_output(hash, index);

// TODO: is this meant to be query.is_confirmed_spent(output_link)?
// bitcoind returns json null for missing or spent output (mempool ignored).
if (output_link.is_terminal() || query.is_spent(output_link))
// bitcoind returns json null for a missing or confirmed-spent output; with
// mempool ignored this matches gettxout's include_mempool=false semantics
// (is_spent would also count unconfirmed/conflicting/invalid-block spenders).
if (output_link.is_terminal() || query.is_confirmed_spent(output_link))
{
send_result({}, 42);
return true;
Expand Down Expand Up @@ -541,16 +548,16 @@ bool protocol_bitcoind_rpc::handle_get_network_info(const code& ec,
if (stopped(ec))
return false;

// TODO: get most of these values from either config or network/node props.

// libbitcoin-server is a node, not a wallet/peer-introspection service;
// peer-dependent fields (connections, addresses) are reported as empty.
// Protocol/relay values come from network configuration. libbitcoin-server
// is a node, not a wallet/peer-introspection service, so peer-dependent
// fields (connections, addresses) and fees are reported as empty/defaults.
const auto& network = network_settings();
send_result(object_t
{
{ "version", 0 },
{ "subversion", std::string{ "/libbitcoin:server/" } },
{ "protocolversion", 70016 },
{ "localrelay", true },
{ "subversion", network.user_agent },
{ "protocolversion", network.protocol_maximum },
{ "localrelay", network.enable_relay },
{ "timeoffset", 0 },
{ "connections", 0 },
{ "networkactive", true },
Expand Down Expand Up @@ -591,8 +598,16 @@ bool protocol_bitcoind_rpc::handle_get_raw_transaction(const code& ec,
return true;
}

// TODO: can verbose be validated, to_integer()?
if (verbose == 0.0)
// bitcoind parses verbose as an integer (ParseVerbosity): level zero yields
// hex, nonzero yields the json object (verbosity 2 fee/prevout not yet done).
size_t level{};
if (!to_integer(level, verbose))
{
send_error(error::invalid_argument);
return true;
}

if (level == zero)
{
send_text(to_text(*tx, tx->serialized_size(witness), witness));
return true;
Expand Down Expand Up @@ -628,7 +643,8 @@ bool protocol_bitcoind_rpc::handle_send_raw_transaction(const code& ec,
return true;
}

// Tx archive not allowed in in v4, must move through node::tx_chaser (v5).
// Tx archive not allowed in v4, must move through node::tx_chaser (v5).
// See libbitcoin-node#1075.
////auto& query = archive();
////const auto hash = tx->hash(false);
////
Expand Down Expand Up @@ -756,6 +772,7 @@ http::request_cptr protocol_bitcoind_rpc::reset_rpc_request() NOEXCEPT
// utility (redundant with protocol_electrum)
// ----------------------------------------------------------------------------
// TODO: move this to node utility and pass through.
// See libbitcoin-node#1075.

bool protocol_bitcoind_rpc::get_pool_context(chain::context& pool) const NOEXCEPT
{
Expand Down
23 changes: 22 additions & 1 deletion src/protocols/bitcoind/protocol_bitcoind_rpc_json.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/
#include <bitcoin/server/protocols/protocol_bitcoind_rpc.hpp>

#include <algorithm>
#include <bitcoin/server/define.hpp>

namespace libbitcoin {
Expand All @@ -32,6 +33,16 @@ uint32_t protocol_bitcoind_rpc::median_time_past(const node::query& query,
return query.get_context(ctx, link) ? ctx.median_time_past : 0_u32;
}

// verificationprogress is approximated as confirmed/candidate height, the best
// available estimate of the chain tip during sync (1.0 once current).
double protocol_bitcoind_rpc::verification_progress(size_t blocks,
size_t headers) NOEXCEPT
{
return is_zero(headers) ? 1.0 :
std::min(1.0, static_cast<double>(blocks) /

@evoskuil evoskuil Jul 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use to_floating(...)

static_cast<double>(headers));

@evoskuil evoskuil Jul 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to cast both values.

}

void protocol_bitcoind_rpc::inject_block_context(boost::json::object& out,
const node::query& query, const database::header_link& link,
const chain::header& header) NOEXCEPT
Expand All @@ -43,9 +54,17 @@ void protocol_bitcoind_rpc::inject_block_context(boost::json::object& out,
const auto top = query.get_top_confirmed();
const auto confirmed = query.is_confirmed_block(link);
out["height"] = height;
out["confirmations"] = add1(floored_subtract(top, height));

// bitcoind reports -1 confirmations for a block not on the active chain.
out["confirmations"] = confirmed ?
static_cast<int64_t>(add1(floored_subtract(top, height))) : -1;

@evoskuil evoskuil Jul 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use to_signed(...)

out["mediantime"] = median_time_past(query, link);

// Cumulative work to this block, big-endian per bitcoind chainwork.
uint256_t work{};
if (query.get_work(work, link))
out["chainwork"] = encode_hash(from_uintx(work));

if (header.previous_block_hash() != null_hash)
out["previousblockhash"] = encode_hash(header.previous_block_hash());

Expand Down Expand Up @@ -89,6 +108,7 @@ boost::json::object protocol_bitcoind_rpc::header_to_bitcoind(
{ "time", header.timestamp() },
{ "nonce", header.nonce() },
{ "bits", encode_base16(to_big_endian(header.bits())) },
{ "target", encode_hash(from_uintx(chain::compact::expand(header.bits()))) },
{ "difficulty", header.difficulty() }
};
}
Expand All @@ -98,6 +118,7 @@ std::string protocol_bitcoind_rpc::chain_name(const node::query& query) NOEXCEPT
const auto genesis = query.get_header_key(query.to_confirmed(zero));

// TODO: create signet chain selector.
// See libbitcoin-system#1908.
using selection = chain::selection;
constexpr auto signet = base16_hash(
"00000008819873e925422c1ff0f99f7cc9bbb232af63a077a480a3633bee1ef6");
Expand Down
Loading