feat: use composable, non conflicting hyperlane metadata wire format to store forward memo and kaspa gadgets #1889
+400
−52
Conversation
This file contains hidden or 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
danwt
commented
Jun 4, 2025
x/forward/hyperlane.go
Outdated
Comment on lines
17
to
35
| func (k Forward) OnHyperlaneMessage(goCtx context.Context, args warpkeeper.OnHyperlaneMessageArgs) error { | ||
| ctx := sdk.UnwrapSDKContext(goCtx) | ||
|
|
||
| memo := args.Metadata | ||
| if len(memo) == 0 { | ||
| hlMetadata, err := types.UnpackHLMetadata(args.Metadata) | ||
| if err != nil { | ||
| return errorsmod.Wrap(err, "unpack hl metadata") | ||
| } | ||
|
|
||
| if hlMetadata == nil || len(hlMetadata.HookForwardToIbc) == 0 { | ||
| // Equivalent to the vanilla token standard. | ||
| return nil | ||
| } | ||
|
|
||
| // if it fails, the original hyperlane transfer recipient got the funds anyway so no need to do anything special (relying on frontend here) | ||
| k.executeWithErrEvent(ctx, func() error { | ||
| d, err := types.UnpackForwardToIBC(memo) | ||
| d, err := types.UnpackForwardToIBC(hlMetadata.HookForwardToIbc) | ||
| if err != nil { | ||
| return errorsmod.Wrap(err, "unpack memo from hyperlane") | ||
| } |
danwt
changed the title
omritoptix
reviewed
Jun 6, 2025
x/forward/types/dt.pb.go
Outdated
Comment on lines
118
to
122
| // optional, can be empty | ||
| HookForwardToIbc []byte `protobuf:"bytes,1,opt,name=hook_forward_to_ibc,json=hookForwardToIbc,proto3" json:"hook_forward_to_ibc,omitempty"` | ||
| // optional, can be empty | ||
| // see https://www.notion.so/dymension/ADR-Kaspa-Bridge-Implementation-206a4a51f86a803980aec7099c826fb4?source=copy_link#208a4a51f86a8093a843cf4b5e903588 | ||
| Kaspa []byte `protobuf:"bytes,2,opt,name=kaspa,proto3" json:"kaspa,omitempty"` |
There was a problem hiding this comment.
assuming they are mutually exclusive, wouldn't it be cleaner to have it as type and data (also going forward(?
There was a problem hiding this comment.
they aren't mutually exclusive
Comment on lines
12
to
13
| // was it actually a forward operation? (maybe not if they dont include forward memo) | ||
| bool was_forwarded = 3; |
There was a problem hiding this comment.
whats the use of this was_forwarded? seems like this event simply shouldn't be emitted if it's not forwarded no?
There was a problem hiding this comment.
It's a bit awkward because IMO something should be emitted if the memo isn't parseable, and at that point you don't know if a forward is intended or not
mtsitrin
approved these changes
Jun 12, 2025
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
Closes #1888
@anhductn2001 in this PR I've had to change the encoding format for the memo that needs to be included in transfers from Ethereum/Solana -> Dymension via Hyperlane. I'll probably need to update the example memos in the manual tests.
Possibly you'll need to update the e2e as well? Let me know if it suddenly fails and if you need help.
I've updated the toolling so that it's possiblett to generate the new format with cli (q forward ...)
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow-up issues.
PR review checkboxes:
I have...
Unreleasedsection inCHANGELOG.mdgodoccommentsSDK Checklist
maptime.Now()sendCoinand notSendCoinsFull security checklist here
For Reviewer:
After reviewer approval: