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

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

Closed
wants to merge 2 commits into from

Conversation

DainisGorbunovs
Copy link
Contributor

@DainisGorbunovs DainisGorbunovs commented Jul 16, 2023

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 NomicFoundation/hardhat@d8056fb.

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 updated packages/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
Copy link

changeset-bot bot commented Jul 16, 2023

⚠️ No Changeset found

Latest commit: 5f8ac03

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
Copy link
Member

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
Copy link

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
Copy link
Contributor Author

DainisGorbunovs commented Jul 16, 2023

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
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<ethers.Contract>;
                               ~~~~~~~~~~~~~~
    'deployContract' is declared here.

 ELIFECYCLE  Test failed. See above for more details.
same as above but also `DeployContractOptions` type annotation is not added
  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 (<anonymous>)
      at fulfilled (test/counter.ts:5:58)



 ELIFECYCLE  Test failed. See above for more details.

@krzkaczor
Copy link
Member

Superseded by: #853

@krzkaczor krzkaczor closed this Jul 17, 2023
@krzkaczor
Copy link
Member

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

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