-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(cli): set receipts pruning for chain without deposit contract #9782
Conversation
let config = self.node_config().prune_config(); | ||
if config.is_some() { | ||
return config | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this an unrelated fix attempt that ranks cli settings higher than what's in the toml?
I don't think we need to do this because we update the toml settings
reth/crates/node/builder/src/launch/common.rs
Lines 123 to 128 in d395df2
/// Save prune config to the toml file if node is a full node. | |
fn save_pruning_config_if_full_node( | |
reth_config: &mut reth_config::Config, | |
config: &NodeConfig, | |
config_path: impl AsRef<std::path::Path>, | |
) -> eyre::Result<()> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not unrelated, blocks syncing op mainnet full node. see docs for PruningArgs
flags.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will ignore all toml settings which I think is incorrect because this always falls back to the default args if run with --full, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah but that's the usual behaviour for any linux tool: cli args have precedence over files. and we have shipped this help for cli --full
flag since at least January
reth/crates/node/core/src/args/pruning.rs
Lines 12 to 15 in 115a58b
/// Run full node. Only the most recent [`MINIMUM_PRUNING_DISTANCE`] block states are stored. | |
/// This flag takes priority over pruning configuration in reth.toml. | |
#[arg(long, default_value_t = false)] | |
pub full: bool, |
let config = self.node_config().prune_config(); | ||
if config.is_some() { | ||
return config | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please undo then this is g2m
PruneMode::Full
, for chains that don't have deposit contract inChainSpec