From 2501e83e9678f16bda19ce2187dda7abaa806eed Mon Sep 17 00:00:00 2001 From: Simon Noetzlin Date: Mon, 4 Sep 2023 16:57:43 +0200 Subject: [PATCH] fix: make `HandleConsumerDoubleVoting` works with provider pubkeys (#1254) * fix double voting cli * fix bug double signing handler * godoc * nits * lint * nit --- tests/integration/double_vote.go | 104 ++++++++++++++++------ x/ccv/provider/keeper/double_vote.go | 59 ++++++++---- x/ccv/provider/keeper/double_vote_test.go | 12 +-- x/ccv/provider/types/msg.go | 12 ++- 4 files changed, 137 insertions(+), 50 deletions(-) diff --git a/tests/integration/double_vote.go b/tests/integration/double_vote.go index 04d976ed61..184f7604bb 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()] + provValSet, err := tmtypes.ValidatorSetFromProto(s.providerChain.LastHeader.ValidatorSet) + s.Require().NoError(err) + 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")) // 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(), + consuValSet, + consuSigner, + s.consumerChain.ChainID, + ) + + consuBadVote := testutil.MakeAndSignVote( + blockID2, + s.consumerCtx().BlockHeight(), + s.consumerCtx().BlockTime(), + consuValSet, + consuSigner, + s.consumerChain.ChainID, + ) + + // create two votes using the provider validator key + provVote := testutil.MakeAndSignVote( blockID1, s.consumerCtx().BlockHeight(), s.consumerCtx().BlockTime(), - valSet, - signer, + provValSet, + provSigner, s.consumerChain.ChainID, ) - badVote := testutil.MakeAndSignVote( + provBadVote := testutil.MakeAndSignVote( blockID2, s.consumerCtx().BlockHeight(), s.consumerCtx().BlockTime(), - valSet, - signer, + provValSet, + provSigner, 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,25 @@ 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 a double voting evidence using the provider validator key + "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,26 +136,37 @@ func (s *CCVTestSuite) TestHandleConsumerDoubleVoting() { }, } - consuAddr := types.NewConsumerConsAddress(sdk.ConsAddress(val.Address.Bytes())) + 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() + + // 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) + } + 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())) } }) } diff --git a/x/ccv/provider/keeper/double_vote.go b/x/ccv/provider/keeper/double_vote.go index ee47e233ee..171be6b250 100644 --- a/x/ccv/provider/keeper/double_vote.go +++ b/x/ccv/provider/keeper/double_vote.go @@ -5,11 +5,12 @@ 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" ) @@ -23,14 +24,14 @@ 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 - consuPubkey, ok := k.GetValidatorConsumerPubKey(ctx, chainID, providerAddr) - if !ok { - return fmt.Errorf("cannot find public key for validator %s and consumer chain %s", providerAddr.String(), chainID) + // get validator pubkey used on the consumer chain + pubkey, err := k.getValidatorPubkeyOnConsumer(ctx, chainID, providerAddr) + if err != nil { + return err } // 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 } @@ -39,7 +40,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 @@ -51,8 +52,12 @@ func (k Keeper) VerifyDoubleVotingEvidence( ctx sdk.Context, evidence tmtypes.DuplicateVoteEvidence, chainID string, - pubkey tmprotocrypto.PublicKey, + 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. @@ -89,21 +94,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 } + +// getValidatorPubkeyOnConsumer returns the public key a validator used on a given consumer chain. +// 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, + providerAddr types.ProviderConsAddress, +) (pubkey cryptotypes.PubKey, err error) { + 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/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, }, { diff --git a/x/ccv/provider/types/msg.go b/x/ccv/provider/types/msg.go index e5dbb16697..b6741a0a43 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 == nil { + return fmt.Errorf("invalid signed header in infraction block header, 'SignedHeader.Header' is nil") } return nil