From 0180e1475aa17d519436f025a6bc82ce4bee5beb Mon Sep 17 00:00:00 2001 From: sampocs Date: Fri, 12 Apr 2024 20:15:20 -0500 Subject: [PATCH 1/3] fixed connection ID reference --- x/stakeibc/keeper/delegation.go | 18 ++---------------- x/stakeibc/keeper/ibc.go | 9 ++++----- x/stakeibc/keeper/interchainaccounts.go | 16 ++-------------- x/stakeibc/keeper/keeper.go | 11 ----------- 4 files changed, 8 insertions(+), 46 deletions(-) diff --git a/x/stakeibc/keeper/delegation.go b/x/stakeibc/keeper/delegation.go index d3162002af..5c10cc654c 100644 --- a/x/stakeibc/keeper/delegation.go +++ b/x/stakeibc/keeper/delegation.go @@ -10,26 +10,12 @@ import ( "github.com/cosmos/gogoproto/proto" "github.com/spf13/cast" - icatypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/types" - "github.com/Stride-Labs/stride/v21/utils" recordstypes "github.com/Stride-Labs/stride/v21/x/records/types" "github.com/Stride-Labs/stride/v21/x/stakeibc/types" ) func (k Keeper) DelegateOnHost(ctx sdk.Context, hostZone types.HostZone, amt sdk.Coin, depositRecord recordstypes.DepositRecord) error { - // TODO: Remove this block and use connection-id from host zone - // the relevant ICA is the delegate account - owner := types.FormatHostZoneICAOwner(hostZone.ChainId, types.ICAAccountType_DELEGATION) - portID, err := icatypes.NewControllerPortID(owner) - if err != nil { - return errorsmod.Wrapf(sdkerrors.ErrInvalidAddress, "%s has no associated portId", owner) - } - connectionId, found := k.GetConnectionIdFromICAPortId(ctx, portID) - if !found { - return errorsmod.Wrapf(types.ErrICAAccountNotFound, "unable to find ICA connection Id for port %s", portID) - } - // Fetch the relevant ICA if hostZone.DelegationIcaAddress == "" { return errorsmod.Wrapf(types.ErrICAAccountNotFound, "no delegation account found for %s", hostZone.ChainId) @@ -79,9 +65,9 @@ func (k Keeper) DelegateOnHost(ctx sdk.Context, hostZone types.HostZone, amt sdk } // Send the transaction through SubmitTx - _, err = k.SubmitTxsStrideEpoch(ctx, connectionId, msgs, types.ICAAccountType_DELEGATION, ICACallbackID_Delegate, marshalledCallbackArgs) + _, err = k.SubmitTxsStrideEpoch(ctx, hostZone.ConnectionId, msgs, types.ICAAccountType_DELEGATION, ICACallbackID_Delegate, marshalledCallbackArgs) if err != nil { - return errorsmod.Wrapf(err, "Failed to SubmitTxs for connectionId %s on %s. Messages: %s", connectionId, hostZone.ChainId, msgs) + return errorsmod.Wrapf(err, "Failed to SubmitTxs for connectionId %s on %s. Messages: %s", hostZone.ConnectionId, hostZone.ChainId, msgs) } k.Logger(ctx).Info(utils.LogWithHostZone(hostZone.ChainId, "ICA MsgDelegates Successfully Sent")) diff --git a/x/stakeibc/keeper/ibc.go b/x/stakeibc/keeper/ibc.go index 9b34767828..a1e6eba73f 100644 --- a/x/stakeibc/keeper/ibc.go +++ b/x/stakeibc/keeper/ibc.go @@ -17,11 +17,10 @@ import ( ) func (k Keeper) OnChanOpenAck(ctx sdk.Context, portID, channelID string) error { - // Lookup the ICA address and chainId from the port and connection - controllerConnectionId, found := k.GetConnectionIdFromICAPortId(ctx, portID) - if !found { - k.Logger(ctx).Info(fmt.Sprintf("portId %s has no associated ICA account", portID)) - return nil + // Lookup connection ID, counterparty chain ID, and ICA address from the channel ID + controllerConnectionId, _, err := k.IBCKeeper.ChannelKeeper.GetChannelConnection(ctx, portID, channelID) + if err != nil { + return err } address, found := k.ICAControllerKeeper.GetInterchainAccountAddress(ctx, controllerConnectionId, portID) if !found { diff --git a/x/stakeibc/keeper/interchainaccounts.go b/x/stakeibc/keeper/interchainaccounts.go index 45409cf4c7..fa77481a13 100644 --- a/x/stakeibc/keeper/interchainaccounts.go +++ b/x/stakeibc/keeper/interchainaccounts.go @@ -24,18 +24,6 @@ const ( ) func (k Keeper) SetWithdrawalAddressOnHost(ctx sdk.Context, hostZone types.HostZone) error { - // TODO: Remove this block and use connection-id from host zone - // The relevant ICA is the delegate account - owner := types.FormatHostZoneICAOwner(hostZone.ChainId, types.ICAAccountType_DELEGATION) - portID, err := icatypes.NewControllerPortID(owner) - if err != nil { - return errorsmod.Wrapf(sdkerrors.ErrInvalidAddress, "%s has no associated portId", owner) - } - connectionId, found := k.GetConnectionIdFromICAPortId(ctx, portID) - if !found { - return errorsmod.Wrapf(types.ErrICAAccountNotFound, "unable to find ICA connection Id for port %s", portID) - } - // Fetch the relevant ICA if hostZone.DelegationIcaAddress == "" { k.Logger(ctx).Error(fmt.Sprintf("Zone %s is missing a delegation address!", hostZone.ChainId)) @@ -56,9 +44,9 @@ func (k Keeper) SetWithdrawalAddressOnHost(ctx sdk.Context, hostZone types.HostZ WithdrawAddress: hostZone.WithdrawalIcaAddress, }, } - _, err = k.SubmitTxsStrideEpoch(ctx, connectionId, msgs, types.ICAAccountType_DELEGATION, "", nil) + _, err := k.SubmitTxsStrideEpoch(ctx, hostZone.ConnectionId, msgs, types.ICAAccountType_DELEGATION, "", nil) if err != nil { - return errorsmod.Wrapf(sdkerrors.ErrInvalidRequest, "Failed to SubmitTxs for %s, %s, %s", connectionId, hostZone.ChainId, msgs) + return errorsmod.Wrapf(sdkerrors.ErrInvalidRequest, "Failed to SubmitTxs for %s, %s, %s", hostZone.ConnectionId, hostZone.ChainId, msgs) } return nil diff --git a/x/stakeibc/keeper/keeper.go b/x/stakeibc/keeper/keeper.go index f8d4435429..e491c99d32 100644 --- a/x/stakeibc/keeper/keeper.go +++ b/x/stakeibc/keeper/keeper.go @@ -108,17 +108,6 @@ func (k Keeper) GetAuthority() string { return k.authority } -// Searches all interchain accounts and finds the connection ID that corresponds with a given port ID -func (k Keeper) GetConnectionIdFromICAPortId(ctx sdk.Context, portId string) (connectionId string, found bool) { - icas := k.ICAControllerKeeper.GetAllInterchainAccounts(ctx) - for _, ica := range icas { - if ica.PortId == portId { - return ica.ConnectionId, true - } - } - return "", false -} - func (k Keeper) GetICATimeoutNanos(ctx sdk.Context, epochType string) (uint64, error) { epochTracker, found := k.GetEpochTracker(ctx, epochType) if !found { From 753bd6ce21f1d6ccf0fcb850a2f12f371d67c0b6 Mon Sep 17 00:00:00 2001 From: sampocs Date: Mon, 15 Apr 2024 10:35:23 -0500 Subject: [PATCH 2/3] fixed unit tests --- app/apptesting/test_helpers.go | 4 +++- x/stakeibc/keeper/ibc_test.go | 7 +++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/app/apptesting/test_helpers.go b/app/apptesting/test_helpers.go index db6a918ab5..2977d84cbb 100644 --- a/app/apptesting/test_helpers.go +++ b/app/apptesting/test_helpers.go @@ -549,9 +549,11 @@ func (s *AppTestHelper) MockICAChannel(connectionId, channelId, owner, address s // Create an open channel with the ICA port portId, _ := icatypes.NewControllerPortID(owner) channel := channeltypes.Channel{ - State: channeltypes.OPEN, + State: channeltypes.OPEN, + ConnectionHops: []string{connectionId}, } s.App.IBCKeeper.ChannelKeeper.SetChannel(s.Ctx, portId, channelId, channel) + s.App.IBCKeeper.ConnectionKeeper.SetConnection(s.Ctx, connectionId, connectiontypes.ConnectionEnd{}) // Then set the address and make the channel active s.App.ICAControllerKeeper.SetInterchainAccountAddress(s.Ctx, connectionId, portId, address) diff --git a/x/stakeibc/keeper/ibc_test.go b/x/stakeibc/keeper/ibc_test.go index f649ebc482..1b4e02ff3c 100644 --- a/x/stakeibc/keeper/ibc_test.go +++ b/x/stakeibc/keeper/ibc_test.go @@ -46,11 +46,10 @@ func (s *KeeperTestSuite) TestOnChanOpenAck() { tradeOwner := types.FormatTradeRouteICAOwner(tradeChainId, RewardDenom, HostDenom, types.ICAAccountType_CONVERTER_TRADE) tradePortId, _ := icatypes.NewControllerPortID(tradeOwner) - // Mock out an ICA address for each - s.App.ICAControllerKeeper.SetInterchainAccountAddress(s.Ctx, delegationConnectionId, delegationPortId, delegationAddress) - s.App.ICAControllerKeeper.SetInterchainAccountAddress(s.Ctx, tradeConnectionId, tradePortId, tradeAddress) + // Mock out the relevant clients, channels, and ICA addresses so the callback can map back to the relevant info + s.MockICAChannel(delegationConnectionId, delegationChannelId, delegationOwner, delegationAddress) + s.MockICAChannel(tradeConnectionId, tradeChannelId, tradeOwner, tradeAddress) - // Mock out a client and connection for each channel so the callback can map back from portId to chainId s.MockClientAndConnection(delegationChainId, delegationClientId, delegationConnectionId) s.MockClientAndConnection(tradeChainId, tradeClientId, tradeConnectionId) From 811aeb2b8a65934e30ae237d13ab3b82a6e2d343 Mon Sep 17 00:00:00 2001 From: sampocs Date: Mon, 15 Apr 2024 15:03:53 -0500 Subject: [PATCH 3/3] fixed unit tests --- x/stakeibc/keeper/msg_server_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/x/stakeibc/keeper/msg_server_test.go b/x/stakeibc/keeper/msg_server_test.go index 974f578a3a..2223e6d8a8 100644 --- a/x/stakeibc/keeper/msg_server_test.go +++ b/x/stakeibc/keeper/msg_server_test.go @@ -1916,15 +1916,15 @@ func (s *KeeperTestSuite) SetupTestCreateTradeRoute() (msg types.MsgCreateTradeR minSwapAmount := sdkmath.NewInt(100) maxSwapAmount := sdkmath.NewInt(1_000) - // Mock out connections for the reward an trade chain so that an ICA registration can be submitted - s.MockClientAndConnection(rewardChainId, "07-tendermint-0", rewardConnectionId) - s.MockClientAndConnection(tradeChainId, "07-tendermint-1", tradeConnectionId) - // Register an exisiting ICA account for the unwind ICA to test that // existing accounts are re-used owner := types.FormatTradeRouteICAOwner(rewardChainId, RewardDenom, HostDenom, types.ICAAccountType_CONVERTER_UNWIND) s.MockICAChannel(rewardConnectionId, "channel-0", owner, unwindAddress) + // Mock out connections for the reward an trade chain so that an ICA registration can be submitted + s.MockClientAndConnection(rewardChainId, "07-tendermint-0", rewardConnectionId) + s.MockClientAndConnection(tradeChainId, "07-tendermint-1", tradeConnectionId) + // Create a host zone with an exisiting withdrawal address hostZone := types.HostZone{ ChainId: HostChainId,