Skip to content

tighten up bitcoin_core_sv2 JDP ValidationContext #617

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

Draft
plebhash wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

tighten up bitcoin_core_sv2 JDP ValidationContext #617

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
015efce
bitcoin_core_sv2 + jd-server: stale-tip by prev_hash and simplify val…
plebhash Jul 15, 2026
5458158
bitcoin_core_sv2: replace createNewBlock with getTip in force_update_…
plebhash Jul 15, 2026
277b9e2
bitcoin_core_sv2: map bad-cb-height checkBlock rejection to stale-cha…
plebhash Jul 15, 2026
ba5418d
integration-tests: rewrite JDP stale scenario to use missing-txs retry
plebhash Jul 15, 2026
1253474
integration-tests: add JDP IO coverage for stale-at-arrival declare jobs
plebhash Jul 15, 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
5 changes: 2 additions & 3 deletions bitcoin-core-sv2/src/runtime_api/job_declaration_protocol/io.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,15 @@ use tokio::sync::oneshot;

/// Snapshot of the template parameters used by the validator at decision time.
///
/// This lets callers distinguish stale-tip races from other validation failures.
/// Stale-chain-tip detection relies solely on [`prev_hash`](Self::prev_hash)
/// drift; nbits is used for validation of SetCustomMiningJob.
///
/// Please check <https://github.com/stratum-mining/sv2-apps/issues/364>
/// for more details on the regression that motivated this field.
#[derive(Debug, Clone, Copy)]
pub struct ValidationContext {
pub prev_hash: BlockHash,
pub nbits: CompactTarget,
pub min_ntime: u32,
}

/// A request sent from `jd-server` to the [`BitcoinCoreSv2JDP`](super::BitcoinCoreSv2JDP) IPC
Expand Down Expand Up @@ -45,7 +45,6 @@ pub enum JdResponse {
Success {
prev_hash: BlockHash,
nbits: CompactTarget,
min_ntime: u32,
/// Txids for all transactions (excluding coinbase), in the same order as the declared
/// wtxid_list. Enables the caller to build the txid merkle tree for validating
/// SetCustomMiningJob.merkle_path.
Expand Down
104 changes: 35 additions & 69 deletions bitcoin-core-sv2/src/unix_capnp/v30x/job_declaration_protocol/handlers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@

use crate::{
runtime_api::job_declaration_protocol::io::{JdResponse, ValidationContext},
unix_capnp::{
v30x::job_declaration_protocol::BitcoinCoreSv2JDP,
v31x_v30x::job_declaration_protocol::mempool::decode_bip34_height_from_coinbase_script_sig,
},
unix_capnp::v30x::job_declaration_protocol::BitcoinCoreSv2JDP,
};
use stratum_core::{
bitcoin::{
Expand Down Expand Up @@ -49,19 +46,7 @@ impl BitcoinCoreSv2JDP {
coinbase_tx.input[0].script_sig
);

let declared_bip34_height = coinbase_tx
.input
.first()
.and_then(|input| {
decode_bip34_height_from_coinbase_script_sig(input.script_sig.as_bytes())
})
// Some templates/coinbase formats do not expose BIP34 height in canonical
// scriptSig push form (e.g. opcode-encoded small integers in tests/regtest).
// Fall back to coinbase lock_time to avoid panics and keep a stable
// stale-tip comparison signal.
.unwrap_or_else(|| coinbase_tx.lock_time.to_consensus_u32());

let (initial_validation_context, initial_bip34_height, txdata) = {
let (initial_validation_context, ntime, txdata) = {
let mut mempool_mirror = self.mempool_mirror.borrow_mut();

// Add the missing transactions to the mempool mirror
Expand All @@ -73,19 +58,11 @@ impl BitcoinCoreSv2JDP {
let nbits = mempool_mirror
.get_current_nbits()
.expect("current_nbits must be set");
let min_ntime = mempool_mirror
.get_current_min_ntime()
.expect("current_min_ntime must be set");

let initial_validation_context = ValidationContext {
prev_hash,
nbits,
min_ntime,
};
let ntime = mempool_mirror
.get_current_ntime()
.expect("current_ntime must be set");

let initial_bip34_height = mempool_mirror
.get_current_bip34_height()
.expect("current_bip34_height must be set");
let initial_validation_context = ValidationContext { prev_hash, nbits };

// Now verify that all wtxids from the declared job are available
let missing_wtxids = mempool_mirror.verify(&wtxid_list);
Expand All @@ -102,28 +79,27 @@ impl BitcoinCoreSv2JDP {
let txdata = mempool_mirror.get_txdata(&wtxid_list);

info!(
"Using prevhash: {:?}, nbits: {:?}, min_ntime: {}, bip34_height: {} from mempool mirror",
initial_validation_context.prev_hash,
initial_validation_context.nbits,
initial_validation_context.min_ntime,
initial_bip34_height
"Using prevhash: {:?}, nbits: {:?}, ntime: {} from mempool mirror",
initial_validation_context.prev_hash, initial_validation_context.nbits, ntime,
);

(initial_validation_context, initial_bip34_height, txdata)
(initial_validation_context, ntime, txdata)
}; // mempool_mirror dropped here, we don't want to hold it across await points

let txid_list: Vec<Txid> = txdata.iter().map(|tx| tx.compute_txid()).collect();

let mut check_block_reason_for_stale: Option<String> = None;

let valid_job = {
let mut all_transactions = Vec::with_capacity(1 + txdata.len());
all_transactions.push(coinbase_tx.clone());
all_transactions.extend(txdata);

let num_transactions = all_transactions.len();

// Use the min_ntime from the template as the block timestamp
// Use the template ntime as the block timestamp
// This ensures we meet Bitcoin Core's timestamp validation rules
let block_time = initial_validation_context.min_ntime;
let block_time = ntime;

let header = Header {
version,
Expand Down Expand Up @@ -212,6 +188,8 @@ impl BitcoinCoreSv2JDP {
debug = ?check_block_debug,
"Bitcoin Core rejected the block via checkBlock"
);
check_block_reason_for_stale =
check_block_reason.ok().and_then(|r| r.to_string().ok());
debug!(
"Block details - version: {:?}, prev_blockhash: {:?}, bits: {:?}, num_txs: {}",
version,
Expand All @@ -233,66 +211,54 @@ impl BitcoinCoreSv2JDP {
};

if !valid_job {
// On checkBlock failure, force-refresh template + mirror before classifying the error.
// On checkBlock failure, update local prev_hash before classifying the error.
// The template monitor updates mempool_mirror asynchronously, so we need to avoid races
// where validation can run on context A while chain tip has already moved to context B.
// Refreshing here narrows this TOCTOU window and lets us correctly emit
// Updating prev_hash here narrows this TOCTOU window and lets us correctly emit
// `stale-chain-tip` instead of generic `invalid-job` when context drift occurred.
if let Err(e) = self.force_update_mempool_mirror().await {
//
// Additionally, a `bad-cb-height` rejection from checkBlock signals a job that was
// already stale at arrival (coinbase built from an old tip) and is mapped to
// stale-chain-tip even without prev_hash drift.
if let Err(e) = self.force_update_mempool_mirror_prev_hash().await {
debug!(
error = ?e,
"Failed to force-refresh template/mempool mirror after checkBlock failure; continuing with current validation context"
"Failed to update prev_hash after checkBlock failure; continuing with current validation context"
);
}
}

let (latest_validation_context, latest_bip34_height) = {
let latest_validation_context = {
let mempool_mirror = self.mempool_mirror.borrow();
let latest_validation_context = ValidationContext {
ValidationContext {
prev_hash: mempool_mirror
.get_current_prev_hash()
.expect("current_prev_hash must be set"),
nbits: mempool_mirror
.get_current_nbits()
.expect("current_nbits must be set"),
min_ntime: mempool_mirror
.get_current_min_ntime()
.expect("current_min_ntime must be set"),
};
let latest_bip34_height = mempool_mirror
.get_current_bip34_height()
.expect("current_bip34_height must be set");
(latest_validation_context, latest_bip34_height)
}
};

let response = if valid_job {
JdResponse::Success {
prev_hash: initial_validation_context.prev_hash,
nbits: initial_validation_context.nbits,
min_ntime: initial_validation_context.min_ntime,
txid_list,
}
} else {
let stale_at_arrival_by_bip34 = declared_bip34_height != latest_bip34_height;
let context_drifted = initial_validation_context.prev_hash
!= latest_validation_context.prev_hash
|| initial_validation_context.nbits != latest_validation_context.nbits
|| initial_validation_context.min_ntime != latest_validation_context.min_ntime
|| initial_bip34_height != latest_bip34_height
|| stale_at_arrival_by_bip34;

let error_code = if context_drifted {
let context_drifted =
initial_validation_context.prev_hash != latest_validation_context.prev_hash;

let stale_at_arrival = matches!(
check_block_reason_for_stale.as_deref(),
Some("bad-cb-height")
);

let error_code = if context_drifted || stale_at_arrival {
debug!(
initial_prev_hash = ?initial_validation_context.prev_hash,
initial_nbits = ?initial_validation_context.nbits,
initial_min_ntime = initial_validation_context.min_ntime,
initial_bip34_height,
declared_bip34_height,
latest_prev_hash = ?latest_validation_context.prev_hash,
latest_nbits = ?latest_validation_context.nbits,
latest_min_ntime = latest_validation_context.min_ntime,
latest_bip34_height,
stale_at_arrival_by_bip34,
"Detected stale chain tip during DeclareMiningJob validation; classifying error as stale-chain-tip"
);
ERROR_CODE_DECLARE_MINING_JOB_STALE_CHAIN_TIP
Expand Down
84 changes: 51 additions & 33 deletions bitcoin-core-sv2/src/unix_capnp/v30x/job_declaration_protocol/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use bitcoin_capnp_types::{
};
use bitcoin_capnp_types_v30 as bitcoin_capnp_types;
use std::{cell::RefCell, path::Path, rc::Rc};
use stratum_core::bitcoin::{Block, consensus::deserialize};
use stratum_core::bitcoin::{Block, BlockHash, consensus::deserialize, hashes::Hash};
use tokio::net::UnixStream;
use tokio_util::compat::*;
pub use tokio_util::sync::CancellationToken;
Expand Down Expand Up @@ -268,60 +268,78 @@ impl BitcoinCoreSv2JDP {
Ok(())
}

/// Forces a synchronous template refresh from Bitcoin Core, then refreshes the mempool mirror.
/// Checks whether the chain tip has changed via `getTip` and, if so, updates the mempool
/// mirror's prev_hash.
///
/// This is useful after `checkBlock` failures to reduce classification races where the async
/// `waitNext` monitor has not yet advanced `current_template_ipc_client`.
///
/// It differs from update_mempool_mirror in the sense that it doesn't assume a new template is
/// available. It forces the template refresh before updating MempoolMirror.
/// This is called after `checkBlock` failures to reduce stale-tip classification races.
/// We avoid `createNewBlock` (which is expensive) because only the prev_hash is needed for
/// stale detection.
///
/// On transient `"thread busy"` IPC contention, this method retries a few times with
/// a short backoff before returning the error.
pub(crate) async fn force_update_mempool_mirror(&self) -> Result<(), BitcoinCoreSv2JDPError> {
pub(crate) async fn force_update_mempool_mirror_prev_hash(
&self,
) -> Result<(), BitcoinCoreSv2JDPError> {
const MAX_ATTEMPTS: usize = 3;
const RETRY_BACKOFF_MS: u64 = 25;

let mut last_error: Option<BitcoinCoreSv2JDPError> = None;

for attempt in 1..=MAX_ATTEMPTS {
let result = async {
let mut create_new_block_request =
self.mining_ipc_client.create_new_block_request();
let result: Result<(), BitcoinCoreSv2JDPError> = async {
let mut get_tip_request = self.mining_ipc_client.get_tip_request();

let mut create_new_block_options =
create_new_block_request.get().get_options().map_err(|e| {
error!("Failed to get createNewBlock options: {e}");
get_tip_request
.get()
.get_context()
.map_err(|e| {
error!("Failed to get getTip request context: {e}");
e
})?;

create_new_block_options.set_use_mempool(true);
})?
.set_thread(self.thread_ipc_client.clone());

let create_new_block_response =
create_new_block_request.send().promise.await.map_err(|e| {
error!("Failed to send createNewBlock request: {e}");
e
})?;
let get_tip_response = get_tip_request.send().promise.await.map_err(|e| {
error!("Failed to send getTip request: {e}");
e
})?;

let new_template_ipc_client = create_new_block_response
let tip_hash_bytes = get_tip_response
.get()
.map_err(|e| {
error!("Failed to read createNewBlock response: {e}");
error!("Failed to read getTip response: {e}");
e
})?
.get_result()
.map_err(|e| {
error!("Failed to get BlockTemplate from createNewBlock: {e}");
error!("Failed to get tip result from getTip: {e}");
e
})?
.get_hash()
.map_err(|e| {
error!("Failed to get tip hash from getTip result: {e}");
e
})?;

let tip_hash = BlockHash::from_slice(tip_hash_bytes).map_err(|e| {
error!("Failed to parse tip hash from getTip: {e}");
BitcoinCoreSv2JDPError::CapnpError(capnp::Error::failed(format!(
"Failed to parse tip hash: {e}"
)))
})?;

{
let mut current_template_ipc_client =
self.current_template_ipc_client.borrow_mut();
*current_template_ipc_client = new_template_ipc_client;
let mut mempool_mirror = self.mempool_mirror.borrow_mut();
if mempool_mirror.get_current_prev_hash() != Some(tip_hash) {
debug!(
old_prev_hash = ?mempool_mirror.get_current_prev_hash(),
new_prev_hash = ?tip_hash,
"Chain tip changed; updating mempool mirror prev_hash"
);
mempool_mirror.set_current_prev_hash(tip_hash);
}
}

self.update_mempool_mirror().await
Ok(())
}
.await;

Expand All @@ -332,7 +350,7 @@ impl BitcoinCoreSv2JDP {
error = ?e,
attempt,
max_attempts = MAX_ATTEMPTS,
"Transient IPC contention during force_update_mempool_mirror (thread busy); retrying"
"Transient IPC contention during force_update_mempool_mirror_prev_hash (thread busy); retrying"
);
last_error = Some(e);
tokio::time::sleep(std::time::Duration::from_millis(RETRY_BACKOFF_MS)).await;
Expand All @@ -341,11 +359,11 @@ impl BitcoinCoreSv2JDP {
}
}

// ideally the retry logic should never allow execution to reach here
// but if it does, we just bubble up the error
// Normally one of the retry attempts returns early (Ok or Err).
// If execution reaches here, no terminal error was captured, so synthesize one.
Err(last_error.unwrap_or_else(|| {
BitcoinCoreSv2JDPError::CapnpError(capnp::Error::failed(
"force_update_mempool_mirror exhausted retries without a terminal error"
"force_update_mempool_mirror_prev_hash exhausted retries without a terminal error"
.to_string(),
))
}))
Expand Down
Loading
Loading