-
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
feat(pipeline): prune receipts based on log emitters #4044
Conversation
Codecov Report
... and 9 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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.
nit and question re format
pub fn prune_target_block( | ||
&self, | ||
tip: BlockNumber, | ||
min_blocks: u64, | ||
prune_part: PrunePart, | ||
) -> Result<Option<(BlockNumber, PruneMode)>, PrunePartError> { |
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.
can we add a sanity test for this, code looks correct afaict, just to be sure
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.
LGTM modulo Matt's comments
testing on sepolia by including only receipts from the deposit contract and another random contract |
worked fine. receipt table ended the pipeline with ~15MB keeping all receipts from the last 128 blocks, and further only the ones (2 contracts) that were present in the configuration |
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.
tests look good,
lgtm
This allows us to only include the deposit contract receipts, alongside whatever the user chooses to keep (ref #2753) . (for pipeline only)