Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

Removes invariant is_serialized_with_abs param#33154

Merged
brooksprumo merged 1 commit intosolana-labs:masterfrom
brooksprumo:pruned-banks-handler/is-serialized-with-abs
Sep 6, 2023
Merged

Removes invariant is_serialized_with_abs param#33154
brooksprumo merged 1 commit intosolana-labs:masterfrom
brooksprumo:pruned-banks-handler/is-serialized-with-abs

Conversation

@brooksprumo
Copy link
Copy Markdown
Contributor

Problem

PrunedBanksRequestHandler::handle_request() takes a parameter, is_serialized_with_abs, that is always called with true. I can't think of a reason why—from this function—we'd want to call AccountsDb::purge_slot() with a different value. So having this parameter seems unnecessary at best, and error-prone at worst.

Summary of Changes

Remove the param.

@brooksprumo brooksprumo self-assigned this Sep 5, 2023
@brooksprumo brooksprumo force-pushed the pruned-banks-handler/is-serialized-with-abs branch from 3cfd172 to bfd15c7 Compare September 6, 2023 01:29
@brooksprumo brooksprumo marked this pull request as ready for review September 6, 2023 15:39
Copy link
Copy Markdown
Contributor

@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
would be useful to look at 1.14 or something and see when it was used and how its use got removed (if ever). maybe that was a mistake.

@brooksprumo
Copy link
Copy Markdown
Contributor Author

would be useful to look at 1.14 or something and see when it was used and how its use got removed (if ever).

Looks like this parameter was added back in 2021 here #17319. It has always been true.

The old handle_pruned_banks() was public, so maybe that was to ensure future-proofing if other callers of this function were added. Now, we only call this function from within ABS (or a bank test) (and I'm removing the pub over here #33155), so we'll know there are no non-ABS callers.

@brooksprumo brooksprumo merged commit 9e156f8 into solana-labs:master Sep 6, 2023
@brooksprumo brooksprumo deleted the pruned-banks-handler/is-serialized-with-abs branch September 6, 2023 16:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants