search

MASQ-Project / MASQ-Node-issues

This repo contains the issues that are used for planning MASQ Node product work. It has no code in it, only GitHub issue tickets
https://masq.ai/
31 stars 12 forks source link

SetupReporter redesign #767

Open dnwiebe opened 8 months ago

dnwiebe commented 8 months ago

Our SetupReporter dates from the earliest days of the Daemon, when we had a vague idea of what we wanted to do but didn't know any of the details. As our knowledge of how the Node should be set up has increased, we have added little special cases and hacks to the SetupReporter, and now it is a truly admirable pile of sadness.

Rename the existing SetupReporter to something like SetupReporterOld and replace it with a clean-sheet original that is designed from the beginning to give us what we want now, but with simpler and clearer code and concepts. Make sure it has the same behavior as the existing SetupReporter (except perhaps in corner cases) by running the existing high-level tests on it and making sure they pass.

This will require some understanding of what the existing SetupReporter does, but not too much: we don't want to replicate all the existing complexity produced by a series of ugly hacks: we want simple and clear code.

If GH-698 is not already complete, look at it and decide whether this card should be blocked on it, or whether this card should include it.

It may help to set out some rules from the beginning of the redesign--rules that we didn't know when we first wrote SetupReporter but have since formulated from experience. For example:

  1. If config-file is specified in a configuration file, that should be an error.
  2. There are a few intricate dependencies in defaulting (for example, if you need data-directory or real-user to find the config file, then it should be an error to specify data-directory or real-user in the config file...but if you can get to the config file without needing those parameters, specifying them in the config file is perfectly reasonable) that need to be investigated, understood, and arranged with a simple, understandable set of rules.
  3. The setup logic should not make guesses or assumptions about what configuration the user wants: if performing the configuration requires choices driven by obscure special cases (see above), the code should emit an error message demanding clearer configuration and refuse the command.
  4. When it comes to finding configuration errors, there's a little bit of that that's done by masq (mostly relegated to clap), but there's a lot of it that's done both by SetupReporter and by the Node, mostly in NodeConfigurator. It needs to be done in both places, because when the Daemon is being set up, the Node is not yet running. These two sets of validation need to be synchronized; it might make sense to fix things so that both validations are done by the same piece of code somehow. (Remember, the Daemon and the Node use the same binary, so whatever code ends up in that binary is available to both. Just don't break the Daemon/Node rules.)
  5. The setup logic should be simple enough to document somewhere accessible to the user: preferably in the clap help, but at least in an .md document that the clap help can point to.
  6. Remember that the Daemon (with the SetupReporter inside it) is running in a different process than the UI, with a different set of environment variables. The environment changes the user makes to the UI process will not affect the Daemon environment that SetupReporter sees. This means that the Daemon's process may have environment values that will shadow any config-file values the user sets; the response the Daemon sends to the setup command from the UI should make this visible.
  7. The same phenomenon above doesn't precisely apply to the Daemon and the Node: they run in different processes with different environments, but the Node inherits its environment from the Daemon when the Daemon starts it, so it should have access to the same values that are set in the Daemon's process.
  8. The same--or at least roughly the same--clap schema is used for the setup command in masq, for the Daemon in processing the parts of the setup command, and for the Node in its bootstrap configuration. This raises the question of where defaults should be applied. If a parameter--say, min-hops--has a clap default, should that default be passed from masq (or the GUI) to the Daemon, or should the value of min-hops be left out so that it can be defaulted later? If later, when is later? Should it be passed from the Daemon to the Node on the command line, or should it be left off the command line so that the Node can default it when it bootstraps? Note that this question must be answered on more than one level, since there are defaults that come from clap, defaults (like clandestine-port) that come from the database (which may not exist when the Daemon runs), and defaults that are computed based on the values of other parameters (like the value of config-file that depends on data-directory and real-user).