Scoped WorldState#9028
Conversation
Would be better then if IWorldState was the scope; so is a compilation constraint rather than a mode switch of worldstate? e.g. rather than using (stateProvider.BeginScope(null))
{
stateProvider.CreateAccount(TestItem.AddressA, UInt256.One);
stateProvider.CreateAccount(TestItem.AddressB, UInt256.One);
stateProvider.Commit(London.Instance);you would do using (var state = stateProvider.BeginScope(null))
{
state.CreateAccount(TestItem.AddressA, UInt256.One);
state.CreateAccount(TestItem.AddressB, UInt256.One);
state.Commit(London.Instance); |
|
I tried that about 1 year ago, @tanishqjasoria tried that about half year ago. Result it A LOT of refactor. And we still dont have proper worldstate scope. |
LukaszRozmej
left a comment
There was a problem hiding this comment.
Would be better if IWorldState was the scope and it would have been created from something like IWoldStateSource/Manager
|
|
||
| InitializeTestState(test, stateProvider, specProvider); | ||
| // Genesis processing | ||
| using (stateProvider.BeginScope(null)) |
There was a problem hiding this comment.
Can you make some constant called Genesis for the parameter?
| worldState.SetBaseBlock(header); | ||
| return new ReadOnlyTxProcessingScope(transactionProcessor, worldState); | ||
| IDisposable closer = worldState.BeginScope(header); | ||
| return new ReadOnlyTxProcessingScope(transactionProcessor, closer, worldState); |
There was a problem hiding this comment.
closer is terrible name, maybe just call it currentScope?
|
There was a problem hiding this comment.
Pull Request Overview
This PR introduces scoped WorldState by replacing SetBaseBlock with a disposable BeginScope pattern. The change ensures that any IWorldState operations must occur within a properly opened and closed scope, enhancing safety and preventing misuse of state operations outside of valid contexts.
- Replace
SetBaseBlockmethod withBeginScope(BlockHeader baseBlock)that returns a disposable - Add scope validation to prevent WorldState operations outside of valid scopes
- Update all test code to use the new scoped pattern with proper dispose handling
Reviewed Changes
Copilot reviewed 59 out of 59 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Nethermind/Nethermind.State/WorldState.cs | Core implementation of scoped WorldState with BeginScope method and scope validation guards |
| src/Nethermind/Nethermind.Evm/State/IWorldState.cs | Interface changes removing SetBaseBlock and adding BeginScope and IsInScope |
| src/Nethermind/Nethermind.Consensus/Processing/BranchProcessor.cs | Updated to use scoped WorldState pattern in block processing |
| Multiple test files | Updated test implementations to use BeginScope with proper dispose patterns |
Comments suppressed due to low confidence (2)
src/Nethermind/Nethermind.Consensus/Processing/BranchProcessor.cs:69
- [nitpick] The variable name
stateProvideris inconsistent with the field name_stateProviderused elsewhere in the class. Consider using the field directly or renaming for consistency.
if (stateProvider.IsInScope)
src/Nethermind/Nethermind.Consensus/Processing/BranchProcessor.cs:85
- The variable name
stateProvidershould be_stateProviderto match the class field being used throughout the method.
worldStateCloser = stateProvider.BeginScope(baseBlock);
8694204 to
91e14bd
Compare
e4f31cb to
f2d5d45
Compare
f2d5d45 to
5749f95
Compare
5749f95 to
3ee99cd
Compare
| // Init state if we need system calls before actual processing starts | ||
| worldState.SetBaseBlock(blockTree.Head?.Header); | ||
|
|
| worldState.SetBaseBlock(header); | ||
| return new ReadOnlyTxProcessingScope(transactionProcessor, worldState); | ||
| IDisposable closer = worldState.BeginScope(header); | ||
| return new ReadOnlyTxProcessingScope(transactionProcessor, closer, worldState); |
SetBaseBlockwithBeginScope(BlockHeader baseBlock)that returns a disposable.IWorldStatecan only be called in between starting and closing of the scope.WorldStateandBranchProcessor.Types of changes
What types of changes does your code introduce?
Testing
Requires testing
If yes, did you write tests?
Notes on testing