Skip to content

Update more blockstore.rs tests to use merkle shreds#6163

Merged
steviez merged 7 commits intoanza-xyz:masterfrom
alexpyattaev:blockstore_legacy
Jul 24, 2025
Merged

Update more blockstore.rs tests to use merkle shreds#6163
steviez merged 7 commits intoanza-xyz:masterfrom
alexpyattaev:blockstore_legacy

Conversation

@alexpyattaev
Copy link
Copy Markdown

@alexpyattaev alexpyattaev commented May 8, 2025

Problem

Legacy shreds need to go #5982

Summary of Changes

Port more tests in blockstore.rs to merkle

@alexpyattaev alexpyattaev marked this pull request as ready for review May 8, 2025 21:24
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 8, 2025

Codecov Report

❌ Patch coverage is 92.22222% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.2%. Comparing base (72d54da) to head (ecaf48e).
⚠️ Report is 2760 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##           master    #6163     +/-   ##
=========================================
- Coverage    83.3%    83.2%   -0.1%     
=========================================
  Files         854      854             
  Lines      374597   374573     -24     
=========================================
- Hits       312087   311992     -95     
- Misses      62510    62581     +71     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@alexpyattaev alexpyattaev requested a review from steviez May 8, 2025 21:48
Copy link
Copy Markdown

@steviez steviez left a comment

Choose a reason for hiding this comment

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

This one fell off my radar, but two quick comments:

  • Can you give a more descriptive name than "fix more tests". Maybe something like "Update blockstore tests to use merkle shreds"
  • I see you have updated two calls of set_last_in_slot(). It looks like there are 6 in total; want to knock them all out in this PR and then we can tie set_last_in_slot() cleanup in as well

@alexpyattaev alexpyattaev changed the title fix more tests in blockstore.rs Update more blockstore.rs tests to use merkle shreds May 14, 2025
Copy link
Copy Markdown

@steviez steviez left a comment

Choose a reason for hiding this comment

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

Overall looks good, just the minor items I called out. Also, can you please rebase to tip of master ?

Comment thread ledger/src/blockstore.rs Outdated
Comment thread ledger/src/blockstore.rs Outdated
Comment thread ledger/src/blockstore.rs Outdated
Comment thread ledger/src/blockstore.rs Outdated
@alexpyattaev alexpyattaev requested a review from steviez July 23, 2025 14:55
Copy link
Copy Markdown

@steviez steviez left a comment

Choose a reason for hiding this comment

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

LGTM, sorry again for such the long delay in reviewing this but there is some tricky stuff so glad we spent the time to properly dig in.

I'm going to merge this so I can rebase some of the subsequent PR's I have ready

@steviez steviez merged commit 6866e77 into anza-xyz:master Jul 24, 2025
41 checks passed
@alexpyattaev alexpyattaev deleted the blockstore_legacy branch July 24, 2025 10:14
puhtaytow pushed a commit to puhtaytow/agave that referenced this pull request Jul 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants