Skip to content

Conversation

@CriesofCarrots
Copy link

@CriesofCarrots CriesofCarrots commented Apr 2, 2024

Problem

Code for partitioned epoch rewards is peppered throughout bank.rs. It is very hard to trace the flows, and figure out which structs and methods are needed for calculation vs distribution, or for sysvar handling.

Summary of Changes

Moves epoch_rewards_hasher runtime functionality and associated unit tests into partitioned_epoch_rewards submodule
Moves integration-like tests into submodule
Moves (test-only) compare functionality into submodule

Best reviewed by commit

This is the last PR refactoring this section of Bank code.

Closes #510

@codecov-commenter
Copy link

codecov-commenter commented Apr 3, 2024

Codecov Report

Attention: Patch coverage is 99.04306% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 81.8%. Comparing base (f6e02e6) to head (d2a9830).

Additional details and impacted files
@@            Coverage Diff            @@
##           master     #553     +/-   ##
=========================================
- Coverage    81.8%    81.8%   -0.1%     
=========================================
  Files         848      849      +1     
  Lines      229160   229163      +3     
=========================================
- Hits       187547   187537     -10     
- Misses      41613    41626     +13     

@CriesofCarrots CriesofCarrots added the noCI Suppress CI on this Pull Request label Apr 3, 2024
@CriesofCarrots CriesofCarrots removed the noCI Suppress CI on this Pull Request label Apr 3, 2024
@CriesofCarrots CriesofCarrots requested a review from joncinque April 3, 2024 20:02
@CriesofCarrots
Copy link
Author

There are a few distinct pieces to this one. I lumped them together because they are all relatively small, but let me know if you prefer this to be split up.

@CriesofCarrots
Copy link
Author

CriesofCarrots commented Apr 3, 2024

Rebased to pick up cargo audit fix
(edit) Hmm... github isn't requiring me to get this re-reviewed. Is it smart enough to determine nothing in the diff has changed? Or are branch-protection rules broken?

@joncinque
Copy link

It looks like the rebase didn't dismiss my review, which is... good?

@CriesofCarrots
Copy link
Author

It looks like the rebase didn't dismiss my review, which is... good?

🤷‍♀️ It is definitely unexpected to me! I suppose it's convenient, though.

@CriesofCarrots CriesofCarrots added the automerge automerge Merge this Pull Request automatically once CI passes label Apr 3, 2024
@mergify mergify bot merged commit f4307ad into anza-xyz:master Apr 3, 2024
OliverNChalk pushed a commit to OliverNChalk/agave that referenced this pull request Nov 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge automerge Merge this Pull Request automatically once CI passes

Projects

No open projects

Development

Successfully merging this pull request may close these issues.

3 participants