From 5e295f8322988c19c7dc4852aecd95df8b21d4a3 Mon Sep 17 00:00:00 2001 From: Joe Bowman Date: Thu, 23 Feb 2023 05:35:39 +0000 Subject: [PATCH] fix issue with negative available per validator (#316) * fix issue with negative available per validator * upgrade handler to remove invalid redelegationEnd * test for upgradehandler v1.4.0-rc8 * table based test for v45 and v46 callbacks * fix:lint * removed debug log lines --------- Co-authored-by: Ajaz Ahmed Ansari Co-authored-by: Alex Johnson --- app/upgrades.go | 28 ++ app/upgrades_test.go | 64 ++++ .../keeper/ibc_packet_handlers.go | 25 +- .../keeper/ibc_packet_handlers_test.go | 305 ++++++++++++------ x/interchainstaking/keeper/redemptions.go | 8 +- 5 files changed, 320 insertions(+), 110 deletions(-) diff --git a/app/upgrades.go b/app/upgrades.go index ca62a8ef8..5ad872505 100644 --- a/app/upgrades.go +++ b/app/upgrades.go @@ -26,6 +26,7 @@ const ( v010400UpgradeName = "v1.4.0" v010400rc6UpgradeName = "v1.4.0-rc6" v010400rc7UpgradeName = "v1.4.0-rc7" + v010400rc8UpgradeName = "v1.4.0-rc8" ) func isTest(ctx sdk.Context) bool { @@ -50,6 +51,7 @@ func setUpgradeHandlers(app *Quicksilver) { app.UpgradeKeeper.SetUpgradeHandler(v010400UpgradeName, v010400UpgradeHandler(app)) app.UpgradeKeeper.SetUpgradeHandler(v010400rc6UpgradeName, v010400rc6UpgradeHandler(app)) app.UpgradeKeeper.SetUpgradeHandler(v010400rc7UpgradeName, noOpHandler(app)) + app.UpgradeKeeper.SetUpgradeHandler(v010400rc8UpgradeName, v010400rc8UpgradeHandler(app)) // When a planned update height is reached, the old binary will panic // writing on disk the height and name of the update that triggered it @@ -198,3 +200,29 @@ func v010400rc6UpgradeHandler(app *Quicksilver) upgradetypes.UpgradeHandler { return app.mm.RunMigrations(ctx, app.configurator, fromVM) } } + +func v010400rc8UpgradeHandler(app *Quicksilver) upgradetypes.UpgradeHandler { + return func(ctx sdk.Context, _ upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) { + // remove expired failed redelegation records + app.InterchainstakingKeeper.IterateZones(ctx, func(index int64, zone icstypes.Zone) (stop bool) { + app.InterchainstakingKeeper.IterateAllDelegations(ctx, &zone, func(delegation icstypes.Delegation) (stop bool) { + if delegation.RedelegationEnd < 0 { + delegation.RedelegationEnd = 0 + app.InterchainstakingKeeper.SetDelegation(ctx, &zone, delegation) + } + return false + }) + return false + }) + + app.InterchainstakingKeeper.IterateRedelegationRecords(ctx, func(_ int64, key []byte, record icstypes.RedelegationRecord) (stop bool) { + if record.CompletionTime.Unix() <= 0 { + app.InterchainstakingKeeper.Logger(ctx).Info("Removing delegation record", "chainid", record.ChainId, "source", record.Source, "destination", record.Destination, "epoch", record.EpochNumber) + app.InterchainstakingKeeper.DeleteRedelegationRecord(ctx, record.ChainId, record.Source, record.Destination, record.EpochNumber) + } + return false + }) + + return app.mm.RunMigrations(ctx, app.configurator, fromVM) + } +} diff --git a/app/upgrades_test.go b/app/upgrades_test.go index 63eeeab13..b1035662c 100644 --- a/app/upgrades_test.go +++ b/app/upgrades_test.go @@ -165,6 +165,7 @@ func (suite *AppTestSuite) initTestZone() { DelegationAddress: "juno1z89utvygweg5l56fsk8ak7t6hh88fd0azcjpz5", Height: 10, ValidatorAddress: "junovaloper185hgkqs8q8ysnc8cvkgd8j2knnq2m0ah6ae73gntv9ampgwpmrxqlfzywn", + RedelegationEnd: -62135596800, } suite.GetQuicksilverApp(suite.chainA).InterchainstakingKeeper.SetDelegation(suite.chainA.GetContext(), &zone, delRecord) @@ -280,3 +281,66 @@ func (s *AppTestSuite) TestV010400rc6UpgradeHandler() { s.Require().Equal(0, len(redelegations)) } + +func (s *AppTestSuite) TestV010400rc8UpgradeHandler() { + app := s.GetQuicksilverApp(s.chainA) + handler := v010400rc8UpgradeHandler(app) + ctx := s.chainA.GetContext() + + zone, _ := app.InterchainstakingKeeper.GetZone(ctx, "osmosis-1") + osmodels := []icstypes.Delegation{{Amount: sdk.NewCoin(zone.BaseDenom, sdk.NewInt(17000)), + DelegationAddress: "osmo1t7egva48prqmzl59x5ngv4zx0dtrwewc9m7z44", + Height: 10, + ValidatorAddress: "osmovaloper1clpqr4nrk4khgkxj78fcwwh6dl3uw4ep88n0y4", + RedelegationEnd: -62135596800, + }, { + Amount: sdk.NewCoin(zone.BaseDenom, sdk.NewInt(17005)), + DelegationAddress: "osmo1t7egva48prqmzl59x5ngv4zx0dtrwewc9m7z44", + Height: 11, + ValidatorAddress: "osmovaloper1hjct6q7npsspsg3dgvzk3sdf89spmlpf6t4agt", + RedelegationEnd: 0, + }, + } + + for _, dels := range osmodels { + app.InterchainstakingKeeper.SetDelegation(ctx, &zone, dels) + } + + zone, _ = app.InterchainstakingKeeper.GetZone(ctx, "uni-5") + + var negRedelEndsBefore []icstypes.Delegation + app.InterchainstakingKeeper.IterateZones(ctx, func(index int64, zone icstypes.Zone) (stop bool) { + app.InterchainstakingKeeper.IterateAllDelegations(ctx, &zone, func(delegation icstypes.Delegation) (stop bool) { + if delegation.RedelegationEnd < 0 { + negRedelEndsBefore = append(negRedelEndsBefore, delegation) + } + return false + }) + return false + }) + + s.Require().Equal(2, len(negRedelEndsBefore)) + + redelegations := app.InterchainstakingKeeper.ZoneRedelegationRecords(ctx, "osmosis-1") + s.Require().Equal(1, len(redelegations)) + + _, err := handler(ctx, types.Plan{}, app.mm.GetVersionMap()) + s.Require().NoError(err) + + var negRedelEndsAfter []icstypes.Delegation + + app.InterchainstakingKeeper.IterateZones(ctx, func(index int64, zone icstypes.Zone) (stop bool) { + app.InterchainstakingKeeper.IterateAllDelegations(ctx, &zone, func(delegation icstypes.Delegation) (stop bool) { + if delegation.RedelegationEnd < 0 { + negRedelEndsAfter = append(negRedelEndsAfter, delegation) + } + return false + }) + return false + }) + + s.Require().Equal(0, len(negRedelEndsAfter)) + redelegations = app.InterchainstakingKeeper.ZoneRedelegationRecords(ctx, "osmosis-1") + s.Require().Equal(0, len(redelegations)) + +} diff --git a/x/interchainstaking/keeper/ibc_packet_handlers.go b/x/interchainstaking/keeper/ibc_packet_handlers.go index fb740052a..a23c987da 100644 --- a/x/interchainstaking/keeper/ibc_packet_handlers.go +++ b/x/interchainstaking/keeper/ibc_packet_handlers.go @@ -117,7 +117,9 @@ func (k *Keeper) HandleAcknowledgement(ctx sdk.Context, packet channeltypes.Pack //nolint:staticcheck // SA1019 ignore this! var msgData *sdk.MsgData var msgResponse []byte + var msgResponseType string if len(txMsgData.MsgResponses) > 0 { + msgResponseType = txMsgData.MsgResponses[msgIndex].GetTypeUrl() msgResponse = txMsgData.MsgResponses[msgIndex].GetValue() } else if len(txMsgData.Data) > 0 { msgData = txMsgData.Data[msgIndex] @@ -143,7 +145,7 @@ func (k *Keeper) HandleAcknowledgement(ctx sdk.Context, packet channeltypes.Pack } response := lsmstakingtypes.MsgRedeemTokensforSharesResponse{} - if msgResponse != nil { + if msgResponseType != "" { err = proto.Unmarshal(msgResponse, &response) if err != nil { k.Logger(ctx).Error("unable to unpack MsgRedeemTokensforShares response", "error", err) @@ -168,7 +170,7 @@ func (k *Keeper) HandleAcknowledgement(ctx sdk.Context, packet channeltypes.Pack return nil } response := lsmstakingtypes.MsgTokenizeSharesResponse{} - if msgResponse != nil { + if msgResponseType != "" { err = proto.Unmarshal(msgResponse, &response) if err != nil { k.Logger(ctx).Error("unable to unpack MsgTokenizeShares response", "error", err) @@ -192,7 +194,7 @@ func (k *Keeper) HandleAcknowledgement(ctx sdk.Context, packet channeltypes.Pack return nil } response := stakingtypes.MsgDelegateResponse{} - if msgResponse != nil { + if msgResponseType != "" { err = proto.Unmarshal(msgResponse, &response) if err != nil { k.Logger(ctx).Error("unable to unpack MsgDelegate response", "error", err) @@ -214,7 +216,7 @@ func (k *Keeper) HandleAcknowledgement(ctx sdk.Context, packet channeltypes.Pack case "/cosmos.staking.v1beta1.MsgBeginRedelegate": if success { response := stakingtypes.MsgBeginRedelegateResponse{} - if msgResponse != nil { + if msgResponseType != "" { err = proto.Unmarshal(msgResponse, &response) if err != nil { k.Logger(ctx).Error("unable to unpack MsgBeginRedelegate response", "error", err) @@ -240,7 +242,7 @@ func (k *Keeper) HandleAcknowledgement(ctx sdk.Context, packet channeltypes.Pack case "/cosmos.staking.v1beta1.MsgUndelegate": if success { response := stakingtypes.MsgUndelegateResponse{} - if msgResponse != nil { + if msgResponseType != "" { err = proto.Unmarshal(msgResponse, &response) if err != nil { k.Logger(ctx).Error("unable to unpack MsgUndelegate response", "error", err) @@ -270,7 +272,7 @@ func (k *Keeper) HandleAcknowledgement(ctx sdk.Context, packet channeltypes.Pack return nil } response := banktypes.MsgSendResponse{} - if msgResponse != nil { + if msgResponseType != "" { err = proto.Unmarshal(msgResponse, &response) if err != nil { k.Logger(ctx).Error("unable to unpack MsgSend response", "error", err) @@ -294,7 +296,7 @@ func (k *Keeper) HandleAcknowledgement(ctx sdk.Context, packet channeltypes.Pack return nil } response := distrtypes.MsgSetWithdrawAddressResponse{} - if msgResponse != nil { + if msgResponseType != "" { err = proto.Unmarshal(msgResponse, &response) if err != nil { k.Logger(ctx).Error("unable to unpack MsgSetWithdrawAddress response", "error", err) @@ -317,7 +319,7 @@ func (k *Keeper) HandleAcknowledgement(ctx sdk.Context, packet channeltypes.Pack return nil } response := ibctransfertypes.MsgTransferResponse{} - if msgResponse != nil { + if msgResponseType != "" { err = proto.Unmarshal(msgResponse, &response) if err != nil { k.Logger(ctx).Error("unable to unpack MsgTransfer response", "error", err) @@ -573,6 +575,9 @@ func (k *Keeper) HandleTokenizedShares(ctx sdk.Context, msg sdk.Msg, sharesAmoun } func (k *Keeper) HandleBeginRedelegate(ctx sdk.Context, msg sdk.Msg, completion time.Time, memo string) error { + if completion.IsZero() { + return errors.New("invalid zero nil completion time") + } parts := strings.Split(memo, "/") if len(parts) != 2 || parts[0] != "rebalance" { return errors.New("unexpected epoch rebalance memo format") @@ -633,6 +638,10 @@ func (k *Keeper) HandleFailedBeginRedelegate(ctx sdk.Context, msg sdk.Msg, memo } func (k *Keeper) HandleUndelegate(ctx sdk.Context, msg sdk.Msg, completion time.Time, memo string) error { + if completion.IsZero() { + return errors.New("invalid zero nil completion time") + } + k.Logger(ctx).Info("Received MsgUndelegate acknowledgement") // first, type assertion. we should have stakingtypes.MsgUndelegate undelegateMsg, ok := msg.(*stakingtypes.MsgUndelegate) diff --git a/x/interchainstaking/keeper/ibc_packet_handlers_test.go b/x/interchainstaking/keeper/ibc_packet_handlers_test.go index 78e232d77..ee045dc70 100644 --- a/x/interchainstaking/keeper/ibc_packet_handlers_test.go +++ b/x/interchainstaking/keeper/ibc_packet_handlers_test.go @@ -8,12 +8,12 @@ import ( "time" sdkmath "cosmossdk.io/math" - codectypes "github.com/cosmos/cosmos-sdk/codec/types" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/bech32" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" + distrtypes "github.com/cosmos/cosmos-sdk/x/distribution/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" icatypes "github.com/cosmos/ibc-go/v5/modules/apps/27-interchain-accounts/types" ibctransfertypes "github.com/cosmos/ibc-go/v5/modules/apps/transfer/types" @@ -300,13 +300,13 @@ func (s *KeeperTestSuite) TestHandleQueuedUnbondings() { EpochNumber: 1, Source: zone.Validators[3].ValoperAddress, Destination: zone.Validators[0].ValoperAddress, - Amount: 50000, + Amount: 500000, CompletionTime: time.Now().Add(time.Hour), }, } }, expectTransition: []bool{false}, - expectError: true, + expectError: false, }, { name: "mixed - locked tokens but both succeed (previously failed)", @@ -1485,136 +1485,239 @@ func (s *KeeperTestSuite) TestRebalanceDueToDelegationChange() { } func (s *KeeperTestSuite) Test_v045Callback() { - s.SetupTest() - s.setupTestZones() - app := s.GetQuicksilverApp(s.chainA) - ctx := s.chainA.GetContext() - app.BankKeeper.MintCoins(ctx, icstypes.ModuleName, sdk.NewCoins(sdk.NewCoin("denom", sdk.NewInt(100)))) + tests := []struct { + name string + setStatements func(ctx sdk.Context, app *app.Quicksilver) ([]sdk.Msg, []byte) + assertStatements func(ctx sdk.Context, app *app.Quicksilver) bool + }{{ + name: "msg response with some data", + setStatements: func(ctx sdk.Context, app *app.Quicksilver) ([]sdk.Msg, []byte) { + app.BankKeeper.MintCoins(ctx, icstypes.ModuleName, sdk.NewCoins(sdk.NewCoin("denom", sdk.NewInt(100)))) + sender := utils.GenerateAccAddressForTest() + senderAddr, _ := sdk.Bech32ifyAddressBytes("cosmos", sender) + transferMsg := ibctransfertypes.MsgTransfer{ + SourcePort: "transfer", + SourceChannel: "channel-0", + Token: sdk.NewCoin("denom", sdk.NewInt(100)), + Sender: senderAddr, + Receiver: app.AccountKeeper.GetModuleAddress(icstypes.ModuleName).String(), + } + response := ibctransfertypes.MsgTransferResponse{ + Sequence: 1, + } - sender := utils.GenerateAccAddressForTest() - senderAddr, _ := sdk.Bech32ifyAddressBytes("cosmos", sender) + respBytes := icatypes.ModuleCdc.MustMarshal(&response) + return []sdk.Msg{&transferMsg}, respBytes + }, + assertStatements: func(ctx sdk.Context, app *app.Quicksilver) bool { + txMacc := app.AccountKeeper.GetModuleAddress(icstypes.ModuleName) + feeMacc := app.AccountKeeper.GetModuleAddress(authtypes.FeeCollectorName) + txMaccBalance2 := app.BankKeeper.GetAllBalances(ctx, txMacc) + feeMaccBalance2 := app.BankKeeper.GetAllBalances(ctx, feeMacc) - txMacc := app.AccountKeeper.GetModuleAddress(icstypes.ModuleName) - feeMacc := app.AccountKeeper.GetModuleAddress(authtypes.FeeCollectorName) - txMaccBalance := app.BankKeeper.GetAllBalances(ctx, txMacc) - feeMaccBalance := app.BankKeeper.GetAllBalances(ctx, feeMacc) + // assert that ics module balance is now 100denom less than before HandleMsgTransfer() - transferMsg := ibctransfertypes.MsgTransfer{ - SourcePort: "transfer", - SourceChannel: "channel-0", - Token: sdk.NewCoin("denom", sdk.NewInt(100)), - Sender: senderAddr, - Receiver: app.AccountKeeper.GetModuleAddress(icstypes.ModuleName).String(), - } + if txMaccBalance2.AmountOf("denom").Equal(sdk.ZeroInt()) && feeMaccBalance2.AmountOf("denom").Equal(sdk.NewInt(100)) { + return true + } + return false - response := ibctransfertypes.MsgTransferResponse{ - Sequence: 1, - } + }, + }, + { + name: "msg response with nil data", + setStatements: func(ctx sdk.Context, app *app.Quicksilver) ([]sdk.Msg, []byte) { + zone, found := app.InterchainstakingKeeper.GetZone(ctx, s.chainB.ChainID) + if !found { + s.Fail("unable to retrieve zone for test") + } - txMsgData := &sdk.TxMsgData{ - Data: []*sdk.MsgData{{MsgType: "/bob", Data: icatypes.ModuleCdc.MustMarshal(&response)}}, - MsgResponses: []*codectypes.Any{}, - } + msgSetWithdrawAddress := distrtypes.MsgSetWithdrawAddress{ + DelegatorAddress: zone.PerformanceAddress.Address, + WithdrawAddress: zone.WithdrawalAddress.Address, + } - ackData := icatypes.ModuleCdc.MustMarshal(txMsgData) + response := distrtypes.MsgSetWithdrawAddressResponse{} - acknowledgement := channeltypes.Acknowledgement{ - Response: &channeltypes.Acknowledgement_Result{ - Result: ackData, + respBytes := icatypes.ModuleCdc.MustMarshal(&response) + return []sdk.Msg{&msgSetWithdrawAddress}, respBytes + }, + assertStatements: func(ctx sdk.Context, app *app.Quicksilver) bool { + zone, found := app.InterchainstakingKeeper.GetZone(ctx, s.chainB.ChainID) + if !found { + s.Fail("unable to retrieve zone for test") + } + // assert that withdraw address is set + if zone.WithdrawalAddress.Address == zone.PerformanceAddress.WithdrawalAddress { + return true + } + return false + + }, }, } - pdBytes, err := icatypes.SerializeCosmosTx(icatypes.ModuleCdc, []sdk.Msg{&transferMsg}) - s.Require().NoError(err) - packetData := icatypes.InterchainAccountPacketData{ - Type: icatypes.EXECUTE_TX, - Data: pdBytes, - Memo: "test_acknowledgement", - } + for _, test := range tests { + s.Run(test.name, func() { + s.SetupTest() + s.setupTestZones() - packetBytes, err := icatypes.ModuleCdc.MarshalJSON(&packetData) - s.Require().NoError(err) - packet := channeltypes.Packet{ - Data: packetBytes, - } + app := s.GetQuicksilverApp(s.chainA) + ctx := s.chainA.GetContext() - s.Require().NoError(app.InterchainstakingKeeper.HandleAcknowledgement(ctx, packet, icatypes.ModuleCdc.MustMarshalJSON(&acknowledgement))) + msg, msgResponseBytes := test.setStatements(ctx, app) - txMaccBalance2 := app.BankKeeper.GetAllBalances(ctx, txMacc) - feeMaccBalance2 := app.BankKeeper.GetAllBalances(ctx, feeMacc) + txMsgData := &sdk.TxMsgData{ + Data: []*sdk.MsgData{{MsgType: "/bob", Data: msgResponseBytes}}, + MsgResponses: []*codectypes.Any{}, + } - // assert that ics module balance is now 100denom less than before HandleMsgTransfer() - s.Require().Equal(txMaccBalance.AmountOf("denom").Sub(txMaccBalance2.AmountOf("denom")), sdk.NewInt(100)) - // assert that fee collector module balance is now 100denom more than before HandleMsgTransfer() - s.Require().Equal(feeMaccBalance2.AmountOf("denom").Sub(feeMaccBalance.AmountOf("denom")), sdk.NewInt(100)) + ackData := icatypes.ModuleCdc.MustMarshal(txMsgData) + + acknowledgement := channeltypes.Acknowledgement{ + Response: &channeltypes.Acknowledgement_Result{ + Result: ackData, + }, + } + + pdBytes, err := icatypes.SerializeCosmosTx(icatypes.ModuleCdc, msg) + s.Require().NoError(err) + packetData := icatypes.InterchainAccountPacketData{ + Type: icatypes.EXECUTE_TX, + Data: pdBytes, + Memo: "test_acknowledgement", + } + + packetBytes, err := icatypes.ModuleCdc.MarshalJSON(&packetData) + s.Require().NoError(err) + packet := channeltypes.Packet{ + Data: packetBytes, + } + + s.Require().NoError(app.InterchainstakingKeeper.HandleAcknowledgement(ctx, packet, icatypes.ModuleCdc.MustMarshalJSON(&acknowledgement))) + + s.Require().True(test.assertStatements(ctx, app)) + + }) + } } func (s *KeeperTestSuite) Test_v046Callback() { - s.SetupTest() - s.setupTestZones() - app := s.GetQuicksilverApp(s.chainA) - ctx := s.chainA.GetContext() - app.BankKeeper.MintCoins(ctx, icstypes.ModuleName, sdk.NewCoins(sdk.NewCoin("denom", sdk.NewInt(100)))) + tests := []struct { + name string + setStatements func(ctx sdk.Context, app *app.Quicksilver) ([]sdk.Msg, *codectypes.Any) + assertStatements func(ctx sdk.Context, app *app.Quicksilver) bool + }{{ + name: "msg response with some data", + setStatements: func(ctx sdk.Context, app *app.Quicksilver) ([]sdk.Msg, *codectypes.Any) { + app.BankKeeper.MintCoins(ctx, icstypes.ModuleName, sdk.NewCoins(sdk.NewCoin("denom", sdk.NewInt(100)))) + sender := utils.GenerateAccAddressForTest() + senderAddr, _ := sdk.Bech32ifyAddressBytes("cosmos", sender) + transferMsg := ibctransfertypes.MsgTransfer{ + SourcePort: "transfer", + SourceChannel: "channel-0", + Token: sdk.NewCoin("denom", sdk.NewInt(100)), + Sender: senderAddr, + Receiver: app.AccountKeeper.GetModuleAddress(icstypes.ModuleName).String(), + } + response := ibctransfertypes.MsgTransferResponse{ + Sequence: 1, + } - sender := utils.GenerateAccAddressForTest() - senderAddr, _ := sdk.Bech32ifyAddressBytes("cosmos", sender) + anyresponse, _ := codectypes.NewAnyWithValue(&response) + return []sdk.Msg{&transferMsg}, anyresponse + }, + assertStatements: func(ctx sdk.Context, app *app.Quicksilver) bool { + txMacc := app.AccountKeeper.GetModuleAddress(icstypes.ModuleName) + feeMacc := app.AccountKeeper.GetModuleAddress(authtypes.FeeCollectorName) + txMaccBalance2 := app.BankKeeper.GetAllBalances(ctx, txMacc) + feeMaccBalance2 := app.BankKeeper.GetAllBalances(ctx, feeMacc) - txMacc := app.AccountKeeper.GetModuleAddress(icstypes.ModuleName) - feeMacc := app.AccountKeeper.GetModuleAddress(authtypes.FeeCollectorName) - txMaccBalance := app.BankKeeper.GetAllBalances(ctx, txMacc) - feeMaccBalance := app.BankKeeper.GetAllBalances(ctx, feeMacc) + // assert that ics module balance is now 100denom less than before HandleMsgTransfer() - transferMsg := ibctransfertypes.MsgTransfer{ - SourcePort: "transfer", - SourceChannel: "channel-0", - Token: sdk.NewCoin("denom", sdk.NewInt(100)), - Sender: senderAddr, - Receiver: app.AccountKeeper.GetModuleAddress(icstypes.ModuleName).String(), - } + if txMaccBalance2.AmountOf("denom").Equal(sdk.ZeroInt()) && feeMaccBalance2.AmountOf("denom").Equal(sdk.NewInt(100)) { + return true + } + return false - response := ibctransfertypes.MsgTransferResponse{ - Sequence: 1, - } + }, + }, + { + name: "msg response with nil data", + setStatements: func(ctx sdk.Context, app *app.Quicksilver) ([]sdk.Msg, *codectypes.Any) { + zone, found := app.InterchainstakingKeeper.GetZone(ctx, s.chainB.ChainID) + if !found { + s.Fail("unable to retrieve zone for test") + } - anyResponse, err := codectypes.NewAnyWithValue(&response) - s.Require().NoError(err) + msgSetWithdrawAddress := distrtypes.MsgSetWithdrawAddress{ + DelegatorAddress: zone.PerformanceAddress.Address, + WithdrawAddress: zone.WithdrawalAddress.Address, + } - txMsgData := &sdk.TxMsgData{ - Data: []*sdk.MsgData{}, - MsgResponses: []*codectypes.Any{anyResponse}, - } + response := distrtypes.MsgSetWithdrawAddressResponse{} - ackData := icatypes.ModuleCdc.MustMarshal(txMsgData) + anyresponse, _ := codectypes.NewAnyWithValue(&response) + return []sdk.Msg{&msgSetWithdrawAddress}, anyresponse + }, + assertStatements: func(ctx sdk.Context, app *app.Quicksilver) bool { + zone, found := app.InterchainstakingKeeper.GetZone(ctx, s.chainB.ChainID) + if !found { + s.Fail("unable to retrieve zone for test") + } + // assert that withdraw address is set + if zone.WithdrawalAddress.Address == zone.PerformanceAddress.WithdrawalAddress { + return true + } + return false - acknowledgement := channeltypes.Acknowledgement{ - Response: &channeltypes.Acknowledgement_Result{ - Result: ackData, + }, }, } - pdBytes, err := icatypes.SerializeCosmosTx(icatypes.ModuleCdc, []sdk.Msg{&transferMsg}) - s.Require().NoError(err) - packetData := icatypes.InterchainAccountPacketData{ - Type: icatypes.EXECUTE_TX, - Data: pdBytes, - Memo: "test_acknowledgement", - } + for _, test := range tests { + s.Run(test.name, func() { + s.SetupTest() + s.setupTestZones() - packetBytes, err := icatypes.ModuleCdc.MarshalJSON(&packetData) - s.Require().NoError(err) - packet := channeltypes.Packet{ - Data: packetBytes, - } + app := s.GetQuicksilverApp(s.chainA) + ctx := s.chainA.GetContext() - s.Require().NoError(app.InterchainstakingKeeper.HandleAcknowledgement(ctx, packet, icatypes.ModuleCdc.MustMarshalJSON(&acknowledgement))) + msg, anyResp := test.setStatements(ctx, app) - txMaccBalance2 := app.BankKeeper.GetAllBalances(ctx, txMacc) - feeMaccBalance2 := app.BankKeeper.GetAllBalances(ctx, feeMacc) + txMsgData := &sdk.TxMsgData{ + Data: []*sdk.MsgData{}, + MsgResponses: []*codectypes.Any{anyResp}, + } - // assert that ics module balance is now 100denom less than before HandleMsgTransfer() - s.Require().Equal(txMaccBalance.AmountOf("denom").Sub(txMaccBalance2.AmountOf("denom")), sdk.NewInt(100)) - // assert that fee collector module balance is now 100denom more than before HandleMsgTransfer() - s.Require().Equal(feeMaccBalance2.AmountOf("denom").Sub(feeMaccBalance.AmountOf("denom")), sdk.NewInt(100)) + ackData := icatypes.ModuleCdc.MustMarshal(txMsgData) + + acknowledgement := channeltypes.Acknowledgement{ + Response: &channeltypes.Acknowledgement_Result{ + Result: ackData, + }, + } + + pdBytes, err := icatypes.SerializeCosmosTx(icatypes.ModuleCdc, msg) + s.Require().NoError(err) + packetData := icatypes.InterchainAccountPacketData{ + Type: icatypes.EXECUTE_TX, + Data: pdBytes, + Memo: "test_acknowledgement", + } + + packetBytes, err := icatypes.ModuleCdc.MarshalJSON(&packetData) + s.Require().NoError(err) + packet := channeltypes.Packet{ + Data: packetBytes, + } + + s.Require().NoError(app.InterchainstakingKeeper.HandleAcknowledgement(ctx, packet, icatypes.ModuleCdc.MustMarshalJSON(&acknowledgement))) + + s.Require().True(test.assertStatements(ctx, app)) + + }) + } } diff --git a/x/interchainstaking/keeper/redemptions.go b/x/interchainstaking/keeper/redemptions.go index c0827595a..0b0813162 100644 --- a/x/interchainstaking/keeper/redemptions.go +++ b/x/interchainstaking/keeper/redemptions.go @@ -107,9 +107,13 @@ func (k *Keeper) GetUnlockedTokensForZone(ctx sdk.Context, zone *types.Zone) (ma thisAvailable, found := availablePerValidator[redelegation.Destination] if found { availablePerValidator[redelegation.Destination] = thisAvailable.Sub(sdk.NewInt(redelegation.Amount)) + if availablePerValidator[redelegation.Destination].LT(sdk.ZeroInt()) { + panic("negative available amount; unable to continue") + } + total = total.Sub(sdk.NewInt(redelegation.Amount)) } - total.Sub(sdk.NewInt(redelegation.Amount)) } + return availablePerValidator, total } @@ -304,6 +308,7 @@ func DetermineAllocationsForUndelegation(currentAllocations map[string]math.Int, for idx := range deltas { deltas[idx].Weight = deltas[idx].Weight.Sub(sdk.NewDecFromInt(maxValue)).Abs() sum = sum.Add(deltas[idx].Weight.TruncateInt().Abs()) + } // unequalSplit is the portion of input that should be distributed in attempt to make targets == 0 @@ -316,6 +321,7 @@ func DetermineAllocationsForUndelegation(currentAllocations map[string]math.Int, if !ok { availablePerValidator[deltas[idx].ValoperAddress] = sdk.ZeroInt() } + if allocation.TruncateInt().GT(availablePerValidator[deltas[idx].ValoperAddress]) { allocation = sdk.NewDecFromInt(availablePerValidator[deltas[idx].ValoperAddress]) availablePerValidator[deltas[idx].ValoperAddress] = sdk.ZeroInt()