-
Notifications
You must be signed in to change notification settings - Fork 310
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
Move code reference to message #705
base: master
Are you sure you want to change the base?
Conversation
c3499c2
to
a1df1d1
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #705 +/- ##
=======================================
Coverage 93.49% 93.50%
=======================================
Files 25 25
Lines 3861 3865 +4
Branches 396 396
=======================================
+ Hits 3610 3614 +4
Misses 139 139
Partials 112 112 |
@@ -745,13 +739,9 @@ class VM | |||
/// but without providing the Host context and interface. |
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.
Update comment above
) -> ::evmc_vm::ffi::evmc_result | ||
{ | ||
use evmc_vm::EvmcVm; | ||
|
||
// TODO: context is optional in case of the "precompiles" capability |
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.
why is this removed?
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 wasn't able to translate the check easily and also not sure if this check isn't to heavy to be included in the bindings (unless this helps Rust translation somehow).
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.
Maybe TODO
is worth to leave? It seems to point to something that is not yet implemented in bindings, but might be useful.
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 don't think the check for instance/msg null should be removed. This is the place it ensures it won't be null.
No description provided.