harden tProxy Sv1 session and channel lifecycle

[GitGab19]

This PR is built on top of #741 and:

• Tracks SV1 setup progress and enables job notifications after both mining.subscribe and mining.authorize complete.
• Requires upstream jobs to support version rolling.
• Disconnects SV1 miners when the upstream closes their channel.
• Applies extranonce prefix changes without recreating channels.
• Honors mining.extranonce.subscribe, notifying supported miners and disconnecting unsupported ones.
• Handles malformed mining notifications without panicking.
• Starts vardiff only after the downstream SV2 channel is open.
• Retains bounded, job-specific validation state so late SV1 shares use the target and extranonce assigned to their job without treating difficulty updates as clean jobs.
• Returns standard SV1 rejection errors for missing jobs, duplicate shares, and low-difficulty shares, with bounded per-downstream duplicate tracking.
• Logs unexpected upstream-channel validation failures while keeping expected target filtering at debug level.

companion stratum-mining/stratum#2319

Closes #36
Closes #139
Closes #164
Closes #386
Closes #650

[bit-aloo]

Clanker review:

Correctness issues

1. mining.set_extranonce can be silently lost during the subscribe window

Locations: downstream.rs:650–700, sv1_server/mod.rs:794

The downstream task decides whether to forward the notification using session_state.is_subscribed() at processing time. However, that flag is flipped by the server task only after the subscribe response has been built and sent.

The race looks like this:

1. The server builds the subscribe response with the old extranonce1.

2. The server sends the response.

3. The downstream task processes the queued SetExtranonce event.

   • is_subscribed == false
   • It updates extranonce1 silently.
   • It drops the cached notification.

4. The server calls record_response(Subscribe).

At this point, the miner has the old extranonce1, while every new job context is recorded with the new one. All shares from that miner then fail hash validation until it reconnects.

The window is only microseconds, but it creates a correctness hole in exactly the path this PR is trying to harden.

Suggested fix: Make the decision in the server task at enqueue time in handle_upstream_extranonce_change. This is the same task that runs record_response, so there is no race.

For example:

Sv1ServerEvent::SetExtranonce {
    message,
    notify_miner: bool,
}

When the miner is not yet subscribed, eagerly apply extranonce1 / extranonce2_len under the lock so the upcoming subscribe response carries the new value. The event then only preserves FIFO ordering: drop cached_notify and decrement the pending counter.

When the miner is already subscribed, the subscribe response was sent before the event was enqueued, so FIFO guarantees the correct wire order.

---

2. Aggregated bootstrap job rewinds the shared keepalive history

Locations: sv1_server/mod.rs:1150–1195, sv1_server/mod.rs:1880

A late joiner's bootstrap job (child channel_id) is activate()d into the shared AGGREGATED_CHANNEL_ID store with clean_jobs = false.

Suppose the active job is:

orig#3  (nTime = T0 + 3i)

The bootstrap orig replaces it as active, while orig#3 moves to past.

The next create_keepalive_job then derives from orig and emits:

orig#4  (nTime = T0 + i)

to every miner.

That's the exact slice miners already exhausted under orig#1. They re-search it, and their own accepted_share_hashes cache rejects any rediscovered shares as duplicates.

This was latent before—the old Vec::last() had the same shape—but the new code explicitly special-cases bootstrap jobs, so this seems like the right place to finish fixing it.

Suggested fix: When:

m.channel_id != AGGREGATED_CHANNEL_ID

and the store already has an active job whose original ID equals notify.job_id, don't mutate the shared store. Instead, send the late joiner the current stored active notify with clean_jobs = true.

---

3. Late aggregated joiners can hang at mining.subscribe for up to a block

Locations: channel_manager/mod.rs:1052–1070, channel_manager/mod.rs:1252

The deferral itself is correct, and the doc comment is honest about the cost: "next block plus one job."

The problem is that SV1 miners typically time out a pending subscribe after ~30–60 seconds and reconnect.

During a prefix transition, this can produce a loop:

subscribe
  → deferred
  → miner times out
  → reconnects
  → subscribe
  → deferred
  → ...

When the compatible job finally arrives, N child channels may be opened and immediately closed through:

DownstreamNotPresent → CloseChannel

This isn't a leak, but it creates unnecessary churn, and the miner sees a connection failure rather than simply a delayed setup.

Possible no-wait alternative

The required building blocks already exist:

1. Allocate the child prefix.
2. Call set_upstream_prefix(old_bytes) on it.
3. Create the ExtendedChannel.
4. Replay the aggregated channel's active/future jobs, each with its captured prefix, switching the child's upstream region before each replay.
5. Call set_upstream_extranonce_prefix(new).

The subscribe response then advertises the old variant, and forward_job_to_sv1_server already emits mining.set_extranonce on the first new-prefix job.

Tradeoff: A miner that doesn't support set_extranonce gets one disconnect later instead of waiting now.

Either approach seems reasonable, but it's worth making the choice explicit. If the deferral stays, I'd at least add a bounded timeout that rejects the request with an error so the miner fails fast.

---

4. A supporting miner can be disconnected mid-handshake

Location: downstream.rs:686

The disconnect currently fires when:

is_subscribed && !supports_set_extranonce

However, mining.extranonce.subscribe commonly arrives after mining.subscribe, and for some firmware it can even arrive after authorize.

A prefix change landing in that window therefore disconnects a miner that would have supported the change once setup completed.

Suggested fix: While Starting, cache the set_extranonce and re-evaluate it at SetupComplete. At that point, either send it after the setup responses or disconnect if the miner still doesn't support it.

Ergonomics / simplification

5. Sv1ServerEvent should carry typed messages, not JSON

Locations: downstream.rs:40, downstream.rs:579–650

The server currently:

1. Builds a typed Notify.
2. Converts it to json_rpc::Message.
3. Sends it through Sv1ServerEvent.
4. Every downstream task re-parses it with Notify::try_from(notification.clone()).
5. The downstream also string-matches notification.method.

This happens once per miner per job.

An event enum like:

Notify(Arc<Notify>)
SetDifficulty(Message)
SetExtranonce(SetExtranonce)
SetupComplete

would move JSON conversion to the socket-write boundary.

It would also remove:

• InvalidMiningNotifyNotification
• InvalidSetExtranonceNotification
• the "cached mining.notify was not a notification" branch
• two of the new unit tests that only cover unparseable internal messages

Also, Sv1ServerEvent::notification(m) is just a one-line alias for:

Sv1ServerEvent::Notification(m)

so I'd drop it.

---

6. Duplicated extranonce-size arithmetic in handle_set_extranonce_prefix

Locations: mining_message_handler.rs:529, mining_message_handler.rs:623

Both branches manually compute:

upstream + local_prefix_and_index + rollable <= MAX_EXTRANONCE_LEN

before calling:

allocator.set_upstream_prefix(...)
channel.set_upstream_extranonce_prefix(...)

Those methods already perform the same validation transactionally—I verified this in the pinned channels_sv2.

Roughly 60 lines could therefore collapse into mapping ExceedsMaxLength into the appropriate fallback.

The only thing lost is the numeric:

InvalidExtranonceSize {
    prefix_len,
    rollable_size,
}

payload, which can be reconstructed from the channel at the error site if it's still useful.

---

7. Collision-ID derivation is hard to read

Location: mining_message_handler.rs:88–115

The current code first checks a collision predicate, then uses another nested if to recover which ID actually collided.

A helper like:

fn colliding_channel_id(&self, m) -> Option<ChannelId>

could return the offending ID directly.

That would read more clearly and avoid having the predicate and ID-recovery logic drift apart.

---

8. handle_set_target_without_vardiff only disconnects the first failed sender

Location: sv1_server/mod.rs:1629

Finishing the broadcast is the right fix, but if two miners are dead, the second one isn't noticed until the next SetTarget.

Since this already runs in the server task, calling:

handle_downstream_disconnect(...)

inline for each failed sender seems simpler than threading a single ID through Action::Disconnect.

---

9. Sv1AcceptedShareCache::insert_if_new is O(4096) per share

Location: downstream.rs:160

VecDeque::contains scans up to 4096 32-byte hashes on every submit.

This is probably fine at current rates, but if this ever sits in front of many miners, a HashSet + VecDeque pair would give O(1) membership checks while preserving bounded eviction order.

Nits / housekeeping

• utils.rs:36–62 — The # Arguments / # Returns documentation for validate_sv1_share now attaches to Sv1ShareValidationOutcome, because the enum was inserted between the comment and the function. Move the enum above the doc block.

• bitcoin-core-sv2/Cargo.toml / stratum-apps/Cargo.toml — These point at a personal fork branch. They should be reverted once the Stratum PR lands, following the same pattern as the codec branch.

• bitcoin_core_ipc_jdp_io.rs — This is purely a doc-comment reflow and appears unrelated to this change.

• #[allow(clippy::result_large_err)] on disconnect_downstream_for_upstream_close — This suggests TproxyError has grown fairly large. Boxing the error kind is out of scope here, but probably worth a follow-up.