Skip to content

test(random-beacon): remove Waffle before Hardhat 3 - #4305

Merged
piotr-roslaniec merged 4 commits into
devfrom
codex/beacon-remove-waffle
Sep 13, 2026
Merged

piotr-roslaniec merged 4 commits into
devfrom
codex/beacon-remove-waffle

Conversation

@mswilkison

Copy link
Copy Markdown
Contributor

Random Beacon now uses Hardhat Chai Matchers 1.x and Hardhat Network Helpers 1.x on its current Hardhat 2 / ethers v5 stack. Tests use loadFixture and ethers.provider; Waffle, its Ganache dependency tree, and the now-unused ethereumjs-abi resolution are removed. The existing deployment fixtures and test assertions are retained.

This advances #4209 and #4213. It also adds solidity/docs/hardhat-3-migration.md to record the investigation and remaining gates for #4295: the public deployment export chain, proposed dual publication, compatible helper/plugin versions, cross-package script loading, artifact compatibility, and the rollout/validation order. The three tracking issues remain open: ethers v6, the remaining plugin migrations, and the coordinated Hardhat 3/Rocketh conversion are still required.

Stacked on #4304, following the TypeChain/strict/ES2020/types/plugin/lint preparation in #4297–#4304. Coordinate the runtime prerequisite #4205 and CI branch support #4217 with the stack. The maintained deployment-v1 update remains the independent #4294.

Validation on Node 24.11.1: all 955 beacon tests pass with the replacement fixture and assertion libraries; strict typechecking, export compilation, and full lint pass. No tests are removed or newly skipped. The final rebuilt contract artifacts still match all 159 baseline ABIs and executable runtimes after stripping Solidity metadata. ECDSA's 673 tests (44 existing pending), strict checks, export builds, and fresh TypeChain generation passed on the prerequisite stack.

@coderabbitai

coderabbitai Bot commented Sep 8, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 9c24ecbe-cca4-41bb-8ad4-e403c48db3f9

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

…able

PR #4305 introduced @nomicfoundation/hardhat-chai-matchers and
@nomicfoundation/hardhat-network-helpers, both Hardhat 2-only, but the
gates table didn't list either as a future Hardhat 3 migration gate.
Add both to their respective rows.
@piotr-roslaniec

Copy link
Copy Markdown
Collaborator

Independent verification (multi-agent review, not a CI run) of the Waffle -> Hardhat Chai Matchers migration:

Ran yarn build && yarn test in solidity/random-beacon on Node 22.23.1 (matching this repo's CI pin, not the 24.11.1 noted in the PR description) at two points:

  • This branch's HEAD (ec43371ee)
  • The PR's merge-base 3fb39c551 (pre-PR, Waffle intact), in a separate worktree

Result: byte-for-byte identical on both -- 191 passing, 10 failing, same 10 suites (BeaconDkgValidator, RandomBeacon.Authorization/Callback/GroupCreation/Parameters/Relay/Rewards, RandomBeaconGovernance, ReimbursementPool, System e2e), same root cause on every failure: Error: deployed NuCypherToken contract not found from @threshold-network/solidity-contracts/export/deploy/00_resolve_nucypher_token.js (gated on hre.network.tags.allowStubs, unrelated to this PR -- neither branch touches that script or that network-tags wiring).

This is strong evidence the matcher/fixture-loader swap is behavior-preserving for everything this sandbox can execute, and that the 10 failures are a pre-existing environment gap (this sandbox can't resolve/stub NuCypherToken) rather than a regression from this PR. It does not reproduce the full "955 tests pass" claim -- this sandbox lacks whatever this repo's CI provides to satisfy that deploy script (likely network/RPC config unrelated to Waffle). Recommend running the full suite once in an environment matching CI (or landing #4217 so CI can run directly on this branch) before this lands, since a plugin-registration defect in the matcher swap would only be observable at runtime, not via this diff/typecheck.

Full multi-lens review: 2 confirmed P2 findings (this one, now addressed by the parity run above; and a migration-doc gates-table gap, fixed in ec43371), 6 findings raised and independently refuted after adversarial validation.

@piotr-roslaniec

Copy link
Copy Markdown
Collaborator

Follow-up to the verification above: closed the remaining assertion-strength gap empirically.

  • Diffed the sorted list of all 191 passing test titles between the merge-base (Waffle) run and this branch's HEAD (chai-matchers) run: zero drift, exact match -- no test silently changed behavior anywhere in the executable subset.
  • Mutation spot-check on AltBn128.test.ts: temporarily changed one .revertedWith expected string to a wrong value, ran that file alone, confirmed Hardhat Chai Matchers correctly failed with AssertionError: Expected transaction to be reverted with reason '...', but it reverted with reason 'Invalid G1 bytes length', then reverted the change (git checkout --, confirmed clean tree before committing anything else).

This confirms the new matcher still enforces exact-string revert-reason strictness rather than silently degrading -- the one thing static review alone couldn't rule out.

@piotr-roslaniec

Copy link
Copy Markdown
Collaborator

Correction + stronger evidence on the two comments above:

Count correction: I wrote "2 confirmed P2 findings ... 6 findings raised and independently refuted" in the first comment. Post-report review reclassified one of the six refuted findings (the RandomBeacon.Parameters.test.ts:27 shared-fixture-cache observation) from fully-dropped to a P3 informational note, since the refutation itself established a true, diff-attributable fact worth keeping on record (Waffle silently no-op'd a stale snapshot revert there; the new library throws loudly instead — a real behavior change, just not a defect). Corrected tally: 3 confirmed (2×P2 + 1×P3 informational) / 5 refuted, not 2/6.

Stronger mutation evidence: the earlier mutation spot-check used an unrelated wrong string, which fails under both the old and new assertion library and so didn't actually isolate anything. Redid it properly with a strict prefix of the real revert message on AltBn128.test.ts (expected string "Invalid G1 bytes" against the actual "Invalid G1 bytes length"), run against both the merge-base (Waffle) and this branch's HEAD in isolation:

  • Waffle (merge-base 3fb39c551): 21/21 passing — the truncated string silently matched via substring search.
  • Hardhat Chai Matchers (this branch's HEAD): 20 passing / 1 failing — AssertionError: Expected transaction to be reverted with reason 'Invalid G1 bytes', but it reverted with reason 'Invalid G1 bytes length'.

That's the precise strictness change this migration makes: a truncated/prefix revert-reason expectation that would have silently passed under Waffle now correctly fails. Mutation reverted in both worktrees immediately after observing the result; working tree confirmed clean before anything further was committed.

@piotr-roslaniec

Copy link
Copy Markdown
Collaborator

Important scoping correction to the parity/mutation evidence above — it covers less of the diff than it may have read as covering.

Of the 15 test files this PR's diff touches, only 6 (AltBn128, BLS, Groups, Groups.Expiration, Groups.Termination, ModUtils — all using a local/stub deploy) are among the 191 tests that actually executed in any run in this verification. The other 9 (BeaconDkgValidator, RandomBeacon.Callback/GroupCreation/Parameters/Relay/Rewards, RandomBeaconGovernance, ReimbursementPool, system/e2e) all call randomBeaconDeployment()/deployments.fixture() and were blocked at the before/beforeEach hook stage by the pre-existing NuCypherToken environment gap in both the Waffle baseline and this branch's HEAD — their individual test bodies never ran, in either configuration, anywhere in this review.

That specifically includes RandomBeacon.Parameters.test.ts (the shared-fixture-cache file from the P3 note above) and RandomBeacon.GroupCreation.test.ts/RandomBeacon.Relay.test.ts (the double-snapshot-layering pattern raised elsewhere in the review), and it's exactly where any external-library-sourced revert strings (TokenStaking/sortition-pools/OpenZeppelin) would show up if the matcher swap affected them — the one residual risk static review flagged as unable to close.

So: the 191/191 zero-drift result and the prefix-mutation proof ("Invalid G1 bytes" passing under Waffle, correctly failing under Hardhat Chai Matchers) are real and solid evidence for the local-fixture pattern in those 6 files. They are not evidence, one way or the other, for the other 9 — those remain backed only by static review, same as before any of this verification ran. Worth an actual CI/local run against a correctly-configured environment before relying on the 955-test claim for the full suite.

Base automatically changed from codex/solidity-solhint-6 to codex/solidity-prettier-3 September 13, 2026 11:56
Base automatically changed from codex/solidity-prettier-3 to codex/solidity-eslint-10 September 13, 2026 12:00
Base automatically changed from codex/solidity-eslint-10 to codex/beacon-hardhat-verify September 13, 2026 12:12
Base automatically changed from codex/beacon-hardhat-verify to codex/solidity-node24-types September 13, 2026 12:19
Base automatically changed from codex/solidity-node24-types to dev September 13, 2026 16:57
…ffle

# Conflicts:
#	solidity/docs/hardhat-3-migration.md
#	solidity/random-beacon/hardhat.config.ts
#	solidity/random-beacon/package.json
#	solidity/random-beacon/test/BeaconDkgValidator.test.ts
#	solidity/random-beacon/test/RandomBeacon.GroupCreation.test.ts
#	solidity/random-beacon/yarn.lock
@piotr-roslaniec
piotr-roslaniec merged commit 0d82498 into dev Sep 13, 2026
37 checks passed
@piotr-roslaniec
piotr-roslaniec deleted the codex/beacon-remove-waffle branch September 13, 2026 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants