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

Refactor Bank EAH fetch method #2477

Merged
merged 1 commit into from
Aug 19, 2024
Merged

Conversation

steviez
Copy link

@steviez steviez commented Aug 7, 2024

Summary of Changes

Shift the logic for checking if the hash should be fetched for this Bank (slot) inside the fetch method

@steviez steviez force-pushed the eah_fetch_refactor branch from 55a9cae to 54f5d80 Compare August 19, 2024 16:55
@steviez steviez changed the title Refactor Bank EAH fetch methods Move EAH should Aug 19, 2024
@steviez steviez changed the title Move EAH should Refactor Bank EAH fetch method Aug 19, 2024
Shift the check for whether *this* bank should include the EAH inside
the fetch function; will avoid code duplication later
@steviez steviez force-pushed the eah_fetch_refactor branch from 54f5d80 to 07ddd83 Compare August 19, 2024 19:31
@steviez steviez requested a review from brooksprumo August 19, 2024 20:44
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:

@@ -5264,9 +5263,13 @@ impl Bank {
self.parent_slot() < stop_slot && self.slot() >= stop_slot
}

/// If the epoch accounts hash should be included in this Bank, then fetch it. If the EAH
/// If the epoch accounts hash should be included in this Bank, then fetch it. If the EAH

Choose a reason for hiding this comment

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

😢 rip double space

Copy link
Author

Choose a reason for hiding this comment

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

I guess that change probably should have been left out, but there are more instances of single-space than double-space in the file. I prefer single-space but don't feel too strongly about it. I do feel more strongly about consistency tho, in which case updating to single-space here gets us closer to uniformity

Given how many times I had to rebase on tip of master + push in order to get CI through, decided to push

@steviez steviez merged commit 7ae496a into anza-xyz:master Aug 19, 2024
40 checks passed
@steviez steviez deleted the eah_fetch_refactor branch August 19, 2024 22:00
ray-kast pushed a commit to abklabs/agave that referenced this pull request Nov 27, 2024
Shift the check for whether *this* bank should include the EAH inside
the fetch function; will avoid code duplication later
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.

2 participants