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

Change StorageType to Just Be An Enum #873

Merged
merged 6 commits into from
Jun 22, 2023
Merged

Conversation

graydon
Copy link
Contributor

@graydon graydon commented Jun 22, 2023

This changes StorageType to just be a plain Rust enum (rather than a Val wrapper) in preparation for removal of StorageType from ScVal.

It also includes a change expanding StorageType (the type at the env-interface layer) from 2 cases to 3 -- including ::Instance -- deviating a bit from ContractDataType (the type that controls the storage layer). It includes a projection down to that lower layer type though.

This PR isn't 100% complete, it depends on some more changes incoming at the XDR level -- and it appears it's going to have to integrate a bunch of pending XDR changes from other work -- but I thought I'd post it for examination. I think it's not too bad a solution to the problem. We still get to use StorageType in env.json and we get a strong enum type on both sides of the interface. We just ignore / bypass Val entirely.

@graydon
Copy link
Contributor Author

graydon commented Jun 22, 2023

Corresponding XDR change: stellar/stellar-xdr#118

@graydon graydon requested review from sisuresh and dmkozh June 22, 2023 02:37
@graydon
Copy link
Contributor Author

graydon commented Jun 22, 2023

@dmkozh and/or @sisuresh I ran out of time this evening updating this PR to the changes in the XDR (and the last bunch probably need additional eyes from someone who knows more of what the intended semantic changes are, I was somewhat guessing). Feel free to pick up and extend this with the remainder if you get to it before me, otherwise I'll pick up tomorrow.

@sisuresh sisuresh force-pushed the storagetype-is-just-an-enum branch from d9f9a0d to 253d03a Compare June 22, 2023 17:08
@sisuresh sisuresh marked this pull request as ready for review June 22, 2023 18:30
@sisuresh sisuresh requested a review from a team as a code owner June 22, 2023 18:30
@graydon graydon enabled auto-merge (squash) June 22, 2023 18:36
@graydon graydon merged commit bf09e81 into main Jun 22, 2023
@graydon graydon deleted the storagetype-is-just-an-enum branch June 22, 2023 18:47
sisuresh added a commit to stellar/rs-soroban-sdk that referenced this pull request Jun 22, 2023
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.

3 participants