-
Notifications
You must be signed in to change notification settings - Fork 76
Feature/add no reorg #822
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
Feature/add no reorg #822
Conversation
apoorvsadana
left a comment
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.
let's discuss on the standup regarding how we can test this once!
madara/node/src/cli/l2.rs
Outdated
| /// the node will stop with an error instead of performing a reorg. This is useful for | ||
| /// operators who want to manually handle chain divergences. | ||
| #[clap(env = "MADARA_NO_REORG", long, default_value_t = true)] | ||
| pub no_reorg: bool, |
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.
| pub no_reorg: bool, | |
| pub disable_reorg: bool, |
heemankv
left a comment
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.
Please change no_reorg to disable_reorg
Also have we tested that the full node is able to sync even after one of the blocks had mismatched, I think we might also have to add disable_all_verifications(true) in l2.rs
So that this block could continue, Let's test and discuss
|
|
||
| // Check if reorg is disabled | ||
| if self.no_reorg { | ||
| tracing::error!("❌ REORG DETECTED but --no-reorg flag is enabled!"); |
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.
nitpick : Let's re-write -> --no-reorg flag
to Reorganisation is explicitly denied by config or something that suite you better maybe
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.
disagree! previous message was more specific, --no-reorg flag disabled reorging
| tracing::error!(" Incoming parent hash: {:#x}", incoming_parent_hash); | ||
| tracing::error!(""); | ||
| tracing::error!("⚠️ Divergent state detected - the local chain has diverged from the upstream chain."); | ||
| tracing::error!("⚠️ Blockchain reorganization is required but disabled by --no-reorg flag."); |
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.
same as above
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.
- for line 309 as well
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.
same as above!
Pull Request type
Please add the labels corresponding to the type of changes your PR introduces:
What is the current behavior?
Resolves: #NA
What is the new behavior?
Does this introduce a breaking change?
Other information