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

Add size_of const fns for upgradeable loader states #25131

Merged

Conversation

jstarry
Copy link
Contributor

@jstarry jstarry commented May 11, 2022

Problem

The functions for returning the length of various upgradeable loader states are expensive and fallible.

Summary of Changes

Add new const fn methods to make size checks inexpensive and infallible

Fixes #

@jstarry jstarry requested a review from behzadnouri May 11, 2022 10:39
Copy link
Contributor

@behzadnouri behzadnouri left a comment

Choose a reason for hiding this comment

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

lgtm

@@ -17,11 +17,11 @@ pub fn parse_bpf_upgradeable_loader(
UpgradeableLoaderState::Uninitialized => BpfUpgradeableLoaderAccountType::Uninitialized,
UpgradeableLoaderState::Buffer { authority_address } => {
let offset = if authority_address.is_some() {
UpgradeableLoaderState::buffer_data_offset().unwrap()
Copy link
Contributor

Choose a reason for hiding this comment

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

somehow if possible, keeping the offset terminology would probably be more readable at call-sites.
(i.e. not immediately clear why the caller should care about metadata or size of structs here)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, you're definitely right. I think I would like to abstract this all away from the decoder in a follow up PR.

@jstarry jstarry merged commit 6880098 into solana-labs:master May 11, 2022
@jstarry jstarry deleted the refactor/upgradeable-size-of-fns branch May 11, 2022 14:23
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