Skip to content

Remove legacy Hardhat contract tests - #2982

Open
clement-ux wants to merge 5 commits into
masterfrom
chore/remove-hardhat-contract-tests
Open

Remove legacy Hardhat contract tests#2982
clement-ux wants to merge 5 commits into
masterfrom
chore/remove-hardhat-contract-tests

Conversation

@clement-ux

@clement-ux clement-ux commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Why

Foundry is already the tool we use to test smart contracts. The repository still contained the old Hardhat contract test suite, so we had two test systems even though only one was meant to be the source of truth.

This PR removes that retired Hardhat test layer. It does not change the contracts or replace the tests again; the contract coverage already lives in Forge.

What changes

  • remove the legacy Hardhat smart-contract tests and their coverage/fork commands
  • remove dependencies that were only needed by those tests
  • keep the remaining JavaScript task tests and run them separately in CI
  • move a few live files out of test/ to where they actually belong:
    • the Talos task test moves to tasks/test/
    • Forge Beacon and validator fixtures move next to their Forge consumers
    • ABIs still used by Hardhat tooling move to abi/hardhat/

What stays

Hardhat is still present for operational tasks and, at this point in the stack, the historical deployment and ABI tooling. Those are handled separately in the follow-up PRs.

Stack

This is the first PR in the cleanup:

  1. Remove legacy Hardhat contract tests #2982 — remove legacy Hardhat contract tests
  2. Remove legacy Hardhat deployment scripts #2983 — remove legacy Hardhat deployment scripts
  3. Generate ABI package with Foundry #2985 — generate the ABI package with Foundry

Validation

  • forge build contracts/ --force -j 1
  • affected Forge fixture subtrees compile successfully
  • CompoundingStakingStrategy Forge suite: 116 passing
  • pnpm run test:tasks: 20 passing
  • pnpm run test:scripts: 61 passing
  • pnpm run test:layouts: 11 passing
  • JavaScript lint, typecheck, Prettier and Hardhat task loading pass

The full all-at-once Forge build exceeded the 8 GB local runner limit; the production contracts and affected test subtrees were therefore validated separately.

@sparrowDom sparrowDom left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job, a couple of comments inline.
Also we might want to fix the tasks/smokeTest.js. Those read from the /deploy folder. I know it was already broken before, still this looks like a good time to address it

@@ -1,70 +0,0 @@
const { expect } = require("chai");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

THis file unit tests the Talos tasks/crossChain.js It should not be removed. Maybe it can be moved to: tasks/test/

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -4,33 +4,6 @@

The unit tests here use `MockBeaconProofs` which auto-passes all proof verification. This covers the strategy's state machine logic thoroughly but does **not** exercise the real `BeaconChainProofs` library.

### Hardhat tests not yet ported (candidates for fork tests)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these tests were not yet ported to Foundry.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sparrowDom sparrowDom left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work LGTM

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