rpc: report height and chain work for a held block

changelog.d/rpc-held-block-height.md

@@ -0,0 +1,9 @@
+Fixed
+
+- **`getblockheader` and `getblock` on a held side block report its height
+  and chain work, as Core does.** Both left out `height`. `getblockheader`
+  reported only that block's own work as `chainwork`, and so did
+  `getblock` when the parent was not on the active chain. Both still need
+  the block's ancestors to be held or stored. Once a held ancestor is
+  evicted from the held-body cache, `height` is left out and `chainwork`
+  is that block's own work, as before.

crates/rbitcoin-rpc/src/methods/chain.rs

@@ -560,6 +560,7 @@ fn held_header_value(ctx: &RpcContext, block: &Block, verbose: bool) -> Result<V
     let version = block.header.version.to_consensus();
     let prev = block.header.prev_blockhash.to_byte_array();
     let mediantime = median_time_from_wire(ctx, block.header.time, prev)?;
+    let (height, chainwork) = held_height_and_work_hex(ctx, block);
     let mut obj = json!({
         "hash": hash_hex_display(&block.block_hash().to_byte_array()),
         "confirmations": -1,
@@ -571,10 +572,13 @@ fn held_header_value(ctx: &RpcContext, block: &Block, verbose: bool) -> Result<V
         "nonce": block.header.nonce,
         "bits": format!("{:08x}", bits),
         "difficulty": core_f64_json(difficulty_from_bits(bits)),
-        "chainwork": rbitcoin_primitives::hex_encode(block.header.work().to_be_bytes() ),
+        "chainwork": chainwork,
         "nTx": block.txdata.len(),
         "target": target_hex(bits),
     });
+    if let Some(h) = height {
+        obj["height"] = json!(h);
+    }
     if prev != [0u8; 32] {
         insert_previousblockhash(&mut obj, &hash_hex_display(&prev));
     }
@@ -648,16 +652,42 @@ fn enrich_block_header_json(
     }
 }
 
-fn held_chainwork_hex(ctx: &RpcContext, block: &Block) -> String {
-    let prev = block.header.prev_blockhash.to_byte_array();
-    if let Ok(Some(h)) = ctx.query.height_of_hash(&prev) {
-        if let Some(hub) = ctx.chain.as_ref() {
-            if let Ok(w) = hub.work_through_height(h.0) {
-                return rbitcoin_primitives::hex_encode((w + block.header.work()).to_be_bytes());
-            }
-        }
+/// Height and chain work of a held body: its parent's plus this block. The
+/// parent is on the active chain, a stored header, or another held body.
+fn held_height_and_work(ctx: &RpcContext, block: &Block) -> Option<(u32, bitcoin::Work)> {
+    let hub = ctx.chain.as_ref()?;
+    let mut works = vec![block.header.work()];
+    let mut prev = block.header.prev_blockhash;
+    for depth in 1..=OFF_CHAIN_WALK_MAX {
+        let p = prev.to_byte_array();
+        let (height, base) = if let Ok(Some(h)) = ctx.query.height_of_hash(&p) {
+            (h.0, hub.work_through_height(h.0).ok()?)
+        } else if let Some(loc) = off_chain_header(ctx, &p).ok()? {
+            let base: [u8; 32] = rbitcoin_primitives::hex_decode(&loc.chainwork)
+                .ok()?
+                .try_into()
+                .ok()?;
+            (loc.height, bitcoin::Work::from_be_bytes(base))
+        } else {
+            let parent = hub.held_body(&prev)?;
+            works.push(parent.header.work());
+            prev = parent.header.prev_blockhash;
+            continue;
+        };
+        let work = rbitcoin_net::sum_work(std::iter::once(base).chain(works)).ok()?;
+        return Some((height.checked_add(depth)?, work));
     }
-    rbitcoin_primitives::hex_encode(block.header.work().to_be_bytes())
+    None
+}
+
+/// [`held_height_and_work`] for JSON. Without a known ancestry, only this
+/// block's work and no height.
+fn held_height_and_work_hex(ctx: &RpcContext, block: &Block) -> (Option<u32>, String) {
+    let (height, work) = match held_height_and_work(ctx, block) {
+        Some((h, w)) => (Some(h), w),
+        None => (None, block.header.work()),
+    };
+    (height, rbitcoin_primitives::hex_encode(work.to_be_bytes()))
 }
 
 fn insert_block_size_fields(obj: &mut Value, block: &Block) {
@@ -825,10 +855,10 @@ fn disconnected_block_value(
     let version = block.header.version.to_consensus();
     let prev = block.header.prev_blockhash.to_byte_array();
     let loc = off_chain_header(ctx, hash)?;
-    let chainwork = loc
-        .as_ref()
-        .map(|l| l.chainwork.clone())
-        .unwrap_or_else(|| held_chainwork_hex(ctx, block));
+    let (height, chainwork) = match &loc {
+        Some(l) => (Some(l.height), l.chainwork.clone()),
+        None => held_height_and_work_hex(ctx, block),
+    };
     let mediantime = match &loc {
         Some(loc) => loc.mediantime,
         None => median_time_from_wire(ctx, block.header.time, prev)?,
@@ -845,8 +875,8 @@ fn disconnected_block_value(
         "nTx": block.txdata.len(),
         "tx": txids,
     });
-    if let Some(loc) = loc {
-        obj["height"] = json!(loc.height);
+    if let Some(h) = height {
+        obj["height"] = json!(h);
     }
     if prev != [0u8; 32] {
         insert_previousblockhash(&mut obj, &hash_hex_display(&prev));

crates/rbitcoin-rpc/src/regtest_chain_ops_journey.rs

@@ -582,6 +582,7 @@ fn rpc_regtest_mature_chain_ops() {
     chain_ops_maxfeerate(&ctx, &mut cbs);
     chain_ops_invalidate_and_precious(&ctx, &hub, &mut cbs, &p2wpkh);
     chain_ops_reconsider_most_work(&ctx, &hub, &p2wpkh);
+    chain_ops_held_height_and_work(&ctx, &hub, &p2wpkh);
 
     // Two networks cannot be one chain: the same hub behind a mainnet RPC view.
     ctx.network = Network::Mainnet;
@@ -2703,6 +2704,56 @@ fn chain_ops_reconsider_most_work(
     assert_eq!(best(), h[3]);
 }
 
+/// A held body reports the height and chain work Core gives it, whether its
+/// parent is on the active chain, held, or a stored side header. Each one
+/// here ties the tip, so all are held.
+fn chain_ops_held_height_and_work(
+    ctx: &RpcContext,
+    hub: &rbitcoin_net::ChainHub,
+    p2wpkh: &ScriptBuf,
+) {
+    let tip = best_hash(ctx);
+    let tip_h = tip_count(ctx) as u32;
+    let at = |h: u32| hub.query.header_at_height(Height(h)).unwrap().unwrap().1;
+    let mine = |prev: [u8; 32], time: u32, height: u32| {
+        let prev = BlockHash::from_byte_array(prev);
+        rbitcoin_consensus::mine_regtest_paying(prev, time, height, p2wpkh.clone(), vec![])
+    };
+    let submit = |b: &Block| dispatch(ctx, "submitblock", vec![json!(block_hex(b))]).unwrap();
+    let (parent, base) = (at(tip_h - 1), at(tip_h - 2));
+    let on_active = mine(parent.hash, parent.timestamp + 1000, tip_h);
+    let held_parent = mine(base.hash, base.timestamp + 1001, tip_h - 1);
+    let on_held = mine(held_parent.block_hash().to_byte_array(), base.timestamp + 1002, tip_h);
+    let header_parent = mine(base.hash, base.timestamp + 1003, tip_h - 1);
+    let on_header = mine(header_parent.block_hash().to_byte_array(), base.timestamp + 1004, tip_h);
+    for b in [&on_active, &held_parent, &on_held] {
+        assert_eq!(submit(b), "inconclusive");
+    }
+    let header = rbitcoin_primitives::hex_encode(bitcoin::consensus::encode::serialize(&header_parent.header));
+    dispatch(ctx, "submitheader", vec![json!(header)]).unwrap();
+    assert_eq!(submit(&on_header), "inconclusive");
+    assert_eq!(best_hash(ctx), tip);
+    let header_of = |h: BlockHash| {
+        dispatch(ctx, "getblockheader", vec![json!(h.to_string())]).unwrap()
+    };
+    let at_tip = dispatch(ctx, "getblockheader", vec![tip]).unwrap();
+    let below_tip = header_of(BlockHash::from_byte_array(parent.hash));
+    for (b, want) in [
+        (&on_active, &at_tip),
+        (&held_parent, &below_tip),
+        (&on_held, &at_tip),
+        (&on_header, &at_tip),
+    ] {
+        let hash = json!(b.block_hash().to_string());
+        let block = dispatch(ctx, "getblock", vec![hash, json!(1)]).unwrap();
+        for got in [&header_of(b.block_hash()), &block] {
+            assert_eq!(got["confirmations"], -1, "{got}");
+            assert_eq!(got["height"], want["height"], "{got}");
+            assert_eq!(got["chainwork"], want["chainwork"], "{got}");
+        }
+    }
+}
+
 /// Last beat, because it corrupts the store: a connected spend whose parent
 /// edge in `input.body` is gone is a broken promise, not a txid lookup.
 fn chain_ops_corrupt_input_edge(ctx: &RpcContext, store: &std::path::Path, height: u32) {