-
Notifications
You must be signed in to change notification settings - Fork 38
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 Testnet start #1725
Fix Testnet start #1725
Conversation
WalkthroughThe code update involves disabling a feature related to the management of database state snapshots within a batch execution component. A line of code that created a snapshot of the database state has been commented out, and a note has been added to address a future fix regarding deployment issues caused by unintended mutations to the database. 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 your favorite social media? 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.
@@ -172,7 +172,7 @@ func (executor *batchExecutor) ComputeBatch(context *BatchExecutionContext, fail | |||
if err != nil { | |||
return nil, fmt.Errorf("could not create stateDB. Cause: %w", err) | |||
} | |||
snap := stateDB.Snapshot() | |||
// snap := stateDB.Snapshot() |
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.
Commented out code can lead to confusion and should be accompanied by a clear explanation or a linked issue for context.
- // snap := stateDB.Snapshot()
+ // TODO: Re-enable stateDB.Snapshot() once issue #2654 is resolved.
Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
// snap := stateDB.Snapshot() | |
// TODO: Re-enable stateDB.Snapshot() once issue #2654 is resolved. |
stateDB.RevertToSnapshot(snap) | ||
// todo review why this is failing deployment with "panic: revision id 0 cannot be reverted" - https://github.com/ten-protocol/ten-internal/issues/2654 | ||
//// revert any unexpected mutation to the statedb | ||
//stateDB.RevertToSnapshot(snap) |
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.
Disabling the revert state functionality may have implications for the state consistency. Ensure that the risks are understood and that there is a plan to re-enable this functionality.
- //stateDB.RevertToSnapshot(snap)
+ // FIXME: Re-enable stateDB.RevertToSnapshot(snap) once the underlying issue is resolved.
Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
//stateDB.RevertToSnapshot(snap) | |
// FIXME: Re-enable stateDB.RevertToSnapshot(snap) once the underlying issue is resolved. |
@@ -222,8 +222,9 @@ | |||
len(crossChainTransactions) == 0 && | |||
len(messages) == 0 && | |||
len(transfers) == 0 { | |||
// revert any unexpected mutation to the statedb | |||
stateDB.RevertToSnapshot(snap) | |||
// todo review why this is failing deployment with "panic: revision id 0 cannot be reverted" - https://github.com/ten-protocol/ten-internal/issues/2654 |
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.
The comment regarding the deployment failure should include a reference to the issue tracker for easier follow-up.
+ // TODO: Review the cause of "panic: revision id 0 cannot be reverted" and resolve it according to issue #2654.
Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
// todo review why this is failing deployment with "panic: revision id 0 cannot be reverted" - https://github.com/ten-protocol/ten-internal/issues/2654 | |
// TODO: Review the cause of "panic: revision id 0 cannot be reverted" and resolve it according to issue #2654. |
Why this change is needed
https://github.com/ten-protocol/ten-internal/issues/2654
Testnet fails to start after deploying dev testnet twice.
Seems to be related with the revert state, commenting it out for now.
What changes were made as part of this PR
Please provide a high level list of the changes made
PR checks pre-merging
Please indicate below by ticking the checkbox that you have read and performed the required
PR checks