autopilot hashed sender (#1038)

## Context
During liquid stake and forward, the autopilot "receiver" of the inbound transfer becomes the "sender" of the transfer back to the host. However, downstream applications shouldn't trust this new "sender" so we need to use a generated address instead. 

To be clear, using the original sender would not introduce an attack vector on Stride. However, it could introduce an attack vector on a different zone, _if_ they were to trust the sender. The hashed sender is used to make the assumption more explicit that new zones should not trust the address.

This bug appeared in PFM (if more context is needed):
* [Issue](cosmos/ibc-apps#68)
* [PR fix](cosmos/ibc-apps#71)

## Design Considerations
There wasn't an immediately obvious way to implement this. The complexity arises in that the address used for the inbound transfer doesn't always line up with the address used in the autopilot action. Additionally, if a bank send is required after the transfer (in the event that we transferred to an intermediate recipient), some scenarios would require us to build the IBC denom hash ourselves (not impossible, but adds a lot of unnecessary code).

There didn't seem to be a clean way to refactor the callback such that we use a different receiver based on the action type (i.e. use "hashed" for LS&Forward, but "original" for Claim). As such, I think the only two options were as follows: 

1. _Send inbound transfer to original receiver_
  * ✅  **Claim**: No changes
  * ✅  **Liquid Stake**: No changes
  * ❌  **Liquid Stake and Forward**: Requires us to bank send to hashed receiver (but this is straightforward since the token is not an IBC denom)
2. _Send inbound transfer to hashed receiver_
  * ❌❌ **Claim**: Would require a bank send to the original receiver (we'd need to copy over all the IBC hashing code from ibc-go in order to handle both native and non-native denoms)
  * ❌  **Liquid Stake**: Would require a bank send to the original receiver (but this is straightforward since the token is not an IBC denom)
  * ✅ **Liquid Stake and Forward**: No changes
3. _Decide recipient based on action_
  * ✅  **Claim**: No changes
  * ✅  **Liquid Stake**: No changes
  * ✅ **Claim:** No changes
  * ❌ Requires some somewhat sloppy refactoring since the change must be upstream of the transfer

~I do think using the hashed receiver address as the inbound recipient and actor for each of the autopilot actions, does feel slightly more correct. However, I don't think it's a significant enough argument to justify the additional changes that would be required. Additionally, going with option 1 demands no changes to the existing autopilot functions that have been live on mainnet.~

After discussing offline, we decided to do option 3 and to simplify the autopilot schema so the refactor isn't as messy. This gives us the benefit of not having to worry about doing an extra bank send, but also not duplicating code in multiple places (see [here](#1038 (comment))).

Option 3 is tracked in #1046 

## Brief Changelog
* Added `GenerateHashedAddress` to generate the hashed address from the channel Id a previous sender (body of function taken from PFM)
* Renamed `PacketForwardMetadata` to `AutopilotMetadata`
* Added a bank send to the hashed address in LS and forward
* Set the hashed address as the outbound sender during LS and forward
* Restricted packets to being either autopilot or PFM (but not both)
* Cleaned up the variable names to avoid confusion with all the data structures (open to better names though!)
* Moved structs out of parser.go

app/app.go

@@ -613,6 +613,7 @@ func NewStrideApp(
 		appCodec,
 		keys[autopilottypes.StoreKey],
 		app.GetSubspace(autopilottypes.ModuleName),
+		app.BankKeeper,
 		app.StakeibcKeeper,
 		app.ClaimKeeper,
 		app.TransferKeeper,

x/autopilot/keeper/airdrop.go

@@ -12,16 +12,15 @@ import (
 	channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
 
 	"github.com/Stride-Labs/stride/v16/utils"
-	"github.com/Stride-Labs/stride/v16/x/autopilot/types"
 	claimtypes "github.com/Stride-Labs/stride/v16/x/claim/types"
 	stakeibctypes "github.com/Stride-Labs/stride/v16/x/stakeibc/types"
 )
 
+// Attempt to link a host address with a stride address to enable airdrop claims
 func (k Keeper) TryUpdateAirdropClaim(
 	ctx sdk.Context,
 	packet channeltypes.Packet,
-	data transfertypes.FungibleTokenPacketData,
-	packetMetadata types.ClaimPacketMetadata,
+	transferMetadata transfertypes.FungibleTokenPacketData,
 ) error {
 	params := k.GetParams(ctx)
 	if !params.ClaimActive {
@@ -39,11 +38,11 @@ func (k Keeper) TryUpdateAirdropClaim(
 	}
 
 	// grab relevant addresses
-	senderStrideAddress := utils.ConvertAddressToStrideAddress(data.Sender)
+	senderStrideAddress := utils.ConvertAddressToStrideAddress(transferMetadata.Sender)
 	if senderStrideAddress == "" {
-		return errorsmod.Wrapf(sdkerrors.ErrInvalidAddress, fmt.Sprintf("invalid sender address (%s)", data.Sender))
+		return errorsmod.Wrapf(sdkerrors.ErrInvalidAddress, fmt.Sprintf("invalid sender address (%s)", transferMetadata.Sender))
 	}
-	newStrideAddress := packetMetadata.StrideAddress
+	newStrideAddress := transferMetadata.Receiver
 
 	// find the airdrop for this host chain ID
 	airdrop, found := k.claimKeeper.GetAirdropByChainId(ctx, hostZone.ChainId)
@@ -56,7 +55,7 @@ func (k Keeper) TryUpdateAirdropClaim(
 
 	airdropId := airdrop.AirdropIdentifier
 	k.Logger(ctx).Info(fmt.Sprintf("updating airdrop address %s (orig %s) to %s for airdrop %s",
-		senderStrideAddress, data.Sender, newStrideAddress, airdropId))
+		senderStrideAddress, transferMetadata.Sender, newStrideAddress, airdropId))
 
 	return k.claimKeeper.UpdateAirdropAddress(ctx, senderStrideAddress, newStrideAddress, airdropId)
 }

x/autopilot/keeper/keeper.go

@@ -20,6 +20,7 @@ type (
 		Cdc            codec.BinaryCodec
 		storeKey       storetypes.StoreKey
 		paramstore     paramtypes.Subspace
+		bankKeeper     types.BankKeeper
 		stakeibcKeeper stakeibckeeper.Keeper
 		claimKeeper    claimkeeper.Keeper
 		transferKeeper types.IbcTransferKeeper
@@ -30,6 +31,7 @@ func NewKeeper(
 	Cdc codec.BinaryCodec,
 	storeKey storetypes.StoreKey,
 	ps paramtypes.Subspace,
+	bankKeeper types.BankKeeper,
 	stakeibcKeeper stakeibckeeper.Keeper,
 	claimKeeper claimkeeper.Keeper,
 	transferKeeper types.IbcTransferKeeper,
@@ -43,6 +45,7 @@ func NewKeeper(
 		Cdc:            Cdc,
 		storeKey:       storeKey,
 		paramstore:     ps,
+		bankKeeper:     bankKeeper,
 		stakeibcKeeper: stakeibcKeeper,
 		claimKeeper:    claimKeeper,
 		transferKeeper: transferKeeper,

x/autopilot/keeper/liquidstake.go

@@ -3,11 +3,11 @@ package keeper
 import (
 	"errors"
 	"fmt"
+	"time"
 
 	errorsmod "cosmossdk.io/errors"
-
+	sdkmath "cosmossdk.io/math"
 	sdk "github.com/cosmos/cosmos-sdk/types"
-	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types"
 	channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
 
@@ -16,99 +16,131 @@ import (
 	stakeibctypes "github.com/Stride-Labs/stride/v16/x/stakeibc/types"
 )
 
+const (
+	// If the forward transfer fails, the tokens are sent to the fallback address
+	// which is a less than ideal UX
+	// As a result, we decided to use a long timeout here such, even in the case
+	// of high activity, a timeout should be very unlikely to occur
+	// Empirically we found that times of high market stress took roughly
+	// 2 hours for transfers to complete
+	LiquidStakeForwardTransferTimeout = (time.Hour * 3)
+)
+
+// Attempts to do an autopilot liquid stake (and optional forward)
+// The liquid stake is only allowed if the inbound packet came along a trusted channel
 func (k Keeper) TryLiquidStaking(
 	ctx sdk.Context,
 	packet channeltypes.Packet,
-	newData transfertypes.FungibleTokenPacketData,
-	packetMetadata types.StakeibcPacketMetadata,
+	transferMetadata transfertypes.FungibleTokenPacketData,
+	autopilotMetadata types.StakeibcPacketMetadata,
 ) error {
 	params := k.GetParams(ctx)
 	if !params.StakeibcActive {
 		return errorsmod.Wrapf(types.ErrPacketForwardingInactive, "autopilot stakeibc routing is inactive")
 	}
 
-	// In this case, we can't process a liquid staking transaction, because we're dealing with STRD tokens
-	if transfertypes.ReceiverChainIsSource(packet.GetSourcePort(), packet.GetSourceChannel(), newData.Denom) {
-		return errors.New("the native token is not supported for liquid staking")
-	}
-
-	amount, ok := sdk.NewIntFromString(newData.Amount)
+	// Verify the amount is valid
+	amount, ok := sdk.NewIntFromString(transferMetadata.Amount)
 	if !ok {
 		return errors.New("not a parsable amount field")
 	}
 
-	// Note: newData.denom is base denom e.g. uatom - not ibc/xxx
-	var token = sdk.NewCoin(newData.Denom, amount)
+	// In this case, we can't process a liquid staking transaction, because we're dealing with native tokens (e.g. STRD, stATOM)
+	if transfertypes.ReceiverChainIsSource(packet.GetSourcePort(), packet.GetSourceChannel(), transferMetadata.Denom) {
+		return errors.New("the native token is not supported for liquid staking")
+	}
 
-	prefixedDenom := transfertypes.GetPrefixedDenom(packet.GetDestPort(), packet.GetDestChannel(), newData.Denom)
+	// Note: the denom in the packet is the base denom e.g. uatom - not ibc/xxx
+	// We need to use the port and channel to build the IBC denom
+	prefixedDenom := transfertypes.GetPrefixedDenom(packet.GetDestPort(), packet.GetDestChannel(), transferMetadata.Denom)
 	ibcDenom := transfertypes.ParseDenomTrace(prefixedDenom).IBCDenom()
 
-	hostZone, err := k.stakeibcKeeper.GetHostZoneFromHostDenom(ctx, token.Denom)
+	hostZone, err := k.stakeibcKeeper.GetHostZoneFromHostDenom(ctx, transferMetadata.Denom)
 	if err != nil {
-		return err
+		return fmt.Errorf("host zone not found for denom (%s)", transferMetadata.Denom)
 	}
 
+	// Verify the IBC denom of the packet matches the host zone, to confirm the packet
+	// was sent over a trusted channel
 	if hostZone.IbcDenom != ibcDenom {
 		return fmt.Errorf("ibc denom %s is not equal to host zone ibc denom %s", ibcDenom, hostZone.IbcDenom)
 	}
 
-	strideAddress, err := sdk.AccAddressFromBech32(packetMetadata.StrideAddress)
-	if err != nil {
-		return errorsmod.Wrapf(sdkerrors.ErrInvalidAddress, "invalid stride_address (%s) in autopilot memo", strideAddress)
-	}
-
-	return k.RunLiquidStake(ctx, strideAddress, token, packetMetadata)
+	return k.RunLiquidStake(ctx, amount, transferMetadata, autopilotMetadata)
 }
 
-func (k Keeper) RunLiquidStake(ctx sdk.Context, addr sdk.AccAddress, token sdk.Coin, packetMetadata types.StakeibcPacketMetadata) error {
+// Submits a LiquidStake message from the transfer receiver
+// If a forwarding recipient is specified, the stTokens are ibc transferred
+func (k Keeper) RunLiquidStake(
+	ctx sdk.Context,
+	amount sdkmath.Int,
+	transferMetadata transfertypes.FungibleTokenPacketData,
+	autopilotMetadata types.StakeibcPacketMetadata,
+) error {
 	msg := &stakeibctypes.MsgLiquidStake{
-		Creator:   addr.String(),
-		Amount:    token.Amount,
-		HostDenom: token.Denom,
+		Creator:   transferMetadata.Receiver,
+		Amount:    amount,
+		HostDenom: transferMetadata.Denom,
 	}
 
 	if err := msg.ValidateBasic(); err != nil {
 		return err
 	}
 
 	msgServer := stakeibckeeper.NewMsgServerImpl(k.stakeibcKeeper)
-	result, err := msgServer.LiquidStake(
+	msgResponse, err := msgServer.LiquidStake(
 		sdk.WrapSDKContext(ctx),
 		msg,
 	)
 	if err != nil {
 		return errorsmod.Wrapf(err, "failed to liquid stake")
 	}
 
-	if packetMetadata.IbcReceiver == "" {
+	// If the IBCReceiver is empty, there is no forwarding step
+	if autopilotMetadata.IbcReceiver == "" {
 		return nil
 	}
 
-	hostZone, err := k.stakeibcKeeper.GetHostZoneFromHostDenom(ctx, token.Denom)
+	// Otherwise, if there is forwarding info, submit the IBC transfer
+	return k.IBCTransferStToken(ctx, msgResponse.StToken, transferMetadata, autopilotMetadata)
+}
+
+// Submits an IBC transfer of the stToken to a non-stride zone (either back to the host zone or to a different zone)
+// The sender of the transfer is the hashed receiver of the original autopilot inbound transfer
+func (k Keeper) IBCTransferStToken(
+	ctx sdk.Context,
+	stToken sdk.Coin,
+	transferMetadata transfertypes.FungibleTokenPacketData,
+	autopilotMetadata types.StakeibcPacketMetadata,
+) error {
+	hostZone, err := k.stakeibcKeeper.GetHostZoneFromHostDenom(ctx, transferMetadata.Denom)
 	if err != nil {
 		return err
 	}
 
-	return k.IBCTransferStAsset(ctx, result.StToken, addr.String(), hostZone, packetMetadata)
-}
-
-func (k Keeper) IBCTransferStAsset(ctx sdk.Context, stAsset sdk.Coin, sender string, hostZone *stakeibctypes.HostZone, packetMetadata types.StakeibcPacketMetadata) error {
-	ibcTransferTimeoutNanos := k.stakeibcKeeper.GetParam(ctx, stakeibctypes.KeyIBCTransferTimeoutNanos)
-	timeoutTimestamp := uint64(ctx.BlockTime().UnixNano()) + ibcTransferTimeoutNanos
-	channelId := packetMetadata.TransferChannel
+	// If there's no channelID specified in the packet, default to the channel on the host zone
+	channelId := autopilotMetadata.TransferChannel
 	if channelId == "" {
 		channelId = hostZone.TransferChannelId
 	}
+
+	// Use a long timeout for the transfer
+	timeoutTimestamp := uint64(ctx.BlockTime().UnixNano() + LiquidStakeForwardTransferTimeout.Nanoseconds())
+
+	// Submit the transfer from the hashed address
 	transferMsg := &transfertypes.MsgTransfer{
 		SourcePort:       transfertypes.PortID,
 		SourceChannel:    channelId,
-		Token:            stAsset,
-		Sender:           sender,
-		Receiver:         packetMetadata.IbcReceiver,
+		Token:            stToken,
+		Sender:           transferMetadata.Receiver,
+		Receiver:         autopilotMetadata.IbcReceiver,
 		TimeoutTimestamp: timeoutTimestamp,
-		// TimeoutHeight and Memo are unused
+		Memo:             "autopilot-liquid-stake-and-forward",
+	}
+	_, err = k.transferKeeper.Transfer(sdk.WrapSDKContext(ctx), transferMsg)
+	if err != nil {
+		return errorsmod.Wrapf(err, "failed to submit transfer during autopilot liquid stake and forward")
 	}
 
-	_, err := k.transferKeeper.Transfer(sdk.WrapSDKContext(ctx), transferMsg)
 	return err
 }