Fix how the state of the fork block is handled #1016
Conversation
| /** | ||
| * The forked block needs special handling because the genesis accounts are added to it. | ||
| * This is meant to be called when the initial state is final, so that _initialStateRoot | ||
| * points to this starting fork state. | ||
| */ | ||
| public updateInitialStateRoot() { | ||
| this._stateRootToState.set(this._initialStateRoot, this._state); | ||
| } |
There was a problem hiding this comment.
Instead of adding this function and a long comment explaining its purpose I would put all of the initialization logic inside the FSM constructor. In other words, move this:
https://github.com/nomiclabs/hardhat/blob/b21474ce1df891edd7fb7b94f8bbc079c652b85a/packages/hardhat-core/src/internal/hardhat-network/provider/utils/putGenesisAccounts.ts#L10-L13
and make genesisAccounts a constructor parameter.
There was a problem hiding this comment.
Btw, putAccount is async only for the purpose of compliance with the PStateManager interface. I'd add a private synchronous _putAccount method and call it in the constructor and in the original public putAccount.
| if (blockNumber.eq(this._forkBlockNumber)) { | ||
| this._setStateRoot(toBuffer(this._initialStateRoot)); | ||
| return; | ||
| } |
There was a problem hiding this comment.
I would add a test covering the new if statement. Also this describe name in existing tests is no longer valid:
https://github.com/nomiclabs/hardhat/blob/b21474ce1df891edd7fb7b94f8bbc079c652b85a/packages/hardhat-core/test/internal/hardhat-network/provider/fork/ForkStateManager.ts#L613
There was a problem hiding this comment.
I added this one that I used while developing this: https://github.com/nomiclabs/hardhat/blob/b21474ce1df891edd7fb7b94f8bbc079c652b85a/packages/hardhat-core/test/internal/hardhat-network/provider/modules/eth.ts#L701
| if (blockNumber.eq(this._forkBlockNumber)) { | ||
| this._setStateRoot(toBuffer(this._initialStateRoot)); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Instead of adding this if statement, we could make the this._initialStateRoot equal to the actual stateRoot of the fork block. However this would require fetching the block before FSM construction and would slow down the HardhatNode creation. Having considered this I would leave this the way it is.
There was a problem hiding this comment.
I did exactly the same reasoning, I'm glad that you agree 😅
Closes #1004.