vm: remove isTruthy and isFalsy #2248
Conversation
gabrocheleau
requested review from
jochem-brouwer,
faustbrian,
holgerd77 and
acolytec3
Codecov Report
Flags with carried forward coverage won't be shown. Click here to find out more. |
| @@ -53,8 +45,8 @@ export async function runBlock(this: VM, opts: RunBlockOpts): Promise<RunBlockRe | |||
|
|
|||
| if ( | |||
| this._hardforkByBlockNumber || | |||
| isTruthy(this._hardforkByTTD) || | |||
| isTruthy(opts.hardforkByTTD) | |||
| this._hardforkByTTD !== undefined || | |||
There was a problem hiding this comment.
In vm.ts this field is marked as optional, but it looks to me that it is always being set? I think we can improve here by making that field non-optional?
There was a problem hiding this comment.
I tried making this.hardforkByTTD non-optional (which ends up removing that if altogether) but ended up with a couple failing API tests. I've created an issue for it in case we want to investigate that further (#2260)
There was a problem hiding this comment.
No no, just to add here: there are a lot of development use cases where this is not wanted (that HFs align with the block numbers in regard to a certain chain configuration). So these hardforkBy* options should always be present and available to set.
There was a problem hiding this comment.
The type of hardforkByTTD is undefined | bigint, and with optional I meant that it should be of type bigint (so it should never be undefined), but this ruins the logic of setting the hardforks (I realized this when checking #2260). I'm not sure what you mean "should always be available to set", if you want to set the hardforkByBlockNumber from false to true or vice-versa you have to either hack it in, or you have to create a new VM since it is readonly.
There was a problem hiding this comment.
Plainly meant this should remain an option (in the case this would be in question). 🙂
Extracts the vm package changes from #2233