From 6b7eec66b04b19201712b20a6c09414849765dd0 Mon Sep 17 00:00:00 2001 From: Simon Noetzlin Date: Tue, 29 Aug 2023 15:13:39 +0200 Subject: [PATCH 1/6] fix double voting cli --- x/ccv/provider/client/cli/tx.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/ccv/provider/client/cli/tx.go b/x/ccv/provider/client/cli/tx.go index 1d3ccc0011..ed7c72b6b1 100644 --- a/x/ccv/provider/client/cli/tx.go +++ b/x/ccv/provider/client/cli/tx.go @@ -176,7 +176,7 @@ func NewSubmitConsumerDoubleVotingCmd() *cobra.Command { return err } - msg, err := types.NewMsgSubmitConsumerDoubleVoting(submitter, ev, nil) + msg, err := types.NewMsgSubmitConsumerDoubleVoting(submitter, ev, &header) if err != nil { return err } From 3f57da397a7897a849a80e25bf4d8d9393db19c0 Mon Sep 17 00:00:00 2001 From: Simon Noetzlin Date: Thu, 31 Aug 2023 16:48:42 +0200 Subject: [PATCH 2/6] fix bug double signing handler --- Dockerfile | 2 +- tests/e2e/main.go | 10 +-- tests/integration/double_vote.go | 106 ++++++++++++++++++++------- x/ccv/provider/client/cli/tx.go | 2 +- x/ccv/provider/keeper/double_vote.go | 66 ++++++++++++++--- x/ccv/provider/types/msg.go | 12 ++- 6 files changed, 150 insertions(+), 48 deletions(-) diff --git a/Dockerfile b/Dockerfile index 03939617be..ba351f2e78 100644 --- a/Dockerfile +++ b/Dockerfile @@ -36,7 +36,7 @@ FROM informalofftermatt/cometmock:latest as cometmock-builder # Get GoRelayer FROM informalofftermatt/gorelayer:nogas AS gorelayer-builder -FROM --platform=linux/amd64 fedora:36 +FROM --platform=linux/arm64 fedora:36 RUN dnf update -y RUN dnf install -y which iproute iputils procps-ng vim-minimal tmux net-tools htop jq USER root diff --git a/tests/e2e/main.go b/tests/e2e/main.go index 406a015e63..178118e64b 100644 --- a/tests/e2e/main.go +++ b/tests/e2e/main.go @@ -57,11 +57,11 @@ func main() { } testRuns := []testRunWithSteps{ - {ChangeoverTestRun(), changeoverSteps}, - {DefaultTestRun(), happyPathSteps}, - {DemocracyTestRun(true), democracySteps}, - {DemocracyTestRun(false), rewardDenomConsumerSteps}, - {SlashThrottleTestRun(), slashThrottleSteps}, + // {ChangeoverTestRun(), changeoverSteps}, + // {DefaultTestRun(), happyPathSteps}, + // {DemocracyTestRun(true), democracySteps}, + // {DemocracyTestRun(false), rewardDenomConsumerSteps}, + // {SlashThrottleTestRun(), slashThrottleSteps}, {ConsumerMisbehaviourTestRun(), consumerMisbehaviourSteps}, } if includeMultiConsumer != nil && *includeMultiConsumer { diff --git a/tests/integration/double_vote.go b/tests/integration/double_vote.go index 04d976ed61..e84f08c144 100644 --- a/tests/integration/double_vote.go +++ b/tests/integration/double_vote.go @@ -19,32 +19,58 @@ func (s *CCVTestSuite) TestHandleConsumerDoubleVoting() { s.setDefaultValSigningInfo(*v) } - valSet, err := tmtypes.ValidatorSetFromProto(s.consumerChain.LastHeader.ValidatorSet) + consuValSet, err := tmtypes.ValidatorSetFromProto(s.consumerChain.LastHeader.ValidatorSet) s.Require().NoError(err) + consuVal := consuValSet.Validators[0] + s.Require().NoError(err) + consuSigner := s.consumerChain.Signers[consuVal.Address.String()] - val := valSet.Validators[0] - signer := s.consumerChain.Signers[val.Address.String()] + prvovValSet, err := tmtypes.ValidatorSetFromProto(s.providerChain.LastHeader.ValidatorSet) + s.Require().NoError(err) + prvovVal := prvovValSet.Validators[0] + prvovSigner := s.providerChain.Signers[prvovVal.Address.String()] blockID1 := testutil.MakeBlockID([]byte("blockhash"), 1000, []byte("partshash")) blockID2 := testutil.MakeBlockID([]byte("blockhash2"), 1000, []byte("partshash")) // Note that votes are signed along with the chain ID // see VoteSignBytes in https://github.com/cometbft/cometbft/blob/main/types/vote.go#L139 - vote1 := testutil.MakeAndSignVote( + + // create two votes using the consumer validator key + consuVote := testutil.MakeAndSignVote( blockID1, s.consumerCtx().BlockHeight(), s.consumerCtx().BlockTime(), - valSet, - signer, + consuValSet, + consuSigner, s.consumerChain.ChainID, ) - badVote := testutil.MakeAndSignVote( + consuBadVote := testutil.MakeAndSignVote( blockID2, s.consumerCtx().BlockHeight(), s.consumerCtx().BlockTime(), - valSet, - signer, + consuValSet, + consuSigner, + s.consumerChain.ChainID, + ) + + // create two votes using the provider validator key + provVote := testutil.MakeAndSignVote( + blockID1, + s.consumerCtx().BlockHeight(), + s.consumerCtx().BlockTime(), + prvovValSet, + prvovSigner, + s.consumerChain.ChainID, + ) + + provBadVote := testutil.MakeAndSignVote( + blockID2, + s.consumerCtx().BlockHeight(), + s.consumerCtx().BlockTime(), + prvovValSet, + prvovSigner, s.consumerChain.ChainID, ) @@ -57,10 +83,10 @@ func (s *CCVTestSuite) TestHandleConsumerDoubleVoting() { { "invalid consumer chain id - shouldn't pass", &tmtypes.DuplicateVoteEvidence{ - VoteA: vote1, - VoteB: badVote, - ValidatorPower: val.VotingPower, - TotalVotingPower: val.VotingPower, + VoteA: consuVote, + VoteB: consuBadVote, + ValidatorPower: consuVal.VotingPower, + TotalVotingPower: consuVal.VotingPower, Timestamp: s.consumerCtx().BlockTime(), }, "chainID", @@ -68,12 +94,12 @@ func (s *CCVTestSuite) TestHandleConsumerDoubleVoting() { }, { // create an invalid evidence containing two identical votes - "invalid double voting evidence - shouldn't pass", + "invalid double voting evidence with identical votes - shouldn't pass", &tmtypes.DuplicateVoteEvidence{ - VoteA: vote1, - VoteB: vote1, - ValidatorPower: val.VotingPower, - TotalVotingPower: val.VotingPower, + VoteA: consuVote, + VoteB: consuVote, + ValidatorPower: consuVal.VotingPower, + TotalVotingPower: consuVal.VotingPower, Timestamp: s.consumerCtx().BlockTime(), }, s.consumerChain.ChainID, @@ -84,12 +110,26 @@ func (s *CCVTestSuite) TestHandleConsumerDoubleVoting() { // we create two votes that only differ by their Block IDs and // signed them using the same validator private key and chain ID // of the consumer chain - "valid double voting evidence - should pass", + "valid double voting evidence 1 - should pass", &tmtypes.DuplicateVoteEvidence{ - VoteA: vote1, - VoteB: badVote, - ValidatorPower: val.VotingPower, - TotalVotingPower: val.VotingPower, + VoteA: consuVote, + VoteB: consuBadVote, + ValidatorPower: consuVal.VotingPower, + TotalVotingPower: consuVal.VotingPower, + Timestamp: s.consumerCtx().BlockTime(), + }, + s.consumerChain.ChainID, + true, + }, + { + // create double voting evidence as if we didn't assign + // a new key to the validator on the consumer chain + "valid double voting evidence 2 - should pass", + &tmtypes.DuplicateVoteEvidence{ + VoteA: provVote, + VoteB: provBadVote, + ValidatorPower: consuVal.VotingPower, + TotalVotingPower: consuVal.VotingPower, Timestamp: s.consumerCtx().BlockTime(), }, s.consumerChain.ChainID, @@ -97,27 +137,39 @@ func (s *CCVTestSuite) TestHandleConsumerDoubleVoting() { }, } - consuAddr := types.NewConsumerConsAddress(sdk.ConsAddress(val.Address.Bytes())) + // consumer and provider address of the validator + consuAddr := types.NewConsumerConsAddress(sdk.ConsAddress(consuVal.Address.Bytes())) provAddr := s.providerApp.GetProviderKeeper().GetProviderAddrFromConsumerAddr(s.providerCtx(), s.consumerChain.ChainID, consuAddr) for _, tc := range testCases { s.Run(tc.name, func() { + // reset context for each run + provCtx := s.providerCtx() + + // remove key assigned if the validator provider key is used + if tc.ev.VoteA.ValidatorAddress.String() != consuVal.Address.String() { + s.providerApp.GetProviderKeeper().DeleteKeyAssignments(provCtx, s.consumerChain.ChainID) + } + err = s.providerApp.GetProviderKeeper().HandleConsumerDoubleVoting( - s.providerCtx(), + provCtx, tc.ev, tc.chainID, ) + if tc.expPass { s.Require().NoError(err) // verifies that the jailing has occurred - s.Require().True(s.providerApp.GetTestStakingKeeper().IsValidatorJailed(s.providerCtx(), provAddr.ToSdkConsAddr())) + s.Require().True(s.providerApp.GetTestStakingKeeper().IsValidatorJailed(provCtx, provAddr.ToSdkConsAddr())) } else { s.Require().Error(err) // verifies that no jailing and has occurred - s.Require().False(s.providerApp.GetTestStakingKeeper().IsValidatorJailed(s.providerCtx(), provAddr.ToSdkConsAddr())) + s.Require().False(s.providerApp.GetTestStakingKeeper().IsValidatorJailed(provCtx, provAddr.ToSdkConsAddr())) } }) } } + +// use new context diff --git a/x/ccv/provider/client/cli/tx.go b/x/ccv/provider/client/cli/tx.go index ed7c72b6b1..a4b6e233e0 100644 --- a/x/ccv/provider/client/cli/tx.go +++ b/x/ccv/provider/client/cli/tx.go @@ -153,7 +153,7 @@ Examples: func NewSubmitConsumerDoubleVotingCmd() *cobra.Command { cmd := &cobra.Command{ - Use: "submit consumer-double-voting [evidence] [infraction_header]", + Use: "submit-consumer-double-voting [evidence] [infraction_header]", Short: "submit a double voting evidence for a consumer chain", Args: cobra.ExactArgs(2), RunE: func(cmd *cobra.Command, args []string) error { diff --git a/x/ccv/provider/keeper/double_vote.go b/x/ccv/provider/keeper/double_vote.go index ee47e233ee..b1123e8b3b 100644 --- a/x/ccv/provider/keeper/double_vote.go +++ b/x/ccv/provider/keeper/double_vote.go @@ -5,17 +5,22 @@ import ( "fmt" cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec" + 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" ccvtypes "github.com/cosmos/interchain-security/v2/x/ccv/types" - tmprotocrypto "github.com/tendermint/tendermint/proto/tendermint/crypto" tmtypes "github.com/tendermint/tendermint/types" ) // HandleConsumerDoubleVoting verifies a double voting evidence for a given a consumer chain and, // if successful, executes the jailing of the malicious validator. func (k Keeper) HandleConsumerDoubleVoting(ctx sdk.Context, evidence *tmtypes.DuplicateVoteEvidence, chainID string) error { + + k.Logger(ctx).Info("received double voting evidence", "chain: ", chainID, "evidence: ", evidence, "current block heigt", + ctx.BlockHeight()) + // get the validator's consensus address on the provider providerAddr := k.GetProviderAddrFromConsumerAddr( ctx, @@ -24,13 +29,26 @@ func (k Keeper) HandleConsumerDoubleVoting(ctx sdk.Context, evidence *tmtypes.Du ) // get the consumer chain public key assigned to the validator - consuPubkey, ok := k.GetValidatorConsumerPubKey(ctx, chainID, providerAddr) - if !ok { + pubkey, err := k.getValidatorConsumerPubKey(ctx, chainID, providerAddr) + if err != nil { + // val, ok := k.stakingKeeper.GetValidatorByConsAddr(ctx, providerAddr.ToSdkConsAddr()) + // if !ok { + // return fmt.Errorf("cannot find validator %s", providerAddr.String()) + // } + // pk, err := val.ConsPubKey() + // if err != nil { + // return fmt.Errorf("cannot find public key for validator %s", &providerAddr) + // } + + // pubkey, err = cryptocodec.ToTmProtoPublicKey(pk) + // if err != nil { + // return fmt.Errorf("cannot convert public key for validator %s", &providerAddr) + // } return fmt.Errorf("cannot find public key for validator %s and consumer chain %s", providerAddr.String(), chainID) } // verifies the double voting evidence using the consumer chain public key - if err := k.VerifyDoubleVotingEvidence(ctx, *evidence, chainID, consuPubkey); err != nil { + if err := k.VerifyDoubleVotingEvidence(ctx, *evidence, chainID, pubkey); err != nil { return err } @@ -51,7 +69,7 @@ func (k Keeper) VerifyDoubleVotingEvidence( ctx sdk.Context, evidence tmtypes.DuplicateVoteEvidence, chainID string, - pubkey tmprotocrypto.PublicKey, + pubkey cryptotypes.PubKey, ) error { // Note that since we're only jailing validators for double voting on a consumer chain, // the age of the evidence is irrelevant and therefore isn't checked. @@ -89,21 +107,45 @@ func (k Keeper) VerifyDoubleVotingEvidence( ) } - pk, err := cryptocodec.FromTmProtoPublicKey(pubkey) - if err != nil { - return err - } - va := evidence.VoteA.ToProto() vb := evidence.VoteB.ToProto() // signatures must be valid - if !pk.VerifySignature(tmtypes.VoteSignBytes(chainID, va), evidence.VoteA.Signature) { + if !pubkey.VerifySignature(tmtypes.VoteSignBytes(chainID, va), evidence.VoteA.Signature) { return fmt.Errorf("verifying VoteA: %w", tmtypes.ErrVoteInvalidSignature) } - if !pk.VerifySignature(tmtypes.VoteSignBytes(chainID, vb), evidence.VoteB.Signature) { + if !pubkey.VerifySignature(tmtypes.VoteSignBytes(chainID, vb), evidence.VoteB.Signature) { return fmt.Errorf("verifying VoteB: %w", tmtypes.ErrVoteInvalidSignature) } return nil } + +// getValidatorConsumerPubKey returns the public key used by the given validator +// on a consumer chain +func (k Keeper) getValidatorConsumerPubKey( + ctx sdk.Context, + chainID string, + providerAddr types.ProviderConsAddress, +) (pubkey cryptotypes.PubKey, err error) { + // get the consumer chain public key assigned to the validator + tmPK, ok := k.GetValidatorConsumerPubKey(ctx, chainID, providerAddr) + if ok { + pubkey, err = cryptocodec.FromTmProtoPublicKey(tmPK) + if err != nil { + return + } + } else { + val, ok := k.stakingKeeper.GetValidatorByConsAddr(ctx, providerAddr.ToSdkConsAddr()) + if !ok { + err = fmt.Errorf("cannot find validator %s", providerAddr.String()) + return + } + pubkey, err = val.ConsPubKey() + if err != nil { + return + } + } + + return +} diff --git a/x/ccv/provider/types/msg.go b/x/ccv/provider/types/msg.go index e5dbb16697..4e2b3dc9e9 100644 --- a/x/ccv/provider/types/msg.go +++ b/x/ccv/provider/types/msg.go @@ -210,8 +210,16 @@ func (msg MsgSubmitConsumerDoubleVoting) ValidateBasic() error { return fmt.Errorf("double voting evidence cannot be nil") } - if msg.InfractionBlockHeader.Header == nil { - return fmt.Errorf("infraction header cannot be nil") + if msg.InfractionBlockHeader == nil { + return fmt.Errorf("infraction block header cannot be nil") + } + + if msg.InfractionBlockHeader.SignedHeader == nil { + return fmt.Errorf("signed header in infraction block header cannot be nil") + } + + if msg.InfractionBlockHeader.SignedHeader.Header.ChainID == "" { + return fmt.Errorf("chain ID of signed header in infraction block header cannot be empty") } return nil From 0f94fcb0eaa67d19feaaefd22b090c87cb6ed1b3 Mon Sep 17 00:00:00 2001 From: Simon Noetzlin Date: Thu, 31 Aug 2023 17:29:38 +0200 Subject: [PATCH 3/6] godoc --- tests/integration/double_vote.go | 2 -- x/ccv/provider/keeper/double_vote.go | 32 ++++++++--------------- x/ccv/provider/keeper/double_vote_test.go | 12 ++++----- 3 files changed, 17 insertions(+), 29 deletions(-) diff --git a/tests/integration/double_vote.go b/tests/integration/double_vote.go index e84f08c144..1248d59088 100644 --- a/tests/integration/double_vote.go +++ b/tests/integration/double_vote.go @@ -171,5 +171,3 @@ func (s *CCVTestSuite) TestHandleConsumerDoubleVoting() { }) } } - -// use new context diff --git a/x/ccv/provider/keeper/double_vote.go b/x/ccv/provider/keeper/double_vote.go index b1123e8b3b..9db6166444 100644 --- a/x/ccv/provider/keeper/double_vote.go +++ b/x/ccv/provider/keeper/double_vote.go @@ -28,23 +28,10 @@ func (k Keeper) HandleConsumerDoubleVoting(ctx sdk.Context, evidence *tmtypes.Du types.NewConsumerConsAddress(sdk.ConsAddress(evidence.VoteA.ValidatorAddress.Bytes())), ) - // get the consumer chain public key assigned to the validator - pubkey, err := k.getValidatorConsumerPubKey(ctx, chainID, providerAddr) + // get validator pubkey used on the consumer chain + pubkey, err := k.getValidatorPubkeyOnConsumer(ctx, chainID, providerAddr) if err != nil { - // val, ok := k.stakingKeeper.GetValidatorByConsAddr(ctx, providerAddr.ToSdkConsAddr()) - // if !ok { - // return fmt.Errorf("cannot find validator %s", providerAddr.String()) - // } - // pk, err := val.ConsPubKey() - // if err != nil { - // return fmt.Errorf("cannot find public key for validator %s", &providerAddr) - // } - - // pubkey, err = cryptocodec.ToTmProtoPublicKey(pk) - // if err != nil { - // return fmt.Errorf("cannot convert public key for validator %s", &providerAddr) - // } - return fmt.Errorf("cannot find public key for validator %s and consumer chain %s", providerAddr.String(), chainID) + return err } // verifies the double voting evidence using the consumer chain public key @@ -57,7 +44,7 @@ func (k Keeper) HandleConsumerDoubleVoting(ctx sdk.Context, evidence *tmtypes.Du k.Logger(ctx).Info( "confirmed equivocation", - "byzantine validator address", providerAddr, + "byzantine validator address", providerAddr.String(), ) return nil @@ -71,6 +58,10 @@ func (k Keeper) VerifyDoubleVotingEvidence( chainID string, pubkey cryptotypes.PubKey, ) error { + if pubkey == nil { + return fmt.Errorf("validator public key cannot be empty") + } + // Note that since we're only jailing validators for double voting on a consumer chain, // the age of the evidence is irrelevant and therefore isn't checked. @@ -121,14 +112,13 @@ func (k Keeper) VerifyDoubleVotingEvidence( return nil } -// getValidatorConsumerPubKey returns the public key used by the given validator -// on a consumer chain -func (k Keeper) getValidatorConsumerPubKey( +// getValidatorPubkeyOnConsumer returns the public key a validator used on a given consumer chain. +// It can either be an assigned consumer public key or the primary validator public key. +func (k Keeper) getValidatorPubkeyOnConsumer( ctx sdk.Context, chainID string, providerAddr types.ProviderConsAddress, ) (pubkey cryptotypes.PubKey, err error) { - // get the consumer chain public key assigned to the validator tmPK, ok := k.GetValidatorConsumerPubKey(ctx, chainID, providerAddr) if ok { pubkey, err = cryptocodec.FromTmProtoPublicKey(tmPK) diff --git a/x/ccv/provider/keeper/double_vote_test.go b/x/ccv/provider/keeper/double_vote_test.go index 9bb789c381..cc8280b14d 100644 --- a/x/ccv/provider/keeper/double_vote_test.go +++ b/x/ccv/provider/keeper/double_vote_test.go @@ -4,11 +4,11 @@ import ( "testing" "time" + cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec" + cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" testutil "github.com/cosmos/interchain-security/v2/testutil/crypto" testkeeper "github.com/cosmos/interchain-security/v2/testutil/keeper" "github.com/stretchr/testify/require" - tmencoding "github.com/tendermint/tendermint/crypto/encoding" - tmprotocrypto "github.com/tendermint/tendermint/proto/tendermint/crypto" tmtypes "github.com/tendermint/tendermint/types" ) @@ -31,17 +31,17 @@ func TestVerifyDoubleVotingEvidence(t *testing.T) { ctx = ctx.WithBlockTime(time.Now()) - valPubkey1, err := tmencoding.PubKeyToProto(val1.PubKey) + valPubkey1, err := cryptocodec.FromTmPubKeyInterface(val1.PubKey) require.NoError(t, err) - valPubkey2, err := tmencoding.PubKeyToProto(val2.PubKey) + valPubkey2, err := cryptocodec.FromTmPubKeyInterface(val2.PubKey) require.NoError(t, err) testCases := []struct { name string votes []*tmtypes.Vote chainID string - pubkey tmprotocrypto.PublicKey + pubkey cryptotypes.PubKey expPass bool }{ { @@ -209,7 +209,7 @@ func TestVerifyDoubleVotingEvidence(t *testing.T) { ), }, chainID, - tmprotocrypto.PublicKey{}, + nil, false, }, { From 81ed2b2a2c62b2eb8ca60b40834e8cfefca2cf9c Mon Sep 17 00:00:00 2001 From: Simon Noetzlin Date: Thu, 31 Aug 2023 17:30:34 +0200 Subject: [PATCH 4/6] nits --- Dockerfile | 2 +- tests/e2e/main.go | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Dockerfile b/Dockerfile index ba351f2e78..03939617be 100644 --- a/Dockerfile +++ b/Dockerfile @@ -36,7 +36,7 @@ FROM informalofftermatt/cometmock:latest as cometmock-builder # Get GoRelayer FROM informalofftermatt/gorelayer:nogas AS gorelayer-builder -FROM --platform=linux/arm64 fedora:36 +FROM --platform=linux/amd64 fedora:36 RUN dnf update -y RUN dnf install -y which iproute iputils procps-ng vim-minimal tmux net-tools htop jq USER root diff --git a/tests/e2e/main.go b/tests/e2e/main.go index 178118e64b..406a015e63 100644 --- a/tests/e2e/main.go +++ b/tests/e2e/main.go @@ -57,11 +57,11 @@ func main() { } testRuns := []testRunWithSteps{ - // {ChangeoverTestRun(), changeoverSteps}, - // {DefaultTestRun(), happyPathSteps}, - // {DemocracyTestRun(true), democracySteps}, - // {DemocracyTestRun(false), rewardDenomConsumerSteps}, - // {SlashThrottleTestRun(), slashThrottleSteps}, + {ChangeoverTestRun(), changeoverSteps}, + {DefaultTestRun(), happyPathSteps}, + {DemocracyTestRun(true), democracySteps}, + {DemocracyTestRun(false), rewardDenomConsumerSteps}, + {SlashThrottleTestRun(), slashThrottleSteps}, {ConsumerMisbehaviourTestRun(), consumerMisbehaviourSteps}, } if includeMultiConsumer != nil && *includeMultiConsumer { From bb78f8fac6ad718e506dabec43bd2c252e9ded0b Mon Sep 17 00:00:00 2001 From: Simon Noetzlin Date: Fri, 1 Sep 2023 17:36:34 +0200 Subject: [PATCH 5/6] lint --- x/ccv/provider/keeper/double_vote.go | 1 - 1 file changed, 1 deletion(-) diff --git a/x/ccv/provider/keeper/double_vote.go b/x/ccv/provider/keeper/double_vote.go index 9db6166444..d23056d467 100644 --- a/x/ccv/provider/keeper/double_vote.go +++ b/x/ccv/provider/keeper/double_vote.go @@ -17,7 +17,6 @@ import ( // HandleConsumerDoubleVoting verifies a double voting evidence for a given a consumer chain and, // if successful, executes the jailing of the malicious validator. func (k Keeper) HandleConsumerDoubleVoting(ctx sdk.Context, evidence *tmtypes.DuplicateVoteEvidence, chainID string) error { - k.Logger(ctx).Info("received double voting evidence", "chain: ", chainID, "evidence: ", evidence, "current block heigt", ctx.BlockHeight()) From 0b6d75a934d2986025e131b8d396fae203334e5f Mon Sep 17 00:00:00 2001 From: Simon Noetzlin Date: Fri, 1 Sep 2023 18:43:32 +0200 Subject: [PATCH 6/6] nit --- tests/integration/double_vote.go | 22 +++++++++++----------- x/ccv/provider/keeper/double_vote.go | 6 ++---- x/ccv/provider/types/msg.go | 4 ++-- 3 files changed, 15 insertions(+), 17 deletions(-) diff --git a/tests/integration/double_vote.go b/tests/integration/double_vote.go index 1248d59088..184f7604bb 100644 --- a/tests/integration/double_vote.go +++ b/tests/integration/double_vote.go @@ -25,10 +25,10 @@ func (s *CCVTestSuite) TestHandleConsumerDoubleVoting() { s.Require().NoError(err) consuSigner := s.consumerChain.Signers[consuVal.Address.String()] - prvovValSet, err := tmtypes.ValidatorSetFromProto(s.providerChain.LastHeader.ValidatorSet) + provValSet, err := tmtypes.ValidatorSetFromProto(s.providerChain.LastHeader.ValidatorSet) s.Require().NoError(err) - prvovVal := prvovValSet.Validators[0] - prvovSigner := s.providerChain.Signers[prvovVal.Address.String()] + provVal := provValSet.Validators[0] + provSigner := s.providerChain.Signers[provVal.Address.String()] blockID1 := testutil.MakeBlockID([]byte("blockhash"), 1000, []byte("partshash")) blockID2 := testutil.MakeBlockID([]byte("blockhash2"), 1000, []byte("partshash")) @@ -60,8 +60,8 @@ func (s *CCVTestSuite) TestHandleConsumerDoubleVoting() { blockID1, s.consumerCtx().BlockHeight(), s.consumerCtx().BlockTime(), - prvovValSet, - prvovSigner, + provValSet, + provSigner, s.consumerChain.ChainID, ) @@ -69,8 +69,8 @@ func (s *CCVTestSuite) TestHandleConsumerDoubleVoting() { blockID2, s.consumerCtx().BlockHeight(), s.consumerCtx().BlockTime(), - prvovValSet, - prvovSigner, + provValSet, + provSigner, s.consumerChain.ChainID, ) @@ -122,8 +122,7 @@ func (s *CCVTestSuite) TestHandleConsumerDoubleVoting() { true, }, { - // create double voting evidence as if we didn't assign - // a new key to the validator on the consumer chain + // create a double voting evidence using the provider validator key "valid double voting evidence 2 - should pass", &tmtypes.DuplicateVoteEvidence{ VoteA: provVote, @@ -137,7 +136,6 @@ func (s *CCVTestSuite) TestHandleConsumerDoubleVoting() { }, } - // consumer and provider address of the validator consuAddr := types.NewConsumerConsAddress(sdk.ConsAddress(consuVal.Address.Bytes())) provAddr := s.providerApp.GetProviderKeeper().GetProviderAddrFromConsumerAddr(s.providerCtx(), s.consumerChain.ChainID, consuAddr) @@ -146,7 +144,9 @@ func (s *CCVTestSuite) TestHandleConsumerDoubleVoting() { // reset context for each run provCtx := s.providerCtx() - // remove key assigned if the validator provider key is used + // if the evidence was built using the validator provider address and key, + // we remove the consumer key assigned to the validator otherwise + // HandleConsumerDoubleVoting uses the consumer key to verify the signature if tc.ev.VoteA.ValidatorAddress.String() != consuVal.Address.String() { s.providerApp.GetProviderKeeper().DeleteKeyAssignments(provCtx, s.consumerChain.ChainID) } diff --git a/x/ccv/provider/keeper/double_vote.go b/x/ccv/provider/keeper/double_vote.go index d23056d467..171be6b250 100644 --- a/x/ccv/provider/keeper/double_vote.go +++ b/x/ccv/provider/keeper/double_vote.go @@ -17,9 +17,6 @@ import ( // HandleConsumerDoubleVoting verifies a double voting evidence for a given a consumer chain and, // if successful, executes the jailing of the malicious validator. func (k Keeper) HandleConsumerDoubleVoting(ctx sdk.Context, evidence *tmtypes.DuplicateVoteEvidence, chainID string) error { - k.Logger(ctx).Info("received double voting evidence", "chain: ", chainID, "evidence: ", evidence, "current block heigt", - ctx.BlockHeight()) - // get the validator's consensus address on the provider providerAddr := k.GetProviderAddrFromConsumerAddr( ctx, @@ -112,7 +109,8 @@ func (k Keeper) VerifyDoubleVotingEvidence( } // getValidatorPubkeyOnConsumer returns the public key a validator used on a given consumer chain. -// It can either be an assigned consumer public key or the primary validator public key. +// Note that it can either be an assigned public key or the same public key the validator uses +// on the provider chain. func (k Keeper) getValidatorPubkeyOnConsumer( ctx sdk.Context, chainID string, diff --git a/x/ccv/provider/types/msg.go b/x/ccv/provider/types/msg.go index 4e2b3dc9e9..b6741a0a43 100644 --- a/x/ccv/provider/types/msg.go +++ b/x/ccv/provider/types/msg.go @@ -218,8 +218,8 @@ func (msg MsgSubmitConsumerDoubleVoting) ValidateBasic() error { return fmt.Errorf("signed header in infraction block header cannot be nil") } - if msg.InfractionBlockHeader.SignedHeader.Header.ChainID == "" { - return fmt.Errorf("chain ID of signed header in infraction block header cannot be empty") + if msg.InfractionBlockHeader.SignedHeader.Header == nil { + return fmt.Errorf("invalid signed header in infraction block header, 'SignedHeader.Header' is nil") } return nil