diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 736e299c4..9064cee60 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -10,6 +10,8 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi - CLI/RPC/Config + - [evidence] \#4725 Remove `Pubkey` from DuplicateVoteEvidence + - Apps - P2P Protocol diff --git a/evidence/reactor_test.go b/evidence/reactor_test.go index bdbd28810..eeedae2d1 100644 --- a/evidence/reactor_test.go +++ b/evidence/reactor_test.go @@ -13,7 +13,6 @@ import ( dbm "github.com/tendermint/tm-db" cfg "github.com/tendermint/tendermint/config" - "github.com/tendermint/tendermint/crypto/secp256k1" "github.com/tendermint/tendermint/libs/log" "github.com/tendermint/tendermint/p2p" sm "github.com/tendermint/tendermint/state" @@ -214,7 +213,7 @@ func TestListMessageValidationBasic(t *testing.T) { {"Good ListMessage", func(evList *ListMessage) {}, false}, {"Invalid ListMessage", func(evList *ListMessage) { evList.Evidence = append(evList.Evidence, - &types.DuplicateVoteEvidence{PubKey: secp256k1.GenPrivKey().PubKey()}) + &types.DuplicateVoteEvidence{}) }, true}, } for _, tc := range testCases { diff --git a/rpc/client/evidence_test.go b/rpc/client/evidence_test.go index 3bead4592..153506ed6 100644 --- a/rpc/client/evidence_test.go +++ b/rpc/client/evidence_test.go @@ -29,7 +29,7 @@ func newEvidence(t *testing.T, val *privval.FilePV, vote2.Signature, err = val.Key.PrivKey.Sign(vote2.SignBytes(chainID)) require.NoError(t, err) - return types.NewDuplicateVoteEvidence(val.Key.PubKey, vote, vote2) + return types.NewDuplicateVoteEvidence(vote, vote2) } func makeEvidences( @@ -123,7 +123,7 @@ func TestBroadcastEvidence_DuplicateVoteEvidence(t *testing.T) { require.NoError(t, err) client.WaitForHeight(c, status.SyncInfo.LatestBlockHeight+2, nil) - ed25519pub := correct.PubKey.(ed25519.PubKeyEd25519) + ed25519pub := pv.Key.PubKey.(ed25519.PubKeyEd25519) rawpub := ed25519pub[:] result2, err := c.ABCIQuery("/val", rawpub) require.NoError(t, err) diff --git a/types/evidence.go b/types/evidence.go index 9dd7c9823..d63a13c5f 100644 --- a/types/evidence.go +++ b/types/evidence.go @@ -19,7 +19,7 @@ import ( const ( // MaxEvidenceBytes is a maximum size of any evidence (including amino overhead). - MaxEvidenceBytes int64 = 484 + MaxEvidenceBytes int64 = 444 // An invalid field in the header from LunaticValidatorEvidence. // Must be a function of the ABCI application state. @@ -117,16 +117,15 @@ func MaxEvidencePerBlock(blockMaxBytes int64) (int64, int64) { // DuplicateVoteEvidence contains evidence a validator signed two conflicting // votes. type DuplicateVoteEvidence struct { - PubKey crypto.PubKey - VoteA *Vote - VoteB *Vote + VoteA *Vote + VoteB *Vote } var _ Evidence = &DuplicateVoteEvidence{} // NewDuplicateVoteEvidence creates DuplicateVoteEvidence with right ordering given // two conflicting votes. If one of the votes is nil, evidence returned is nil as well -func NewDuplicateVoteEvidence(pubkey crypto.PubKey, vote1 *Vote, vote2 *Vote) *DuplicateVoteEvidence { +func NewDuplicateVoteEvidence(vote1 *Vote, vote2 *Vote) *DuplicateVoteEvidence { var voteA, voteB *Vote if vote1 == nil || vote2 == nil { return nil @@ -139,9 +138,8 @@ func NewDuplicateVoteEvidence(pubkey crypto.PubKey, vote1 *Vote, vote2 *Vote) *D voteB = vote1 } return &DuplicateVoteEvidence{ - PubKey: pubkey, - VoteA: voteA, - VoteB: voteB, + VoteA: voteA, + VoteB: voteB, } } @@ -163,7 +161,7 @@ func (dve *DuplicateVoteEvidence) Time() time.Time { // Address returns the address of the validator. func (dve *DuplicateVoteEvidence) Address() []byte { - return dve.PubKey.Address() + return dve.VoteA.ValidatorAddress } // Hash returns the hash of the evidence. @@ -247,9 +245,6 @@ func (dve *DuplicateVoteEvidence) Equal(ev Evidence) bool { // ValidateBasic performs basic validation. func (dve *DuplicateVoteEvidence) ValidateBasic() error { - if len(dve.PubKey.Bytes()) == 0 { - return errors.New("empty PubKey") - } if dve.VoteA == nil || dve.VoteB == nil { return fmt.Errorf("one or both of the votes are empty %v, %v", dve.VoteA, dve.VoteB) } @@ -424,9 +419,8 @@ OUTER_LOOP: // immediately slashable (#F1). if ev.H1.Commit.Round == ev.H2.Commit.Round { evList = append(evList, &DuplicateVoteEvidence{ - PubKey: val.PubKey, - VoteA: ev.H1.Commit.GetVote(i), - VoteB: ev.H2.Commit.GetVote(j), + VoteA: ev.H1.Commit.GetVote(i), + VoteB: ev.H2.Commit.GetVote(j), }) } else { // if H1.Round != H2.Round we need to run full detection procedure => not diff --git a/types/evidence_test.go b/types/evidence_test.go index 54ac79884..099a3cc99 100644 --- a/types/evidence_test.go +++ b/types/evidence_test.go @@ -10,7 +10,6 @@ import ( "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto/ed25519" - "github.com/tendermint/tendermint/crypto/secp256k1" "github.com/tendermint/tendermint/crypto/tmhash" tmrand "github.com/tendermint/tendermint/libs/rand" ) @@ -108,15 +107,49 @@ func TestMaxEvidenceBytes(t *testing.T) { blockID2 := makeBlockID(tmhash.Sum([]byte("blockhash2")), math.MaxInt64, tmhash.Sum([]byte("partshash"))) const chainID = "mychain" ev := &DuplicateVoteEvidence{ - PubKey: secp256k1.GenPrivKey().PubKey(), // use secp because it's pubkey is longer - VoteA: makeVote(t, val, chainID, math.MaxInt64, math.MaxInt64, math.MaxInt64, math.MaxInt64, blockID), - VoteB: makeVote(t, val, chainID, math.MaxInt64, math.MaxInt64, math.MaxInt64, math.MaxInt64, blockID2), + VoteA: makeVote(t, val, chainID, math.MaxInt64, math.MaxInt64, math.MaxInt64, math.MaxInt64, blockID), + VoteB: makeVote(t, val, chainID, math.MaxInt64, math.MaxInt64, math.MaxInt64, math.MaxInt64, blockID2), } - bz, err := cdc.MarshalBinaryLengthPrefixed(ev) - require.NoError(t, err) + //TODO: Add other types of evidence to test and set MaxEvidenceBytes accordingly + + // evl := &LunaticValidatorEvidence{ + // Header: makeHeaderRandom(), + // Vote: makeVote(t, val, chainID, math.MaxInt64, math.MaxInt64, math.MaxInt64, math.MaxInt64, blockID2), + + // InvalidHeaderField: "", + // } + + // evp := &PhantomValidatorEvidence{ + // Header: makeHeaderRandom(), + // Vote: makeVote(t, val, chainID, math.MaxInt64, math.MaxInt64, math.MaxInt64, math.MaxInt64, blockID2), + + // LastHeightValidatorWasInSet: math.MaxInt64, + // } + + // signedHeader := SignedHeader{Header: makeHeaderRandom(), Commit: randCommit(time.Now())} + // evc := &ConflictingHeadersEvidence{ + // H1: &signedHeader, + // H2: &signedHeader, + // } + + testCases := []struct { + testName string + evidence Evidence + }{ + {"DuplicateVote", ev}, + // {"LunaticValidatorEvidence", evl}, + // {"PhantomValidatorEvidence", evp}, + // {"ConflictingHeadersEvidence", evc}, + } + + for _, tt := range testCases { + bz, err := cdc.MarshalBinaryLengthPrefixed(tt.evidence) + require.NoError(t, err, tt.testName) + + assert.LessOrEqual(t, MaxEvidenceBytes, int64(len(bz)), tt.testName) + } - assert.EqualValues(t, MaxEvidenceBytes, len(bz)) } func randomDuplicatedVoteEvidence(t *testing.T) *DuplicateVoteEvidence { @@ -160,10 +193,9 @@ func TestDuplicateVoteEvidenceValidation(t *testing.T) { for _, tc := range testCases { tc := tc t.Run(tc.testName, func(t *testing.T) { - pk := secp256k1.GenPrivKey().PubKey() vote1 := makeVote(t, val, chainID, math.MaxInt64, math.MaxInt64, math.MaxInt64, 0x02, blockID) vote2 := makeVote(t, val, chainID, math.MaxInt64, math.MaxInt64, math.MaxInt64, 0x02, blockID2) - ev := NewDuplicateVoteEvidence(pk, vote1, vote2) + ev := NewDuplicateVoteEvidence(vote1, vote2) tc.malleateEvidence(ev) assert.Equal(t, tc.expectErr, ev.ValidateBasic() != nil, "Validate Basic had an unexpected result") }) diff --git a/types/protobuf_test.go b/types/protobuf_test.go index 6f6e6198b..a3fce9396 100644 --- a/types/protobuf_test.go +++ b/types/protobuf_test.go @@ -136,9 +136,8 @@ func TestABCIEvidence(t *testing.T) { pubKey, err := val.GetPubKey() require.NoError(t, err) ev := &DuplicateVoteEvidence{ - PubKey: pubKey, - VoteA: makeVote(t, val, chainID, 0, 10, 2, 1, blockID), - VoteB: makeVote(t, val, chainID, 0, 10, 2, 1, blockID2), + VoteA: makeVote(t, val, chainID, 0, 10, 2, 1, blockID), + VoteB: makeVote(t, val, chainID, 0, 10, 2, 1, blockID2), } abciEv := TM2PB.Evidence( ev, diff --git a/types/vote.go b/types/vote.go index 37520fec3..807519518 100644 --- a/types/vote.go +++ b/types/vote.go @@ -31,12 +31,12 @@ type ErrVoteConflictingVotes struct { } func (err *ErrVoteConflictingVotes) Error() string { - return fmt.Sprintf("conflicting votes from validator %X", err.PubKey.Address()) + return fmt.Sprintf("conflicting votes from validator %X", err.VoteA.ValidatorAddress) } func NewConflictingVoteError(val *Validator, vote1, vote2 *Vote) *ErrVoteConflictingVotes { return &ErrVoteConflictingVotes{ - NewDuplicateVoteEvidence(val.PubKey, vote1, vote2), + NewDuplicateVoteEvidence(vote1, vote2), } }