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.

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

feat: implement slashing functionality on the provider chain (ADR-013) #1275

Merged
merged 31 commits into from
Sep 27, 2023
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
93a0db5
first version
insumity Sep 7, 2023
ea87bdc
fix mocks
insumity Sep 8, 2023
177e1db
move slashing to other file and check tombstoning
insumity Sep 8, 2023
e1bdfff
add tests that checks that Slash is called
insumity Sep 11, 2023
bbff2ee
add FIXME msg
insumity Sep 11, 2023
dfef410
small changes
insumity Sep 11, 2023
8b49af1
add fixme
insumity Sep 11, 2023
bf89509
fix test
insumity Sep 11, 2023
868e0d2
modified E2E tests and general cleaning up
insumity Sep 12, 2023
9ba3a3d
clean up
insumity Sep 12, 2023
b1a6f31
feat!: Cryptographic verification of equivocation (#1287)
mpoke Sep 14, 2023
d92a4c8
undelegations are getting slashed integraiton test
insumity Sep 14, 2023
72abcf4
Merge branch 'release/v2.1.x-lsm' into insumity/adr-013-impl-on-feat
insumity Sep 14, 2023
b46ace4
fix merge issues
insumity Sep 14, 2023
59d8192
fix mocks
insumity Sep 14, 2023
31c090d
fix lint issue
insumity Sep 14, 2023
79f8f18
Merge branch 'feat/ics-misbehaviour-handling' into insumity/adr-013-i…
insumity Sep 22, 2023
ed35638
took into account Simon's comments
insumity Sep 22, 2023
62212c7
go.sum changes
insumity Sep 22, 2023
e5b46dd
gosec fix
insumity Sep 22, 2023
4cb6283
fix linter issue
insumity Sep 22, 2023
6f15a61
cherry-picked ADR-05 so markdown link checker does not complain
insumity Sep 22, 2023
93c2cdb
lint
sainoe Sep 22, 2023
1d3ee78
Use cached context to get tokens in undelegations and redelegations.
insumity Sep 25, 2023
5c053bb
return the error
insumity Sep 26, 2023
f8633b5
lint issue
insumity Sep 26, 2023
f96f1bc
take into account Philip's comments
insumity Sep 26, 2023
d3dc0bc
clean up
insumity Sep 26, 2023
39bbcf5
fix flakey test
insumity Sep 27, 2023
694dce7
lint issue
insumity Sep 27, 2023
366e3b1
fix error returns and fix flaky test in a better way
insumity Sep 27, 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
12 changes: 11 additions & 1 deletion tests/e2e/steps_consumer_misbehaviour.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ func stepsCauseConsumerMisbehaviour(consumerName string) []Step {
state: State{},
},
// detect the ICS misbehaviour
// and jail alice on the provider
// and jail and slash alice on the provider
{
action: detectConsumerEvidenceAction{
chain: chainID(consumerName),
Expand All @@ -230,6 +230,10 @@ func stepsCauseConsumerMisbehaviour(consumerName string) []Step {
validatorID("alice"): 511,
validatorID("bob"): 20,
},
RepresentativePowers: &map[validatorID]uint{
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using RepresentativePowers was the only way I found to verify that slashing indeed takes place.
Using ValPowers was not useful because after a validator gets jailed, the validator's voting power is 0. I also could not unjail and then check ValPowers similarly to what is done in steps_multi_consumer_downtime.go because in contrast to a downtime test, for misbehaviour & equivocation we also we also tombstone the validator.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems ok to me. Could rename it to something like StakedTokens instead of RepresentativePowers (looking at the code, I think that's what this actually is, right?)

validatorID("alice"): 511000000,
validatorID("bob"): 20000000,
},
},
chainID(consumerName): ChainState{
ValPowers: &map[validatorID]uint{
Expand All @@ -255,6 +259,12 @@ func stepsCauseConsumerMisbehaviour(consumerName string) []Step {
validatorID("alice"): 0,
validatorID("bob"): 20,
},
// "alice" should be slashed on the provider, hence representative
// power is 511000000 - 0.05 * 511000000 = 485450000
RepresentativePowers: &map[validatorID]uint{
validatorID("alice"): 485450000,
validatorID("bob"): 20000000,
},
// The consumer light client should be frozen on the provider
ClientsFrozenHeights: &map[string]clienttypes.Height{
consumerClientID: {
Expand Down
19 changes: 18 additions & 1 deletion tests/e2e/steps_double_sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,11 @@ func stepsCauseDoubleSignOnConsumer(consumerName, providerName string) []Step {
validatorID("bob"): 500,
validatorID("carol"): 500,
},
RepresentativePowers: &map[validatorID]uint{
validatorID("alice"): 500000000,
validatorID("bob"): 500000000,
validatorID("carol"): 500000000,
},
},
chainID(consumerName): ChainState{
ValPowers: &map[validatorID]uint{
Expand All @@ -155,7 +160,7 @@ func stepsCauseDoubleSignOnConsumer(consumerName, providerName string) []Step {
},
},
// detect the double voting infraction
// and jail bob on the provider
// and jail and slashing of bob on the provider
{
action: detectConsumerEvidenceAction{
chain: chainID(consumerName),
Expand All @@ -167,6 +172,13 @@ func stepsCauseDoubleSignOnConsumer(consumerName, providerName string) []Step {
validatorID("bob"): 0,
validatorID("carol"): 500,
},
// "bob" gets slashed on the provider chain, hence representative
// power is 500000000 - 0.05 * 500000000 = 475000000
RepresentativePowers: &map[validatorID]uint{
validatorID("alice"): 500000000,
validatorID("bob"): 475000000,
validatorID("carol"): 500000000,
},
},
chainID(consumerName): ChainState{
ValPowers: &map[validatorID]uint{
Expand All @@ -192,6 +204,11 @@ func stepsCauseDoubleSignOnConsumer(consumerName, providerName string) []Step {
validatorID("bob"): 0,
validatorID("carol"): 500,
},
RepresentativePowers: &map[validatorID]uint{
validatorID("alice"): 500000000,
validatorID("bob"): 475000000,
validatorID("carol"): 500000000,
},
},
chainID(consumerName): ChainState{
ValPowers: &map[validatorID]uint{
Expand Down
17 changes: 14 additions & 3 deletions tests/integration/double_vote.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
)

// TestHandleConsumerDoubleVoting verifies that handling a double voting evidence
// of a consumer chain results in the expected jailing of the malicious validator
// of a consumer chain results in the expected tombstoning, jailing, and slashing of the malicious validator
func (s *CCVTestSuite) TestHandleConsumerDoubleVoting() {
s.SetupCCVChannel(s.path)
// required to have the consumer client revision height greater than 0
Expand Down Expand Up @@ -159,6 +159,9 @@ func (s *CCVTestSuite) TestHandleConsumerDoubleVoting() {
consuAddr := types.NewConsumerConsAddress(sdk.ConsAddress(consuVal.Address.Bytes()))
provAddr := s.providerApp.GetProviderKeeper().GetProviderAddrFromConsumerAddr(s.providerCtx(), s.consumerChain.ChainID, consuAddr)

validator, _ := s.providerApp.GetTestStakingKeeper().GetValidator(s.providerCtx(), provAddr.ToSdkConsAddr().Bytes())
initialTokens := validator.GetTokens().ToDec()

for _, tc := range testCases {
s.Run(tc.name, func() {
// reset context for each run
Expand All @@ -185,13 +188,21 @@ func (s *CCVTestSuite) TestHandleConsumerDoubleVoting() {
if tc.expPass {
s.Require().NoError(err)

// verifies that the jailing has occurred
// verifies that the jailing and tombstoning has occurred
s.Require().True(s.providerApp.GetTestStakingKeeper().IsValidatorJailed(provCtx, provAddr.ToSdkConsAddr()))
s.Require().True(s.providerApp.GetTestSlashingKeeper().IsTombstoned(provCtx, provAddr.ToSdkConsAddr()))

// verifies that the validator gets slashed and has fewer tokens after the slashing
validator, _ := s.providerApp.GetTestStakingKeeper().GetValidator(provCtx, provAddr.ToSdkConsAddr().Bytes())
slashFraction := s.providerApp.GetTestSlashingKeeper().SlashFractionDoubleSign(provCtx)
actualTokens := validator.GetTokens().ToDec()
s.Require().True(initialTokens.Sub(initialTokens.Mul(slashFraction)).Equal(actualTokens))
} else {
s.Require().Error(err)

// verifies that no jailing and has occurred
// verifies that no jailing and no tombstoning has occurred
s.Require().False(s.providerApp.GetTestStakingKeeper().IsValidatorJailed(provCtx, provAddr.ToSdkConsAddr()))
s.Require().False(s.providerApp.GetTestSlashingKeeper().IsTombstoned(provCtx, provAddr.ToSdkConsAddr()))
}
})
}
Expand Down
12 changes: 11 additions & 1 deletion tests/integration/misbehaviour.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,16 +53,26 @@ func (s *CCVTestSuite) TestHandleConsumerMisbehaviour() {
),
}

// we assume that all validators have the same number of initial tokens
validator, _ := s.getValByIdx(0)
initialTokens := validator.GetTokens().ToDec()

err := s.providerApp.GetProviderKeeper().HandleConsumerMisbehaviour(s.providerCtx(), *misb)
s.NoError(err)

// verify that validators are jailed and tombstoned
// verify that validators are jailed, tombstoned, and slashed
for _, v := range clientTMValset.Validators {
consuAddr := sdk.ConsAddress(v.Address.Bytes())
provAddr := s.providerApp.GetProviderKeeper().GetProviderAddrFromConsumerAddr(s.providerCtx(), s.consumerChain.ChainID, types.NewConsumerConsAddress(consuAddr))
val, ok := s.providerApp.GetTestStakingKeeper().GetValidatorByConsAddr(s.providerCtx(), provAddr.Address)
s.Require().True(ok)
s.Require().True(val.Jailed)
s.Require().True(s.providerApp.GetTestSlashingKeeper().IsTombstoned(s.providerCtx(), provAddr.ToSdkConsAddr()))

validator, _ := s.providerApp.GetTestStakingKeeper().GetValidator(s.providerCtx(), provAddr.ToSdkConsAddr().Bytes())
slashFraction := s.providerApp.GetTestSlashingKeeper().SlashFractionDoubleSign(s.providerCtx())
actualTokens := validator.GetTokens().ToDec()
s.Require().True(initialTokens.Sub(initialTokens.Mul(slashFraction)).Equal(actualTokens))
}
}

Expand Down
91 changes: 66 additions & 25 deletions testutil/keeper/mocks.go

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

6 changes: 2 additions & 4 deletions x/ccv/provider/keeper/double_vote.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@ package keeper
import (
"bytes"
"fmt"

cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"

sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/cosmos/interchain-security/v2/x/ccv/provider/types"
Expand Down Expand Up @@ -33,8 +31,8 @@ func (k Keeper) HandleConsumerDoubleVoting(
types.NewConsumerConsAddress(sdk.ConsAddress(evidence.VoteA.ValidatorAddress.Bytes())),
)

// execute the jailing
k.JailValidator(ctx, providerAddr)
k.SlashValidator(ctx, providerAddr)
k.JailAndTombstoneValidator(ctx, providerAddr)

k.Logger(ctx).Info(
"confirmed equivocation",
Expand Down
6 changes: 3 additions & 3 deletions x/ccv/provider/keeper/misbehaviour.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
package keeper

import (
"github.com/cosmos/interchain-security/v2/x/ccv/provider/types"

sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
ibcclienttypes "github.com/cosmos/ibc-go/v4/modules/core/02-client/types"
ibctmtypes "github.com/cosmos/ibc-go/v4/modules/light-clients/07-tendermint/types"
"github.com/cosmos/interchain-security/v2/x/ccv/provider/types"
tmtypes "github.com/tendermint/tendermint/types"
)

Expand Down Expand Up @@ -41,7 +40,8 @@ func (k Keeper) HandleConsumerMisbehaviour(ctx sdk.Context, misbehaviour ibctmty
misbehaviour.Header1.Header.ChainID,
types.NewConsumerConsAddress(sdk.ConsAddress(v.Address.Bytes())),
)
k.JailValidator(ctx, providerAddr)
k.SlashValidator(ctx, providerAddr)
k.JailAndTombstoneValidator(ctx, providerAddr)
provAddrs = append(provAddrs, providerAddr)
}

Expand Down
Loading