-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
feat: add TreeConfig #9833
feat: add TreeConfig #9833
Conversation
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.
one suggestion
and we can re-export that type from etherem-engine like we did with other types already
reth/crates/ethereum/engine/src/service.rs
Lines 13 to 16 in cf7698a
pub use reth_engine_tree::{ | |
chain::{ChainEvent, ChainOrchestrator}, | |
engine::EngineApiEvent, | |
}; |
/// Engine tree configuration builder. | ||
#[derive(Debug, Default)] | ||
pub struct TreeConfigBuilder { | ||
persistence_threshold: Option<u64>, | ||
block_buffer_limit: Option<u32>, | ||
max_invalid_header_cache_length: Option<u32>, | ||
} |
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.
I think the builder type is redundant here if we move the with setters to Treeconfig directly so you can configure it like TreeConfig::default().with_().with()
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.
makes sense, done
/// Maximum number of blocks to be kept only in memory without triggering persistence. | ||
const PERSISTENCE_THRESHOLD: u64 = 256; | ||
/// Number of pending blocks that cannot be executed due to missing parent and | ||
/// are kept in cache. | ||
const DEFAULT_BLOCK_BUFFER_LIMIT: u32 = 256; | ||
/// Number of invalid headers to keep in cache. | ||
const DEFAULT_MAX_INVALID_HEADER_CACHE_LENGTH: u32 = 256; |
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.
lets keep the constants but move to 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.
very nice
Closes #9821
The configuration is done at the launcher level (which adds a new dep to
reth-ethereum-node
), we could do it at the service level too, let me know wdyt