Skip to content

channels_sv2: server channels wedge on SetNewPrevHash with no queued future template (group + standard) #2301

Description

@plebhash

Per the TDP spec §7.3, a Template Provider must send SetNewPrevHash immediately upon validating a new best block, and must have sent at least one future NewTemplate before it:

Upon successful validation of a new best block, the server MUST immediately provide a SetNewPrevHash message.
Prior to that, the server MUST send at least one, but potentially multiple NewTemplate messages with future_template flag set.

So a SetNewPrevHash arriving with no future template queued is a protocol violation by the peer — but two of the three server channel types respond to it by rejecting the message and never recording the new chain tip, which leaves the channel in a state strictly worse than the violation itself. The chain-tip fields (prev_hash, n_bits, header_timestamp) are self-contained and do not depend on template_id resolving to anything, so discarding them buys nothing and costs the channel its ability to recover.

1. server/group.rs

on_set_new_prev_hash unconditionally returns TemplateIdNotFound when no future jobs are queued, leaving chain_tip unset/stale (the rustdoc even documents this: "Returns an error if no matching future job is found, leaving the chain tip untouched"):

match self.job_store.has_future_jobs() {
false => {
return Err(GroupChannelError::TemplateIdNotFound);
}

2. server/standard.rs

Identical branch, identical consequence:

false => {
return Err(StandardChannelError::TemplateIdNotFound);
}

the model fix already exists in-tree

server/extended.rs handles the no-future-jobs case correctly: it demotes the previously-active job, marks past jobs stale, clears the target mappings, flushes seen shares, and still updates the chain tip, keeping TemplateIdNotFound only for the case where future jobs exist but none matches:

// extended channels dedicated to custom work don't need to keep track of future jobs
match self.job_store.has_future_jobs() {
false => {
// demote the previously-active job to past so that the subsequent
// mark_past_jobs_as_stale call moves it into the stale set. without this,
// a late share for the still-active old job would skip the stale check in
// validate_share and panic on the missing job_id_to_target entry that we
// just cleared.
self.job_store.deactivate_job();
// explicitly mark past jobs as stale, because we're not going to
// do it implicitly via activate_future_job in case this extended channel is doing custom work
self.job_store.mark_past_jobs_as_stale();
// there is no active job in this branch, so any previous job target
// mappings are obsolete after the chain tip update.
self.job_id_to_target.clear();

impact

Low severity, but the failure is unrecoverable and silent in one of its two forms:

  • Established channel (chain tip already set): the tip stays on the previous block. Every subsequent non-future NewTemplate builds jobs committed to a dead prev_hash, and validate_share keeps validating against the stale tip — the channel keeps accepting and crediting work that can never form a block.
  • Channel that never received a tip: the next valid non-future NewTemplate fails with ChainTipNotSet, wedging the job pipeline in a persistent error path.
  • In both cases TemplateIdNotFound carries no error_code, so the error documentation's recommendation of shutdown for non-protocol variants applies — an embedding pool/proxy may terminate a shared process over one peer's misbehaviour.

reachability

This requires a misbehaving or racing peer; it is not part of a conforming TDP flow. The realistic paths are:

  • a TP emitting two SetNewPrevHash in rapid succession (fast blocks, reorg) without a future NewTemplate in between;
  • an embedding application that does not backfill newly opened channels with the pending future template. Note the reference pool in sv2-apps does: handle_open_standard_mining_channel refuses to open a channel when no future template is cached (PoolErrorKind::FutureTemplateNotPresent) and otherwise feeds it last_future_template before replying — so this path is closed there, but it is not guaranteed by the library.

The point of the fix is therefore recovery from a peer that broke the rules, not conformance: a channel should not be permanently poisoned by one malformed message when the recoverable state is sitting in that same message.

fix

Mirror the extended channel's false branch in both files, minus the state each channel doesn't have:

  • standard: demote active job → mark_past_jobs_as_stale → clear job_id_to_targetflush_seen_shares → update chain_tipOk(()). The demote-then-stale ordering matters: without it a late share for the still-active old job skips the stale check in validate_share and panics on the job_id_to_target entry that was just cleared.
  • group: group channels track no shares and (post-channels_sv2: bound job-storage retention (future templates, past jobs, replaced group jobs) #2290) retain no past jobs, so the branch reduces to dropping the active job and updating chain_tip. Prefer dropping it outright (a new JobStore::clear_active_job) over routing it through deactivate_job + mark_past_jobs_as_stale — group channels never validate shares against stored jobs, so the stale set would only pin the job's retired extranonce prefix for an extra tip.

In both, TemplateIdNotFound remains for the true-branch case (future jobs queued, none matching). Update both rustdocs, which currently document the buggy behavior as intended, and state that the branch handles a non-conforming peer rather than a normal flow.

related: the in-tree TDP docs are wrong

Worth fixing separately, since it is what makes this defect look like correct behavior on inspection. sv2/subprotocols/template-distribution/src/set_new_prev_hash.rs omits the spec's mandatory precondition and contradicts itself:

/// Message used by an upstream(Template Provider) to indicate the latest block header hash
/// to mine on.
///
/// Upon validating a new best block, the upstream **must** immediately send this message.
///
/// If a [`crate::NewTemplate`] message has previously been sent with the
/// [`crate::NewTemplate::future_template`] flag set, the [`SetNewPrevHash::template_id`] field
/// **should** be set to the [`crate::NewTemplate::template_id`].

  • struct doc: "If a NewTemplate message has previously been sent with the future_template flag set, the template_id field should be set to…" — conditional, SHOULD
  • field doc: "This must be identical to previously sent NewTemplate message" — unconditional, MUST

Neither states that the TP MUST send at least one future template before every SetNewPrevHash. The conditional reading is what originally framed this finding as "a valid state transition"; it is not one.

regression tests

  • group: send SetNewPrevHash to a fresh GroupChannel with no queued future template; assert Ok and that the chain tip records the message's prev_hash/header_timestamp/n_bits, and that no job was activated. PoC available on https://github.com/project-loupe/audit-stratum/issues/128 (its expectation is correct; its assertion message states the superseded justification).
  • standard: the mirrored test, plus the two properties group doesn't have — a late share for the previously active job must be rejected as Stale (not panic on the cleared job_id_to_target), and the channel must accept the next non-future NewTemplate instead of failing ChainTipNotSet.
    `

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions