feat(jds): validate and bind user_identifier to downstream connections - #839
feat(jds): validate and bind user_identifier to downstream connections#839nulllpc wants to merge 1 commit into
Conversation
This implements the identity tracking requirements for JDS to ensure reliable token allocation and job declaration activity. - Bind user_identifier to the Downstream state during AllocateMiningJobToken - Reject empty strings or mid-session mismatched identities with a new InvalidUserIdentifier disconnect error to enforce connection consistency - Remove raw user_identifier logging in the declaration handler to prevent unnecessary identity leakage - Pass the bound user_identity to JobValidationEngine::handle_set_custom_mining_job so upstream pools can authenticate the custom job - Add integration tests verifying JDS correctly disconnects downstreams that send empty or mismatched identities, while allowing replayed identities
| //! - `DeclareMiningJob` rejects a coinbase that does not carry exactly one input, without tearing | ||
| //! down the IPC connection. |
| // This test verifies that JDS requires all subsequent token allocations on a single | ||
| // connection to use the exact same identity, preventing mid-session identity swapping. |
There was a problem hiding this comment.
hmm why isn't a JDC allowed to allocate tokens under different indentities?
maybe JDC opened multiple channels with Pool, each with a different user_identity
| // This test verifies that JDS accepts subsequent token allocations on a single | ||
| // connection as long as they use the exact same (replayed) identity. |
There was a problem hiding this comment.
hmm why isn't a JDC allowed to allocate tokens under different indentities?
maybe JDC opened multiple channels with Pool, each with a different user_identity
| async fn handle_set_custom_mining_job( | ||
| &self, | ||
| downstream_id: DownstreamId, | ||
| user_identity: Option<String>, |
There was a problem hiding this comment.
why is this Option?
on Pool, every channel always has a user_identity
| async fn handle_set_custom_mining_job( | ||
| &self, | ||
| downstream_id: DownstreamId, | ||
| user_identity: Option<String>, |
There was a problem hiding this comment.
why is this Option?
on Pool, every channel always has a user_identity
| /// The authenticated user identity bound to this connection via AllocateMiningJobToken. | ||
| pub user_identity: SharedLock<Option<String>>, |
There was a problem hiding this comment.
I don't think adding this parameter to Downstream is the right choice
this design seems heavily based around the assumption that every JDC will always have one single Mining Channel with Pool
while that's currently true with SRI Pool, that's not a restriction imposed by spec, and a JDC that opens multiple Channels is theoretically possible
we want to enforce cohesion between token allocation and user_identity... but enforcing 1 single user_identity per Connection seems like a misguided assumption
There was a problem hiding this comment.
we're not yet in the right direction here
we should be touching JDS token_management/mod.rs and adding a new user_identity: String field to AllocatedTokenData and ActiveTokenData
(tbh I think those types deserve a dedicated commit turning them from tuples into structs)
then during the execution of JobDeclarator::handle_set_custom_mining_job we should make sure the channel's user_identity (passed from Pool) matches the one associated with the active token
|
Thanks @plebhash for the review. I'll refine the PR. I'll turn it into draft for now |
2 participants
This implements the identity tracking requirements for JDS to ensure reliable token allocation and job declaration activity.
Solves #779