Skip to content

Removes test_hash_calculation from AccountsBackgroundService handle_snapshot_requests()#6988

Merged
brooksprumo merged 1 commit intoanza-xyz:masterfrom
brooksprumo:lthash/215/test_hash_calculation
Jul 16, 2025
Merged

Removes test_hash_calculation from AccountsBackgroundService handle_snapshot_requests()#6988
brooksprumo merged 1 commit intoanza-xyz:masterfrom
brooksprumo:lthash/215/test_hash_calculation

Conversation

@brooksprumo
Copy link
Copy Markdown

Problem

The test_hash_calculation parameter to AccountsBackgroundService's handle_snapshot_requests() is unused and should be removed.

Summary of Changes

Remove the param, then update all the callers.

@brooksprumo brooksprumo self-assigned this Jul 16, 2025
Comment on lines -604 to -608
request_handlers.handle_snapshot_requests(
false, // test_hash_calculation. Will be removed soon.
non_snapshot_time,
&exit,
)
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Here's the start/main change: test_hash_calculation is always false, so we can remove the param.

&snapshot_root_bank,
snapshot_storages,
status_cache_slot_deltas,
accounts_hash_for_testing,
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

accounts_hash_for_testing is always None because up on line 330 previous_accounts_hash is also always None, because up on line 203 test_hash_calculation is always false. So we remove accounts_hash_for_testing too.

@brooksprumo brooksprumo marked this pull request as ready for review July 16, 2025 02:49
@brooksprumo brooksprumo requested review from HaoranYi and roryharr July 16, 2025 02:49
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.2%. Comparing base (78d5954) to head (f07df07).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6988   +/-   ##
=======================================
  Coverage    83.2%    83.2%           
=======================================
  Files         853      853           
  Lines      375531   375482   -49     
=======================================
- Hits       312586   312566   -20     
+ Misses      62945    62916   -29     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown

@HaoranYi HaoranYi left a comment

Choose a reason for hiding this comment

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

lgtm.

@brooksprumo brooksprumo merged commit 5f75eff into anza-xyz:master Jul 16, 2025
41 checks passed
@brooksprumo brooksprumo deleted the lthash/215/test_hash_calculation branch July 16, 2025 16:31
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.

4 participants