-
Notifications
You must be signed in to change notification settings - Fork 58
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
improve(finalizer): remove hardcoded constants #1610
Conversation
Signed-off-by: bennett <[email protected]>
Signed-off-by: bennett <[email protected]>
Signed-off-by: bennett <[email protected]>
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.
Looks good - couple of small comments.
Signed-off-by: bennett <[email protected]>
Signed-off-by: bennett <[email protected]>
@bmzig Looking good! I see some additional hardcoding in:
(I also found a few more and dropped them in this PR: #1611) |
Signed-off-by: Bennett <[email protected]>
The top/bottom are fixed here: a584a4b I'm saving all the hardcoded values in bridges/* for #1524, since I'm aiming to lean down the code/make those instances of hardcoded values generic. |
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.
Just found that I had these comments pending. I'll circle over the PR again now.
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.
LGTM subject to some straggling hardcoded chain IDs, and potentially revisiting the OVM_CHAIN_ID (or add a comment describing the typeof operation).
In preparation for NCA, we should remove all instances where we hardcode the chain IDs so that the bots will not automatically assume we are on mainnet.