From d3e0501c2aaa9a6946cf1e8d84a4da4288783b07 Mon Sep 17 00:00:00 2001 From: Aidan Salzmann Date: Tue, 28 Nov 2023 15:44:42 -0500 Subject: [PATCH 01/19] 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 d42ca579e3..de59481cb8 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 286eb15e2a..d58f914c2c 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 2299d7e9ce..604df7983f 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 e9e8ceff43..b9baa54deb 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 29a2aa8155..2f31be6717 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 53ffefc8ed..b3fce06774 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 7ff3710c75..97e09b1c28 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/19] 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 604df7983f..34b6677fdc 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/19] 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 f58b918f4d..37bceafed2 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/19] 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 b3fce06774..e4365c67ca 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 97e09b1c28..7e8b3d2b06 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 4679e77445..1f03d9e97e 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/19] 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 0000000000..3607d825d2 --- /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/19] 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 de59481cb8..d9b0c4c0b2 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 b9baa54deb..b74ccd94d7 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 3607d825d2..a4051dd67d 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 0000000000..b7504d71e4 --- /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/19] 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 54e04a1d02..2bfa4bf90d 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 2f31be6717..07420577f5 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 e4365c67ca..824cfe4072 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 a4051dd67d..e2c75e90b5 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 7e8b3d2b06..09df298987 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/19] 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 2bfa4bf90d..a4cde0375a 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 07420577f5..186a9ecdff 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 824cfe4072..6bfa24021f 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 e2c75e90b5..e2ba2aff81 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 09df298987..038b218bea 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/19] 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 a4cde0375a..43a4700054 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 186a9ecdff..af43938a18 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/19] 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 e2ba2aff81..3f87deb507 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 038b218bea..f3a0e66fd3 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/19] 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 d9b0c4c0b2..de59481cb8 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 43a4700054..934e4b037f 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 b74ccd94d7..b9baa54deb 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 af43938a18..2f5e2138b7 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 6bfa24021f..55006a9c9a 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 3f87deb507..76cc601d26 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 b7504d71e4..0000000000 --- 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 1f03d9e97e..382eaea99e 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/19] 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 2f5e2138b7..6cad0a810a 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 55006a9c9a..031ae0555b 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 76cc601d26..e5f1f12989 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 f3a0e66fd3..8ef77c8938 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 382eaea99e..8d83afd403 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/19] 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 de59481cb8..d9b0c4c0b2 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 b9baa54deb..a5ded2889d 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 0000000000..b7504d71e4 --- /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/19] 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 6cad0a810a..df92e11026 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/19] 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 df92e11026..36d354eeed 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 e5f1f12989..44e8eaf67b 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 2bf291f924a1587cbb8092237f4602e9d8e3830f Mon Sep 17 00:00:00 2001 From: sampocs Date: Mon, 8 Jan 2024 22:03:42 -0600 Subject: [PATCH 16/19] 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 36d354eeed..da88605f4a 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 17/19] 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 8601f05b4a..edccd1e7ce 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 18/19] 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 edccd1e7ce..bd566491eb 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 cc615694c4..9080f66ead 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 e5559d1f1bd16493866b225cea539a2345410d43 Mon Sep 17 00:00:00 2001 From: sampocs Date: Tue, 9 Jan 2024 21:57:14 -0600 Subject: [PATCH 19/19] autopilot hashed sender option (3) (#1046) --- x/autopilot/keeper/liquidstake.go | 26 +------------------------- x/autopilot/module_ibc.go | 22 ++++++++++++++++++++-- 2 files changed, 21 insertions(+), 27 deletions(-) diff --git a/x/autopilot/keeper/liquidstake.go b/x/autopilot/keeper/liquidstake.go index bd566491eb..ee1cec54dd 100644 --- a/x/autopilot/keeper/liquidstake.go +++ b/x/autopilot/keeper/liquidstake.go @@ -124,30 +124,6 @@ func (k Keeper) IBCTransferStToken( channelId = hostZone.TransferChannelId } - // 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) - // 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 - } - - // First we need to bank send to the hashed address - 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 a long timeout for the transfer timeoutTimestamp := uint64(ctx.BlockTime().UnixNano() + LiquidStakeForwardTransferTimeout.Nanoseconds()) @@ -156,7 +132,7 @@ func (k Keeper) IBCTransferStToken( SourcePort: transfertypes.PortID, SourceChannel: channelId, Token: stToken, - Sender: hashedAddress, + Sender: transferMetadata.Receiver, 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 9e54ddf567..d0fe5c72b5 100644 --- a/x/autopilot/module_ibc.go +++ b/x/autopilot/module_ibc.go @@ -171,9 +171,27 @@ func (im IBCModule) OnRecvPacket( //// At this point, we are officially dealing with an autopilot packet - // Modify the packet data by replacing the JSON metadata field with a receiver address - // to allow the packet to continue down the stack + // Update the reciever in the packet data so that we can pass the packet down the stack + // (since the "receiver" may have technically been a full JSON memo) tokenPacketData.Receiver = autopilotMetadata.Receiver + + // For autopilot liquid stake and forward, we'll override the receiver with a hashed address + // The hashed address will also be the sender of the outbound transfer + // This is to prevent impersonation at downstream zones + // We can identify the forwarding step by whether there's a non-empty IBC receiver field + if routingInfo, ok := autopilotMetadata.RoutingInfo.(types.StakeibcPacketMetadata); ok && + routingInfo.Action == types.LiquidStake && routingInfo.IbcReceiver != "" { + + var err error + hashedReceiver, err := types.GenerateHashedAddress(packet.DestinationChannel, tokenPacketData.Sender) + if err != nil { + return channeltypes.NewErrorAcknowledgement(err) + } + tokenPacketData.Receiver = hashedReceiver + } + + // Now that the receiver's been updated on the transfer metadata, + // modify the original packet so that we can send it down the stack bz, err := transfertypes.ModuleCdc.MarshalJSON(&tokenPacketData) if err != nil { return channeltypes.NewErrorAcknowledgement(err)