Simplify env var names for node URLs

[sergerad]

As per comment:

Not for this PR, but I find the naming a bit confusing here. For example, MIDEN_NODE_VALIDATOR_BLOCK_PRODUCER_URL is the URL at which the block producer can find the validator, right? If so, I think a more "intuitive" approach would be to name it MIDEN_NODE_BLOCK_PRODUCER_VALIDATOR_URL. Same goes for MIDEN_NODE_STORE_BLOCK_PRODUCER_URL etc.

[Mirko-von-Leipzig]

I think this will become quite confusing. The current naming scheme follows a nesting approach whereas this suggestion follows a more proper English naming approach. However this only applies to the env vars which will then no longer line up with the CLI args itself.

As in, we're creating a configuration struct:

node {
    block-producer-config {
        validator-url
        ...
    }
}

which can be "indexed" as node.block-producer.validator-url. Swapping these around just to have the env vars read like English is a mistake imo since there are many x-y and y-x pairs.

That said, there probably is simplification we can make in some cases. As in, we probably don't need to prefix some of these things completely e.g. MIDEN_NODE_VALIDATOR_URL is probably just as fine?

[bobbinth]

I think this will become quite confusing. The current naming scheme follows a nesting approach whereas this suggestion follows a more proper English naming approach. However this only applies to the env vars which will then no longer line up with the CLI args itself.

As in, we're creating a configuration struct:

node {
    block-producer-config {
        validator-url
        ...
    }
}

which can be "indexed" as node.block-producer.validator-url. Swapping these around just to have the env vars read like English is a mistake imo since there are many x-y and y-x pairs.

I think this is what I was trying to suggest. Basically, right now we have the convention MIDEN_NODE_[setting]_[component] and I think it should be MIDEN_NODE_[component]_[setting] (which I think what you are suggesting as well). For example, as I understand it, the current setting MIDEN_NODE_VALIDATOR_BLOCK_PRODUCER_URL is the setting for the block producer (for it to find the validator), but the VALIDATOR comes before the BLOCK_PRODUCER.

Same goes for MIDEN_NODE_STORE_RPC_URL - IIUC, this is the URL at which the RPC component can find the store. And so again, we have MIDEN_NODE_[setting]_[component] convention instead of MIDEN_NODE_[component]_[setting] convention.

But maybe I'm misunderstanding how these settings are used?

That said, there probably is simplification we can make in some cases. As in, we probably don't need to prefix some of these things completely e.g. MIDEN_NODE_VALIDATOR_URL is probably just as fine?

I agree, though we already have MIDEN_NODE_VALIDATOR_URL for specifying the address on which the validator is running (this is related to #1431). So, if we were to change the current MIDEN_NODE_VALIDATOR_URL to something like MIDEN_NODE_VALIDATOR_PORT, then we could use MIDEN_NODE_VALIDATOR_URL to specify the validator URL for services that need to interact with the validator.

[Mirko-von-Leipzig]

Ah, then we have then in both orientations which is why I got confused. Definitely we want the order [component]_[setting].

I imagine what's happened is that we sometimes re-use the server env var in the config for the client.

I had previously looked into https://github.com/cbeck88/conf-rs which is an alternative to clap-derive but which specifically claims to help managing large projects with re-usable configs in subcommands etc. It still uses clap under the hood - its just the derive macro that is changed.

Specifically what stood out to me was:

You can assign a prefix to a structure's fields when flattening it into another structure, and you can similarly do env prefixing in a controlled way.

which I think would completely solve our problem here.