Skip to content
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

Remove reclaim param for store_accounts_frozen #618

Merged

Conversation

HaoranYi
Copy link

@HaoranYi HaoranYi commented Apr 6, 2024

Problem

Follow up #581.

store_frozen_account should never populate reclaims.

Summary of Changes

Remove reclaim param from store_frozen_account.

Fixes #

@HaoranYi HaoranYi changed the title remove reclaim param for store_accounts_frozen Remove reclaim param for store_accounts_frozen Apr 6, 2024
Copy link

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

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

Are there still legitimate, non-test, uses for StoreReclaims::Default? Or can we rip out StoreReclaims entirely?

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.8%. Comparing base (de8e9e6) to head (cc95ceb).
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##           master     #618     +/-   ##
=========================================
- Coverage    81.8%    81.8%   -0.1%     
=========================================
  Files         851      851             
  Lines      230224   230222      -2     
=========================================
- Hits       188527   188525      -2     
  Misses      41697    41697             

@HaoranYi
Copy link
Author

HaoranYi commented Apr 7, 2024

Are there still legitimate, non-test, uses for StoreReclaims::Default? Or can we rip out StoreReclaims entirely?

@brooksprumo
I think so. but it is going to be a bigger PR. We may need to change AccountIndex code too. Need to confirm with @jeffwashington.
It might be better to do that in a different PR? WDYT?

@jeffwashington
Copy link

probably we can rip it out in follow on prs.

Copy link

@jeffwashington jeffwashington left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

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

:shipit:

@HaoranYi HaoranYi merged commit 287d0d2 into anza-xyz:master Apr 8, 2024
38 checks passed
@HaoranYi HaoranYi deleted the accounts-db/refactor_store_account_frozen branch April 8, 2024 19:28
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