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.

Sign up for GitHub

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

Jump to bottom

support full storage key space #100

Merged
merged 6 commits into from
Oct 28, 2024
Merged

support full storage key space #100

merged 6 commits into from
Oct 28, 2024

Conversation

ermalkaleci
Copy link
Contributor

@ermalkaleci ermalkaleci commented Oct 25, 2024 •
edited
Loading

This PR addresses two key issues:

  • The storage pointer values will no longer be truncated to the register size, allowing for the use of arbitrary storage keys, not just those that fit within the register limits.
  • Failed storage value reads will now guarantee to return the zero value on error

xermicus
xermicus reviewed Oct 25, 2024
View reviewed changes
Copy link
Member

@xermicus xermicus left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Very nice catch, thanks!

So this PR fixes two problems: The storage pointer values should not be truncated to the register size so that arbitrary storage keys work, not just ones that fall below our register size. And failed storage value reads now guarantee to return the zero value on error. Could you please add that to the PR description?

crates/llvm-context/src/polkavm/context/mod.rs Show resolved Hide resolved
crates/llvm-context/src/polkavm/context/pointer.rs Outdated Show resolved Hide resolved
crates/llvm-context/src/polkavm/context/pointer.rs Outdated Show resolved Hide resolved
crates/llvm-context/src/polkavm/context/pointer.rs Outdated Show resolved Hide resolved
crates/llvm-context/src/polkavm/context/pointer.rs Outdated Show resolved Hide resolved
@xermicus
Copy link
Member

/tip small

@xermicus
Copy link
Member

Ah okay, of course the tip bot isn't set up for this repo

@ermalkaleci
Copy link
Contributor Author

Very nice catch, thanks!

So this PR fixes two problems: The storage pointer values should not be truncated to the register size so that arbitrary storage keys work, not just ones that fall below our register size. And failed storage value reads now guarantee to return the zero value on error. Could you please add that to the description?

yes sure, I was waiting for you review first :)

xermicus
xermicus approved these changes Oct 26, 2024
View reviewed changes
crates/llvm-context/src/polkavm/context/mod.rs Outdated Show resolved Hide resolved
@xermicus xermicus changed the title fix storage read and write support full storage key space Oct 28, 2024
@xermicus xermicus merged commit aae2510 into paritytech:main Oct 28, 2024
2 checks passed
@ermalkaleci ermalkaleci deleted the fix/storage branch October 28, 2024 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xermicus xermicus xermicus approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ermalkaleci @xermicus