roles CLI standardization

[plebhash]

Discussion for refactoring bringing Standardization to the CLIs on application level of the SRI stack (roles)

[plebhash]

toml dependency replacement

As described in #456, we are pointing to a fork for our toml dependency as a workaround for no_std support.

The toml crate is used to parse the configuration file, but we need some alternative that supports no_std from its upstream source (so it can be fetched directly from crates.io instead of a fork on github.com). So I propose we replace toml with config.

The config crate is the perfect candidate, since it supports TOML as a first-class-citizen and also supports no_std (as I validated on this replit)

[plebhash]

clap for managing CLI args

Currently, most of our roles CLI args are parsed directly from std::env::args(). That introduces unnecessary complexity.

clap is a well known crate that provides abstractions for declaration and management of CLI args, so that they can be summarized into a few macros.

This was already introduced by @Fi3 into the implementation of roles/test-utils/mining-device via #819

I propose we standardize this clap-based approach for all roles.

[plebhash]

args module split

The way CLI args are declared and parsed is slightly different on each role implementation. There are similar coding patterns, but files are organized under different strategies.

It's not good to have scattered coding patterns across the different roles implementations. If we follow one specific CLI pattern in one role, we should do the same on all roles. This improves readability and maintainability of the codebase.

I propose we standardize roles CLI args as a standalone args module to live on the root level of every role binary crate. This module should have:

• declaration of struct Args with clap macros
• definition of fn process_cli_args(), responsible for returning a *Config object

The args module is already implemented on some roles:

• translator
• jd-client

[plebhash]

*Result and *Error structs

Each roles has its own flavor of Result and Error types.

We have a very concise programming pattern around this convention, but unfortunately the variable names are not consistent.

Sometimes they follow a naming-style like RoleResult and RoleError, but sometimes they are simply called Result and Error.

I propose we standardize this so that every role follows the naming-style like RoleResult and RoleError:

• PoolResult + PoolError
• JdsResult + JdsError
• JdcResult + JdcError
• ProxyResult + ProxyError
• TProxyResult + TProxyError

This will avoid name clashes and improve code readability.

[rrybarczyk]

I think that we should keep each workspace's Result type as Result. And then export in the mod.rs like:

mod error;

pub use error::{Error as <Role>Error, Result as <Role>Result};

So jd_client::mod would be:

mod error;

pub use error::{Error as JdcError, Result as JdcResult};

[rrybarczyk]

My above comment is suboptimal because many traits cannot use each role's internal Result type, instead using std::result::Result. I think it is cleaner with how @plebhash originally proposed, using <Role>Result/<Role>Error both internally and externally.

[plebhash]

*Config structs

The same issue from *Result and *Error also applies to the Config struct. All roles have one, but sometimes they are named under a RoleConfig, or sometimes just Config.

I propose we standardize this so that every role follows the naming-style like RoleConfig:

• PoolConfig
• JdsConfig
• JdcConfig
• ProxyConfig
• TProxyConfig

This will avoid name clashes and improve code readability.

Also, some roles have a standalone module inside lib for the Config declaration (e.g.: roles/translator/src/lib/proxy_config.rs). I propose we also standardize this pattern for all roles, so that each RoleConfig is declared under src/lib/<role>_config.rs:

• jd_server/src/lib/jds_config.rs
• jd_client/src/lib/jdc_config.rs
• mining-proxy/src/lib/proxy_config.rs
• pool/src/lib/pool_config.rs
• translator/src/lib/tproxy_config.rs

Finally, I also propose we standardize that fn get_coinbase_output is always declared inside this module alongside RoleConfig (since it is basically a getter for config.coinbase_outputs). This only applies to pool, JDC and JDS, since proxies do not have coinbase_output on their config files

[rrybarczyk]

What about:

• jd_server/src/lib/config.rs
• jd_client/src/lib/config.rs
• mining-proxy/src/lib/config.rs
• pool/src/lib/config.rs
• translator/src/lib/config.rs

Then define each module's config struct as Config, for example jd_server::jds_config::JdsConfig -> jd_server::config::Config. Then in jd_server::mod:

pub mod config;
pub use config::Config as JdsConfig

Or in mining_proxy::config, change ProxyConfig -> Config, then in mining_proxy::mod:

pub(crate) mod config;
pub use config::Config as ProxyConfig;

It is a small thing, but I like this convention because the naming conventions are uniform across each role's workspace so it feels cleaner and the file names are slightly shorter.