reward converter - restore trade route ICA (#1023)

dockernet/scripts/community-pool-staking/stake_proposal.sh

@@ -4,7 +4,7 @@ SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
 source ${SCRIPT_DIR}/../../config.sh
 
 deposit_ica_account=$(GET_ICA_ADDR DYDX community_pool_deposit)
-proposal_file=${STATE}/${DYDX_NODE_PREFIX}1/pool.json
+proposal_file=${STATE}/${DYDX_NODE_PREFIX}1/proposal.json
 cat << EOF > $proposal_file
 {
   "title": "Community Spend: Liquid stake",

proto/stride/stakeibc/tx.proto

@@ -1,7 +1,6 @@
 syntax = "proto3";
 package stride.stakeibc;
 
-import "stride/stakeibc/ica_account.proto";
 import "stride/stakeibc/validator.proto";
 
 option go_package = "github.com/Stride-Labs/stride/v16/x/stakeibc/types";
@@ -178,7 +177,8 @@ message MsgDeleteValidatorResponse {}
 message MsgRestoreInterchainAccount {
   string creator = 1;
   string chain_id = 2;
-  ICAAccountType account_type = 3;
+  string connection_id = 3;
+  string account_owner = 4;
 }
 message MsgRestoreInterchainAccountResponse {}
 

x/stakeibc/client/cli/tx_restore_interchain_account.go

@@ -1,8 +1,6 @@
 package cli
 
 import (
-	"errors"
-	"fmt"
 	"strings"
 
 	"github.com/cosmos/cosmos-sdk/client"
@@ -15,24 +13,26 @@ import (
 
 func CmdRestoreInterchainAccount() *cobra.Command {
 	cmd := &cobra.Command{
-		Use:   "restore-interchain-account [chain-id] [account-type]",
+		Use:   "restore-interchain-account [chain-id] [connection-id] [account-owner]",
 		Short: "Broadcast message restore-interchain-account",
 		Long: strings.TrimSpace(
-			fmt.Sprintf(`Restores a closed channel associated with an interchain account.
-Specify the interchain account type as either: %s, %s, %s, or %s`,
-				types.ICAAccountType_DELEGATION,
-				types.ICAAccountType_WITHDRAWAL,
-				types.ICAAccountType_REDEMPTION,
-				types.ICAAccountType_FEE)),
-		Args: cobra.ExactArgs(2),
+			`Restores a closed channel associated with an interchain account.
+Specify the chain ID and account owner - where the owner is the alias for the ICA account
+
+For host zone ICA accounts, the owner is of the form {chainId}.{accountType}
+ex:
+>>> strided tx restore-interchain-account cosmoshub-4 connection-0 cosmoshub-4.DELEGATION 
+
+For trade route ICA accounts, the owner is of the form:
+    {chainId}.{rewardDenom}-{hostDenom}.{accountType}
+ex:
+>>> strided tx restore-interchain-account dydx-mainnet-1 connection-1 dydx-mainnet-1.uusdc-udydx.CONVERTER_TRADE 
+		`),
+		Args: cobra.ExactArgs(3),
 		RunE: func(cmd *cobra.Command, args []string) (err error) {
-			argChainId := args[0]
-			argAccountType := args[1]
-
-			accountType, found := types.ICAAccountType_value[argAccountType]
-			if !found {
-				return errors.New("Invalid account type.")
-			}
+			chainId := args[0]
+			connectionId := args[1]
+			accountOwner := args[2]
 
 			clientCtx, err := client.GetClientTxContext(cmd)
 			if err != nil {
@@ -41,8 +41,9 @@ Specify the interchain account type as either: %s, %s, %s, or %s`,
 
 			msg := types.NewMsgRestoreInterchainAccount(
 				clientCtx.GetFromAddress().String(),
-				argChainId,
-				types.ICAAccountType(accountType),
+				chainId,
+				connectionId,
+				accountOwner,
 			)
 			if err := msg.ValidateBasic(); err != nil {
 				return err

x/stakeibc/keeper/host_zone.go

@@ -74,9 +74,9 @@ func (k Keeper) GetHostZoneFromTransferChannelID(ctx sdk.Context, channelID stri
 }
 
 // RemoveHostZone removes a hostZone from the store
-func (k Keeper) RemoveHostZone(ctx sdk.Context, chain_id string) {
+func (k Keeper) RemoveHostZone(ctx sdk.Context, chainId string) {
 	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefix(types.HostZoneKey))
-	store.Delete([]byte(chain_id))
+	store.Delete([]byte(chainId))
 }
 
 // GetAllHostZone returns all hostZone

x/stakeibc/keeper/icqcallbacks_withdrawal_reward_balance_test.go

@@ -18,7 +18,7 @@ import (
 // Therefore we need to setup traderoute fields used in the entire transfer (with pfm)
 func (s *KeeperTestSuite) SetupWithdrawalRewardBalanceCallbackTestCase() BalanceQueryCallbackTestCase {
 	// Create the connection between Stride and HostChain with the withdrawal account initialized
-	withdrawalAccountOwner := types.FormatTradeRouteICAOwner(HostChainId, RewardDenom, HostDenom, types.ICAAccountType_WITHDRAWAL)
+	withdrawalAccountOwner := types.FormatHostZoneICAOwner(HostChainId, types.ICAAccountType_WITHDRAWAL)
 	withdrawalChannelId, withdrawalPortId := s.CreateICAChannel(withdrawalAccountOwner)
 
 	route := types.TradeRoute{

x/stakeibc/keeper/msg_server_restore_interchain_account.go

@@ -6,8 +6,8 @@ import (
 
 	errorsmod "cosmossdk.io/errors"
 	sdk "github.com/cosmos/cosmos-sdk/types"
-	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	icatypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/types"
+	connectiontypes "github.com/cosmos/ibc-go/v7/modules/core/03-connection/types"
 
 	recordtypes "github.com/Stride-Labs/stride/v16/x/records/types"
 	"github.com/Stride-Labs/stride/v16/x/stakeibc/types"
@@ -16,52 +16,43 @@ import (
 func (k msgServer) RestoreInterchainAccount(goCtx context.Context, msg *types.MsgRestoreInterchainAccount) (*types.MsgRestoreInterchainAccountResponse, error) {
 	ctx := sdk.UnwrapSDKContext(goCtx)
 
-	// Confirm host zone exists
-	hostZone, found := k.GetHostZone(ctx, msg.ChainId)
-	if !found {
-		k.Logger(ctx).Error(fmt.Sprintf("Host Zone not found: %s", msg.ChainId))
-		return nil, types.ErrInvalidHostZone
-	}
-
 	// Get ConnectionEnd (for counterparty connection)
-	connectionEnd, found := k.IBCKeeper.ConnectionKeeper.GetConnection(ctx, hostZone.ConnectionId)
+	connectionEnd, found := k.IBCKeeper.ConnectionKeeper.GetConnection(ctx, msg.ConnectionId)
 	if !found {
-		errMsg := fmt.Sprintf("invalid connection id from host %s, %s not found", msg.ChainId, hostZone.ConnectionId)
-		k.Logger(ctx).Error(errMsg)
-		return nil, errorsmod.Wrapf(sdkerrors.ErrInvalidRequest, errMsg)
+		return nil, errorsmod.Wrapf(connectiontypes.ErrConnectionNotFound, "connection %s not found", msg.ConnectionId)
 	}
 	counterpartyConnection := connectionEnd.Counterparty
 
 	// only allow restoring an account if it already exists
-	owner := types.FormatHostZoneICAOwner(msg.ChainId, msg.AccountType)
-	portID, err := icatypes.NewControllerPortID(owner)
+	portID, err := icatypes.NewControllerPortID(msg.AccountOwner)
 	if err != nil {
-		errMsg := fmt.Sprintf("could not create portID for ICA controller account address: %s", owner)
-		k.Logger(ctx).Error(errMsg)
 		return nil, err
 	}
-	_, exists := k.ICAControllerKeeper.GetInterchainAccountAddress(ctx, hostZone.ConnectionId, portID)
+	_, exists := k.ICAControllerKeeper.GetInterchainAccountAddress(ctx, msg.ConnectionId, portID)
 	if !exists {
-		errMsg := fmt.Sprintf("ICA controller account address not found: %s", owner)
-		k.Logger(ctx).Error(errMsg)
-		return nil, errorsmod.Wrapf(types.ErrInvalidInterchainAccountAddress, errMsg)
+		return nil, errorsmod.Wrapf(types.ErrInvalidInterchainAccountAddress,
+			"ICA controller account address not found: %s", msg.AccountOwner)
 	}
 
 	appVersion := string(icatypes.ModuleCdc.MustMarshalJSON(&icatypes.Metadata{
 		Version:                icatypes.Version,
-		ControllerConnectionId: hostZone.ConnectionId,
+		ControllerConnectionId: msg.ConnectionId,
 		HostConnectionId:       counterpartyConnection.ConnectionId,
 		Encoding:               icatypes.EncodingProtobuf,
 		TxType:                 icatypes.TxTypeSDKMultiMsg,
 	}))
 
-	if err := k.ICAControllerKeeper.RegisterInterchainAccount(ctx, hostZone.ConnectionId, owner, appVersion); err != nil {
-		k.Logger(ctx).Error(fmt.Sprintf("unable to register %s account : %s", msg.AccountType.String(), err))
-		return nil, err
+	if err := k.ICAControllerKeeper.RegisterInterchainAccount(ctx, msg.ConnectionId, msg.AccountOwner, appVersion); err != nil {
+		return nil, errorsmod.Wrapf(err, "unable to register account for owner %s", msg.AccountOwner)
 	}
 
 	// If we're restoring a delegation account, we also have to reset record state
-	if msg.AccountType == types.ICAAccountType_DELEGATION {
+	if msg.AccountOwner == types.FormatHostZoneICAOwner(msg.ChainId, types.ICAAccountType_DELEGATION) {
+		hostZone, found := k.GetHostZone(ctx, msg.ChainId)
+		if !found {
+			return nil, types.ErrHostZoneNotFound.Wrapf("delegation ICA supplied, but no associated host zone")
+		}
+
 		// revert DELEGATION_IN_PROGRESS records for the closed ICA channel (so that they can be staked)
 		depositRecords := k.RecordsKeeper.GetAllDepositRecord(ctx)
 		for _, depositRecord := range depositRecords {

x/stakeibc/keeper/msg_server_restore_interchain_account_test.go

@@ -165,9 +165,10 @@ func (s *KeeperTestSuite) SetupRestoreInterchainAccount(createDelegationICAChann
 	}
 
 	defaultMsg := types.MsgRestoreInterchainAccount{
-		Creator:     "creatoraddress",
-		ChainId:     HostChainId,
-		AccountType: types.ICAAccountType_DELEGATION,
+		Creator:      "creatoraddress",
+		ChainId:      HostChainId,
+		ConnectionId: ibctesting.FirstConnectionID,
+		AccountOwner: types.FormatHostZoneICAOwner(HostChainId, types.ICAAccountType_DELEGATION),
 	}
 
 	return RestoreInterchainAccountTestCase{
@@ -279,31 +280,34 @@ func (s *KeeperTestSuite) TestRestoreInterchainAccount_InvalidConnectionId() {
 	tc := s.SetupRestoreInterchainAccount(false)
 
 	// Update the connectionId on the host zone so that it doesn't exist
-	hostZone, found := s.App.StakeibcKeeper.GetHostZone(s.Ctx, tc.validMsg.ChainId)
-	s.Require().True(found)
-	hostZone.ConnectionId = "fake_connection"
-	s.App.StakeibcKeeper.SetHostZone(s.Ctx, hostZone)
+	invalidMsg := tc.validMsg
+	invalidMsg.ConnectionId = "fake_connection"
 
-	_, err := s.GetMsgServer().RestoreInterchainAccount(sdk.WrapSDKContext(s.Ctx), &tc.validMsg)
-	s.Require().EqualError(err, "invalid connection id from host GAIA, fake_connection not found: invalid request")
+	_, err := s.GetMsgServer().RestoreInterchainAccount(sdk.WrapSDKContext(s.Ctx), &invalidMsg)
+	s.Require().ErrorContains(err, "connection fake_connection not found")
 }
 
 func (s *KeeperTestSuite) TestRestoreInterchainAccount_CannotRestoreNonExistentAcct() {
 	tc := s.SetupRestoreInterchainAccount(false)
+
+	// Attempt to restore an account that does not exist
 	msg := tc.validMsg
-	msg.AccountType = types.ICAAccountType_WITHDRAWAL
+	msg.AccountOwner = types.FormatHostZoneICAOwner(HostChainId, types.ICAAccountType_WITHDRAWAL)
 
 	_, err := s.GetMsgServer().RestoreInterchainAccount(sdk.WrapSDKContext(s.Ctx), &msg)
 	s.Require().ErrorContains(err, "ICA controller account address not found: GAIA.WITHDRAWAL")
 }
 
-func (s *KeeperTestSuite) TestRestoreInterchainAccount_FailsForIncorrectHostZone() {
-	tc := s.SetupRestoreInterchainAccount(false)
-	invalidMsg := tc.validMsg
-	invalidMsg.ChainId = "incorrectchainid"
+func (s *KeeperTestSuite) TestRestoreInterchainAccount_HostZoneNotFound() {
+	tc := s.SetupRestoreInterchainAccount(true)
+	s.closeICAChannel(tc.delegationPortID, tc.delegationChannelID)
 
-	_, err := s.GetMsgServer().RestoreInterchainAccount(sdk.WrapSDKContext(s.Ctx), &invalidMsg)
-	s.Require().ErrorContains(err, "host zone not registered")
+	// Delete the host zone so the lookup fails
+	// (this check only runs for the delegation channel)
+	s.App.StakeibcKeeper.RemoveHostZone(s.Ctx, HostChainId)
+
+	_, err := s.GetMsgServer().RestoreInterchainAccount(sdk.WrapSDKContext(s.Ctx), &tc.validMsg)
+	s.Require().ErrorContains(err, "delegation ICA supplied, but no associated host zone")
 }
 
 func (s *KeeperTestSuite) TestRestoreInterchainAccount_RevertDepositRecords_Failure() {
@@ -333,7 +337,7 @@ func (s *KeeperTestSuite) TestRestoreInterchainAccount_NoRecordChange_Success()
 
 	// Restore the channel
 	msg := tc.validMsg
-	msg.AccountType = types.ICAAccountType_WITHDRAWAL
+	msg.AccountOwner = types.FormatHostZoneICAOwner(HostChainId, types.ICAAccountType_WITHDRAWAL)
 	s.restoreChannelAndVerifySuccess(msg, portID, channelID)
 
 	// Verify the record status' were NOT reverted

x/stakeibc/keeper/reward_converter.go

@@ -149,7 +149,7 @@ func (k Keeper) TransferRewardTokensHostToTrade(ctx sdk.Context, amount sdkmath.
 
 	// Send the ICA tx to kick off transfer from hostZone through rewardZone to the tradeZone (no callbacks)
 	hostAccount := route.HostAccount
-	withdrawalOwner := types.FormatTradeRouteICAOwnerFromRouteId(hostAccount.ChainId, route.GetRouteId(), hostAccount.Type)
+	withdrawalOwner := types.FormatHostZoneICAOwner(hostAccount.ChainId, hostAccount.Type)
 	err = k.SubmitICATxWithoutCallback(ctx, hostAccount.ConnectionId, withdrawalOwner, msgs, msg.TimeoutTimestamp)
 	if err != nil {
 		return errorsmod.Wrapf(err, "Failed to submit ICA tx, Messages: %+v", msgs)

x/stakeibc/keeper/reward_converter_test.go

@@ -39,7 +39,7 @@ type TransferRewardHostToTradeTestCase struct {
 
 func (s *KeeperTestSuite) SetupTransferRewardTokensHostToTradeTestCase() TransferRewardHostToTradeTestCase {
 	// Create an ICA channel for the transfer submission
-	owner := types.FormatTradeRouteICAOwner(HostChainId, RewardDenom, HostDenom, types.ICAAccountType_WITHDRAWAL)
+	owner := types.FormatHostZoneICAOwner(HostChainId, types.ICAAccountType_WITHDRAWAL)
 	channelId, portId := s.CreateICAChannel(owner)
 
 	// Define components of transfer message

x/stakeibc/types/message_restore_interchain_account.go

@@ -1,20 +1,23 @@
 package types
 
 import (
+	"strings"
+
 	errorsmod "cosmossdk.io/errors"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 )
 
-const TypeMsgRestoreInterchainAccount = "register_interchain_account"
+const TypeMsgRestoreInterchainAccount = "restore_interchain_account"
 
 var _ sdk.Msg = &MsgRestoreInterchainAccount{}
 
-func NewMsgRestoreInterchainAccount(creator string, chainId string, accountType ICAAccountType) *MsgRestoreInterchainAccount {
+func NewMsgRestoreInterchainAccount(creator, chainId, connectionId, owner string) *MsgRestoreInterchainAccount {
 	return &MsgRestoreInterchainAccount{
-		Creator:     creator,
-		ChainId:     chainId,
-		AccountType: accountType,
+		Creator:      creator,
+		ChainId:      chainId,
+		ConnectionId: connectionId,
+		AccountOwner: owner,
 	}
 }
 
@@ -44,5 +47,17 @@ func (msg *MsgRestoreInterchainAccount) ValidateBasic() error {
 	if err != nil {
 		return errorsmod.Wrapf(sdkerrors.ErrInvalidAddress, "invalid creator address (%s)", err)
 	}
+	if msg.ChainId == "" {
+		return errorsmod.Wrapf(sdkerrors.ErrInvalidRequest, "chain ID must be specified")
+	}
+	if !strings.HasPrefix(msg.ConnectionId, "connection-") {
+		return errorsmod.Wrapf(sdkerrors.ErrInvalidRequest, "connection ID must be specified")
+	}
+	if msg.AccountOwner == "" {
+		return errorsmod.Wrapf(sdkerrors.ErrInvalidRequest, "ICA account owner must be specified")
+	}
+	if !strings.HasPrefix(msg.AccountOwner, msg.ChainId) {
+		return errorsmod.Wrapf(sdkerrors.ErrInvalidRequest, "ICA account owner does not match chain ID")
+	}
 	return nil
 }