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

fix: recursively size variable size structs in your buffer layouts #27624

Merged
merged 1 commit into from
Sep 7, 2022
Merged

fix: recursively size variable size structs in your buffer layouts #27624

merged 1 commit into from
Sep 7, 2022

Conversation

steveluscher
Copy link
Contributor

Problem

Imagine you have a data structure like this:

Layout.structure([
  u8('instruction'),
  Layout.structure([
    rustString('variableSizeString'),
  ]);
]);

The present implementation of getAlloc will not dive into that inner structure and size it, so the total size of this entire layout will – incorrectly – be 1.

Summary of Changes

  • When getAlloc encounters a structure it recurses into it to obtain its size.

@@ -152,6 +152,9 @@ export function getAlloc(type: any, fields: any): number {
if (Array.isArray(field)) {
return field.length * getItemAlloc(item.elementLayout);
}
} else if ('fields' in item) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will match both Structure and BitStructure objects, but I did not test it with BitStructure. I presume that BitStructure objects can't be of variable length, so they will match the if (item.span >= 0) check above.

@steveluscher
Copy link
Contributor Author

steveluscher commented Sep 6, 2022

I encountered this bug while creating #27627.

@codecov
Copy link

codecov bot commented Sep 6, 2022

Codecov Report

Merging #27624 (1d37805) into master (e779032) will decrease coverage by 0.3%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master   #27624     +/-   ##
=========================================
- Coverage    76.9%    76.6%   -0.4%     
=========================================
  Files          48       52      +4     
  Lines        2505     2659    +154     
  Branches      355      366     +11     
=========================================
+ Hits         1927     2037    +110     
- Misses        448      488     +40     
- Partials      130      134      +4     

@steveluscher steveluscher merged commit a94ada8 into solana-labs:master Sep 7, 2022
@steveluscher steveluscher deleted the recursively-size-structs-buffer-layout branch September 7, 2022 05:37
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.

None yet

1 participant