Skip to content

Commit

Permalink
Merge bitcoin/bitcoin#28590: assumeutxo: change getchainstates RPC to…
Browse files Browse the repository at this point in the history 
… return a list of chainstates

a9ef702 assumeutxo: change getchainstates RPC to return a list of chainstates (Ryan Ofsky)

Pull request description:

  Current `getchainstates` RPC returns "normal" and "snapshot" fields which are not ideal because it requires new "normal" and "snapshot" terms to be defined, and the definitions are not really consistent with internal code. (In the RPC interface, the "snapshot" chainstate becomes the "normal" chainstate after it is validated, while in internal code there is no "normal chainstate" and the "snapshot chainstate" is still called that temporarily after it is validated).

  The current `getchainstates` RPC is also awkward to use if you to want information about the most-work chainstate, because you have to look at the "snapshot" field if it exists, and otherwise fall back to the "normal" field.

  Fix these issues by having `getchainstates` just return a flat list of chainstates ordered by work, and adding a new chainstate "validated" field alongside the existing "snapshot_blockhash" field so it is explicit if a chainstate was originally loaded from a snapshot, and whether the snapshot has been validated.

  This change was motivated by comment thread in bitcoin/bitcoin#28562 (comment)

ACKs for top commit:
  Sjors:
    re-ACK a9ef702
  jamesob:
    re-ACK a9ef702
  achow101:
    ACK a9ef702

Tree-SHA512: b364e2e96675fb7beaaee60c4dff4b69e6bc2d8a30dea1ba094265633d1cddf9dbf1c5ce20c07d6e23222cf1e92a195acf6227e4901f3962e81a1e53a43490aa
  • Loading branch information
@achow101
achow101 committed Oct 5, 2023
2 parents 6e5cf8e + a9ef702 commit 0b2c93b
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 37 deletions.
22 changes: 10 additions & 12 deletions src/rpc/blockchain.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2807,6 +2807,7 @@ const std::vector<RPCResult> RPCHelpForChainstate{
{RPCResult::Type::STR_HEX, "snapshot_blockhash", /*optional=*/true, "the base block of the snapshot this chainstate is based on, if any"},
{RPCResult::Type::NUM, "coins_db_cache_bytes", "size of the coinsdb cache"},
{RPCResult::Type::NUM, "coins_tip_cache_bytes", "size of the coinstip cache"},
{RPCResult::Type::BOOL, "validated", "whether the chainstate is fully validated. True if all blocks in the chainstate were validated, false if the chain is based on a snapshot and the snapshot has not yet been validated."},
};

static RPCHelpMan getchainstates()
Expand All @@ -2818,8 +2819,7 @@ return RPCHelpMan{
RPCResult{
RPCResult::Type::OBJ, "", "", {
{RPCResult::Type::NUM, "headers", "the number of headers seen so far"},
{RPCResult::Type::OBJ, "normal", /*optional=*/true, "fully validated chainstate containing blocks this node has validated starting from the genesis block", RPCHelpForChainstate},
{RPCResult::Type::OBJ, "snapshot", /*optional=*/true, "only present if an assumeutxo snapshot is loaded. Partially validated chainstate containing blocks this node has validated starting from the snapshot. After the snapshot is validated (when the 'normal' chainstate advances far enough to validate it), this chainstate will replace and become the 'normal' chainstate.", RPCHelpForChainstate},
{RPCResult::Type::ARR, "chainstates", "list of the chainstates ordered by work, with the most-work (active) chainstate last", {{RPCResult::Type::OBJ, "", "", RPCHelpForChainstate},}},
}
},
RPCExamples{
Expand All @@ -2834,7 +2834,7 @@ return RPCHelpMan{
NodeContext& node = EnsureAnyNodeContext(request.context);
ChainstateManager& chainman = *node.chainman;

auto make_chain_data = [&](const Chainstate& cs) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
auto make_chain_data = [&](const Chainstate& cs, bool validated) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
AssertLockHeld(::cs_main);
UniValue data(UniValue::VOBJ);
if (!cs.m_chain.Tip()) {
Expand All @@ -2852,20 +2852,18 @@ return RPCHelpMan{
if (cs.m_from_snapshot_blockhash) {
data.pushKV("snapshot_blockhash", cs.m_from_snapshot_blockhash->ToString());
}
data.pushKV("validated", validated);
return data;
};

if (chainman.GetAll().size() > 1) {
for (Chainstate* chainstate : chainman.GetAll()) {
obj.pushKV(
chainstate->m_from_snapshot_blockhash ? "snapshot" : "normal",
make_chain_data(*chainstate));
}
} else {
obj.pushKV("normal", make_chain_data(chainman.ActiveChainstate()));
}
obj.pushKV("headers", chainman.m_best_header ? chainman.m_best_header->nHeight : -1);

const auto& chainstates = chainman.GetAll();
UniValue obj_chainstates{UniValue::VARR};
for (Chainstate* cs : chainstates) {
obj_chainstates.push_back(make_chain_data(*cs, !cs->m_from_snapshot_blockhash || chainstates.size() == 1));
}
obj.pushKV("chainstates", std::move(obj_chainstates));
return obj;
}
};
Expand Down
47 changes: 22 additions & 25 deletions test/functional/feature_assumeutxo.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,13 @@ def no_sync():
assert_equal(loaded['coins_loaded'], SNAPSHOT_BASE_HEIGHT)
assert_equal(loaded['base_height'], SNAPSHOT_BASE_HEIGHT)

monitor = n1.getchainstates()
assert_equal(monitor['normal']['blocks'], START_HEIGHT)
assert_equal(monitor['snapshot']['blocks'], SNAPSHOT_BASE_HEIGHT)
assert_equal(monitor['snapshot']['snapshot_blockhash'], dump_output['base_hash'])
normal, snapshot = n1.getchainstates()["chainstates"]
assert_equal(normal['blocks'], START_HEIGHT)
assert_equal(normal.get('snapshot_blockhash'), None)
assert_equal(normal['validated'], True)
assert_equal(snapshot['blocks'], SNAPSHOT_BASE_HEIGHT)
assert_equal(snapshot['snapshot_blockhash'], dump_output['base_hash'])
assert_equal(snapshot['validated'], False)

assert_equal(n1.getblockchaininfo()["blocks"], SNAPSHOT_BASE_HEIGHT)

Expand Down Expand Up @@ -159,20 +162,11 @@ def no_sync():
self.connect_nodes(0, 1)

self.log.info(f"Ensuring snapshot chain syncs to tip. ({FINAL_HEIGHT})")

def check_for_final_height():
chainstates = n1.getchainstates()
# The background validation may have completed before we run our first
# check, so accept a final blockheight from either chainstate type.
cs = chainstates.get('snapshot') or chainstates.get('normal')
return cs['blocks'] == FINAL_HEIGHT

wait_until_helper(check_for_final_height)
wait_until_helper(lambda: n1.getchainstates()['chainstates'][-1]['blocks'] == FINAL_HEIGHT)
self.sync_blocks(nodes=(n0, n1))

self.log.info("Ensuring background validation completes")
# N.B.: the `snapshot` key disappears once the background validation is complete.
wait_until_helper(lambda: not n1.getchainstates().get('snapshot'))
wait_until_helper(lambda: len(n1.getchainstates()['chainstates']) == 1)

# Ensure indexes have synced.
completed_idx_state = {
Expand All @@ -189,8 +183,8 @@ def check_for_final_height():

assert_equal(n.getblockchaininfo()["blocks"], FINAL_HEIGHT)

assert_equal(n.getchainstates()['normal']['blocks'], FINAL_HEIGHT)
assert_equal(n.getchainstates().get('snapshot'), None)
chainstate, = n.getchainstates()['chainstates']
assert_equal(chainstate['blocks'], FINAL_HEIGHT)

if i != 0:
# Ensure indexes have synced for the assumeutxo node
Expand All @@ -208,17 +202,20 @@ def check_for_final_height():
assert_equal(loaded['coins_loaded'], SNAPSHOT_BASE_HEIGHT)
assert_equal(loaded['base_height'], SNAPSHOT_BASE_HEIGHT)

monitor = n2.getchainstates()
assert_equal(monitor['normal']['blocks'], START_HEIGHT)
assert_equal(monitor['snapshot']['blocks'], SNAPSHOT_BASE_HEIGHT)
assert_equal(monitor['snapshot']['snapshot_blockhash'], dump_output['base_hash'])
normal, snapshot = n2.getchainstates()['chainstates']
assert_equal(normal['blocks'], START_HEIGHT)
assert_equal(normal.get('snapshot_blockhash'), None)
assert_equal(normal['validated'], True)
assert_equal(snapshot['blocks'], SNAPSHOT_BASE_HEIGHT)
assert_equal(snapshot['snapshot_blockhash'], dump_output['base_hash'])
assert_equal(snapshot['validated'], False)

self.connect_nodes(0, 2)
wait_until_helper(lambda: n2.getchainstates()['snapshot']['blocks'] == FINAL_HEIGHT)
wait_until_helper(lambda: n2.getchainstates()['chainstates'][-1]['blocks'] == FINAL_HEIGHT)
self.sync_blocks()

self.log.info("Ensuring background validation completes")
wait_until_helper(lambda: not n2.getchainstates().get('snapshot'))
wait_until_helper(lambda: len(n2.getchainstates()['chainstates']) == 1)

completed_idx_state = {
'basic block filter index': COMPLETE_IDX,
Expand All @@ -234,8 +231,8 @@ def check_for_final_height():

assert_equal(n.getblockchaininfo()["blocks"], FINAL_HEIGHT)

assert_equal(n.getchainstates()['normal']['blocks'], FINAL_HEIGHT)
assert_equal(n.getchainstates().get('snapshot'), None)
chainstate, = n.getchainstates()['chainstates']
assert_equal(chainstate['blocks'], FINAL_HEIGHT)

if i != 0:
# Ensure indexes have synced for the assumeutxo node
Expand Down

0 comments on commit 0b2c93b

Please sign in to comment.