NomicFoundation / hardhat

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

_validateHardforks assumes ethereum #1580

Open bigwampa opened 3 years ago

bigwampa commented 3 years ago

_validateHardforks in packages/hardhat-core/src/internal/hardhat-network/provider/node.ts assumes the user is forking ethereum.

In order to support users who are developing on other blockchains, please provide a flag to omit this check or otherwise support other blockchains.

Currently, because hardhat assumes ethereum, any attempt to fork from a specific block number on another blockchain will throw the internal error on line 236:


        throw new InternalError(
          `Cannot fork ${upstreamCommon.chainName()} from block ${forkBlockNumber}.
Hardhat Network's forking functionality only works with blocks from at least spuriousDragon.`
        );
      }```
alcuadrado commented 3 years ago

Can you please elaborate on how this fails? We designed those validations to explicitly allow other networks.

bigwampa commented 3 years ago

Discord comment here: https://discord.com/channels/750408878008827925/750408878008827928/856617376534626304

bigwampa commented 3 years ago

@alcuadrado, thanks for the quick response!

It seems that if the user provides a blocknumber to fork, then they will hit line 235: if (!upstreamCommon.gteHardfork("spuriousDragon")) {

Looking into the definition of gteHardfork() now, but unclear how that wouldn't return false for any valid Avalanche blocknumber.

alcuadrado commented 3 years ago

My understanding is that line 225 will throw if you use a chainId not supported by ethereumjs, and in that case, the return in line 230 will prevent 235 from executing.

Can you provide steps to reproduce it with Avalanche?

bigwampa commented 3 years ago

@alcuadrado, I was working with another engineer on this. They have the reproduction steps currently. I will get them to you shortly. But a brief summary:

We followed the instructions here: https://hardhat.org/guides/mainnet-forking.html and changed the hardhat.config.js to include the following:

networks: {
     hardhat: {
         forking: {
             url: "https://api.avax.network/ext/bc/C/rpc",
             blockNumber: 2405677
         }
    },
},

When we went to run the code we hit the above-mentioned error.

bigwampa commented 3 years ago

@alcuadrado, full hardhat.config.js can be found here: https://github.com/bmino/x-voting/blob/feature/forked_mainnet/hardhat.config.js

I don't think any other files are relevant to this problem.

With this config, we ran npx hardhat test and hit the spuriousDragon error.

fvictorio commented 3 years ago

I haven't look into this yet, but quick comment: that URL seems to return 1 as its chain id. It's very likely that we assume in more than one place that only the Ethereum mainnet has a chain id of 1.

bigwampa commented 3 years ago

Hey @fvictorio, Thanks for looking into this!

The url we provide in the hardhat.config.js is the default url for accessing Avalanche. From their documentation, it should return a chain id of 43114.

It does, however, use a network id of 1.

fvictorio commented 3 years ago

I see. Maybe we should try to use the chain id instead of the network id if possible? I don't know much about the JSON-RPC side of that though. I guess we can try to get the eth_chainId first and fall back to the net_version if that doesn't work. (I believe we have/had a function that did exactly that).

aftermathdigital commented 3 years ago

For what it's worth, this RPC endpoint does support eth_chainId, and does indeed return 43114:

{
  "id": 1,
  "jsonrpc": "2.0",
  "result": "0xa86a"
}

Which is good, because if it didn't, then falling back to net_version would make it indistinguishable from Eth. I don't understand why AVAX chose to use the same networkId as Eth in the first place though...

This PR actually makes the remote chainId accessible where you'd need it for the call to _validateHardforks, but I don't know enough about ethereumjs' implementation to know if you can just switch the networkId for the chainId here.

Edit: Although doing so does allow hardhat to fork AVAX successfully.

aftermathdigital commented 3 years ago

https://github.com/nomiclabs/hardhat/pull/1593

Seems to pass all tests. I haven't added a new test for this, which I feel like I should - I guess that test should check that you CAN fork chains from block before spurious dragon where the networkId is 1 and the chainId isn't.