-
Notifications
You must be signed in to change notification settings - Fork 71
chore: Consolidated OP chain spoke pools #1087
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Faisal Usmani <[email protected]>
hardhat.config.ts
Outdated
"contracts/Zora_SpokePool.sol": LARGE_CONTRACT_COMPILER_SETTINGS, | ||
"contracts/Mode_SpokePool.sol": LARGE_CONTRACT_COMPILER_SETTINGS, | ||
"contracts/Base_SpokePool.sol": LARGE_CONTRACT_COMPILER_SETTINGS, |
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.
Since these are gone we should be able to clean up a bunch here.
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.
This one probably isn't the best candidate here, since it supports neither bridged nor native USDC. I think the Base SpokePool probably would be a better choice.
All of this is subject to whether there's a larger overhaul on the SpokePools wrt. bridge adapter modularity and configuration. But I think it makes sense to have a PR that handles the consolidation anyway, since it's basically inevitable that we delete these.
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.
Aren't all the spoke pools being deleted here the same up to comments?
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.
yes its just comment diff on all these spoke pool, in this case I think github randomly picked Bob to show as name change
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.
On a similar vein as https://github.com/across-protocol/contracts/pull/1087/files#r2355754767, I think you can refactor this entire scripts directory and just make a 001_DeployOpSpokePool.s.sol or something (and delete the other duplicate scripts). That would obviously require some knowledge of the chain ID, but I think you can get that, right?
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.
yup wanted to get an initial feedback on the direction I am taking before I put more effort into it. Going to work on the deploy script next
…isal/consolidate-op-chains
Signed-off-by: Faisal Usmani <[email protected]>
Signed-off-by: Faisal Usmani <[email protected]>
Signed-off-by: Faisal Usmani <[email protected]>
Signed-off-by: Faisal Usmani <[email protected]>
Signed-off-by: Faisal Usmani <[email protected]>
Removed these Spoke Pools:
Base_SpokePool.sol
Mode_SpokePool.sol
Redstone_SpokePool.sol
Zora_SpokePool.sol
Bob_SpokePool.sol
DoctorWho_SpokePool.sol
Created
OP_SpokePool.sol
that can be used for all of them since they all share the same codeLooking for feedback on if this is a good direction to go to