feat: CLI configuration #447

Dodecahedr0x · 2025-07-10T16:33:00Z

This PR is another attempt to allow layered configuration (default -> file -> args -> env) while providing users with information on what parameters they can adjust.

It overrides #346 with the following advantages:

Automatically derives CLI and env var names to avoid repetitions and reduce risk of typos
Single definition of the configuration

The weakness of this approach is that it utilizes small procedural macros to avoid repetition, which can compromise readability and make debugging more challenging.

It introduces a breaking change in the format of the remote because Clap does not accept an enum variant with complex values.

Greptile Summary

Major refactoring of configuration management using Clap for CLI support, introducing layered configuration (default -> file -> args -> env) with automatic CLI and environment variable derivation.

Added magicblock-config-macro crate with procedural macros clap_prefix and clap_from_serde for automatic CLI argument generation
Changed remote configuration format from remote = "devnet" to remote.cluster = "devnet" across all TOML files (breaking change)
Consolidated configuration types from individual crates into centralized magicblock-config crate
Fixed FQDN typo in validator config (renamed from fdqn to fqdn)
Added merge functionality to all config structs for layered configuration support

greptile-apps

_{58 files reviewed, 14 comments}
_{Edit PR Review Bot Settings | Greptile}

greptile-apps · 2025-07-11T06:59:31Z

magicblock-config-macro/README.md

@@ -0,0 +1,74 @@
+# MagicBlock Config Macro
+
+A set a macro helpers to keep the config DRY (Don't Repeat Yourself). It contains two attributes meant to be used on struct that need to derive `serde::Deserialize` and `clap::Args`.


syntax: Grammar error in 'A set a macro helpers' - should be 'A set of macro helpers'

Suggested change

A set a macro helpers to keep the config DRY (Don't Repeat Yourself). It contains two attributes meant to be used on struct that need to derive `serde::Deserialize` and `clap::Args`.

A set of macro helpers to keep the config DRY (Don't Repeat Yourself). It contains two attributes meant to be used on struct that need to derive `serde::Deserialize` and `clap::Args`.

greptile-apps · 2025-07-11T06:59:32Z

magicblock-config-macro/README.md

+    #[serde(default = "helpers::serde_defaults::bool_true")]
+    enabled: bool,
+    #[clap_from_serde_skip]
+    #[serde(default = "helpers::serde_default::url_none")]


syntax: Typo in attribute path: 'serde_default' should be 'serde_defaults' to match the example above

Suggested change

#[serde(default = "helpers::serde_default::url_none")]

#[serde(default = "helpers::serde_defaults::url_none")]

greptile-apps · 2025-07-11T06:59:34Z

magicblock-config/src/ledger.rs

+    #[derive_env_var]
+    #[clap_from_serde_skip]
+    #[arg(help = "Whether to reset the ledger before starting the validator.")]
    #[serde(default = "bool_true")]
    pub reset: bool,


style: reset field has both derive_env_var and clap_from_serde_skip. Inconsistent handling could lead to confusion about whether this is configurable via env vars.

greptile-apps · 2025-07-11T06:59:35Z

magicblock-config/src/accounts_db.rs

+    pub snapshot_frequency: u64,
+}
+
+pub const TEST_SNAPSHOT_FREQUENCY: u64 = 50;


logic: TEST_SNAPSHOT_FREQUENCY used as production default. Should separate test and production constants

greptile-apps · 2025-07-11T06:59:35Z

magicblock-config/tests/read_config.rs

+    env::set_var("REMOTE_WS_URL", base_cluster_ws);
+    let config = MagicBlockConfig::try_parse_config_from_arg(&vec![
+        "--config-file".to_string(),
+        config_file_dir.to_str().unwrap().to_string(),
+    ])
+    .unwrap()
+    .config;


style: Duplicating config loading logic - consider extracting to a helper function

greptile-apps · 2025-07-11T06:59:41Z

magicblock-config/src/rpc.rs

+    #[derive_env_var]
+    #[arg(help = "The address the RPC will listen on.")]
    #[serde(
        default = "default_addr",
        deserialize_with = "deserialize_addr",
        serialize_with = "serialize_addr"
    )]
    pub addr: IpAddr,


logic: max_ws_connections is missing #[derive_env_var] attribute, breaking pattern with other fields and preventing env var configuration

Suggested change

#[derive_env_var]

#[arg(help = "The address the RPC will listen on.")]

#[serde(

default = "default_addr",

deserialize_with = "deserialize_addr",

serialize_with = "serialize_addr"

)]

pub addr: IpAddr,

#[derive_env_var]

#[arg(help = "The address the RPC will listen on.")]

#[serde(

default = "default_addr",

deserialize_with = "deserialize_addr",

serialize_with = "serialize_addr"

)]

pub addr: IpAddr,

#[derive_env_var]

#[arg(help = "The port the RPC will listen on.")]

#[serde(default = "default_port")]

pub port: u16,

#[derive_env_var]

#[arg(help = "The max number of WebSocket connections to accept.")]

#[serde(default = "default_max_ws_connections")]

pub max_ws_connections: usize,

greptile-apps · 2025-07-11T06:59:42Z

magicblock-config/src/rpc.rs

+fn clap_deserialize_addr(s: &str) -> Result<IpAddr, String> {
+    s.parse().map_err(|err| format!("Invalid address: {err}"))
+}


logic: clap_deserialize_addr appears unused - should be referenced by a #[arg(value_parser)] attribute on addr field if intended for CLI parsing

greptile-apps · 2025-07-11T06:59:48Z

magicblock-config/src/lib.rs

+        s.split(':').map(|part| part.to_string()).collect();
+    let [id, path] = parts.as_slice() else {
+        return Err(format!("Invalid program config: {s}"));
+    };


logic: Program config parser assumes exactly 2 parts - doesn't handle paths with colons properly

Suggested change

s.split(':').map(|part| part.to_string()).collect();

let [id, path] = parts.as_slice() else {

return Err(format!("Invalid program config: {s}"));

};

let parts: Vec<String> =

s.splitn(2, ':').map(|part| part.to_string()).collect();

let [id, path] = parts.as_slice() else {

return Err(format!("Invalid program config: {s}"));

};

greptile-apps · 2025-07-11T06:59:50Z

magicblock-config-macro/src/clap_from_serde.rs

+                // Remove the attribute
+                attrs.remove(i);
+                return;


logic: Potential index out of bounds when removing attribute - use drain_filter() or iterate in reverse to safely remove items

greptile-apps · 2025-07-11T06:59:50Z

magicblock-config-macro/src/clap_from_serde.rs

+                            if nv.path.is_ident("default") {
+                                self.meta_attrs
+                                    .push(quote! { default_value_t = #value_token () });
+                            } else if nv.path.is_ident("deserialize_with") {


syntax: Default value has unexpected extra parentheses () which could cause compilation errors

Suggested change

if nv.path.is_ident("default") {

self.meta_attrs

.push(quote! { default_value_t = #value_token () });

} else if nv.path.is_ident("deserialize_with") {

if nv.path.is_ident("default") {

self.meta_attrs

.push(quote! { default_value_t = #value_token });

} else if nv.path.is_ident("deserialize_with") {

bmuddha

Looks legit, cleaner than it was, breaking changes fine for now. Please address the failing integration test, and good to merge.

GabrielePicco

LGTM!

* dev: feat: CLI configuration (#447) Track delegation status (#452) Change account layout (#451)

Dodecahedr0x force-pushed the feat/clap-config branch from 2e0b704 to 41c56b3 Compare July 10, 2025 22:29

Dodecahedr0x requested review from thlorenz, bmuddha and GabrielePicco and removed request for thlorenz and bmuddha July 10, 2025 22:29

Dodecahedr0x marked this pull request as ready for review July 11, 2025 06:59

Dodecahedr0x mentioned this pull request Jul 11, 2025

Show config options #346

Closed

greptile-apps bot reviewed Jul 11, 2025

View reviewed changes

Dodecahedr0x added 6 commits July 11, 2025 16:54

feat(config): create helper macro

b248d5d

feat(config): use clap

d1e1901

fix(config): parsing remote config

bbc522a

fix(config): do not return a result

a7ee5f8

fix(integration): parse remote

aab68c2

feat(config): greptile feedbacks

1058a25

Dodecahedr0x force-pushed the feat/clap-config branch from 7813027 to 1058a25 Compare July 11, 2025 14:55

bmuddha approved these changes Jul 13, 2025

View reviewed changes

Dodecahedr0x added 2 commits July 13, 2025 22:04

fox(config): remote config when parsed from file

90b4255

fix(config): geyser config

c8a3e02

GabrielePicco approved these changes Jul 14, 2025

View reviewed changes

Dodecahedr0x changed the base branch from master to dev July 15, 2025 08:33

Dodecahedr0x merged commit cc63fae into dev Jul 15, 2025
4 checks passed

Dodecahedr0x deleted the feat/clap-config branch July 15, 2025 19:56

thlorenz added a commit that referenced this pull request Jul 17, 2025

Merge branch 'dev' into thlorenz/committor-improve-table-speed

5ed50bd

* dev: feat: CLI configuration (#447) Track delegation status (#452) Change account layout (#451)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: CLI configuration #447

feat: CLI configuration #447

Uh oh!

Dodecahedr0x commented Jul 10, 2025 •

edited by greptile-apps bot

Loading

Uh oh!

greptile-apps bot left a comment

Uh oh!

greptile-apps bot Jul 11, 2025

Uh oh!

greptile-apps bot Jul 11, 2025

Uh oh!

greptile-apps bot Jul 11, 2025

Uh oh!

greptile-apps bot Jul 11, 2025

Uh oh!

greptile-apps bot Jul 11, 2025

Uh oh!

greptile-apps bot Jul 11, 2025

Uh oh!

greptile-apps bot Jul 11, 2025

Uh oh!

greptile-apps bot Jul 11, 2025

Uh oh!

greptile-apps bot Jul 11, 2025

Uh oh!

greptile-apps bot Jul 11, 2025

Uh oh!

bmuddha left a comment

Uh oh!

GabrielePicco left a comment

Uh oh!

Uh oh!

Uh oh!

		@@ -0,0 +1,74 @@
		# MagicBlock Config Macro

		A set a macro helpers to keep the config DRY (Don't Repeat Yourself). It contains two attributes meant to be used on struct that need to derive `serde::Deserialize` and `clap::Args`.

	A set a macro helpers to keep the config DRY (Don't Repeat Yourself). It contains two attributes meant to be used on struct that need to derive `serde::Deserialize` and `clap::Args`.
	A set of macro helpers to keep the config DRY (Don't Repeat Yourself). It contains two attributes meant to be used on struct that need to derive `serde::Deserialize` and `clap::Args`.

	#[serde(default = "helpers::serde_default::url_none")]
	#[serde(default = "helpers::serde_defaults::url_none")]

feat: CLI configuration #447

feat: CLI configuration #447

Uh oh!

Conversation

Dodecahedr0x commented Jul 10, 2025 • edited by greptile-apps bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jul 11, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jul 11, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jul 11, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jul 11, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jul 11, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jul 11, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jul 11, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jul 11, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jul 11, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jul 11, 2025

Choose a reason for hiding this comment

Uh oh!

bmuddha left a comment

Choose a reason for hiding this comment

Uh oh!

GabrielePicco left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Dodecahedr0x commented Jul 10, 2025 •

edited by greptile-apps bot

Loading