Skip to content

Commit

Permalink
fix issue with negative available per validator (#316)
Browse files Browse the repository at this point in the history 
* 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 <[email protected]>
Co-authored-by: Alex Johnson <[email protected]>
  • Loading branch information
@ajansari95
3 people authored Feb 23, 2023
1 parent e19a59a commit 5e295f8
Show file tree
Hide file tree
Showing 5 changed files with 320 additions and 110 deletions.
28 changes: 28 additions & 0 deletions app/upgrades.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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)
}
}
64 changes: 64 additions & 0 deletions app/upgrades_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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))

}
25 changes: 17 additions & 8 deletions x/interchainstaking/keeper/ibc_packet_handlers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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)
Expand Down
Loading

0 comments on commit 5e295f8

Please sign in to comment.