-
Notifications
You must be signed in to change notification settings - Fork 55
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
refactor: Relocate message handler interface #543
Conversation
The MulticallHandler contract currently imports the SpokePool contract and experienced a bytecode change after the following unrelated change: 86593c6 This change resulted in its CREATE2 deployment address changing. The MulticallHandler doesn't actually need any of the SpokePool implementation or its dependencies, and it seems like we'd like the MulticallHandler to be deployed to the same address on as many chains as possible, so factor out the interface to a separate file to insulate it from any unintended changes.
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.
Could we also just do import { AcrossMessageHandler } from "../SpokePool.sol";
in MulticallHandler.sol
? It also makes sense to add it to another file, so I'm fine with either.
Good call - I hadn't thought of that. I tried it though and I can still get the resulting bytecode to change by making simple changes in |
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.
Does this keep the MultiCallHandler create2 address the same?
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.
Good call +1
Signed-off-by: Paul <[email protected]>
The MulticallHandler contract currently imports the SpokePool contract and experienced a bytecode change after the following unrelated commit:
86593c6
This resulted in its CREATE2 deployment address changing. The MulticallHandler doesn't actually need any of the SpokePool implementation or its dependencies, and it seems like we'd like the MulticallHandler to be deployed to the same address on as many chains as possible, so factor out the interface to a separate file to insulate it from any unintended modifications.