Skip to content

Commit

Permalink
fix(cli) Allow Pruning CLI Args to take precedence over TOML configur…
Browse files Browse the repository at this point in the history
…ation (#10774)

Co-authored-by: garwah <garwah@garwah>
  • Loading branch information
garwahl and garwah authored Sep 17, 2024
1 parent 1d0b18c commit c795389
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 2 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/config/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,4 @@ eyre.workspace = true
[dev-dependencies]
tempfile.workspace = true
reth-network-peers.workspace = true
reth-primitives.workspace = true
79 changes: 78 additions & 1 deletion crates/config/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,34 @@ impl PruneConfig {
pub fn has_receipts_pruning(&self) -> bool {
self.segments.receipts.is_some() || !self.segments.receipts_log_filter.is_empty()
}

/// Merges another `PruneConfig` into this one, taking values from the other config if and only
/// if the corresponding value in this config is not set.
pub fn merge(&mut self, other: Option<Self>) {
let Some(other) = other else { return };

// Merge block_interval
if self.block_interval == 0 {
self.block_interval = other.block_interval;
}

// Merge the various segment prune modes
self.segments.sender_recovery =
self.segments.sender_recovery.or(other.segments.sender_recovery);
self.segments.transaction_lookup =
self.segments.transaction_lookup.or(other.segments.transaction_lookup);
self.segments.receipts = self.segments.receipts.or(other.segments.receipts);
self.segments.account_history =
self.segments.account_history.or(other.segments.account_history);
self.segments.storage_history =
self.segments.storage_history.or(other.segments.storage_history);

if self.segments.receipts_log_filter.0.is_empty() &&
!other.segments.receipts_log_filter.0.is_empty()
{
self.segments.receipts_log_filter = other.segments.receipts_log_filter;
}
}
}

/// Helper type to support older versions of Duration deserialization.
Expand All @@ -415,8 +443,11 @@ where
#[cfg(test)]
mod tests {
use super::{Config, EXTENSION};
use crate::PruneConfig;
use reth_network_peers::TrustedPeer;
use std::{path::Path, str::FromStr, time::Duration};
use reth_primitives::Address;
use reth_prune_types::{PruneMode, PruneModes, ReceiptsLogPruneConfig};
use std::{collections::BTreeMap, path::Path, str::FromStr, time::Duration};

fn with_tempdir(filename: &str, proc: fn(&std::path::Path)) {
let temp_dir = tempfile::tempdir().unwrap();
Expand Down Expand Up @@ -893,6 +924,52 @@ receipts = 'full'
assert!(err.contains("invalid value: string \"full\""), "{}", err);
}

#[test]
fn test_prune_config_merge() {
let mut config1 = PruneConfig {
block_interval: 5,
segments: PruneModes {
sender_recovery: Some(PruneMode::Full),
transaction_lookup: None,
receipts: Some(PruneMode::Distance(1000)),
account_history: None,
storage_history: Some(PruneMode::Before(5000)),
receipts_log_filter: ReceiptsLogPruneConfig(BTreeMap::from([(
Address::random(),
PruneMode::Full,
)])),
},
};

let config2 = PruneConfig {
block_interval: 10,
segments: PruneModes {
sender_recovery: Some(PruneMode::Distance(500)),
transaction_lookup: Some(PruneMode::Full),
receipts: Some(PruneMode::Full),
account_history: Some(PruneMode::Distance(2000)),
storage_history: Some(PruneMode::Distance(3000)),
receipts_log_filter: ReceiptsLogPruneConfig(BTreeMap::from([
(Address::random(), PruneMode::Distance(1000)),
(Address::random(), PruneMode::Before(2000)),
])),
},
};

let original_filter = config1.segments.receipts_log_filter.clone();
config1.merge(Some(config2));

// Check that the configuration has been merged. Any configuration present in config1
// should not be overwritten by config2
assert_eq!(config1.block_interval, 5);
assert_eq!(config1.segments.sender_recovery, Some(PruneMode::Full));
assert_eq!(config1.segments.transaction_lookup, Some(PruneMode::Full));
assert_eq!(config1.segments.receipts, Some(PruneMode::Distance(1000)));
assert_eq!(config1.segments.account_history, Some(PruneMode::Distance(2000)));
assert_eq!(config1.segments.storage_history, Some(PruneMode::Before(5000)));
assert_eq!(config1.segments.receipts_log_filter, original_filter);
}

#[test]
fn test_conf_trust_nodes_only() {
let trusted_nodes_only = r"#
Expand Down
10 changes: 9 additions & 1 deletion crates/node/builder/src/launch/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,8 +351,16 @@ impl<R> LaunchContextWith<Attached<WithConfigs, R>> {
}

/// Returns the configured [`PruneConfig`]
/// Any configuration set in CLI will take precedence over those set in toml
pub fn prune_config(&self) -> Option<PruneConfig> {
self.toml_config().prune.clone().or_else(|| self.node_config().prune_config())
let Some(mut node_prune_config) = self.node_config().prune_config() else {
// No CLI config is set, use the toml config.
return self.toml_config().prune.clone();
};

// Otherwise, use the CLI configuration and merge with toml config.
node_prune_config.merge(self.toml_config().prune.clone());
Some(node_prune_config)
}

/// Returns the configured [`PruneModes`], returning the default if no config was available.
Expand Down

0 comments on commit c795389

Please sign in to comment.