Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion crates/node/builder/src/launch/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,7 @@ impl<R, ChainSpec: EthChainSpec> LaunchContextWith<Attached<WithConfigs<ChainSpe
}

/// Returns the configured [`PruneConfig`]
///
/// Any configuration set in CLI will take precedence over those set in toml
pub fn prune_config(&self) -> Option<PruneConfig> {
let Some(mut node_prune_config) = self.node_config().prune_config() else {
Expand Down Expand Up @@ -1086,7 +1087,7 @@ mod tests {
storage_history_full: false,
storage_history_distance: None,
storage_history_before: None,
receipts_log_filter: vec![],
receipts_log_filter: None,
},
..NodeConfig::test()
};
Expand Down
62 changes: 38 additions & 24 deletions crates/node/core/src/args/pruning.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,12 @@ pub struct PruningArgs {
/// Prune receipts before the specified block number. The specified block number is not pruned.
#[arg(long = "prune.receipts.before", value_name = "BLOCK_NUMBER", conflicts_with_all = &["receipts_full", "receipts_distance"])]
pub receipts_before: Option<BlockNumber>,
// Receipts Log Filter
/// Configure receipts log filter. Format:
/// <`address`>:<`prune_mode`>[,<`address`>:<`prune_mode`>...] Where <`prune_mode`> can be
/// 'full', 'distance:<`blocks`>', or 'before:<`block_number`>'
#[arg(long = "prune.receiptslogfilter", value_name = "FILTER_CONFIG", conflicts_with_all = &["receipts_full", "receipts_distance", "receipts_before"], value_parser = parse_receipts_log_filter)]
pub receipts_log_filter: Option<ReceiptsLogPruneConfig>,

// Account History
/// Prunes all account history.
Expand All @@ -81,18 +87,11 @@ pub struct PruningArgs {
/// pruned.
#[arg(long = "prune.storagehistory.before", value_name = "BLOCK_NUMBER", conflicts_with_all = &["storage_history_full", "storage_history_distance"])]
pub storage_history_before: Option<BlockNumber>,

// Receipts Log Filter
/// Configure receipts log filter. Format:
/// <`address`>:<`prune_mode`>[,<`address`>:<`prune_mode`>...] Where <`prune_mode`> can be
/// 'full', 'distance:<`blocks`>', or 'before:<`block_number`>'
#[arg(long = "prune.receiptslogfilter", value_name = "FILTER_CONFIG", value_delimiter = ',', value_parser = parse_receipts_log_filter)]
pub receipts_log_filter: Vec<String>,
}

impl PruningArgs {
/// Returns pruning configuration.
pub fn prune_config(&self, chain_spec: &impl EthChainSpec) -> Option<PruneConfig> {
pub fn prune_config(&self, _chain_spec: &impl EthChainSpec) -> Option<PruneConfig> {
// Initialise with a default prune configuration.
let mut config = PruneConfig::default();

Expand All @@ -103,21 +102,10 @@ impl PruningArgs {
segments: PruneModes {
sender_recovery: Some(PruneMode::Full),
transaction_lookup: None,
// prune all receipts if chain doesn't have deposit contract specified in chain
// spec
receipts: chain_spec
.deposit_contract()
.map(|contract| PruneMode::Before(contract.block))
.or(Some(PruneMode::Distance(MINIMUM_PRUNING_DISTANCE))),
receipts: Some(PruneMode::Distance(MINIMUM_PRUNING_DISTANCE)),
account_history: Some(PruneMode::Distance(MINIMUM_PRUNING_DISTANCE)),
storage_history: Some(PruneMode::Distance(MINIMUM_PRUNING_DISTANCE)),
receipts_log_filter: ReceiptsLogPruneConfig(
chain_spec
.deposit_contract()
.map(|contract| (contract.address, PruneMode::Before(contract.block)))
.into_iter()
.collect(),
),
receipts_log_filter: Default::default(),
},
}
}
Expand All @@ -141,9 +129,18 @@ impl PruningArgs {
if let Some(mode) = self.storage_history_prune_mode() {
config.segments.storage_history = Some(mode);
}
if let Some(receipt_logs) =
self.receipts_log_filter.as_ref().filter(|c| !c.is_empty()).cloned()
{
config.segments.receipts_log_filter = receipt_logs;
// need to remove the receipts segment filter entirely because that takes precendence
// over the logs filter
config.segments.receipts.take();
}

Some(config)
}

const fn sender_recovery_prune_mode(&self) -> Option<PruneMode> {
if self.sender_recovery_full {
Some(PruneMode::Full)
Expand Down Expand Up @@ -205,6 +202,7 @@ impl PruningArgs {
}
}

/// Parses `,` separated pruning info into [`ReceiptsLogPruneConfig`].
pub(crate) fn parse_receipts_log_filter(
value: &str,
) -> Result<ReceiptsLogPruneConfig, ReceiptsLogError> {
Expand Down Expand Up @@ -236,9 +234,8 @@ pub(crate) fn parse_receipts_log_filter(
if parts.len() < 3 {
return Err(ReceiptsLogError::InvalidFilterFormat(filter.to_string()));
}
let block_number = parts[2]
.parse::<BlockNumber>()
.map_err(ReceiptsLogError::InvalidBlockNumber)?;
let block_number =
parts[2].parse::<u64>().map_err(ReceiptsLogError::InvalidBlockNumber)?;
PruneMode::Before(block_number)
}
_ => return Err(ReceiptsLogError::InvalidPruneMode(parts[1].to_string())),
Expand All @@ -251,6 +248,7 @@ pub(crate) fn parse_receipts_log_filter(
#[cfg(test)]
mod tests {
use super::*;
use alloy_primitives::address;
use clap::Parser;

/// A helper type to parse Args more easily
Expand All @@ -262,6 +260,22 @@ mod tests {

#[test]
fn pruning_args_sanity_check() {
let args = CommandParser::<PruningArgs>::parse_from([
"reth",
"--prune.receiptslogfilter",
"0x0000000000000000000000000000000000000003:before:5000000",
])
.args;
let mut config = ReceiptsLogPruneConfig::default();
config.0.insert(
address!("0x0000000000000000000000000000000000000003"),
PruneMode::Before(5000000),
);
assert_eq!(args.receipts_log_filter, Some(config));
}

#[test]
fn parse_receiptslogfilter() {
let default_args = PruningArgs::default();
let args = CommandParser::<PruningArgs>::parse_from(["reth"]).args;
assert_eq!(args, default_args);
Comment on lines +277 to 280
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test function parse_receiptslogfilter doesn't actually test the parsing functionality as its name suggests. It only verifies default arguments, which is already covered by the previous test. Consider either:

  1. Implementing actual parsing tests that validate parse_receipts_log_filter with various inputs (valid addresses, different prune modes, edge cases)
  2. Renaming this function to better reflect its purpose
  3. Removing it if redundant with the pruning_args_sanity_check test

A comprehensive test would validate multiple address formats, all prune modes, and error handling scenarios.

Suggested change
fn parse_receiptslogfilter() {
let default_args = PruningArgs::default();
let args = CommandParser::<PruningArgs>::parse_from(["reth"]).args;
assert_eq!(args, default_args);
#[test]
fn parse_receiptslogfilter() {
// Test with a single address
let args = CommandParser::<PruningArgs>::parse_from([
"reth",
"--receipts-log-filter",
"0x1234567890123456789012345678901234567890",
])
.args;
assert_eq!(
args.receipts_log_filter,
Some(vec![Address::from_str("0x1234567890123456789012345678901234567890").unwrap()])
);
// Test with multiple addresses
let args = CommandParser::<PruningArgs>::parse_from([
"reth",
"--receipts-log-filter",
"0x1234567890123456789012345678901234567890,0xabcdefabcdefabcdefabcdefabcdefabcdefabcd",
])
.args;
assert_eq!(
args.receipts_log_filter,
Some(vec![
Address::from_str("0x1234567890123456789012345678901234567890").unwrap(),
Address::from_str("0xabcdefabcdefabcdefabcdefabcdefabcdefabcd").unwrap(),
])
);
// Test with different prune mode
let args = CommandParser::<PruningArgs>::parse_from([
"reth",
"--prune",
"receipts",
"--receipts-log-filter",
"0x1234567890123456789012345678901234567890",
])
.args;
assert_eq!(args.prune_mode, Some(PruneMode::receipts()));
assert_eq!(
args.receipts_log_filter,
Some(vec![Address::from_str("0x1234567890123456789012345678901234567890").unwrap()])
);
}

Spotted by Diamond

Is this helpful? React 👍 or 👎 to let us know.

Expand Down
Loading