Skip to content
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

ISemver policy #13508

Open
JosepBove opened this issue Dec 20, 2024 · 2 comments
Open

ISemver policy #13508

JosepBove opened this issue Dec 20, 2024 · 2 comments
Labels
T-evm-safety Team: EVM Safety

Comments

@JosepBove
Copy link

JosepBove commented Dec 20, 2024

This issue stems from a discussion with @maurelian about whether RISC-V.sol should implement the ISemver interface and the need to clarify and potentially standardize our policy on the use of ISemver across contracts.

The file RISC-V.sol is currently vendored and does not inherit from ISemver. This might be due to its origin as a copy-paste from another repository where ISemver does not exist. While its inclusion could improve consistency, this raises questions about our general approach to vendor contracts and ISemver.

In our discussion, we also noted that ISemver is primarily used to enforce implementation of versioning. However, this practice might be inconsistent with other parts of the codebase.

Questions that raised:

  • Should all on-chain contracts, including vendor contracts like RISC-V.sol, implement ISemver?
  • Should we enforce the ISemver interface only through checks rather than inheritance, especially for vendor contracts?
@maurelian
Copy link
Contributor

If there was only one actionable thing to come out of this, it would be adding a semgrep check to ensure onchain contracts conform to some standard.

@maurelian maurelian added the T-evm-safety Team: EVM Safety label Dec 20, 2024
@tynes
Copy link
Contributor

tynes commented Dec 20, 2024

  • RISCV.sol should implement ISemver. Its vendored from asterisc because we don't know how to work without a monorepo. https://github.com/ethereum-optimism/asterisc
  • Why would RISCV.sol inherit ISystemConfig? You left out context here. Do you mean IRISCV.sol? If so, @smartcontracts @mds1 are the decision makers here. I am of the opinion yes, because it will make writing assertions easier and have less of an impact on compile times (just import the interface rather than the impl to make the calls)
  • What is the benefit of not inheriting ISemver? Seems like extra tooling requirement that we need to maintain rather than just using the compiler itself

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-evm-safety Team: EVM Safety
Projects
None yet
Development

No branches or pull requests

3 participants