Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

defensive checks in UpdateWithdrawalRecordsForSlash #789

Merged
merged 4 commits into from
Dec 10, 2023
Merged

defensive checks in UpdateWithdrawalRecordsForSlash #789

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
10111a7
fix: defensive checks in UpdateWithdrawalRecordsForSlash; add tests; …
Nov 18, 2023
59337ca
remove debug lines
Nov 18, 2023
e3cd75d
lint
Nov 18, 2023
8f45b1b
Merge branch 'main' into 607-defensive-checks
Nov 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 0 additions & 23 deletions x/interchainstaking/keeper/keeper.go
View file
Open in desktop
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
37 changes: 37 additions & 0 deletions x/interchainstaking/keeper/withdrawal_record.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@ 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 +225,37 @@ 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))
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
}
263 changes: 263 additions & 0 deletions x/interchainstaking/keeper/withdrawal_record_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,263 @@
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)
}
})
}
}
Loading