Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: ganache => hardhat #630

Merged
merged 1 commit into from
Jun 7, 2024
Merged

test: ganache => hardhat #630

merged 1 commit into from
Jun 7, 2024

Conversation

jchappelow
Copy link
Member

@jchappelow jchappelow commented Apr 3, 2024

This replaces Ganache with Hardhat.

Ganache project is now dead, and the advice is to migrate to hardhat: https://consensys.io/blog/consensys-announces-the-sunset-of-truffle-and-ganache-and-new-hardhat

Several things were done to accomplish this:

  • create a hardhat docker image like the one we were using for ganache: our kwildb/hardhat Dockerfile repo and pushed to Hub
  • eth_maxPriorityFeePerGas is not supported by Hardhat (it's a go-ethereum invention, not in the RPC spec): transact() attempts to use unsupported eth_maxPriorityFeePerGas via JSON RPC on Kovan ethereum/go-ethereum#23479. We must set the dynamic transaction fee caps, otherwise the go-ethereum methods try to use eth_maxPriorityFeePerGas to get the "tip" cap with this non-existent RPC
  • contract deploy fails if we don't give it a gas limit set because it tries fee estimation for the deployment transaction and errors with "eth_estimateGas: Contract creation without any data provided". I don't know the cause of this bug, but we can work around it.
  • The compose health check waits for the log "Started HTTP and WebSocket JSON-RPC server" in the hardhat docker container.

NOTE: back to draft because there are too many hoops to jump through until hardhat is updated for it's eth_call handling. See #630 (comment)
The v2.22.5 release fixed the issues: https://github.com/NomicFoundation/hardhat/releases/tag/hardhat%402.22.5

@jchappelow
Copy link
Member Author

jchappelow commented Apr 4, 2024

Changing to draft since this one is proving painful to fully resolve until there is resolution to NomicFoundation/edr#360, such as NomicFoundation/hardhat#4718. In short, hardhat does not recognize input as the field for parameters in eth_call and some other RPCs although that is to-spec, which is understandable since data was the field used broadly and in go-ethereum prior to 1.13.

EDIT: They've resolved the issue in the latest release: https://github.com/NomicFoundation/hardhat/releases/tag/hardhat%402.22.5

@jchappelow jchappelow marked this pull request as draft April 4, 2024 14:22
@jchappelow jchappelow force-pushed the hardhat branch 2 times, most recently from fa9b2ec to f59830b Compare June 4, 2024 17:01
@jchappelow jchappelow marked this pull request as ready for review June 4, 2024 17:01
@jchappelow
Copy link
Member Author

This is finally unblocked. I think we can go ahead with dropping Ganache from our integ tests now.

Copy link
Collaborator

@brennanjl brennanjl left a comment

Choose a reason for hiding this comment

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

Approving but I think @Yaiba should take a look since he knows more on this

@jchappelow jchappelow added this to the v0.8.0 milestone Jun 7, 2024
@jchappelow
Copy link
Member Author

We should make this change before we make the release branch. Our hardhat dockerfile is here: https://github.com/kwilteam/hardhat

@jchappelow jchappelow requested a review from Yaiba June 7, 2024 16:19
Copy link
Contributor

@Yaiba Yaiba left a comment

Choose a reason for hiding this comment

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

LGTM

test/integration/docker-compose.yml.template Show resolved Hide resolved
@jchappelow jchappelow merged commit 942e909 into main Jun 7, 2024
2 checks passed
@jchappelow jchappelow deleted the hardhat branch June 7, 2024 17:08
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.

3 participants