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.

Sign up for GitHub

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

MsgTransfer For Eureka #7957

Merged
merged 13 commits into from
Feb 24, 2025
Merged

MsgTransfer For Eureka #7957

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
ac136ca
initial progress
AdityaSripal Feb 11, 2025
83d35d6
progress
AdityaSripal Feb 11, 2025
e1f325a
Merge branch 'main' of github.com:cosmos/ibc-go into aditya/msgtransf…
AdityaSripal Feb 11, 2025
635a2a2
initial build and existing tests pass
AdityaSripal Feb 11, 2025
556dded
testing
AdityaSripal Feb 11, 2025
3e12678
documentation for tests
AdityaSripal Feb 11, 2025
3e64c07
refactor
AdityaSripal Feb 12, 2025
6750736
merge
AdityaSripal Feb 18, 2025
b7138c5
fix regression
AdityaSripal Feb 18, 2025
80947c0
Merge branch 'main' into aditya/msgtransfer-eureka
gjermundgaraba Feb 21, 2025
fea0633
Update modules/apps/transfer/types/packet.go
AdityaSripal Feb 21, 2025
80a7547
remove clientKeeperv2 necessity in code
AdityaSripal Feb 21, 2025
2a10a5e
remove clientkeeperv2 in transfer keeper
AdityaSripal Feb 21, 2025
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
2 changes: 2 additions & 0 deletions modules/apps/callbacks/testing/simapp/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,8 @@ func NewSimApp(
appCodec, runtime.NewKVStoreService(keys[ibctransfertypes.StoreKey]), app.GetSubspace(ibctransfertypes.ModuleName),
app.IBCFeeKeeper, // ISC4 Wrapper: fee IBC middleware
app.IBCKeeper.ChannelKeeper,
app.IBCKeeper.ChannelKeeperV2,
app.IBCKeeper.ClientV2Keeper,
app.AccountKeeper, app.BankKeeper,
authtypes.NewModuleAddress(govtypes.ModuleName).String(),
)
Expand Down
30 changes: 18 additions & 12 deletions modules/apps/transfer/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,12 @@ type Keeper struct {
cdc codec.BinaryCodec
legacySubspace types.ParamSubspace

ics4Wrapper porttypes.ICS4Wrapper
channelKeeper types.ChannelKeeper
AuthKeeper types.AccountKeeper
BankKeeper types.BankKeeper
ics4Wrapper porttypes.ICS4Wrapper
channelKeeper types.ChannelKeeper
channelKeeperV2 types.ChannelKeeperV2
clientKeeperV2 types.ClientKeeperV2
Copy link
Member Author

Choose a reason for hiding this comment

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

I could remove this if desired

Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice to have less stuff to put in here, but what would that mean for the code?

AuthKeeper types.AccountKeeper
BankKeeper types.BankKeeper

// the address capable of executing a MsgUpdateParams message. Typically, this
// should be the x/gov module account.
Expand All @@ -47,6 +49,8 @@ func NewKeeper(
legacySubspace types.ParamSubspace,
ics4Wrapper porttypes.ICS4Wrapper,
channelKeeper types.ChannelKeeper,
channelKeeperV2 types.ChannelKeeperV2,
clientKeeperV2 types.ClientKeeperV2,
authKeeper types.AccountKeeper,
bankKeeper types.BankKeeper,
authority string,
Expand All @@ -61,14 +65,16 @@ func NewKeeper(
}

return Keeper{
cdc: cdc,
storeService: storeService,
legacySubspace: legacySubspace,
ics4Wrapper: ics4Wrapper,
channelKeeper: channelKeeper,
AuthKeeper: authKeeper,
BankKeeper: bankKeeper,
authority: authority,
cdc: cdc,
storeService: storeService,
legacySubspace: legacySubspace,
ics4Wrapper: ics4Wrapper,
channelKeeper: channelKeeper,
channelKeeperV2: channelKeeperV2,
clientKeeperV2: clientKeeperV2,
AuthKeeper: authKeeper,
BankKeeper: bankKeeper,
authority: authority,
}
}

Expand Down
6 changes: 6 additions & 0 deletions modules/apps/transfer/keeper/keeper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ func (suite *KeeperTestSuite) TestNewKeeper() {
suite.chainA.GetSimApp().GetSubspace(types.ModuleName),
suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper,
suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper,
suite.chainA.GetSimApp().IBCKeeper.ChannelKeeperV2,
suite.chainA.App.GetIBCKeeper().ClientV2Keeper,
suite.chainA.GetSimApp().AccountKeeper,
suite.chainA.GetSimApp().BankKeeper,
suite.chainA.GetSimApp().ICAControllerKeeper.GetAuthority(),
Expand All @@ -72,6 +74,8 @@ func (suite *KeeperTestSuite) TestNewKeeper() {
suite.chainA.GetSimApp().GetSubspace(types.ModuleName),
suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper,
suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper,
suite.chainA.GetSimApp().IBCKeeper.ChannelKeeperV2,
suite.chainA.App.GetIBCKeeper().ClientV2Keeper,
authkeeper.AccountKeeper{}, // empty account keeper
suite.chainA.GetSimApp().BankKeeper,
suite.chainA.GetSimApp().ICAControllerKeeper.GetAuthority(),
Expand All @@ -84,6 +88,8 @@ func (suite *KeeperTestSuite) TestNewKeeper() {
suite.chainA.GetSimApp().GetSubspace(types.ModuleName),
suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper,
suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper,
suite.chainA.GetSimApp().IBCKeeper.ChannelKeeperV2,
suite.chainA.App.GetIBCKeeper().ClientV2Keeper,
suite.chainA.GetSimApp().AccountKeeper,
suite.chainA.GetSimApp().BankKeeper,
"", // authority
Expand Down
89 changes: 65 additions & 24 deletions modules/apps/transfer/keeper/msg_server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/cosmos/ibc-go/v9/modules/apps/transfer/internal/telemetry"
"github.com/cosmos/ibc-go/v9/modules/apps/transfer/types"
channeltypes "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types"
channeltypesv2 "github.com/cosmos/ibc-go/v9/modules/core/04-channel/v2/types"
ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors"
)

Expand All @@ -29,14 +30,19 @@ func (k Keeper) Transfer(goCtx context.Context, msg *types.MsgTransfer) (*types.
return nil, err
}

channel, found := k.channelKeeper.GetChannel(ctx, msg.SourcePort, msg.SourceChannel)
if !found {
return nil, errorsmod.Wrapf(channeltypes.ErrChannelNotFound, "port ID (%s) channel ID (%s)", msg.SourcePort, msg.SourceChannel)
}

appVersion, found := k.ics4Wrapper.GetAppVersion(ctx, msg.SourcePort, msg.SourceChannel)
if !found {
return nil, errorsmod.Wrapf(ibcerrors.ErrInvalidRequest, "application version not found for source port: %s and source channel: %s", msg.SourcePort, msg.SourceChannel)
// if a eurela counterparty exists with source channel, then use IBC V2 protocol
// otherwise use IBC V1 protocol
var (
channel channeltypes.Channel
isIBCV2 bool
)
_, isIBCV2 = k.clientKeeperV2.GetClientCounterparty(ctx, msg.SourceChannel)
if !isIBCV2 {
var found bool
channel, found = k.channelKeeper.GetChannel(ctx, msg.SourcePort, msg.SourceChannel)
if !found {
return nil, errorsmod.Wrapf(channeltypes.ErrChannelNotFound, "port ID (%s) channel ID (%s)", msg.SourcePort, msg.SourceChannel)
Copy link
Member Author

Choose a reason for hiding this comment

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

i could choose to not have clientKeeperV2 in transfer keeper and just send the channelID from the msg directly to chanKeeperV2 and have it error if counterparty doesn't exist. But then we get a counterparty not found error over the channel not found error

}
}

coin := msg.Token
Expand All @@ -54,27 +60,62 @@ func (k Keeper) Transfer(goCtx context.Context, msg *types.MsgTransfer) (*types.
return nil, err
}

if err := k.SendTransfer(ctx, msg.SourcePort, msg.SourceChannel, token, sender); err != nil {
return nil, err
}
packetData := types.NewFungibleTokenPacketData(token.Denom.Path(), token.Amount, sender.String(), msg.Receiver, msg.Memo)

packetDataBytes, err := createPacketDataBytesFromVersion(
appVersion, sender.String(), msg.Receiver, msg.Memo, token,
)
if err != nil {
return nil, err
if err := packetData.ValidateBasic(); err != nil {
return nil, errorsmod.Wrapf(err, "failed to validate %s packet data", types.V1)
}

sequence, err := k.ics4Wrapper.SendPacket(ctx, msg.SourcePort, msg.SourceChannel, msg.TimeoutHeight, msg.TimeoutTimestamp, packetDataBytes)
if err != nil {
return nil, err
}
var sequence uint64
if isIBCV2 {
encoding := msg.Encoding
if encoding == "" {
encoding = types.EncodingJSON
Copy link
Member Author

Choose a reason for hiding this comment

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

default to JSON since that is expected ics20-v1 behaviour

}

data, err := types.MarshalPacketData(packetData, types.V1, encoding)
if err != nil {
return nil, err
}

payload := channeltypesv2.NewPayload(
types.PortID, types.PortID,
types.V1, encoding, data,
)
msg := channeltypesv2.NewMsgSendPacket(
msg.SourceChannel, msg.TimeoutTimestamp,
sender.String(), payload,
)

res, err := k.channelKeeperV2.SendPacket(ctx, msg)
Copy link
Member Author

Choose a reason for hiding this comment

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

SendPacket will call transfer module's OnSendPacket

if err != nil {
return nil, err
}

sequence = res.Sequence
} else {

events.EmitTransferEvent(ctx, sender.String(), msg.Receiver, token, msg.Memo)
if err := k.SendTransfer(ctx, msg.SourcePort, msg.SourceChannel, token, sender); err != nil {
return nil, err
}

packetDataBytes, err := createPacketDataBytesFromVersion(
types.V1, sender.String(), msg.Receiver, msg.Memo, token,
)
if err != nil {
return nil, err
}

destinationPort := channel.Counterparty.PortId
destinationChannel := channel.Counterparty.ChannelId
telemetry.ReportTransfer(msg.SourcePort, msg.SourceChannel, destinationPort, destinationChannel, token)
sequence, err = k.ics4Wrapper.SendPacket(ctx, msg.SourcePort, msg.SourceChannel, msg.TimeoutHeight, msg.TimeoutTimestamp, packetDataBytes)
if err != nil {
return nil, err
}

events.EmitTransferEvent(ctx, sender.String(), msg.Receiver, token, msg.Memo)
destinationPort := channel.Counterparty.PortId
destinationChannel := channel.Counterparty.ChannelId
telemetry.ReportTransfer(msg.SourcePort, msg.SourceChannel, destinationPort, destinationChannel, token)
}

k.Logger(ctx).Info("IBC fungible token transfer", "token", coin, "sender", msg.Sender, "receiver", msg.Receiver)

Expand Down
147 changes: 147 additions & 0 deletions modules/apps/transfer/keeper/msg_server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package keeper_test
import (
"errors"
"strings"
"time"

sdk "github.com/cosmos/cosmos-sdk/types"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
Expand All @@ -11,6 +12,7 @@ import (
abci "github.com/cometbft/cometbft/abci/types"

"github.com/cosmos/ibc-go/v9/modules/apps/transfer/types"
clienttypes "github.com/cosmos/ibc-go/v9/modules/core/02-client/types"
channeltypes "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types"
ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors"
ibctesting "github.com/cosmos/ibc-go/v9/testing"
Expand Down Expand Up @@ -160,6 +162,151 @@ func (suite *KeeperTestSuite) TestMsgTransfer() {
}
}

// TestMsgTransfer tests Transfer rpc handler with IBC V2 protocol
func (suite *KeeperTestSuite) TestMsgTransferIBCV2() {
var msg *types.MsgTransfer
var path *ibctesting.Path

testCases := []struct {
name string
malleate func()
expError error
}{
{
"success",
func() {
msg.Token = ibctesting.TestCoin
},
nil,
},
{
"bank send enabled for denoms",
func() {
err := suite.chainA.GetSimApp().BankKeeper.SetParams(suite.chainA.GetContext(),
banktypes.Params{
SendEnabled: []*banktypes.SendEnabled{
{Denom: sdk.DefaultBondDenom, Enabled: true},
{Denom: ibctesting.SecondaryDenom, Enabled: true},
},
},
)
suite.Require().NoError(err)
},
nil,
},
{
"failure: send transfers disabled",
func() {
suite.chainA.GetSimApp().TransferKeeper.SetParams(suite.chainA.GetContext(),
types.Params{
SendEnabled: false,
},
)
},
types.ErrSendDisabled,
},
{
"failure: invalid sender",
func() {
msg.Sender = "address"
},
errors.New("decoding bech32 failed"),
},
{
"failure: sender is a blocked address",
func() {
msg.Sender = suite.chainA.GetSimApp().AccountKeeper.GetModuleAddress(minttypes.ModuleName).String()
},
ibcerrors.ErrUnauthorized,
},
{
"failure: bank send disabled",
func() {
err := suite.chainA.GetSimApp().BankKeeper.SetParams(suite.chainA.GetContext(),
banktypes.Params{
SendEnabled: []*banktypes.SendEnabled{{Denom: sdk.DefaultBondDenom, Enabled: false}},
},
)
suite.Require().NoError(err)
},
types.ErrSendDisabled,
},
{
"failure: client does not exist",
func() {
msg.SourceChannel = "07-tendermint-500"
},
channeltypes.ErrChannelNotFound,
},
}

for _, tc := range testCases {
tc := tc

suite.Run(tc.name, func() {
suite.SetupTest()

path = ibctesting.NewPath(suite.chainA, suite.chainB)
path.SetupV2()

timeoutTimestamp := uint64(suite.chainA.GetContext().BlockTime().Add(time.Hour).Unix())

msg = types.NewMsgTransfer(
types.PortID,
path.EndpointA.ClientID,
ibctesting.TestCoin,
suite.chainA.SenderAccount.GetAddress().String(),
suite.chainB.SenderAccount.GetAddress().String(),
clienttypes.Height{}, timeoutTimestamp, // only use timeout timestamp
"memo",
)

// send some coins of the second denom from bank module to the sender account as well
err := suite.chainA.GetSimApp().BankKeeper.MintCoins(suite.chainA.GetContext(), types.ModuleName, sdk.NewCoins(ibctesting.SecondaryTestCoin))
suite.Require().NoError(err)
err = suite.chainA.GetSimApp().BankKeeper.SendCoinsFromModuleToAccount(suite.chainA.GetContext(), types.ModuleName, suite.chainA.SenderAccount.GetAddress(), sdk.NewCoins(ibctesting.SecondaryTestCoin))
suite.Require().NoError(err)

tc.malleate()

ctx := suite.chainA.GetContext()

token, err := suite.chainA.GetSimApp().TransferKeeper.TokenFromCoin(ctx, msg.Token)
suite.Require().NoError(err)

res, err := suite.chainA.GetSimApp().TransferKeeper.Transfer(ctx, msg)

// Verify events
var expEvents []abci.Event
events := ctx.EventManager().Events().ToABCIEvents()

expEvents = sdk.Events{
sdk.NewEvent(types.EventTypeTransfer,
sdk.NewAttribute(types.AttributeKeySender, msg.Sender),
sdk.NewAttribute(types.AttributeKeyReceiver, msg.Receiver),
sdk.NewAttribute(types.AttributeKeyDenom, token.Denom.Path()),
sdk.NewAttribute(types.AttributeKeyAmount, msg.Token.Amount.String()),
sdk.NewAttribute(types.AttributeKeyMemo, msg.Memo),
),
sdk.NewEvent(
sdk.EventTypeMessage,
sdk.NewAttribute(sdk.AttributeKeyModule, types.ModuleName),
),
}.ToABCIEvents()

if tc.expError == nil {
suite.Require().NoError(err)
suite.Require().NotNil(res)
suite.Require().NotEqual(res.Sequence, uint64(0))
ibctesting.AssertEvents(&suite.Suite, expEvents, events)
} else {
suite.Require().Nil(res)
suite.Require().True(errors.Is(err, tc.expError) || strings.Contains(err.Error(), tc.expError.Error()), err.Error())
}
})
}
}

// TestUpdateParams tests UpdateParams rpc handler
func (suite *KeeperTestSuite) TestUpdateParams() {
signer := suite.chainA.GetSimApp().TransferKeeper.GetAuthority()
Expand Down
Loading
Loading