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

Tighten wasm interface version checks, and do on upload. Fix #1052. #1053

Merged
merged 1 commit into from
Sep 11, 2023

Conversation

graydon
Copy link
Contributor

@graydon graydon commented Sep 11, 2023

This fixes #1052 by rejecting contracts with unsupported versions during upload (they are already rejected during instantiation/execution). It also slightly tightens the set of allowed contracts, rejecting old-protocol ones with nonzero prerelease and rejecting all future protocol versions.

@graydon graydon requested review from sisuresh and a team as code owners September 11, 2023 21:05
Copy link
Member

@leighmcculloch leighmcculloch left a comment

Choose a reason for hiding this comment

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

One concern about gating upload.

soroban-env-host/src/vm.rs Show resolved Hide resolved
soroban-env-host/src/vm.rs Show resolved Hide resolved
Copy link
Member

@leighmcculloch leighmcculloch left a comment

Choose a reason for hiding this comment

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

RE: #1053 (comment). I don't think we should do the verify the wasm file during upload for the reasons I mention in that thread, because tooling can easily do that and it doesn't have to be something built into the protocol.

But it sounds like that's a decision already agreed on and doesn't need to block this, so approving.

@graydon graydon added this pull request to the merge queue Sep 11, 2023
Merged via the queue into main with commit 494ce9f Sep 11, 2023
9 checks passed
@graydon graydon deleted the tighten-version-checks branch September 11, 2023 23:12
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.

do pre_release number check on upload / RPC
3 participants