-
Notifications
You must be signed in to change notification settings - Fork 61
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
SHL, SHR, SAR, and related tests #352
Conversation
src/patch/mod.rs
Outdated
@@ -177,6 +181,7 @@ impl Patch for EmbeddedByzantiumPatch { | |||
fn has_static_call() -> bool { true } | |||
fn has_revert() -> bool { true } | |||
fn has_return_data() -> bool { true } | |||
fn has_bitwise_shift() -> bool { false } |
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 Patch should be EmbeddedConstantinoplePatch
, or?
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.
Oh. Dunno what I did, but my comment just got removed.
Patch name is OK here, but I gotta add an EmbeddedConstantinoplePatch
indeed
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.
Actually decided to remove the EmbeddedByzantiumPatch
: #353
Oops, forgot to update the |
bdfd39b
to
90dba02
Compare
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.
@mersinvald , can you please fix CI builds
638d4cc
to
a1136d5
Compare
@r8d8 working on it |
8c9bf35
to
b933b0b
Compare
a66bb33
to
0395632
Compare
This PR adds related Constantinople opcodes.
Issue: #350
Main question is how do we approach merging that?
Feature-by-feature or one big PR with all Constantinople changes included?
@whilei
Closes #350