Skip to content

Commit

Permalink
fix: defensive checks in UpdateWithdrawalRecordsForSlash; add tests; f…
Browse files Browse the repository at this point in the history
…ixes #607
  • Loading branch information
Joe Bowman committed Dec 4, 2023
1 parent 9a28c99 commit 83cd0ec
Show file tree
Hide file tree
Showing 3 changed files with 300 additions and 23 deletions.
23 changes: 0 additions & 23 deletions x/interchainstaking/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -416,29 +416,6 @@ func (k *Keeper) SetValidatorForZone(ctx sdk.Context, zone *types.Zone, data []b
return nil
}

func (k *Keeper) UpdateWithdrawalRecordsForSlash(ctx sdk.Context, zone *types.Zone, valoper string, delta sdk.Dec) error {
var err error
k.IterateZoneStatusWithdrawalRecords(ctx, zone.ChainId, types.WithdrawStatusUnbond, func(_ int64, record types.WithdrawalRecord) bool {
recordSubAmount := sdkmath.ZeroInt()
distr := record.Distribution
for _, d := range distr {
if d.Valoper != valoper {
continue
}
newAmount := sdk.NewDec(int64(d.Amount)).Quo(delta).TruncateInt()
thisSubAmount := sdkmath.NewInt(int64(d.Amount)).Sub(newAmount)
recordSubAmount = recordSubAmount.Add(thisSubAmount)
d.Amount = newAmount.Uint64()
k.Logger(ctx).Info("Updated withdrawal record due to slashing", "valoper", valoper, "old_amount", d.Amount, "new_amount", newAmount.Int64(), "sub_amount", thisSubAmount.Int64())
}
record.Distribution = distr
record.Amount = record.Amount.Sub(sdk.NewCoin(zone.BaseDenom, recordSubAmount))
k.SetWithdrawalRecord(ctx, record)
return false
})
return err
}

func (k *Keeper) depositInterval(ctx sdk.Context) zoneItrFn {
return func(index int64, zone *types.Zone) (stop bool) {
if zone.DepositAddress != nil {
Expand Down
38 changes: 38 additions & 0 deletions x/interchainstaking/keeper/withdrawal_record.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ package keeper
import (
"encoding/binary"
"encoding/hex"
"fmt"
"time"

sdkmath "cosmossdk.io/math"
"github.com/cosmos/cosmos-sdk/store/prefix"
sdk "github.com/cosmos/cosmos-sdk/types"

Expand Down Expand Up @@ -222,3 +224,39 @@ func (k *Keeper) AllZoneUnbondingRecords(ctx sdk.Context, chainID string) []type
})
return records
}

func (k *Keeper) UpdateWithdrawalRecordsForSlash(ctx sdk.Context, zone *types.Zone, valoper string, delta sdk.Dec) error {
var err error
k.IterateZoneStatusWithdrawalRecords(ctx, zone.ChainId, types.WithdrawStatusUnbond, func(_ int64, record types.WithdrawalRecord) bool {
recordSubAmount := sdkmath.ZeroInt()
distr := record.Distribution
for _, d := range distr {
if d.Valoper != valoper {
continue
}
distAmount := sdk.NewDec(int64(d.Amount))
if distAmount.IsNegative() {
err = fmt.Errorf("distAmount cannot be negative; suspected overflow")
return true
}

newAmount := distAmount.Quo(delta).TruncateInt()
thisSubAmount := distAmount.TruncateInt().Sub(newAmount)
recordSubAmount = recordSubAmount.Add(thisSubAmount)
d.Amount = newAmount.Uint64()
k.Logger(ctx).Info("Updated withdrawal record due to slashing", "valoper", valoper, "old_amount", d.Amount, "new_amount", newAmount.Int64(), "sub_amount", thisSubAmount.Int64())
}
record.Distribution = distr
subAmount := sdk.NewCoins(sdk.NewCoin(zone.BaseDenom, recordSubAmount))
fmt.Println(subAmount)
fmt.Println(record.Amount)
if !record.Amount.IsAllGT(subAmount) {
err = fmt.Errorf("deductedTotal cannot contain negative coins; suspected overflow")
return true
}
record.Amount = record.Amount.Sub(subAmount...)
k.SetWithdrawalRecord(ctx, record)
return false
})
return err
}
262 changes: 262 additions & 0 deletions x/interchainstaking/keeper/withdrawal_record_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,262 @@
package keeper_test

import (
"cosmossdk.io/math"
sdk "github.com/cosmos/cosmos-sdk/types"
icskeeper "github.com/quicksilver-zone/quicksilver/x/interchainstaking/keeper"
"github.com/quicksilver-zone/quicksilver/x/interchainstaking/types"
)

func (suite *KeeperTestSuite) TestUpdateWithdrawalRecordsForSlash() {
tcs := []struct {
Name string
InitialRecords func(ctx sdk.Context, keeper *icskeeper.Keeper)
ExpectedRecords func(ctx sdk.Context, keeper *icskeeper.Keeper) (out []types.WithdrawalRecord)
Validator func(validators []string) string
Delta sdk.Dec
ExpectError bool
}{
{
Name: "single 5% slashing",
InitialRecords: func(ctx sdk.Context, keeper *icskeeper.Keeper) {
zone, _ := keeper.GetZone(ctx, suite.chainB.ChainID)
validators := keeper.GetValidatorAddresses(ctx, zone.ChainId)
withdrawalRecord := types.WithdrawalRecord{
ChainId: zone.ChainId,
Delegator: zone.DelegationAddress.Address,
Recipient: "cosmos1v4gek4mld0k5yhpe0fsln4takg558cdpyexv2rxr3dh45f2fqrgsw52m97",
Distribution: []*types.Distribution{
{Amount: 10000, Valoper: validators[0]},
{Amount: 10000, Valoper: validators[1]},
{Amount: 10000, Valoper: validators[2]},
{Amount: 10000, Valoper: validators[3]},
},
Amount: sdk.NewCoins(sdk.NewCoin(zone.BaseDenom, math.NewInt(40000))),
BurnAmount: sdk.NewCoin(zone.LocalDenom, math.NewInt(32356)),
Txhash: "3BE21C1057ABBFBC44BE8993D2A4701C751507FF9901AA110B5993BA070C176B",
Status: types.WithdrawStatusUnbond,
}
keeper.SetWithdrawalRecord(ctx, withdrawalRecord)
},
ExpectedRecords: func(ctx sdk.Context, keeper *icskeeper.Keeper) (out []types.WithdrawalRecord) {
zone, _ := keeper.GetZone(ctx, suite.chainB.ChainID)
validators := keeper.GetValidatorAddresses(ctx, zone.ChainId)
out = []types.WithdrawalRecord{
{
ChainId: zone.ChainId,
Delegator: zone.DelegationAddress.Address,
Recipient: "cosmos1v4gek4mld0k5yhpe0fsln4takg558cdpyexv2rxr3dh45f2fqrgsw52m97",
Distribution: []*types.Distribution{
{Amount: 10000, Valoper: validators[0]},
{Amount: 9500, Valoper: validators[1]},
{Amount: 10000, Valoper: validators[2]},
{Amount: 10000, Valoper: validators[3]},
},
Amount: sdk.NewCoins(sdk.NewCoin(zone.BaseDenom, math.NewInt(39500))),
BurnAmount: sdk.NewCoin(zone.LocalDenom, math.NewInt(32356)),
Txhash: "3BE21C1057ABBFBC44BE8993D2A4701C751507FF9901AA110B5993BA070C176B",
Status: types.WithdrawStatusUnbond,
},
}
return out
},
Delta: sdk.NewDecWithPrec(100, 2).Quo(sdk.NewDecWithPrec(95, 2)),
Validator: func(validators []string) string { return validators[1] },

ExpectError: false,
},
{
Name: "multi record 5% slashing",
InitialRecords: func(ctx sdk.Context, keeper *icskeeper.Keeper) {
zone, _ := keeper.GetZone(ctx, suite.chainB.ChainID)
validators := keeper.GetValidatorAddresses(ctx, zone.ChainId)
withdrawalRecord := types.WithdrawalRecord{
ChainId: zone.ChainId,
Delegator: zone.DelegationAddress.Address,
Recipient: "cosmos1v4gek4mld0k5yhpe0fsln4takg558cdpyexv2rxr3dh45f2fqrgsw52m97",
Distribution: []*types.Distribution{
{Amount: 10000, Valoper: validators[0]},
{Amount: 10000, Valoper: validators[1]},
{Amount: 10000, Valoper: validators[2]},
{Amount: 10000, Valoper: validators[3]},
},
Amount: sdk.NewCoins(sdk.NewCoin(zone.BaseDenom, math.NewInt(40000))),
BurnAmount: sdk.NewCoin(zone.LocalDenom, math.NewInt(32356)),
Txhash: "3BE21C1057ABBFBC44BE8993D2A4701C751507FF9901AA110B5993BA070C176B",
Status: types.WithdrawStatusUnbond,
}
keeper.SetWithdrawalRecord(ctx, withdrawalRecord)
withdrawalRecord = types.WithdrawalRecord{
ChainId: zone.ChainId,
Delegator: zone.DelegationAddress.Address,
Recipient: "cosmos1nvkpj5n5mhy2ntvgn2cklntwx9ujvfvcacz5et",
Distribution: []*types.Distribution{
{Amount: 13000, Valoper: validators[0]},
{Amount: 14000, Valoper: validators[1]},
{Amount: 10000, Valoper: validators[2]},
{Amount: 11000, Valoper: validators[3]},
},
Amount: sdk.NewCoins(sdk.NewCoin(zone.BaseDenom, math.NewInt(48000))),
BurnAmount: sdk.NewCoin(zone.LocalDenom, math.NewInt(36503)),
Txhash: "FB087A50A4836CBDFACA70D393AF110C28935276267B7BA2838BE3CEEA08F762",
Status: types.WithdrawStatusUnbond,
}
keeper.SetWithdrawalRecord(ctx, withdrawalRecord)
},
ExpectedRecords: func(ctx sdk.Context, keeper *icskeeper.Keeper) (out []types.WithdrawalRecord) {
zone, _ := keeper.GetZone(ctx, suite.chainB.ChainID)
validators := keeper.GetValidatorAddresses(ctx, zone.ChainId)
out = []types.WithdrawalRecord{
{
ChainId: zone.ChainId,
Delegator: zone.DelegationAddress.Address,
Recipient: "cosmos1v4gek4mld0k5yhpe0fsln4takg558cdpyexv2rxr3dh45f2fqrgsw52m97",
Distribution: []*types.Distribution{
{Amount: 10000, Valoper: validators[0]},
{Amount: 9500, Valoper: validators[1]},
{Amount: 10000, Valoper: validators[2]},
{Amount: 10000, Valoper: validators[3]},
},
Amount: sdk.NewCoins(sdk.NewCoin(zone.BaseDenom, math.NewInt(39500))),
BurnAmount: sdk.NewCoin(zone.LocalDenom, math.NewInt(32356)),
Txhash: "3BE21C1057ABBFBC44BE8993D2A4701C751507FF9901AA110B5993BA070C176B",
Status: types.WithdrawStatusUnbond,
},
{
ChainId: zone.ChainId,
Delegator: zone.DelegationAddress.Address,
Recipient: "cosmos1nvkpj5n5mhy2ntvgn2cklntwx9ujvfvcacz5et",
Distribution: []*types.Distribution{
{Amount: 13000, Valoper: validators[0]},
{Amount: 13300, Valoper: validators[1]},
{Amount: 10000, Valoper: validators[2]},
{Amount: 11000, Valoper: validators[3]},
},
Amount: sdk.NewCoins(sdk.NewCoin(zone.BaseDenom, math.NewInt(47300))),
BurnAmount: sdk.NewCoin(zone.LocalDenom, math.NewInt(36503)),
Txhash: "FB087A50A4836CBDFACA70D393AF110C28935276267B7BA2838BE3CEEA08F762",
Status: types.WithdrawStatusUnbond,
},
}
return out
},
Delta: sdk.NewDecWithPrec(100, 2).Quo(sdk.NewDecWithPrec(95, 2)),
Validator: func(validators []string) string { return validators[1] },
ExpectError: false,
},
{
Name: "overflow test",
InitialRecords: func(ctx sdk.Context, keeper *icskeeper.Keeper) {
zone, _ := keeper.GetZone(ctx, suite.chainB.ChainID)
validators := keeper.GetValidatorAddresses(ctx, zone.ChainId)
withdrawalRecord := types.WithdrawalRecord{
ChainId: zone.ChainId,
Delegator: zone.DelegationAddress.Address,
Recipient: "cosmos1v4gek4mld0k5yhpe0fsln4takg558cdpyexv2rxr3dh45f2fqrgsw52m97",
Distribution: []*types.Distribution{
{Amount: 9223372036854775807 + 1, Valoper: validators[1]}, // max int64 +1 - check for overflow
},
Amount: sdk.NewCoins(sdk.NewCoin(zone.BaseDenom, math.NewInt(40000))),
BurnAmount: sdk.NewCoin(zone.LocalDenom, math.NewInt(32356)),
Txhash: "3BE21C1057ABBFBC44BE8993D2A4701C751507FF9901AA110B5993BA070C176B",
Status: types.WithdrawStatusUnbond,
}
keeper.SetWithdrawalRecord(ctx, withdrawalRecord)
},
ExpectedRecords: func(ctx sdk.Context, keeper *icskeeper.Keeper) (out []types.WithdrawalRecord) {
zone, _ := keeper.GetZone(ctx, suite.chainB.ChainID)
validators := keeper.GetValidatorAddresses(ctx, zone.ChainId)
out = []types.WithdrawalRecord{
{
ChainId: zone.ChainId,
Delegator: zone.DelegationAddress.Address,
Recipient: "cosmos1v4gek4mld0k5yhpe0fsln4takg558cdpyexv2rxr3dh45f2fqrgsw52m97",
Distribution: []*types.Distribution{
{Amount: 9223372036854775807 + 1, Valoper: validators[1]},
},
Amount: sdk.NewCoins(sdk.NewCoin(zone.BaseDenom, math.NewInt(40000))),
BurnAmount: sdk.NewCoin(zone.LocalDenom, math.NewInt(32356)),
Txhash: "3BE21C1057ABBFBC44BE8993D2A4701C751507FF9901AA110B5993BA070C176B",
Status: types.WithdrawStatusUnbond,
},
}
return out
},
Delta: sdk.NewDecWithPrec(100, 2).Quo(sdk.NewDecWithPrec(95, 2)),
Validator: func(validators []string) string { return validators[1] },
ExpectError: true,
},

{
Name: "mismatch test",
InitialRecords: func(ctx sdk.Context, keeper *icskeeper.Keeper) {
zone, _ := keeper.GetZone(ctx, suite.chainB.ChainID)
validators := keeper.GetValidatorAddresses(ctx, zone.ChainId)
withdrawalRecord := types.WithdrawalRecord{
ChainId: zone.ChainId,
Delegator: zone.DelegationAddress.Address,
Recipient: "cosmos1nna7k5lywn99cd63elcfqm6p8c5c4qcug4aef5",
Distribution: []*types.Distribution{
{Amount: 100000000, Valoper: validators[1]}, // slashed amount exceeds total amount
},
Amount: sdk.NewCoins(sdk.NewCoin(zone.BaseDenom, math.NewInt(10))),
BurnAmount: sdk.NewCoin(zone.LocalDenom, math.NewInt(10)),
Txhash: "8A698A142447087E5DC01F7BC3886EC1A6606D377D1FAC766FB279AD09F1407C",
Status: types.WithdrawStatusUnbond,
}
keeper.SetWithdrawalRecord(ctx, withdrawalRecord)
},
ExpectedRecords: func(ctx sdk.Context, keeper *icskeeper.Keeper) (out []types.WithdrawalRecord) {
zone, _ := keeper.GetZone(ctx, suite.chainB.ChainID)
validators := keeper.GetValidatorAddresses(ctx, zone.ChainId)
out = []types.WithdrawalRecord{
{
ChainId: zone.ChainId,
Delegator: zone.DelegationAddress.Address,
Recipient: "cosmos1nna7k5lywn99cd63elcfqm6p8c5c4qcug4aef5",
Distribution: []*types.Distribution{
{Amount: 100000000, Valoper: validators[1]}, // slashed amount exceeds total amount
},
Amount: sdk.NewCoins(sdk.NewCoin(zone.BaseDenom, math.NewInt(10))),
BurnAmount: sdk.NewCoin(zone.LocalDenom, math.NewInt(10)),
Txhash: "8A698A142447087E5DC01F7BC3886EC1A6606D377D1FAC766FB279AD09F1407C",
Status: types.WithdrawStatusUnbond,
},
}
return out
},
Delta: sdk.NewDecWithPrec(100, 2).Quo(sdk.NewDecWithPrec(95, 2)),
Validator: func(validators []string) string { return validators[1] },
ExpectError: true,
},
}

for _, tc := range tcs {
suite.Run(tc.Name, func() {
suite.SetupTest()
suite.setupTestZones()

app := suite.GetQuicksilverApp(suite.chainA)
ctx := suite.chainA.GetContext()

zone, found := app.InterchainstakingKeeper.GetZone(ctx, suite.chainB.ChainID)
suite.True(found)

tc.InitialRecords(ctx, app.InterchainstakingKeeper)

err := app.InterchainstakingKeeper.UpdateWithdrawalRecordsForSlash(ctx, &zone, tc.Validator(app.InterchainstakingKeeper.GetValidatorAddresses(ctx, zone.ChainId)), tc.Delta)
if tc.ExpectError {
suite.Error(err)
} else {
suite.NoError(err)
}
ctx = suite.chainA.GetContext()
for _, expected := range tc.ExpectedRecords(ctx, app.InterchainstakingKeeper) {
wrd, found := app.InterchainstakingKeeper.GetWithdrawalRecord(ctx, zone.ChainId, expected.Txhash, types.WithdrawStatusUnbond)
suite.True(found)
suite.EqualValues(expected, wrd)
}
})
}

}

0 comments on commit 83cd0ec

Please sign in to comment.