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

Define const fn StakeState::size_of for stake account size #24416

Merged
merged 1 commit into from
Apr 19, 2022

Conversation

jstarry
Copy link
Contributor

@jstarry jstarry commented Apr 16, 2022

Problem

It's not obvious what the enforced size of a stake account is

Summary of Changes

  • Added a new const fn StakeState::serialized_size() to use in place of size_of::<StakeState>()
  • Added a test assertion to make it obvious that 200 is the current size

Fixes #

behzadnouri
behzadnouri previously approved these changes Apr 18, 2022
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 but perhaps a const fn serialized_size in StakeState would be better.
Similar to #24435

@jstarry jstarry force-pushed the add-stake-size-const branch from 3cca739 to dc0cc89 Compare April 19, 2022 09:09
@mergify mergify bot dismissed behzadnouri’s stale review April 19, 2022 09:09

Pull request has been modified.

@jstarry jstarry changed the title Add STAKE_ACCOUNT_SIZE constant Add StakeState::serialized_size and static assertion Apr 19, 2022
@jstarry
Copy link
Contributor Author

jstarry commented Apr 19, 2022

lgtm but perhaps a const fn serialized_size in StakeState would be better.

Yeah, that's better, thanks!

@jstarry jstarry force-pushed the add-stake-size-const branch from dc0cc89 to a283dab Compare April 19, 2022 09:36
@jstarry jstarry changed the title Add StakeState::serialized_size and static assertion Define const fn StakeState::size_of for stake account size Apr 19, 2022
@codecov
Copy link

codecov bot commented Apr 19, 2022

Codecov Report

Merging #24416 (33c048c) into master (3155270) will increase coverage by 0.0%.
The diff coverage is n/a.

❗ Current head 33c048c differs from pull request most recent head a283dab. Consider uploading reports for the commit a283dab to get more accurate results

@@           Coverage Diff           @@
##           master   #24416   +/-   ##
=======================================
  Coverage    70.0%    70.0%           
=======================================
  Files          36       36           
  Lines        2255     2257    +2     
  Branches      322      322           
=======================================
+ Hits         1580     1582    +2     
  Misses        560      560           
  Partials      115      115           

@jstarry jstarry merged commit 2ad1baa into solana-labs:master Apr 19, 2022
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Jun 29, 2022
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Jun 29, 2022
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Jun 30, 2022
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