Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Added config options to control HeadTracker's support of finality tags #13336
Added config options to control HeadTracker's support of finality tags #13336
Changes from all commits
958b551
6065e8e
e92524c
8d45414
60ecc35
64a1408
d1245eb
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Shouldn't the default value be false otherwise every chain will override finality tag by default? AFAIK the only chain that needs this as true is Arbitrum
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.
IMHO safer approach is to disable it for all of the chains and only enable it for those that we've seen behave normally. After the optimisation we will enable it for all chains.
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.
I might be a bit confused here, but don't we want to enable Finality Tag on all chains they support it and if they show they have issues, then disable it by setting
FinalityTagBypass = true
? Most chains don't have more than 10k finality depth. Otherwise we would have to explicitly test every chain we are operating on. I mean I'm ok either way, but we need to make sure Finality Tag is enabled on every chain we can. Same thing happened with dynamic transactions. They are still disabled in many chains because no one ever did the switch.