dethcrypto / TypeChain

🔌 TypeScript bindings for Ethereum smart contracts
MIT License
2.73k stars 353 forks source link

fix: [ethers-v6] codegen/hardhat.ts DeployContractOptions expected #852

Closed DainisGorbunovs closed 11 months ago

DainisGorbunovs commented 11 months ago

This is merged now (needed a lint fix and a changeset added):

Issue

@nomicfoundation/hardhat-ethers@3.0.1 added support for transaction overrides in the deployContract helper.

Specifically, these are the changes https://github.com/NomicFoundation/hardhat/commit/d8056fb8bc1116338f61974a8a44cd0defcb2555.

Here's issue raised in hardhat repository:

Fix

Replaced FactoryOptions with DeployContractOptions, and added import for it. Reran the tests to fix other files.

Context

A new project is created:

npm init -y
npm install --save-dev hardhat
npx hardhat # Create a TypeScript project
npx hardhat compile

It installs @nomicfoundation/hardhat-ethers" v3.0.4. As hardhat uses [@typechain/ethers-v6](https://github.com/NomicFoundation/hardhat/blob/c2943366d4f16282c10f92f9f216ac08130a0eb4/yarn.lock#L1672), I've only updatedpackages/target-ethers-v6/src/codegen/hardhat.ts`, which should be enough to resolve the issue.

The provided scripts/deploy.ts contains:

import { ethers } from "hardhat";

async function main() {
  const currentTimestampInSeconds = Math.round(Date.now() / 1000);
  const unlockTime = currentTimestampInSeconds + 60;

  const lockedAmount = ethers.parseEther("0.001");

  const lock = await ethers.deployContract("Lock", [unlockTime], {
    value: lockedAmount,
  });

IDE complains about { value: lockedAmount }. Third parameter in ethers.deployContract used to be for signerOrOptions?: ethers.Signer | FactoryOptions but was changed to signerOrOptions?: ethers.Signer | DeployContractOptions

Visual Studio Code:

No overload matches this call.
  Overload 1 of 4, '(name: "Lock", args: any[], signerOrOptions?: Signer | FactoryOptions | undefined): Promise<Lock>', gave the following error.
    Argument of type '{ value: bigint; }' is not assignable to parameter of type 'Signer | FactoryOptions | undefined'.
      Object literal may only specify known properties, and 'value' does not exist in type 'Signer | FactoryOptions'.
changeset-bot[bot] commented 11 months ago

⚠️ No Changeset found

Latest commit: 5f8ac0376397512d69166b22f4e2a4877cea420e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

krzkaczor commented 11 months ago

There is an example for ethers-v6 could you begin with reproducing the problem in that example. I guess just updating deps just suffice. Ie. we need some kind of regression check to make sure this doesn't break in the future.

socket-security[bot] commented 11 months ago

New dependencies detected. Learn more about Socket for GitHub ↗︎

Packages Version New capabilities Transitives Size Publisher
@nomicfoundation/hardhat-ethers 3.0.4 None +1 282 kB fvictorio
DainisGorbunovs commented 11 months ago

I've updated the following, and can confirm that the test passes:

  1. examples/hardhat/test/counter.ts test for a regression check
  2. @nomicfoundation/hardhat-ethers package dependency version to 3.0.4

Regression checks:

To ensure the written code in counter.ts works properly, I've checked how it behaves in the older hardhat-ethers version, and also if I avoid using type annotation. In these cases, either the test errored, or returned an invalid number (note DeployContractOptions allows us to override settings now). I've set gasPrice to a random number, and expect that it's set that way after deployment.

if `@nomicfoundation/hardhat-ethers` remains at 3.0.0 ```sh An unexpected error occurred: test/counter.ts:5:10 - error TS2724: '"@nomicfoundation/hardhat-ethers/types"' has no exported member named 'DeployContractOptions'. Did you mean 'deployContract'? 5 import { DeployContractOptions } from "@nomicfoundation/hardhat-ethers/types" ~~~~~~~~~~~~~~~~~~~~~ ../../node_modules/.pnpm/@nomicfoundation+hardhat-ethers@3.0.0_ethers@6.3.0_hardhat@2.9.9/node_modules/@nomicfoundation/hardhat-ethers/types/index.d.ts:14:25 14 export declare function deployContract(name: string, signerOrOptions?: ethers.Signer | FactoryOptions): Promise; ~~~~~~~~~~~~~~ 'deployContract' is declared here.  ELIFECYCLE  Test failed. See above for more details. ```
same as above but also `DeployContractOptions` type annotation is not added ```sh Counter count up 1) "before each" hook for "should count up" 0 passing (300ms) 1 failing 1) Counter "before each" hook for "should count up": AssertionError: expected 1875000000 to equal 1637083528. The numerical values of the given "bigint" and "number" inputs were compared, and they differed. + expected - actual -1875000000 +1637083528 at /TypeChain/examples/hardhat/test/counter.ts:29:58 at Generator.next () at fulfilled (test/counter.ts:5:58)  ELIFECYCLE  Test failed. See above for more details. ```
krzkaczor commented 11 months ago

Superseded by: https://github.com/dethcrypto/TypeChain/pull/853

krzkaczor commented 11 months ago

Thanks for this contribution! It will be released in a second.