fix: reuse SDK transaction types in submitters #4598
Conversation
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4598 +/- ##
=======================================
Coverage 73.89% 73.89%
=======================================
Files 100 100
Lines 1421 1421
Branches 180 180
=======================================
Hits 1050 1050
Misses 350 350
Partials 21 21
|
paulbalaji
requested review from
tkporter,
Mo-Hussain,
jmrossy,
yorhodes and
ltyu
as code owners
| @@ -113,5 +113,6 @@ export type ParsedLegacyMultisigIsmMetadata = { | |||
| }; | |||
|
|
|||
| export type Annotated<T> = T & { | |||
| chain: string; // TODO: Change to ChainName after moving to SDK from Utils | |||
There was a problem hiding this comment.
bit confused by this as the transactions chain ID should always map 1:1 to a chain name
There was a problem hiding this comment.
but if we have two chains with the same chainid, we no longer know which one we're submitting to from the chainId alone
There was a problem hiding this comment.
here's an example in the duplicate-chainids pr where having the chain directly is helpful https://github.com/hyperlane-xyz/hyperlane-monorepo/pull/4599/files#diff-d572648c3e0dd7a6d4eff6670862ae1bcb5f1c1f3feb8c0d70fb60a765ae7d48
There was a problem hiding this comment.
oh I see, I misunderstood
Description
Updating the types in the submitters because they were using their own types instead of the existing ones for some reason. Also adding
chainas an argument to theAnnotatedtype, so that we can take advantage of this in #4599 and defend ourselves against futures where chainid/domainid are not the same for EVMThis also fixes the issue currently where there's a small type mismatch between submitters and modules
Drive-by changes
createTransferOwnershipTxfrom the abstract hyperlane module to the evm module deployer, as it's EVM specificRelated issues
useful for #4599, but also something that needs to be fixed itself
Backward compatibility
Yes
Testing
ci