-
Notifications
You must be signed in to change notification settings - Fork 34
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
ci: Add Solidity compatibility check #363
Conversation
|
4ffcece
to
d2b3041
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.
This looks great!
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.
Discussed issues with @samuelburnham in DM.
The CI compatibility job needs to be modified - so that it runs compatibility tests in Rust (only for IPA currently), dynamically generates unit tests for Solidity, copies those tests into fetched solidity-verifier directory and finally runs them. Rust <-> Solidity Incompatibility over particular building block can be detected by failure of dynamically generated (by Rust) and executed (in Solidity) unit-test
c3e433c
to
96c8364
Compare
|
966745b
to
f2fd99a
Compare
|
f2fd99a
to
fe1be68
Compare
|
151e8af
to
a4e7369
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.
LGTM
|
004e6fb
to
d8990f4
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.
Excellent! Thanks
Solidity-verifier compatibility
This surfaces issues early and lets authors/reviewers decide how to address breaking changes, solving the CI portion of #337.
Note
An earlier version of this PR did not recommend making this workflow a required status check due to an incorrect design. The current version only errors when the Rust or basic workflow steps fail, which they never should. This would be properly caught by making the workflow a required status check.
Successful runs shown in below comment and issue