Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor forwarding messages for Transfer and Packet #6655

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
2a0225e
feat(transfer): add unwind, refactor proto structure. gen-all
DimitrisJim Jun 20, 2024
04127e7
tests(transfer/types): fix test failures in types tests.
DimitrisJim Jun 20, 2024
0e3552a
tests(transfer/keeper): fix test failures in keeper tests.
DimitrisJim Jun 20, 2024
878b64c
cli(transfer): fix cli usage. pending flag for unwind.
DimitrisJim Jun 20, 2024
23cf902
tests(callbacks): fix failing tests in callbacks.
DimitrisJim Jun 20, 2024
acc9a95
tests(transfer/internal): fix failures in internal package.
DimitrisJim Jun 20, 2024
74a6a9a
tests(transfer): fix test failures in top level tranfer package.
DimitrisJim Jun 20, 2024
d68b2e4
tests(ica/host/keeper): fix repr of msg transfer in ica host msg exec…
DimitrisJim Jun 20, 2024
040b4fe
lint(all): lint this bad boy
DimitrisJim Jun 20, 2024
3a59487
chore(transfer/types): amend validation for MsgTransfer's ShouldBeFor…
DimitrisJim Jun 20, 2024
18d4927
nit(self): only pass relevant fields to create packet data; minor com…
DimitrisJim Jun 20, 2024
5f78cc2
Apply suggestions from code review
DimitrisJim Jun 20, 2024
babd20f
Merge branch 'feat/ics20-v2-path-forwarding' into jim/refactor-forwar…
DimitrisJim Jun 20, 2024
f01fb8c
chore(merge): fix merge issues.
DimitrisJim Jun 20, 2024
80524a3
chore(proto): mention optional nature of fields.
DimitrisJim Jun 20, 2024
a343a25
memo: do not drop it
DimitrisJim Jun 20, 2024
ba5bbe0
validation: drop requirement on memo being empty on msg transfer.
DimitrisJim Jun 20, 2024
30d3900
Merge branch 'feat/ics20-v2-path-forwarding' into jim/refactor-forwar…
crodriguezvega Jun 20, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -798,7 +798,7 @@ func (suite *KeeperTestSuite) TestJSONOnRecvPacket() {
"timeout_height": { "revision_number": 1, "revision_height": 100 },
"timeout_timestamp": 0,
"memo": "",
"forwarding": { "hops": [], "memo": "" }
"forwarding": { "hops": [], "unwind": false }
}
]
}`)
Expand Down
14 changes: 7 additions & 7 deletions modules/apps/callbacks/ibc_middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
ibcmock "github.com/cosmos/ibc-go/v8/testing/mock"
)

var emptyForwarding = transfertypes.Forwarding{}
var emptyForwardingPacketData = transfertypes.ForwardingPacketData{}

func (s *CallbacksTestSuite) TestNewIBCMiddleware() {
testCases := []struct {
Expand Down Expand Up @@ -188,7 +188,7 @@ func (s *CallbacksTestSuite) TestSendPacket() {
ibctesting.TestAccAddress,
ibctesting.TestAccAddress,
fmt.Sprintf(`{"src_callback": {"address": "%s"}}`, simapp.SuccessContract),
emptyForwarding,
emptyForwardingPacketData,
)

chanCap := s.path.EndpointA.Chain.GetChannelCapability(s.path.EndpointA.ChannelConfig.PortID, s.path.EndpointA.ChannelID)
Expand Down Expand Up @@ -330,7 +330,7 @@ func (s *CallbacksTestSuite) TestOnAcknowledgementPacket() {
ibctesting.TestAccAddress,
ibctesting.TestAccAddress,
fmt.Sprintf(`{"src_callback": {"address":"%s", "gas_limit":"%d"}}`, simapp.SuccessContract, userGasLimit),
emptyForwarding,
emptyForwardingPacketData,
)

packet = channeltypes.Packet{
Expand Down Expand Up @@ -496,7 +496,7 @@ func (s *CallbacksTestSuite) TestOnTimeoutPacket() {
sdk.NewCoins(ibctesting.TestCoin), s.chainA.SenderAccount.GetAddress().String(),
s.chainB.SenderAccount.GetAddress().String(), clienttypes.ZeroHeight(), timeoutTimestamp,
fmt.Sprintf(`{"src_callback": {"address":"%s", "gas_limit":"%d"}}`, ibctesting.TestAccAddress, userGasLimit), // set user gas limit above panic level in mock contract keeper
emptyForwarding,
transfertypes.Forwarding{},
)

res, err := s.chainA.SendMsgs(msg)
Expand Down Expand Up @@ -664,7 +664,7 @@ func (s *CallbacksTestSuite) TestOnRecvPacket() {
ibctesting.TestAccAddress,
s.chainB.SenderAccount.GetAddress().String(),
fmt.Sprintf(`{"dest_callback": {"address":"%s", "gas_limit":"%d"}}`, ibctesting.TestAccAddress, userGasLimit),
emptyForwarding,
emptyForwardingPacketData,
)

packet = channeltypes.Packet{
Expand Down Expand Up @@ -796,7 +796,7 @@ func (s *CallbacksTestSuite) TestWriteAcknowledgement() {
ibctesting.TestAccAddress,
s.chainB.SenderAccount.GetAddress().String(),
fmt.Sprintf(`{"dest_callback": {"address":"%s", "gas_limit":"600000"}}`, ibctesting.TestAccAddress),
emptyForwarding,
emptyForwardingPacketData,
)

packet = channeltypes.Packet{
Expand Down Expand Up @@ -1020,7 +1020,7 @@ func (s *CallbacksTestSuite) TestUnmarshalPacketDataV1() {
Sender: ibctesting.TestAccAddress,
Receiver: ibctesting.TestAccAddress,
Memo: fmt.Sprintf(`{"src_callback": {"address": "%s"}, "dest_callback": {"address":"%s"}}`, ibctesting.TestAccAddress, ibctesting.TestAccAddress),
Forwarding: emptyForwarding,
Forwarding: emptyForwardingPacketData,
}

portID := s.path.EndpointA.ChannelConfig.PortID
Expand Down
10 changes: 2 additions & 8 deletions modules/apps/transfer/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,6 @@ using the {packet-timeout-timestamp} flag. If no timeout value is set then a def
return err
}

// If parsed, set and replace memo.
if len(forwarding.Hops) > 0 {
forwarding.Memo = memo
memo = ""
}

// NOTE: relative timeouts using block height are not supported.
// if the timeouts are not absolute, CLI users rely solely on local clock time in order to calculate relative timestamps.
if !absoluteTimeouts {
Expand Down Expand Up @@ -164,6 +158,6 @@ func parseForwarding(cmd *cobra.Command) (types.Forwarding, error) {
hops = append(hops, hop)
}

forwarding := types.NewForwarding("", hops...)
return forwarding, nil
// TODO(jim): Add flag for unwind value
Copy link
Contributor Author

Choose a reason for hiding this comment

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

return types.NewForwarding(false, hops...), nil
}
4 changes: 2 additions & 2 deletions modules/apps/transfer/ibc_module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ func (suite *TransferTestSuite) TestOnRecvPacket() {
suite.chainA.SenderAccount.GetAddress().String(),
suite.chainB.SenderAccount.GetAddress().String(),
"",
types.NewForwarding("", types.Hop{PortId: "transfer", ChannelId: "channel-0"}),
types.NewForwardingPacketData("", types.Hop{PortId: "transfer", ChannelId: "channel-0"}),
)
packet.Data = packetData.GetBytes()

Expand Down Expand Up @@ -368,7 +368,7 @@ func (suite *TransferTestSuite) TestOnRecvPacket() {
suite.chainA.SenderAccount.GetAddress().String(),
suite.chainB.SenderAccount.GetAddress().String(),
"",
types.Forwarding{},
types.ForwardingPacketData{},
)

tokensBz, err := json.Marshal(packetData.Tokens)
Expand Down
2 changes: 1 addition & 1 deletion modules/apps/transfer/internal/convert/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,6 @@ func PacketDataV1ToV2(packetData types.FungibleTokenPacketData) (types.FungibleT
Sender: packetData.Sender,
Receiver: packetData.Receiver,
Memo: packetData.Memo,
Forwarding: types.Forwarding{},
Forwarding: types.ForwardingPacketData{},
}, nil
}
6 changes: 3 additions & 3 deletions modules/apps/transfer/internal/events/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func EmitTransferEvent(ctx sdk.Context, sender, receiver string, tokens types.To
// EmitOnRecvPacketEvent emits a fungible token packet event in the OnRecvPacket callback
func EmitOnRecvPacketEvent(ctx sdk.Context, packetData types.FungibleTokenPacketDataV2, ack channeltypes.Acknowledgement, ackErr error) {
tokensStr := mustMarshalType[types.Tokens](packetData.Tokens)
forwardingStr := mustMarshalType[types.Forwarding](packetData.Forwarding)
forwardingStr := mustMarshalType[types.ForwardingPacketData](packetData.Forwarding)

eventAttributes := []sdk.Attribute{
sdk.NewAttribute(types.AttributeKeySender, packetData.Sender),
Expand Down Expand Up @@ -64,7 +64,7 @@ func EmitOnRecvPacketEvent(ctx sdk.Context, packetData types.FungibleTokenPacket
// EmitOnAcknowledgementPacketEvent emits a fungible token packet event in the OnAcknowledgementPacket callback
func EmitOnAcknowledgementPacketEvent(ctx sdk.Context, packetData types.FungibleTokenPacketDataV2, ack channeltypes.Acknowledgement) {
tokensStr := mustMarshalType[types.Tokens](packetData.Tokens)
forwardingStr := mustMarshalType[types.Forwarding](packetData.Forwarding)
forwardingStr := mustMarshalType[types.ForwardingPacketData](packetData.Forwarding)

ctx.EventManager().EmitEvents(sdk.Events{
sdk.NewEvent(
Expand Down Expand Up @@ -103,7 +103,7 @@ func EmitOnAcknowledgementPacketEvent(ctx sdk.Context, packetData types.Fungible
// EmitOnTimeoutEvent emits a fungible token packet event in the OnTimeoutPacket callback
func EmitOnTimeoutEvent(ctx sdk.Context, packetData types.FungibleTokenPacketDataV2) {
tokensStr := mustMarshalType[types.Tokens](packetData.Tokens)
forwardingStr := mustMarshalType[types.Forwarding](packetData.Forwarding)
forwardingStr := mustMarshalType[types.ForwardingPacketData](packetData.Forwarding)

ctx.EventManager().EmitEvents(sdk.Events{
sdk.NewEvent(
Expand Down
18 changes: 9 additions & 9 deletions modules/apps/transfer/internal/packet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
"github.com/cosmos/ibc-go/v8/modules/apps/transfer/types"
)

var emptyForwarding = types.Forwarding{}
var emptyForwardingPacketData = types.ForwardingPacketData{}

func TestUnmarshalPacketData(t *testing.T) {
var (
Expand All @@ -37,7 +37,7 @@ func TestUnmarshalPacketData(t *testing.T) {
Denom: types.NewDenom("atom", types.NewTrace("transfer", "channel-0")),
Amount: "1000",
},
}, "sender", "receiver", "", emptyForwarding)
}, "sender", "receiver", "", emptyForwardingPacketData)

packetDataBz = packetData.GetBytes()
version = types.V2
Expand Down Expand Up @@ -94,7 +94,7 @@ func TestPacketV1ToPacketV2(t *testing.T) {
Denom: types.NewDenom("atom", types.NewTrace("transfer", "channel-0")),
Amount: "1000",
},
}, sender, receiver, "", emptyForwarding),
}, sender, receiver, "", emptyForwardingPacketData),
nil,
},
{
Expand All @@ -106,7 +106,7 @@ func TestPacketV1ToPacketV2(t *testing.T) {
Denom: types.NewDenom("atom"),
Amount: "1000",
},
}, sender, receiver, "", emptyForwarding),
}, sender, receiver, "", emptyForwardingPacketData),
nil,
},
{
Expand All @@ -118,7 +118,7 @@ func TestPacketV1ToPacketV2(t *testing.T) {
Denom: types.NewDenom("atom/withslash", types.NewTrace("transfer", "channel-0")),
Amount: "1000",
},
}, sender, receiver, "", emptyForwarding),
}, sender, receiver, "", emptyForwardingPacketData),
nil,
},
{
Expand All @@ -130,7 +130,7 @@ func TestPacketV1ToPacketV2(t *testing.T) {
Denom: types.NewDenom("atom/", types.NewTrace("transfer", "channel-0")),
Amount: "1000",
},
}, sender, receiver, "", emptyForwarding),
}, sender, receiver, "", emptyForwardingPacketData),
nil,
},
{
Expand All @@ -142,7 +142,7 @@ func TestPacketV1ToPacketV2(t *testing.T) {
Denom: types.NewDenom("atom/pool", types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1")),
Amount: "1000",
},
}, sender, receiver, "", emptyForwarding),
}, sender, receiver, "", emptyForwardingPacketData),
nil,
},
{
Expand All @@ -154,7 +154,7 @@ func TestPacketV1ToPacketV2(t *testing.T) {
Denom: types.NewDenom("atom", types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1"), types.NewTrace("transfer-custom", "channel-2")),
Amount: "1000",
},
}, sender, receiver, "", emptyForwarding),
}, sender, receiver, "", emptyForwardingPacketData),
nil,
},
{
Expand All @@ -166,7 +166,7 @@ func TestPacketV1ToPacketV2(t *testing.T) {
Denom: types.NewDenom("atom/pool", types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1"), types.NewTrace("transfer-custom", "channel-2")),
Amount: "1000",
},
}, sender, receiver, "", emptyForwarding),
}, sender, receiver, "", emptyForwardingPacketData),
nil,
},
{
Expand Down
4 changes: 2 additions & 2 deletions modules/apps/transfer/keeper/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,6 @@ func (k Keeper) TokenFromCoin(ctx sdk.Context, coin sdk.Coin) (types.Token, erro
}

// CreatePacketDataBytesFromVersion is a wrapper around createPacketDataBytesFromVersion for testing purposes
func CreatePacketDataBytesFromVersion(appVersion, sender, receiver, memo string, tokens types.Tokens, forwarding types.Forwarding) []byte {
return createPacketDataBytesFromVersion(appVersion, sender, receiver, memo, tokens, forwarding)
func CreatePacketDataBytesFromVersion(appVersion, sender, receiver, memo string, tokens types.Tokens, hops []types.Hop) []byte {
return createPacketDataBytesFromVersion(appVersion, sender, receiver, memo, tokens, hops)
}
10 changes: 3 additions & 7 deletions modules/apps/transfer/keeper/forwarding.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,9 @@ import (

// forwardPacket forwards a fungible FungibleTokenPacketDataV2 to the next hop in the forwarding path.
func (k Keeper) forwardPacket(ctx sdk.Context, data types.FungibleTokenPacketDataV2, packet channeltypes.Packet, receivedCoins sdk.Coins) error {
var memo string

var nextForwardingPath types.Forwarding
if len(data.Forwarding.Hops) == 1 {
memo = data.Forwarding.Memo
} else {
nextForwardingPath = types.NewForwarding(data.Forwarding.Memo, data.Forwarding.Hops[1:]...)
if len(data.Forwarding.Hops) > 1 {
nextForwardingPath = types.NewForwarding(false, data.Forwarding.Hops[1:]...)
}

// sending from the forward escrow address to the original receiver address.
Expand All @@ -34,7 +30,7 @@ func (k Keeper) forwardPacket(ctx sdk.Context, data types.FungibleTokenPacketDat
data.Receiver,
clienttypes.ZeroHeight(),
packet.TimeoutTimestamp,
memo,
data.Forwarding.DestinationMemo,
nextForwardingPath,
)

Expand Down
2 changes: 1 addition & 1 deletion modules/apps/transfer/keeper/mbt_relay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ func FungibleTokenPacketFromTla(packet TlaFungibleTokenPacket) FungibleTokenPack
AddressFromString(packet.Data.Sender),
AddressFromString(packet.Data.Receiver),
"",
types.Forwarding{},
types.ForwardingPacketData{},
),
}
}
Expand Down
13 changes: 10 additions & 3 deletions modules/apps/transfer/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ func (k Keeper) sendTransfer(
tokens = append(tokens, token)
}

packetDataBytes := createPacketDataBytesFromVersion(appVersion, sender.String(), receiver, memo, tokens, forwarding)
packetDataBytes := createPacketDataBytesFromVersion(appVersion, sender.String(), receiver, memo, tokens, forwarding.Hops)

sequence, err := k.ics4Wrapper.SendPacket(ctx, channelCap, sourcePort, sourceChannel, timeoutHeight, timeoutTimestamp, packetDataBytes)
if err != nil {
Expand Down Expand Up @@ -431,7 +431,7 @@ func (k Keeper) tokenFromCoin(ctx sdk.Context, coin sdk.Coin) (types.Token, erro
}

// createPacketDataBytesFromVersion creates the packet data bytes to be sent based on the application version.
func createPacketDataBytesFromVersion(appVersion, sender, receiver, memo string, tokens types.Tokens, forwarding types.Forwarding) []byte {
func createPacketDataBytesFromVersion(appVersion, sender, receiver, memo string, tokens types.Tokens, hops []types.Hop) []byte {
var packetDataBytes []byte
switch appVersion {
case types.V1:
Expand All @@ -444,7 +444,14 @@ func createPacketDataBytesFromVersion(appVersion, sender, receiver, memo string,
packetData := types.NewFungibleTokenPacketData(token.Denom.Path(), token.Amount, sender, receiver, memo)
packetDataBytes = packetData.GetBytes()
case types.V2:
packetData := types.NewFungibleTokenPacketDataV2(tokens, sender, receiver, memo, forwarding)
// If forwarding is needed, move memo to forwarding packet data and set packet.Memo to empty string.
var forwardingPacketData types.ForwardingPacketData
if len(hops) > 0 {
forwardingPacketData = types.NewForwardingPacketData(memo, hops...)
memo = ""
}

packetData := types.NewFungibleTokenPacketDataV2(tokens, sender, receiver, memo, forwardingPacketData)
packetDataBytes = packetData.GetBytes()
default:
panic(fmt.Errorf("app version must be one of %s", types.SupportedVersions))
Expand Down
Loading
Loading