Translator panics on share submission when user_identity is a Bitcoin address

[warioishere]

Bug Report

Description

The translator (tproxy) panics with the following error as soon as a connected SV1 miner submits a share, when user_identity in the config is a Bitcoin address:

thread 'tokio-runtime-worker' panicked at miner-apps/translator/src/lib/sv1/sv1_server/sv1_server.rs:461:18:
called `Result::unwrap()` on an `Err` value: "UserIdentity exceeds 32 bytes"

Root Cause

When a SV1 miner connects, the translator constructs a per-miner identity string (sv1_server.rs:718):

let user_identity = format!("{}.miner{}", self.config.user_identity, miner_id);

On every share submission, the translator attempts to attach a UserIdentity TLV (Worker-Specific Hashrate Tracking extension 0x0002) to the SubmitSharesExtended message (sv1_server.rs:460-461):

UserIdentity::new(&user_identity_string)
    .unwrap()  // panics here

UserIdentity in extensions-sv2 has a hard limit of MAX_USER_IDENTITY_LENGTH = 32 bytes. Bitcoin addresses are up to 64 bytes (e.g. regtest taproot bcrt1p...), which immediately exceeds this limit.

Use Case / Motivation

We are running an SV2 server in solo mining mode where the server parses the user_identity field from OpenExtendedMiningChannel and substitutes the pool's coinbase Bitcoin address with the address provided in user_identity. This allows each miner/operator to receive block rewards directly to their own address.

This approach already works correctly for the JD-client: it passes the Bitcoin address via OpenExtendedMiningChannel.user_identity (typed as Str0255, 255-byte limit — no problem), and does not create a UserIdentity TLV on share submissions (there is even a TODO comment at jd-client/src/lib/channel_manager/downstream_message_handler.rs:1306 acknowledging this is not yet implemented).

The translator, however, unconditionally creates the TLV on every share, causing the crash.

Affected Versions

• translator_sv2 — crashes on first share when user_identity length > 32 bytes
• jd-client — works correctly today (no TLV on shares, address passed via Str0255)

Proposed Quick Fix

Make the UserIdentity TLV on share submissions opt-in via a config flag, defaulting to false (disabled), matching current JD-client behaviour:

# Attach worker identity TLV to share submissions for per-worker hashrate tracking.
# Requires upstream to support extension 0x0002.
# Note: user_identity must be <= 32 bytes when enabled.
enable_worker_identity_tlv = false

In sv1_server.rs, replace:

// Only add TLV fields with user identity in non-aggregated mode
let tlv_fields = if is_non_aggregated() {
    let user_identity_string = ...;
    UserIdentity::new(&user_identity_string)
        .unwrap()   // <-- panics
        .to_tlv()
        .ok()
        .map(|tlv| vec![tlv])
} else {
    None
};

With:

let tlv_fields = if self.config.enable_worker_identity_tlv {
    let user_identity_string = ...;
    UserIdentity::new(&user_identity_string)
        .ok()
        .and_then(|ui| ui.to_tlv().ok())
        .map(|tlv| vec![tlv])
} else {
    None
};

Longer-Term Issue

The 32-byte limit in extensions-sv2 (MAX_USER_IDENTITY_LENGTH) and parsers-sv2 (tlv_extensions/mod.rs) is incompatible with Bitcoin addresses being used as worker identities. Bitcoin addresses range up to 64 bytes (regtest bech32m taproot). This limit likely needs to be revisited at the spec level (EXTENSION_TYPE=0x0002) to support address-based identity for a possible solomining mode,, but that is a separate discussion.

Steps to Reproduce

1. Set user_identity in the translator config to any Bitcoin address (e.g. bcrt1qqz93nn9k2r08mallruhgfy8f7xfkkkge93chf5)
2. Connect a SV1 miner to the translator
3. Wait for the miner to submit a share
4. Translator panics

Expected Behaviour

The translator should not crash. If the UserIdentity TLV cannot be constructed (e.g. identity too long), it should either skip the TLV gracefully or be disabled via config.

[GitGab19]

We don't need any new additional fields in the config file (the mentioned enable_worker_identity_tlv).

TLV is appended by tProxy in case the extension n.2 is required by it (see https://github.com/stratum-mining/sv2-spec/blob/main/extensions/0x0002-worker-specific-hashrate-tracking.md), and that is set in the config files inside required_extensions.

So I think the problem is that on the tproxy-config-local-jdc-example.toml we enable that extension, so I would simply deactivate it by removing it in that config example for now (since it's not really used in any case).

[warioishere]

@GitGab19 Thanks for the clarification. I traced the code and found there are actually two gates:

1. SV1 server (sv1_server.rs:453) — builds the TLV whenever is_non_aggregated() is true. This is purely config-based (aggregate_channels = false), with no check on whether extension 0x0002 was negotiated. The .unwrap() panic happens here.

2. Channel manager (channel_manager.rs:571-578) — checks whether 0x0002 was actually negotiated before encoding the TLV into the frame. If not negotiated, the TLV is silently dropped.

So you're right that the TLV never reaches upstream if 0x0002 isn't in required_extensions. But the panic still triggers because the SV1 server unconditionally builds the UserIdentity and calls .unwrap() before the channel manager can drop it. The crash happens even when the TLV would be thrown away.

Shouldn't the SV1 server also check whether the extension was negotiated before attempting to build the TLV? That way it would:

• Skip unnecessary work when the extension isn't active
• Avoid the panic entirely

Also a second question: why is the TLV built in is_non_aggregated() mode? In non-aggregated mode each miner has its own channel, so the upstream already identifies workers via channel_id. TLV would add value in is_aggregated() mode where all miners share one channel and can't be distinguished otherwise. Is the condition intentionally is_non_aggregated() or should it be is_aggregated()?

[GitGab19]

Shouldn't the SV1 server also check whether the extension was negotiated before attempting to build the TLV?

This is what PR #276 is doing.

Also a second question: why is the TLV built in is_non_aggregated() mode? In non-aggregated mode each miner has its own channel, so the upstream already identifies workers via channel_id

That's because of one very subtle detail related to Sv1: we get the worker_name of the machine only in the mining.authorize message, but before that, we need to open the channel for the machine, to answer to the previous mining.subscribe message (with the extranonce1 and extranonce2_size values).

Once we get the mining.authorize, we can append it as TLV in the SubmitSharesExtended messages, and the upstream will receive it, being able to show the same worker_name configured on the machine.

And in order to have granular data at the upstream (pool) level, it's way better to not aggregate channels, that's why we append the TLV only in that case. When you aggregate everything into a single channel, it becomes harder to have precise measurements at the pool level with machine-level granularity.

[warioishere]

@GitGab19 Thanks for the detailed explanation, that makes sense now. I didn't realize authorize() overwrites data.user_identity after handle_open_channel_request sets it from the config — so the TLV carries the miner's worker name from mining.authorize, not the config value.

That means PR #276 would fix my crash as well, since:

• The extension negotiation check prevents TLV from being built when 0x0002 isn't negotiated
• The truncation at authorize time handles the case when it is

And for my use case (solo mining with a BTC address as user_identity), the full address still reaches the upstream via OpenExtendedMiningChannel.user_identity (Str0255), which is set at channel open time and unaffected by the TLV limit.

I closed my PR #300 since #276 covers this. Looking forward to that one getting merged.