-
Notifications
You must be signed in to change notification settings - Fork 15
Set JD Mode from config #180
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we setting the mode in the handle_setup_connection_success?
JDC should get the mode from config at bootstrap, and then use its value to properly set the DECLARE_TX_DATA flag in the SetupConnection to be sent to the JDS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is subtle. I also recollected this while looking into the code, I had kind of forgotten how the fallback and upstream + JDS invariants work. Let me explain.
If we look at the construction of the SetupMessage for the JobDeclarator, we can see the inclusion of self.mode in the message constructor:
| let setup_connection = get_setup_connection_message_jds(&self.socket_address, &self.mode); |
SetupConnection.Success handler is the runtime mode in which the JDC is currently running.
During its execution lifetime, the JDC can have two possible mode values:
- The one it gets from the config
- and, solo mining.
The SetupConnection.Success from the JDS is the last message in the upstream connection or fallback flow, after which we consider that the upstream is not malicious and is fit for processing. That’s the reason why we set JDMode here. Earlier, we were setting JDMode based on the flag received in the success message. In contrast, now it is being set based on self.mode, which is the mode we get from the config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why can't we set the value to this JDMODE during the initial bootstrapping, but we have to set it in the handler here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because JDMode is a runtime value, it drives the control flow of the JDC. Its value can transition from SoloMining → FullTemplate / CoinbaseOnly → … and so on, and may change again if a fallback happens at any point in between. I recall that we discussed this behavior while developing the JDC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But why can't we start JDC by setting its JDMODE with the value coming from config, instead of starting it from SoloMining mode?
Now I don't remember the old discussions about this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rationale is that JDMode should always be in a valid state. Until the mode specified in the config is confirmed (by successfully connecting to the upstream), the JDC should not operate in a partially initialized or inconsistent mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO we should start JDC in the mode which is set in the config, without starting it in SoloMining mode besides of the mode.
Then if we cannot connect to the upstream, or we get an error from it, we trigger fallback.
In the future we could eventually add SoloMining as a new mode variant, which can be used by users who want to do solo-mining using JDC.
closes: #179