From d3e0501c2aaa9a6946cf1e8d84a4da4288783b07 Mon Sep 17 00:00:00 2001 From: Aidan Salzmann Date: Tue, 28 Nov 2023 15:44:42 -0500 Subject: [PATCH 01/30] pull in LS + forward changes from https://github.com/Stride-Labs/stride/pull/771/files --- app/app.go | 4 ++- dockernet/config.sh | 7 ++++ dockernet/tests/integration_tests.bats | 47 ++++++++++++++++++++++++++ x/autopilot/keeper/keeper.go | 4 +++ x/autopilot/keeper/liquidstake.go | 47 ++++++++++++++++++++++---- x/autopilot/module_ibc.go | 14 +++++--- x/autopilot/types/parser.go | 18 ++++++++-- 7 files changed, 126 insertions(+), 15 deletions(-) diff --git a/app/app.go b/app/app.go index d42ca579e..de59481cb 100644 --- a/app/app.go +++ b/app/app.go @@ -613,7 +613,9 @@ func NewStrideApp( keys[autopilottypes.StoreKey], app.GetSubspace(autopilottypes.ModuleName), app.StakeibcKeeper, - app.ClaimKeeper) + app.ClaimKeeper, + app.TransferKeeper, + ) autopilotModule := autopilot.NewAppModule(appCodec, app.AutopilotKeeper) app.VestingKeeper = evmosvestingkeeper.NewKeeper( diff --git a/dockernet/config.sh b/dockernet/config.sh index 286eb15e2..d58f914c2 100755 --- a/dockernet/config.sh +++ b/dockernet/config.sh @@ -57,6 +57,13 @@ STWALK_DENOM="stuwalk" STEVMOS_DENOM="staevmos" STDYDX_DENOM="studydx" +IBC_GAIA_CHANNEL_0_STATOM_DENOM='ibc/054A44EC8D9B68B9A6F0D5708375E00A5569A28F21E0064FF12CADC3FEF1D04F' +IBC_GAIA_CHANNEL_1_STATOM_DENOM='ibc/8B21DA0E34A49AE151FEEBCCF3AFE1188E24BA8E19439FB93434DF6008E7E228' +IBC_GAIA_CHANNEL_2_STATOM_DENOM='ibc/60CB7A5465C318C8F68F603D78721A2ECC1DA2D0E905C6AD9ACD1CAC3F0DB22D' +IBC_GAIA_CHANNEL_3_STATOM_DENOM='ibc/0C0FD07C29EB075C18EA77B73CF9FCE68A268E0738C9F5B11D13E418AD889437' + +IBC_GAIA_STATOM_DENOM=$IBC_GAIA_CHANNEL_0_STATOM_DENOM + IBC_STRD_DENOM='ibc/FF6C2E86490C1C4FBBD24F55032831D2415B9D7882F85C3CC9C2401D79362BEA' IBC_GAIA_CHANNEL_0_DENOM='ibc/27394FB092D2ECCD56123C74F36E4C1F926001CEADA9CA97EA622B25F41E5EB2' diff --git a/dockernet/tests/integration_tests.bats b/dockernet/tests/integration_tests.bats index 2299d7e9c..604df7983 100644 --- a/dockernet/tests/integration_tests.bats +++ b/dockernet/tests/integration_tests.bats @@ -152,6 +152,53 @@ setup_file() { assert_equal "$diff" $STAKE_AMOUNT } +@test "[INTEGRATION-BASIC-$CHAIN_NAME] transfer st$HOST_DENOM to host chain" { + # get initial balances + sttoken_balance_start=$($STRIDE_MAIN_CMD q bank balances $(STRIDE_ADDRESS) --denom st$HOST_DENOM | GETBAL) + stibctoken_balance_start=$($HOST_MAIN_CMD q bank balances $HOST_VAL_ADDRESS --denom $IBC_GAIA_STATOM_DENOM | GETBAL) + + # do IBC transfer + $STRIDE_MAIN_CMD tx ibc-transfer transfer transfer $STRIDE_TRANFER_CHANNEL $HOST_VAL_ADDRESS ${PACKET_FORWARD_STAKE_AMOUNT}st${HOST_DENOM} --from $STRIDE_VAL -y & + WAIT_FOR_BLOCK $STRIDE_LOGS 8 + + # make sure stATOM balance decreased + sttoken_balance_end=$($STRIDE_MAIN_CMD q bank balances $(STRIDE_ADDRESS) --denom st$HOST_DENOM | GETBAL) + stibctoken_balance_end=$($HOST_MAIN_CMD q bank balances $HOST_VAL_ADDRESS --denom $IBC_GAIA_STATOM_DENOM | GETBAL) + sttoken_balance_diff=$(($sttoken_balance_start-$sttoken_balance_end)) + stibctoken_balance_diff=$(($stibctoken_balance_end-$stibctoken_balance_start)) + assert_equal "$sttoken_balance_diff" "$PACKET_FORWARD_STAKE_AMOUNT" + assert_equal "$stibctoken_balance_diff" "$PACKET_FORWARD_STAKE_AMOUNT" +} + +@test "[INTEGRATION-BASIC-$CHAIN_NAME] packet forwarding automatically liquid stake and ibc transfer stAsset to original network" { + skip "DefaultActive set to false, skip test" + memo='{ "autopilot": { "receiver": "'"$(STRIDE_ADDRESS)"'", "stakeibc": { "action": "LiquidStake", "ibc_receiver": "'$HOST_VAL_ADDRESS'" } } }' + + # get initial balances + stibctoken_balance_start=$($HOST_MAIN_CMD q bank balances $HOST_VAL_ADDRESS --denom $IBC_GAIA_STATOM_DENOM | GETBAL) + + # Send the IBC transfer with the JSON memo + transfer_msg_prefix="$HOST_MAIN_CMD tx ibc-transfer transfer transfer $HOST_TRANSFER_CHANNEL" + if [[ "$CHAIN_NAME" == "GAIA" ]]; then + # For GAIA (ibc-v3), pass the memo into the receiver field + $transfer_msg_prefix "$memo" ${PACKET_FORWARD_STAKE_AMOUNT}${HOST_DENOM} --from $HOST_VAL -y + elif [[ "$CHAIN_NAME" == "HOST" ]]; then + # For HOST (ibc-v5), pass an address for a receiver and the memo in the --memo field + $transfer_msg_prefix $(STRIDE_ADDRESS) ${PACKET_FORWARD_STAKE_AMOUNT}${HOST_DENOM} --memo "$memo" --from $HOST_VAL -y + else + # For all other hosts, skip this test + skip "Packet forward liquid stake test is only run on GAIA and HOST" + fi + + # Wait for the transfer to complete + WAIT_FOR_BALANCE_CHANGE $CHAIN_NAME $HOST_VAL_ADDRESS $IBC_GAIA_STATOM_DENOM + + # make sure stATOM balance increased + stibctoken_balance_end=$($HOST_MAIN_CMD q bank balances $HOST_VAL_ADDRESS --denom $IBC_GAIA_STATOM_DENOM | GETBAL) + stibctoken_balance_diff=$(($stibctoken_balance_end-$stibctoken_balance_start)) + assert_equal "$stibctoken_balance_diff" "$PACKET_FORWARD_STAKE_AMOUNT" +} + # check that tokens on the host are staked @test "[INTEGRATION-BASIC-$CHAIN_NAME] tokens on $CHAIN_NAME were staked" { # wait for another epoch to pass so that tokens are staked diff --git a/x/autopilot/keeper/keeper.go b/x/autopilot/keeper/keeper.go index e9e8ceff4..b9baa54de 100644 --- a/x/autopilot/keeper/keeper.go +++ b/x/autopilot/keeper/keeper.go @@ -9,6 +9,7 @@ import ( "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" + ibctransferkeeper "github.com/cosmos/ibc-go/v7/modules/apps/transfer/keeper" "github.com/Stride-Labs/stride/v16/x/autopilot/types" claimkeeper "github.com/Stride-Labs/stride/v16/x/claim/keeper" @@ -22,6 +23,7 @@ type ( paramstore paramtypes.Subspace stakeibcKeeper stakeibckeeper.Keeper claimKeeper claimkeeper.Keeper + transferKeeper ibctransferkeeper.Keeper } ) @@ -31,6 +33,7 @@ func NewKeeper( ps paramtypes.Subspace, stakeibcKeeper stakeibckeeper.Keeper, claimKeeper claimkeeper.Keeper, + transferKeeper ibctransferkeeper.Keeper, ) *Keeper { // set KeyTable if it has not already been set if !ps.HasKeyTable() { @@ -43,6 +46,7 @@ func NewKeeper( paramstore: ps, stakeibcKeeper: stakeibcKeeper, claimKeeper: claimKeeper, + transferKeeper: transferKeeper, } } diff --git a/x/autopilot/keeper/liquidstake.go b/x/autopilot/keeper/liquidstake.go index 29a2aa815..2f31be671 100644 --- a/x/autopilot/keeper/liquidstake.go +++ b/x/autopilot/keeper/liquidstake.go @@ -40,7 +40,7 @@ func (k Keeper) TryLiquidStaking( // Note: newData.denom is base denom e.g. uatom - not ibc/xxx var token = sdk.NewCoin(newData.Denom, amount) - prefixedDenom := transfertypes.GetDenomPrefix(packet.GetDestPort(), packet.GetDestChannel()) + newData.Denom + prefixedDenom := transfertypes.GetPrefixedDenom(packet.GetDestPort(), packet.GetDestChannel(), newData.Denom) ibcDenom := transfertypes.ParseDenomTrace(prefixedDenom).IBCDenom() hostZone, err := k.stakeibcKeeper.GetHostZoneFromHostDenom(ctx, token.Denom) @@ -57,10 +57,10 @@ func (k Keeper) TryLiquidStaking( return errorsmod.Wrapf(sdkerrors.ErrInvalidAddress, "invalid stride_address (%s) in autopilot memo", strideAddress) } - return k.RunLiquidStake(ctx, strideAddress, token) + return k.RunLiquidStake(ctx, strideAddress, token, packetMetadata) } -func (k Keeper) RunLiquidStake(ctx sdk.Context, addr sdk.AccAddress, token sdk.Coin) error { +func (k Keeper) RunLiquidStake(ctx sdk.Context, addr sdk.AccAddress, token sdk.Coin, packetMetadata types.StakeibcPacketMetadata) error { msg := &stakeibctypes.MsgLiquidStake{ Creator: addr.String(), Amount: token.Amount, @@ -72,12 +72,47 @@ func (k Keeper) RunLiquidStake(ctx sdk.Context, addr sdk.AccAddress, token sdk.C } msgServer := stakeibckeeper.NewMsgServerImpl(k.stakeibcKeeper) - _, err := msgServer.LiquidStake( + result, err := msgServer.LiquidStake( sdk.WrapSDKContext(ctx), msg, ) if err != nil { - return errorsmod.Wrapf(sdkerrors.ErrInsufficientFunds, err.Error()) + return errorsmod.Wrapf(err, err.Error()) } - return nil + + if packetMetadata.IbcReceiver == "" { + return nil + } + + hostZone, err := k.stakeibcKeeper.GetHostZoneFromHostDenom(ctx, token.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 channelId == "" { + channelId = hostZone.TransferChannelId + } + transferMsg := &transfertypes.MsgTransfer{ + SourcePort: transfertypes.PortID, + SourceChannel: channelId, + Token: stAsset, + // TODO: does this reintroduce the bug in PFM where senders can be spoofed? + // If so, should we instead call PFM directly to forward the packet? + // Or should we obfuscate the sender, making it a random address? + Sender: sender, + Receiver: packetMetadata.IbcReceiver, + TimeoutTimestamp: timeoutTimestamp, + // TimeoutHeight: clienttypes.Height{}, + // Memo: "stTokenIBCTransfer", + } + + _, err := k.transferKeeper.Transfer(sdk.WrapSDKContext(ctx), transferMsg) + return err } diff --git a/x/autopilot/module_ibc.go b/x/autopilot/module_ibc.go index 53ffefc8e..b3fce0677 100644 --- a/x/autopilot/module_ibc.go +++ b/x/autopilot/module_ibc.go @@ -18,7 +18,7 @@ import ( ibcexported "github.com/cosmos/ibc-go/v7/modules/core/exported" ) -const MaxMemoCharLength = 256 +const MaxMemoCharLength = 512 // IBC MODULE IMPLEMENTATION // IBCModule implements the ICS26 interface for transfer given the transfer keeper. @@ -196,10 +196,14 @@ func (im IBCModule) OnRecvPacket( } im.keeper.Logger(ctx).Info(fmt.Sprintf("Forwaring packet from %s to stakeibc", newData.Sender)) - // Try to liquid stake - return an ack error if it fails, otherwise return the ack generated from the earlier packet propogation - if err := im.keeper.TryLiquidStaking(ctx, packet, newData, routingInfo); err != nil { - im.keeper.Logger(ctx).Error(fmt.Sprintf("Error liquid staking packet from autopilot for %s: %s", newData.Sender, err.Error())) - return channeltypes.NewErrorAcknowledgement(err) + switch routingInfo.Action { + case types.LiquidStake: + // Try to liquid stake - return an ack error if it fails, otherwise return the ack generated from the earlier packet propogation + if err := im.keeper.TryLiquidStaking(ctx, packet, newData, routingInfo); err != nil { + im.keeper.Logger(ctx).Error(fmt.Sprintf("Error liquid staking packet from autopilot for %s: %s", newData.Sender, err.Error())) + return channeltypes.NewErrorAcknowledgement(err) + } + // case types.RedeemStake: TODO: add redeem stake logic } return ack diff --git a/x/autopilot/types/parser.go b/x/autopilot/types/parser.go index 7ff3710c7..97e09b1c2 100644 --- a/x/autopilot/types/parser.go +++ b/x/autopilot/types/parser.go @@ -25,13 +25,21 @@ type ModuleRoutingInfo interface { Validate() error } +const LiquidStake = "LiquidStake" +const RedeemStake = "RedeemStake" + // Packet metadata info specific to Stakeibc (e.g. 1-click liquid staking) type StakeibcPacketMetadata struct { - Action string `json:"action"` - StrideAddress string + // TODO: use a constant here + Action string `json:"action"` + // TODO: remove StrideAddress + StrideAddress string + IbcReceiver string `json:"ibc_receiver,omitempty"` + TransferChannel string `json:"transfer_channel,omitempty"` } // Packet metadata info specific to Claim (e.g. airdrops for non-118 coins) +// TODO: remove this struct type ClaimPacketMetadata struct { StrideAddress string } @@ -43,7 +51,10 @@ func (m StakeibcPacketMetadata) Validate() error { if err != nil { return err } - if m.Action != "LiquidStake" { + switch m.Action { + case LiquidStake: + case RedeemStake: + default: return errorsmod.Wrapf(ErrUnsupportedStakeibcAction, "action %s is not supported", m.Action) } @@ -51,6 +62,7 @@ func (m StakeibcPacketMetadata) Validate() error { } // Validate claim packet metadata includes the stride address +// TODO: remove this function func (m ClaimPacketMetadata) Validate() error { _, err := sdk.AccAddressFromBech32(m.StrideAddress) if err != nil { From 1c6ed0bb30d6122e5e64cf939d76bd879b5aa644 Mon Sep 17 00:00:00 2001 From: Aidan Salzmann Date: Tue, 28 Nov 2023 16:33:35 -0500 Subject: [PATCH 02/30] integration test working --- dockernet/tests/integration_tests.bats | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/dockernet/tests/integration_tests.bats b/dockernet/tests/integration_tests.bats index 604df7983..34b6677fd 100644 --- a/dockernet/tests/integration_tests.bats +++ b/dockernet/tests/integration_tests.bats @@ -152,26 +152,7 @@ setup_file() { assert_equal "$diff" $STAKE_AMOUNT } -@test "[INTEGRATION-BASIC-$CHAIN_NAME] transfer st$HOST_DENOM to host chain" { - # get initial balances - sttoken_balance_start=$($STRIDE_MAIN_CMD q bank balances $(STRIDE_ADDRESS) --denom st$HOST_DENOM | GETBAL) - stibctoken_balance_start=$($HOST_MAIN_CMD q bank balances $HOST_VAL_ADDRESS --denom $IBC_GAIA_STATOM_DENOM | GETBAL) - - # do IBC transfer - $STRIDE_MAIN_CMD tx ibc-transfer transfer transfer $STRIDE_TRANFER_CHANNEL $HOST_VAL_ADDRESS ${PACKET_FORWARD_STAKE_AMOUNT}st${HOST_DENOM} --from $STRIDE_VAL -y & - WAIT_FOR_BLOCK $STRIDE_LOGS 8 - - # make sure stATOM balance decreased - sttoken_balance_end=$($STRIDE_MAIN_CMD q bank balances $(STRIDE_ADDRESS) --denom st$HOST_DENOM | GETBAL) - stibctoken_balance_end=$($HOST_MAIN_CMD q bank balances $HOST_VAL_ADDRESS --denom $IBC_GAIA_STATOM_DENOM | GETBAL) - sttoken_balance_diff=$(($sttoken_balance_start-$sttoken_balance_end)) - stibctoken_balance_diff=$(($stibctoken_balance_end-$stibctoken_balance_start)) - assert_equal "$sttoken_balance_diff" "$PACKET_FORWARD_STAKE_AMOUNT" - assert_equal "$stibctoken_balance_diff" "$PACKET_FORWARD_STAKE_AMOUNT" -} - @test "[INTEGRATION-BASIC-$CHAIN_NAME] packet forwarding automatically liquid stake and ibc transfer stAsset to original network" { - skip "DefaultActive set to false, skip test" memo='{ "autopilot": { "receiver": "'"$(STRIDE_ADDRESS)"'", "stakeibc": { "action": "LiquidStake", "ibc_receiver": "'$HOST_VAL_ADDRESS'" } } }' # get initial balances From 596250b7e87c21a50e5c9362ea7e839248043fed Mon Sep 17 00:00:00 2001 From: Aidan Salzmann Date: Tue, 28 Nov 2023 16:52:56 -0500 Subject: [PATCH 03/30] fix unittest --- x/autopilot/keeper/airdrop_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/autopilot/keeper/airdrop_test.go b/x/autopilot/keeper/airdrop_test.go index f58b918f4..37bceafed 100644 --- a/x/autopilot/keeper/airdrop_test.go +++ b/x/autopilot/keeper/airdrop_test.go @@ -245,7 +245,7 @@ func (s *KeeperTestSuite) TestAirdropOnRecvPacket() { destinationPortID: transfertypes.PortID, packetData: transfertypes.FungibleTokenPacketData{ Receiver: strideAddress, - Memo: strings.Repeat("X", 300), + Memo: strings.Repeat("X", 513), }, transferShouldSucceed: false, airdropShouldUpdate: false, From 220c8f1c445011271a4c67951fdad78e324b9a70 Mon Sep 17 00:00:00 2001 From: sampocs Date: Thu, 21 Dec 2023 15:09:45 -0600 Subject: [PATCH 04/30] restrict to either autopilot or pfm --- x/autopilot/module_ibc.go | 3 ++- x/autopilot/types/parser.go | 12 ++++++++++-- x/autopilot/types/parser_test.go | 10 ++++++++++ 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/x/autopilot/module_ibc.go b/x/autopilot/module_ibc.go index b3fce0677..e4365c67c 100644 --- a/x/autopilot/module_ibc.go +++ b/x/autopilot/module_ibc.go @@ -161,8 +161,9 @@ func (im IBCModule) OnRecvPacket( return channeltypes.NewErrorAcknowledgement(err) } - // If the parsed metadata is nil, that means there is no forwarding logic + // If the parsed metadata is nil, that means there is no autopilot forwarding logic // Pass the packet down to the next middleware + // PFM packets will also go down this path if packetForwardMetadata == nil { return im.app.OnRecvPacket(ctx, packet, relayer) } diff --git a/x/autopilot/types/parser.go b/x/autopilot/types/parser.go index 97e09b1c2..7e8b3d2b0 100644 --- a/x/autopilot/types/parser.go +++ b/x/autopilot/types/parser.go @@ -14,6 +14,7 @@ type RawPacketMetadata struct { Stakeibc *StakeibcPacketMetadata `json:"stakeibc,omitempty"` Claim *ClaimPacketMetadata `json:"claim,omitempty"` } `json:"autopilot"` + Forward *interface{} `json:"forward"` } type PacketForwardMetadata struct { @@ -76,7 +77,7 @@ func (m ClaimPacketMetadata) Validate() error { // In the ICS-20 packet, the metadata can optionally indicate a module to route to (e.g. stakeibc) // The PacketForwardMetadata returned from this function contains attributes for each autopilot supported module // It can only be forward to one module per packet -// Returns nil if there was no metadata found +// Returns nil if there was no autopilot metadata found func ParsePacketMetadata(metadata string) (*PacketForwardMetadata, error) { // If we can't unmarshal the metadata into a PacketMetadata struct, // assume packet forwarding was no used and pass back nil so that autopilot is ignored @@ -85,7 +86,14 @@ func ParsePacketMetadata(metadata string) (*PacketForwardMetadata, error) { return nil, nil } - // If no forwarding logic was used for autopilot, return the metadata with each disabled + // Packets cannot be used for both autopilot and PFM at the same time + // If both fields were provided, reject the packet + if raw.Autopilot != nil && raw.Forward != nil { + return nil, errorsmod.Wrapf(ErrInvalidPacketMetadata, "autopilot and pfm cannot both be used in the same packet") + } + + // If no forwarding logic was used for autopilot, return nil to indicate that + // there's no autopilot action needed if raw.Autopilot == nil { return nil, nil } diff --git a/x/autopilot/types/parser_test.go b/x/autopilot/types/parser_test.go index 4679e7744..1f03d9e97 100644 --- a/x/autopilot/types/parser_test.go +++ b/x/autopilot/types/parser_test.go @@ -125,6 +125,11 @@ func TestParsePacketMetadata(t *testing.T) { metadata: validAddress, // normal address - not autopilot JSON expectedNilMetadata: true, }, + { + name: "PFM transfer", + metadata: `{"forward": {}}`, + expectedNilMetadata: true, + }, { name: "empty memo", metadata: "", @@ -140,6 +145,11 @@ func TestParsePacketMetadata(t *testing.T) { metadata: `{ "other_module": { } }`, expectedNilMetadata: true, }, + { + name: "both autopilot and pfm in the memo", + metadata: `{"autopilot": {}, "forward": {}}`, + expectedErr: "autopilot and pfm cannot both be used in the same packet", + }, { name: "empty receiver address", metadata: `{ "autopilot": { } }`, From 528994a6cebc49a7a0802c10c631f4dc2808dc49 Mon Sep 17 00:00:00 2001 From: sampocs Date: Thu, 21 Dec 2023 18:03:16 -0600 Subject: [PATCH 05/30] added hash receiver helper --- x/autopilot/types/autopilot.go | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 x/autopilot/types/autopilot.go diff --git a/x/autopilot/types/autopilot.go b/x/autopilot/types/autopilot.go new file mode 100644 index 000000000..3607d825d --- /dev/null +++ b/x/autopilot/types/autopilot.go @@ -0,0 +1,29 @@ +package types + +import ( + fmt "fmt" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/address" +) + +// GenerateHashedReceiver returns the receiver address for a given channel and original sender. +// It overrides the receiver address to be a hash of the channel/origSender so that +// the receiver address is deterministic and can be used to identify the sender on the +// initial chain. + +// GenerateHashedReceiver generates a new receiver address for a packet, by hashing +// the channel and original sender. +// This makes the receiver address deterministic and can used to identify the sender +// on the initial chain. +// Additionally, this prevents a forwarded packet from inpersonating a different account +// when moving to the next hop (i.e. receiver of this hop, becomes sender of next) +// +// This function was borrowed from PFM +func GenerateHashedReceiver(channelId, originalSender string) (string, error) { + senderStr := fmt.Sprintf("%s/%s", channelId, originalSender) + senderHash32 := address.Hash(ModuleName, []byte(senderStr)) + sender := sdk.AccAddress(senderHash32[:20]) + bech32Prefix := sdk.GetConfig().GetBech32AccountAddrPrefix() + return sdk.Bech32ifyAddressBytes(bech32Prefix, sender) +} From 264cc287d5965824c6bcee4ca8bf6022b37ebb99 Mon Sep 17 00:00:00 2001 From: sampocs Date: Fri, 22 Dec 2023 09:17:50 -0600 Subject: [PATCH 06/30] added bank keeper to autopilot --- app/app.go | 1 + x/autopilot/keeper/keeper.go | 3 +++ x/autopilot/types/autopilot.go | 34 +++++++++++++++++++++++++++ x/autopilot/types/expected_keepers.go | 9 +++++++ 4 files changed, 47 insertions(+) create mode 100644 x/autopilot/types/expected_keepers.go diff --git a/app/app.go b/app/app.go index de59481cb..d9b0c4c0b 100644 --- a/app/app.go +++ b/app/app.go @@ -612,6 +612,7 @@ func NewStrideApp( appCodec, keys[autopilottypes.StoreKey], app.GetSubspace(autopilottypes.ModuleName), + app.BankKeeper, app.StakeibcKeeper, app.ClaimKeeper, app.TransferKeeper, diff --git a/x/autopilot/keeper/keeper.go b/x/autopilot/keeper/keeper.go index b9baa54de..b74ccd94d 100644 --- a/x/autopilot/keeper/keeper.go +++ b/x/autopilot/keeper/keeper.go @@ -21,6 +21,7 @@ type ( Cdc codec.BinaryCodec storeKey storetypes.StoreKey paramstore paramtypes.Subspace + bankkeeper types.BankKeeper stakeibcKeeper stakeibckeeper.Keeper claimKeeper claimkeeper.Keeper transferKeeper ibctransferkeeper.Keeper @@ -31,6 +32,7 @@ func NewKeeper( Cdc codec.BinaryCodec, storeKey storetypes.StoreKey, ps paramtypes.Subspace, + bankKeeper types.BankKeeper, stakeibcKeeper stakeibckeeper.Keeper, claimKeeper claimkeeper.Keeper, transferKeeper ibctransferkeeper.Keeper, @@ -44,6 +46,7 @@ func NewKeeper( Cdc: Cdc, storeKey: storeKey, paramstore: ps, + bankkeeper: bankKeeper, stakeibcKeeper: stakeibcKeeper, claimKeeper: claimKeeper, transferKeeper: transferKeeper, diff --git a/x/autopilot/types/autopilot.go b/x/autopilot/types/autopilot.go index 3607d825d..a4051dd67 100644 --- a/x/autopilot/types/autopilot.go +++ b/x/autopilot/types/autopilot.go @@ -1,12 +1,46 @@ package types import ( + "errors" fmt "fmt" + sdkmath "cosmossdk.io/math" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/address" + transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" ) +// TokenPacketMetadata is meant to replicate transfertypes.FungibleTokenPacketData +// but it drops the original sender (who is untrusted) and adds a hashed receiver +// that can be used for any forwarding +type TokenPacketMetadata struct { + OriginalReceiver string + HashedReceiver string + Amount sdkmath.Int + Denom string +} + +// Builds a TokenPacketMetadata object using the fields of a FungibleTokenPacketData +// and adding a hashed receiver +func NewTokenPacketMetadata(channelId string, data transfertypes.FungibleTokenPacketData) (TokenPacketMetadata, error) { + hashedReceiver, err := GenerateHashedReceiver(channelId, data.Sender) + if err != nil { + return TokenPacketMetadata{}, err + } + + amount, ok := sdk.NewIntFromString(data.Amount) + if !ok { + return TokenPacketMetadata{}, errors.New("not a parsable amount field") + } + + return TokenPacketMetadata{ + OriginalReceiver: data.Receiver, + HashedReceiver: hashedReceiver, + Amount: amount, + Denom: data.Denom, + }, nil +} + // GenerateHashedReceiver returns the receiver address for a given channel and original sender. // It overrides the receiver address to be a hash of the channel/origSender so that // the receiver address is deterministic and can be used to identify the sender on the diff --git a/x/autopilot/types/expected_keepers.go b/x/autopilot/types/expected_keepers.go new file mode 100644 index 000000000..b7504d71e --- /dev/null +++ b/x/autopilot/types/expected_keepers.go @@ -0,0 +1,9 @@ +package types + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" +) + +type BankKeeper interface { + SendCoins(ctx sdk.Context, senderAddr sdk.AccAddress, recipientAddr sdk.AccAddress, amt sdk.Coins) error +} From 45b1465305f01984dadcb88f887570619701217f Mon Sep 17 00:00:00 2001 From: sampocs Date: Fri, 22 Dec 2023 09:52:48 -0600 Subject: [PATCH 07/30] first pass at hashed recipient implementation --- x/autopilot/keeper/airdrop.go | 16 ++++++++--- x/autopilot/keeper/liquidstake.go | 45 ++++++++++++++----------------- x/autopilot/module_ibc.go | 18 +++++++++---- x/autopilot/types/autopilot.go | 2 ++ x/autopilot/types/parser.go | 1 - 5 files changed, 47 insertions(+), 35 deletions(-) diff --git a/x/autopilot/keeper/airdrop.go b/x/autopilot/keeper/airdrop.go index 54e04a1d0..2bfa4bf90 100644 --- a/x/autopilot/keeper/airdrop.go +++ b/x/autopilot/keeper/airdrop.go @@ -20,8 +20,7 @@ import ( func (k Keeper) TryUpdateAirdropClaim( ctx sdk.Context, packet channeltypes.Packet, - data transfertypes.FungibleTokenPacketData, - packetMetadata types.ClaimPacketMetadata, + data types.TokenPacketMetadata, ) error { params := k.GetParams(ctx) if !params.ClaimActive { @@ -43,7 +42,7 @@ func (k Keeper) TryUpdateAirdropClaim( if senderStrideAddress == "" { return errorsmod.Wrapf(sdkerrors.ErrInvalidAddress, fmt.Sprintf("invalid sender address (%s)", data.Sender)) } - newStrideAddress := packetMetadata.StrideAddress + newStrideAddress := data.OriginalReceiver // find the airdrop for this host chain ID airdrop, found := k.claimKeeper.GetAirdropByChainId(ctx, hostZone.ChainId) @@ -58,5 +57,14 @@ func (k Keeper) TryUpdateAirdropClaim( k.Logger(ctx).Info(fmt.Sprintf("updating airdrop address %s (orig %s) to %s for airdrop %s", senderStrideAddress, data.Sender, newStrideAddress, airdropId)) - return k.claimKeeper.UpdateAirdropAddress(ctx, senderStrideAddress, newStrideAddress, airdropId) + if err := k.claimKeeper.UpdateAirdropAddress(ctx, senderStrideAddress, newStrideAddress, airdropId); err != nil { + return err + } + + // Finally send token back to the original reciever (since the hashed receiver was used for the transfer) + fromAddress := sdk.MustAccAddressFromBech32(data.HashedReceiver) + toAddress := sdk.MustAccAddressFromBech32(data.OriginalReceiver) + token := sdk.NewCoin(data.Denom, data.Amount) + + return k.bankkeeper.SendCoins(ctx, fromAddress, toAddress, sdk.NewCoins(token)) } diff --git a/x/autopilot/keeper/liquidstake.go b/x/autopilot/keeper/liquidstake.go index 2f31be671..07420577f 100644 --- a/x/autopilot/keeper/liquidstake.go +++ b/x/autopilot/keeper/liquidstake.go @@ -7,7 +7,6 @@ import ( errorsmod "cosmossdk.io/errors" 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" @@ -19,7 +18,7 @@ import ( func (k Keeper) TryLiquidStaking( ctx sdk.Context, packet channeltypes.Packet, - newData transfertypes.FungibleTokenPacketData, + newData types.TokenPacketMetadata, packetMetadata types.StakeibcPacketMetadata, ) error { params := k.GetParams(ctx) @@ -32,13 +31,8 @@ func (k Keeper) TryLiquidStaking( return errors.New("the native token is not supported for liquid staking") } - amount, ok := sdk.NewIntFromString(newData.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) + // Note: denom is base denom e.g. uatom - not ibc/xxx + var token = sdk.NewCoin(newData.Denom, newData.Amount) prefixedDenom := transfertypes.GetPrefixedDenom(packet.GetDestPort(), packet.GetDestChannel(), newData.Denom) ibcDenom := transfertypes.ParseDenomTrace(prefixedDenom).IBCDenom() @@ -52,19 +46,14 @@ func (k Keeper) TryLiquidStaking( 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, newData, packetMetadata) } -func (k Keeper) RunLiquidStake(ctx sdk.Context, addr sdk.AccAddress, token sdk.Coin, packetMetadata types.StakeibcPacketMetadata) error { +func (k Keeper) RunLiquidStake(ctx sdk.Context, transferMetadata types.TokenPacketMetadata, packetMetadata types.StakeibcPacketMetadata) error { msg := &stakeibctypes.MsgLiquidStake{ - Creator: addr.String(), - Amount: token.Amount, - HostDenom: token.Denom, + Creator: transferMetadata.HashedReceiver, + Amount: transferMetadata.Amount, + HostDenom: transferMetadata.Denom, } if err := msg.ValidateBasic(); err != nil { @@ -80,16 +69,22 @@ func (k Keeper) RunLiquidStake(ctx sdk.Context, addr sdk.AccAddress, token sdk.C return errorsmod.Wrapf(err, err.Error()) } - if packetMetadata.IbcReceiver == "" { - return nil - } - - hostZone, err := k.stakeibcKeeper.GetHostZoneFromHostDenom(ctx, token.Denom) + hostZone, err := k.stakeibcKeeper.GetHostZoneFromHostDenom(ctx, transferMetadata.Denom) if err != nil { return err } - return k.IBCTransferStAsset(ctx, result.StToken, addr.String(), hostZone, packetMetadata) + // If the IBCReceiver is empty, there is no forwarding step + // but we still need to transfer the stTokens to the original recipient + if packetMetadata.IbcReceiver == "" { + fromAddress := sdk.MustAccAddressFromBech32(transferMetadata.HashedReceiver) + toAddress := sdk.MustAccAddressFromBech32(transferMetadata.OriginalReceiver) + + return k.bankkeeper.SendCoins(ctx, fromAddress, toAddress, sdk.NewCoins(result.StToken)) + } + + // Otherwise, if there is forwarding info, submit the IBC transfer + return k.IBCTransferStAsset(ctx, result.StToken, transferMetadata.HashedReceiver, hostZone, packetMetadata) } func (k Keeper) IBCTransferStAsset(ctx sdk.Context, stAsset sdk.Coin, sender string, hostZone *stakeibctypes.HostZone, packetMetadata types.StakeibcPacketMetadata) error { diff --git a/x/autopilot/module_ibc.go b/x/autopilot/module_ibc.go index e4365c67c..824cfe407 100644 --- a/x/autopilot/module_ibc.go +++ b/x/autopilot/module_ibc.go @@ -7,7 +7,6 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" - ibctransfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types" porttypes "github.com/cosmos/ibc-go/v7/modules/core/05-port/types" @@ -168,10 +167,19 @@ func (im IBCModule) OnRecvPacket( return im.app.OnRecvPacket(ctx, packet, relayer) } + //// At this point, we are officially dealing with an autopilot packet + + // Build a new token packet metadata that includes a hashed receiver address + tokenPacketData, err := types.NewTokenPacketMetadata(packet.DestinationChannel, data) + if err != nil { + return channeltypes.NewErrorAcknowledgement(err) + } + // Modify the packet data by replacing the JSON metadata field with a receiver address // to allow the packet to continue down the stack + // Use the hashed receiver to prevent impersonation in downstream applications newData := data - newData.Receiver = packetForwardMetadata.Receiver + newData.Receiver = tokenPacketData.HashedReceiver bz, err := transfertypes.ModuleCdc.MarshalJSON(&newData) if err != nil { return channeltypes.NewErrorAcknowledgement(err) @@ -200,7 +208,7 @@ func (im IBCModule) OnRecvPacket( switch routingInfo.Action { case types.LiquidStake: // Try to liquid stake - return an ack error if it fails, otherwise return the ack generated from the earlier packet propogation - if err := im.keeper.TryLiquidStaking(ctx, packet, newData, routingInfo); err != nil { + if err := im.keeper.TryLiquidStaking(ctx, packet, tokenPacketData, routingInfo); err != nil { im.keeper.Logger(ctx).Error(fmt.Sprintf("Error liquid staking packet from autopilot for %s: %s", newData.Sender, err.Error())) return channeltypes.NewErrorAcknowledgement(err) } @@ -217,7 +225,7 @@ func (im IBCModule) OnRecvPacket( } im.keeper.Logger(ctx).Info(fmt.Sprintf("Forwaring packet from %s to claim", newData.Sender)) - if err := im.keeper.TryUpdateAirdropClaim(ctx, packet, newData, routingInfo); err != nil { + if err := im.keeper.TryUpdateAirdropClaim(ctx, packet, tokenPacketData); err != nil { im.keeper.Logger(ctx).Error(fmt.Sprintf("Error updating airdrop claim from autopilot for %s: %s", newData.Sender, err.Error())) return channeltypes.NewErrorAcknowledgement(err) } @@ -274,5 +282,5 @@ func (im IBCModule) WriteAcknowledgement( // GetAppVersion returns the interchain accounts metadata. func (im IBCModule) GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) { - return ibctransfertypes.Version, true // im.keeper.GetAppVersion(ctx, portID, channelID) + return transfertypes.Version, true // im.keeper.GetAppVersion(ctx, portID, channelID) } diff --git a/x/autopilot/types/autopilot.go b/x/autopilot/types/autopilot.go index a4051dd67..e2c75e90b 100644 --- a/x/autopilot/types/autopilot.go +++ b/x/autopilot/types/autopilot.go @@ -14,6 +14,7 @@ import ( // but it drops the original sender (who is untrusted) and adds a hashed receiver // that can be used for any forwarding type TokenPacketMetadata struct { + Sender string OriginalReceiver string HashedReceiver string Amount sdkmath.Int @@ -34,6 +35,7 @@ func NewTokenPacketMetadata(channelId string, data transfertypes.FungibleTokenPa } return TokenPacketMetadata{ + Sender: data.Sender, OriginalReceiver: data.Receiver, HashedReceiver: hashedReceiver, Amount: amount, diff --git a/x/autopilot/types/parser.go b/x/autopilot/types/parser.go index 7e8b3d2b0..09df29898 100644 --- a/x/autopilot/types/parser.go +++ b/x/autopilot/types/parser.go @@ -4,7 +4,6 @@ import ( "encoding/json" errorsmod "cosmossdk.io/errors" - sdk "github.com/cosmos/cosmos-sdk/types" ) From 99db7240ed37186bac6fe5635ff3735d70da467b Mon Sep 17 00:00:00 2001 From: sampocs Date: Fri, 22 Dec 2023 10:07:19 -0600 Subject: [PATCH 08/30] cleaned up variable names --- x/autopilot/keeper/airdrop.go | 16 ++++----- x/autopilot/keeper/liquidstake.go | 14 ++++---- x/autopilot/module_ibc.go | 54 ++++++++++++++++--------------- x/autopilot/types/autopilot.go | 12 +++---- x/autopilot/types/parser.go | 8 ++--- 5 files changed, 53 insertions(+), 51 deletions(-) diff --git a/x/autopilot/keeper/airdrop.go b/x/autopilot/keeper/airdrop.go index 2bfa4bf90..a4cde0375 100644 --- a/x/autopilot/keeper/airdrop.go +++ b/x/autopilot/keeper/airdrop.go @@ -20,7 +20,7 @@ import ( func (k Keeper) TryUpdateAirdropClaim( ctx sdk.Context, packet channeltypes.Packet, - data types.TokenPacketMetadata, + transferMetadata types.AutopilotTransferMetadata, ) error { params := k.GetParams(ctx) if !params.ClaimActive { @@ -38,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 := data.OriginalReceiver + newStrideAddress := transferMetadata.OriginalReceiver // find the airdrop for this host chain ID airdrop, found := k.claimKeeper.GetAirdropByChainId(ctx, hostZone.ChainId) @@ -55,16 +55,16 @@ 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)) if err := k.claimKeeper.UpdateAirdropAddress(ctx, senderStrideAddress, newStrideAddress, airdropId); err != nil { return err } // Finally send token back to the original reciever (since the hashed receiver was used for the transfer) - fromAddress := sdk.MustAccAddressFromBech32(data.HashedReceiver) - toAddress := sdk.MustAccAddressFromBech32(data.OriginalReceiver) - token := sdk.NewCoin(data.Denom, data.Amount) + fromAddress := sdk.MustAccAddressFromBech32(transferMetadata.HashedReceiver) + toAddress := sdk.MustAccAddressFromBech32(transferMetadata.OriginalReceiver) + token := sdk.NewCoin(transferMetadata.Denom, transferMetadata.Amount) return k.bankkeeper.SendCoins(ctx, fromAddress, toAddress, sdk.NewCoins(token)) } diff --git a/x/autopilot/keeper/liquidstake.go b/x/autopilot/keeper/liquidstake.go index 07420577f..186a9ecdf 100644 --- a/x/autopilot/keeper/liquidstake.go +++ b/x/autopilot/keeper/liquidstake.go @@ -18,8 +18,8 @@ import ( func (k Keeper) TryLiquidStaking( ctx sdk.Context, packet channeltypes.Packet, - newData types.TokenPacketMetadata, - packetMetadata types.StakeibcPacketMetadata, + transferMetadata types.AutopilotTransferMetadata, + actionMetadata types.StakeibcPacketMetadata, ) error { params := k.GetParams(ctx) if !params.StakeibcActive { @@ -27,14 +27,14 @@ func (k Keeper) TryLiquidStaking( } // 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) { + if transfertypes.ReceiverChainIsSource(packet.GetSourcePort(), packet.GetSourceChannel(), transferMetadata.Denom) { return errors.New("the native token is not supported for liquid staking") } // Note: denom is base denom e.g. uatom - not ibc/xxx - var token = sdk.NewCoin(newData.Denom, newData.Amount) + var token = sdk.NewCoin(transferMetadata.Denom, transferMetadata.Amount) - prefixedDenom := transfertypes.GetPrefixedDenom(packet.GetDestPort(), packet.GetDestChannel(), newData.Denom) + prefixedDenom := transfertypes.GetPrefixedDenom(packet.GetDestPort(), packet.GetDestChannel(), transferMetadata.Denom) ibcDenom := transfertypes.ParseDenomTrace(prefixedDenom).IBCDenom() hostZone, err := k.stakeibcKeeper.GetHostZoneFromHostDenom(ctx, token.Denom) @@ -46,10 +46,10 @@ func (k Keeper) TryLiquidStaking( return fmt.Errorf("ibc denom %s is not equal to host zone ibc denom %s", ibcDenom, hostZone.IbcDenom) } - return k.RunLiquidStake(ctx, newData, packetMetadata) + return k.RunLiquidStake(ctx, transferMetadata, actionMetadata) } -func (k Keeper) RunLiquidStake(ctx sdk.Context, transferMetadata types.TokenPacketMetadata, packetMetadata types.StakeibcPacketMetadata) error { +func (k Keeper) RunLiquidStake(ctx sdk.Context, transferMetadata types.AutopilotTransferMetadata, packetMetadata types.StakeibcPacketMetadata) error { msg := &stakeibctypes.MsgLiquidStake{ Creator: transferMetadata.HashedReceiver, Amount: transferMetadata.Amount, diff --git a/x/autopilot/module_ibc.go b/x/autopilot/module_ibc.go index 824cfe407..6bfa24021 100644 --- a/x/autopilot/module_ibc.go +++ b/x/autopilot/module_ibc.go @@ -124,38 +124,38 @@ func (im IBCModule) OnRecvPacket( packet.Sequence, packet.SourcePort, packet.SourceChannel, packet.DestinationPort, packet.DestinationChannel)) // NOTE: acknowledgement will be written synchronously during IBC handler execution. - var data transfertypes.FungibleTokenPacketData - if err := transfertypes.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil { + var tokenPacketData transfertypes.FungibleTokenPacketData + if err := transfertypes.ModuleCdc.UnmarshalJSON(packet.GetData(), &tokenPacketData); err != nil { return channeltypes.NewErrorAcknowledgement(err) } // Error any transactions with a Memo or Receiver field are greater than the max characters - if len(data.Memo) > MaxMemoCharLength { - return channeltypes.NewErrorAcknowledgement(errorsmod.Wrapf(types.ErrInvalidMemoSize, "memo length: %d", len(data.Memo))) + if len(tokenPacketData.Memo) > MaxMemoCharLength { + return channeltypes.NewErrorAcknowledgement(errorsmod.Wrapf(types.ErrInvalidMemoSize, "memo length: %d", len(tokenPacketData.Memo))) } - if len(data.Receiver) > MaxMemoCharLength { - return channeltypes.NewErrorAcknowledgement(errorsmod.Wrapf(types.ErrInvalidMemoSize, "receiver length: %d", len(data.Receiver))) + if len(tokenPacketData.Receiver) > MaxMemoCharLength { + return channeltypes.NewErrorAcknowledgement(errorsmod.Wrapf(types.ErrInvalidMemoSize, "receiver length: %d", len(tokenPacketData.Receiver))) } // ibc-go v5 has a Memo field that can store forwarding info // For older version of ibc-go, the data must be stored in the receiver field var metadata string - if data.Memo != "" { // ibc-go v5+ - metadata = data.Memo + if tokenPacketData.Memo != "" { // ibc-go v5+ + metadata = tokenPacketData.Memo } else { // before ibc-go v5 - metadata = data.Receiver + metadata = tokenPacketData.Receiver } // If a valid receiver address has been provided and no memo, // this is clearly just an normal IBC transfer // Pass down the stack immediately instead of parsing - _, err := sdk.AccAddressFromBech32(data.Receiver) - if err == nil && data.Memo == "" { + _, err := sdk.AccAddressFromBech32(tokenPacketData.Receiver) + if err == nil && tokenPacketData.Memo == "" { return im.app.OnRecvPacket(ctx, packet, relayer) } // parse out any forwarding info - packetForwardMetadata, err := types.ParsePacketMetadata(metadata) + autopilotActionMetadata, err := types.ParseAutopilotActionMetadata(metadata) if err != nil { return channeltypes.NewErrorAcknowledgement(err) } @@ -163,14 +163,15 @@ func (im IBCModule) OnRecvPacket( // If the parsed metadata is nil, that means there is no autopilot forwarding logic // Pass the packet down to the next middleware // PFM packets will also go down this path - if packetForwardMetadata == nil { + if autopilotActionMetadata == nil { return im.app.OnRecvPacket(ctx, packet, relayer) } //// At this point, we are officially dealing with an autopilot packet // Build a new token packet metadata that includes a hashed receiver address - tokenPacketData, err := types.NewTokenPacketMetadata(packet.DestinationChannel, data) + // This will be used for the remaining autopilot actions after the packet's been sent down the stack + autopilotTransferMetadata, err := types.NewAutopilotTransferMetadata(packet.DestinationChannel, tokenPacketData) if err != nil { return channeltypes.NewErrorAcknowledgement(err) } @@ -178,9 +179,9 @@ func (im IBCModule) OnRecvPacket( // Modify the packet data by replacing the JSON metadata field with a receiver address // to allow the packet to continue down the stack // Use the hashed receiver to prevent impersonation in downstream applications - newData := data - newData.Receiver = tokenPacketData.HashedReceiver - bz, err := transfertypes.ModuleCdc.MarshalJSON(&newData) + newTokenPacketData := tokenPacketData + newTokenPacketData.Receiver = autopilotTransferMetadata.HashedReceiver + bz, err := transfertypes.ModuleCdc.MarshalJSON(&newTokenPacketData) if err != nil { return channeltypes.NewErrorAcknowledgement(err) } @@ -194,22 +195,23 @@ func (im IBCModule) OnRecvPacket( } autopilotParams := im.keeper.GetParams(ctx) + sender := autopilotTransferMetadata.Sender // If the transfer was successful, then route to the corresponding module, if applicable - switch routingInfo := packetForwardMetadata.RoutingInfo.(type) { + switch routingInfo := autopilotActionMetadata.RoutingInfo.(type) { case types.StakeibcPacketMetadata: // If stakeibc routing is inactive (but the packet had routing info in the memo) return an ack error if !autopilotParams.StakeibcActive { - im.keeper.Logger(ctx).Error(fmt.Sprintf("Packet from %s had stakeibc routing info but autopilot stakeibc routing is disabled", newData.Sender)) + im.keeper.Logger(ctx).Error(fmt.Sprintf("Packet from %s had stakeibc routing info but autopilot stakeibc routing is disabled", sender)) return channeltypes.NewErrorAcknowledgement(types.ErrPacketForwardingInactive) } - im.keeper.Logger(ctx).Info(fmt.Sprintf("Forwaring packet from %s to stakeibc", newData.Sender)) + im.keeper.Logger(ctx).Info(fmt.Sprintf("Forwaring packet from %s to stakeibc", sender)) switch routingInfo.Action { case types.LiquidStake: // Try to liquid stake - return an ack error if it fails, otherwise return the ack generated from the earlier packet propogation - if err := im.keeper.TryLiquidStaking(ctx, packet, tokenPacketData, routingInfo); err != nil { - im.keeper.Logger(ctx).Error(fmt.Sprintf("Error liquid staking packet from autopilot for %s: %s", newData.Sender, err.Error())) + if err := im.keeper.TryLiquidStaking(ctx, packet, autopilotTransferMetadata, routingInfo); err != nil { + im.keeper.Logger(ctx).Error(fmt.Sprintf("Error liquid staking packet from autopilot for %s: %s", sender, err.Error())) return channeltypes.NewErrorAcknowledgement(err) } // case types.RedeemStake: TODO: add redeem stake logic @@ -220,13 +222,13 @@ func (im IBCModule) OnRecvPacket( case types.ClaimPacketMetadata: // If claim routing is inactive (but the packet had routing info in the memo) return an ack error if !autopilotParams.ClaimActive { - im.keeper.Logger(ctx).Error(fmt.Sprintf("Packet from %s had claim routing info but autopilot claim routing is disabled", newData.Sender)) + im.keeper.Logger(ctx).Error(fmt.Sprintf("Packet from %s had claim routing info but autopilot claim routing is disabled", sender)) return channeltypes.NewErrorAcknowledgement(types.ErrPacketForwardingInactive) } - im.keeper.Logger(ctx).Info(fmt.Sprintf("Forwaring packet from %s to claim", newData.Sender)) + im.keeper.Logger(ctx).Info(fmt.Sprintf("Forwaring packet from %s to claim", sender)) - if err := im.keeper.TryUpdateAirdropClaim(ctx, packet, tokenPacketData); err != nil { - im.keeper.Logger(ctx).Error(fmt.Sprintf("Error updating airdrop claim from autopilot for %s: %s", newData.Sender, err.Error())) + if err := im.keeper.TryUpdateAirdropClaim(ctx, packet, autopilotTransferMetadata); err != nil { + im.keeper.Logger(ctx).Error(fmt.Sprintf("Error updating airdrop claim from autopilot for %s: %s", sender, err.Error())) return channeltypes.NewErrorAcknowledgement(err) } diff --git a/x/autopilot/types/autopilot.go b/x/autopilot/types/autopilot.go index e2c75e90b..e2ba2aff8 100644 --- a/x/autopilot/types/autopilot.go +++ b/x/autopilot/types/autopilot.go @@ -13,7 +13,7 @@ import ( // TokenPacketMetadata is meant to replicate transfertypes.FungibleTokenPacketData // but it drops the original sender (who is untrusted) and adds a hashed receiver // that can be used for any forwarding -type TokenPacketMetadata struct { +type AutopilotTransferMetadata struct { Sender string OriginalReceiver string HashedReceiver string @@ -21,20 +21,20 @@ type TokenPacketMetadata struct { Denom string } -// Builds a TokenPacketMetadata object using the fields of a FungibleTokenPacketData +// Builds a AutopilotTransferMetadata object using the fields of a FungibleTokenPacketData // and adding a hashed receiver -func NewTokenPacketMetadata(channelId string, data transfertypes.FungibleTokenPacketData) (TokenPacketMetadata, error) { +func NewAutopilotTransferMetadata(channelId string, data transfertypes.FungibleTokenPacketData) (AutopilotTransferMetadata, error) { hashedReceiver, err := GenerateHashedReceiver(channelId, data.Sender) if err != nil { - return TokenPacketMetadata{}, err + return AutopilotTransferMetadata{}, err } amount, ok := sdk.NewIntFromString(data.Amount) if !ok { - return TokenPacketMetadata{}, errors.New("not a parsable amount field") + return AutopilotTransferMetadata{}, errors.New("not a parsable amount field") } - return TokenPacketMetadata{ + return AutopilotTransferMetadata{ Sender: data.Sender, OriginalReceiver: data.Receiver, HashedReceiver: hashedReceiver, diff --git a/x/autopilot/types/parser.go b/x/autopilot/types/parser.go index 09df29898..038b218be 100644 --- a/x/autopilot/types/parser.go +++ b/x/autopilot/types/parser.go @@ -16,7 +16,7 @@ type RawPacketMetadata struct { Forward *interface{} `json:"forward"` } -type PacketForwardMetadata struct { +type AutopilotActionMetadata struct { Receiver string RoutingInfo ModuleRoutingInfo } @@ -74,10 +74,10 @@ func (m ClaimPacketMetadata) Validate() error { // Parse packet metadata intended for autopilot // In the ICS-20 packet, the metadata can optionally indicate a module to route to (e.g. stakeibc) -// The PacketForwardMetadata returned from this function contains attributes for each autopilot supported module +// The AutopilotActionMetadata returned from this function contains attributes for each autopilot supported module // It can only be forward to one module per packet // Returns nil if there was no autopilot metadata found -func ParsePacketMetadata(metadata string) (*PacketForwardMetadata, error) { +func ParseAutopilotActionMetadata(metadata string) (*AutopilotActionMetadata, error) { // If we can't unmarshal the metadata into a PacketMetadata struct, // assume packet forwarding was no used and pass back nil so that autopilot is ignored var raw RawPacketMetadata @@ -127,7 +127,7 @@ func ParsePacketMetadata(metadata string) (*PacketForwardMetadata, error) { return nil, errorsmod.Wrapf(err, ErrInvalidPacketMetadata.Error()) } - return &PacketForwardMetadata{ + return &AutopilotActionMetadata{ Receiver: raw.Autopilot.Receiver, RoutingInfo: routingInfo, }, nil From f9d5d092049b2f5345f200ca71785932fbf8e8c2 Mon Sep 17 00:00:00 2001 From: sampocs Date: Fri, 22 Dec 2023 10:22:45 -0600 Subject: [PATCH 09/30] cleanup and docs --- x/autopilot/keeper/airdrop.go | 1 + x/autopilot/keeper/liquidstake.go | 77 +++++++++++++++++++------------ 2 files changed, 48 insertions(+), 30 deletions(-) diff --git a/x/autopilot/keeper/airdrop.go b/x/autopilot/keeper/airdrop.go index a4cde0375..43a470005 100644 --- a/x/autopilot/keeper/airdrop.go +++ b/x/autopilot/keeper/airdrop.go @@ -17,6 +17,7 @@ import ( 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, diff --git a/x/autopilot/keeper/liquidstake.go b/x/autopilot/keeper/liquidstake.go index 186a9ecdf..af43938a1 100644 --- a/x/autopilot/keeper/liquidstake.go +++ b/x/autopilot/keeper/liquidstake.go @@ -15,6 +15,8 @@ import ( stakeibctypes "github.com/Stride-Labs/stride/v16/x/stakeibc/types" ) +// 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, @@ -31,17 +33,18 @@ func (k Keeper) TryLiquidStaking( return errors.New("the native token is not supported for liquid staking") } - // Note: denom is base denom e.g. uatom - not ibc/xxx - var token = sdk.NewCoin(transferMetadata.Denom, transferMetadata.Amount) - + // 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 fmt.Errorf("host zone not found for denom (%s)", token.Denom) + 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) } @@ -49,7 +52,14 @@ func (k Keeper) TryLiquidStaking( return k.RunLiquidStake(ctx, transferMetadata, actionMetadata) } -func (k Keeper) RunLiquidStake(ctx sdk.Context, transferMetadata types.AutopilotTransferMetadata, packetMetadata types.StakeibcPacketMetadata) error { +// Submits a LiquidStake message from the hashed receiver +// If a forwarding recipient is specified, the stTokens are ibc transferred +// If there is no forwarding recipient, they are bank sent to the original receiver +func (k Keeper) RunLiquidStake( + ctx sdk.Context, + transferMetadata types.AutopilotTransferMetadata, + actionMetadata types.StakeibcPacketMetadata, +) error { msg := &stakeibctypes.MsgLiquidStake{ Creator: transferMetadata.HashedReceiver, Amount: transferMetadata.Amount, @@ -61,7 +71,7 @@ func (k Keeper) RunLiquidStake(ctx sdk.Context, transferMetadata types.Autopilot } msgServer := stakeibckeeper.NewMsgServerImpl(k.stakeibcKeeper) - result, err := msgServer.LiquidStake( + msgResponse, err := msgServer.LiquidStake( sdk.WrapSDKContext(ctx), msg, ) @@ -69,45 +79,52 @@ func (k Keeper) RunLiquidStake(ctx sdk.Context, transferMetadata types.Autopilot return errorsmod.Wrapf(err, err.Error()) } - hostZone, err := k.stakeibcKeeper.GetHostZoneFromHostDenom(ctx, transferMetadata.Denom) - if err != nil { - return err - } - // If the IBCReceiver is empty, there is no forwarding step // but we still need to transfer the stTokens to the original recipient - if packetMetadata.IbcReceiver == "" { + if actionMetadata.IbcReceiver == "" { fromAddress := sdk.MustAccAddressFromBech32(transferMetadata.HashedReceiver) toAddress := sdk.MustAccAddressFromBech32(transferMetadata.OriginalReceiver) - return k.bankkeeper.SendCoins(ctx, fromAddress, toAddress, sdk.NewCoins(result.StToken)) + return k.bankkeeper.SendCoins(ctx, fromAddress, toAddress, sdk.NewCoins(msgResponse.StToken)) } // Otherwise, if there is forwarding info, submit the IBC transfer - return k.IBCTransferStAsset(ctx, result.StToken, transferMetadata.HashedReceiver, hostZone, packetMetadata) + return k.IBCTransferStToken(ctx, msgResponse.StToken, transferMetadata, actionMetadata) } -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 +// 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 types.AutopilotTransferMetadata, + actionMetadata types.StakeibcPacketMetadata, +) error { + hostZone, err := k.stakeibcKeeper.GetHostZoneFromHostDenom(ctx, transferMetadata.Denom) + if err != nil { + return err + } + + // Use the default transfer timeout of 10 minutes + timeoutTimestamp := uint64(ctx.BlockTime().UnixNano()) + transfertypes.DefaultRelativePacketTimeoutTimestamp + + // If there's no channelID specified in the packet, default to the channel on the host zone + channelId := actionMetadata.TransferChannel if channelId == "" { channelId = hostZone.TransferChannelId } + + // The transfer message is sent from the hashed receiver to prevent impersonation transferMsg := &transfertypes.MsgTransfer{ - SourcePort: transfertypes.PortID, - SourceChannel: channelId, - Token: stAsset, - // TODO: does this reintroduce the bug in PFM where senders can be spoofed? - // If so, should we instead call PFM directly to forward the packet? - // Or should we obfuscate the sender, making it a random address? - Sender: sender, - Receiver: packetMetadata.IbcReceiver, + SourcePort: transfertypes.PortID, + SourceChannel: channelId, + Token: stToken, + Sender: transferMetadata.HashedReceiver, + Receiver: actionMetadata.IbcReceiver, TimeoutTimestamp: timeoutTimestamp, - // TimeoutHeight: clienttypes.Height{}, - // Memo: "stTokenIBCTransfer", + Memo: "autopilot-liquid-stake-and-forward", } - _, err := k.transferKeeper.Transfer(sdk.WrapSDKContext(ctx), transferMsg) + _, err = k.transferKeeper.Transfer(sdk.WrapSDKContext(ctx), transferMsg) return err } From 3bdfcd7815ea1be176c74fff59998ef9d8c6b86b Mon Sep 17 00:00:00 2001 From: sampocs Date: Fri, 22 Dec 2023 13:40:48 -0600 Subject: [PATCH 10/30] moved types to autopilot --- x/autopilot/types/autopilot.go | 26 ++++++++++++++++++++++++++ x/autopilot/types/parser.go | 18 ------------------ 2 files changed, 26 insertions(+), 18 deletions(-) diff --git a/x/autopilot/types/autopilot.go b/x/autopilot/types/autopilot.go index e2ba2aff8..3f87deb50 100644 --- a/x/autopilot/types/autopilot.go +++ b/x/autopilot/types/autopilot.go @@ -10,6 +10,20 @@ import ( transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" ) +// RawPacketMetadata defines the raw JSON memo that's used in an autopilot transfer +// The PFM forward key is also used here to validate that the packet is not trying +// to use autopilot and PFM at the same time +// As a result, only the forward key is needed, cause the actual parsing of the PFM +// packet will occur in the PFM module +type RawPacketMetadata struct { + Autopilot *struct { + Receiver string `json:"receiver"` + Stakeibc *StakeibcPacketMetadata `json:"stakeibc,omitempty"` + Claim *ClaimPacketMetadata `json:"claim,omitempty"` + } `json:"autopilot"` + Forward *interface{} `json:"forward"` +} + // TokenPacketMetadata is meant to replicate transfertypes.FungibleTokenPacketData // but it drops the original sender (who is untrusted) and adds a hashed receiver // that can be used for any forwarding @@ -21,6 +35,18 @@ type AutopilotTransferMetadata struct { Denom string } +// AutopilotActionMetadata stores the metadata that's specific to the autopilot action +// e.g. Fields required for LiquidStake +type AutopilotActionMetadata struct { + Receiver string + RoutingInfo ModuleRoutingInfo +} + +// ModuleRoutingInfo defines the interface required for each autopilot action +type ModuleRoutingInfo interface { + Validate() error +} + // Builds a AutopilotTransferMetadata object using the fields of a FungibleTokenPacketData // and adding a hashed receiver func NewAutopilotTransferMetadata(channelId string, data transfertypes.FungibleTokenPacketData) (AutopilotTransferMetadata, error) { diff --git a/x/autopilot/types/parser.go b/x/autopilot/types/parser.go index 038b218be..f3a0e66fd 100644 --- a/x/autopilot/types/parser.go +++ b/x/autopilot/types/parser.go @@ -7,24 +7,6 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" ) -type RawPacketMetadata struct { - Autopilot *struct { - Receiver string `json:"receiver"` - Stakeibc *StakeibcPacketMetadata `json:"stakeibc,omitempty"` - Claim *ClaimPacketMetadata `json:"claim,omitempty"` - } `json:"autopilot"` - Forward *interface{} `json:"forward"` -} - -type AutopilotActionMetadata struct { - Receiver string - RoutingInfo ModuleRoutingInfo -} - -type ModuleRoutingInfo interface { - Validate() error -} - const LiquidStake = "LiquidStake" const RedeemStake = "RedeemStake" From eb408d26eb7c4e6123fe7a5559379a59c11d39bc Mon Sep 17 00:00:00 2001 From: sampocs Date: Fri, 22 Dec 2023 16:01:57 -0600 Subject: [PATCH 11/30] walked back almost all the changes from above :( --- app/app.go | 1 - x/autopilot/keeper/airdrop.go | 16 ++--------- x/autopilot/keeper/keeper.go | 3 -- x/autopilot/keeper/liquidstake.go | 33 +++++++++++++-------- x/autopilot/module_ibc.go | 15 +++------- x/autopilot/types/autopilot.go | 41 ++++----------------------- x/autopilot/types/expected_keepers.go | 9 ------ x/autopilot/types/parser_test.go | 2 +- 8 files changed, 35 insertions(+), 85 deletions(-) delete mode 100644 x/autopilot/types/expected_keepers.go diff --git a/app/app.go b/app/app.go index d9b0c4c0b..de59481cb 100644 --- a/app/app.go +++ b/app/app.go @@ -612,7 +612,6 @@ func NewStrideApp( appCodec, keys[autopilottypes.StoreKey], app.GetSubspace(autopilottypes.ModuleName), - app.BankKeeper, app.StakeibcKeeper, app.ClaimKeeper, app.TransferKeeper, diff --git a/x/autopilot/keeper/airdrop.go b/x/autopilot/keeper/airdrop.go index 43a470005..934e4b037 100644 --- a/x/autopilot/keeper/airdrop.go +++ b/x/autopilot/keeper/airdrop.go @@ -12,7 +12,6 @@ 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" ) @@ -21,7 +20,7 @@ import ( func (k Keeper) TryUpdateAirdropClaim( ctx sdk.Context, packet channeltypes.Packet, - transferMetadata types.AutopilotTransferMetadata, + transferMetadata transfertypes.FungibleTokenPacketData, ) error { params := k.GetParams(ctx) if !params.ClaimActive { @@ -43,7 +42,7 @@ func (k Keeper) TryUpdateAirdropClaim( if senderStrideAddress == "" { return errorsmod.Wrapf(sdkerrors.ErrInvalidAddress, fmt.Sprintf("invalid sender address (%s)", transferMetadata.Sender)) } - newStrideAddress := transferMetadata.OriginalReceiver + newStrideAddress := transferMetadata.Receiver // find the airdrop for this host chain ID airdrop, found := k.claimKeeper.GetAirdropByChainId(ctx, hostZone.ChainId) @@ -58,14 +57,5 @@ func (k Keeper) TryUpdateAirdropClaim( k.Logger(ctx).Info(fmt.Sprintf("updating airdrop address %s (orig %s) to %s for airdrop %s", senderStrideAddress, transferMetadata.Sender, newStrideAddress, airdropId)) - if err := k.claimKeeper.UpdateAirdropAddress(ctx, senderStrideAddress, newStrideAddress, airdropId); err != nil { - return err - } - - // Finally send token back to the original reciever (since the hashed receiver was used for the transfer) - fromAddress := sdk.MustAccAddressFromBech32(transferMetadata.HashedReceiver) - toAddress := sdk.MustAccAddressFromBech32(transferMetadata.OriginalReceiver) - token := sdk.NewCoin(transferMetadata.Denom, transferMetadata.Amount) - - return k.bankkeeper.SendCoins(ctx, fromAddress, toAddress, sdk.NewCoins(token)) + return k.claimKeeper.UpdateAirdropAddress(ctx, senderStrideAddress, newStrideAddress, airdropId) } diff --git a/x/autopilot/keeper/keeper.go b/x/autopilot/keeper/keeper.go index b74ccd94d..b9baa54de 100644 --- a/x/autopilot/keeper/keeper.go +++ b/x/autopilot/keeper/keeper.go @@ -21,7 +21,6 @@ type ( Cdc codec.BinaryCodec storeKey storetypes.StoreKey paramstore paramtypes.Subspace - bankkeeper types.BankKeeper stakeibcKeeper stakeibckeeper.Keeper claimKeeper claimkeeper.Keeper transferKeeper ibctransferkeeper.Keeper @@ -32,7 +31,6 @@ func NewKeeper( Cdc codec.BinaryCodec, storeKey storetypes.StoreKey, ps paramtypes.Subspace, - bankKeeper types.BankKeeper, stakeibcKeeper stakeibckeeper.Keeper, claimKeeper claimkeeper.Keeper, transferKeeper ibctransferkeeper.Keeper, @@ -46,7 +44,6 @@ func NewKeeper( Cdc: Cdc, storeKey: storeKey, paramstore: ps, - bankkeeper: bankKeeper, stakeibcKeeper: stakeibcKeeper, claimKeeper: claimKeeper, transferKeeper: transferKeeper, diff --git a/x/autopilot/keeper/liquidstake.go b/x/autopilot/keeper/liquidstake.go index af43938a1..2f5e2138b 100644 --- a/x/autopilot/keeper/liquidstake.go +++ b/x/autopilot/keeper/liquidstake.go @@ -20,7 +20,7 @@ import ( func (k Keeper) TryLiquidStaking( ctx sdk.Context, packet channeltypes.Packet, - transferMetadata types.AutopilotTransferMetadata, + transferMetadata transfertypes.FungibleTokenPacketData, actionMetadata types.StakeibcPacketMetadata, ) error { params := k.GetParams(ctx) @@ -57,12 +57,17 @@ func (k Keeper) TryLiquidStaking( // If there is no forwarding recipient, they are bank sent to the original receiver func (k Keeper) RunLiquidStake( ctx sdk.Context, - transferMetadata types.AutopilotTransferMetadata, + transferMetadata transfertypes.FungibleTokenPacketData, actionMetadata types.StakeibcPacketMetadata, ) error { + amount, ok := sdk.NewIntFromString(transferMetadata.Amount) + if !ok { + return errors.New("not a parsable amount field") + } + msg := &stakeibctypes.MsgLiquidStake{ - Creator: transferMetadata.HashedReceiver, - Amount: transferMetadata.Amount, + Creator: transferMetadata.Receiver, + Amount: amount, HostDenom: transferMetadata.Denom, } @@ -80,12 +85,8 @@ func (k Keeper) RunLiquidStake( } // If the IBCReceiver is empty, there is no forwarding step - // but we still need to transfer the stTokens to the original recipient if actionMetadata.IbcReceiver == "" { - fromAddress := sdk.MustAccAddressFromBech32(transferMetadata.HashedReceiver) - toAddress := sdk.MustAccAddressFromBech32(transferMetadata.OriginalReceiver) - - return k.bankkeeper.SendCoins(ctx, fromAddress, toAddress, sdk.NewCoins(msgResponse.StToken)) + return nil } // Otherwise, if there is forwarding info, submit the IBC transfer @@ -97,7 +98,7 @@ func (k Keeper) RunLiquidStake( func (k Keeper) IBCTransferStToken( ctx sdk.Context, stToken sdk.Coin, - transferMetadata types.AutopilotTransferMetadata, + transferMetadata transfertypes.FungibleTokenPacketData, actionMetadata types.StakeibcPacketMetadata, ) error { hostZone, err := k.stakeibcKeeper.GetHostZoneFromHostDenom(ctx, transferMetadata.Denom) @@ -114,12 +115,20 @@ func (k Keeper) IBCTransferStToken( channelId = hostZone.TransferChannelId } - // The transfer message is sent from the hashed receiver to prevent impersonation + // Generate a hashed address for the sender to prevent impersonation at downstream zones + // Note: The channel ID here is different than the one used in PFM + // (we use the outbound channelID, they use the inbound channelID) + // DOUBLE CHECK ME that it shouldn't matter + hashedSender, err := types.GenerateHashedSender(channelId, transferMetadata.Sender) + if err != nil { + return err + } + transferMsg := &transfertypes.MsgTransfer{ SourcePort: transfertypes.PortID, SourceChannel: channelId, Token: stToken, - Sender: transferMetadata.HashedReceiver, + Sender: hashedSender, Receiver: actionMetadata.IbcReceiver, TimeoutTimestamp: timeoutTimestamp, Memo: "autopilot-liquid-stake-and-forward", diff --git a/x/autopilot/module_ibc.go b/x/autopilot/module_ibc.go index 6bfa24021..55006a9c9 100644 --- a/x/autopilot/module_ibc.go +++ b/x/autopilot/module_ibc.go @@ -169,18 +169,11 @@ func (im IBCModule) OnRecvPacket( //// At this point, we are officially dealing with an autopilot packet - // Build a new token packet metadata that includes a hashed receiver address - // This will be used for the remaining autopilot actions after the packet's been sent down the stack - autopilotTransferMetadata, err := types.NewAutopilotTransferMetadata(packet.DestinationChannel, tokenPacketData) - if err != nil { - return channeltypes.NewErrorAcknowledgement(err) - } - // Modify the packet data by replacing the JSON metadata field with a receiver address // to allow the packet to continue down the stack // Use the hashed receiver to prevent impersonation in downstream applications newTokenPacketData := tokenPacketData - newTokenPacketData.Receiver = autopilotTransferMetadata.HashedReceiver + newTokenPacketData.Receiver = autopilotActionMetadata.Receiver bz, err := transfertypes.ModuleCdc.MarshalJSON(&newTokenPacketData) if err != nil { return channeltypes.NewErrorAcknowledgement(err) @@ -195,7 +188,7 @@ func (im IBCModule) OnRecvPacket( } autopilotParams := im.keeper.GetParams(ctx) - sender := autopilotTransferMetadata.Sender + sender := tokenPacketData.Sender // If the transfer was successful, then route to the corresponding module, if applicable switch routingInfo := autopilotActionMetadata.RoutingInfo.(type) { @@ -210,7 +203,7 @@ func (im IBCModule) OnRecvPacket( switch routingInfo.Action { case types.LiquidStake: // Try to liquid stake - return an ack error if it fails, otherwise return the ack generated from the earlier packet propogation - if err := im.keeper.TryLiquidStaking(ctx, packet, autopilotTransferMetadata, routingInfo); err != nil { + if err := im.keeper.TryLiquidStaking(ctx, packet, newTokenPacketData, routingInfo); err != nil { im.keeper.Logger(ctx).Error(fmt.Sprintf("Error liquid staking packet from autopilot for %s: %s", sender, err.Error())) return channeltypes.NewErrorAcknowledgement(err) } @@ -227,7 +220,7 @@ func (im IBCModule) OnRecvPacket( } im.keeper.Logger(ctx).Info(fmt.Sprintf("Forwaring packet from %s to claim", sender)) - if err := im.keeper.TryUpdateAirdropClaim(ctx, packet, autopilotTransferMetadata); err != nil { + if err := im.keeper.TryUpdateAirdropClaim(ctx, packet, newTokenPacketData); err != nil { im.keeper.Logger(ctx).Error(fmt.Sprintf("Error updating airdrop claim from autopilot for %s: %s", sender, err.Error())) return channeltypes.NewErrorAcknowledgement(err) } diff --git a/x/autopilot/types/autopilot.go b/x/autopilot/types/autopilot.go index 3f87deb50..76cc601d2 100644 --- a/x/autopilot/types/autopilot.go +++ b/x/autopilot/types/autopilot.go @@ -1,13 +1,11 @@ package types import ( - "errors" fmt "fmt" sdkmath "cosmossdk.io/math" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/address" - transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" ) // RawPacketMetadata defines the raw JSON memo that's used in an autopilot transfer @@ -47,42 +45,15 @@ type ModuleRoutingInfo interface { Validate() error } -// Builds a AutopilotTransferMetadata object using the fields of a FungibleTokenPacketData -// and adding a hashed receiver -func NewAutopilotTransferMetadata(channelId string, data transfertypes.FungibleTokenPacketData) (AutopilotTransferMetadata, error) { - hashedReceiver, err := GenerateHashedReceiver(channelId, data.Sender) - if err != nil { - return AutopilotTransferMetadata{}, err - } - - amount, ok := sdk.NewIntFromString(data.Amount) - if !ok { - return AutopilotTransferMetadata{}, errors.New("not a parsable amount field") - } - - return AutopilotTransferMetadata{ - Sender: data.Sender, - OriginalReceiver: data.Receiver, - HashedReceiver: hashedReceiver, - Amount: amount, - Denom: data.Denom, - }, nil -} - -// GenerateHashedReceiver returns the receiver address for a given channel and original sender. -// It overrides the receiver address to be a hash of the channel/origSender so that -// the receiver address is deterministic and can be used to identify the sender on the -// initial chain. - -// GenerateHashedReceiver generates a new receiver address for a packet, by hashing +// GenerateHashedSender generates a new address for a packet, by hashing // the channel and original sender. -// This makes the receiver address deterministic and can used to identify the sender -// on the initial chain. -// Additionally, this prevents a forwarded packet from inpersonating a different account -// when moving to the next hop (i.e. receiver of this hop, becomes sender of next) +// This makes the address deterministic and can used to identify the sender +// from the preivous hop +// Additionally, this prevents a forwarded packet from impersonating a different account +// when moving to the next hop (i.e. receiver of one hop, becomes sender of next) // // This function was borrowed from PFM -func GenerateHashedReceiver(channelId, originalSender string) (string, error) { +func GenerateHashedSender(channelId, originalSender string) (string, error) { senderStr := fmt.Sprintf("%s/%s", channelId, originalSender) senderHash32 := address.Hash(ModuleName, []byte(senderStr)) sender := sdk.AccAddress(senderHash32[:20]) diff --git a/x/autopilot/types/expected_keepers.go b/x/autopilot/types/expected_keepers.go deleted file mode 100644 index b7504d71e..000000000 --- a/x/autopilot/types/expected_keepers.go +++ /dev/null @@ -1,9 +0,0 @@ -package types - -import ( - sdk "github.com/cosmos/cosmos-sdk/types" -) - -type BankKeeper interface { - SendCoins(ctx sdk.Context, senderAddr sdk.AccAddress, recipientAddr sdk.AccAddress, amt sdk.Coins) error -} diff --git a/x/autopilot/types/parser_test.go b/x/autopilot/types/parser_test.go index 1f03d9e97..382eaea99 100644 --- a/x/autopilot/types/parser_test.go +++ b/x/autopilot/types/parser_test.go @@ -184,7 +184,7 @@ func TestParsePacketMetadata(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - parsedData, actualErr := types.ParsePacketMetadata(tc.metadata) + parsedData, actualErr := types.ParseAutopilotActionMetadata(tc.metadata) if tc.expectedErr == "" { require.NoError(t, actualErr) From 8ee8ef2348725540ecafb00e5f7f574e28b61e16 Mon Sep 17 00:00:00 2001 From: sampocs Date: Fri, 22 Dec 2023 16:17:15 -0600 Subject: [PATCH 12/30] cleanup again --- x/autopilot/keeper/liquidstake.go | 38 ++++++++++++++++--------------- x/autopilot/module_ibc.go | 18 +++++++-------- x/autopilot/types/autopilot.go | 14 +----------- x/autopilot/types/parser.go | 6 ++--- x/autopilot/types/parser_test.go | 2 +- 5 files changed, 33 insertions(+), 45 deletions(-) diff --git a/x/autopilot/keeper/liquidstake.go b/x/autopilot/keeper/liquidstake.go index 2f5e2138b..6cad0a810 100644 --- a/x/autopilot/keeper/liquidstake.go +++ b/x/autopilot/keeper/liquidstake.go @@ -5,7 +5,7 @@ import ( "fmt" errorsmod "cosmossdk.io/errors" - + sdkmath "cosmossdk.io/math" sdk "github.com/cosmos/cosmos-sdk/types" transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types" @@ -21,14 +21,20 @@ func (k Keeper) TryLiquidStaking( ctx sdk.Context, packet channeltypes.Packet, transferMetadata transfertypes.FungibleTokenPacketData, - actionMetadata types.StakeibcPacketMetadata, + 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 + // Verify the amount is valid + amount, ok := sdk.NewIntFromString(transferMetadata.Amount) + if !ok { + return errors.New("not a parsable amount field") + } + + // 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") } @@ -49,22 +55,17 @@ func (k Keeper) TryLiquidStaking( return fmt.Errorf("ibc denom %s is not equal to host zone ibc denom %s", ibcDenom, hostZone.IbcDenom) } - return k.RunLiquidStake(ctx, transferMetadata, actionMetadata) + return k.RunLiquidStake(ctx, amount, transferMetadata, autopilotMetadata) } -// Submits a LiquidStake message from the hashed receiver +// Submits a LiquidStake message from the transfer receiver // If a forwarding recipient is specified, the stTokens are ibc transferred -// If there is no forwarding recipient, they are bank sent to the original receiver func (k Keeper) RunLiquidStake( ctx sdk.Context, + amount sdkmath.Int, transferMetadata transfertypes.FungibleTokenPacketData, - actionMetadata types.StakeibcPacketMetadata, + autopilotMetadata types.StakeibcPacketMetadata, ) error { - amount, ok := sdk.NewIntFromString(transferMetadata.Amount) - if !ok { - return errors.New("not a parsable amount field") - } - msg := &stakeibctypes.MsgLiquidStake{ Creator: transferMetadata.Receiver, Amount: amount, @@ -85,12 +86,12 @@ func (k Keeper) RunLiquidStake( } // If the IBCReceiver is empty, there is no forwarding step - if actionMetadata.IbcReceiver == "" { + if autopilotMetadata.IbcReceiver == "" { return nil } // Otherwise, if there is forwarding info, submit the IBC transfer - return k.IBCTransferStToken(ctx, msgResponse.StToken, transferMetadata, actionMetadata) + 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) @@ -99,7 +100,7 @@ func (k Keeper) IBCTransferStToken( ctx sdk.Context, stToken sdk.Coin, transferMetadata transfertypes.FungibleTokenPacketData, - actionMetadata types.StakeibcPacketMetadata, + autopilotMetadata types.StakeibcPacketMetadata, ) error { hostZone, err := k.stakeibcKeeper.GetHostZoneFromHostDenom(ctx, transferMetadata.Denom) if err != nil { @@ -110,12 +111,13 @@ func (k Keeper) IBCTransferStToken( timeoutTimestamp := uint64(ctx.BlockTime().UnixNano()) + transfertypes.DefaultRelativePacketTimeoutTimestamp // If there's no channelID specified in the packet, default to the channel on the host zone - channelId := actionMetadata.TransferChannel + channelId := autopilotMetadata.TransferChannel if channelId == "" { channelId = hostZone.TransferChannelId } - // Generate a hashed address for the sender to prevent impersonation at downstream zones + // Generate a hashed address for the sender to the next hop, + // to prevent impersonation at downstream zones // Note: The channel ID here is different than the one used in PFM // (we use the outbound channelID, they use the inbound channelID) // DOUBLE CHECK ME that it shouldn't matter @@ -129,7 +131,7 @@ func (k Keeper) IBCTransferStToken( SourceChannel: channelId, Token: stToken, Sender: hashedSender, - Receiver: actionMetadata.IbcReceiver, + Receiver: autopilotMetadata.IbcReceiver, TimeoutTimestamp: timeoutTimestamp, Memo: "autopilot-liquid-stake-and-forward", } diff --git a/x/autopilot/module_ibc.go b/x/autopilot/module_ibc.go index 55006a9c9..031ae0555 100644 --- a/x/autopilot/module_ibc.go +++ b/x/autopilot/module_ibc.go @@ -154,8 +154,8 @@ func (im IBCModule) OnRecvPacket( return im.app.OnRecvPacket(ctx, packet, relayer) } - // parse out any forwarding info - autopilotActionMetadata, err := types.ParseAutopilotActionMetadata(metadata) + // parse out any autopilot forwarding info + autopilotMetadata, err := types.ParseAutopilotMetadata(metadata) if err != nil { return channeltypes.NewErrorAcknowledgement(err) } @@ -163,7 +163,7 @@ func (im IBCModule) OnRecvPacket( // If the parsed metadata is nil, that means there is no autopilot forwarding logic // Pass the packet down to the next middleware // PFM packets will also go down this path - if autopilotActionMetadata == nil { + if autopilotMetadata == nil { return im.app.OnRecvPacket(ctx, packet, relayer) } @@ -171,10 +171,8 @@ func (im IBCModule) OnRecvPacket( // Modify the packet data by replacing the JSON metadata field with a receiver address // to allow the packet to continue down the stack - // Use the hashed receiver to prevent impersonation in downstream applications - newTokenPacketData := tokenPacketData - newTokenPacketData.Receiver = autopilotActionMetadata.Receiver - bz, err := transfertypes.ModuleCdc.MarshalJSON(&newTokenPacketData) + tokenPacketData.Receiver = autopilotMetadata.Receiver + bz, err := transfertypes.ModuleCdc.MarshalJSON(&tokenPacketData) if err != nil { return channeltypes.NewErrorAcknowledgement(err) } @@ -191,7 +189,7 @@ func (im IBCModule) OnRecvPacket( sender := tokenPacketData.Sender // If the transfer was successful, then route to the corresponding module, if applicable - switch routingInfo := autopilotActionMetadata.RoutingInfo.(type) { + switch routingInfo := autopilotMetadata.RoutingInfo.(type) { case types.StakeibcPacketMetadata: // If stakeibc routing is inactive (but the packet had routing info in the memo) return an ack error if !autopilotParams.StakeibcActive { @@ -203,7 +201,7 @@ func (im IBCModule) OnRecvPacket( switch routingInfo.Action { case types.LiquidStake: // Try to liquid stake - return an ack error if it fails, otherwise return the ack generated from the earlier packet propogation - if err := im.keeper.TryLiquidStaking(ctx, packet, newTokenPacketData, routingInfo); err != nil { + if err := im.keeper.TryLiquidStaking(ctx, packet, tokenPacketData, routingInfo); err != nil { im.keeper.Logger(ctx).Error(fmt.Sprintf("Error liquid staking packet from autopilot for %s: %s", sender, err.Error())) return channeltypes.NewErrorAcknowledgement(err) } @@ -220,7 +218,7 @@ func (im IBCModule) OnRecvPacket( } im.keeper.Logger(ctx).Info(fmt.Sprintf("Forwaring packet from %s to claim", sender)) - if err := im.keeper.TryUpdateAirdropClaim(ctx, packet, newTokenPacketData); err != nil { + if err := im.keeper.TryUpdateAirdropClaim(ctx, packet, tokenPacketData); err != nil { im.keeper.Logger(ctx).Error(fmt.Sprintf("Error updating airdrop claim from autopilot for %s: %s", sender, err.Error())) return channeltypes.NewErrorAcknowledgement(err) } diff --git a/x/autopilot/types/autopilot.go b/x/autopilot/types/autopilot.go index 76cc601d2..e5f1f1298 100644 --- a/x/autopilot/types/autopilot.go +++ b/x/autopilot/types/autopilot.go @@ -3,7 +3,6 @@ package types import ( fmt "fmt" - sdkmath "cosmossdk.io/math" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/address" ) @@ -22,20 +21,9 @@ type RawPacketMetadata struct { Forward *interface{} `json:"forward"` } -// TokenPacketMetadata is meant to replicate transfertypes.FungibleTokenPacketData -// but it drops the original sender (who is untrusted) and adds a hashed receiver -// that can be used for any forwarding -type AutopilotTransferMetadata struct { - Sender string - OriginalReceiver string - HashedReceiver string - Amount sdkmath.Int - Denom string -} - // AutopilotActionMetadata stores the metadata that's specific to the autopilot action // e.g. Fields required for LiquidStake -type AutopilotActionMetadata struct { +type AutopilotMetadata struct { Receiver string RoutingInfo ModuleRoutingInfo } diff --git a/x/autopilot/types/parser.go b/x/autopilot/types/parser.go index f3a0e66fd..8ef77c893 100644 --- a/x/autopilot/types/parser.go +++ b/x/autopilot/types/parser.go @@ -56,10 +56,10 @@ func (m ClaimPacketMetadata) Validate() error { // Parse packet metadata intended for autopilot // In the ICS-20 packet, the metadata can optionally indicate a module to route to (e.g. stakeibc) -// The AutopilotActionMetadata returned from this function contains attributes for each autopilot supported module +// The AutopilotMetadata returned from this function contains attributes for each autopilot supported module // It can only be forward to one module per packet // Returns nil if there was no autopilot metadata found -func ParseAutopilotActionMetadata(metadata string) (*AutopilotActionMetadata, error) { +func ParseAutopilotMetadata(metadata string) (*AutopilotMetadata, error) { // If we can't unmarshal the metadata into a PacketMetadata struct, // assume packet forwarding was no used and pass back nil so that autopilot is ignored var raw RawPacketMetadata @@ -109,7 +109,7 @@ func ParseAutopilotActionMetadata(metadata string) (*AutopilotActionMetadata, er return nil, errorsmod.Wrapf(err, ErrInvalidPacketMetadata.Error()) } - return &AutopilotActionMetadata{ + return &AutopilotMetadata{ Receiver: raw.Autopilot.Receiver, RoutingInfo: routingInfo, }, nil diff --git a/x/autopilot/types/parser_test.go b/x/autopilot/types/parser_test.go index 382eaea99..8d83afd40 100644 --- a/x/autopilot/types/parser_test.go +++ b/x/autopilot/types/parser_test.go @@ -184,7 +184,7 @@ func TestParsePacketMetadata(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - parsedData, actualErr := types.ParseAutopilotActionMetadata(tc.metadata) + parsedData, actualErr := types.ParseAutopilotMetadata(tc.metadata) if tc.expectedErr == "" { require.NoError(t, actualErr) From b1d45031a66d7ba2125df880b998ea43c886f27b Mon Sep 17 00:00:00 2001 From: sampocs Date: Wed, 27 Dec 2023 17:41:20 -0600 Subject: [PATCH 13/30] added bank keeper again --- app/app.go | 1 + x/autopilot/keeper/keeper.go | 3 +++ x/autopilot/types/expected_keepers.go | 9 +++++++++ 3 files changed, 13 insertions(+) create mode 100644 x/autopilot/types/expected_keepers.go diff --git a/app/app.go b/app/app.go index de59481cb..d9b0c4c0b 100644 --- a/app/app.go +++ b/app/app.go @@ -612,6 +612,7 @@ func NewStrideApp( appCodec, keys[autopilottypes.StoreKey], app.GetSubspace(autopilottypes.ModuleName), + app.BankKeeper, app.StakeibcKeeper, app.ClaimKeeper, app.TransferKeeper, diff --git a/x/autopilot/keeper/keeper.go b/x/autopilot/keeper/keeper.go index b9baa54de..a5ded2889 100644 --- a/x/autopilot/keeper/keeper.go +++ b/x/autopilot/keeper/keeper.go @@ -21,6 +21,7 @@ type ( Cdc codec.BinaryCodec storeKey storetypes.StoreKey paramstore paramtypes.Subspace + bankKeeper types.BankKeeper stakeibcKeeper stakeibckeeper.Keeper claimKeeper claimkeeper.Keeper transferKeeper ibctransferkeeper.Keeper @@ -31,6 +32,7 @@ func NewKeeper( Cdc codec.BinaryCodec, storeKey storetypes.StoreKey, ps paramtypes.Subspace, + bankKeeper types.BankKeeper, stakeibcKeeper stakeibckeeper.Keeper, claimKeeper claimkeeper.Keeper, transferKeeper ibctransferkeeper.Keeper, @@ -44,6 +46,7 @@ func NewKeeper( Cdc: Cdc, storeKey: storeKey, paramstore: ps, + bankKeeper: bankKeeper, stakeibcKeeper: stakeibcKeeper, claimKeeper: claimKeeper, transferKeeper: transferKeeper, diff --git a/x/autopilot/types/expected_keepers.go b/x/autopilot/types/expected_keepers.go new file mode 100644 index 000000000..b7504d71e --- /dev/null +++ b/x/autopilot/types/expected_keepers.go @@ -0,0 +1,9 @@ +package types + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" +) + +type BankKeeper interface { + SendCoins(ctx sdk.Context, senderAddr sdk.AccAddress, recipientAddr sdk.AccAddress, amt sdk.Coins) error +} From 208eac7ad8701f7be73a09cab5dab7d5d196cb82 Mon Sep 17 00:00:00 2001 From: sampocs Date: Wed, 27 Dec 2023 17:48:08 -0600 Subject: [PATCH 14/30] added bank send to hashed sender --- x/autopilot/keeper/liquidstake.go | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/x/autopilot/keeper/liquidstake.go b/x/autopilot/keeper/liquidstake.go index 6cad0a810..df92e1102 100644 --- a/x/autopilot/keeper/liquidstake.go +++ b/x/autopilot/keeper/liquidstake.go @@ -107,9 +107,6 @@ func (k Keeper) IBCTransferStToken( return err } - // Use the default transfer timeout of 10 minutes - timeoutTimestamp := uint64(ctx.BlockTime().UnixNano()) + transfertypes.DefaultRelativePacketTimeoutTimestamp - // If there's no channelID specified in the packet, default to the channel on the host zone channelId := autopilotMetadata.TransferChannel if channelId == "" { @@ -121,21 +118,32 @@ func (k Keeper) IBCTransferStToken( // Note: The channel ID here is different than the one used in PFM // (we use the outbound channelID, they use the inbound channelID) // DOUBLE CHECK ME that it shouldn't matter - hashedSender, err := types.GenerateHashedSender(channelId, transferMetadata.Sender) + hashedAddress, err := types.GenerateHashedSender(channelId, transferMetadata.Sender) if err != nil { return err } + // First we need to bank send to the hashed address + originalReceiver := sdk.MustAccAddressFromBech32(transferMetadata.Receiver) + hashedAccount := sdk.MustAccAddressFromBech32(hashedAddress) + if err := k.bankKeeper.SendCoins(ctx, originalReceiver, hashedAccount, sdk.NewCoins(stToken)); err != nil { + return err + } + + // Use the default transfer timeout of 10 minutes + timeoutTimestamp := uint64(ctx.BlockTime().UnixNano()) + transfertypes.DefaultRelativePacketTimeoutTimestamp + + // Submit the transfer from the hashed sender transferMsg := &transfertypes.MsgTransfer{ SourcePort: transfertypes.PortID, SourceChannel: channelId, Token: stToken, - Sender: hashedSender, + Sender: hashedAddress, Receiver: autopilotMetadata.IbcReceiver, TimeoutTimestamp: timeoutTimestamp, Memo: "autopilot-liquid-stake-and-forward", } - _, err = k.transferKeeper.Transfer(sdk.WrapSDKContext(ctx), transferMsg) + return err } From 2c22d176995f414e92a08a3a6f275d732ac7c386 Mon Sep 17 00:00:00 2001 From: sampocs Date: Wed, 27 Dec 2023 18:18:02 -0600 Subject: [PATCH 15/30] renamed hashed address function --- x/autopilot/keeper/liquidstake.go | 16 +++++++++++----- x/autopilot/types/autopilot.go | 2 +- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/x/autopilot/keeper/liquidstake.go b/x/autopilot/keeper/liquidstake.go index df92e1102..36d354eee 100644 --- a/x/autopilot/keeper/liquidstake.go +++ b/x/autopilot/keeper/liquidstake.go @@ -118,22 +118,28 @@ func (k Keeper) IBCTransferStToken( // Note: The channel ID here is different than the one used in PFM // (we use the outbound channelID, they use the inbound channelID) // DOUBLE CHECK ME that it shouldn't matter - hashedAddress, err := types.GenerateHashedSender(channelId, transferMetadata.Sender) + hashedAddress, err := types.GenerateHashedAddress(channelId, transferMetadata.Sender) if err != nil { return err } // First we need to bank send to the hashed address - originalReceiver := sdk.MustAccAddressFromBech32(transferMetadata.Receiver) - hashedAccount := sdk.MustAccAddressFromBech32(hashedAddress) - if err := k.bankKeeper.SendCoins(ctx, originalReceiver, hashedAccount, sdk.NewCoins(stToken)); err != nil { + originalReceiver, err := sdk.AccAddressFromBech32(transferMetadata.Receiver) + if err != nil { + return err + } + hashedSender, err := sdk.AccAddressFromBech32(hashedAddress) + if err != nil { + return err + } + if err := k.bankKeeper.SendCoins(ctx, originalReceiver, hashedSender, sdk.NewCoins(stToken)); err != nil { return err } // Use the default transfer timeout of 10 minutes timeoutTimestamp := uint64(ctx.BlockTime().UnixNano()) + transfertypes.DefaultRelativePacketTimeoutTimestamp - // Submit the transfer from the hashed sender + // Submit the transfer from the hashed address transferMsg := &transfertypes.MsgTransfer{ SourcePort: transfertypes.PortID, SourceChannel: channelId, diff --git a/x/autopilot/types/autopilot.go b/x/autopilot/types/autopilot.go index e5f1f1298..44e8eaf67 100644 --- a/x/autopilot/types/autopilot.go +++ b/x/autopilot/types/autopilot.go @@ -41,7 +41,7 @@ type ModuleRoutingInfo interface { // when moving to the next hop (i.e. receiver of one hop, becomes sender of next) // // This function was borrowed from PFM -func GenerateHashedSender(channelId, originalSender string) (string, error) { +func GenerateHashedAddress(channelId, originalSender string) (string, error) { senderStr := fmt.Sprintf("%s/%s", channelId, originalSender) senderHash32 := address.Hash(ModuleName, []byte(senderStr)) sender := sdk.AccAddress(senderHash32[:20]) From c0ca4f55fba465b125df6a8f0aac386927c8d36e Mon Sep 17 00:00:00 2001 From: sampocs Date: Thu, 28 Dec 2023 14:58:55 -0600 Subject: [PATCH 16/30] added keepers to store fallback address --- x/autopilot/keeper/fallback.go | 39 +++++++++++++++++++++++++++++ x/autopilot/keeper/fallback_test.go | 22 ++++++++++++++++ x/autopilot/types/keys.go | 20 +++++++++++++++ 3 files changed, 81 insertions(+) create mode 100644 x/autopilot/keeper/fallback.go create mode 100644 x/autopilot/keeper/fallback_test.go diff --git a/x/autopilot/keeper/fallback.go b/x/autopilot/keeper/fallback.go new file mode 100644 index 000000000..cccb5bc96 --- /dev/null +++ b/x/autopilot/keeper/fallback.go @@ -0,0 +1,39 @@ +package keeper + +import ( + "github.com/cosmos/cosmos-sdk/store/prefix" + sdk "github.com/cosmos/cosmos-sdk/types" + + "github.com/Stride-Labs/stride/v16/x/autopilot/types" +) + +// Stores a fallback address for an outbound transfer +func (k Keeper) SetTransferFallbackAddress(ctx sdk.Context, channelId string, sequence uint64, address string) { + store := prefix.NewStore(ctx.KVStore(k.storeKey), types.TransferFallbackAddressPrefix) + key := types.GetTransferFallbackAddressKeyPrefix(channelId, sequence) + value := []byte(address) + store.Set(key, value) +} + +// Removes a fallback address from the store +// This is used after the ack or timeout for a packet has been received +func (k Keeper) RemoveTransferFallbackAddress(ctx sdk.Context, channelId string, sequence uint64) { + store := prefix.NewStore(ctx.KVStore(k.storeKey), types.TransferFallbackAddressPrefix) + key := types.GetTransferFallbackAddressKeyPrefix(channelId, sequence) + store.Delete(key) +} + +// Returns a fallback address, given the channel ID and sequence number of the packet +// If no fallback address has been stored, return false +func (k Keeper) GetTransferFallbackAddress(ctx sdk.Context, channelId string, sequence uint64) (address string, found bool) { + store := prefix.NewStore(ctx.KVStore(k.storeKey), types.TransferFallbackAddressPrefix) + + key := types.GetTransferFallbackAddressKeyPrefix(channelId, sequence) + valueBz := store.Get(key) + + if len(valueBz) == 0 { + return "", false + } + + return string(valueBz), true +} diff --git a/x/autopilot/keeper/fallback_test.go b/x/autopilot/keeper/fallback_test.go new file mode 100644 index 000000000..a70c5900f --- /dev/null +++ b/x/autopilot/keeper/fallback_test.go @@ -0,0 +1,22 @@ +package keeper_test + +// Tests Get/Set/RemoveTransferFallbackAddress +func (s *KeeperTestSuite) TestTransferFallbackAddress() { + channelId := "channel-0" + sequence := uint64(100) + expectedAddress := "stride1xjp08gxef09fck6yj2lg0vrgpcjhqhp055ffhj" + + // Add a new fallback address + s.App.AutopilotKeeper.SetTransferFallbackAddress(s.Ctx, channelId, sequence, expectedAddress) + + // Confirm we can retrieve it + actualAddress, found := s.App.AutopilotKeeper.GetTransferFallbackAddress(s.Ctx, channelId, sequence) + s.Require().True(found, "address should have been found") + s.Require().Equal(expectedAddress, actualAddress, "fallback addres") + + // Remove it and confirm we can no longer retrieve it + s.App.AutopilotKeeper.RemoveTransferFallbackAddress(s.Ctx, channelId, sequence) + + _, found = s.App.AutopilotKeeper.GetTransferFallbackAddress(s.Ctx, channelId, sequence) + s.Require().False(found, "address should have been removed") +} diff --git a/x/autopilot/types/keys.go b/x/autopilot/types/keys.go index 57b6309e6..eb85bb3ad 100644 --- a/x/autopilot/types/keys.go +++ b/x/autopilot/types/keys.go @@ -1,5 +1,7 @@ package types +import "encoding/binary" + const ( // ModuleName defines the module name ModuleName = "autopilot" @@ -13,3 +15,21 @@ const ( // QuerierRoute defines the module's query routing key QuerierRoute = ModuleName ) + +var ( + TransferFallbackAddressPrefix = []byte("fallback") + + FallbackAddressChannelPrefixLength int = 16 +) + +// Builds the store key prefix for a fallback address, key'd by channel ID and sequence number +// The serialized channelId is set to a fixed array size to assist deserialization +func GetTransferFallbackAddressKeyPrefix(channelId string, sequenceNumber uint64) []byte { + channelIdBz := make([]byte, FallbackAddressChannelPrefixLength) + copy(channelIdBz[:], channelId) + + sequenceNumberBz := make([]byte, 8) + binary.BigEndian.PutUint64(sequenceNumberBz, sequenceNumber) + + return append(channelIdBz, sequenceNumberBz...) +} From a119b8db54ac2afcce499961b56c153922e2c500 Mon Sep 17 00:00:00 2001 From: sampocs Date: Thu, 28 Dec 2023 15:58:01 -0600 Subject: [PATCH 17/30] implemented onAck and onTimeout --- x/autopilot/keeper/ibc.go | 160 ++++++++++++++++++++++++++++++ x/autopilot/keeper/liquidstake.go | 11 +- x/autopilot/module_ibc.go | 21 +++- 3 files changed, 186 insertions(+), 6 deletions(-) create mode 100644 x/autopilot/keeper/ibc.go diff --git a/x/autopilot/keeper/ibc.go b/x/autopilot/keeper/ibc.go new file mode 100644 index 000000000..7caeb4c30 --- /dev/null +++ b/x/autopilot/keeper/ibc.go @@ -0,0 +1,160 @@ +package keeper + +import ( + "fmt" + + errorsmod "cosmossdk.io/errors" + sdkmath "cosmossdk.io/math" + sdk "github.com/cosmos/cosmos-sdk/types" + transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" + channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types" +) + +// Deserializes the acknowledgement and returns a bool indicating whether it was successful or was an ack error +func (k Keeper) CheckAcknowledgementStatus(ctx sdk.Context, acknowledgementBz []byte) (success bool, err error) { + // Unmarshal the raw ack response + var acknowledgement channeltypes.Acknowledgement + if err := transfertypes.ModuleCdc.UnmarshalJSON(acknowledgementBz, &acknowledgement); err != nil { + return false, errorsmod.Wrapf(err, "cannot unmarshal ICS-20 transfer packet acknowledgement") + } + + // The ack can come back as either AcknowledgementResult (success) or AcknowledgementError (failure) + switch response := acknowledgement.Response.(type) { + case *channeltypes.Acknowledgement_Result: + if len(response.Result) == 0 { + return false, errorsmod.Wrapf(channeltypes.ErrInvalidAcknowledgement, "acknowledgement result cannot be empty") + } + return true, nil + + case *channeltypes.Acknowledgement_Error: + k.Logger(ctx).Error(fmt.Sprintf("autopilot acknowledgement error: %s", response.Error)) + return false, nil + + default: + return false, errorsmod.Wrapf(channeltypes.ErrInvalidAcknowledgement, "unsupported acknowledgement response field type %T", response) + } +} + +// Build an sdk.Coin type from the transfer metadata which includes strings for the amount and denom +func (k Keeper) BuildCoinFromTransferMetadata(transferMetadata transfertypes.FungibleTokenPacketData) (coin sdk.Coin, err error) { + amount, ok := sdkmath.NewIntFromString(transferMetadata.Amount) + if !ok { + return coin, fmt.Errorf("unable to parse amount from transfer packet: %v", transferMetadata) + } + coin = sdk.NewCoin(transferMetadata.Denom, amount) + return coin, nil +} + +// In the event of an ack error after a outbound transfer, we'll have to bank send to a fallback address +func (k Keeper) SendToFallbackAddress(ctx sdk.Context, packet channeltypes.Packet, fallbackAddress string) error { + // First unmarshal the transfer metadata to get the sender/reciever, and token amount/denom + var transferMetadata transfertypes.FungibleTokenPacketData + if err := transfertypes.ModuleCdc.UnmarshalJSON(packet.GetData(), &transferMetadata); err != nil { + return err + } + + // Pull out the original sender of the transfer which will also be the bank sender + sender := transferMetadata.Sender + senderAccount, err := sdk.AccAddressFromBech32(sender) + if err != nil { + return err + } + fallbackAccount, err := sdk.AccAddressFromBech32(fallbackAddress) + if err != nil { + return err + } + + // Build the token from the transfer metadata + token, err := k.BuildCoinFromTransferMetadata(transferMetadata) + if err != nil { + return err + } + + // Finally send to the fallback account + if err := k.bankKeeper.SendCoins(ctx, senderAccount, fallbackAccount, sdk.NewCoins(token)); err != nil { + return err + } + + return nil +} + +// If there was a failed ack from an outbound transfer of one of the autopilot actions, +// we'll need to check if there was a fallback address. If one was stored, bank send +// to that fallback address +// If the ack was successful, we should delete the address (if it exists) +func (k Keeper) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Packet, acknowledgement []byte) error { + // Retrieve the fallback address for the given packet + // We use the packet source channel here since this will correspond with the channel on Stride + channelId := packet.GetSourceChannel() + sequence := packet.Sequence + fallbackAddress, fallbackAddressFound := k.GetTransferFallbackAddress(ctx, channelId, sequence) + + // If there was no fallback address, there's nothing else to do + if !fallbackAddressFound { + return nil + } + + // Check whether the ack was successful or was an ack error + success, err := k.CheckAcknowledgementStatus(ctx, acknowledgement) + if err != nil { + return err + } + + // If successful, remove the fallback address + if success { + k.RemoveTransferFallbackAddress(ctx, channelId, sequence) + return nil + } + + // If the ack was an error, we'll need to bank send to the fallback address + return k.SendToFallbackAddress(ctx, packet, fallbackAddress) +} + +// If there's a timed out packet, we'll infinitely retry the transfer +func (k Keeper) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet) error { + // Retrieve the fallback address from the original packet + // We use the packet source channel here since this will correspond with the channel on Stride + channelId := packet.GetSourceChannel() + originalSequence := packet.Sequence + fallbackAddress, fallbackAddressFound := k.GetTransferFallbackAddress(ctx, channelId, originalSequence) + + // If there was no fallback address, this packet was not from an autopilot action and there's no need to retry + if !fallbackAddressFound { + return nil + } + + // If this was from an autopilot action, unmarshal the transfer metadata to get the original transfer info + var transferMetadata transfertypes.FungibleTokenPacketData + if err := transfertypes.ModuleCdc.UnmarshalJSON(packet.GetData(), &transferMetadata); err != nil { + return err + } + + // Build the token from the transfer metadata + token, err := k.BuildCoinFromTransferMetadata(transferMetadata) + if err != nil { + return err + } + + // Submit the transfer again with a new timeout + timeoutTimestamp := uint64(ctx.BlockTime().UnixNano()) + transfertypes.DefaultRelativePacketTimeoutTimestamp + msgTransfer := transfertypes.MsgTransfer{ + SourcePort: transfertypes.PortID, + SourceChannel: packet.SourceChannel, + Token: token, + Sender: transferMetadata.Sender, + Receiver: transferMetadata.Receiver, + TimeoutTimestamp: timeoutTimestamp, + Memo: transferMetadata.Memo, + } + retryResponse, err := k.transferKeeper.Transfer(ctx, &msgTransfer) + if err != nil { + return err + } + + // Update the fallback address to use the new sequence number + updatedSequence := retryResponse.Sequence + k.RemoveTransferFallbackAddress(ctx, channelId, originalSequence) + k.SetTransferFallbackAddress(ctx, channelId, updatedSequence, fallbackAddress) + + return nil +} diff --git a/x/autopilot/keeper/liquidstake.go b/x/autopilot/keeper/liquidstake.go index 36d354eee..6f0d23d15 100644 --- a/x/autopilot/keeper/liquidstake.go +++ b/x/autopilot/keeper/liquidstake.go @@ -124,7 +124,8 @@ func (k Keeper) IBCTransferStToken( } // First we need to bank send to the hashed address - originalReceiver, err := sdk.AccAddressFromBech32(transferMetadata.Receiver) + originalReceiverAddress := transferMetadata.Receiver + originalReceiver, err := sdk.AccAddressFromBech32(originalReceiverAddress) if err != nil { return err } @@ -149,7 +150,13 @@ func (k Keeper) IBCTransferStToken( TimeoutTimestamp: timeoutTimestamp, Memo: "autopilot-liquid-stake-and-forward", } - _, err = k.transferKeeper.Transfer(sdk.WrapSDKContext(ctx), transferMsg) + transferResponse, err := k.transferKeeper.Transfer(sdk.WrapSDKContext(ctx), transferMsg) + if err != nil { + return err + } + + // Store the original receiver as the fallback address in case the transfer fails + k.SetTransferFallbackAddress(ctx, channelId, transferResponse.Sequence, originalReceiverAddress) return err } diff --git a/x/autopilot/module_ibc.go b/x/autopilot/module_ibc.go index 031ae0555..f103e15b4 100644 --- a/x/autopilot/module_ibc.go +++ b/x/autopilot/module_ibc.go @@ -237,8 +237,15 @@ func (im IBCModule) OnAcknowledgementPacket( acknowledgement []byte, relayer sdk.AccAddress, ) error { - im.keeper.Logger(ctx).Info(fmt.Sprintf("[IBC-TRANSFER] OnAcknowledgementPacket %v", packet)) - return im.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer) + im.keeper.Logger(ctx).Info(fmt.Sprintf("OnAcknowledgementPacket (Autopilot): Packet %v, Acknowledgement %v", packet, acknowledgement)) + // First pass the packet down the stack so that, in the event of an ack failure, + // the tokens are refunded to the original sender + if err := im.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer); err != nil { + return err + } + // Then process the autopilot-specific callback + // This will handle bank sending to a fallback address if the original transfer failed + return im.keeper.OnAcknowledgementPacket(ctx, packet, acknowledgement) } // OnTimeoutPacket implements the IBCModule interface @@ -247,8 +254,14 @@ func (im IBCModule) OnTimeoutPacket( packet channeltypes.Packet, relayer sdk.AccAddress, ) error { - im.keeper.Logger(ctx).Error(fmt.Sprintf("[IBC-TRANSFER] OnTimeoutPacket %v", packet)) - return im.app.OnTimeoutPacket(ctx, packet, relayer) + im.keeper.Logger(ctx).Info(fmt.Sprintf("OnTimeoutPacket (Autopilot): Packet %v", packet)) + // First pass the packet down the stack so that the tokens are refunded to the original sender + if err := im.app.OnTimeoutPacket(ctx, packet, relayer); err != nil { + return err + } + // Then process the autopilot-specific callback + // This will handle a retry in the event that there was a timeout during an autopilot action + return im.keeper.OnTimeoutPacket(ctx, packet) } // This is implemented by ICS4 and all middleware that are wrapping base application. From f10257487d49dd54637ce69fc8f79e9df4668030 Mon Sep 17 00:00:00 2001 From: sampocs Date: Thu, 28 Dec 2023 17:36:21 -0600 Subject: [PATCH 18/30] added unit test for helpers --- x/autopilot/keeper/ibc.go | 10 +-- x/autopilot/keeper/ibc_test.go | 128 +++++++++++++++++++++++++++++++++ 2 files changed, 133 insertions(+), 5 deletions(-) create mode 100644 x/autopilot/keeper/ibc_test.go diff --git a/x/autopilot/keeper/ibc.go b/x/autopilot/keeper/ibc.go index 7caeb4c30..42d74201f 100644 --- a/x/autopilot/keeper/ibc.go +++ b/x/autopilot/keeper/ibc.go @@ -46,10 +46,10 @@ func (k Keeper) BuildCoinFromTransferMetadata(transferMetadata transfertypes.Fun } // In the event of an ack error after a outbound transfer, we'll have to bank send to a fallback address -func (k Keeper) SendToFallbackAddress(ctx sdk.Context, packet channeltypes.Packet, fallbackAddress string) error { +func (k Keeper) SendToFallbackAddress(ctx sdk.Context, packetData []byte, fallbackAddress string) error { // First unmarshal the transfer metadata to get the sender/reciever, and token amount/denom var transferMetadata transfertypes.FungibleTokenPacketData - if err := transfertypes.ModuleCdc.UnmarshalJSON(packet.GetData(), &transferMetadata); err != nil { + if err := transfertypes.ModuleCdc.UnmarshalJSON(packetData, &transferMetadata); err != nil { return err } @@ -57,11 +57,11 @@ func (k Keeper) SendToFallbackAddress(ctx sdk.Context, packet channeltypes.Packe sender := transferMetadata.Sender senderAccount, err := sdk.AccAddressFromBech32(sender) if err != nil { - return err + return errorsmod.Wrapf(err, "invalid sender address") } fallbackAccount, err := sdk.AccAddressFromBech32(fallbackAddress) if err != nil { - return err + return errorsmod.Wrapf(err, "invalid fallback address") } // Build the token from the transfer metadata @@ -107,7 +107,7 @@ func (k Keeper) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Pac } // If the ack was an error, we'll need to bank send to the fallback address - return k.SendToFallbackAddress(ctx, packet, fallbackAddress) + return k.SendToFallbackAddress(ctx, packet.GetData(), fallbackAddress) } // If there's a timed out packet, we'll infinitely retry the transfer diff --git a/x/autopilot/keeper/ibc_test.go b/x/autopilot/keeper/ibc_test.go new file mode 100644 index 000000000..06523f705 --- /dev/null +++ b/x/autopilot/keeper/ibc_test.go @@ -0,0 +1,128 @@ +package keeper_test + +import ( + sdkmath "cosmossdk.io/math" + sdk "github.com/cosmos/cosmos-sdk/types" + transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" + channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types" +) + +func (s *KeeperTestSuite) TestCheckAcknowledgementStatus() { + // Test with a successful ack + ackSuccess := transfertypes.ModuleCdc.MustMarshalJSON(&channeltypes.Acknowledgement{ + Response: &channeltypes.Acknowledgement_Result{ + Result: []byte{1}, // just has to be non-empty + }, + }) + success, err := s.App.AutopilotKeeper.CheckAcknowledgementStatus(s.Ctx, ackSuccess) + s.Require().True(success, "ack success should return true") + s.Require().NoError(err) + + // Test with an ack error + // Success should be false, but there should be no error returned + errorString := "some error" + ackFailure := transfertypes.ModuleCdc.MustMarshalJSON(&channeltypes.Acknowledgement{ + Response: &channeltypes.Acknowledgement_Error{ + Error: errorString, + }, + }) + success, err = s.App.AutopilotKeeper.CheckAcknowledgementStatus(s.Ctx, ackFailure) + s.Require().False(success, "ack failure should return false") + s.Require().NoError(err) + + // Test with an ack result that is missing the "result" field + // It should return an error + ackResultError := transfertypes.ModuleCdc.MustMarshalJSON(&channeltypes.Acknowledgement{ + Response: &channeltypes.Acknowledgement_Result{ + Result: []byte{}, // empty result throws an error + }, + }) + _, err = s.App.AutopilotKeeper.CheckAcknowledgementStatus(s.Ctx, ackResultError) + s.Require().ErrorContains(err, "acknowledgement result cannot be empty") + + // Test with invalid ack data that can't be unmarshaled + randomBytes := []byte{1, 2, 3} + _, err = s.App.AutopilotKeeper.CheckAcknowledgementStatus(s.Ctx, randomBytes) + s.Require().ErrorContains(err, "cannot unmarshal ICS-20 transfer packet acknowledgement") +} + +func (s *KeeperTestSuite) TestBuildCoinFromTransferMetadata() { + denom := "denom" + amountString := "10000" + amountInt := sdk.NewInt(10000) + + // Test with valid packet data + expectedToken := sdk.NewCoin(denom, amountInt) + transferMetadata := transfertypes.FungibleTokenPacketData{ + Denom: denom, + Amount: amountString, + } + actualToken, err := s.App.AutopilotKeeper.BuildCoinFromTransferMetadata(transferMetadata) + s.Require().NoError(err) + s.Require().Equal(expectedToken, actualToken, "token") + + // Test with invalid packet data + invalidMetadata := transfertypes.FungibleTokenPacketData{ + Denom: denom, + Amount: "", + } + _, err = s.App.AutopilotKeeper.BuildCoinFromTransferMetadata(invalidMetadata) + s.Require().ErrorContains(err, "unable to parse amount from transfer packet") +} + +func (s *KeeperTestSuite) TestSendToFallbackAddress() { + senderAccount := s.TestAccs[0] + fallbackAccount := s.TestAccs[1] + + denom := "denom" + amountInt := sdk.NewInt(10000) + amountString := "10000" + + // Fund the sender + zeroCoin := sdk.NewCoin(denom, sdkmath.ZeroInt()) + balanceCoin := sdk.NewCoin(denom, amountInt) + s.FundAccount(senderAccount, balanceCoin) + + // Send to the fallback address with a valid input + packetDataBz := transfertypes.ModuleCdc.MustMarshalJSON(&transfertypes.FungibleTokenPacketData{ + Denom: denom, + Amount: amountString, + Sender: senderAccount.String(), + }) + err := s.App.AutopilotKeeper.SendToFallbackAddress(s.Ctx, packetDataBz, fallbackAccount.String()) + s.Require().NoError(err, "no error expected when sending to fallback address") + + // Check that the funds were transferred + senderBalance := s.App.BankKeeper.GetBalance(s.Ctx, senderAccount, denom) + s.CompareCoins(zeroCoin, senderBalance, "sender should have lost tokens") + + fallbackBalance := s.App.BankKeeper.GetBalance(s.Ctx, fallbackAccount, denom) + s.CompareCoins(balanceCoin, fallbackBalance, "fallback should have gained tokens") + + // Test with an invalid sender address - it should error + invalidPacketDataBz := transfertypes.ModuleCdc.MustMarshalJSON(&transfertypes.FungibleTokenPacketData{ + Denom: denom, + Amount: amountString, + Sender: "invalid_sender", + }) + err = s.App.AutopilotKeeper.SendToFallbackAddress(s.Ctx, invalidPacketDataBz, fallbackAccount.String()) + s.Require().ErrorContains(err, "invalid sender address") + + // Test with an invalid fallback address - it should error + err = s.App.AutopilotKeeper.SendToFallbackAddress(s.Ctx, packetDataBz, "invalid_fallback") + s.Require().ErrorContains(err, "invalid fallback address") + + // Test with an invalid amount - it should error + invalidPacketDataBz = transfertypes.ModuleCdc.MustMarshalJSON(&transfertypes.FungibleTokenPacketData{ + Denom: denom, + Amount: "", + Sender: senderAccount.String(), + }) + err = s.App.AutopilotKeeper.SendToFallbackAddress(s.Ctx, invalidPacketDataBz, fallbackAccount.String()) + s.Require().ErrorContains(err, "unable to parse amount") + + // Finally, try to call the send function again with a valid input, + // it should fail since the sender now has an insufficient balance + err = s.App.AutopilotKeeper.SendToFallbackAddress(s.Ctx, packetDataBz, fallbackAccount.String()) + s.Require().ErrorContains(err, "insufficient funds") +} From 619c48075454fcedb412ff9252ac37554e0b4453 Mon Sep 17 00:00:00 2001 From: sampocs Date: Thu, 28 Dec 2023 18:27:48 -0600 Subject: [PATCH 19/30] added unit tests for on ack packet --- x/autopilot/keeper/ibc.go | 6 +- x/autopilot/keeper/ibc_test.go | 149 +++++++++++++++++++++++++++++++-- 2 files changed, 144 insertions(+), 11 deletions(-) diff --git a/x/autopilot/keeper/ibc.go b/x/autopilot/keeper/ibc.go index 42d74201f..749953afc 100644 --- a/x/autopilot/keeper/ibc.go +++ b/x/autopilot/keeper/ibc.go @@ -94,15 +94,17 @@ func (k Keeper) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Pac return nil } + // Remove the fallback address since the packet is no longer pending + k.RemoveTransferFallbackAddress(ctx, channelId, sequence) + // Check whether the ack was successful or was an ack error success, err := k.CheckAcknowledgementStatus(ctx, acknowledgement) if err != nil { return err } - // If successful, remove the fallback address + // If successful, no additional action is necessary if success { - k.RemoveTransferFallbackAddress(ctx, channelId, sequence) return nil } diff --git a/x/autopilot/keeper/ibc_test.go b/x/autopilot/keeper/ibc_test.go index 06523f705..d5fc40286 100644 --- a/x/autopilot/keeper/ibc_test.go +++ b/x/autopilot/keeper/ibc_test.go @@ -7,6 +7,15 @@ import ( channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types" ) +type OnAckPacketTestCase struct { + ChannelId string + Sequence uint64 + Token sdk.Coin + Packet channeltypes.Packet + SenderAccount sdk.AccAddress + FallbackAccount sdk.AccAddress +} + func (s *KeeperTestSuite) TestCheckAcknowledgementStatus() { // Test with a successful ack ackSuccess := transfertypes.ModuleCdc.MustMarshalJSON(&channeltypes.Acknowledgement{ @@ -48,14 +57,13 @@ func (s *KeeperTestSuite) TestCheckAcknowledgementStatus() { func (s *KeeperTestSuite) TestBuildCoinFromTransferMetadata() { denom := "denom" - amountString := "10000" - amountInt := sdk.NewInt(10000) + amount := sdk.NewInt(10000) // Test with valid packet data - expectedToken := sdk.NewCoin(denom, amountInt) + expectedToken := sdk.NewCoin(denom, amount) transferMetadata := transfertypes.FungibleTokenPacketData{ Denom: denom, - Amount: amountString, + Amount: amount.String(), } actualToken, err := s.App.AutopilotKeeper.BuildCoinFromTransferMetadata(transferMetadata) s.Require().NoError(err) @@ -75,18 +83,17 @@ func (s *KeeperTestSuite) TestSendToFallbackAddress() { fallbackAccount := s.TestAccs[1] denom := "denom" - amountInt := sdk.NewInt(10000) - amountString := "10000" + amount := sdk.NewInt(10000) // Fund the sender zeroCoin := sdk.NewCoin(denom, sdkmath.ZeroInt()) - balanceCoin := sdk.NewCoin(denom, amountInt) + balanceCoin := sdk.NewCoin(denom, amount) s.FundAccount(senderAccount, balanceCoin) // Send to the fallback address with a valid input packetDataBz := transfertypes.ModuleCdc.MustMarshalJSON(&transfertypes.FungibleTokenPacketData{ Denom: denom, - Amount: amountString, + Amount: amount.String(), Sender: senderAccount.String(), }) err := s.App.AutopilotKeeper.SendToFallbackAddress(s.Ctx, packetDataBz, fallbackAccount.String()) @@ -102,7 +109,7 @@ func (s *KeeperTestSuite) TestSendToFallbackAddress() { // Test with an invalid sender address - it should error invalidPacketDataBz := transfertypes.ModuleCdc.MustMarshalJSON(&transfertypes.FungibleTokenPacketData{ Denom: denom, - Amount: amountString, + Amount: amount.String(), Sender: "invalid_sender", }) err = s.App.AutopilotKeeper.SendToFallbackAddress(s.Ctx, invalidPacketDataBz, fallbackAccount.String()) @@ -126,3 +133,127 @@ func (s *KeeperTestSuite) TestSendToFallbackAddress() { err = s.App.AutopilotKeeper.SendToFallbackAddress(s.Ctx, packetDataBz, fallbackAccount.String()) s.Require().ErrorContains(err, "insufficient funds") } + +func (s *KeeperTestSuite) SetupTestOnAcknowledgementPacket() OnAckPacketTestCase { + senderAccount := s.TestAccs[0] + fallbackAccount := s.TestAccs[1] + + sequence := uint64(1) + channelId := "channel-0" + denom := "denom" + amount := sdk.NewInt(10000) + token := sdk.NewCoin(denom, amount) + + // Set a fallback addresses + s.App.AutopilotKeeper.SetTransferFallbackAddress(s.Ctx, channelId, sequence, fallbackAccount.String()) + + // Fund the sender account + s.FundAccount(senderAccount, token) + + // Build the IBC packet + transferMetadata := transfertypes.FungibleTokenPacketData{ + Denom: "denom", + Amount: amount.String(), + Sender: senderAccount.String(), + } + packet := channeltypes.Packet{ + Sequence: sequence, + SourceChannel: channelId, + Data: transfertypes.ModuleCdc.MustMarshalJSON(&transferMetadata), + } + + return OnAckPacketTestCase{ + ChannelId: channelId, + Sequence: sequence, + Token: token, + Packet: packet, + SenderAccount: senderAccount, + FallbackAccount: fallbackAccount, + } +} + +func (s *KeeperTestSuite) TestOnAcknowledgementPacket_AckSuccess() { + tc := s.SetupTestOnAcknowledgementPacket() + + // Build a successful ack + ackSuccess := transfertypes.ModuleCdc.MustMarshalJSON(&channeltypes.Acknowledgement{ + Response: &channeltypes.Acknowledgement_Result{ + Result: []byte{1}, // just has to be non-empty + }, + }) + + // Call OnAckPacket with the successful ack + err := s.App.AutopilotKeeper.OnAcknowledgementPacket(s.Ctx, tc.Packet, ackSuccess) + s.Require().NoError(err, "no error expected during OnAckPacket") + + // Confirm the fallback address was removed + _, found := s.App.AutopilotKeeper.GetTransferFallbackAddress(s.Ctx, tc.ChannelId, tc.Sequence) + s.Require().False(found, "fallback address should have been removed") + + // Confirm the fallback address has not received any coins + zeroCoin := sdk.NewCoin(tc.Token.Denom, sdk.ZeroInt()) + fallbackBalance := s.App.BankKeeper.GetBalance(s.Ctx, tc.FallbackAccount, tc.Token.Denom) + s.CompareCoins(zeroCoin, fallbackBalance, "fallback account should not have received funds") +} + +func (s *KeeperTestSuite) TestOnAcknowledgementPacket_AckFailure() { + tc := s.SetupTestOnAcknowledgementPacket() + + // Build an error ack + ackFailure := transfertypes.ModuleCdc.MustMarshalJSON(&channeltypes.Acknowledgement{ + Response: &channeltypes.Acknowledgement_Error{}, + }) + + // Call OnAckPacket with the successful ack + err := s.App.AutopilotKeeper.OnAcknowledgementPacket(s.Ctx, tc.Packet, ackFailure) + s.Require().NoError(err, "no error expected during OnAckPacket") + + // Confirm tokens were sent to the fallback address + zeroCoin := sdk.NewCoin(tc.Token.Denom, sdk.ZeroInt()) + senderBalance := s.App.BankKeeper.GetBalance(s.Ctx, tc.SenderAccount, tc.Token.Denom) + fallbackBalance := s.App.BankKeeper.GetBalance(s.Ctx, tc.FallbackAccount, tc.Token.Denom) + s.CompareCoins(zeroCoin, senderBalance, "sender account should have lost funds") + s.CompareCoins(tc.Token, fallbackBalance, "fallback account should have received funds") + + // Confirm the fallback address was removed + _, found := s.App.AutopilotKeeper.GetTransferFallbackAddress(s.Ctx, tc.ChannelId, tc.Sequence) + s.Require().False(found, "fallback address should have been removed") +} + +func (s *KeeperTestSuite) TestOnAcknowledgementPacket_InvalidAck() { + tc := s.SetupTestOnAcknowledgementPacket() + + // Build an invalid ack to force an error + invalidAck := transfertypes.ModuleCdc.MustMarshalJSON(&channeltypes.Acknowledgement{ + Response: &channeltypes.Acknowledgement_Result{ + Result: []byte{}, // empty result causes an error + }, + }) + + // Call OnAckPacket with the invalid ack + err := s.App.AutopilotKeeper.OnAcknowledgementPacket(s.Ctx, tc.Packet, invalidAck) + s.Require().ErrorContains(err, "invalid acknowledgement") +} + +func (s *KeeperTestSuite) TestOnAcknowledgementPacket_NoOp() { + tc := s.SetupTestOnAcknowledgementPacket() + + // Remove the fallback address so that there is no action necessary in the callback + s.App.AutopilotKeeper.RemoveTransferFallbackAddress(s.Ctx, tc.ChannelId, tc.Sequence) + + // Call OnAckPacket and confirm there was no error + // The ack argument here doesn't matter cause the no-op check is upstream + err := s.App.AutopilotKeeper.OnAcknowledgementPacket(s.Ctx, tc.Packet, []byte{}) + s.Require().NoError(err, "no error expected during on ack packet") + + // Check that no funds were moved + zeroCoin := sdk.NewCoin(tc.Token.Denom, sdk.ZeroInt()) + senderBalance := s.App.BankKeeper.GetBalance(s.Ctx, tc.SenderAccount, tc.Token.Denom) + fallbackBalance := s.App.BankKeeper.GetBalance(s.Ctx, tc.FallbackAccount, tc.Token.Denom) + s.CompareCoins(tc.Token, senderBalance, "sender account should have lost funds") + s.CompareCoins(zeroCoin, fallbackBalance, "fallback account should have received funds") +} + +func (s *KeeperTestSuite) TestOnTimeoutPacket() { + +} From baaeb507cc25f356430d90b45ed4fbefb1543f97 Mon Sep 17 00:00:00 2001 From: sampocs Date: Thu, 28 Dec 2023 19:11:42 -0600 Subject: [PATCH 20/30] added unit tests for full callbacks --- x/autopilot/keeper/ibc.go | 12 +-- x/autopilot/keeper/ibc_test.go | 177 +++++++++++++++++++++++++++++---- 2 files changed, 164 insertions(+), 25 deletions(-) diff --git a/x/autopilot/keeper/ibc.go b/x/autopilot/keeper/ibc.go index 749953afc..52bae57f7 100644 --- a/x/autopilot/keeper/ibc.go +++ b/x/autopilot/keeper/ibc.go @@ -85,7 +85,7 @@ func (k Keeper) SendToFallbackAddress(ctx sdk.Context, packetData []byte, fallba func (k Keeper) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Packet, acknowledgement []byte) error { // Retrieve the fallback address for the given packet // We use the packet source channel here since this will correspond with the channel on Stride - channelId := packet.GetSourceChannel() + channelId := packet.SourceChannel sequence := packet.Sequence fallbackAddress, fallbackAddressFound := k.GetTransferFallbackAddress(ctx, channelId, sequence) @@ -109,14 +109,14 @@ func (k Keeper) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Pac } // If the ack was an error, we'll need to bank send to the fallback address - return k.SendToFallbackAddress(ctx, packet.GetData(), fallbackAddress) + return k.SendToFallbackAddress(ctx, packet.Data, fallbackAddress) } // If there's a timed out packet, we'll infinitely retry the transfer func (k Keeper) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet) error { // Retrieve the fallback address from the original packet // We use the packet source channel here since this will correspond with the channel on Stride - channelId := packet.GetSourceChannel() + channelId := packet.SourceChannel originalSequence := packet.Sequence fallbackAddress, fallbackAddressFound := k.GetTransferFallbackAddress(ctx, channelId, originalSequence) @@ -127,8 +127,8 @@ func (k Keeper) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet) err // If this was from an autopilot action, unmarshal the transfer metadata to get the original transfer info var transferMetadata transfertypes.FungibleTokenPacketData - if err := transfertypes.ModuleCdc.UnmarshalJSON(packet.GetData(), &transferMetadata); err != nil { - return err + if err := transfertypes.ModuleCdc.UnmarshalJSON(packet.Data, &transferMetadata); err != nil { + return errorsmod.Wrapf(err, "unable to unmarshal ICS-20 packet data") } // Build the token from the transfer metadata @@ -150,7 +150,7 @@ func (k Keeper) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet) err } retryResponse, err := k.transferKeeper.Transfer(ctx, &msgTransfer) if err != nil { - return err + return errorsmod.Wrapf(err, "unable to submit transfer retry of %+v", msgTransfer) } // Update the fallback address to use the new sequence number diff --git a/x/autopilot/keeper/ibc_test.go b/x/autopilot/keeper/ibc_test.go index d5fc40286..049508fc3 100644 --- a/x/autopilot/keeper/ibc_test.go +++ b/x/autopilot/keeper/ibc_test.go @@ -5,17 +5,23 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types" + ibctesting "github.com/cosmos/ibc-go/v7/testing" ) -type OnAckPacketTestCase struct { - ChannelId string - Sequence uint64 - Token sdk.Coin - Packet channeltypes.Packet - SenderAccount sdk.AccAddress - FallbackAccount sdk.AccAddress +type PacketCallbackTestCase struct { + ChannelId string + OriginalSequence uint64 + RetrySequence uint64 + Token sdk.Coin + Packet channeltypes.Packet + SenderAccount sdk.AccAddress + FallbackAccount sdk.AccAddress } +// -------------------------------------------------------------- +// IBC Callback Helpers +// -------------------------------------------------------------- + func (s *KeeperTestSuite) TestCheckAcknowledgementStatus() { // Test with a successful ack ackSuccess := transfertypes.ModuleCdc.MustMarshalJSON(&channeltypes.Acknowledgement{ @@ -134,7 +140,11 @@ func (s *KeeperTestSuite) TestSendToFallbackAddress() { s.Require().ErrorContains(err, "insufficient funds") } -func (s *KeeperTestSuite) SetupTestOnAcknowledgementPacket() OnAckPacketTestCase { +// -------------------------------------------------------------- +// OnAcknowledgementPacket +// -------------------------------------------------------------- + +func (s *KeeperTestSuite) SetupTestOnAcknowledgementPacket() PacketCallbackTestCase { senderAccount := s.TestAccs[0] fallbackAccount := s.TestAccs[1] @@ -162,13 +172,13 @@ func (s *KeeperTestSuite) SetupTestOnAcknowledgementPacket() OnAckPacketTestCase Data: transfertypes.ModuleCdc.MustMarshalJSON(&transferMetadata), } - return OnAckPacketTestCase{ - ChannelId: channelId, - Sequence: sequence, - Token: token, - Packet: packet, - SenderAccount: senderAccount, - FallbackAccount: fallbackAccount, + return PacketCallbackTestCase{ + ChannelId: channelId, + OriginalSequence: sequence, + Token: token, + Packet: packet, + SenderAccount: senderAccount, + FallbackAccount: fallbackAccount, } } @@ -187,7 +197,7 @@ func (s *KeeperTestSuite) TestOnAcknowledgementPacket_AckSuccess() { s.Require().NoError(err, "no error expected during OnAckPacket") // Confirm the fallback address was removed - _, found := s.App.AutopilotKeeper.GetTransferFallbackAddress(s.Ctx, tc.ChannelId, tc.Sequence) + _, found := s.App.AutopilotKeeper.GetTransferFallbackAddress(s.Ctx, tc.ChannelId, tc.OriginalSequence) s.Require().False(found, "fallback address should have been removed") // Confirm the fallback address has not received any coins @@ -216,7 +226,7 @@ func (s *KeeperTestSuite) TestOnAcknowledgementPacket_AckFailure() { s.CompareCoins(tc.Token, fallbackBalance, "fallback account should have received funds") // Confirm the fallback address was removed - _, found := s.App.AutopilotKeeper.GetTransferFallbackAddress(s.Ctx, tc.ChannelId, tc.Sequence) + _, found := s.App.AutopilotKeeper.GetTransferFallbackAddress(s.Ctx, tc.ChannelId, tc.OriginalSequence) s.Require().False(found, "fallback address should have been removed") } @@ -239,7 +249,7 @@ func (s *KeeperTestSuite) TestOnAcknowledgementPacket_NoOp() { tc := s.SetupTestOnAcknowledgementPacket() // Remove the fallback address so that there is no action necessary in the callback - s.App.AutopilotKeeper.RemoveTransferFallbackAddress(s.Ctx, tc.ChannelId, tc.Sequence) + s.App.AutopilotKeeper.RemoveTransferFallbackAddress(s.Ctx, tc.ChannelId, tc.OriginalSequence) // Call OnAckPacket and confirm there was no error // The ack argument here doesn't matter cause the no-op check is upstream @@ -254,6 +264,135 @@ func (s *KeeperTestSuite) TestOnAcknowledgementPacket_NoOp() { s.CompareCoins(zeroCoin, fallbackBalance, "fallback account should have received funds") } -func (s *KeeperTestSuite) TestOnTimeoutPacket() { +// -------------------------------------------------------------- +// OnTimeoutPacket +// -------------------------------------------------------------- + +func (s *KeeperTestSuite) SetupTestOnTimeoutPacket() PacketCallbackTestCase { + senderAccount := s.TestAccs[0] + fallbackAccount := s.TestAccs[1] + receiverAccount := s.TestAccs[2] + + chainId := "chain-0" + denom := "denom" + amount := sdk.NewInt(10000) + transferToken := sdk.NewCoin(denom, amount) + + // Create transfer channel so the retry can be submitted + s.CreateTransferChannel(chainId) + channelId := ibctesting.FirstChannelID + + // Determine the next sequence number along that channel (which will be the seqence number for the retry) + expectedRetrySequence, ok := s.App.IBCKeeper.ChannelKeeper.GetNextSequenceSend(s.Ctx, transfertypes.PortID, channelId) + s.Require().True(ok) + + // Store a fallback address on the previous sequence number (from the original timed out transfer) + originalSequence := expectedRetrySequence - 1 + s.App.AutopilotKeeper.SetTransferFallbackAddress(s.Ctx, channelId, originalSequence, fallbackAccount.String()) + + // Fund the sender address so they have sufficient tokens to re-submit a transfer + s.FundAccount(senderAccount, transferToken) + + // Build the packet data + transferMetadata := transfertypes.FungibleTokenPacketData{ + Sender: senderAccount.String(), + Receiver: receiverAccount.String(), + Denom: denom, + Amount: amount.String(), + } + packetDataBz := transfertypes.ModuleCdc.MustMarshalJSON(&transferMetadata) + packet := channeltypes.Packet{ + Sequence: originalSequence, + SourceChannel: channelId, + Data: packetDataBz, + } + + return PacketCallbackTestCase{ + ChannelId: channelId, + OriginalSequence: originalSequence, + RetrySequence: expectedRetrySequence, + Token: transferToken, + Packet: packet, + SenderAccount: senderAccount, + } +} + +func (s *KeeperTestSuite) TestOnTimeoutPacket_SuccessfulRetry() { + tc := s.SetupTestOnTimeoutPacket() + + // Call OnTimeoutPacket + err := s.App.AutopilotKeeper.OnTimeoutPacket(s.Ctx, tc.Packet) + s.Require().NoError(err, "no error expected when calling OnTimeoutPacket") + + // Check that the sender's funds were escrowed (from the retry) + zeroCoin := sdk.NewCoin(tc.Token.Denom, sdkmath.ZeroInt()) + senderBalance := s.App.BankKeeper.GetBalance(s.Ctx, tc.SenderAccount, tc.Token.Denom) + s.CompareCoins(zeroCoin, senderBalance, "the sender should have no more funds after the retry") + + escrowAddress := transfertypes.GetEscrowAddress(transfertypes.PortID, tc.ChannelId) + escrowBalance := s.App.BankKeeper.GetBalance(s.Ctx, escrowAddress, tc.Token.Denom) + s.CompareCoins(tc.Token, escrowBalance, "escrow balance should have increased") + + // Check that the fallback address was moved to a new sequence number + _, found := s.App.AutopilotKeeper.GetTransferFallbackAddress(s.Ctx, tc.ChannelId, tc.OriginalSequence) + s.Require().False(found, "fallback address should no longer be on the old sequence number") + + _, found = s.App.AutopilotKeeper.GetTransferFallbackAddress(s.Ctx, tc.ChannelId, tc.RetrySequence) + s.Require().True(found, "fallback address should now use the new sequence number") +} + +func (s *KeeperTestSuite) TestOnTimeoutPacket_NoOp() { + tc := s.SetupTestOnTimeoutPacket() + + // Remove the fallback address + s.App.AutopilotKeeper.RemoveTransferFallbackAddress(s.Ctx, tc.ChannelId, tc.OriginalSequence) + + // Call OnTimeoutPacket - this should be a no-op since there's no fallback data + err := s.App.AutopilotKeeper.OnTimeoutPacket(s.Ctx, tc.Packet) + s.Require().NoError(err, "no error expected when calling OnTimeoutPacket") + + // Confirm the sender still has his original tokens (since the retry was not submitted) + senderBalance := s.App.BankKeeper.GetBalance(s.Ctx, tc.SenderAccount, tc.Token.Denom) + s.CompareCoins(tc.Token, senderBalance, "the sender balance should not have changed") +} + +func (s *KeeperTestSuite) TestOnTimeoutPacket_InvalidPacketData() { + tc := s.SetupTestOnTimeoutPacket() + + // Modify the packet data so that it will fail unmarshalling + tc.Packet.Data = []byte{1, 2, 3} + + err := s.App.AutopilotKeeper.OnTimeoutPacket(s.Ctx, tc.Packet) + s.Require().ErrorContains(err, "unable to unmarshal ICS-20 packet data") +} + +func (s *KeeperTestSuite) TestOnTimeoutPacket_InvalidToken() { + tc := s.SetupTestOnTimeoutPacket() + + // Modify the token in the packet data so that the amount is empty + transferMetadata := transfertypes.FungibleTokenPacketData{ + Denom: tc.Token.Denom, + Amount: "", + } + tc.Packet.Data = transfertypes.ModuleCdc.MustMarshalJSON(&transferMetadata) + + // Call OnTimeoutPacket - it should fail + err := s.App.AutopilotKeeper.OnTimeoutPacket(s.Ctx, tc.Packet) + s.Require().ErrorContains(err, "unable to parse amount from transfer packet") +} + +func (s *KeeperTestSuite) TestOnTimeoutPacket_FailedToRetry() { + tc := s.SetupTestOnTimeoutPacket() + + // Modify the packet data so that the sender is an invalid address + transferMetadata := transfertypes.FungibleTokenPacketData{ + Sender: "invalid_address", + Denom: tc.Token.Denom, + Amount: tc.Token.Amount.String(), + } + tc.Packet.Data = transfertypes.ModuleCdc.MustMarshalJSON(&transferMetadata) + // Call OnTimeoutPacket - it should be unable to submit the retry + err := s.App.AutopilotKeeper.OnTimeoutPacket(s.Ctx, tc.Packet) + s.Require().ErrorContains(err, "unable to submit transfer retry") } From c8288dcf9e4b71b9cf5670e16bf848d29cc6f539 Mon Sep 17 00:00:00 2001 From: sampocs Date: Fri, 29 Dec 2023 16:46:17 -0600 Subject: [PATCH 21/30] info -> error log --- x/autopilot/module_ibc.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/autopilot/module_ibc.go b/x/autopilot/module_ibc.go index f103e15b4..159ea582b 100644 --- a/x/autopilot/module_ibc.go +++ b/x/autopilot/module_ibc.go @@ -254,7 +254,7 @@ func (im IBCModule) OnTimeoutPacket( packet channeltypes.Packet, relayer sdk.AccAddress, ) error { - im.keeper.Logger(ctx).Info(fmt.Sprintf("OnTimeoutPacket (Autopilot): Packet %v", packet)) + im.keeper.Logger(ctx).Error(fmt.Sprintf("OnTimeoutPacket (Autopilot): Packet %v", packet)) // First pass the packet down the stack so that the tokens are refunded to the original sender if err := im.app.OnTimeoutPacket(ctx, packet, relayer); err != nil { return err From 299f5858b3ea15cb6ba1b12dde4da5a6eabb4d73 Mon Sep 17 00:00:00 2001 From: sampocs Date: Wed, 3 Jan 2024 19:34:05 -0600 Subject: [PATCH 22/30] autopilot liquid stake and forward unit tests (#1041) --- app/apptesting/test_helpers.go | 3 +- x/autopilot/keeper/keeper_test.go | 11 + x/autopilot/keeper/liquidstake.go | 8 +- x/autopilot/keeper/liquidstake_test.go | 669 +++++++++++++++++-------- x/stakeibc/keeper/host_zone.go | 2 +- 5 files changed, 481 insertions(+), 212 deletions(-) diff --git a/app/apptesting/test_helpers.go b/app/apptesting/test_helpers.go index 6ad11e6f3..86613a49b 100644 --- a/app/apptesting/test_helpers.go +++ b/app/apptesting/test_helpers.go @@ -16,7 +16,6 @@ import ( upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" "github.com/cosmos/gogoproto/proto" icatypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/types" - ibctransfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types" connectiontypes "github.com/cosmos/ibc-go/v7/modules/core/03-connection/types" @@ -421,7 +420,7 @@ func (s *AppTestHelper) CreateAndStoreIBCDenom(baseDenom string) (ibcDenom strin } func (s *AppTestHelper) MarshalledICS20PacketData() sdk.AccAddress { - data := ibctransfertypes.FungibleTokenPacketData{} + data := transfertypes.FungibleTokenPacketData{} return data.GetBytes() } diff --git a/x/autopilot/keeper/keeper_test.go b/x/autopilot/keeper/keeper_test.go index 9adfbd57f..8f0445a60 100644 --- a/x/autopilot/keeper/keeper_test.go +++ b/x/autopilot/keeper/keeper_test.go @@ -9,6 +9,17 @@ import ( "github.com/Stride-Labs/stride/v16/x/autopilot/types" ) +const ( + HostChainId = "chain-0" + HostBechPrefix = "cosmos" + HostAddress = "cosmos16plylpsgxechajltx9yeseqexzdzut9g8vla4k" + HostDenom = "uatom" + + Atom = "uatom" + Strd = "ustrd" + Osmo = "uosmo" +) + type KeeperTestSuite struct { apptesting.AppTestHelper QueryClient types.QueryClient diff --git a/x/autopilot/keeper/liquidstake.go b/x/autopilot/keeper/liquidstake.go index 6f0d23d15..55fcd7f9c 100644 --- a/x/autopilot/keeper/liquidstake.go +++ b/x/autopilot/keeper/liquidstake.go @@ -36,7 +36,7 @@ func (k Keeper) TryLiquidStaking( // 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") + return fmt.Errorf("native token is not supported for liquid staking (%s)", transferMetadata.Denom) } // Note: the denom in the packet is the base denom e.g. uatom - not ibc/xxx @@ -46,7 +46,7 @@ func (k Keeper) TryLiquidStaking( hostZone, err := k.stakeibcKeeper.GetHostZoneFromHostDenom(ctx, transferMetadata.Denom) if err != nil { - return fmt.Errorf("host zone not found for denom (%s)", transferMetadata.Denom) + return err } // Verify the IBC denom of the packet matches the host zone, to confirm the packet @@ -82,7 +82,7 @@ func (k Keeper) RunLiquidStake( msg, ) if err != nil { - return errorsmod.Wrapf(err, err.Error()) + return errorsmod.Wrapf(err, "failed to liquid stake") } // If the IBCReceiver is empty, there is no forwarding step @@ -152,7 +152,7 @@ func (k Keeper) IBCTransferStToken( } transferResponse, err := k.transferKeeper.Transfer(sdk.WrapSDKContext(ctx), transferMsg) if err != nil { - return err + return errorsmod.Wrapf(err, "failed to submit transfer during autopilot liquid stake and forward") } // Store the original receiver as the fallback address in case the transfer fails diff --git a/x/autopilot/keeper/liquidstake_test.go b/x/autopilot/keeper/liquidstake_test.go index 24642d783..7eab137e0 100644 --- a/x/autopilot/keeper/liquidstake_test.go +++ b/x/autopilot/keeper/liquidstake_test.go @@ -2,254 +2,513 @@ package keeper_test import ( "fmt" - "time" - "github.com/cometbft/cometbft/crypto/ed25519" + sdkmath "cosmossdk.io/math" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/ibc-go/v7/modules/apps/transfer" transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" - clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types" channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types" - - recordsmodule "github.com/Stride-Labs/stride/v16/x/records" - - sdk "github.com/cosmos/cosmos-sdk/types" + ibctesting "github.com/cosmos/ibc-go/v7/testing" "github.com/Stride-Labs/stride/v16/x/autopilot" "github.com/Stride-Labs/stride/v16/x/autopilot/types" epochtypes "github.com/Stride-Labs/stride/v16/x/epochs/types" - minttypes "github.com/Stride-Labs/stride/v16/x/mint/types" + recordsmodule "github.com/Stride-Labs/stride/v16/x/records" recordstypes "github.com/Stride-Labs/stride/v16/x/records/types" stakeibctypes "github.com/Stride-Labs/stride/v16/x/stakeibc/types" ) -func getStakeibcPacketMetadata(address, action string) string { +var ( + // Arbitrary channel ID on the non-stride zone + SourceChannelOnHost = "channel-1000" + + // Building a mapping to of base denom to expected denom traces for the transfer packet data + // This is all assuming the packet has been sent to stride (the FungibleTokenPacketData has + // a denom-trace for the Denom field, instead of an IBC hash) + ReceivePacketDenomTraces = map[string]string{ + // For host zone tokens, since stride is the first hop, there's no port/channel in the denom trace path + Atom: Atom, + Osmo: Osmo, + // For strd, the other zone's channel ID is appended to the denom trace + Strd: transfertypes.GetPrefixedDenom(transfertypes.PortID, SourceChannelOnHost, Strd), + } +) + +// Helper function to create the autopilot JSON payload for a liquid stake +func getLiquidStakePacketMetadata(receiver, ibcReceiver, transferChannelId string) string { return fmt.Sprintf(` { "autopilot": { "receiver": "%[1]s", - "stakeibc": { "action": "%[2]s" } + "stakeibc": { "action": "LiquidStake", "ibc_receiver": "%[2]s", "transfer_channel": "%[3]s" } } - }`, address, action) + }`, receiver, ibcReceiver, transferChannelId) } -func (suite *KeeperTestSuite) TestLiquidStakeOnRecvPacket() { - now := time.Now() - - packet := channeltypes.Packet{ - Sequence: 1, - SourcePort: "transfer", - SourceChannel: "channel-0", - DestinationPort: "transfer", - DestinationChannel: "channel-0", - Data: []byte{}, - TimeoutHeight: clienttypes.Height{}, - TimeoutTimestamp: 0, +// Helper function to mock out all the state needed to test autopilot liquid stake +// A transfer channel-0 is created, and the state is mocked out with an atom host zone +// +// Note: The testing framework is limited to one transfer channel per test, which is channel-0. +// If there's an outbound transfer, it must be on channel-0. So when testing a transfer along +// a non-host-zone channel (e.g. a transfer of statom to Osmosis), a different `strideToHostChannelId` +// channel ID must be passed to this function +// +// Returns the ibc denom of the native token +func (s *KeeperTestSuite) SetupAutopilotLiquidStake( + featureEnabled bool, + strideToHostChannelId string, + depositAddress sdk.AccAddress, + liquidStaker sdk.AccAddress, +) (nativeTokenIBCDenom string) { + // Create a transfer channel on channel-0 for the outbound transfer + // Note: We pass a dummy chain ID cause all that matters here is + // that channel-0 exists, it does not have to line up with the host zone + s.CreateTransferChannel("chain-0") + + // Set whether the feature is active + params := s.App.AutopilotKeeper.GetParams(s.Ctx) + params.StakeibcActive = featureEnabled + s.App.AutopilotKeeper.SetParams(s.Ctx, params) + + // Set the epoch tracker to lookup the deposit record + s.App.StakeibcKeeper.SetEpochTracker(s.Ctx, stakeibctypes.EpochTracker{ + EpochIdentifier: epochtypes.STRIDE_EPOCH, + EpochNumber: 1, + }) + + // Set deposit record to store the new liquid stake + s.App.RecordsKeeper.SetDepositRecord(s.Ctx, recordstypes.DepositRecord{ + Id: 1, + DepositEpochNumber: 1, + Amount: sdk.ZeroInt(), + HostZoneId: HostChainId, + Status: recordstypes.DepositRecord_TRANSFER_QUEUE, + }) + + // Set the host zone - this should have the actual IBC denom + prefixedDenom := transfertypes.GetPrefixedDenom(transfertypes.PortID, strideToHostChannelId, HostDenom) + nativeTokenIBCDenom = transfertypes.ParseDenomTrace(prefixedDenom).IBCDenom() + + s.App.StakeibcKeeper.SetHostZone(s.Ctx, stakeibctypes.HostZone{ + ChainId: HostChainId, + HostDenom: HostDenom, + RedemptionRate: sdk.NewDec(1), // used to determine the stAmount + DepositAddress: depositAddress.String(), + IbcDenom: nativeTokenIBCDenom, + TransferChannelId: strideToHostChannelId, + }) + + return nativeTokenIBCDenom +} + +func (s *KeeperTestSuite) CheckLiquidStakeSucceeded( + liquidStakeAmount sdkmath.Int, + liquidStakerAddress sdk.AccAddress, + depositAddress sdk.AccAddress, + nativeTokenIBCDenom string, + expectedForwardChannelId string, +) { + // If there was a forwarding step, the stTokens will end up in the escrow account + // Otherwise, they'll be in the liquid staker's account + stTokenRecipient := liquidStakerAddress + if expectedForwardChannelId != "" { + escrowAddress := transfertypes.GetEscrowAddress(transfertypes.PortID, expectedForwardChannelId) + stTokenRecipient = escrowAddress + } + + // Confirm the liquid staker has lost his native tokens + stakerBalance := s.App.BankKeeper.GetBalance(s.Ctx, liquidStakerAddress, nativeTokenIBCDenom) + s.Require().Zero(stakerBalance.Amount.Int64(), "liquid staker should have lost host tokens") + + // Confirm the deposit address now has the native tokens + depositBalance := s.App.BankKeeper.GetBalance(s.Ctx, depositAddress, nativeTokenIBCDenom) + s.Require().Equal(liquidStakeAmount.Int64(), depositBalance.Amount.Int64(), "deposit address should have gained host tokens") + + // Confirm the stToken's were minted and sent to the recipient + recipientBalance := s.App.BankKeeper.GetBalance(s.Ctx, stTokenRecipient, "st"+HostDenom) + s.Require().Equal(liquidStakeAmount.Int64(), recipientBalance.Amount.Int64(), "st token recipient balance") + + // If there was a forwarding step, confirm the fallback address was stored + // The fallback address in all these tests is the same as the liquid staker + if expectedForwardChannelId != "" { + address, found := s.App.AutopilotKeeper.GetTransferFallbackAddress(s.Ctx, expectedForwardChannelId, 1) + s.Require().True(found, "fallback address should have been found") + s.Require().Equal(liquidStakerAddress.String(), address, "fallback address") } +} - atomHostDenom := "uatom" - prefixedDenom := transfertypes.GetPrefixedDenom(packet.GetDestPort(), packet.GetDestChannel(), atomHostDenom) - atomIbcDenom := transfertypes.ParseDenomTrace(prefixedDenom).IBCDenom() - prefixedDenom2 := transfertypes.GetPrefixedDenom(packet.GetDestPort(), "channel-1000", atomHostDenom) - atomIbcDenom2 := transfertypes.ParseDenomTrace(prefixedDenom2).IBCDenom() +// Tests TryLiquidStake directly - beginning after the inbound autopilot transfer has passed down the stack +func (s *KeeperTestSuite) TestTryLiquidStake() { + liquidStakerOnStride := s.TestAccs[0] + depositAddress := s.TestAccs[1] + forwardRecipientOnHost := HostAddress - strdDenom := "ustrd" - prefixedDenom = transfertypes.GetPrefixedDenom(packet.GetSourcePort(), packet.GetSourceChannel(), strdDenom) - strdIbcDenom := transfertypes.ParseDenomTrace(prefixedDenom).IBCDenom() + stakeAmount := sdk.NewInt(1000000) - addr1 := sdk.AccAddress(ed25519.GenPrivKey().PubKey().Address().Bytes()) testCases := []struct { - forwardingActive bool - recvDenom string - packetData transfertypes.FungibleTokenPacketData - destChannel string - expSuccess bool - expLiquidStake bool + name string + enabled bool + liquidStakeDenom string + liquidStakeAmount string + autopilotMetadata types.StakeibcPacketMetadata + hostZoneChannelID string // defaults to channel-0 if not specified + inboundTransferChannnelId string // defaults to channel-0 if not specified + expectedForwardChannelId string // defaults to empty (no forwarding) + expectedError string }{ - { // params not enabled - forwardingActive: false, - packetData: transfertypes.FungibleTokenPacketData{ - Denom: "uatom", - Amount: "1000000", - Sender: "cosmos16plylpsgxechajltx9yeseqexzdzut9g8vla4k", - Receiver: getStakeibcPacketMetadata(addr1.String(), "LiquidStake"), - Memo: "", - }, - destChannel: "channel-0", - recvDenom: atomIbcDenom, - expSuccess: false, - expLiquidStake: false, + { + // Normal autopilot liquid stake with no transfer + name: "successful liquid stake with atom", + enabled: true, + liquidStakeDenom: Atom, + liquidStakeAmount: stakeAmount.String(), }, - { // strd denom - forwardingActive: true, - packetData: transfertypes.FungibleTokenPacketData{ - Denom: strdIbcDenom, - Amount: "1000000", - Sender: "cosmos16plylpsgxechajltx9yeseqexzdzut9g8vla4k", - Receiver: getStakeibcPacketMetadata(addr1.String(), "LiquidStake"), - Memo: "", + { + // Liquid stake and forward, using the default host channel ID + name: "successful liquid stake and forward atom to the hub", + enabled: true, + liquidStakeDenom: Atom, + liquidStakeAmount: stakeAmount.String(), + autopilotMetadata: types.StakeibcPacketMetadata{ + IbcReceiver: forwardRecipientOnHost, }, - destChannel: "channel-0", - recvDenom: "ustrd", - expSuccess: false, - expLiquidStake: false, + expectedForwardChannelId: ibctesting.FirstChannelID, // default for host zone }, - { // all okay - forwardingActive: true, - packetData: transfertypes.FungibleTokenPacketData{ - Denom: "uatom", - Amount: "1000000", - Sender: "cosmos16plylpsgxechajltx9yeseqexzdzut9g8vla4k", - Receiver: getStakeibcPacketMetadata(addr1.String(), "LiquidStake"), - Memo: "", + { + // Liquid stake and forward, using a custom channel ID + // Host Zone Channel: channel-1, Outbound Transfer Channel: channel-0 + name: "successful liquid stake and forward atom to osmo", + enabled: true, + liquidStakeDenom: Atom, + liquidStakeAmount: stakeAmount.String(), + autopilotMetadata: types.StakeibcPacketMetadata{ + IbcReceiver: forwardRecipientOnHost, + TransferChannel: "channel-0", // custom channel (different than host channel below) }, - destChannel: "channel-0", - recvDenom: atomIbcDenom, - expSuccess: true, - expLiquidStake: true, + inboundTransferChannnelId: "channel-1", + hostZoneChannelID: "channel-1", + expectedForwardChannelId: "channel-0", }, - { // ibc denom uatom from different channel - forwardingActive: true, - packetData: transfertypes.FungibleTokenPacketData{ - Denom: "uatom", - Amount: "1000000", - Sender: "cosmos16plylpsgxechajltx9yeseqexzdzut9g8vla4k", - Receiver: getStakeibcPacketMetadata(addr1.String(), "LiquidStake"), - Memo: "", - }, - destChannel: "channel-1000", - recvDenom: atomIbcDenom2, - expSuccess: false, - expLiquidStake: false, + { + // Error caused by autopilot disabled + name: "autopilot disabled", + enabled: false, + liquidStakeDenom: Atom, + liquidStakeAmount: stakeAmount.String(), + expectedError: "autopilot stakeibc routing is inactive", }, - { // all okay with memo liquidstaking since ibc-go v5.1.0 - forwardingActive: true, - packetData: transfertypes.FungibleTokenPacketData{ - Denom: "uatom", - Amount: "1000000", - Sender: "cosmos16plylpsgxechajltx9yeseqexzdzut9g8vla4k", - Receiver: addr1.String(), - Memo: getStakeibcPacketMetadata(addr1.String(), "LiquidStake"), - }, - destChannel: "channel-0", - recvDenom: atomIbcDenom, - expSuccess: true, - expLiquidStake: true, + { + // Error caused an invalid amount in the packet + name: "invalid token amount", + enabled: true, + liquidStakeDenom: Atom, + liquidStakeAmount: "", + expectedError: "not a parsable amount field", }, - { // all okay with no functional part - forwardingActive: true, - packetData: transfertypes.FungibleTokenPacketData{ - Denom: "uatom", - Amount: "1000000", - Sender: "cosmos16plylpsgxechajltx9yeseqexzdzut9g8vla4k", - Receiver: addr1.String(), - Memo: "", - }, - destChannel: "channel-0", - recvDenom: atomIbcDenom, - expSuccess: true, - expLiquidStake: false, + { + // Error caused by the transfer of a non-native token + // (i.e. a token that originated on stride) + name: "unable to liquid stake native token", + enabled: true, + liquidStakeDenom: Strd, + liquidStakeAmount: stakeAmount.String(), + expectedError: "native token is not supported for liquid staking", }, - { // invalid stride address (receiver) - forwardingActive: true, - packetData: transfertypes.FungibleTokenPacketData{ - Denom: "uatom", - Amount: "1000000", - Sender: "cosmos16plylpsgxechajltx9yeseqexzdzut9g8vla4k", - Receiver: getStakeibcPacketMetadata("invalid_address", "LiquidStake"), - Memo: "", - }, - destChannel: "channel-0", - recvDenom: atomIbcDenom, - expSuccess: false, - expLiquidStake: false, + { + // Error caused by the transfer of non-host zone token + name: "unable to liquid stake non-host zone token", + enabled: true, + liquidStakeDenom: Osmo, + liquidStakeAmount: stakeAmount.String(), + expectedError: "No HostZone for uosmo denom found", + }, + { + // Error caused by a mismatched IBC denom + // Invoked by specifiying a different host zone channel ID + name: "ibc denom does not match host zone", + enabled: true, + liquidStakeDenom: Atom, + liquidStakeAmount: stakeAmount.String(), + hostZoneChannelID: "channel-0", + inboundTransferChannnelId: "channel-1", // Different than host zone + expectedError: "is not equal to host zone ibc denom", }, - { // invalid stride address (memo) - forwardingActive: true, - packetData: transfertypes.FungibleTokenPacketData{ - Denom: "uatom", - Amount: "1000000", - Sender: "cosmos16plylpsgxechajltx9yeseqexzdzut9g8vla4k", - Receiver: addr1.String(), - Memo: getStakeibcPacketMetadata("invalid_address", "LiquidStake"), + { + // Error caused by a failed validate basic before the liquid stake + // Invoked by passing a negative amount + name: "failed liquid stake validate basic", + enabled: true, + liquidStakeDenom: Atom, + liquidStakeAmount: "-10000", + expectedError: "amount liquid staked must be positive and nonzero", + }, + { + // Error caused by a failed liquid stake + // Invoked by trying to liquid stake more tokens than the staker has available + name: "failed to liquid stake", + enabled: true, + liquidStakeDenom: Atom, + liquidStakeAmount: stakeAmount.Add(sdkmath.NewInt(100000)).String(), // greater than balance + expectedError: "failed to liquid stake", + }, + { + // Failed to send transfer during forwarding step + // Invoked by specifying a non-existent channel ID + name: "failed to forward transfer", + enabled: true, + liquidStakeDenom: Atom, + liquidStakeAmount: stakeAmount.String(), + autopilotMetadata: types.StakeibcPacketMetadata{ + IbcReceiver: forwardRecipientOnHost, + TransferChannel: "channel-100", // does not exist }, - destChannel: "channel-0", - recvDenom: atomIbcDenom, - expSuccess: false, - expLiquidStake: false, + expectedError: "failed to submit transfer during autopilot liquid stake and forward", + }, + } + + for _, tc := range testCases { + s.Run(tc.name, func() { + // Fill in the default channel ID's if they weren't specified + if tc.hostZoneChannelID == "" { + tc.hostZoneChannelID = ibctesting.FirstChannelID + } + if tc.inboundTransferChannnelId == "" { + tc.inboundTransferChannnelId = ibctesting.FirstChannelID + } + + transferMetadata := transfertypes.FungibleTokenPacketData{ + Denom: ReceivePacketDenomTraces[tc.liquidStakeDenom], + Amount: tc.liquidStakeAmount, + Receiver: liquidStakerOnStride.String(), + } + packet := channeltypes.Packet{ + SourcePort: transfertypes.PortID, + SourceChannel: SourceChannelOnHost, + DestinationPort: transfertypes.PortID, + DestinationChannel: tc.inboundTransferChannnelId, + } + + s.SetupTest() + nativeTokenIBCDenom := s.SetupAutopilotLiquidStake(tc.enabled, tc.hostZoneChannelID, depositAddress, liquidStakerOnStride) + + // Since this tested function is normally downstream of the inbound IBC transfer, + // we have to fund the staker with ibc/atom before calling this function so + // they can liquid stake + s.FundAccount(liquidStakerOnStride, sdk.NewCoin(nativeTokenIBCDenom, stakeAmount)) + + err := s.App.AutopilotKeeper.TryLiquidStaking(s.Ctx, packet, transferMetadata, tc.autopilotMetadata) + + if tc.expectedError == "" { + s.Require().NoError(err, "%s - no error expected when attempting liquid stake", tc.name) + s.CheckLiquidStakeSucceeded( + stakeAmount, + liquidStakerOnStride, + depositAddress, + nativeTokenIBCDenom, + tc.expectedForwardChannelId, + ) + } else { + s.Require().ErrorContains(err, tc.expectedError, tc.name) + } + }) + } +} + +// Tests the full OnRecvPacket callback, with liquid staking specific test cases +func (s *KeeperTestSuite) TestOnRecvPacket_LiquidStake() { + liquidStakerOnStride := s.TestAccs[0] + depositAddress := s.TestAccs[1] + forwardRecipientOnHost := HostAddress + + stakeAmount := sdk.NewInt(1000000) + + testCases := []struct { + name string + enabled bool + liquidStakeDenom string + transferReceiver string + transferMemo string + hostZoneChannelID string // defaults to channel-0 if not specified + inboundTransferChannnelId string // defaults to channel-0 if not specified + expectedForwardChannelId string // defaults to empty (no forwarding) + expectedSuccess bool + expectedLiquidStake bool + }{ + { + name: "successful liquid stake with metadata in receiver", + enabled: true, + liquidStakeDenom: Atom, + transferReceiver: getLiquidStakePacketMetadata(liquidStakerOnStride.String(), "", ""), + transferMemo: "", + expectedSuccess: true, + expectedLiquidStake: true, + }, + { + name: "successful liquid stake with metadata in the memo", + enabled: true, + liquidStakeDenom: Atom, + transferReceiver: liquidStakerOnStride.String(), + transferMemo: getLiquidStakePacketMetadata(liquidStakerOnStride.String(), "", ""), + expectedSuccess: true, + expectedLiquidStake: true, + }, + { + name: "successful liquid stake and forward to default host (receiver)", + enabled: true, + liquidStakeDenom: Atom, + transferReceiver: getLiquidStakePacketMetadata(liquidStakerOnStride.String(), forwardRecipientOnHost, ""), + transferMemo: "", + expectedForwardChannelId: ibctesting.FirstChannelID, + expectedSuccess: true, + expectedLiquidStake: true, + }, + { + name: "successful liquid stake and forward to default host (memo)", + enabled: true, + liquidStakeDenom: Atom, + transferReceiver: liquidStakerOnStride.String(), + transferMemo: getLiquidStakePacketMetadata(liquidStakerOnStride.String(), forwardRecipientOnHost, ""), + expectedForwardChannelId: ibctesting.FirstChannelID, + expectedSuccess: true, + expectedLiquidStake: true, + }, + { + name: "successful liquid stake and forward to custom transfer channel (receiver)", + enabled: true, + liquidStakeDenom: Atom, + transferReceiver: getLiquidStakePacketMetadata(liquidStakerOnStride.String(), forwardRecipientOnHost, "channel-0"), + transferMemo: "", + hostZoneChannelID: "channel-1", + inboundTransferChannnelId: "channel-1", + expectedForwardChannelId: "channel-0", // different than host zone, specified in memo + expectedSuccess: true, + expectedLiquidStake: true, + }, + { + name: "successful liquid stake and forward to custom transfer channel (memo)", + enabled: true, + liquidStakeDenom: Atom, + transferReceiver: liquidStakerOnStride.String(), + transferMemo: getLiquidStakePacketMetadata(liquidStakerOnStride.String(), forwardRecipientOnHost, "channel-0"), + hostZoneChannelID: "channel-1", + inboundTransferChannnelId: "channel-1", + expectedForwardChannelId: "channel-0", // different than host zone, specified in memo + expectedSuccess: true, + expectedLiquidStake: true, + }, + { + name: "normal transfer with no liquid stake", + enabled: true, + liquidStakeDenom: Atom, + transferReceiver: liquidStakerOnStride.String(), + transferMemo: "", + expectedSuccess: true, + expectedLiquidStake: false, + }, + { + name: "autopilot disabled", + enabled: false, + liquidStakeDenom: Atom, + transferReceiver: liquidStakerOnStride.String(), + transferMemo: getLiquidStakePacketMetadata(liquidStakerOnStride.String(), "", ""), + expectedSuccess: false, + }, + { + name: "invalid stride address (receiver)", + enabled: true, + liquidStakeDenom: Osmo, + transferReceiver: getLiquidStakePacketMetadata("XXX", "", ""), + transferMemo: "", + expectedSuccess: false, + }, + { + name: "invalid stride address (memo)", + enabled: true, + liquidStakeDenom: Osmo, + transferReceiver: liquidStakerOnStride.String(), + transferMemo: getLiquidStakePacketMetadata("XXX", "", ""), + expectedSuccess: false, + }, + { + name: "not host denom", + enabled: true, + liquidStakeDenom: Osmo, + transferReceiver: liquidStakerOnStride.String(), + transferMemo: getLiquidStakePacketMetadata(liquidStakerOnStride.String(), "", ""), + expectedSuccess: false, + }, + { + name: "failed to outbound transfer", + enabled: true, + liquidStakeDenom: Atom, + transferReceiver: getLiquidStakePacketMetadata(liquidStakerOnStride.String(), forwardRecipientOnHost, "channel-999"), // channel DNE + transferMemo: "", + expectedSuccess: false, + }, + { + name: "valid uatom token from invalid channel", + enabled: true, + liquidStakeDenom: Atom, + transferReceiver: getLiquidStakePacketMetadata(liquidStakerOnStride.String(), "", ""), + transferMemo: "", + hostZoneChannelID: "channel-0", + inboundTransferChannnelId: "channel-999", // channel DNE + expectedSuccess: false, }, } - for i, tc := range testCases { - suite.Run(fmt.Sprintf("Case %d", i), func() { - packet.DestinationChannel = tc.destChannel - packet.Data = transfertypes.ModuleCdc.MustMarshalJSON(&tc.packetData) - - suite.SetupTest() // reset - ctx := suite.Ctx - - suite.App.AutopilotKeeper.SetParams(ctx, types.Params{StakeibcActive: tc.forwardingActive}) - - // set epoch tracker for env - suite.App.StakeibcKeeper.SetEpochTracker(ctx, stakeibctypes.EpochTracker{ - EpochIdentifier: epochtypes.STRIDE_EPOCH, - EpochNumber: 1, - NextEpochStartTime: uint64(now.Unix()), - Duration: 43200, - }) - // set deposit record for env - suite.App.RecordsKeeper.SetDepositRecord(ctx, recordstypes.DepositRecord{ - Id: 1, - Amount: sdk.NewInt(100), - Denom: atomIbcDenom, - HostZoneId: "hub-1", - Status: recordstypes.DepositRecord_TRANSFER_QUEUE, - DepositEpochNumber: 1, - Source: recordstypes.DepositRecord_STRIDE, - }) - // set host zone for env - suite.App.StakeibcKeeper.SetHostZone(ctx, stakeibctypes.HostZone{ - ChainId: "hub-1", - ConnectionId: "connection-0", - Bech32Prefix: "cosmos", - TransferChannelId: "channel-0", - IbcDenom: atomIbcDenom, - HostDenom: atomHostDenom, - RedemptionRate: sdk.NewDec(1), - DepositAddress: addr1.String(), - }) - - // mint coins to be spent on liquid staking - coins := sdk.Coins{sdk.NewInt64Coin(tc.recvDenom, 1000000)} - err := suite.App.BankKeeper.MintCoins(ctx, minttypes.ModuleName, coins) - suite.Require().NoError(err) - err = suite.App.BankKeeper.SendCoinsFromModuleToAccount(ctx, minttypes.ModuleName, addr1, coins) - suite.Require().NoError(err) - - transferIBCModule := transfer.NewIBCModule(suite.App.TransferKeeper) - recordsStack := recordsmodule.NewIBCModule(suite.App.RecordsKeeper, transferIBCModule) - routerIBCModule := autopilot.NewIBCModule(suite.App.AutopilotKeeper, recordsStack) + for _, tc := range testCases { + s.Run(tc.name, func() { + s.SetupTest() // reset + + // Fill in the default channel ID's if they weren't specified + if tc.hostZoneChannelID == "" { + tc.hostZoneChannelID = ibctesting.FirstChannelID + } + if tc.inboundTransferChannnelId == "" { + tc.inboundTransferChannnelId = ibctesting.FirstChannelID + } + + transferMetadata := transfertypes.FungibleTokenPacketData{ + Sender: HostAddress, + Receiver: tc.transferReceiver, + Denom: ReceivePacketDenomTraces[tc.liquidStakeDenom], + Amount: stakeAmount.String(), + Memo: tc.transferMemo, + } + packet := channeltypes.Packet{ + SourcePort: transfertypes.PortID, + SourceChannel: SourceChannelOnHost, + DestinationPort: transfertypes.PortID, + DestinationChannel: tc.inboundTransferChannnelId, + Data: transfertypes.ModuleCdc.MustMarshalJSON(&transferMetadata), + } + + nativeTokenIBCDenom := s.SetupAutopilotLiquidStake(tc.enabled, tc.hostZoneChannelID, depositAddress, liquidStakerOnStride) + + transferIBCModule := transfer.NewIBCModule(s.App.TransferKeeper) + recordsStack := recordsmodule.NewIBCModule(s.App.RecordsKeeper, transferIBCModule) + routerIBCModule := autopilot.NewIBCModule(s.App.AutopilotKeeper, recordsStack) ack := routerIBCModule.OnRecvPacket( - ctx, + s.Ctx, packet, - addr1, + s.TestAccs[2], // arbitrary relayer address - not actually used ) - if tc.expSuccess { - suite.Require().True(ack.Success(), "ack should be successful - ack: %+v", string(ack.Acknowledgement())) - - // Check funds were transferred - coin := suite.App.BankKeeper.GetBalance(suite.Ctx, addr1, tc.recvDenom) - suite.Require().Equal("2000000", coin.Amount.String(), "balance should have updated after successful transfer") - - // check minted balance for liquid staking - allBalance := suite.App.BankKeeper.GetAllBalances(ctx, addr1) - liquidBalance := suite.App.BankKeeper.GetBalance(ctx, addr1, "stuatom") - if tc.expLiquidStake { - suite.Require().True(liquidBalance.Amount.IsPositive(), "liquid balance should be positive but was %s", allBalance.String()) - } else { - suite.Require().True(liquidBalance.Amount.IsZero(), "liquid balance should be zero but was %s", allBalance.String()) + + if tc.expectedSuccess { + s.Require().True(ack.Success(), "ack should be successful - ack: %+v", string(ack.Acknowledgement())) + + if tc.expectedLiquidStake { + s.CheckLiquidStakeSucceeded( + stakeAmount, + liquidStakerOnStride, + depositAddress, + nativeTokenIBCDenom, + tc.expectedForwardChannelId, + ) } } else { - suite.Require().False(ack.Success(), "ack should have failed - ack: %+v", string(ack.Acknowledgement())) + s.Require().False(ack.Success(), "ack should have failed - ack: %+v", string(ack.Acknowledgement())) } }) } diff --git a/x/stakeibc/keeper/host_zone.go b/x/stakeibc/keeper/host_zone.go index 2630c54f5..fbaed1ea8 100644 --- a/x/stakeibc/keeper/host_zone.go +++ b/x/stakeibc/keeper/host_zone.go @@ -48,7 +48,7 @@ func (k Keeper) GetHostZoneFromHostDenom(ctx sdk.Context, denom string) (*types. if matchZone.ChainId != "" { return &matchZone, nil } - return nil, errorsmod.Wrapf(sdkerrors.ErrUnknownRequest, "No HostZone for %s found", denom) + return nil, errorsmod.Wrapf(sdkerrors.ErrUnknownRequest, "No HostZone for %s denom found", denom) } // GetHostZoneFromTransferChannelID returns a HostZone from a transfer channel ID From 1bfa7ca62db9fd8789844d6375cecb5599eb9e12 Mon Sep 17 00:00:00 2001 From: sampocs Date: Mon, 8 Jan 2024 18:46:57 -0600 Subject: [PATCH 23/30] renamed key function --- x/autopilot/keeper/fallback.go | 6 +++--- x/autopilot/types/keys.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/x/autopilot/keeper/fallback.go b/x/autopilot/keeper/fallback.go index cccb5bc96..8ef3d5967 100644 --- a/x/autopilot/keeper/fallback.go +++ b/x/autopilot/keeper/fallback.go @@ -10,7 +10,7 @@ import ( // Stores a fallback address for an outbound transfer func (k Keeper) SetTransferFallbackAddress(ctx sdk.Context, channelId string, sequence uint64, address string) { store := prefix.NewStore(ctx.KVStore(k.storeKey), types.TransferFallbackAddressPrefix) - key := types.GetTransferFallbackAddressKeyPrefix(channelId, sequence) + key := types.GetTransferFallbackAddressKey(channelId, sequence) value := []byte(address) store.Set(key, value) } @@ -19,7 +19,7 @@ func (k Keeper) SetTransferFallbackAddress(ctx sdk.Context, channelId string, se // This is used after the ack or timeout for a packet has been received func (k Keeper) RemoveTransferFallbackAddress(ctx sdk.Context, channelId string, sequence uint64) { store := prefix.NewStore(ctx.KVStore(k.storeKey), types.TransferFallbackAddressPrefix) - key := types.GetTransferFallbackAddressKeyPrefix(channelId, sequence) + key := types.GetTransferFallbackAddressKey(channelId, sequence) store.Delete(key) } @@ -28,7 +28,7 @@ func (k Keeper) RemoveTransferFallbackAddress(ctx sdk.Context, channelId string, func (k Keeper) GetTransferFallbackAddress(ctx sdk.Context, channelId string, sequence uint64) (address string, found bool) { store := prefix.NewStore(ctx.KVStore(k.storeKey), types.TransferFallbackAddressPrefix) - key := types.GetTransferFallbackAddressKeyPrefix(channelId, sequence) + key := types.GetTransferFallbackAddressKey(channelId, sequence) valueBz := store.Get(key) if len(valueBz) == 0 { diff --git a/x/autopilot/types/keys.go b/x/autopilot/types/keys.go index eb85bb3ad..2b225e6ad 100644 --- a/x/autopilot/types/keys.go +++ b/x/autopilot/types/keys.go @@ -22,9 +22,9 @@ var ( FallbackAddressChannelPrefixLength int = 16 ) -// Builds the store key prefix for a fallback address, key'd by channel ID and sequence number +// Builds the store key for a fallback address, key'd by channel ID and sequence number // The serialized channelId is set to a fixed array size to assist deserialization -func GetTransferFallbackAddressKeyPrefix(channelId string, sequenceNumber uint64) []byte { +func GetTransferFallbackAddressKey(channelId string, sequenceNumber uint64) []byte { channelIdBz := make([]byte, FallbackAddressChannelPrefixLength) copy(channelIdBz[:], channelId) From cd7ce0be5a5d160432db40d7278f15292eeb2687 Mon Sep 17 00:00:00 2001 From: sampocs Date: Mon, 8 Jan 2024 18:55:10 -0600 Subject: [PATCH 24/30] replaced CheckAckStatus with helper from icacallbacks --- x/autopilot/keeper/ibc.go | 33 ++++++--------------------------- 1 file changed, 6 insertions(+), 27 deletions(-) diff --git a/x/autopilot/keeper/ibc.go b/x/autopilot/keeper/ibc.go index 52bae57f7..335ff4589 100644 --- a/x/autopilot/keeper/ibc.go +++ b/x/autopilot/keeper/ibc.go @@ -8,32 +8,10 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types" -) - -// Deserializes the acknowledgement and returns a bool indicating whether it was successful or was an ack error -func (k Keeper) CheckAcknowledgementStatus(ctx sdk.Context, acknowledgementBz []byte) (success bool, err error) { - // Unmarshal the raw ack response - var acknowledgement channeltypes.Acknowledgement - if err := transfertypes.ModuleCdc.UnmarshalJSON(acknowledgementBz, &acknowledgement); err != nil { - return false, errorsmod.Wrapf(err, "cannot unmarshal ICS-20 transfer packet acknowledgement") - } - - // The ack can come back as either AcknowledgementResult (success) or AcknowledgementError (failure) - switch response := acknowledgement.Response.(type) { - case *channeltypes.Acknowledgement_Result: - if len(response.Result) == 0 { - return false, errorsmod.Wrapf(channeltypes.ErrInvalidAcknowledgement, "acknowledgement result cannot be empty") - } - return true, nil - case *channeltypes.Acknowledgement_Error: - k.Logger(ctx).Error(fmt.Sprintf("autopilot acknowledgement error: %s", response.Error)) - return false, nil - - default: - return false, errorsmod.Wrapf(channeltypes.ErrInvalidAcknowledgement, "unsupported acknowledgement response field type %T", response) - } -} + "github.com/Stride-Labs/stride/v16/x/icacallbacks" + icacallbacktypes "github.com/Stride-Labs/stride/v16/x/icacallbacks/types" +) // Build an sdk.Coin type from the transfer metadata which includes strings for the amount and denom func (k Keeper) BuildCoinFromTransferMetadata(transferMetadata transfertypes.FungibleTokenPacketData) (coin sdk.Coin, err error) { @@ -98,13 +76,14 @@ func (k Keeper) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Pac k.RemoveTransferFallbackAddress(ctx, channelId, sequence) // Check whether the ack was successful or was an ack error - success, err := k.CheckAcknowledgementStatus(ctx, acknowledgement) + isICATx := false + ackResponse, err := icacallbacks.UnpackAcknowledgementResponse(ctx, k.Logger(ctx), packet.Data, isICATx) if err != nil { return err } // If successful, no additional action is necessary - if success { + if ackResponse.Status == icacallbacktypes.AckResponseStatus_SUCCESS { return nil } From 8958d804d9bfae6e561ba38976024526d15c5529 Mon Sep 17 00:00:00 2001 From: sampocs Date: Mon, 8 Jan 2024 19:58:23 -0600 Subject: [PATCH 25/30] removed retry and replaced with send to fallback address --- x/autopilot/keeper/ibc.go | 84 ++++++++-------------------------- x/autopilot/keeper/ibc_test.go | 62 ------------------------- 2 files changed, 20 insertions(+), 126 deletions(-) diff --git a/x/autopilot/keeper/ibc.go b/x/autopilot/keeper/ibc.go index 335ff4589..e67b73812 100644 --- a/x/autopilot/keeper/ibc.go +++ b/x/autopilot/keeper/ibc.go @@ -13,16 +13,6 @@ import ( icacallbacktypes "github.com/Stride-Labs/stride/v16/x/icacallbacks/types" ) -// Build an sdk.Coin type from the transfer metadata which includes strings for the amount and denom -func (k Keeper) BuildCoinFromTransferMetadata(transferMetadata transfertypes.FungibleTokenPacketData) (coin sdk.Coin, err error) { - amount, ok := sdkmath.NewIntFromString(transferMetadata.Amount) - if !ok { - return coin, fmt.Errorf("unable to parse amount from transfer packet: %v", transferMetadata) - } - coin = sdk.NewCoin(transferMetadata.Denom, amount) - return coin, nil -} - // In the event of an ack error after a outbound transfer, we'll have to bank send to a fallback address func (k Keeper) SendToFallbackAddress(ctx sdk.Context, packetData []byte, fallbackAddress string) error { // First unmarshal the transfer metadata to get the sender/reciever, and token amount/denom @@ -43,10 +33,11 @@ func (k Keeper) SendToFallbackAddress(ctx sdk.Context, packetData []byte, fallba } // Build the token from the transfer metadata - token, err := k.BuildCoinFromTransferMetadata(transferMetadata) - if err != nil { - return err + amount, ok := sdkmath.NewIntFromString(transferMetadata.Amount) + if !ok { + return fmt.Errorf("unable to parse amount from transfer packet: %v", transferMetadata) } + token := sdk.NewCoin(transferMetadata.Denom, amount) // Finally send to the fallback account if err := k.bankKeeper.SendCoins(ctx, senderAccount, fallbackAccount, sdk.NewCoins(token)); err != nil { @@ -56,11 +47,10 @@ func (k Keeper) SendToFallbackAddress(ctx sdk.Context, packetData []byte, fallba return nil } -// If there was a failed ack from an outbound transfer of one of the autopilot actions, -// we'll need to check if there was a fallback address. If one was stored, bank send -// to that fallback address +// If there was a timeout or failed ack from an outbound transfer of one of the autopilot actions, +// we'll need to check if there was a fallback address. If one was stored, bank send to that address // If the ack was successful, we should delete the address (if it exists) -func (k Keeper) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Packet, acknowledgement []byte) error { +func (k Keeper) HandleFallbackAddress(ctx sdk.Context, packet channeltypes.Packet, acknowledgement []byte, packetTimedOut bool) error { // Retrieve the fallback address for the given packet // We use the packet source channel here since this will correspond with the channel on Stride channelId := packet.SourceChannel @@ -75,7 +65,12 @@ func (k Keeper) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Pac // Remove the fallback address since the packet is no longer pending k.RemoveTransferFallbackAddress(ctx, channelId, sequence) - // Check whether the ack was successful or was an ack error + // If the packet timed out, send to the fallback address + if packetTimedOut { + return k.SendToFallbackAddress(ctx, packet.Data, fallbackAddress) + } + + // If the packet did not timeout, check whether the ack was successful or was an ack error isICATx := false ackResponse, err := icacallbacks.UnpackAcknowledgementResponse(ctx, k.Logger(ctx), packet.Data, isICATx) if err != nil { @@ -87,55 +82,16 @@ func (k Keeper) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Pac return nil } - // If the ack was an error, we'll need to bank send to the fallback address + // If there was an ack error, we'll need to bank send to the fallback address return k.SendToFallbackAddress(ctx, packet.Data, fallbackAddress) } -// If there's a timed out packet, we'll infinitely retry the transfer +// OnTimeoutPacket should always send to the fallback address func (k Keeper) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet) error { - // Retrieve the fallback address from the original packet - // We use the packet source channel here since this will correspond with the channel on Stride - channelId := packet.SourceChannel - originalSequence := packet.Sequence - fallbackAddress, fallbackAddressFound := k.GetTransferFallbackAddress(ctx, channelId, originalSequence) - - // If there was no fallback address, this packet was not from an autopilot action and there's no need to retry - if !fallbackAddressFound { - return nil - } - - // If this was from an autopilot action, unmarshal the transfer metadata to get the original transfer info - var transferMetadata transfertypes.FungibleTokenPacketData - if err := transfertypes.ModuleCdc.UnmarshalJSON(packet.Data, &transferMetadata); err != nil { - return errorsmod.Wrapf(err, "unable to unmarshal ICS-20 packet data") - } - - // Build the token from the transfer metadata - token, err := k.BuildCoinFromTransferMetadata(transferMetadata) - if err != nil { - return err - } - - // Submit the transfer again with a new timeout - timeoutTimestamp := uint64(ctx.BlockTime().UnixNano()) + transfertypes.DefaultRelativePacketTimeoutTimestamp - msgTransfer := transfertypes.MsgTransfer{ - SourcePort: transfertypes.PortID, - SourceChannel: packet.SourceChannel, - Token: token, - Sender: transferMetadata.Sender, - Receiver: transferMetadata.Receiver, - TimeoutTimestamp: timeoutTimestamp, - Memo: transferMetadata.Memo, - } - retryResponse, err := k.transferKeeper.Transfer(ctx, &msgTransfer) - if err != nil { - return errorsmod.Wrapf(err, "unable to submit transfer retry of %+v", msgTransfer) - } - - // Update the fallback address to use the new sequence number - updatedSequence := retryResponse.Sequence - k.RemoveTransferFallbackAddress(ctx, channelId, originalSequence) - k.SetTransferFallbackAddress(ctx, channelId, updatedSequence, fallbackAddress) + return k.HandleFallbackAddress(ctx, packet, []byte{}, true) +} - return nil +// OnAcknowledgementPacket should send to the fallback address if the ack is an ack error +func (k Keeper) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Packet, acknowledgement []byte) error { + return k.HandleFallbackAddress(ctx, packet, acknowledgement, false) } diff --git a/x/autopilot/keeper/ibc_test.go b/x/autopilot/keeper/ibc_test.go index 049508fc3..0957d8b09 100644 --- a/x/autopilot/keeper/ibc_test.go +++ b/x/autopilot/keeper/ibc_test.go @@ -22,68 +22,6 @@ type PacketCallbackTestCase struct { // IBC Callback Helpers // -------------------------------------------------------------- -func (s *KeeperTestSuite) TestCheckAcknowledgementStatus() { - // Test with a successful ack - ackSuccess := transfertypes.ModuleCdc.MustMarshalJSON(&channeltypes.Acknowledgement{ - Response: &channeltypes.Acknowledgement_Result{ - Result: []byte{1}, // just has to be non-empty - }, - }) - success, err := s.App.AutopilotKeeper.CheckAcknowledgementStatus(s.Ctx, ackSuccess) - s.Require().True(success, "ack success should return true") - s.Require().NoError(err) - - // Test with an ack error - // Success should be false, but there should be no error returned - errorString := "some error" - ackFailure := transfertypes.ModuleCdc.MustMarshalJSON(&channeltypes.Acknowledgement{ - Response: &channeltypes.Acknowledgement_Error{ - Error: errorString, - }, - }) - success, err = s.App.AutopilotKeeper.CheckAcknowledgementStatus(s.Ctx, ackFailure) - s.Require().False(success, "ack failure should return false") - s.Require().NoError(err) - - // Test with an ack result that is missing the "result" field - // It should return an error - ackResultError := transfertypes.ModuleCdc.MustMarshalJSON(&channeltypes.Acknowledgement{ - Response: &channeltypes.Acknowledgement_Result{ - Result: []byte{}, // empty result throws an error - }, - }) - _, err = s.App.AutopilotKeeper.CheckAcknowledgementStatus(s.Ctx, ackResultError) - s.Require().ErrorContains(err, "acknowledgement result cannot be empty") - - // Test with invalid ack data that can't be unmarshaled - randomBytes := []byte{1, 2, 3} - _, err = s.App.AutopilotKeeper.CheckAcknowledgementStatus(s.Ctx, randomBytes) - s.Require().ErrorContains(err, "cannot unmarshal ICS-20 transfer packet acknowledgement") -} - -func (s *KeeperTestSuite) TestBuildCoinFromTransferMetadata() { - denom := "denom" - amount := sdk.NewInt(10000) - - // Test with valid packet data - expectedToken := sdk.NewCoin(denom, amount) - transferMetadata := transfertypes.FungibleTokenPacketData{ - Denom: denom, - Amount: amount.String(), - } - actualToken, err := s.App.AutopilotKeeper.BuildCoinFromTransferMetadata(transferMetadata) - s.Require().NoError(err) - s.Require().Equal(expectedToken, actualToken, "token") - - // Test with invalid packet data - invalidMetadata := transfertypes.FungibleTokenPacketData{ - Denom: denom, - Amount: "", - } - _, err = s.App.AutopilotKeeper.BuildCoinFromTransferMetadata(invalidMetadata) - s.Require().ErrorContains(err, "unable to parse amount from transfer packet") -} - func (s *KeeperTestSuite) TestSendToFallbackAddress() { senderAccount := s.TestAccs[0] fallbackAccount := s.TestAccs[1] From 55635d09b90ec9109de936c914fbd717d2aa5e28 Mon Sep 17 00:00:00 2001 From: sampocs Date: Mon, 8 Jan 2024 20:15:58 -0600 Subject: [PATCH 26/30] fixed unit tests --- x/autopilot/keeper/ibc.go | 2 +- x/autopilot/keeper/ibc_test.go | 200 +++++++++------------------------ 2 files changed, 53 insertions(+), 149 deletions(-) diff --git a/x/autopilot/keeper/ibc.go b/x/autopilot/keeper/ibc.go index e67b73812..159cb5b77 100644 --- a/x/autopilot/keeper/ibc.go +++ b/x/autopilot/keeper/ibc.go @@ -72,7 +72,7 @@ func (k Keeper) HandleFallbackAddress(ctx sdk.Context, packet channeltypes.Packe // If the packet did not timeout, check whether the ack was successful or was an ack error isICATx := false - ackResponse, err := icacallbacks.UnpackAcknowledgementResponse(ctx, k.Logger(ctx), packet.Data, isICATx) + ackResponse, err := icacallbacks.UnpackAcknowledgementResponse(ctx, k.Logger(ctx), acknowledgement, isICATx) if err != nil { return err } diff --git a/x/autopilot/keeper/ibc_test.go b/x/autopilot/keeper/ibc_test.go index 0957d8b09..408a51980 100644 --- a/x/autopilot/keeper/ibc_test.go +++ b/x/autopilot/keeper/ibc_test.go @@ -5,7 +5,6 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types" - ibctesting "github.com/cosmos/ibc-go/v7/testing" ) type PacketCallbackTestCase struct { @@ -18,6 +17,44 @@ type PacketCallbackTestCase struct { FallbackAccount sdk.AccAddress } +func (s *KeeperTestSuite) SetupTestHandleFallbackPacket() PacketCallbackTestCase { + senderAccount := s.TestAccs[0] + fallbackAccount := s.TestAccs[1] + + sequence := uint64(1) + channelId := "channel-0" + denom := "denom" + amount := sdk.NewInt(10000) + token := sdk.NewCoin(denom, amount) + + // Set a fallback addresses + s.App.AutopilotKeeper.SetTransferFallbackAddress(s.Ctx, channelId, sequence, fallbackAccount.String()) + + // Fund the sender account + s.FundAccount(senderAccount, token) + + // Build the IBC packet + transferMetadata := transfertypes.FungibleTokenPacketData{ + Denom: "denom", + Amount: amount.String(), + Sender: senderAccount.String(), + } + packet := channeltypes.Packet{ + Sequence: sequence, + SourceChannel: channelId, + Data: transfertypes.ModuleCdc.MustMarshalJSON(&transferMetadata), + } + + return PacketCallbackTestCase{ + ChannelId: channelId, + OriginalSequence: sequence, + Token: token, + Packet: packet, + SenderAccount: senderAccount, + FallbackAccount: fallbackAccount, + } +} + // -------------------------------------------------------------- // IBC Callback Helpers // -------------------------------------------------------------- @@ -82,46 +119,8 @@ func (s *KeeperTestSuite) TestSendToFallbackAddress() { // OnAcknowledgementPacket // -------------------------------------------------------------- -func (s *KeeperTestSuite) SetupTestOnAcknowledgementPacket() PacketCallbackTestCase { - senderAccount := s.TestAccs[0] - fallbackAccount := s.TestAccs[1] - - sequence := uint64(1) - channelId := "channel-0" - denom := "denom" - amount := sdk.NewInt(10000) - token := sdk.NewCoin(denom, amount) - - // Set a fallback addresses - s.App.AutopilotKeeper.SetTransferFallbackAddress(s.Ctx, channelId, sequence, fallbackAccount.String()) - - // Fund the sender account - s.FundAccount(senderAccount, token) - - // Build the IBC packet - transferMetadata := transfertypes.FungibleTokenPacketData{ - Denom: "denom", - Amount: amount.String(), - Sender: senderAccount.String(), - } - packet := channeltypes.Packet{ - Sequence: sequence, - SourceChannel: channelId, - Data: transfertypes.ModuleCdc.MustMarshalJSON(&transferMetadata), - } - - return PacketCallbackTestCase{ - ChannelId: channelId, - OriginalSequence: sequence, - Token: token, - Packet: packet, - SenderAccount: senderAccount, - FallbackAccount: fallbackAccount, - } -} - func (s *KeeperTestSuite) TestOnAcknowledgementPacket_AckSuccess() { - tc := s.SetupTestOnAcknowledgementPacket() + tc := s.SetupTestHandleFallbackPacket() // Build a successful ack ackSuccess := transfertypes.ModuleCdc.MustMarshalJSON(&channeltypes.Acknowledgement{ @@ -145,7 +144,7 @@ func (s *KeeperTestSuite) TestOnAcknowledgementPacket_AckSuccess() { } func (s *KeeperTestSuite) TestOnAcknowledgementPacket_AckFailure() { - tc := s.SetupTestOnAcknowledgementPacket() + tc := s.SetupTestHandleFallbackPacket() // Build an error ack ackFailure := transfertypes.ModuleCdc.MustMarshalJSON(&channeltypes.Acknowledgement{ @@ -169,7 +168,7 @@ func (s *KeeperTestSuite) TestOnAcknowledgementPacket_AckFailure() { } func (s *KeeperTestSuite) TestOnAcknowledgementPacket_InvalidAck() { - tc := s.SetupTestOnAcknowledgementPacket() + tc := s.SetupTestHandleFallbackPacket() // Build an invalid ack to force an error invalidAck := transfertypes.ModuleCdc.MustMarshalJSON(&channeltypes.Acknowledgement{ @@ -184,7 +183,7 @@ func (s *KeeperTestSuite) TestOnAcknowledgementPacket_InvalidAck() { } func (s *KeeperTestSuite) TestOnAcknowledgementPacket_NoOp() { - tc := s.SetupTestOnAcknowledgementPacket() + tc := s.SetupTestHandleFallbackPacket() // Remove the fallback address so that there is no action necessary in the callback s.App.AutopilotKeeper.RemoveTransferFallbackAddress(s.Ctx, tc.ChannelId, tc.OriginalSequence) @@ -206,81 +205,27 @@ func (s *KeeperTestSuite) TestOnAcknowledgementPacket_NoOp() { // OnTimeoutPacket // -------------------------------------------------------------- -func (s *KeeperTestSuite) SetupTestOnTimeoutPacket() PacketCallbackTestCase { - senderAccount := s.TestAccs[0] - fallbackAccount := s.TestAccs[1] - receiverAccount := s.TestAccs[2] - - chainId := "chain-0" - denom := "denom" - amount := sdk.NewInt(10000) - transferToken := sdk.NewCoin(denom, amount) - - // Create transfer channel so the retry can be submitted - s.CreateTransferChannel(chainId) - channelId := ibctesting.FirstChannelID - - // Determine the next sequence number along that channel (which will be the seqence number for the retry) - expectedRetrySequence, ok := s.App.IBCKeeper.ChannelKeeper.GetNextSequenceSend(s.Ctx, transfertypes.PortID, channelId) - s.Require().True(ok) - - // Store a fallback address on the previous sequence number (from the original timed out transfer) - originalSequence := expectedRetrySequence - 1 - s.App.AutopilotKeeper.SetTransferFallbackAddress(s.Ctx, channelId, originalSequence, fallbackAccount.String()) - - // Fund the sender address so they have sufficient tokens to re-submit a transfer - s.FundAccount(senderAccount, transferToken) - - // Build the packet data - transferMetadata := transfertypes.FungibleTokenPacketData{ - Sender: senderAccount.String(), - Receiver: receiverAccount.String(), - Denom: denom, - Amount: amount.String(), - } - packetDataBz := transfertypes.ModuleCdc.MustMarshalJSON(&transferMetadata) - packet := channeltypes.Packet{ - Sequence: originalSequence, - SourceChannel: channelId, - Data: packetDataBz, - } - - return PacketCallbackTestCase{ - ChannelId: channelId, - OriginalSequence: originalSequence, - RetrySequence: expectedRetrySequence, - Token: transferToken, - Packet: packet, - SenderAccount: senderAccount, - } -} - -func (s *KeeperTestSuite) TestOnTimeoutPacket_SuccessfulRetry() { - tc := s.SetupTestOnTimeoutPacket() +func (s *KeeperTestSuite) TestOnTimeoutPacket_Successful() { + tc := s.SetupTestHandleFallbackPacket() // Call OnTimeoutPacket err := s.App.AutopilotKeeper.OnTimeoutPacket(s.Ctx, tc.Packet) s.Require().NoError(err, "no error expected when calling OnTimeoutPacket") - // Check that the sender's funds were escrowed (from the retry) - zeroCoin := sdk.NewCoin(tc.Token.Denom, sdkmath.ZeroInt()) + // Confirm tokens were sent to the fallback address + zeroCoin := sdk.NewCoin(tc.Token.Denom, sdk.ZeroInt()) senderBalance := s.App.BankKeeper.GetBalance(s.Ctx, tc.SenderAccount, tc.Token.Denom) - s.CompareCoins(zeroCoin, senderBalance, "the sender should have no more funds after the retry") - - escrowAddress := transfertypes.GetEscrowAddress(transfertypes.PortID, tc.ChannelId) - escrowBalance := s.App.BankKeeper.GetBalance(s.Ctx, escrowAddress, tc.Token.Denom) - s.CompareCoins(tc.Token, escrowBalance, "escrow balance should have increased") + fallbackBalance := s.App.BankKeeper.GetBalance(s.Ctx, tc.FallbackAccount, tc.Token.Denom) + s.CompareCoins(zeroCoin, senderBalance, "sender account should have lost funds") + s.CompareCoins(tc.Token, fallbackBalance, "fallback account should have received funds") - // Check that the fallback address was moved to a new sequence number + // Confirm the fallback address was removed _, found := s.App.AutopilotKeeper.GetTransferFallbackAddress(s.Ctx, tc.ChannelId, tc.OriginalSequence) - s.Require().False(found, "fallback address should no longer be on the old sequence number") - - _, found = s.App.AutopilotKeeper.GetTransferFallbackAddress(s.Ctx, tc.ChannelId, tc.RetrySequence) - s.Require().True(found, "fallback address should now use the new sequence number") + s.Require().False(found, "fallback address should have been removed") } func (s *KeeperTestSuite) TestOnTimeoutPacket_NoOp() { - tc := s.SetupTestOnTimeoutPacket() + tc := s.SetupTestHandleFallbackPacket() // Remove the fallback address s.App.AutopilotKeeper.RemoveTransferFallbackAddress(s.Ctx, tc.ChannelId, tc.OriginalSequence) @@ -293,44 +238,3 @@ func (s *KeeperTestSuite) TestOnTimeoutPacket_NoOp() { senderBalance := s.App.BankKeeper.GetBalance(s.Ctx, tc.SenderAccount, tc.Token.Denom) s.CompareCoins(tc.Token, senderBalance, "the sender balance should not have changed") } - -func (s *KeeperTestSuite) TestOnTimeoutPacket_InvalidPacketData() { - tc := s.SetupTestOnTimeoutPacket() - - // Modify the packet data so that it will fail unmarshalling - tc.Packet.Data = []byte{1, 2, 3} - - err := s.App.AutopilotKeeper.OnTimeoutPacket(s.Ctx, tc.Packet) - s.Require().ErrorContains(err, "unable to unmarshal ICS-20 packet data") -} - -func (s *KeeperTestSuite) TestOnTimeoutPacket_InvalidToken() { - tc := s.SetupTestOnTimeoutPacket() - - // Modify the token in the packet data so that the amount is empty - transferMetadata := transfertypes.FungibleTokenPacketData{ - Denom: tc.Token.Denom, - Amount: "", - } - tc.Packet.Data = transfertypes.ModuleCdc.MustMarshalJSON(&transferMetadata) - - // Call OnTimeoutPacket - it should fail - err := s.App.AutopilotKeeper.OnTimeoutPacket(s.Ctx, tc.Packet) - s.Require().ErrorContains(err, "unable to parse amount from transfer packet") -} - -func (s *KeeperTestSuite) TestOnTimeoutPacket_FailedToRetry() { - tc := s.SetupTestOnTimeoutPacket() - - // Modify the packet data so that the sender is an invalid address - transferMetadata := transfertypes.FungibleTokenPacketData{ - Sender: "invalid_address", - Denom: tc.Token.Denom, - Amount: tc.Token.Amount.String(), - } - tc.Packet.Data = transfertypes.ModuleCdc.MustMarshalJSON(&transferMetadata) - - // Call OnTimeoutPacket - it should be unable to submit the retry - err := s.App.AutopilotKeeper.OnTimeoutPacket(s.Ctx, tc.Packet) - s.Require().ErrorContains(err, "unable to submit transfer retry") -} From 2bf291f924a1587cbb8092237f4602e9d8e3830f Mon Sep 17 00:00:00 2001 From: sampocs Date: Mon, 8 Jan 2024 22:03:42 -0600 Subject: [PATCH 27/30] updated timeout to 3 hours --- x/autopilot/keeper/liquidstake.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/x/autopilot/keeper/liquidstake.go b/x/autopilot/keeper/liquidstake.go index 36d354eee..da88605f4 100644 --- a/x/autopilot/keeper/liquidstake.go +++ b/x/autopilot/keeper/liquidstake.go @@ -3,6 +3,7 @@ package keeper import ( "errors" "fmt" + "time" errorsmod "cosmossdk.io/errors" sdkmath "cosmossdk.io/math" @@ -15,6 +16,10 @@ import ( stakeibctypes "github.com/Stride-Labs/stride/v16/x/stakeibc/types" ) +const ( + 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( @@ -136,8 +141,8 @@ func (k Keeper) IBCTransferStToken( return err } - // Use the default transfer timeout of 10 minutes - timeoutTimestamp := uint64(ctx.BlockTime().UnixNano()) + transfertypes.DefaultRelativePacketTimeoutTimestamp + // Use a conservative timeout for the transfer + timeoutTimestamp := uint64(ctx.BlockTime().UnixNano() + LiquidStakeForwardTransferTimeout.Nanoseconds()) // Submit the transfer from the hashed address transferMsg := &transfertypes.MsgTransfer{ From a814f92d5ea7081b87b25fa669e5b4c9a4978961 Mon Sep 17 00:00:00 2001 From: sampocs Date: Tue, 9 Jan 2024 10:02:54 -0600 Subject: [PATCH 28/30] updated timeout comments --- x/autopilot/keeper/liquidstake.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/x/autopilot/keeper/liquidstake.go b/x/autopilot/keeper/liquidstake.go index 8601f05b4..edccd1e7c 100644 --- a/x/autopilot/keeper/liquidstake.go +++ b/x/autopilot/keeper/liquidstake.go @@ -17,6 +17,12 @@ import ( ) 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) ) @@ -141,7 +147,7 @@ func (k Keeper) IBCTransferStToken( return err } - // Use a conservative timeout for the transfer + // Use a long timeout for the transfer timeoutTimestamp := uint64(ctx.BlockTime().UnixNano() + LiquidStakeForwardTransferTimeout.Nanoseconds()) // Submit the transfer from the hashed address From 5042669e66800d392eda8a1ca6316aa2323fc84b Mon Sep 17 00:00:00 2001 From: sampocs Date: Tue, 9 Jan 2024 16:40:15 -0600 Subject: [PATCH 29/30] updated checkmes --- x/autopilot/keeper/liquidstake.go | 3 ++- x/stakeibc/keeper/msg_server_redeem_stake.go | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/x/autopilot/keeper/liquidstake.go b/x/autopilot/keeper/liquidstake.go index edccd1e7c..bd566491e 100644 --- a/x/autopilot/keeper/liquidstake.go +++ b/x/autopilot/keeper/liquidstake.go @@ -128,7 +128,8 @@ func (k Keeper) IBCTransferStToken( // to prevent impersonation at downstream zones // Note: The channel ID here is different than the one used in PFM // (we use the outbound channelID, they use the inbound channelID) - // DOUBLE CHECK ME that it shouldn't matter + // However, the only thing that matters is that the address is obfuscated, + // the additional fields beyond sender just provide more collision resistance hashedAddress, err := types.GenerateHashedAddress(channelId, transferMetadata.Sender) if err != nil { return err diff --git a/x/stakeibc/keeper/msg_server_redeem_stake.go b/x/stakeibc/keeper/msg_server_redeem_stake.go index cc615694c..9080f66ea 100644 --- a/x/stakeibc/keeper/msg_server_redeem_stake.go +++ b/x/stakeibc/keeper/msg_server_redeem_stake.go @@ -49,7 +49,6 @@ func (k msgServer) RedeemStake(goCtx context.Context, msg *types.MsgRedeemStake) // construct desired unstaking amount from host zone stDenom := types.StAssetDenomFromHostZoneDenom(hostZone.HostDenom) - // ASSUMPTION (CHECK ME): it doesn't matter that RRs can change _within_ an epoch (due to LSM) nativeAmount := sdk.NewDecFromInt(msg.Amount).Mul(hostZone.RedemptionRate).RoundInt() if nativeAmount.GT(hostZone.TotalDelegations) { From ba9330cf997dccba7c8f65e430201c5d718bf53f Mon Sep 17 00:00:00 2001 From: sampocs Date: Wed, 10 Jan 2024 22:10:11 -0600 Subject: [PATCH 30/30] fixed unit test after merge --- x/autopilot/keeper/liquidstake_test.go | 4 +++- x/autopilot/types/parser.go | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/x/autopilot/keeper/liquidstake_test.go b/x/autopilot/keeper/liquidstake_test.go index 7eab137e0..31338c42f 100644 --- a/x/autopilot/keeper/liquidstake_test.go +++ b/x/autopilot/keeper/liquidstake_test.go @@ -170,7 +170,8 @@ func (s *KeeperTestSuite) TestTryLiquidStake() { liquidStakeDenom: Atom, liquidStakeAmount: stakeAmount.String(), autopilotMetadata: types.StakeibcPacketMetadata{ - IbcReceiver: forwardRecipientOnHost, + StrideAddress: liquidStakerOnStride.String(), // fallback address + IbcReceiver: forwardRecipientOnHost, }, expectedForwardChannelId: ibctesting.FirstChannelID, // default for host zone }, @@ -182,6 +183,7 @@ func (s *KeeperTestSuite) TestTryLiquidStake() { liquidStakeDenom: Atom, liquidStakeAmount: stakeAmount.String(), autopilotMetadata: types.StakeibcPacketMetadata{ + StrideAddress: liquidStakerOnStride.String(), // fallback address IbcReceiver: forwardRecipientOnHost, TransferChannel: "channel-0", // custom channel (different than host channel below) }, diff --git a/x/autopilot/types/parser.go b/x/autopilot/types/parser.go index cae87861b..c7770c5ef 100644 --- a/x/autopilot/types/parser.go +++ b/x/autopilot/types/parser.go @@ -12,7 +12,8 @@ const RedeemStake = "RedeemStake" // Packet metadata info specific to Stakeibc (e.g. 1-click liquid staking) type StakeibcPacketMetadata struct { - Action string `json:"action"` + Action string `json:"action"` + // TODO [cleanup]: Rename to FallbackAddress StrideAddress string IbcReceiver string `json:"ibc_receiver,omitempty"` TransferChannel string `json:"transfer_channel,omitempty"`