Skip to content

Commit

Permalink
feat: pause unbondings during equivocation proposal voting period
Browse files Browse the repository at this point in the history
Fix cosmos#747

The change registers 2 gov module hooks in the provider module:

- `AfterProposalDeposit`: if the proposal is an equivocation proposal in
  voting period, then call `PutUnbondingOnHold` for each unbondings of
  each validators found in the proposal.

- `AfterProposalVotingPeriodEnded`: if the proposal is an equivocation
  proposal, then call `UnbondingCanComplete` for each unbondings of each
  validators found in the proposal.

A new key is also added, to store the equivocation proposal ID for which
unbondings have been put on hold. This covers 2 specific cases:

- The gov module allows additional deposits even if the proposal is
  already in voting period. So when `AfterProposalDeposit` is invoked,
  we have to make sure the proposal is in voting period for the first time
  before puting the unbondings on hold. This is simply handled by
  checking if the proposal ID exists in the store at the beginning of
  the hook, and then storing it if not.

- If the provider chain is upgraded with this change and there's already
  an equivocation proposal in voting period, `AfterProposalVotingPeriodEnded`
  could be invoked without the initial `AfterProposalDeposit`, so some
  unbondings could be un-paused while they hadn't been paused (conflicts with
  `AfterUnbondingInitiated` hook). To prevent that, we check the
  proposal ID exists in the store, which means `AfterProposalDeposit`
  has been called prevouisly.

Co-authored-by: Albert Le Batteux <[email protected]>
  • Loading branch information
tbruyelle and albttx committed Mar 17, 2023
1 parent 57d3b54 commit 51d80be
Show file tree
Hide file tree
Showing 12 changed files with 573 additions and 32 deletions.
14 changes: 12 additions & 2 deletions app/provider/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ func New(
stakingtypes.NewMultiStakingHooks(
app.DistrKeeper.Hooks(),
app.SlashingKeeper.Hooks(),
app.ProviderKeeper.Hooks(),
app.ProviderKeeper.Hooks(app.GovKeeper),
),
)

Expand Down Expand Up @@ -426,7 +426,7 @@ func New(
AddRoute(providertypes.RouterKey, ibcprovider.NewProviderProposalHandler(app.ProviderKeeper)).
AddRoute(ibcclienttypes.RouterKey, ibcclient.NewClientProposalHandler(app.IBCKeeper.ClientKeeper))

app.GovKeeper = govkeeper.NewKeeper(
govKeeper := govkeeper.NewKeeper(
appCodec,
keys[govtypes.StoreKey],
app.GetSubspace(govtypes.ModuleName),
Expand All @@ -435,6 +435,11 @@ func New(
&stakingKeeper,
govRouter,
)
app.GovKeeper = *govKeeper.SetHooks(
govtypes.NewMultiGovHooks(
app.ProviderKeeper.Hooks(govKeeper),
),
)

app.TransferKeeper = ibctransferkeeper.NewKeeper(
appCodec,
Expand Down Expand Up @@ -763,6 +768,11 @@ func (app *App) GetE2eStakingKeeper() e2e.E2eStakingKeeper {
return app.StakingKeeper
}

// GetE2eGovKeeper implements the ProviderApp interface.
func (app *App) GetE2eGovKeeper() e2e.E2eGovKeeper {
return app.GovKeeper
}

// GetE2eBankKeeper implements the ProviderApp interface.
func (app *App) GetE2eBankKeeper() e2e.E2eBankKeeper {
return app.BankKeeper
Expand Down
50 changes: 50 additions & 0 deletions tests/e2e/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,56 @@ func checkStakingUnbondingOps(s *CCVTestSuite, id uint64, found bool, onHold boo
}
}

// unbondingsOnHold is a handy struct to hold the different kinds of unbonding
// refcount.
type unbondingsOnHold struct {
unbondingDelegationRefCount int64
validatorUnbondingRefCount int64
redelegationRefCount int64
}

// checkStakingUnbondingOnHoldRefCount checks that valAddr's unbonding refcounts
// match expected.
func checkStakingUnbondingOnHoldRefCount(s *CCVTestSuite, valAddr sdk.ValAddress, expected unbondingsOnHold) {

// check unbondingDelegation
ubds := s.providerApp.GetE2eStakingKeeper().GetUnbondingDelegationsFromValidator(s.providerCtx(), valAddr)
if expected.unbondingDelegationRefCount == 0 {
s.Assert().Empty(ubds, "expected no unbonding delegation")
} else {
s.Assert().NotEmpty(ubds, "no unbonding delegation found")
for _, ubd := range ubds {
s.Assert().NotEmpty(ubd.Entries, "no unbonding delegation entries found")
for _, entry := range ubd.Entries {
s.Assert().Equal(expected.unbondingDelegationRefCount, entry.UnbondingOnHoldRefCount,
"wrong unbonding delegation ref count")
}
}
}

// check redelegation
reds := s.providerApp.GetE2eStakingKeeper().GetRedelegationsFromSrcValidator(s.providerCtx(), valAddr)
if expected.redelegationRefCount == 0 {
s.Assert().Empty(reds, "expected no redelegation")
} else {
s.Assert().NotEmpty(reds, "no redelegation found")
for _, ubd := range reds {
s.Assert().NotEmpty(ubd.Entries, "no redelegation entries found")
for _, entry := range ubd.Entries {
s.Assert().Equal(expected.redelegationRefCount, entry.UnbondingOnHoldRefCount,
"wrong redelegation ref count")
}
}
}

// check validator unbonding
val, found := s.providerApp.GetE2eStakingKeeper().GetValidator(s.providerCtx(), valAddr)
if s.Assert().True(found, "validator not found") {
s.Assert().Equal(expected.validatorUnbondingRefCount, val.UnbondingOnHoldRefCount,
"wrong validator unbonding ref count")
}
}

func checkCCVUnbondingOp(s *CCVTestSuite, providerCtx sdk.Context, chainID string, valUpdateID uint64, found bool, msgAndArgs ...interface{}) {
entries := s.providerApp.GetProviderKeeper().GetUnbondingOpsFromIndex(providerCtx, chainID, valUpdateID)
if found {
Expand Down
243 changes: 243 additions & 0 deletions tests/e2e/proposal.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,243 @@
package e2e

import (
"time"

sdk "github.com/cosmos/cosmos-sdk/types"
evidencetypes "github.com/cosmos/cosmos-sdk/x/evidence/types"
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
providertypes "github.com/cosmos/interchain-security/x/ccv/provider/types"
)

func (s *CCVTestSuite) TestPauseUnbondingOnEquivocationProposal() {
tests := []struct {
name string
setup func() govtypes.Content
expectedWithoutProp unbondingsOnHold
expectedDuringProp unbondingsOnHold
}{
{
// Assert a text proposal doesn't pause any existing unbondings
name: "text proposal and unbonding delegation",
setup: func() govtypes.Content {
var (
bondAmt = sdk.NewInt(10000000)
delAddr = s.providerChain.SenderAccount.GetAddress()
)
// delegate bondAmt and undelegate it
delegateAndUndelegate(s, delAddr, bondAmt, 1)

return govtypes.NewTextProposal("title", "desc")
},
expectedWithoutProp: unbondingsOnHold{
unbondingDelegationRefCount: 1,
},
expectedDuringProp: unbondingsOnHold{
unbondingDelegationRefCount: 1,
},
},
{
// Assert an equivocation proposal pauses nothing if there's no existing
// unbondings.
name: "equivocation proposal and no unbonding",
setup: func() govtypes.Content {
var (
val, _ = s.getValByIdx(0)
consAddr, _ = val.GetConsAddr()
)
return providertypes.NewEquivocationProposal("title", "desc",
[]*evidencetypes.Equivocation{{
Height: 1,
Power: 1,
Time: time.Now(),
ConsensusAddress: consAddr.String(),
}},
)
},
expectedWithoutProp: unbondingsOnHold{},
expectedDuringProp: unbondingsOnHold{},
},
{
// Assert an equivocation proposal pauses unbonding delegations
name: "equivocation proposal and unbonding delegation",
setup: func() govtypes.Content {
// create an unbonding entry of type UnbondingDelegation
var (
bondAmt = sdk.NewInt(10000000)
delAddr = s.providerChain.SenderAccount.GetAddress()
val, _ = s.getValByIdx(0)
consAddr, _ = val.GetConsAddr()
)
// delegate bondAmt and undelegate it
delegateAndUndelegate(s, delAddr, bondAmt, 1)

return providertypes.NewEquivocationProposal("title", "desc",
[]*evidencetypes.Equivocation{{
Height: 1,
Power: 1,
Time: time.Now(),
ConsensusAddress: consAddr.String(),
}},
)
},
expectedWithoutProp: unbondingsOnHold{
unbondingDelegationRefCount: 1,
},
expectedDuringProp: unbondingsOnHold{
unbondingDelegationRefCount: 2,
},
},
{
// Assert an equivocation proposal pauses redelegations
name: "equivocation proposal and redelegate",
setup: func() govtypes.Content {
// create an unbonding entry of type UnbondingDelegation
var (
bondAmt = sdk.NewInt(10000000)
delAddr = s.providerChain.SenderAccount.GetAddress()
val, valSrcAddr = s.getValByIdx(0)
_, valDstAddr = s.getValByIdx(1)
consAddr, _ = val.GetConsAddr()
)
// delegate bondAmt and redelegate it
delegateAndRedelegate(s, delAddr, valSrcAddr, valDstAddr, bondAmt)

return providertypes.NewEquivocationProposal("title", "desc",
[]*evidencetypes.Equivocation{{
Height: 1,
Power: 1,
Time: time.Now(),
ConsensusAddress: consAddr.String(),
}},
)
},
expectedWithoutProp: unbondingsOnHold{
redelegationRefCount: 1,
},
expectedDuringProp: unbondingsOnHold{
redelegationRefCount: 2,
},
},
{
// Assert an equivocation proposal pauses validator unbonding
name: "equivocation proposal and validator unbonding",
setup: func() govtypes.Content {
var (
delAddr = s.providerChain.SenderAccount.GetAddress()
val, valAddr = s.getValByIdx(0)
consAddr, _ = val.GetConsAddr()
)
// unbond validator by undelegate all his delegation (test setup only
// delegates from delAddr, there's no validator self delegation)
undelegate(s, delAddr, valAddr, sdk.NewDec(1))

return providertypes.NewEquivocationProposal("title", "desc",
[]*evidencetypes.Equivocation{{
Height: 1,
Power: 1,
Time: time.Now(),
ConsensusAddress: consAddr.String(),
}},
)
},
expectedWithoutProp: unbondingsOnHold{
unbondingDelegationRefCount: 1,
validatorUnbondingRefCount: 1,
},
expectedDuringProp: unbondingsOnHold{
unbondingDelegationRefCount: 2,
validatorUnbondingRefCount: 2,
},
},
}
submitProp := func(s *CCVTestSuite, content govtypes.Content) uint64 {
proposal, err := s.providerApp.GetE2eGovKeeper().SubmitProposal(s.providerCtx(), content)
s.Require().NoError(err)
return proposal.ProposalId
}
addDepositProp := func(s *CCVTestSuite, proposalID uint64, depositAmt sdk.Coins) {
depositorAddr := s.providerChain.SenderAccount.GetAddress()
_, err := s.providerApp.GetE2eGovKeeper().AddDeposit(
s.providerCtx(), proposalID, depositorAddr, depositAmt,
)
s.Require().NoError(err)
}
waitFor := func(s *CCVTestSuite, period time.Duration) {
s.providerChain.CurrentHeader.Time = s.providerChain.CurrentHeader.Time.Add(period)
// Need to advance 2 blocks because the time set at the line above is only
// applied for the second block
s.providerChain.NextBlock()
s.providerChain.NextBlock()
}
for i, tt := range tests {
s.Run(tt.name, func() {
if i+1 < len(tests) {
// reset suite to reset provider client
defer s.SetupTest()
}

//-----------------------------------------
// Setup

var (
govContent = tt.setup()
val, valAddr = s.getValByIdx(0)
consAddr, _ = val.GetConsAddr()
govKeeper = s.providerApp.GetE2eGovKeeper()
dustAmt = sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(1)))
minDepositAmt = govKeeper.GetDepositParams(s.providerCtx()).MinDeposit
)
// Equivocation proposal requires validator signing info and a slash log
s.setDefaultValSigningInfo(*s.providerChain.Vals.Validators[0])
s.providerApp.GetProviderKeeper().SetSlashLog(s.providerCtx(),
providertypes.NewProviderConsAddress(consAddr))
// Reduce voting period
votingParams := govKeeper.GetVotingParams(s.providerCtx())
votingParams.VotingPeriod = 3 * time.Second
govKeeper.SetVotingParams(s.providerCtx(), votingParams)
// Reduce deposit period
depositParams := govKeeper.GetDepositParams(s.providerCtx())
depositParams.MaxDepositPeriod = 3 * time.Second
govKeeper.SetDepositParams(s.providerCtx(), depositParams)
// must move one block forward because unbonding validators are detected
// during EndBlock.
s.providerChain.NextBlock()
checkStakingUnbondingOnHoldRefCount(s, valAddr, tt.expectedWithoutProp)

//-----------------------------------------
// #1 Create a proposal, activate it and wait for voting period

proposalID := submitProp(s, govContent)
checkStakingUnbondingOnHoldRefCount(s, valAddr, tt.expectedWithoutProp)
// assert that until voting period starts, there's no pause triggered
addDepositProp(s, proposalID, dustAmt)
checkStakingUnbondingOnHoldRefCount(s, valAddr, tt.expectedWithoutProp)
// activate prop
addDepositProp(s, proposalID, minDepositAmt)
checkStakingUnbondingOnHoldRefCount(s, valAddr, tt.expectedDuringProp)
// assert that an additionnal deposit done after the activation doesn't
// trigger additionnal pauses
addDepositProp(s, proposalID, dustAmt)
checkStakingUnbondingOnHoldRefCount(s, valAddr, tt.expectedDuringProp)
// ends the voting period
waitFor(s, votingParams.VotingPeriod)
checkStakingUnbondingOnHoldRefCount(s, valAddr, tt.expectedWithoutProp)
s.Assert().False(
s.providerApp.GetProviderKeeper().HasEquivocationProposal(s.providerCtx(), proposalID),
"proposalID should be removed from store after voting period",
)

//-----------------------------------------
// #2 Create an other proposal but let it expire (not enough deposit)

proposalID = submitProp(s, govContent)
checkStakingUnbondingOnHoldRefCount(s, valAddr, tt.expectedWithoutProp)
waitFor(s, depositParams.MaxDepositPeriod)
checkStakingUnbondingOnHoldRefCount(s, valAddr, tt.expectedWithoutProp)
s.Assert().False(
s.providerApp.GetProviderKeeper().HasEquivocationProposal(s.providerCtx(), proposalID),
"proposalID shouldn't be registered if proposal didn't reach the voting period",
)
})
}
}
3 changes: 3 additions & 0 deletions testutil/e2e/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ type ProviderApp interface {
GetE2eSlashingKeeper() E2eSlashingKeeper
// Returns a distribution keeper interface with more capabilities than the expected_keepers interface
GetE2eDistributionKeeper() E2eDistributionKeeper
// Returns a gov keeper interface with more capabilities than the expected_keepers interface
GetE2eGovKeeper() E2eGovKeeper
}

// The interface that any consumer app must implement to be compatible with e2e tests
Expand Down Expand Up @@ -140,6 +142,7 @@ type E2eMintKeeper interface {

type E2eGovKeeper interface {
GetDepositParams(ctx sdk.Context) govtypes.DepositParams
SetDepositParams(sdk.Context, govtypes.DepositParams)
GetVotingParams(ctx sdk.Context) govtypes.VotingParams
SetVotingParams(ctx sdk.Context, votingParams govtypes.VotingParams)
SubmitProposal(ctx sdk.Context, content govtypes.Content) (govtypes.Proposal, error)
Expand Down
Loading

0 comments on commit 51d80be

Please sign in to comment.