feat: Script to wire existing networks to newly deployed network#802
feat: Script to wire existing networks to newly deployed network#802onnovisser merged 12 commits intomainfrom
Conversation
script/WireToNewNetwork.s.sol
Outdated
| nonce = safe.getNonce(); | ||
| } | ||
|
|
||
| IAdapter[] memory adapters = new IAdapter[](2); |
There was a problem hiding this comment.
We should build this dynamically based on the configuration
There was a problem hiding this comment.
As discussed, we can do that in a later iteration which makes this script generic for any network.
| assertTrue(found, "SendPayload not emitted by MultiAdapter for Chainlink"); | ||
| } | ||
|
|
||
| function testWireEthereum() external { |
There was a problem hiding this comment.
I suppose eventually we want to run the test for every network.
script/WireToNewNetwork.s.sol
Outdated
| nonce = safe.getNonce(); | ||
| } | ||
|
|
||
| IAdapter[] memory adapters = new IAdapter[](2); |
There was a problem hiding this comment.
As discussed, we can do that in a later iteration which makes this script generic for any network.
script/WireToNewNetwork.s.sol
Outdated
|
|
||
| if (targetConn.axelar) { | ||
| address axelarAdapter = source.contracts.axelarAdapter; | ||
| bytes memory data = abi.encode(target.adapters.axelar.axelarId, target.contracts.axelarAdapter); |
There was a problem hiding this comment.
We need to cast the axelar adapter address to string, otherwise it will silently fail to relay via Axelar which has happened multiple times already. It's a real pain.
| bytes memory data = abi.encode(target.adapters.axelar.axelarId, target.contracts.axelarAdapter); | |
| bytes memory data = abi.encode(target.adapters.axelar.axelarId, vm.toString(target.contracts.axelarAdapter)); |
There was a problem hiding this comment.
wow, good catch.
wischli
left a comment
wischli
left a comment
There was a problem hiding this comment.
Did another round of reviews. Really only nits that you can ignore for now and can be tackled in a follow-up when we make this generic. Just wanted to flag them now so we have them on our radar.
|
|
||
| receive() external payable {} | ||
|
|
||
| function _testCase(string memory networkName) internal { |
There was a problem hiding this comment.
Nit: After looking over this once more, I feel like we should also validate the post wiring adapter configuration. AFAICT, if initAdapters were misconfigured (e.g., wrong threshold, wrong quorum), the test could still pass. So we shuld add something like
IMultiAdapter multiAdapter = IMultiAdapter(source.contracts.multiAdapter);
assertGt(multiAdapter.quorum(target.network.centrifugeId, PoolId.wrap(0)), 0, "Quorum not set");
assertEq(multiAdapter.threshold(target.network.centrifugeId, PoolId.wrap(0)), targetConn.threshold, "Wrong threshold");
| address sendLib = lzEndpoint.defaultSendLibrary(targetEid); | ||
| address recvLib = lzEndpoint.defaultReceiveLibrary(targetEid); |
There was a problem hiding this comment.
Hypothetically, these could return address(0) if LayerZero hasn't configured default libraries for the targetEid on the source chain. I think we should add a validation layer here to be safe
| address sendLib = lzEndpoint.defaultSendLibrary(targetEid); | |
| address recvLib = lzEndpoint.defaultReceiveLibrary(targetEid); | |
| address sendLib = lzEndpoint.defaultSendLibrary(targetEid); | |
| address recvLib = lzEndpoint.defaultReceiveLibrary(targetEid); | |
| require(sendLib != address(0) && recvLib != address(0), "LZ default libraries not configured for target EID"); |
| uint256 idx; | ||
|
|
||
| if (targetConn.layerZero) { | ||
| address lzAdapter = source.contracts.layerZeroAdapter; |
There was a problem hiding this comment.
Nit for later generic use: If targetConn.layerZero is true but source.contracts.layerZeroAdapter == address(0) due to the adapter not being deployed yet on source chain the script calls OpsGuardian.wire(address(0), ...) which is harder to debug as its a low-level revert. Suppose we should ensure none of the addresses point to the default 0 one.
| // function testWirePharos() external { | ||
| // _testCase("pharos"); | ||
| // } |
There was a problem hiding this comment.
Nit: We should add a comment why this is disabled.
There was a problem hiding this comment.
I'll try enabling it, maybe the CI has a Pharos api key
There was a problem hiding this comment.
Ah right, that one was already wired to Monad
|
Coverage after merging wire-to-new-network into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
lemunozm
left a comment
lemunozm
left a comment
There was a problem hiding this comment.
Good script here for future iterations as well!
3 participants
Product requirements
Design notes
TODOs