Enhance Executive Audit Validations#338
Conversation
|
A streamlined Executive Audit Checklist that includes all of these same validations can be found here. I can include this exact checklist template into this document, if the team feels this is appropriate. Otherwise, I could also link to this document. I have not updated names to use the new branding, as I assume this will be done as a separate piece of work. |
| The solidity compiler optimizations should be `No with 200 runs`. You can view this on Etherscan under "Contract > Optimizations Enabled". This is to avoid solidity compiler exploits which can be triggered using only a specific number of runs. The only exception to this rule is for very large spells the contract size may be prohibitive and require optimizations to be enabled. In this case, the choice of the number of runs should be an expected value such as `1` or `100`. | ||
|
|
||
| ### Validate the EVM version and license file | ||
| The solidity compiler settings should be `default evmVersion`. You can view this on Etherscan under "Contract > Other Settings." If this is not the case, there should be an explanation as to why. |
There was a problem hiding this comment.
I have a doubt regarding this part, I understand the point, but I have seen some spells with the compiler setting London EvmVersion. I'm not sure if the developers already allow that variation. We should check on it.
There was a problem hiding this comment.
Thank you for checking. There is existing executive audit documentation that requires default, but about 50% of the spells I've validated so far have either default or london. It would be good to reach a resolution on the policy, so we can either update the documentation or require it in the development process.
There was a problem hiding this comment.
I agree, I will discuss it with the team to gather other opinions and will let you know.
| ### Validate the solc compiler version matches `spells-mainnet` | ||
| The compiler version for the deployed spell should match the version specified on that spell's branch in the `spells-mainnet` repo. This is defined at the top of the `Makefile` under `--use solc:0.8.16 build`. The commit hash in the version does not need to be validated. | ||
|
|
||
| ### Spell actions validations and provenance |
There was a problem hiding this comment.
This could perhaps be added to Spell Actions since it is part of the exhaustive control. What do you think?
There was a problem hiding this comment.
As in, move it out of the Non-Exhaustive Checklist sectoin, and have it be right before that section (and after Office Hours?)
There was a problem hiding this comment.
It could be in the place you mentioned, or even within the same Spell Actions section, at the end of it, since we are still following the actions.
Co-authored-by: boet <166723020+boet1@users.noreply.github.com>
Co-authored-by: boet <166723020+boet1@users.noreply.github.com>
Co-authored-by: boet <166723020+boet1@users.noreply.github.com>
Co-authored-by: boet <166723020+boet1@users.noreply.github.com>
Co-authored-by: boet <166723020+boet1@users.noreply.github.com>
2 participants
This PR updates the executive audit validations to include additional validations that are listed in supplementary AD material. It also makes some minor spelling edits, and updates references to
changelogto usechainlog.