-
Notifications
You must be signed in to change notification settings - Fork 43
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
Host function protocol gating #1348
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I pushed a couple minor fixes, looks good overall!
clean up link-time vs run-time checks
c2419be
to
2b358ed
Compare
"args": [], | ||
"return": "Val", | ||
"docs": "A dummy function for testing the protocol gating. Takes 0 arguments and performs no-op", | ||
"min_supported_protocol": 19, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this a protocol change? I thought it was only intended for testing. I think it should still be gated by protocol 21, or otherwise we would risk a divergence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This won't have an effect on protocol because no contract can link to it. It's min and max protocol are both specified as 19. So if a contract tries, it will fail the validation check during the upload (same as linking to a non-existent function today).
The reason this has to be 19 (or any value lower than 20) is because the test includes the case where the ledger protocol version > host function max protocol (see test_host_protocol_gating_for_wasm
), and the ledger protocol version cannot be larger than the env version (20, which is compiled in and non-overridable).
Note setting the min_supported_protocol to 21 is using the exact same mechanism for protection against contract built with protocol 20. So effectively this function is always protocol gated out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, that's clever. Thanks for the explanation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I've added a comment to the function docs in #1356
…y` (#1356) ### What Add this [explanation](#1348 (comment)) to the doc. ### Why [TODO: Why this change is being made. Include any context required to understand the why.] ### Known limitations [TODO or N/A]
What
Allow host functions to specify its supported protocol bounds, e.g.
And adds logic (mostly macro generated) that automatically checks and enforces these protocol bounds in various paths:
impl Env for VmCallerEnv
.Tests are added in
tests/protocol_gate.rs
.For more context, visit the design doc