build(solidity)!: remove @defi-wonderland/smock - #4203
Merged
piotr-roslaniec merged 4 commits intoJul 27, 2026
Merged
Conversation
Adds `contracts/test/MockContract.sol` and `test/helpers/mock.ts` to both packages, with their own self-tests. Nothing uses them yet -- smock is still installed and every existing call site is untouched, so this changes no behaviour. The removal is the next commit. Ported from threshold-network/tbtc-v2#1062, where it has since replaced 554 smock call sites and runs green on hardhat 2.29 and Node 24. It carries the fixes that migration surfaced, including two found only last week: argument comparison by value rather than representation, and relocating an address-pinned mock before configuring it rather than after, since `hardhat_setCode` copies code and not storage. smock configured its fakes by mutating in-process JavaScript state inside Hardhat's EVM, which is why its API was synchronous -- and why it breaks on every hardhat past 2.19. This drives an ordinary deployed contract over the public provider API, so it depends on nothing Hardhat can change underneath it. The one behavioural difference is that configuration is a transaction and must be awaited, which is what makes the next commit touch call sites at all. `allowBlocksWithSameTimestamp` is set in both configs because of that: a transaction mines a block, and Hardhat otherwise forces each block at least a second after its parent, so configuring a mock would silently advance the chain clock. The helper pins the next block's timestamp before every write; this option is what lets it reuse the current one. Verified in both packages -- 22 self-tests each, covering the zero-value return layer, calldata-specific answers, reverts, call recording, `msg.value`, the STATICCALL restriction on recording, and address pinning: ecdsa 22 passing random-beacon 22 passing Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four files: the shared `fakeTokenStaking` helper and the three suites that use it. `smock.fake` becomes `createMock`, `FakeContract` becomes `Mock`, and the `reverts` configuration is awaited, since configuring this mock is a transaction rather than a JavaScript assignment. The interesting part is what came back. Three contexts were parked as `context.skip` behind // FIXME: Blocked by wonderland-archive/smock#101 so the "token staking seize call fails" paths -- the ones asserting that a failing `seize` does not take the whole operation down with it -- had not run in years. They are enabled here and pass against `MockContract`, which is the point of replacing smock rather than merely pinning it. Also drops three `tokenStakingFake.seize.reset()` teardowns. Each ran *after* `restoreSnapshot()`, and the mock is created inside that snapshot, so the reset was operating on state where the mock no longer existed. Under smock it was a synchronous no-op; here it would be an unawaited transaction. before 926 passing / 6 pending / 0 failing after 954 passing / 0 pending / 0 failing 954 = 926 + 22 mock self-tests + the 6 recovered. No test file loses coverage and nothing is skipped any more. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sixteen files. `smock.fake` becomes `createMock`, `FakeContract` becomes `Mock`, `chai.use(smock.matchers)` goes, and the chai matchers become explicit helper calls -- `to.have.been.called` and the `to.be.calledWith` spelling both appear here, so both forms are rewritten. Configuration is awaited throughout, since it is a transaction now rather than a JavaScript assignment. before 650 passing / 44 pending / 0 failing after 673 passing / 44 pending / 0 failing 673 = 650 + the 23 mock self-tests. No suite loses a test. Four things needed judgement rather than a mechanical rewrite. `expectCalledWith` is new, added to the helper here. smock's `calledWith` says nothing about how many times a function ran, so translating those two sites to the existing `expectCalledOnceWith` would have quietly added an assertion the tests never made. It has its own self-test, including the negative case. Five assertions in the TIP-092 migration suite checked that `IStaking.authorizedStake` had been called. It is `view`, so Solidity reaches it by STATICCALL, where recording is impossible -- the mock refuses the assertion loudly rather than answering zero. Four of the five already asserted the value the mock returns, which is the same proof by a stronger route, so the call check is dropped with a comment saying why. The fifth, `updateOperatorStatus`, had no other assertion at all; it now asserts the operator is up to date, which is only true if the registry read the allowlist and synced the pool to it. `smock.mock` at WalletRegistry.RandomBeacon.test.ts:236 was never a mock in any meaningful sense -- it deployed `RandomBeaconStub` and only ever read `.address`, with no `setVariable` or `getVariable` anywhere. It is now a plain factory deploy. `resetMock` is gone. Both callers were smock-era bookkeeping that became harmful once resetting meant sending a transaction. In the RandomBeacon suite it ran *after* `restoreSnapshot()`, so it was both redundant and, left unawaited, landed in the following context and wiped the call log that context asserted on. In `createNewWallet` it mined a block in the middle of a shared helper, which shifted `WalletRegistry - Wallets`'s cumulative state enough to flip an `isWalletMember` assertion in the full run while passing in isolation. Nothing depends on either reset; the suite is green without them. One gas budget moved. `approveDkgResult` reaches the wallet owner, and recording a call SSTOREs its calldata -- 447k against a 274k +/- 1000 budget. Switching recording off for that suite's wallet owner, which nothing in the file inspects, brings it to ~286k. The remaining ~12k is `MockContract`'s fallback dispatch, real work that smock's fake did not do, so the expectation moves 274 000 -> 286 000 and the tolerance stays. In production the wallet owner is a real contract and costs more than either number. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The dependency is gone from both packages. Upstream archived it with:⚠️ DEPRECATED – DO NOT USE It may contain outdated, insecure, or vulnerable code and should not be used in production or as a dependency in any project. No support, updates, or security patches will be provided. npm carries the matching deprecation and the repository is archived. That is the reason this work happened. The toolchain ceiling was only what made it unavoidable rather than schedulable: no smock version reaches hardhat 2.26+, and hardhat 2.26+ is what Node 24 requires, so this also unblocks that bump. Verified with smock fully uninstalled -- not merely unused: random-beacon 955 passing / 0 pending / 0 failing ecdsa 673 passing / 44 pending / 0 failing Against the pre-migration baselines of 926/6/0 and 650/44/0. The increases are the 23 mock self-tests in each package and the six tests in random-beacon that smock's issue #101 had kept behind `context.skip` since they were written. BREAKING CHANGE: `@defi-wonderland/smock` is no longer a dependency. Any out-of-tree test importing it will fail to resolve. Use `createMock` from `test/helpers/mock` instead; the API is the same shape except that configuration returns a promise and must be awaited. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
mswilkison
force-pushed
the
chore/solidity-remove-smock
branch
from
July 27, 2026 16:15
65bb0b2 to
d925302
Compare
mswilkison
added a commit
that referenced
this pull request
Jul 27, 2026
The end of the chain, and the reason for it. Five workflows, two Dockerfiles and two `engines` fields go from Node 18.15.0 -- end of life since 2025-04-30 -- straight to 24.11.1, the current LTS, supported until 2028-04-30. This was not reachable before. Node 24 needs hardhat >= 2.26, because below it Hardhat's solc download fails with HH502: npm's undici and Node 24's bundled undici share a global-dispatcher symbol, so the download reaches Node's internal dispatcher, which rejects `maxRedirections`. hardhat >= 2.21 in turn broke @defi-wonderland/smock, and no smock version ever reached 2.26. The dependency was archived, so no version ever would. That is why #4203 removed it and #4204 raised hardhat before this commit could exist. The failure that started it, now passing on the same machine and the same runtime -- cold compiler cache, which is what a CI runner always has: before Error HH502: Couldn't download compiler versions list. after Compiled 75 Solidity files successfully Verified, whole suite, both packages, on Node 24.11.1: ecdsa 673 passing / 44 pending / 0 failing random-beacon 955 passing / 0 pending / 0 failing Docker base images are pinned to `node:24.11.1-alpine` rather than the floating `node:24-alpine`, matching the exact version CI pins, so the two runtimes cannot drift apart. This supersedes #4201, which moved the same nine files from 18 to 22 while 24 was still out of reach. If that lands first this needs a one-token rebase per site; if this chain lands, #4201 can be closed. BREAKING CHANGE: the solidity packages now require Node >= 24.0.0. Contributors on Node 18 or 20 will need to upgrade. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
piotr-roslaniec
merged commit Jul 27, 2026
a1178f0
into
chore/solidity-hardhat-2.19-smock-pin
16 checks passed
piotr-roslaniec
added a commit
that referenced
this pull request
Jul 27, 2026
**Stacked on #4203.** The last dependency step: hardhat 2.19.5 → **2.29.0** in both packages. smock capped this at 2.19.5. It is gone as of #4203, so the ceiling lifts — and 2.26+ is what Node 24 requires. Below it, Hardhat's solc download fails on Node 24 with `HH502`, because npm's undici and Node 24's bundled undici share a global-dispatcher symbol. ## Verified Whole suite, both packages, both runtimes: | | Node 22 | Node 24 | | --- | --- | --- | | `ecdsa` | 673 / 44 pending / 0 | **673 / 44 / 0** | | `random-beacon` | 955 / 0 pending / 0 | **955 / 0 / 0** | Identical to the pre-bump results, so nothing regressed — and it is safe to land before or after the Node bump in #4201, in either order. ## The other thing 2.29 brings | | before | after | | --- | --- | --- | | `random-beacon` | 3m | **23s** | | `ecdsa` | 2m | **31s** | That is EDR, the Rust execution engine, backported into the 2.x line. ## Two tests needed adjusting `challengeDkgResult` has an inline gas check, and two tests deliberately underfund the call to trigger it. hardhat 2.29 defaults to a newer hardfork, where EIP-7623's intrinsic calldata floor is enforced *before* execution: ``` ProviderError: Transaction requires gas floor of 161720 but got limit of 100000 ``` The node rejected the transaction outright, so the contract's own check never ran — the tests had stopped testing the thing they name. The limits move to just above each floor (`100000 → 170000`, `200000 → 220000`), still far short of what the call needs to complete, so both revert inside the contract as intended. The alternative was pinning the hardfork back to 2.19.5's default. Rejected: mainnet is past it, and running the suite against an older EVM than production is a worse trade than adjusting two gas limits. ## After this Node 22 → 24 in the five workflows, two Dockerfiles and two `engines` fields — a small follow-up, and the reason the whole chain existed. #4201 already moved those from 18 to 22; this makes 24 reachable for the first time. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated the Hardhat development tooling to version 2.29.0 for the ECDSA and random beacon packages. * **Tests** * Updated gas-limit scenarios to accurately validate insufficient-gas behavior under current transaction gas rules. * Clarified test coverage for contract-level gas checks. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #4202. Removes
@defi-wonderland/smock, which upstream archivedwith this notice:
That is the reason. The toolchain ceiling — no smock version reaches hardhat
2.26+, which is what Node 24 needs — is what made it unavoidable rather than
schedulable, and removing it unblocks that bump.
Result
Verified with smock uninstalled, not merely unused:
random-beaconecdsaThe increases are the 23 mock self-tests in each package, plus six tests in
random-beaconthat smock kept disabled. They sat behind— the paths asserting that a failing
seizedoes not take the whole operationdown with it. They pass against
MockContract. That is the point of replacingsmock rather than pinning it.
The replacement
contracts/test/MockContract.solplustest/helpers/mock.ts, ported fromthreshold-network/tbtc-v2#1062 where it has replaced 554 call sites and runs on
hardhat 2.29 / Node 24. smock configured fakes by mutating in-process
JavaScript inside Hardhat's EVM, which is why it broke on every hardhat past
2.19; this drives an ordinary deployed contract over the public provider API.
The one behavioural difference is that configuration is a transaction and
must be awaited, which is why call sites change at all.
allowBlocksWithSameTimestampis set in both configs so that configuring a mock cannot advance the chain clock.
Judgement calls worth reviewing
expectCalledWithis new. smock'scalledWithsays nothing about callcount, so reusing the existing
expectCalledOnceWithfor those two sites wouldhave silently strengthened the assertion. It has its own self-test including
the negative case.
Five assertions on a
viewfunction had to change. The TIP-092 suitechecked that
IStaking.authorizedStakewas called; it isview, so Solidityreaches it by STATICCALL where recording is impossible, and the mock refuses
loudly rather than answering zero. Four already asserted the value the mock
returns — the same proof by a stronger route — so the call check is dropped
with a comment. The fifth,
updateOperatorStatus, had no other assertion;it now asserts the operator is up to date, which holds only if the registry
read the allowlist and synced the pool to it.
resetMockis gone, and this one caused a real failure. Both callers weresmock-era bookkeeping that became harmful once resetting meant sending a
transaction. In the RandomBeacon suite it ran after
restoreSnapshot(), soit was redundant and, unawaited, landed in the next context and wiped the call
log that context asserted on. In
createNewWalletit mined a block inside ashared helper, which shifted cumulative state enough to flip an
isWalletMemberassertion in the full run while passing in isolation.One gas budget moved, and it is the only assertion relaxed rather than
replaced.
approveDkgResultreaches the wallet owner; recording a call SSTOREsits calldata — 447k against a 274k ± 1000 budget. Switching recording off for
that suite (nothing there inspects the wallet owner's calls) gives ~286k. The
residual ~12k is
MockContract's fallback dispatch, real work smock's fake didnot do, so the expectation moves to 286 000 and the tolerance stays. In
production the wallet owner is a real contract and costs more than either.
smock.mockwas a false alarm. The one use only ever read.address, withno
setVariable/getVariable, so it is now a plain factory deploy.Next
hardhat 2.19.5 → 2.29 and Node 22 → 24, which this unblocks.