Support cosmos contract byte lengths other than 32 #3147
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tkporter
requested review from
yorhodes,
jmrossy and
daniel-savu
as code owners
|
jmrossy
reviewed
Jan 11, 2024
Co-authored-by: Daniel Savu <[email protected]>
…-network/abacus-monorepo into trevor/add-contract-address-bytes
… trevor/add-contract-address-bytes
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #3147 +/- ##
=======================================
Coverage 67.18% 67.18%
=======================================
Files 101 101
Lines 1021 1021
Branches 106 106
=======================================
Hits 686 686
Misses 291 291
Partials 44 44
|
jmrossy
reviewed
Jan 22, 2024
… trevor/add-contract-address-bytes
ltyu
pushed a commit
to ltyu/hyperlane-monorepo
that referenced
this pull request
Mar 13, 2024
### Description fixes hyperlane-xyz#3143 Initially went down a path that would use protocol-specific types for addresses as suggested by hyperlane-xyz#3143. I had made a `HyperlaneConnectionConf` enum with protocol-specific variants whose values were `addresses: CoreContractAddresses<ProtocolSpecificAddressType>` and `connection: ProtocolSpecificConnectionConf`. This worked pretty well until I hit the ISM logic. Because hyperlane-core is where the Mailbox trait is defined, the return type of `recipient_ism` in the trait cannot involve protocol specific types. It can't be moved to hyperlane-base because hyperlane-base imports the chain crates, so we'd have a cyclic dependency. I experimented with moving away from H256 to something like a Vec<u8> or string, but this felt a bit weird. In the end we decided to keep H256s as the global representation for contract addresses for now, with the intent of eventually changing this, and to support the varying length situation in a cosmos config ### Drive-by changes - Added some cosmos specific agent configurations into the sdk - Moved to bech32_prefix in the agents for consistency with what the SDK's chain metadata already does - I guess no one's ran cargo test in a while so vectors/message.json got a new v3 message lol ### Related issues Fixes hyperlane-xyz#3143 ### Backward compatibility Changes prefix to bech32_prefix in the agent config, and now requires `contractAddressBytes` ### Testing Tested merged with hyperlane-xyz#3144 and all worked --------- Co-authored-by: Daniel Savu <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
fixes #3143
Initially went down a path that would use protocol-specific types for addresses as suggested by #3143. I had made a
HyperlaneConnectionConfenum with protocol-specific variants whose values wereaddresses: CoreContractAddresses<ProtocolSpecificAddressType>andconnection: ProtocolSpecificConnectionConf. This worked pretty well until I hit the ISM logic.Because hyperlane-core is where the Mailbox trait is defined, the return type of
recipient_ismin the trait cannot involve protocol specific types. It can't be moved to hyperlane-base because hyperlane-base imports the chain crates, so we'd have a cyclic dependency. I experimented with moving away from H256 to something like a Vec or string, but this felt a bit weird.In the end we decided to keep H256s as the global representation for contract addresses for now, with the intent of eventually changing this, and to support the varying length situation in a cosmos config
Drive-by changes
Related issues
Fixes #3143
Backward compatibility
Changes prefix to bech32_prefix in the agent config, and now requires
contractAddressBytesTesting
Tested merged with #3144 and all worked