Convert StatelessVerkleStateManager usage to type in vm
#4021
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
packages/vm/src/runBlock.ts
Outdated
| stateManager.initVerkleExecutionWitness!(block.header.number, block.executionWitness) | ||
|
|
||
| if (stateManager instanceof StatelessVerkleStateManager) { | ||
| if ('verifyPostState' in stateManager) { |
There was a problem hiding this comment.
The line was previously very descriptive stateManager instanceof StatelessVerkleStateManager such that it is clear that stateManager is actually a StatelessVerkleStateManager. Could you add a comment here? Check if stateManager is of type StatelessVerkleStateManager for instance
packages/vm/src/runBlock.ts
Outdated
| } | ||
|
|
||
| if (!(vm.stateManager instanceof StatelessVerkleStateManager)) { | ||
| if (!('verifyPostState' in vm.stateManager)) { |
There was a problem hiding this comment.
Here a comment for StatelessVerkleStateManager
packages/vm/src/runTx.ts
Outdated
| !(vm.stateManager instanceof StatefulVerkleStateManager) && | ||
| !(vm.stateManager instanceof StatelessVerkleStateManager) | ||
| ) { | ||
| if (!('verifyPostState' in vm.stateManager) && !('verifyPostState' in vm.stateManager)) { |
There was a problem hiding this comment.
Here thus a check if it's a *VerkleStateManager type (Stateful or Stateless)
packages/vm/src/runTx.ts
Outdated
| !(vm.stateManager instanceof StatefulVerkleStateManager) && | ||
| !(vm.stateManager instanceof StatelessVerkleStateManager) | ||
| ) { | ||
| if (!('verifyPostState' in vm.stateManager) && !('verifyPostState' in vm.stateManager)) { |
There was a problem hiding this comment.
Isn't this checking the same condition twice?
There was a problem hiding this comment.
Ah, good point, I did not catch this 😅 If verifyPostState is only a method of StatefulVerkleStateManager and StatelessVerkleStateManager and not of any other StateManager then we can indeed compactify it to one check (and not the same check twice)
There was a problem hiding this comment.
Hah, sorry I missed that. Will update!
packages/vm/src/runBlock.ts
Outdated
| stateManager.initVerkleExecutionWitness!(block.header.number, block.executionWitness) | ||
|
|
||
| if (stateManager instanceof StatelessVerkleStateManager) { | ||
| if ('verifyPostState' in stateManager) { |
There was a problem hiding this comment.
I think it would be nice to have a code comment directly on the original verifyPostState method how the method is (mis-)used and that it should therefore not be renamed.
4 participants
This modifies our usage of
StatelessVerkleStateManagerinvmto import it only as a type so it does not get included in a bundle where theStatelessVerkleStateManagerisn't actually used