chore: Foundry script for OP Adapter deloyment + deploy Ink adapter #1228
Conversation
This PR adds support for CCTP to the OP Adapter implementation. Signed-off-by: Paul <108695806+pxrl@users.noreply.github.com>
| | Optimism_Adapter | [0xE1e74B3D6A8E2A479B62958D4E4E6eEaea5B612b](https://etherscan.io/address/0xE1e74B3D6A8E2A479B62958D4E4E6eEaea5B612b) | | ||
| | Optimism_Adapter | [0x3562e309C6C79626E5F0Cf746FB5Bf4f6b8EebE5](https://etherscan.io/address/0x3562e309C6C79626E5F0Cf746FB5Bf4f6b8EebE5) | |
There was a problem hiding this comment.
Driveby comment - we shouldn't have two Optimism_Adapter instances - that's pre-existing but can be tidied up here.
There was a problem hiding this comment.
Nice catch, this is known problem, it only happens in the md file and the script needs to be updated to only take the latest one
| return uint256(cctpDomain); | ||
| return uint32(uint256(cctpDomain)); |
There was a problem hiding this comment.
The CCTP Domain ID is max 32 bits, so truncate it here after bounds checking.
| struct OpStackAddresses { | ||
| address L1CrossDomainMessenger; | ||
| address L1StandardBridge; | ||
| address L1BlastBridge; |
There was a problem hiding this comment.
The Blast bridge is a total snowflake and I don't anticipate any further deployments there because the chain seems a bit stale these days. If we do need to support additional Blast depoyments in future then I'd propose we handle it as a special case, rather than baking it into the common OP deployment.
There was a problem hiding this comment.
nb. the bones of this file were taken from script/024DeployBaseAdapter.s.sol.
There was a problem hiding this comment.
Not sure if it's inteded to add this; can easily drop it if not.
There was a problem hiding this comment.
This is fine since we want to track the latest deployments and they are needs to generate broadcast/deployed-addresses.json
pxrl
changed the title
broadcast/deployed-addresses.md
Outdated
| | LpTokenFactory | [0x7dB69eb9F52eD773E9b03f5068A1ea0275b2fD9d](https://etherscan.io/address/0x7dB69eb9F52eD773E9b03f5068A1ea0275b2fD9d) | | ||
| | Mode_Adapter | [0xf1B59868697f3925b72889ede818B9E7ba0316d0](https://etherscan.io/address/0xf1B59868697f3925b72889ede818B9E7ba0316d0) | | ||
| | MulticallHandler | [0x0F7Ae28dE1C8532170AD4ee566B5801485c13a0E](https://etherscan.io/address/0x0F7Ae28dE1C8532170AD4ee566B5801485c13a0E) | | ||
| | OP_Adapter | [0x83057C549C6489899651012F84A36fA58FE2079e](https://etherscan.io/address/0x83057C549C6489899651012F84A36fA58FE2079e) | |
There was a problem hiding this comment.
Reviving old comment - this should ideally be OP_Adapter_<chainId> (OP_Adapter_57073 in this case).
script/056DeployOPAdapter.s.sol
Outdated
|
|
||
| // Get the current chain ID | ||
| uint256 chainId = block.chainid; | ||
| uint32 cctpDomain = getCircleDomainId(spokeChainId); |
There was a problem hiding this comment.
This has to be conditional on hasCctpDomain() because otherwise it'll revert.
| // nb. This is fragile. @todo: Improve. | ||
| switch (contractName) { | ||
| case "Universal_Adapter": | ||
| cctpDomainId = tx.arguments.at(3); | ||
| oftDstEid = tx.arguments.at(5); | ||
| break; | ||
| case "OP_Adapter": | ||
| cctpDomainId = tx.arguments.at(6); | ||
| break; | ||
| } |
There was a problem hiding this comment.
@fusmanii I extended this to support the OP adapter contract, but we're relying on a hard coupling of constructor argument ordering here. I've just extended what we already have but wondered if you had any thoughts about how it could be improved. wdyt?
There was a problem hiding this comment.
Yes I agree its a bit fragile. I think a better approach would be to have the deploy script save the chain id and then have this script read that so that its not relying argument ordering
pxrl
changed the title
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
This is fine since we want to track the latest deployments and they are needs to generate broadcast/deployed-addresses.json
| | Optimism_Adapter | [0xE1e74B3D6A8E2A479B62958D4E4E6eEaea5B612b](https://etherscan.io/address/0xE1e74B3D6A8E2A479B62958D4E4E6eEaea5B612b) | | ||
| | Optimism_Adapter | [0x3562e309C6C79626E5F0Cf746FB5Bf4f6b8EebE5](https://etherscan.io/address/0x3562e309C6C79626E5F0Cf746FB5Bf4f6b8EebE5) | |
There was a problem hiding this comment.
Nice catch, this is known problem, it only happens in the md file and the script needs to be updated to only take the latest one
| // nb. This is fragile. @todo: Improve. | ||
| switch (contractName) { | ||
| case "Universal_Adapter": | ||
| cctpDomainId = tx.arguments.at(3); | ||
| oftDstEid = tx.arguments.at(5); | ||
| break; | ||
| case "OP_Adapter": | ||
| cctpDomainId = tx.arguments.at(6); | ||
| break; | ||
| } |
There was a problem hiding this comment.
Yes I agree its a bit fragile. I think a better approach would be to have the deploy script save the chain id and then have this script read that so that its not relying argument ordering
This PR adds support for CCTP to the OP Adapter implementation.