-
Notifications
You must be signed in to change notification settings - Fork 100
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
Remove compiled Ethereum contracts from version control and switch to using artifact files #564
Conversation
2721bda
to
42d35a8
Compare
… using artifact files
42d35a8
to
707db3f
Compare
) | ||
let abi, bytecode | ||
if (ethContractArtifactPath) { | ||
({ abi, bytecode } = JSON.parse(fs.readFileSync(ethContractArtifactPath))) |
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.
Are externals parenthesis required here?
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.
Yes, see here.
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.
TIL that solidity compilers guarantees that compilation is deterministic on every platform.
One problem I see with current approach is that it is easier to mess up the right contract to use during deployments and interaction. Because it is possible to have different version on the contract and on the repository at the same time. For properly solving #541 we should publish instead these contracts in this repository github release page, and download it from there. |
I think there is a more general problem. Suppose that we change the API for relayers, and modify the relayers' code accordingly. Now, if someone is using the new relayer code to interact with the old contract (or vise versa), it will break with some confusing error messages. So, just publishing the contracts is not the full solution, we need to add some check to make sure that the contracts the relayers are interacting with are compatible with the relayers' version. |
… using artifact files (#564)
Fixes #491, #555.
Partial fix for #541.