-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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 how the state of the fork block is handled #1016
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,7 +36,8 @@ const notSupportedError = (method: string) => | |
|
||
export class ForkStateManager implements PStateManager { | ||
private _state: State = ImmutableMap(); | ||
private _stateRoot: string = randomHash(); | ||
private _initialStateRoot: string = randomHash(); | ||
private _stateRoot: string = this._initialStateRoot; | ||
private _stateRootToState: Map<string, State> = new Map(); | ||
private _originalStorageCache: Map<string, Buffer> = new Map(); | ||
private _stateCheckpoints: string[] = []; | ||
|
@@ -50,6 +51,15 @@ export class ForkStateManager implements PStateManager { | |
this._state = ImmutableMap(); | ||
} | ||
|
||
/** | ||
* 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); | ||
} | ||
|
||
public copy(): ForkStateManager { | ||
const fsm = new ForkStateManager( | ||
this._jsonRpcClient, | ||
|
@@ -319,6 +329,10 @@ export class ForkStateManager implements PStateManager { | |
if (this._stateCheckpoints.length !== 0) { | ||
throw checkpointedError("setBlockContext"); | ||
} | ||
if (blockNumber.eq(this._forkBlockNumber)) { | ||
this._setStateRoot(toBuffer(this._initialStateRoot)); | ||
return; | ||
} | ||
Comment on lines
+320
to
+323
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would add a test covering the new if statement. Also this describe name in existing tests is no longer valid: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Comment on lines
+320
to
+323
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of adding this if statement, we could make the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did exactly the same reasoning, I'm glad that you agree 😅 |
||
if (blockNumber.gt(this._forkBlockNumber)) { | ||
this._setStateRoot(stateRoot); | ||
return; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Btw,
putAccount
is async only for the purpose of compliance with thePStateManager
interface. I'd add a private synchronous_putAccount
method and call it in the constructor and in the original publicputAccount
.