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

Verify account space specified in annotation #1531

Merged
merged 1 commit into from
Sep 26, 2023

Conversation

LucasSte
Copy link
Contributor

@LucasSte LucasSte commented Sep 14, 2023

On Solana, we can specify the space necessary for a contract's data account in the constructor with an annotation. We do not verify, however, if the declared space is enough to hold all the contract variables. As a consequence, we could call the constructor without any issue using a very small space value and hit other runtime errors later.

PS: I found it unpleasant to keep iterating over the constructor annotations to find the one we were looking for, so I replaced the vector by a struct to hold the permitted annotations.

@LucasSte LucasSte force-pushed the enough-space branch 3 times, most recently from b439ad4 to 9ba34f4 Compare September 15, 2023 19:20
@LucasSte LucasSte marked this pull request as ready for review September 15, 2023 19:26
@LucasSte LucasSte added the solana The Solana target label Sep 15, 2023
src/codegen/mod.rs Outdated Show resolved Hide resolved
src/sema/ast.rs Outdated Show resolved Hide resolved
src/sema/ast.rs Outdated Show resolved Hide resolved
src/codegen/solana_deploy.rs Outdated Show resolved Hide resolved
src/codegen/mod.rs Outdated Show resolved Hide resolved
src/codegen/mod.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@xermicus xermicus left a comment

Choose a reason for hiding this comment

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

LGTM

@LucasSte LucasSte merged commit 614d413 into hyperledger-solang:main Sep 26, 2023
16 checks passed
@LucasSte LucasSte deleted the enough-space branch September 26, 2023 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
solana The Solana target
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants