Skip to content

Commit

Permalink
fix!: update unbonding pausing for PSS (#1728)
Browse files Browse the repository at this point in the history
* draft PSS unbonding fix

* fix hook logic to retrieve validator address from ubd op

* add unbonding pausing unit-test

* remove panic in hook
  • Loading branch information
sainoe committed Apr 11, 2024
1 parent f9344e2 commit 795c9fd
Show file tree
Hide file tree
Showing 4 changed files with 188 additions and 2 deletions.
59 changes: 59 additions & 0 deletions testutil/keeper/mocks.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

59 changes: 58 additions & 1 deletion x/ccv/provider/keeper/hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
v1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"

"github.com/cosmos/interchain-security/v4/x/ccv/provider/types"
providertypes "github.com/cosmos/interchain-security/v4/x/ccv/provider/types"
ccvtypes "github.com/cosmos/interchain-security/v4/x/ccv/types"
)
Expand Down Expand Up @@ -35,8 +36,64 @@ func (k *Keeper) Hooks() Hooks {
func (h Hooks) AfterUnbondingInitiated(ctx sdk.Context, id uint64) error {
var consumerChainIDS []string

// get validator address from unbonding operation
unbondingType, found := h.k.stakingKeeper.GetUnbondingType(ctx, id)
vadAddrBech32 := ""
if !found {
ctx.Logger().Error("undefined type for unbonding operation id: %d", id)
return nil
}

switch unbondingType {
case stakingtypes.UnbondingType_UnbondingDelegation:
ubd, found := h.k.stakingKeeper.GetUnbondingDelegationByUnbondingID(ctx, id)
if !found {
ctx.Logger().Error("unfound ubonding delegation for unbonding id: %d", id)
return nil
}
vadAddrBech32 = ubd.ValidatorAddress
case stakingtypes.UnbondingType_Redelegation:
red, found := h.k.stakingKeeper.GetRedelegationByUnbondingID(ctx, id)
if !found {
ctx.Logger().Error("unfound relegation for unbonding operation id: %d", id)
return nil
}
vadAddrBech32 = red.ValidatorSrcAddress
case stakingtypes.UnbondingType_ValidatorUnbonding:
val, found := h.k.stakingKeeper.GetValidatorByUnbondingID(ctx, id)
if !found {
ctx.Logger().Error("unfound validator for unbonding operation id: %d", id)
return nil
}
vadAddrBech32 = val.OperatorAddress
default:
ctx.Logger().Error("invalid unbonding operation type: %s", unbondingType)
return nil
}

valAddr, err := sdk.ValAddressFromBech32(vadAddrBech32)
if err != nil {
ctx.Logger().Error(err.Error())
return nil
}

validator, found := h.k.stakingKeeper.GetValidator(ctx, valAddr)
if !found {
ctx.Logger().Error("unfound validator for validator address %s", vadAddrBech32)
return nil
}

consAddr, err := validator.GetConsAddr()
if err != nil {
ctx.Logger().Error(err.Error())
return nil
}

// get all consumers where the validator is in the validator set
for _, chain := range h.k.GetAllConsumerChains(ctx) {
consumerChainIDS = append(consumerChainIDS, chain.ChainId)
if h.k.IsConsumerValidator(ctx, chain.ChainId, types.NewProviderConsAddress(consAddr)) {
consumerChainIDS = append(consumerChainIDS, chain.ChainId)
}
}

if len(consumerChainIDS) == 0 {
Expand Down
68 changes: 67 additions & 1 deletion x/ccv/provider/keeper/relay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ import (

"cosmossdk.io/math"

codectypes "github.com/cosmos/cosmos-sdk/codec/types"
cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
sdk "github.com/cosmos/cosmos-sdk/types"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"

Expand Down Expand Up @@ -475,6 +477,10 @@ func TestHandleVSCMaturedPacket(t *testing.T) {

// Start first unbonding without any consumers registered
var unbondingOpId uint64 = 1
gomock.InOrder(
mocks.MockStakingKeeper.EXPECT().GetUnbondingType(ctx, unbondingOpId).Return(stakingtypes.UnbondingType_Undefined, false),
)

err := pk.Hooks().AfterUnbondingInitiated(ctx, unbondingOpId)
require.NoError(t, err)
// Check that no unbonding op was stored
Expand All @@ -485,12 +491,34 @@ func TestHandleVSCMaturedPacket(t *testing.T) {
pk.IncrementValidatorSetUpdateId(ctx)
require.Equal(t, uint64(2), pk.GetValidatorSetUpdateId(ctx))

// Registered first consumer
// Register first consumer
pk.SetConsumerClientId(ctx, "chain-1", "client-1")

// Create 2 validators
vals := []stakingtypes.Validator{}
valsPk := []cryptotypes.PubKey{}
for i := 0; i < 2; i++ {
pubkey, err := cryptocodec.FromTmPubKeyInterface(cryptotestutil.NewCryptoIdentityFromIntSeed(54321 + i).TMCryptoPubKey())
require.NoError(t, err)
valsPk = append(valsPk, pubkey)
pkAny, err := codectypes.NewAnyWithValue(pubkey)
require.NoError(t, err)
vals = append(vals, stakingtypes.Validator{ConsensusPubkey: pkAny})
}

// Opt-in one validator to consumer
pk.SetConsumerValidator(ctx, "chain-1", types.ConsumerValidator{ProviderConsAddr: valsPk[0].Address()})

// Start second unbonding
unbondingOpId = 2
gomock.InOrder(
mocks.MockStakingKeeper.EXPECT().GetUnbondingType(ctx, unbondingOpId).Return(stakingtypes.UnbondingType_UnbondingDelegation, true),
mocks.MockStakingKeeper.EXPECT().GetUnbondingDelegationByUnbondingID(ctx, unbondingOpId).Return(
stakingtypes.UnbondingDelegation{
ValidatorAddress: sdk.ValAddress([]byte{1}).String(),
}, true),
mocks.MockStakingKeeper.EXPECT().GetValidator(ctx, sdk.ValAddress([]byte{1})).
Return(vals[0], true),
mocks.MockStakingKeeper.EXPECT().PutUnbondingOnHold(ctx, unbondingOpId).Return(nil),
)
err = pk.Hooks().AfterUnbondingInitiated(ctx, unbondingOpId)
Expand All @@ -514,10 +542,21 @@ func TestHandleVSCMaturedPacket(t *testing.T) {
// Registered second consumer
pk.SetConsumerClientId(ctx, "chain-2", "client-2")

// Opt-in both validators to second consumer
pk.SetConsumerValidator(ctx, "chain-2", types.ConsumerValidator{ProviderConsAddr: valsPk[0].Address()})
pk.SetConsumerValidator(ctx, "chain-2", types.ConsumerValidator{ProviderConsAddr: valsPk[1].Address()})

// Start third and fourth unbonding
unbondingOpIds := []uint64{3, 4}
for _, id := range unbondingOpIds {
gomock.InOrder(
mocks.MockStakingKeeper.EXPECT().GetUnbondingType(ctx, id).Return(stakingtypes.UnbondingType_Redelegation, true),
mocks.MockStakingKeeper.EXPECT().GetRedelegationByUnbondingID(ctx, id).Return(
stakingtypes.Redelegation{
ValidatorSrcAddress: sdk.ValAddress([]byte{1}).String(),
}, true),
mocks.MockStakingKeeper.EXPECT().GetValidator(ctx, sdk.ValAddress([]byte{1})).
Return(vals[0], true),
mocks.MockStakingKeeper.EXPECT().PutUnbondingOnHold(ctx, id).Return(nil),
)
err = pk.Hooks().AfterUnbondingInitiated(ctx, id)
Expand All @@ -538,6 +577,33 @@ func TestHandleVSCMaturedPacket(t *testing.T) {
require.Equal(t, unbondingOpIds, ids)
}

// Increment vscID
pk.IncrementValidatorSetUpdateId(ctx)
require.Equal(t, uint64(4), pk.GetValidatorSetUpdateId(ctx))

// Start fith unbonding
unbondingOpId = 5
gomock.InOrder(
mocks.MockStakingKeeper.EXPECT().GetUnbondingType(ctx, unbondingOpId).Return(stakingtypes.UnbondingType_ValidatorUnbonding, true),
mocks.MockStakingKeeper.EXPECT().GetValidatorByUnbondingID(ctx, unbondingOpId).Return(
stakingtypes.Validator{
OperatorAddress: sdk.ValAddress([]byte{1}).String(),
}, true),
mocks.MockStakingKeeper.EXPECT().GetValidator(ctx, sdk.ValAddress([]byte{1})).
Return(vals[1], true),
mocks.MockStakingKeeper.EXPECT().PutUnbondingOnHold(ctx, unbondingOpId).Return(nil),
)
err = pk.Hooks().AfterUnbondingInitiated(ctx, unbondingOpId)
require.NoError(t, err)

// Check that an unbonding op was stored for chain-2 only
// since it's the only consumer the unbonding validator has opted-in to
expectedChains = []string{"chain-2"}
unbondingOp, found = pk.GetUnbondingOp(ctx, unbondingOpId)
require.True(t, found)
require.Equal(t, unbondingOpId, unbondingOp.Id)
require.Equal(t, expectedChains, unbondingOp.UnbondingConsumerChains)

// Handle VSCMatured packet from chain-1 for vscID 1.
// Note that no VSCPacket was sent as the chain was not yet registered,
// but the code should still work
Expand Down
4 changes: 4 additions & 0 deletions x/ccv/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ type StakingKeeper interface {
IterateLastValidatorPowers(ctx sdk.Context, cb func(addr sdk.ValAddress, power int64) (stop bool))
PowerReduction(ctx sdk.Context) math.Int
PutUnbondingOnHold(ctx sdk.Context, id uint64) error
GetUnbondingDelegationByUnbondingID(ctx sdk.Context, id uint64) (ubd stakingtypes.UnbondingDelegation, found bool)
GetRedelegationByUnbondingID(ctx sdk.Context, id uint64) (red stakingtypes.Redelegation, found bool)
GetValidatorByUnbondingID(ctx sdk.Context, id uint64) (val stakingtypes.Validator, found bool)
IterateValidators(ctx sdk.Context, f func(index int64, validator stakingtypes.ValidatorI) (stop bool))
Validator(ctx sdk.Context, addr sdk.ValAddress) stakingtypes.ValidatorI
IsValidatorJailed(ctx sdk.Context, addr sdk.ConsAddress) bool
Expand All @@ -54,6 +57,7 @@ type StakingKeeper interface {
GetUnbondingDelegationsFromValidator(ctx sdk.Context, valAddr sdk.ValAddress) (ubds []stakingtypes.UnbondingDelegation)
GetRedelegationsFromSrcValidator(ctx sdk.Context, valAddr sdk.ValAddress) (reds []stakingtypes.Redelegation)
GetUnbondingType(ctx sdk.Context, id uint64) (unbondingType stakingtypes.UnbondingType, found bool)
BlockValidatorUpdates(ctx sdk.Context) []abci.ValidatorUpdate
}

// SlashingKeeper defines the contract expected to perform ccv slashing
Expand Down

0 comments on commit 795c9fd

Please sign in to comment.