NomicFoundation / hardhat

Hardhat is a development environment to compile, deploy, test, and debug your Ethereum software.
https://hardhat.org
Other
7.33k stars 1.42k forks source link

[hardhat-waffle] types for "loadFixture" are broken #849

Open PaulRBerg opened 4 years ago

PaulRBerg commented 4 years ago

Description

As explained on Discord, the 2.1.0 release for @nomiclabs/buidler-waffle broke the loadFixture function.

The root of the issue is in the fixtures.ts file. If overrideSigners is not manually overridden by end-users, the default loadFixture function has this argument to 'undefined', as per its definition in index.ts:

loadFixture: buidlerCreateFixtureLoader(buidlerWaffleProvider)

As you can see, there is no second or third argument, so overrideSigners ends up being assigned to 'undefined'.

Current Workaround

Fortunately, it is possible to band-aid this bug by creating a custom fixture loader, as per the Waffle docs. Here's how I did it:

import { Fixture } from "ethereum-waffle";
import { Signer } from "@ethersproject/abstract-signer";
import { waffle } from "@nomiclabs/buidler";

const { createFixtureLoader } = waffle;

export function loadFixture(signers: Signer[]): <T>(fixture: Fixture<T>) => Promise<T> {
  return createFixtureLoader(Object.values(signers));
}

Possible Solution

The definition of loadFixture should be changed to something like this:

loadFixture: buidlerCreateFixtureLoader(buidlerWaffleProvider, await bre.ethers.getSigners()),

I would have made a PR, but:

PaulRBerg commented 4 years ago

I've also just noticed that there is another potentially linked issue with the implementation of fixtures in buidler-waffle.

In the fixtures.ts file, the types are Signer, but in type-extensions.d.ts file, the types are the implicit ones provided by Waffle, i.e. Wallet. This means that end-users must cast the Signer array as a Wallet] array, which works in most cases, but it goes against what Richard Moore (creator of ethers.js) recommends doing:

Waffle should certainly not be using Wallet as their low-level Signer as Wallet is a fairly high-level Signer. The Signer sub-class should be what they use, since a Wallet requires a private key, which many situations do not have access to.

Possibly related: https://github.com/EthWorks/Waffle/pull/355

PaulRBerg commented 3 years ago

Hey @alcuadrado, has this been addressed in Hardhat?

Mrtenz commented 3 years ago

@paulrberg I was running into this issue today with Hardhat, so it seems like this is not fixed yet.

fvictorio commented 3 years ago

Hi, I recently was able to use loadFixture by just using the signers from ethers.getSigners() instead of the ones that loadFixture receives. So the workaround is to just declare your fixture without any parameters.

I'm going to leave this open because at some point we need to put some serious effort into making waffle work more seamlessly with hardhat, and this is one of the several things that should work better.

PaulRBerg commented 3 years ago

I can confirm that it works now.

Yes, increasing compatibility between Hardhat and Waffle (or even implementing some of the latter's features natively) would go a long way.

PaulRBerg commented 3 years ago

Update: it doesn't work. The only improvement is that the types match now. I am getting this error:

TypeError: Cannot read property '0' of undefined
      at /Users/paulrberg/projects/prb-math/test/shared/fixtures.ts:28:35
      at step (test/shared/fixtures.ts:33:23)
      at Object.next (test/shared/fixtures.ts:14:53)
      at /Users/paulrberg/projects/prb-math/test/shared/fixtures.ts:8:71
      at new Promise (<anonymous>)
      at __awaiter (test/shared/fixtures.ts:4:12)
      at unitFixturePrbMathUd60x18 (test/shared/fixtures.ts:65:12)
      at Proxy.load (node_modules/@nomiclabs/hardhat-waffle/src/fixtures.ts:32:26)
      at Context.<anonymous> (test/prbMathUd60x18/PRBMathUD60x18.ts:9:45)
      at step (test/prbMathUd60x18/PRBMathUD60x18.ts:33:23)

The issue is that the array of signers is empty.

fvictorio commented 3 years ago

The issue is that the array of signers is empty.

Which array of signers? The one received by the fixture? As far as I could tell, the fixture isn't receiving anything, that's why I mentioned that you should just manually get the signers instead of using the arguments. That's it, try:

async function myFixture() {
  const signers = await ethers.getSigners()
  ...

and use that.

PaulRBerg commented 3 years ago

Which array of signers? The one received by the fixture?

Yes.

that's why I mentioned that you should just manually get the signers instead of using the arguments.

Oh, I see.

Diego-c22 commented 2 years ago

You can create different functions to call a fixture with different arguments and use that functions with loadFixture()