-
Notifications
You must be signed in to change notification settings - Fork 23
Description
Extension negotiation (RequestExtensions → RequestExtensionsSuccess/Error) completes asynchronously after the SV1 server has already started accepting downstream connections. This creates race conditions where extension-dependent behavior cannot be reliably determined at the time it's needed.
Background: How We Encountered This
While investigating a panic in the translator (PR #197), we discovered:
- A downstream SV1 miner sends mining.authorize with a username like very-long-pool-username.worker1
- The translator attempts to create a UserIdentity TLV (Extension n.2 - Worker Specific Hashrate Tracking)
- The UserIdentity field has a protocol-specified maximum of 32 bytes
- If the username exceeds 32 bytes, the translator panics on unwrap()
During PR review, GitGab19 noted: "We don't check the length at authorize time because we don't know if we need to use the extension or not there."
Investigation revealed the translator sends RequestExtensions during setup_connection() but does not wait for the response before returning. The SV1 server starts immediately, accepting downstream connections before extension negotiation completes.
Key code paths:
| Step | File | Lines | Description |
|---|---|---|---|
| Send RequestExtensions | miner-apps/translator/sv2/upstream/upstream.rs | 276-300 | Sends message but doesn't await response |
| Return without waiting | miner-apps/translator/sv2/upstream/upstream.rs | 302 | Ok(()) returned immediately |
| Start SV1 server | miner-apps/translator/lib/mod.rs | 324-330 | sv1_server_instance.start() called before extensions known |
| Async message loop | miner-apps/translator/sv2/channel_manager/channel_manager.rs | 184 | handle_upstream_frame() in select! loop |
| Route to extensions handler | miner-apps/translator/sv2/channel_manager/channel_manager.rs | 244-248 | MessageType::Extensions routed here |
| Store negotiated extensions | miner-apps/translator/sv2/channel_manager/extensions_message_handler.rs | 59-61 | data.negotiated_extensions = supported |
Proposed Solution
Extension negotiation must complete synchronously before accepting downstream connections:
- TCP + Noise handshake with upstream
- Send SetupConnection
- Receive SetupConnection.Success
- Send RequestExtensions
- Receive RequestExtensionsSuccess/Error ◄── WAIT HERE
- Return from setup_connection() with known extension state in channel manager
- Start SV1 server ◄── EXTENSIONS ARE NOW KNOWN
- Accept miner connections with full knowledge of capabilities
This way all asynchronous message handling is conducted with the channel-manager having full knowledge of extensions and can handle according.
Graceful Degradation for Extensions?
Should extensions prevent mining if the Downstream doesn't comply with the extension?
For the username extension specifically, we could probably just hash whatever string the Downstream Sv1 provides and use that Upstream with the extension since that would be 32 bytes.