Skip to content
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

Support for editing config file (to an extent) via web dashboard #49

Draft
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

luke-jr
Copy link
Contributor

@luke-jr luke-jr commented Dec 13, 2024

Built on top of #25

Mostly functional, two FIXMEs remain that should have minimal issues until followup. Would be nice to avoid restarting ever, but that can be a future improvement.

@luke-jr luke-jr added the enhancement New feature or request label Dec 13, 2024
@luke-jr luke-jr requested a review from wizkid057 December 13, 2024 23:02
@luke-jr luke-jr added this to the v0.3.0 milestone Dec 14, 2024
@luke-jr luke-jr mentioned this pull request Dec 14, 2024
Copy link
Collaborator

@wizkid057 wizkid057 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK

Copy link
Collaborator

@wizkid057 wizkid057 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3bb50b8

Sorry, missed this when reviewing... GBT cURL call has an aggressive timeout so that the block template thread never hangs.

Booting all stratum clients after a single failure is probably too aggressive, considering the 1 second retry already in play. Any number of benign things can prevent this call from returning, especially if the node isn't on the same machine.

If we are working on something valid, I'd suggest not booting stratum clients until GBT fails for long enough for the the current job to be stale.

@szarka
Copy link
Contributor

szarka commented Dec 16, 2024

Is there anything in this patch, or elsewhere, that adds access control for the dashboard? AFAIK, that has to be done external to datum_gateway itself right now, but it's not too much of a threat to just display the stats. Adding config editing changes that.

@luke-jr
Copy link
Contributor Author

luke-jr commented Dec 17, 2024

@szarka This feature is disabled by default and needs to be turned on with a new config file setting.

Currently, it does not have any special access control, and that could be a possible follow-up PR.

Note that it intentionally does not "read back" the RPC password, only allows changing it.

@szarka
Copy link
Contributor

szarka commented Dec 17, 2024

@szarka This feature is disabled by default and needs to be turned on with a new config file setting.

Nice. 👍

@BitcoinMechanic
Copy link
Contributor

Let's add authentication to this. Should require (or least give the user the option of requiring) username/password to log in to the dashboard.

@luke-jr
Copy link
Contributor Author

luke-jr commented Jan 15, 2025

Not sure authentication is realistic: right now, the webserver is entirely unencrypted and can just be MITM'd...

@luke-jr luke-jr marked this pull request as draft January 16, 2025 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants