Skip to content
Merged
7 changes: 6 additions & 1 deletion modules/apps/transfer/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func (msg MsgTransfer) ValidateBasic() error {
}

// We cannot have non-empty memo and non-empty forwarding hops at the same time.
if len(msg.Forwarding.Hops) > 0 && msg.Memo != "" {
if msg.ShouldBeForwarded() && msg.Memo != "" {
return errorsmod.Wrapf(ErrInvalidMemo, "memo must be empty if forwarding path hops is not empty: %s, %s", msg.Memo, msg.Forwarding.Hops)
}

Expand All @@ -131,6 +131,11 @@ func (msg MsgTransfer) GetCoins() sdk.Coins {
return coins
}

// ShouldBeForwarded determines if the transfer should be forwarded to the next hop.
func (msg MsgTransfer) ShouldBeForwarded() bool {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wdyt about HasForwarding / HasForwardingHops?

Usually would prefer this kind of naming convention with Has or Is prefix

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That could make sense as well. I can change this if that is our usual convention

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

considering the restructuring we will do might be best to leave on hold for time being until it becomes clear what these conditions will end up being (for e.g ShouldBeForwarded would probably also need to check the unwind field of msg transfer).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or smash it in now and refactor, same to me 😅

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I'll merge this now to get it out of the way, but we can revisit this later when we know more about the unwind and all the other discussion related to this.

return len(msg.Forwarding.Hops) > 0
}

// isValidIBCCoin returns true if the token provided is valid,
// and should be used to transfer tokens.
func isValidIBCCoin(coin sdk.Coin) bool {
Expand Down
2 changes: 1 addition & 1 deletion modules/apps/transfer/types/packet.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ func (ftpd FungibleTokenPacketDataV2) ValidateBasic() error {
}

// We cannot have non-empty memo and non-empty forwarding path hops at the same time.
if len(ftpd.Forwarding.Hops) > 0 && ftpd.Memo != "" {
if ftpd.ShouldBeForwarded() && ftpd.Memo != "" {
return errorsmod.Wrapf(ErrInvalidMemo, "memo must be empty if forwarding path hops is not empty: %s, %s", ftpd.Memo, ftpd.Forwarding.Hops)
}

Expand Down
2 changes: 1 addition & 1 deletion modules/apps/transfer/types/transfer_authorization.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func (a TransferAuthorization) Accept(goCtx context.Context, msg proto.Message)

memo := msgTransfer.Memo
// in the case of forwarded transfers, the actual memo is stored in the forwarding path until the final destination
if len(msgTransfer.Forwarding.Hops) > 0 {
if msgTransfer.ShouldBeForwarded() {
memo = msgTransfer.Forwarding.Memo
}

Expand Down