chore: modify custom components for custom start

[tac0turtle]

Description

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Performance improvement
• Refactoring

Related Issues

Fixes #(issue)

Checklist

• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published

Testing

Additional Notes

[claude]

Claude finished @tac0turtle's task —— View job

---

PR Review in Progress

Reviewing this PR for:

• Code quality and best practices
• Potential bugs or issues
• Performance considerations
• Security concerns
• Test coverage

Code Review Summary

This PR adds support for configurable activation heights for both the base fee redirect and mint precompile features. The implementation is solid overall but I have several recommendations:

✅ Strengths

1. Good Separation of Concerns: The configuration logic is well-separated into its own module (crates/node/src/config.rs)
2. Comprehensive Test Coverage: Excellent test coverage with edge cases like zero addresses, invalid data, and activation height behavior
3. Backwards Compatibility: The changes maintain backwards compatibility with existing configurations
4. Clear APIs: The configuration API is intuitive with methods like base_fee_redirect_settings() and mint_precompile_settings()

⚠️ Issues and Recommendations

Security/Configuration Concerns

1. Field Naming Inconsistency: crates/node/src/config.rs:22 - The field name mintPrecompileAdmin in JSON should be mintAdmin to match the internal struct field:

   #[serde(default, rename = "mintPrecompileAdmin")]  // Should be "mintAdmin"
   pub mint_admin: Option<Address>,

2. Zero Address Handling: The current logic filters out zero addresses for mint admin but not for base fee sink. This creates inconsistency:

   // Lines 57-60: Only mint_admin gets zero-address filtering
   config.mint_admin = extras.mint_admin
       .and_then(|addr| if addr.is_zero() { None } else { Some(addr) });

Code Quality Issues

3. Struct Visibility: crates/node/src/config.rs:6 - ChainspecEvolveConfig should be pub or have explicit visibility since it's used in tests:

   #[derive(Debug, Clone, Serialize, Deserialize, Default)]
   struct ChainspecEvolveConfig {  // Should be pub struct

4. Field Organization: As noted in the review comments, activation heights should be grouped with their corresponding features:

   pub struct EvolvePayloadBuilderConfig {
       pub base_fee_sink: Option<Address>,
       pub base_fee_redirect_activation_height: Option<u64>, // Move next to base_fee_sink
       pub mint_admin: Option<Address>,
       pub mint_precompile_activation_height: Option<u64>,   // Move next to mint_admin
   }

Documentation and Comments

5. Missing Documentation: Key methods lack documentation:

   pub fn base_fee_sink_for_block(&self, block_number: u64) -> Option<Address> {
       // Add docs explaining this checks activation height

6. JSON Configuration Discrepancy: The genesis file uses mintPrecompileAdmin but the code expects mintAdmin based on the serde rename.

📋 Minor Improvements

1. Test Organization: Consider extracting the ADMIN_PROXY_RUNTIME constant to a shared test utility
2. Error Messages: The ConfigError::InvalidConfig variant is unused and could be removed
3. Validation: The validate() method is currently a no-op but could validate activation height constraints

🔍 Genesis File Review

The etc/ev-reth-genesis.json configuration looks correct, but the field name mintPrecompileAdmin doesn't match what the deserializer expects (mintAdmin based on the code).

✅ Security Assessment

• No security vulnerabilities identified
• Proper input validation for addresses
• Safe defaults with activation heights defaulting to 0
• Zero address handling prevents accidental misconfigurations

✅ Performance Assessment

• No performance concerns - configuration is loaded once at startup
• Efficient block-height checks with simple integer comparisons
• Minimal memory overhead for the configuration data

✅ Overall Assessment

This is a well-structured PR that successfully implements configurable activation heights for custom components. The main issues are related to field naming consistency and code organization rather than functional problems.

Recommendation: Address the field naming inconsistency and consider the organizational improvements, but this is otherwise ready for merge after those fixes.

[damiannolan]

Is it such that the base fee sink and mint precompile features are both to be included in the same "upgrade" ?

If you wanted to align with eth naming convention you could name the upgrade and give it the config upgradeNameBlock: {block_number} e.g. evolveBlock: 100000

Also fine to keep using the activation height convention.

PR seems pretty clear to follow. Nice job

[damiannolan]

Does this also need to include something about mint admin? or is it completely optional?

...opt right

If no mint admin is specified, the precompile is still available but will reject all calls.