feat(evm): parity uplift — subgraph mappings, Foundry invariants, slither CI, gas gate - #180
Conversation
…ther CI, gas gate Brings the EVM stack to parity with Stellar: - Full subgraph with AssemblyScript mappings and a subgraph.yaml for all five contracts (event handlers for Announcer/Registry/Names, call handlers for Sender/Withdrawer); graph codegen && graph build pass against the real ABIs. - Foundry invariant suite under evm/foundry covering sender balance conservation, withdrawer atomicity, and name-registration monotonicity (256 runs per invariant), plus a committed gas snapshot with a +5% CI diff gate. - Slither config + CI step enforcing zero High/Medium findings; adds targeted slither-disable comments for intentional patterns and initializes one local. - evm/README.md documenting the four new local commands. Closes wraith-protocol#157
|
@gbengaeben Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
Bumps the resolved @types/node entry so `npm ci` in the CI subgraph step no longer fails out-of-sync validation.
truthixify
left a comment
There was a problem hiding this comment.
@gbengaeben the work here is good and I want to be clear about that before the one thing I am asking you to change.
Stripping out vendored code and lockfiles, you authored 31 files and about 1820 lines, and all of it is the right shape:
- three real invariant suites (sender balance conservation, withdrawer atomicity, name registration monotonicity) rather than unit tests wearing an invariant label
- five AssemblyScript mappings with
networks.jsonindirection, which is what I asked for givenWraithNames,WraithSenderandWraithWithdrawerhave no deployed addresses - slither wired at
fail-on: mediumwith SARIF upload, and the suppressions done as per-siteslither-disable-next-linewith a written justification each, inERC6538Registry.solandWraithWithdrawer.sol. I had suggested config-level suppressions; per-site with a reason is better forincorrect-equalitybecause the justification travels with the code. Good call. - excluding the invariant suites from the gas snapshot because they carry run-to-run
reverts:counters, with a comment explaining why. That is a subtle failure mode to have anticipated. - symlinking
lib/openzeppelintoevm/node_modules/@openzeppelininstead of installing OZ twice. Neat, and correctly ordered afternpm ciin the job.
The change I need: forge-std is vendored, not linked.
evm/foundry/lib/forge-std/ is committed as 68 files and 30266 lines of third-party source, which is 94 percent of this PR. There is no .gitmodules, so it is a plain tree rather than a pinned reference. That lands permanently in every clone, every git grep, and every future diff, and there is no recorded version to audit or bump.
It is also inconsistent with what you did for the other two libs, which are symlinks, so I suspect this was accidental rather than deliberate.
Either fix is fine:
# option A, pinned submodule (my preference, it records a version)
git rm -r --cached evm/foundry/lib/forge-std
rm -rf evm/foundry/lib/forge-std
cd evm/foundry && forge install foundry-rs/forge-std
# then add `submodules: recursive` to the checkout step in the evm job
# option B, install in CI
echo "evm/foundry/lib/forge-std/" >> .gitignore
git rm -r --cached evm/foundry/lib/forge-std
# and run `forge install` before `forge test` in the evm job
A prefers reproducibility, B is less friction for contributors. Your call, just not the current state.
Two smaller notes, neither blocking:
evm/subgraph/package-lock.jsonat 5998 lines is fine to keep, since the subgraph job runsnpm ciandevm/is already npm rather than pnpm. Just confirming that was deliberate and not a straynpm install.- The openzeppelin symlink silently depends on
npm cihaving run inevm/first. It holds in CI because of step ordering, but someone runningforge testin a fresh clone will get a confusing failure. Worth one line inevm/README.md.
Replace the 68-file vendored forge-std tree (~30K lines, unpinned) with a git submodule pinned to v1.16.2, matching the previously vendored version. This records a reviewable and bumpable version, keeps third-party source out of every clone and future diff, and is consistent with the symlinks already used for the openzeppelin and wraith-contracts libraries. Enable recursive submodule checkout in the evm CI job so forge resolves forge-std on a fresh clone. Document in the evm README that forge-std is a submodule and that the openzeppelin symlink requires `npm ci` beforehand.
The forge-std submodule's auxiliary files (CI yaml, CONTRIBUTING.md, JSON fixtures) follow upstream formatting and do not match this repo's prettier config, failing the evm job's `prettier --check` step. Skip the whole foundry/lib directory the same way node_modules already is; dependency code is not formatted to this project's rules. Project-owned sources under evm/contracts are still checked.
|
Merged @gbengaeben. forge-std is now a pinned submodule at v1.16.2 with |
Summary
Closes #157 — brings the EVM side up to parity with the Stellar stack.
What's included
Subgraph (
evm/subgraph/)subgraph.yamlreplacing the instant-config-only setup, withdataSourcesfor all five contracts.evm/subgraph/mappings/: event handlers forERC5564Announcer,ERC6538Registry, andWraithNames; call handlers forWraithSenderandWraithWithdrawer.Announcement,StealthMetaAddress,Name,Send,Withdrawal).graph codegen && graph buildpass against the real contract ABIs. Names/Sender/Withdrawer addresses are placeholders (marked TODO) until those contracts are deployed.Foundry invariant tests (
evm/foundry/)foundry.tomlwith 256 runs per invariant.test/invariant/:evm/foundry/.gas-snapshot; CI runsforge snapshot --check --tolerance 5so any PR increasing gas >5% fails.Slither (
evm/slither.config.json, CI)slither . --config-file slither.config.json --fail-mediumreturns zero High / Medium.slither-disable-next-linecomments where a finding is an intentional pattern (exact zero-balance guard, destructuredtryRecovertuple).CI (
.github/workflows/ci.yml)Docs — new
evm/README.mdcovering the four new local commands (forge invariants, gas snapshot, slither, subgraph codegen/build).Verification (all local)
npx hardhat test— 44 passingforge test --match-path 'test/invariant/*'— 3 suites, 256 runs each, passingforge snapshot --no-match-path 'test/invariant/*' --check --tolerance 5— passingslither . --config-file slither.config.json --fail-medium— 0 High / 0 Mediumgraph codegen && graph build— success