Fix tProxy issues found by Loupe by GitGab19 · Pull Request #741 · stratum-mining/sv2-apps

GitGab19 · 2026-08-19T15:43:29Z

This PR:

Clarify the intentional vardiff target-ordering policy.
Make SV1 handshake completion idempotent.
Continue difficulty broadcasts after an individual downstream disconnects.
Bound SV1 handshake messages buffered before channel establishment.
Document that payout verification defaults to disabled and that this trusts the upstream payout policy.
Reject upstream channel and group identifiers that collide in non-aggregated mode.
Generate and retain one shared aggregated keepalive job per tick, then deliver it to every eligible downstream.

Closes project-loupe/audit-sv2-apps#26
Closes #743
Closes project-loupe/audit-sv2-apps#126
Closes project-loupe/audit-sv2-apps#127
Closes #744
Closes project-loupe/audit-sv2-apps#130
Closes #745
Closes project-loupe/audit-sv2-apps#187
Closes project-loupe/audit-sv2-apps#185
Closes #811
Closes project-loupe/audit-sv2-apps#188
Closes #812
Closes project-loupe/audit-sv2-apps#195
Closes #813

bit-aloo

I need to think around the reconnect design.

GitGab19 · 2026-08-27T10:12:00Z

Just pushed three more commits to cover the new #811, #812, and #813 found by Loupe.

plebhash · 2026-09-08T01:02:08Z

d713bdc seems to be going in a different direction than what the scope of #811 proposes

no hard opinions on which direction is best, just pointing out the contradiction in case its not intentional

plebhash · 2026-09-08T01:15:56Z

commits have titles but no message bodies

as described on this link listed on CONTRIBUTING.md, we should start enforcing this on all commits, so that we always have in-tree documented rationales for all code changes

(highly recommended to have this on AGENTS.md/CLAUDE.md)

GitGab19 · 2026-09-08T07:59:04Z

d713bdc seems to be going in a different direction than what the scope of #811 proposes

no hard opinions on which direction is best, just pointing out the contradiction in case its not intentional

Yeah it's intentional, but thanks for pointing it out.

commits have titles but no message bodies

Completely agree, I'll add bodies soon.

GitGab19 · 2026-09-09T19:57:58Z

I just modified the keepalive jobs commit: 3dafaf3a.

Keepalive jobs now go only to miners whose own interval has elapsed, reusing the shared job when no new mutation is due. Late joiners also receive the latest shared job without resetting its history or rewinding nTime.

bit-aloo

Few questions and suggestions..

Document that a new vardiff target takes effect with the next mining.notify rather than immediately. This preserves the difficulty advertised with each job so late shares are not checked against a newer target.

The authorize-triggered notification flush must run only once. Keep Pending, Completing, and Complete with the notification cache under one lock so newer work cannot overtake the flush or remain stranded when normal forwarding is enabled. Flush cached mining notifications in difficulty-before-notify order; these are not repeated handshake responses. Preserve the existing difficulty and clean-jobs policy.

A failed send to one disconnected miner previously aborted the broadcast and starved healthy downstreams of their new difficulty. Finish the fan-out before returning the disconnect action for the failed peer.

An SV1 miner can pipeline setup traffic while its SV2 channel is opening. Cap that per-downstream queue at eight messages and disconnect on the ninth so an unauthenticated peer cannot grow it without bound.

Keep the existing Bitcoin Core IPC integration test aligned with rustfmt output. This is a formatting-only change with no runtime behavior impact.

Make the default trust boundary explicit: payout verification is disabled unless configured, so tProxy otherwise accepts the upstream payout policy. Operators can then make an informed choice when enabling the check.

Channel and group identifiers share routing state, so accepting a reused or reserved identifier can reinterpret later messages for the wrong owner. Reject collisions transactionally, including the aggregated sentinel ID.

Aggregated miners share job history, but keepalive delivery follows each miner's own idle interval. Bound shared nTime mutation by wall-clock time and reuse the latest keepalive for miners that become due between mutations. Replay the current shared job for late joiners without clearing history, rewinding nTime, or restarting the aggregate timer. Normal upstream jobs remain on their normal delivery path.

bit-aloo

ACK. Clanker suggested some non blocker, but I don't think they were interesting to block work here.

GitGab19 force-pushed the fix/tproxy-loupe-followups branch 3 times, most recently from d08e12b to 639735c Compare August 24, 2026 09:17

bit-aloo reviewed Aug 25, 2026

View reviewed changes

Comment thread miner-apps/translator/src/lib/sv1/sv1_server/mod.rs

Comment thread miner-apps/translator/src/lib/sv1/downstream.rs Outdated

Comment thread miner-apps/translator/src/lib/translator_runtime.rs Outdated

Comment thread miner-apps/translator/src/lib/sv1/sv1_server/mod.rs Outdated

GitGab19 force-pushed the fix/tproxy-loupe-followups branch from 639735c to 2c00d65 Compare August 25, 2026 13:38

GitGab19 mentioned this pull request Aug 25, 2026

handle Reconnect and ChannelEndpointChanged consistently #780

Open

GitGab19 force-pushed the fix/tproxy-loupe-followups branch from 2c00d65 to fe92ddd Compare August 25, 2026 13:40

GitGab19 marked this pull request as draft August 27, 2026 09:28

GitGab19 force-pushed the fix/tproxy-loupe-followups branch from fe92ddd to c6a57a5 Compare August 27, 2026 10:07

GitGab19 marked this pull request as ready for review August 27, 2026 10:08

GitGab19 requested a review from bit-aloo August 27, 2026 10:12

bit-aloo reviewed Aug 27, 2026

View reviewed changes

Comment thread miner-apps/translator/src/lib/sv1/sv1_server/mod.rs

Comment thread miner-apps/translator/src/lib/sv2/channel_manager/mining_message_handler.rs Outdated

Comment thread miner-apps/translator/src/lib/sv1/sv1_server/mod.rs Outdated

GitGab19 force-pushed the fix/tproxy-loupe-followups branch from c6a57a5 to 733d19d Compare August 27, 2026 12:36

GitGab19 mentioned this pull request Aug 27, 2026

clean up channel and downstream state on every terminal path #777

Open

GitGab19 force-pushed the fix/tproxy-loupe-followups branch 2 times, most recently from c5e7015 to ba90896 Compare August 27, 2026 14:43

GitGab19 mentioned this pull request Aug 27, 2026

harden tProxy Sv1 session and channel lifecycle #823

Merged

GitGab19 force-pushed the fix/tproxy-loupe-followups branch from ba90896 to b5e41c1 Compare August 31, 2026 04:14

plebhash reviewed Sep 8, 2026

View reviewed changes

Comment thread integration-tests/tests/bitcoin_core_ipc_jdp_io.rs

plebhash reviewed Sep 8, 2026

View reviewed changes

Comment thread miner-apps/translator/README.md Outdated

GitGab19 force-pushed the fix/tproxy-loupe-followups branch 2 times, most recently from d5ea928 to 471ce4b Compare September 9, 2026 08:46

plebhash approved these changes Sep 9, 2026

View reviewed changes

GitGab19 force-pushed the fix/tproxy-loupe-followups branch 3 times, most recently from 02e572c to 3dafaf3 Compare September 9, 2026 19:49

plebhash approved these changes Sep 9, 2026

View reviewed changes

bit-aloo reviewed Sep 10, 2026

View reviewed changes

GitGab19 force-pushed the fix/tproxy-loupe-followups branch from 3dafaf3 to e386164 Compare September 10, 2026 10:21

GitGab19 added 8 commits September 11, 2026 10:45

docs(translator): clarify vardiff target ordering policy

975eea6

Document that a new vardiff target takes effect with the next mining.notify rather than immediately. This preserves the difficulty advertised with each job so late shares are not checked against a newer target.

fix(translator): complete difficulty broadcasts after peer disconnects

a8f6e50

A failed send to one disconnected miner previously aborted the broadcast and starved healthy downstreams of their new difficulty. Finish the fan-out before returning the disconnect action for the failed peer.

fix(translator): bound pre-channel SV1 message buffering

5991f07

An SV1 miner can pipeline setup traffic while its SV2 channel is opening. Cap that per-downstream queue at eight messages and disconnect on the ninth so an unauthenticated peer cannot grow it without bound.

chore: format Bitcoin Core IPC integration test

a3329bd

Keep the existing Bitcoin Core IPC integration test aligned with rustfmt output. This is a formatting-only change with no runtime behavior impact.

docs(translator): clarify payout verification default

7af9de0

Make the default trust boundary explicit: payout verification is disabled unless configured, so tProxy otherwise accepts the upstream payout policy. Operators can then make an informed choice when enabling the check.

fix(translator): reject colliding channel identifiers

044bb22

Channel and group identifiers share routing state, so accepting a reused or reserved identifier can reinterpret later messages for the wrong owner. Reject collisions transactionally, including the aggregated sentinel ID.

GitGab19 force-pushed the fix/tproxy-loupe-followups branch from e386164 to 912a11a Compare September 11, 2026 09:20

bit-aloo approved these changes Sep 11, 2026

View reviewed changes

bit-aloo merged commit 6278601 into stratum-mining:main Sep 11, 2026
10 checks passed

GitGab19 mentioned this pull request Sep 11, 2026

enforce a collision-free channel and group ID namespace #812

Closed

Conversation

GitGab19 commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bit-aloo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GitGab19 commented Aug 27, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

plebhash commented Sep 8, 2026

Uh oh!

plebhash commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GitGab19 commented Sep 8, 2026

Uh oh!

GitGab19 commented Sep 9, 2026

Uh oh!

bit-aloo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bit-aloo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

GitGab19 commented Aug 19, 2026 •

edited

Loading

plebhash commented Sep 8, 2026 •

edited

Loading