Skip to content

Commit

Permalink
chore(transfer): make Forwarding non-null (#6618)
Browse files Browse the repository at this point in the history
* chore(transfer): make Forwarding non-null

* chore(transfer): always validate forwarding.
  • Loading branch information
DimitrisJim authored and bznein committed Jun 18, 2024
1 parent 139facf commit 0670659
Show file tree
Hide file tree
Showing 39 changed files with 243 additions and 251 deletions.
8 changes: 4 additions & 4 deletions e2e/tests/transfer/authz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ func (suite *AuthzTransferTestSuite) TestAuthz_MsgTransfer_Succeeds() {
suite.GetTimeoutHeight(ctx, chainB),
0,
"",
nil,
transfertypes.Forwarding{},
)

protoAny, err := codectypes.NewAnyWithValue(transferMsg)
Expand Down Expand Up @@ -188,7 +188,7 @@ func (suite *AuthzTransferTestSuite) TestAuthz_MsgTransfer_Succeeds() {
suite.GetTimeoutHeight(ctx, chainB),
0,
"",
nil,
transfertypes.Forwarding{},
)

protoAny, err := codectypes.NewAnyWithValue(transferMsg)
Expand Down Expand Up @@ -271,7 +271,7 @@ func (suite *AuthzTransferTestSuite) TestAuthz_InvalidTransferAuthorizations() {
suite.GetTimeoutHeight(ctx, chainB),
0,
"",
nil,
transfertypes.Forwarding{},
)

protoAny, err := codectypes.NewAnyWithValue(transferMsg)
Expand Down Expand Up @@ -332,7 +332,7 @@ func (suite *AuthzTransferTestSuite) TestAuthz_InvalidTransferAuthorizations() {
suite.GetTimeoutHeight(ctx, chainB),
0,
"",
nil,
transfertypes.Forwarding{},
)

protoAny, err := codectypes.NewAnyWithValue(transferMsg)
Expand Down
5 changes: 3 additions & 2 deletions e2e/tests/transfer/incentivized_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/cosmos/ibc-go/e2e/testsuite/query"
"github.com/cosmos/ibc-go/e2e/testvalues"
feetypes "github.com/cosmos/ibc-go/v8/modules/apps/29-fee/types"
transfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types"
channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types"
)

Expand Down Expand Up @@ -211,7 +212,7 @@ func (s *IncentivizedTransferTestSuite) TestMsgPayPacketFee_InvalidReceiverAccou
s.GetTimeoutHeight(ctx, chainB),
0,
"",
nil,
transfertypes.Forwarding{},
)
txResp := s.BroadcastMessages(ctx, chainA, chainAWallet, msgTransfer)
// this message should be successful, as receiver account is not validated on the sending chain.
Expand Down Expand Up @@ -350,7 +351,7 @@ func (s *IncentivizedTransferTestSuite) TestMultiMsg_MsgPayPacketFeeSingleSender
s.GetTimeoutHeight(ctx, chainB),
0,
"",
nil,
transfertypes.Forwarding{},
)
resp := s.BroadcastMessages(ctx, chainA, chainAWallet, msgPayPacketFee, msgTransfer)
s.AssertTxSuccess(resp)
Expand Down
3 changes: 2 additions & 1 deletion e2e/tests/transfer/upgrades_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/cosmos/ibc-go/e2e/testsuite/query"
"github.com/cosmos/ibc-go/e2e/testvalues"
feetypes "github.com/cosmos/ibc-go/v8/modules/apps/29-fee/types"
transfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types"
channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types"
)

Expand Down Expand Up @@ -208,7 +209,7 @@ func (s *TransferChannelUpgradesTestSuite) TestChannelUpgrade_WithFeeMiddleware_
s.GetTimeoutHeight(ctx, chainB),
0,
"",
nil,
transfertypes.Forwarding{},
)
resp := s.BroadcastMessages(ctx, chainA, chainAWallet, msgPayPacketFee, msgTransfer)
s.AssertTxSuccess(resp)
Expand Down
3 changes: 2 additions & 1 deletion e2e/tests/upgrades/upgrade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/cosmos/ibc-go/e2e/testsuite/query"
"github.com/cosmos/ibc-go/e2e/testvalues"
feetypes "github.com/cosmos/ibc-go/v8/modules/apps/29-fee/types"
transfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types"
v7migrations "github.com/cosmos/ibc-go/v8/modules/core/02-client/migrations/v7"
clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
connectiontypes "github.com/cosmos/ibc-go/v8/modules/core/03-connection/types"
Expand Down Expand Up @@ -966,7 +967,7 @@ func (s *UpgradeTestSuite) TestV8ToV8_1ChainUpgrade_ChannelUpgrades() {
s.GetTimeoutHeight(ctx, chainB),
0,
"",
nil,
transfertypes.Forwarding{},
)
resp := s.BroadcastMessages(ctx, chainA, chainAWallet, msgPayPacketFee, msgTransfer)
s.AssertTxSuccess(resp)
Expand Down
2 changes: 1 addition & 1 deletion e2e/testsuite/testsuite.go
Original file line number Diff line number Diff line change
Expand Up @@ -625,7 +625,7 @@ func getValidatorsAndFullNodes(chainIdx int) (int, int) {
}

// GetMsgTransfer returns a MsgTransfer that is constructed based on the channel version
func GetMsgTransfer(portID, channelID, version string, tokens sdk.Coins, sender, receiver string, timeoutHeight clienttypes.Height, timeoutTimestamp uint64, memo string, forwarding *transfertypes.Forwarding) *transfertypes.MsgTransfer {
func GetMsgTransfer(portID, channelID, version string, tokens sdk.Coins, sender, receiver string, timeoutHeight clienttypes.Height, timeoutTimestamp uint64, memo string, forwarding transfertypes.Forwarding) *transfertypes.MsgTransfer {
if len(tokens) == 0 {
panic(errors.New("tokens cannot be empty"))
}
Expand Down
3 changes: 2 additions & 1 deletion e2e/testsuite/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"github.com/cosmos/ibc-go/e2e/testsuite/sanitize"
"github.com/cosmos/ibc-go/e2e/testvalues"
feetypes "github.com/cosmos/ibc-go/v8/modules/apps/29-fee/types"
transfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types"
clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types"
)
Expand Down Expand Up @@ -297,7 +298,7 @@ func (s *E2ETestSuite) Transfer(ctx context.Context, chain ibc.Chain, user ibc.W
transferVersion = version.AppVersion
}

msg := GetMsgTransfer(portID, channelID, transferVersion, tokens, sender, receiver, timeoutHeight, timeoutTimestamp, memo, nil)
msg := GetMsgTransfer(portID, channelID, transferVersion, tokens, sender, receiver, timeoutHeight, timeoutTimestamp, memo, transfertypes.Forwarding{})

return s.BroadcastMessages(ctx, chain, user, msg)
}
Expand Down
4 changes: 2 additions & 2 deletions modules/apps/27-interchain-accounts/host/keeper/relay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
suite.chainB.GetTimeoutHeight(),
0,
"",
nil,
transfertypes.Forwarding{},
)

data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []proto.Message{msg}, encoding)
Expand Down Expand Up @@ -389,7 +389,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
suite.chainB.GetTimeoutHeight(),
0,
"",
nil,
transfertypes.Forwarding{},
)

data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []proto.Message{msg}, encoding)
Expand Down
2 changes: 1 addition & 1 deletion modules/apps/29-fee/keeper/events_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func (suite *KeeperTestSuite) TestDistributeFeeEvent() {
path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID,
sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(100))), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(),
clienttypes.NewHeight(1, 100), 0, "",
nil,
transfertypes.Forwarding{},
)

res, err := suite.chainA.SendMsgs(msgPayPacketFee, msgTransfer)
Expand Down
4 changes: 2 additions & 2 deletions modules/apps/29-fee/transfer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (suite *FeeTestSuite) TestFeeTransfer() {

msgs := []sdk.Msg{
types.NewMsgPayPacketFee(fee, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, suite.chainA.SenderAccount.GetAddress().String(), nil),
transfertypes.NewMsgTransfer(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, tc.coinsToTransfer, suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), clienttypes.NewHeight(1, 100), 0, "", nil),
transfertypes.NewMsgTransfer(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, tc.coinsToTransfer, suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), clienttypes.NewHeight(1, 100), 0, "", transfertypes.Forwarding{}),
}

res, err := suite.chainA.SendMsgs(msgs...)
Expand Down Expand Up @@ -157,7 +157,7 @@ func (suite *FeeTestSuite) TestTransferFeeUpgrade() {
fee := types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee)
msgs := []sdk.Msg{
types.NewMsgPayPacketFee(fee, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, suite.chainA.SenderAccount.GetAddress().String(), nil),
transfertypes.NewMsgTransfer(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, sdk.NewCoins(ibctesting.TestCoin), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), clienttypes.NewHeight(1, 100), 0, "", nil),
transfertypes.NewMsgTransfer(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, sdk.NewCoins(ibctesting.TestCoin), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), clienttypes.NewHeight(1, 100), 0, "", transfertypes.Forwarding{}),
}

res, err := suite.chainA.SendMsgs(msgs...)
Expand Down
14 changes: 8 additions & 6 deletions modules/apps/callbacks/ibc_middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import (
ibcmock "github.com/cosmos/ibc-go/v8/testing/mock"
)

var emptyForwarding = transfertypes.Forwarding{}

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

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

packet = channeltypes.Packet{
Expand Down Expand Up @@ -494,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
nil,
emptyForwarding,
)

res, err := s.chainA.SendMsgs(msg)
Expand Down Expand Up @@ -662,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),
nil,
emptyForwarding,
)

packet = channeltypes.Packet{
Expand Down Expand Up @@ -794,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),
nil,
emptyForwarding,
)

packet = channeltypes.Packet{
Expand Down Expand Up @@ -1018,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: nil,
Forwarding: emptyForwarding,
}

portID := s.path.EndpointA.ChannelConfig.PortID
Expand Down
2 changes: 1 addition & 1 deletion modules/apps/callbacks/replay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ func (s *CallbacksTestSuite) ExecuteFailedTransfer(memo string) {
s.chainA.SenderAccount.GetAddress().String(),
s.chainB.SenderAccount.GetAddress().String(),
clienttypes.NewHeight(1, 100), 0, memo,
nil,
transfertypes.Forwarding{},
)

res, err := s.chainA.SendMsgs(msg)
Expand Down
4 changes: 2 additions & 2 deletions modules/apps/callbacks/transfer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ func (s *CallbacksTestSuite) ExecuteTransfer(memo string) {
s.chainA.SenderAccount.GetAddress().String(),
s.chainB.SenderAccount.GetAddress().String(),
clienttypes.NewHeight(1, 100), 0, memo,
nil,
transfertypes.Forwarding{},
)

res, err := s.chainA.SendMsgs(msg)
Expand Down Expand Up @@ -228,7 +228,7 @@ func (s *CallbacksTestSuite) ExecuteTransferTimeout(memo string) {
s.chainA.SenderAccount.GetAddress().String(),
s.chainB.SenderAccount.GetAddress().String(),
timeoutHeight, timeoutTimestamp, memo,
nil,
transfertypes.Forwarding{},
)

res, err := s.chainA.SendMsgs(msg)
Expand Down
10 changes: 5 additions & 5 deletions modules/apps/transfer/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ using the {packet-timeout-timestamp} flag. If no timeout value is set then a def
}

// If parsed, set and replace memo.
if forwarding != nil {
if len(forwarding.Hops) > 0 {
forwarding.Memo = memo
memo = ""
}
Expand Down Expand Up @@ -142,22 +142,22 @@ using the {packet-timeout-timestamp} flag. If no timeout value is set then a def

// parseForwarding parses the forwarding flag into a Forwarding object or nil if the flag is not specified. If the flag cannot
// be parsed or the hops aren't in the portID/channelID format an error is returned.
func parseForwarding(cmd *cobra.Command) (*types.Forwarding, error) {
func parseForwarding(cmd *cobra.Command) (types.Forwarding, error) {
var hops []types.Hop

forwardingString, err := cmd.Flags().GetString(flagForwarding)
if err != nil {
return nil, err
return types.Forwarding{}, err
}
if strings.TrimSpace(forwardingString) == "" {
return nil, nil
return types.Forwarding{}, nil
}

pairs := strings.Split(forwardingString, ",")
for _, pair := range pairs {
pairSplit := strings.Split(pair, "/")
if len(pairSplit) != 2 {
return nil, fmt.Errorf("expected a portID/channelID pair, found %s", pair)
return types.Forwarding{}, fmt.Errorf("expected a portID/channelID pair, found %s", pair)
}

hop := types.Hop{PortId: pairSplit[0], ChannelId: pairSplit[1]}
Expand Down
2 changes: 1 addition & 1 deletion modules/apps/transfer/ibc_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ func (im IBCModule) OnRecvPacket(

im.keeper.Logger(ctx).Info("successfully handled ICS-20 packet", "sequence", packet.Sequence)

if data.Forwarding != nil {
if data.ShouldBeForwarded() {
// NOTE: acknowledgement will be written asynchronously
return nil
}
Expand Down
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 @@ -355,7 +355,7 @@ func (suite *TransferTestSuite) TestOnRecvPacket() {
suite.chainA.SenderAccount.GetAddress().String(),
suite.chainB.SenderAccount.GetAddress().String(),
"",
nil,
types.Forwarding{},
)

tokensBz, err := json.Marshal(packetData.Tokens)
Expand Down Expand Up @@ -476,7 +476,7 @@ func (suite *TransferTestSuite) TestOnTimeoutPacket() {
timeoutHeight,
0,
"",
nil,
types.Forwarding{},
)
res, err := suite.chainA.SendMsgs(msg)
suite.Require().NoError(err) // message committed
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: nil,
Forwarding: types.Forwarding{},
}, nil
}
Loading

0 comments on commit 0670659

Please sign in to comment.