chore: refactor MessagIdHook/ISM tests into a common ExternalBridgeTest contract
#4490
Conversation
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4490 +/- ##
==========================================
+ Coverage 81.37% 82.01% +0.63%
==========================================
Files 100 100
Lines 1412 1412
Branches 178 178
==========================================
+ Hits 1149 1158 +9
+ Misses 263 254 -9
|
aroralanuk
changed the title
ExternalBridgeTest contractExternalBridgeTest contract
| // async call - native bridges might have try catch block to prevent revert | ||
| try | ||
| this.externalBridgeDestinationCallWrapper(externalCalldata, 0) | ||
| {} catch {} |
There was a problem hiding this comment.
optimism has a try catch in their l2Messenger.relayMessage() so it doesn't revert.
| ); | ||
| } | ||
|
|
||
| // try catch block to prevent revert |
There was a problem hiding this comment.
this is not descriptive enough to be useful
There was a problem hiding this comment.
add more detail here:
"wrapper function needed for _externalBridgeDestinationCall because try catch cannot call an internal function"
Description
We currently have 6 different versions of ISMs derived from AbstractMessageIdAuthorizedIsm:
But all of them have their own custom test suite right now, with widely overlapping testing areas. For standardizing the tests, I've created a base test contract
ExternalBridgeTestthat handles the common tests. This has a few benefits:I have moved OPL2ToL1, ArbL2ToL1, OPStack, and ERC5164 to the new base test, and I'll forgo the rest for now because of the time sensitivity of audit remediations. I've created an issue tracking the remaining work here: https://github.com/hyperlane-xyz/issues/issues/1384
Drive-by changes
None
Related issues
Related to https://github.com/chainlight-io/2024-08-hyperlane/issues/3
Backward compatibility
Yes
Testing
Unit