-
Notifications
You must be signed in to change notification settings - Fork 39
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
statedb revert to snapshot for ignored batches #1713
Conversation
WalkthroughThe update in the code concerns the introduction of a mechanism to take a snapshot of the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on X ? TipsChat with CodeRabbit Bot (
|
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- go/enclave/components/batch_executor.go (2 hunks)
Additional comments: 5
go/enclave/components/batch_executor.go (5)
171-171: Creation of a snapshot of
stateDB
is a good practice to ensure that the state can be reverted if necessary. This aligns with the PR objective to maintain the integrity ofstateDB
.221-222: The use of
stateDB.RevertToSnapshot(snap)
is correctly placed within the conditional block that checks for the absence of transactions to process. This ensures that thestateDB
is only reverted when necessary, which is a critical part of the PR objective.218-225: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [168-225]
The snapshot and revert logic within the
ComputeBatch
function appears to be well-contained and should not interfere with other parts of the function. However, it is important to ensure that the snapshot is taken at the correct point in the function's execution and that no other state changes are missed when reverting.Verification successful
The snapshot creation in the
ComputeBatch
function is correctly placed immediately after thestateDB
initialization and before any balance mutations. This ensures that the revert logic can correctly undo any changes if the conditions for reverting are met. The verification process has not revealed any issues with the snapshot and revert logic within theComputeBatch
function.* 168-170: The error handling for the creation of the `stateDB` is present and correctly returns an error if the `stateDB` cannot be created. This is important for the robustness of the `ComputeBatch` function.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the snapshot is taken before any state mutations that could be affected by the revert logic. rg --type go "stateDB\.(Set|Sub|Add)Balance" go/enclave/components/batch_executor.goLength of output: 246
Script:
#!/bin/bash # Check the order of balance mutations in relation to the snapshot creation within the `ComputeBatch` function. rg --type go -C 10 "snap := stateDB.Snapshot()" go/enclave/components/batch_executor.goLength of output: 984
- 218-225: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [1-414]
No other changes related to the PR objective are present in the rest of the file. The rest of the code remains unchanged and should function as before, assuming the snapshot functionality does not introduce side effects.
Why this change is needed
to avoid unexpected statedb mutations, revert to snapshot for ignored batches