decred / vspd

A Voting Service Provider (VSP) for the Decred network.
ISC License
19 stars 20 forks source link

Introduce vspadmin binary. #476

Closed jholdstock closed 3 months ago

jholdstock commented 4 months ago

vspadmin is a new binary which implements one-off admin tasks which are necessarily interactive and thus do not fit neatly into the long-lived vspd binary.

The initial features added to vspadmin are creating blank databases and default config files, both part of the deployment procedure for a new instance of vspd.

In future this binary can become home for additional features such as changing the admin password (#281, #311) and changing the fee xpub (#461).

Closes #465

jholdstock commented 3 months ago

I really appreciate the review even if you don't agree with the premise, thanks.

Software should just work by default and then you can tweak it from there...

While I strongly agree with this sentiment, I don't believe it's possible with vspd because there are several things which must be manually configured before vspd is able to operate - e.g. an xpub key for collecting fees, connection details for voting wallets.

Do you have any other suggestions for how to solve this problem? I guess it would be possible to add default values which get the process to start without any manual config, but then it would just throw a ton of errors and not actually be able to operate.

I'll also mention that this PR is not making the existing situation worse, perhaps just shining a light on it. The manual steps were already required and they are still required after - the only thing which has changed is which binary needs to be executed.

dajohi commented 3 months ago

createdatabase - can vspd make the database if it doesn't exist?

writeconfig - have vspd write one if it doesn't exist, and then exit.

jholdstock commented 3 months ago

@dajohi - That is precisely how it worked before this PR, the functionality to perform the two tasks has just moved from one bin to another.

davecgh commented 3 months ago

I'm aware there are several things that have to be configured, such as the fee xpub. The same type of scenario is true (except for seeds instead of a pubkey) in e.g. dcrwallet, dcrdex, Cryptopower, etc. Requiring the user to perform some setup, preferably guided by the application itself, is something that is totally expected and often can't be avoided, such as in the case of the xpub.

However, there is a very big difference between them needing to set a config flag, add an entry to a config file, etc, and needing to acquire and run an entirely separate binary. I know it might not seem like much for we dev types, but having to go compile an entirely separate binary (and with the instructions as presented here, you even need to first go download, install, and configure golang itself to be able to do the go run it prescribes) is a pretty steep set of steps when you don't already know how all of that works.

To me, it is a much better user experience to simply complain with a detailed message if things are required. For example, if you were to try to run vspd and it said "An extended public key is required to be set via the --feexpub option .... etc`. It's pretty clear what needs to be done and there is no whole other set of hoops to jump through to go find and build another binary, configure that other binary to point at the right config locations, etc, etc.

Since vspd already offers a web-based interface, an even better approach, imo, would be to do it it dcrdex does. When you run it, it points the user at the web address and provides a graphical setup wizard to do all of the one-time setup things. It's certainly a little bit more work, but it provides a much nicer UX overall.

If you're concerned about being able to automate it, that could still work via providing the right combination of CLI options.

jholdstock commented 3 months ago

Perhaps the database creation and config writing could stay in the vspd binary, but the original inspiration behind the new binary was implementing xpub changing. I considered adding it to the web UI but I think it is too sensitive of an action to be exposed on the internet, especially when it could be implemented on the server CLI with less effort. Shoe-horning it into the existing CLI flags feels really clumsy and not intuitive.

Its interesting that you mention dcrdex as your example because actually that is where the inspiration for this PR came from.

The dcrdex client does indeed guide the user through an easy GUI, which is obviously correct because it is an application aimed at end users. The server however is composed of a number of CLI bins, notably the main server daemon and a separate admin binary (as well as some other small utilities). Its a more elegant solution than just cramming everything into one binary IMO.

We have specified in the VSP operator guide ever since dcrstakepool launched that operators are expected to be experienced sysadmins and required to build go from source, so I'm not sure I agree that running a different bin is a heavy burden. To be brutally honest about it, if somebody cant figure out how to run a binary or how to build go from source, we probably don't want them operating a VSP anyway.

davecgh commented 3 months ago

Understood. I mean, you're the repo lead, so it's up to you. It's just a serious pet peeve of mine having separate bins for basic setup tasks. Interesting point about DEX, though I'll note that I've never had to run any different bins for DEX despite running it for years at this point.