From e9d55381079af1fdd22bc0f4cba2812e79f88967 Mon Sep 17 00:00:00 2001 From: Callum Michael Waters Date: Tue, 14 Apr 2020 13:40:59 +0200 Subject: [PATCH 01/14] created docs.go for evidence package --- evidence/doc.go | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 evidence/doc.go diff --git a/evidence/doc.go b/evidence/doc.go new file mode 100644 index 000000000..0dc5ecfca --- /dev/null +++ b/evidence/doc.go @@ -0,0 +1,36 @@ +/* +Package evidence handles all evidence storage and gossiping from detection to block proposal. +For the different types of evidence refer to the `evidence.go` file in the types package. + +## Gossiping + +The core functionality begins with the evidence reactor (see reactor. +go) which operates both the sending and receiving of evidence. + +The `Receive` function takes a list of evidence and does the following: + 1. breaks it down into individual evidence if it is `Composite Evidence` (see types/evidence. +go#ConflictingHeadersEvidence) + 2. checks that it does not already have the evidence stored + 3. verifies the evidence against the nodes state (see state/validation.go#VerifyEvidence) + 4. stores the evidence to a db and a concurrent list + +The gossiping of evidence is initiated when a peer is added which starts a go routine to broadcast currently +uncommitted evidence at intervals of 60 seconds (set by the by broadcastEvidenceIntervalS). +It uses a concurrent list to store the evidence and before sending verifies that each evidence is still valid in the +sense that it has not exceeded the max evidence age and height which should be set to be equal to the "trusting +period" (see types/params.go#EvidenceParams). + +## Proposing + +When a new block is being proposed (in state/execution.go#CreateProposalBlock), +`PendingEvidence(maxNum)` is called to send up to the maxNum number of uncommitted evidence, from the evidence store, +based on a priority that is a product of the age of the evidence and the voting power of the malicious validator. + +Once the proposed evidence is submitted, +the evidence is marked as committed and is moved from the broadcasted set to the committed set ( +the committed set is used to verify whether new evidence has actually already been submitted). +As a result it is also removed from the concurrent list so that it is no longer gossiped. + +Last Update: 14/04/20 +*/ +package evidence From b524b61252c3e86f4016a2344211855fe510cc7b Mon Sep 17 00:00:00 2001 From: Callum Date: Thu, 16 Apr 2020 12:28:25 +0200 Subject: [PATCH 02/14] clean up doc --- evidence/doc.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/evidence/doc.go b/evidence/doc.go index 0dc5ecfca..a669b7a28 100644 --- a/evidence/doc.go +++ b/evidence/doc.go @@ -1,6 +1,7 @@ /* Package evidence handles all evidence storage and gossiping from detection to block proposal. -For the different types of evidence refer to the `evidence.go` file in the types package. +For the different types of evidence refer to the `evidence.go` file in the types package +or https://github.com/tendermint/spec/blob/master/spec/consensus/light-client/accountability.md. ## Gossiping @@ -17,7 +18,7 @@ go#ConflictingHeadersEvidence) The gossiping of evidence is initiated when a peer is added which starts a go routine to broadcast currently uncommitted evidence at intervals of 60 seconds (set by the by broadcastEvidenceIntervalS). It uses a concurrent list to store the evidence and before sending verifies that each evidence is still valid in the -sense that it has not exceeded the max evidence age and height which should be set to be equal to the "trusting +sense that it has not exceeded the max evidence age and height. This should be set to be equal to the "trusting period" (see types/params.go#EvidenceParams). ## Proposing From b25faa761f740ed915f52beb104931197e357df3 Mon Sep 17 00:00:00 2001 From: Tess Rinearson Date: Mon, 27 Apr 2020 12:30:04 +0200 Subject: [PATCH 03/14] .github: move checklist from PR description into an auto-comment (#4745) The checklist and message in the default PR description were sometimes getting included in commit messages. This change moves that text from the PR description to a comment from a [Probot tool](https://probot.github.io/apps/auto-comment/). --- .github/PULL_REQUEST_TEMPLATE.md | 15 ++------------- .github/auto-comment.yml | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 13 deletions(-) create mode 100644 .github/auto-comment.yml diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 4af8eec23..975ad1cf5 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1,18 +1,7 @@ ## Description - +_Please add a description of the changes that this PR introduces and the files that +are the most critical to review._ Closes: #XXX ---- - -For contributor use: - -- [ ] Wrote tests -- [ ] Updated CHANGELOG_PENDING.md -- [ ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work. -- [ ] Updated relevant documentation (`docs/`) and code comments -- [ ] Re-reviewed `Files changed` in the Github PR explorer -- [ ] Applied Appropriate Labels diff --git a/.github/auto-comment.yml b/.github/auto-comment.yml new file mode 100644 index 000000000..7d2d07acd --- /dev/null +++ b/.github/auto-comment.yml @@ -0,0 +1,15 @@ +pullRequestOpened: > + :wave: Thanks for creating a PR! + + Before we can merge this PR, please make sure that all the following items have been + checked off. If any of the checklist items are not applicable, please leave them but + write a little note why. + + - [ ] Wrote tests + - [ ] Updated CHANGELOG_PENDING.md + - [ ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work. + - [ ] Updated relevant documentation (`docs/`) and code comments + - [ ] Re-reviewed `Files changed` in the Github PR explorer + - [ ] Applied Appropriate Labels + + Thank you for your contribution to Tendermint! :rocket: \ No newline at end of file From 3c41c72026d4be0f262e700a6ce3f3e6417379b8 Mon Sep 17 00:00:00 2001 From: Tess Rinearson Date: Mon, 27 Apr 2020 14:59:07 +0200 Subject: [PATCH 04/14] .github: fix whitespace for autocomment (#4747) --- .github/auto-comment.yml | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/.github/auto-comment.yml b/.github/auto-comment.yml index 7d2d07acd..49425cda5 100644 --- a/.github/auto-comment.yml +++ b/.github/auto-comment.yml @@ -1,15 +1,16 @@ pullRequestOpened: > - :wave: Thanks for creating a PR! + :wave: Thanks for creating a PR! Before we can merge this PR, please make sure that all the following items have been checked off. If any of the checklist items are not applicable, please leave them but - write a little note why. + write a little note why. - - [ ] Wrote tests - - [ ] Updated CHANGELOG_PENDING.md - - [ ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work. - - [ ] Updated relevant documentation (`docs/`) and code comments - - [ ] Re-reviewed `Files changed` in the Github PR explorer - - [ ] Applied Appropriate Labels + - [ ] Wrote tests + - [ ] Updated CHANGELOG_PENDING.md + - [ ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work. + - [ ] Updated relevant documentation (`docs/`) and code comments + - [ ] Re-reviewed `Files changed` in the Github PR explorer + - [ ] Applied Appropriate Labels - Thank you for your contribution to Tendermint! :rocket: \ No newline at end of file + + Thank you for your contribution to Tendermint! :rocket: \ No newline at end of file From fefdc6634e6e75020f8b856d30ffec557d053ed0 Mon Sep 17 00:00:00 2001 From: Erik Grinaker Date: Mon, 27 Apr 2020 17:33:20 +0200 Subject: [PATCH 05/14] crypto: remove SimpleHashFromMap() and SimpleProofsFromMap() Fixes #2593 @alexanderbez This is used in a single place in the SDK, how upset are you about removing it? ______ For contributor use: - [x] ~Wrote tests~ - [x] Updated CHANGELOG_PENDING.md - [x] Linked to Github issue with discussion and accepted design OR link to spec that describes this work. - [x] Updated relevant documentation (`docs/`) and code comments - [x] Re-reviewed `Files changed` in the Github PR explorer - [x] Applied Appropriate Labels --- CHANGELOG_PENDING.md | 3 +- crypto/merkle/simple_map.go | 95 -------------------------------- crypto/merkle/simple_map_test.go | 49 ---------------- crypto/merkle/simple_proof.go | 25 --------- crypto/merkle/simple_tree.go | 12 ---- 5 files changed, 2 insertions(+), 182 deletions(-) delete mode 100644 crypto/merkle/simple_map.go delete mode 100644 crypto/merkle/simple_map_test.go diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 8d34ab8f9..736e299c4 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -15,9 +15,10 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi - P2P Protocol - Go API + - [crypto] [\#4721](https://github.com/tendermint/tendermint/pull/4721) Remove `SimpleHashFromMap()` and `SimpleProofsFromMap()` (@erikgrinaker) -- Blockchain Protocol +- Blockchain Protocol ### FEATURES: - [evidence] [\#4532](https://github.com/tendermint/tendermint/pull/4532) Handle evidence from light clients (@melekes) diff --git a/crypto/merkle/simple_map.go b/crypto/merkle/simple_map.go deleted file mode 100644 index 840bebd51..000000000 --- a/crypto/merkle/simple_map.go +++ /dev/null @@ -1,95 +0,0 @@ -package merkle - -import ( - "bytes" - - amino "github.com/tendermint/go-amino" - - "github.com/tendermint/tendermint/crypto/tmhash" - "github.com/tendermint/tendermint/libs/kv" -) - -// Merkle tree from a map. -// Leaves are `hash(key) | hash(value)`. -// Leaves are sorted before Merkle hashing. -type simpleMap struct { - kvs kv.Pairs - sorted bool -} - -func newSimpleMap() *simpleMap { - return &simpleMap{ - kvs: nil, - sorted: false, - } -} - -// Set creates a kv pair of the key and the hash of the value, -// and then appends it to simpleMap's kv pairs. -func (sm *simpleMap) Set(key string, value []byte) { - sm.sorted = false - - // The value is hashed, so you can - // check for equality with a cached value (say) - // and make a determination to fetch or not. - vhash := tmhash.Sum(value) - - sm.kvs = append(sm.kvs, kv.Pair{ - Key: []byte(key), - Value: vhash, - }) -} - -// Hash Merkle root hash of items sorted by key -// (UNSTABLE: and by value too if duplicate key). -func (sm *simpleMap) Hash() []byte { - sm.Sort() - return hashKVPairs(sm.kvs) -} - -func (sm *simpleMap) Sort() { - if sm.sorted { - return - } - sm.kvs.Sort() - sm.sorted = true -} - -// Returns a copy of sorted KVPairs. -// NOTE these contain the hashed key and value. -func (sm *simpleMap) KVPairs() kv.Pairs { - sm.Sort() - kvs := make(kv.Pairs, len(sm.kvs)) - copy(kvs, sm.kvs) - return kvs -} - -//---------------------------------------- - -// A local extension to KVPair that can be hashed. -// Key and value are length prefixed and concatenated, -// then hashed. -type KVPair kv.Pair - -// Bytes returns key || value, with both the -// key and value length prefixed. -func (kv KVPair) Bytes() []byte { - var b bytes.Buffer - err := amino.EncodeByteSlice(&b, kv.Key) - if err != nil { - panic(err) - } - err = amino.EncodeByteSlice(&b, kv.Value) - if err != nil { - panic(err) - } - return b.Bytes() -} - -func hashKVPairs(kvs kv.Pairs) []byte { - kvsH := make([][]byte, len(kvs)) - for i, kvp := range kvs { - kvsH[i] = KVPair(kvp).Bytes() - } - return SimpleHashFromByteSlices(kvsH) -} diff --git a/crypto/merkle/simple_map_test.go b/crypto/merkle/simple_map_test.go deleted file mode 100644 index 20868a782..000000000 --- a/crypto/merkle/simple_map_test.go +++ /dev/null @@ -1,49 +0,0 @@ -package merkle - -import ( - "fmt" - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestSimpleMap(t *testing.T) { - tests := []struct { - keys []string - values []string // each string gets converted to []byte in test - want string - }{ - {[]string{"key1"}, []string{"value1"}, "a44d3cc7daba1a4600b00a2434b30f8b970652169810d6dfa9fb1793a2189324"}, - {[]string{"key1"}, []string{"value2"}, "0638e99b3445caec9d95c05e1a3fc1487b4ddec6a952ff337080360b0dcc078c"}, - // swap order with 2 keys - { - []string{"key1", "key2"}, - []string{"value1", "value2"}, - "8fd19b19e7bb3f2b3ee0574027d8a5a4cec370464ea2db2fbfa5c7d35bb0cff3", - }, - { - []string{"key2", "key1"}, - []string{"value2", "value1"}, - "8fd19b19e7bb3f2b3ee0574027d8a5a4cec370464ea2db2fbfa5c7d35bb0cff3", - }, - // swap order with 3 keys - { - []string{"key1", "key2", "key3"}, - []string{"value1", "value2", "value3"}, - "1dd674ec6782a0d586a903c9c63326a41cbe56b3bba33ed6ff5b527af6efb3dc", - }, - { - []string{"key1", "key3", "key2"}, - []string{"value1", "value3", "value2"}, - "1dd674ec6782a0d586a903c9c63326a41cbe56b3bba33ed6ff5b527af6efb3dc", - }, - } - for i, tc := range tests { - db := newSimpleMap() - for i := 0; i < len(tc.keys); i++ { - db.Set(tc.keys[i], []byte(tc.values[i])) - } - got := db.Hash() - assert.Equal(t, tc.want, fmt.Sprintf("%x", got), "Hash didn't match on tc %d", i) - } -} diff --git a/crypto/merkle/simple_proof.go b/crypto/merkle/simple_proof.go index 44b97f606..26c8baeda 100644 --- a/crypto/merkle/simple_proof.go +++ b/crypto/merkle/simple_proof.go @@ -47,31 +47,6 @@ func SimpleProofsFromByteSlices(items [][]byte) (rootHash []byte, proofs []*Simp return } -// SimpleProofsFromMap generates proofs from a map. The keys/values of the map will be used as the keys/values -// in the underlying key-value pairs. -// The keys are sorted before the proofs are computed. -func SimpleProofsFromMap(m map[string][]byte) (rootHash []byte, proofs map[string]*SimpleProof, keys []string) { - sm := newSimpleMap() - for k, v := range m { - sm.Set(k, v) - } - sm.Sort() - kvs := sm.kvs - kvsBytes := make([][]byte, len(kvs)) - for i, kvp := range kvs { - kvsBytes[i] = KVPair(kvp).Bytes() - } - - rootHash, proofList := SimpleProofsFromByteSlices(kvsBytes) - proofs = make(map[string]*SimpleProof) - keys = make([]string, len(proofList)) - for i, kvp := range kvs { - proofs[string(kvp.Key)] = proofList[i] - keys[i] = string(kvp.Key) - } - return -} - // Verify that the SimpleProof proves the root hash. // Check sp.Index/sp.Total manually if needed func (sp *SimpleProof) Verify(rootHash []byte, leaf []byte) error { diff --git a/crypto/merkle/simple_tree.go b/crypto/merkle/simple_tree.go index 2a57bbe72..d2b931ec7 100644 --- a/crypto/merkle/simple_tree.go +++ b/crypto/merkle/simple_tree.go @@ -91,18 +91,6 @@ func SimpleHashFromByteSlicesIterative(input [][]byte) []byte { } } -// SimpleHashFromMap computes a Merkle tree from sorted map. -// Like calling SimpleHashFromHashers with -// `item = []byte(Hash(key) | Hash(value))`, -// sorted by `item`. -func SimpleHashFromMap(m map[string][]byte) []byte { - sm := newSimpleMap() - for k, v := range m { - sm.Set(k, v) - } - return sm.Hash() -} - // getSplitPoint returns the largest power of 2 less than length func getSplitPoint(length int) int { if length < 1 { From 496ee91fccb25ea8757bbae99ba7ae91def43d16 Mon Sep 17 00:00:00 2001 From: Callum Date: Tue, 28 Apr 2020 06:35:10 +0200 Subject: [PATCH 06/14] made suggested changes --- evidence/doc.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/evidence/doc.go b/evidence/doc.go index a669b7a28..016ea555f 100644 --- a/evidence/doc.go +++ b/evidence/doc.go @@ -3,25 +3,27 @@ Package evidence handles all evidence storage and gossiping from detection to bl For the different types of evidence refer to the `evidence.go` file in the types package or https://github.com/tendermint/spec/blob/master/spec/consensus/light-client/accountability.md. -## Gossiping +Gossiping The core functionality begins with the evidence reactor (see reactor. go) which operates both the sending and receiving of evidence. The `Receive` function takes a list of evidence and does the following: - 1. breaks it down into individual evidence if it is `Composite Evidence` (see types/evidence. -go#ConflictingHeadersEvidence) - 2. checks that it does not already have the evidence stored - 3. verifies the evidence against the nodes state (see state/validation.go#VerifyEvidence) - 4. stores the evidence to a db and a concurrent list + +1. Breaks it down into individual evidence if it is `Composite Evidence` (see types/evidence.go#ConflictingHeadersEvidence) + +2. Checks that it does not already have the evidence stored + +3. Verifies the evidence against the node's state (see state/validation.go#VerifyEvidence) + +4. Stores the evidence to a db and a concurrent list The gossiping of evidence is initiated when a peer is added which starts a go routine to broadcast currently uncommitted evidence at intervals of 60 seconds (set by the by broadcastEvidenceIntervalS). It uses a concurrent list to store the evidence and before sending verifies that each evidence is still valid in the -sense that it has not exceeded the max evidence age and height. This should be set to be equal to the "trusting -period" (see types/params.go#EvidenceParams). +sense that it has not exceeded the max evidence age and height (see types/params.go#EvidenceParams). -## Proposing +Proposing When a new block is being proposed (in state/execution.go#CreateProposalBlock), `PendingEvidence(maxNum)` is called to send up to the maxNum number of uncommitted evidence, from the evidence store, @@ -31,7 +33,5 @@ Once the proposed evidence is submitted, the evidence is marked as committed and is moved from the broadcasted set to the committed set ( the committed set is used to verify whether new evidence has actually already been submitted). As a result it is also removed from the concurrent list so that it is no longer gossiped. - -Last Update: 14/04/20 */ package evidence From 1972b1f9feb92cad143499e6ecc1ded598e56467 Mon Sep 17 00:00:00 2001 From: Callum Date: Tue, 28 Apr 2020 06:38:40 +0200 Subject: [PATCH 07/14] lint fix --- evidence/doc.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/evidence/doc.go b/evidence/doc.go index 016ea555f..77bd8ef75 100644 --- a/evidence/doc.go +++ b/evidence/doc.go @@ -10,7 +10,8 @@ go) which operates both the sending and receiving of evidence. The `Receive` function takes a list of evidence and does the following: -1. Breaks it down into individual evidence if it is `Composite Evidence` (see types/evidence.go#ConflictingHeadersEvidence) +1. Breaks it down into individual evidence if it is `Composite Evidence` +(see types/evidence.go#ConflictingHeadersEvidence) 2. Checks that it does not already have the evidence stored From a5a84e11f1140e034d9a079d04b0cabe5e420555 Mon Sep 17 00:00:00 2001 From: Marko Date: Tue, 28 Apr 2020 07:47:20 +0200 Subject: [PATCH 08/14] evidence: remove pubkey from duplicate vote evidence this pr brings over the removal of pubkey from duplicatevote evidence from proto-breakage ref #4580 --- CHANGELOG_PENDING.md | 2 ++ evidence/reactor_test.go | 3 +-- rpc/client/evidence_test.go | 4 +-- types/evidence.go | 24 +++++++----------- types/evidence_test.go | 50 ++++++++++++++++++++++++++++++------- types/protobuf_test.go | 5 ++-- types/vote.go | 4 +-- 7 files changed, 59 insertions(+), 33 deletions(-) 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), } } From f31f4327b5cee09e90fb6034a3970e4df14dddf5 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Tue, 28 Apr 2020 12:51:51 +0400 Subject: [PATCH 09/14] privval: remove deprecated `OldFilePV` The old format was deprecated in v0.28. It's time we remove it. --- CHANGELOG_PENDING.md | 4 +- config/config.go | 6 -- node/node.go | 23 +----- privval/file.go | 2 +- privval/file_deprecated.go | 81 -------------------- privval/file_deprecated_test.go | 84 --------------------- scripts/privValUpgrade.go | 45 ----------- scripts/privValUpgrade_test.go | 129 -------------------------------- 8 files changed, 6 insertions(+), 368 deletions(-) delete mode 100644 privval/file_deprecated.go delete mode 100644 privval/file_deprecated_test.go delete mode 100644 scripts/privValUpgrade.go delete mode 100644 scripts/privValUpgrade_test.go diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 9064cee60..d861a9168 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -17,10 +17,12 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi - P2P Protocol - Go API - - [crypto] [\#4721](https://github.com/tendermint/tendermint/pull/4721) Remove `SimpleHashFromMap()` and `SimpleProofsFromMap()` (@erikgrinaker) + - [crypto] [\#4721](https://github.com/tendermint/tendermint/pull/4721) Remove `SimpleHashFromMap()` and `SimpleProofsFromMap()` (@erikgrinaker) + - [privval] [\#4744](https://github.com/tendermint/tendermint/pull/4744) Remove deprecated `OldFilePV` (@melekes) - Blockchain Protocol + ### FEATURES: - [evidence] [\#4532](https://github.com/tendermint/tendermint/pull/4532) Handle evidence from light clients (@melekes) diff --git a/config/config.go b/config/config.go index c246493e4..173713279 100644 --- a/config/config.go +++ b/config/config.go @@ -263,12 +263,6 @@ func (cfg BaseConfig) PrivValidatorStateFile() string { return rootify(cfg.PrivValidatorState, cfg.RootDir) } -// OldPrivValidatorFile returns the full path of the priv_validator.json from pre v0.28.0. -// TODO: eventually remove. -func (cfg BaseConfig) OldPrivValidatorFile() string { - return rootify(oldPrivValPath, cfg.RootDir) -} - // NodeKeyFile returns the full path to the node_key.json file func (cfg BaseConfig) NodeKeyFile() string { return rootify(cfg.NodeKey, cfg.RootDir) diff --git a/node/node.go b/node/node.go index c7d02fe28..a59a3d19e 100644 --- a/node/node.go +++ b/node/node.go @@ -7,7 +7,6 @@ import ( "net" "net/http" _ "net/http/pprof" // nolint: gosec // securely exposed on separate, optional port - "os" "strings" "time" @@ -88,31 +87,13 @@ type Provider func(*cfg.Config, log.Logger) (*Node, error) // PrivValidator, ClientCreator, GenesisDoc, and DBProvider. // It implements NodeProvider. func DefaultNewNode(config *cfg.Config, logger log.Logger) (*Node, error) { - // Generate node PrivKey nodeKey, err := p2p.LoadOrGenNodeKey(config.NodeKeyFile()) if err != nil { - return nil, err - } - - // Convert old PrivValidator if it exists. - oldPrivVal := config.OldPrivValidatorFile() - newPrivValKey := config.PrivValidatorKeyFile() - newPrivValState := config.PrivValidatorStateFile() - if _, err := os.Stat(oldPrivVal); !os.IsNotExist(err) { - oldPV, err := privval.LoadOldFilePV(oldPrivVal) - if err != nil { - return nil, fmt.Errorf("error reading OldPrivValidator from %v: %v", oldPrivVal, err) - } - logger.Info("Upgrading PrivValidator file", - "old", oldPrivVal, - "newKey", newPrivValKey, - "newState", newPrivValState, - ) - oldPV.Upgrade(newPrivValKey, newPrivValState) + return nil, fmt.Errorf("failed to load or gen node key %s: %w", config.NodeKeyFile(), err) } return NewNode(config, - privval.LoadOrGenFilePV(newPrivValKey, newPrivValState), + privval.LoadOrGenFilePV(config.PrivValidatorKeyFile(), config.PrivValidatorStateFile()), nodeKey, proxy.DefaultClientCreator(config.ProxyApp, config.ABCI, config.DBDir()), DefaultGenesisDocProviderFunc(config), diff --git a/privval/file.go b/privval/file.go index 5f07ac525..4a71f8a6a 100644 --- a/privval/file.go +++ b/privval/file.go @@ -32,7 +32,7 @@ func voteToStep(vote *types.Vote) int8 { case types.PrecommitType: return stepPrecommit default: - panic("Unknown vote type") + panic(fmt.Sprintf("Unknown vote type: %v", vote.Type)) } } diff --git a/privval/file_deprecated.go b/privval/file_deprecated.go deleted file mode 100644 index c30c273d7..000000000 --- a/privval/file_deprecated.go +++ /dev/null @@ -1,81 +0,0 @@ -package privval - -import ( - "io/ioutil" - "os" - - "github.com/tendermint/tendermint/crypto" - "github.com/tendermint/tendermint/libs/bytes" - "github.com/tendermint/tendermint/types" -) - -// OldFilePV is the old version of the FilePV, pre v0.28.0. -// Deprecated: Use FilePV instead. -type OldFilePV struct { - Address types.Address `json:"address"` - PubKey crypto.PubKey `json:"pub_key"` - LastHeight int64 `json:"last_height"` - LastRound int `json:"last_round"` - LastStep int8 `json:"last_step"` - LastSignature []byte `json:"last_signature,omitempty"` - LastSignBytes bytes.HexBytes `json:"last_signbytes,omitempty"` - PrivKey crypto.PrivKey `json:"priv_key"` - - filePath string -} - -// LoadOldFilePV loads an OldFilePV from the filePath. -func LoadOldFilePV(filePath string) (*OldFilePV, error) { - pvJSONBytes, err := ioutil.ReadFile(filePath) - if err != nil { - return nil, err - } - pv := &OldFilePV{} - err = cdc.UnmarshalJSON(pvJSONBytes, &pv) - if err != nil { - return nil, err - } - - // overwrite pubkey and address for convenience - pv.PubKey = pv.PrivKey.PubKey() - pv.Address = pv.PubKey.Address() - - pv.filePath = filePath - return pv, nil -} - -// Upgrade convets the OldFilePV to the new FilePV, separating the immutable and mutable components, -// and persisting them to the keyFilePath and stateFilePath, respectively. -// It renames the original file by adding ".bak". -func (oldFilePV *OldFilePV) Upgrade(keyFilePath, stateFilePath string) *FilePV { - privKey := oldFilePV.PrivKey - pvKey := FilePVKey{ - PrivKey: privKey, - PubKey: privKey.PubKey(), - Address: privKey.PubKey().Address(), - filePath: keyFilePath, - } - - pvState := FilePVLastSignState{ - Height: oldFilePV.LastHeight, - Round: oldFilePV.LastRound, - Step: oldFilePV.LastStep, - Signature: oldFilePV.LastSignature, - SignBytes: oldFilePV.LastSignBytes, - filePath: stateFilePath, - } - - // Save the new PV files - pv := &FilePV{ - Key: pvKey, - LastSignState: pvState, - } - pv.Save() - - // Rename the old PV file - err := os.Rename(oldFilePV.filePath, oldFilePV.filePath+".bak") - if err != nil { - panic(err) - } - return pv -} diff --git a/privval/file_deprecated_test.go b/privval/file_deprecated_test.go deleted file mode 100644 index f850c23f1..000000000 --- a/privval/file_deprecated_test.go +++ /dev/null @@ -1,84 +0,0 @@ -package privval_test - -import ( - "io/ioutil" - "os" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/tendermint/tendermint/privval" -) - -const lastSignBytes = "750802110500000000000000220B08B398F3E00510F48DA6402A480A20F" + - "C258973076512999C3E6839A22E9FBDB1B77CF993E8A9955412A41A59D4" + - "CAD312240A20C971B286ACB8AAA6FCA0365EB0A660B189EDC08B46B5AF2" + - "995DEFA51A28D215B10013211746573742D636861696E2D533245415533" - -const oldPrivvalContent = `{ - "address": "1D8089FAFDFAE4A637F3D616E17B92905FA2D91D", - "pub_key": { - "type": "tendermint/PubKeyEd25519", - "value": "r3Yg2AhDZ745CNTpavsGU+mRZ8WpRXqoJuyqjN8mJq0=" - }, - "last_height": "5", - "last_round": "0", - "last_step": 3, - "last_signature": "CTr7b9ZQlrJJf+12rPl5t/YSCUc/KqV7jQogCfFJA24e7hof69X6OMT7eFLVQHyodPjD/QTA298XHV5ejxInDQ==", - "last_signbytes": "` + lastSignBytes + `", - "priv_key": { - "type": "tendermint/PrivKeyEd25519", - "value": "7MwvTGEWWjsYwjn2IpRb+GYsWi9nnFsw8jPLLY1UtP6vdiDYCENnvjkI1Olq+wZT6ZFnxalFeqgm7KqM3yYmrQ==" - } -}` - -func TestLoadAndUpgrade(t *testing.T) { - - oldFilePath := initTmpOldFile(t) - defer os.Remove(oldFilePath) - newStateFile, err := ioutil.TempFile("", "priv_validator_state*.json") - defer os.Remove(newStateFile.Name()) - require.NoError(t, err) - newKeyFile, err := ioutil.TempFile("", "priv_validator_key*.json") - defer os.Remove(newKeyFile.Name()) - require.NoError(t, err) - - oldPV, err := privval.LoadOldFilePV(oldFilePath) - assert.NoError(t, err) - newPV := oldPV.Upgrade(newKeyFile.Name(), newStateFile.Name()) - - assertEqualPV(t, oldPV, newPV) - assert.NoError(t, err) - upgradedPV := privval.LoadFilePV(newKeyFile.Name(), newStateFile.Name()) - assertEqualPV(t, oldPV, upgradedPV) - oldPV, err = privval.LoadOldFilePV(oldFilePath + ".bak") - require.NoError(t, err) - assertEqualPV(t, oldPV, upgradedPV) -} - -func assertEqualPV(t *testing.T, oldPV *privval.OldFilePV, newPV *privval.FilePV) { - assert.Equal(t, oldPV.Address, newPV.Key.Address) - assert.Equal(t, oldPV.Address, newPV.GetAddress()) - assert.Equal(t, oldPV.PubKey, newPV.Key.PubKey) - npv, err := newPV.GetPubKey() - require.NoError(t, err) - assert.Equal(t, oldPV.PubKey, npv) - assert.Equal(t, oldPV.PrivKey, newPV.Key.PrivKey) - - assert.Equal(t, oldPV.LastHeight, newPV.LastSignState.Height) - assert.Equal(t, oldPV.LastRound, newPV.LastSignState.Round) - assert.Equal(t, oldPV.LastSignature, newPV.LastSignState.Signature) - assert.Equal(t, oldPV.LastSignBytes, newPV.LastSignState.SignBytes) - assert.Equal(t, oldPV.LastStep, newPV.LastSignState.Step) -} - -func initTmpOldFile(t *testing.T) string { - tmpFile, err := ioutil.TempFile("", "priv_validator_*.json") - require.NoError(t, err) - t.Logf("created test file %s", tmpFile.Name()) - _, err = tmpFile.WriteString(oldPrivvalContent) - require.NoError(t, err) - - return tmpFile.Name() -} diff --git a/scripts/privValUpgrade.go b/scripts/privValUpgrade.go deleted file mode 100644 index 1882a3663..000000000 --- a/scripts/privValUpgrade.go +++ /dev/null @@ -1,45 +0,0 @@ -package main - -import ( - "fmt" - "os" - - "github.com/tendermint/tendermint/libs/log" - "github.com/tendermint/tendermint/privval" -) - -var ( - logger = log.NewTMLogger(log.NewSyncWriter(os.Stdout)) -) - -func main() { - args := os.Args[1:] - if len(args) != 3 { - fmt.Println("Expected three args: ") - fmt.Println( - "Eg. ~/.tendermint/config/priv_validator.json" + - " ~/.tendermint/config/priv_validator_key.json" + - " ~/.tendermint/data/priv_validator_state.json", - ) - os.Exit(1) - } - err := loadAndUpgrade(args[0], args[1], args[2]) - if err != nil { - fmt.Println(err) - os.Exit(1) - } -} - -func loadAndUpgrade(oldPVPath, newPVKeyPath, newPVStatePath string) error { - oldPV, err := privval.LoadOldFilePV(oldPVPath) - if err != nil { - return fmt.Errorf("error reading OldPrivValidator from %v: %v", oldPVPath, err) - } - logger.Info("Upgrading PrivValidator file", - "old", oldPVPath, - "newKey", newPVKeyPath, - "newState", newPVStatePath, - ) - oldPV.Upgrade(newPVKeyPath, newPVStatePath) - return nil -} diff --git a/scripts/privValUpgrade_test.go b/scripts/privValUpgrade_test.go deleted file mode 100644 index 287c4fc50..000000000 --- a/scripts/privValUpgrade_test.go +++ /dev/null @@ -1,129 +0,0 @@ -package main - -import ( - "fmt" - "io/ioutil" - "os" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/tendermint/tendermint/privval" -) - -const lastSignBytes = "750802110500000000000000220B08B398F3E00510F48DA6402A480A20FC25" + - "8973076512999C3E6839A22E9FBDB1B77CF993E8A9955412A41A59D4CAD312240A20C971B286ACB8AA" + - "A6FCA0365EB0A660B189EDC08B46B5AF2995DEFA51A28D215B10013211746573742D636861696E2D533245415533" - -const oldPrivvalContent = `{ - "address": "1D8089FAFDFAE4A637F3D616E17B92905FA2D91D", - "pub_key": { - "type": "tendermint/PubKeyEd25519", - "value": "r3Yg2AhDZ745CNTpavsGU+mRZ8WpRXqoJuyqjN8mJq0=" - }, - "last_height": "5", - "last_round": "0", - "last_step": 3, - "last_signature": "CTr7b9ZQlrJJf+12rPl5t/YSCUc/KqV7jQogCfFJA24e7hof69X6OMT7eFLVQHyodPjD/QTA298XHV5ejxInDQ==", - "last_signbytes": "` + lastSignBytes + `", - "priv_key": { - "type": "tendermint/PrivKeyEd25519", - "value": "7MwvTGEWWjsYwjn2IpRb+GYsWi9nnFsw8jPLLY1UtP6vdiDYCENnvjkI1Olq+wZT6ZFnxalFeqgm7KqM3yYmrQ==" - } -}` - -func TestLoadAndUpgrade(t *testing.T) { - - oldFilePath := initTmpOldFile(t) - defer os.Remove(oldFilePath) - newStateFile, err := ioutil.TempFile("", "priv_validator_state*.json") - defer os.Remove(newStateFile.Name()) - require.NoError(t, err) - newKeyFile, err := ioutil.TempFile("", "priv_validator_key*.json") - defer os.Remove(newKeyFile.Name()) - require.NoError(t, err) - emptyOldFile, err := ioutil.TempFile("", "priv_validator_empty*.json") - require.NoError(t, err) - defer os.Remove(emptyOldFile.Name()) - - type args struct { - oldPVPath string - newPVKeyPath string - newPVStatePath string - } - tests := []struct { - name string - args args - wantErr bool - wantPanic bool - }{ - {"successful upgrade", - args{oldPVPath: oldFilePath, newPVKeyPath: newKeyFile.Name(), newPVStatePath: newStateFile.Name()}, - false, false, - }, - {"unsuccessful upgrade: empty old privval file", - args{oldPVPath: emptyOldFile.Name(), newPVKeyPath: newKeyFile.Name(), newPVStatePath: newStateFile.Name()}, - true, false, - }, - {"unsuccessful upgrade: invalid new paths (1/3)", - args{oldPVPath: oldFilePath, newPVKeyPath: "", newPVStatePath: newStateFile.Name()}, - false, true, - }, - {"unsuccessful upgrade: invalid new paths (2/3)", - args{oldPVPath: oldFilePath, newPVKeyPath: newKeyFile.Name(), newPVStatePath: ""}, - false, true, - }, - {"unsuccessful upgrade: invalid new paths (3/3)", - args{oldPVPath: oldFilePath, newPVKeyPath: "", newPVStatePath: ""}, - false, true, - }, - } - for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { - // need to re-write the file everytime because upgrading renames it - err := ioutil.WriteFile(oldFilePath, []byte(oldPrivvalContent), 0600) - require.NoError(t, err) - if tt.wantPanic { - require.Panics(t, func() { loadAndUpgrade(tt.args.oldPVPath, tt.args.newPVKeyPath, tt.args.newPVStatePath) }) - } else { - err = loadAndUpgrade(tt.args.oldPVPath, tt.args.newPVKeyPath, tt.args.newPVStatePath) - if tt.wantErr { - assert.Error(t, err) - fmt.Println("ERR", err) - } else { - assert.NoError(t, err) - upgradedPV := privval.LoadFilePV(tt.args.newPVKeyPath, tt.args.newPVStatePath) - oldPV, err := privval.LoadOldFilePV(tt.args.oldPVPath + ".bak") - require.NoError(t, err) - - assert.Equal(t, oldPV.Address, upgradedPV.Key.Address) - assert.Equal(t, oldPV.Address, upgradedPV.GetAddress()) - assert.Equal(t, oldPV.PubKey, upgradedPV.Key.PubKey) - upv, err := upgradedPV.GetPubKey() - require.NoError(t, err) - assert.Equal(t, oldPV.PubKey, upv) - assert.Equal(t, oldPV.PrivKey, upgradedPV.Key.PrivKey) - - assert.Equal(t, oldPV.LastHeight, upgradedPV.LastSignState.Height) - assert.Equal(t, oldPV.LastRound, upgradedPV.LastSignState.Round) - assert.Equal(t, oldPV.LastSignature, upgradedPV.LastSignState.Signature) - assert.Equal(t, oldPV.LastSignBytes, upgradedPV.LastSignState.SignBytes) - assert.Equal(t, oldPV.LastStep, upgradedPV.LastSignState.Step) - - } - } - }) - } -} - -func initTmpOldFile(t *testing.T) string { - tmpfile, err := ioutil.TempFile("", "priv_validator_*.json") - require.NoError(t, err) - t.Logf("created test file %s", tmpfile.Name()) - _, err = tmpfile.WriteString(oldPrivvalContent) - require.NoError(t, err) - - return tmpfile.Name() -} From 8f463cf35ca0cdad6238871c3dd98ce4751a7631 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Tue, 28 Apr 2020 14:05:09 +0400 Subject: [PATCH 10/14] p2p: set RecvMessageCapacity to maxMsgSize in all reactors to prevent malicious nodes from sending us large messages (~21MB, which is the default `RecvMessageCapacity`) This allows us to remove unnecessary `maxMsgSize` check in `decodeMsg`. Since each channel has a msg capacity set to `maxMsgSize`, there's no need to check it again in `decodeMsg`. Closes #1503 --- blockchain/v0/reactor.go | 3 --- blockchain/v1/reactor.go | 3 --- blockchain/v2/reactor.go | 3 --- consensus/reactor.go | 3 --- evidence/reactor.go | 4 +--- mempool/reactor.go | 10 ++++------ p2p/pex/pex_reactor.go | 10 ++++------ 7 files changed, 9 insertions(+), 27 deletions(-) diff --git a/blockchain/v0/reactor.go b/blockchain/v0/reactor.go index 247222160..c82457bee 100644 --- a/blockchain/v0/reactor.go +++ b/blockchain/v0/reactor.go @@ -394,9 +394,6 @@ func RegisterBlockchainMessages(cdc *amino.Codec) { } func decodeMsg(bz []byte) (msg BlockchainMessage, err error) { - if len(bz) > maxMsgSize { - return msg, fmt.Errorf("msg exceeds max size (%d > %d)", len(bz), maxMsgSize) - } err = cdc.UnmarshalBinaryBare(bz, &msg) return } diff --git a/blockchain/v1/reactor.go b/blockchain/v1/reactor.go index 28a314b8a..315dbffbf 100644 --- a/blockchain/v1/reactor.go +++ b/blockchain/v1/reactor.go @@ -539,9 +539,6 @@ func RegisterBlockchainMessages(cdc *amino.Codec) { } func decodeMsg(bz []byte) (msg BlockchainMessage, err error) { - if len(bz) > maxMsgSize { - return msg, fmt.Errorf("msg exceeds max size (%d > %d)", len(bz), maxMsgSize) - } err = cdc.UnmarshalBinaryBare(bz, &msg) return } diff --git a/blockchain/v2/reactor.go b/blockchain/v2/reactor.go index ff89ee94c..d68b27a41 100644 --- a/blockchain/v2/reactor.go +++ b/blockchain/v2/reactor.go @@ -453,9 +453,6 @@ func RegisterBlockchainMessages(cdc *amino.Codec) { } func decodeMsg(bz []byte) (msg BlockchainMessage, err error) { - if len(bz) > maxMsgSize { - return msg, fmt.Errorf("msg exceeds max size (%d > %d)", len(bz), maxMsgSize) - } err = cdc.UnmarshalBinaryBare(bz, &msg) return } diff --git a/consensus/reactor.go b/consensus/reactor.go index c8c344ac8..99f40ac0b 100644 --- a/consensus/reactor.go +++ b/consensus/reactor.go @@ -1404,9 +1404,6 @@ func RegisterMessages(cdc *amino.Codec) { } func decodeMsg(bz []byte) (msg Message, err error) { - if len(bz) > maxMsgSize { - return msg, fmt.Errorf("msg exceeds max size (%d > %d)", len(bz), maxMsgSize) - } err = cdc.UnmarshalBinaryBare(bz, &msg) return } diff --git a/evidence/reactor.go b/evidence/reactor.go index 8314fa0ef..2a9554707 100644 --- a/evidence/reactor.go +++ b/evidence/reactor.go @@ -51,6 +51,7 @@ func (evR *Reactor) GetChannels() []*p2p.ChannelDescriptor { { ID: EvidenceChannel, Priority: 5, + RecvMessageCapacity: maxMsgSize, }, } } @@ -235,9 +236,6 @@ func RegisterMessages(cdc *amino.Codec) { } func decodeMsg(bz []byte) (msg Message, err error) { - if len(bz) > maxMsgSize { - return msg, fmt.Errorf("msg exceeds max size (%d > %d)", len(bz), maxMsgSize) - } err = cdc.UnmarshalBinaryBare(bz, &msg) return } diff --git a/mempool/reactor.go b/mempool/reactor.go index fda12c021..d64bb7868 100644 --- a/mempool/reactor.go +++ b/mempool/reactor.go @@ -137,10 +137,12 @@ func (memR *Reactor) OnStart() error { // GetChannels implements Reactor. // It returns the list of channels for this reactor. func (memR *Reactor) GetChannels() []*p2p.ChannelDescriptor { + maxMsgSize := calcMaxMsgSize(memR.config.MaxTxBytes) return []*p2p.ChannelDescriptor{ { - ID: MempoolChannel, - Priority: 5, + ID: MempoolChannel, + Priority: 5, + RecvMessageCapacity: maxMsgSize, }, } } @@ -271,10 +273,6 @@ func RegisterMessages(cdc *amino.Codec) { } func (memR *Reactor) decodeMsg(bz []byte) (msg Message, err error) { - maxMsgSize := calcMaxMsgSize(memR.config.MaxTxBytes) - if l := len(bz); l > maxMsgSize { - return msg, ErrTxTooLarge{maxMsgSize, l} - } err = cdc.UnmarshalBinaryBare(bz, &msg) return } diff --git a/p2p/pex/pex_reactor.go b/p2p/pex/pex_reactor.go index d06814195..8bdb0d77f 100644 --- a/p2p/pex/pex_reactor.go +++ b/p2p/pex/pex_reactor.go @@ -180,9 +180,10 @@ func (r *Reactor) OnStop() { func (r *Reactor) GetChannels() []*conn.ChannelDescriptor { return []*conn.ChannelDescriptor{ { - ID: PexChannel, - Priority: 1, - SendQueueCapacity: 10, + ID: PexChannel, + Priority: 1, + SendQueueCapacity: 10, + RecvMessageCapacity: maxMsgSize, }, } } @@ -771,9 +772,6 @@ func RegisterMessages(cdc *amino.Codec) { } func decodeMsg(bz []byte) (msg Message, err error) { - if len(bz) > maxMsgSize { - return msg, fmt.Errorf("msg exceeds max size (%d > %d)", len(bz), maxMsgSize) - } err = cdc.UnmarshalBinaryBare(bz, &msg) return } From f23f21f001e36ac3ec0bd874c24eb85d9e246fb6 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Tue, 28 Apr 2020 15:04:45 +0400 Subject: [PATCH 11/14] evidence: protect valToLastHeight w/ mtx to prevent data races note: when we check the ConflictingHeadersEvidence, the copy is used. Closes #4749 Also, iterate over valToLastHeight instead of loading validators. While it might be slower than accessing a single key in goleveldb, the new code is better adapted to ConsensusParams changes. --- evidence/pool.go | 46 ++++++++++++++++++++++++++-------------------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/evidence/pool.go b/evidence/pool.go index 348a21018..7947fe0b4 100644 --- a/evidence/pool.go +++ b/evidence/pool.go @@ -28,17 +28,17 @@ type Pool struct { // needed to load validators to verify evidence stateDB dbm.DB + // needed to load headers to verify evidence blockStore *store.BlockStore + mtx sync.Mutex + // latest state + state sm.State // a map of active validators and respective last heights validator is active // if it was in validator set after EvidenceParams.MaxAgeNumBlocks or // currently is (ie. [MaxAgeNumBlocks, CurrentHeight]) // In simple words, it means it's still bonded -> therefore slashable. valToLastHeight valToLastHeightMap - - // latest state - mtx sync.Mutex - state sm.State } // Validator.Address -> Last height it was in validator set @@ -90,8 +90,8 @@ func (evpool *Pool) PendingEvidence(maxNum int64) []types.Evidence { return evidence } -// Update uses the latest block to update the state, the ValToLastHeight map for evidence expiration -// and to mark committed evidence +// Update uses the latest block & state to update its copy of the state, +// validator to last height map and calls MarkEvidenceAsCommitted. func (evpool *Pool) Update(block *types.Block, state sm.State) { // sanity check if state.LastBlockHeight != block.Height { @@ -103,14 +103,13 @@ func (evpool *Pool) Update(block *types.Block, state sm.State) { ) } - // update the state - evpool.mtx.Lock() - evpool.state = state - evpool.mtx.Unlock() - // remove evidence from pending and mark committed evpool.MarkEvidenceAsCommitted(block.Height, block.Time, block.Evidence.Evidence) + // update the state + evpool.mtx.Lock() + defer evpool.mtx.Unlock() + evpool.state = state evpool.updateValToLastHeight(block.Height, state) } @@ -141,7 +140,15 @@ func (evpool *Pool) AddEvidence(evidence types.Evidence) error { return err } - evList = ce.Split(&blockMeta.Header, valSet, evpool.valToLastHeight) + // XXX: Copy here since this should be a rare case. + evpool.mtx.Lock() + valToLastHeightCopy := make(valToLastHeightMap, len(evpool.valToLastHeight)) + for k, v := range evpool.valToLastHeight { + valToLastHeightCopy[k] = v + } + evpool.mtx.Unlock() + + evList = ce.Split(&blockMeta.Header, valSet, valToLastHeightCopy) } for _, ev := range evList { @@ -262,6 +269,9 @@ func (evpool *Pool) SetLogger(l log.Logger) { // long time ago (> ConsensusParams.Evidence.MaxAgeDuration && > // ConsensusParams.Evidence.MaxAgeNumBlocks). func (evpool *Pool) ValidatorLastHeight(address []byte) int64 { + evpool.mtx.Lock() + defer evpool.mtx.Unlock() + h, ok := evpool.valToLastHeight[string(address)] if !ok { return 0 @@ -352,15 +362,11 @@ func (evpool *Pool) updateValToLastHeight(blockHeight int64, state sm.State) { } // Remove validators outside of MaxAgeNumBlocks & MaxAgeDuration. - removeHeight := blockHeight - evpool.State().ConsensusParams.Evidence.MaxAgeNumBlocks + removeHeight := blockHeight - state.ConsensusParams.Evidence.MaxAgeNumBlocks if removeHeight >= 1 { - valSet, err := sm.LoadValidators(evpool.stateDB, removeHeight) - if err != nil { - for _, val := range valSet.Validators { - h, ok := evpool.valToLastHeight[string(val.Address)] - if ok && h == removeHeight { - delete(evpool.valToLastHeight, string(val.Address)) - } + for val, height := range evpool.valToLastHeight { + if height <= removeHeight { + delete(evpool.valToLastHeight, val) } } } From 071bcfe169c6755142bf55c8ed66635a141a96a2 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Tue, 28 Apr 2020 15:33:42 +0400 Subject: [PATCH 12/14] docs: state we don't support non constant time crypto on 32 bit architectures or ARM Closes #2103 --- docs/tendermint-core/running-in-production.md | 40 ++++++++++++------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/docs/tendermint-core/running-in-production.md b/docs/tendermint-core/running-in-production.md index 20eb8910d..548fb31bd 100644 --- a/docs/tendermint-core/running-in-production.md +++ b/docs/tendermint-core/running-in-production.md @@ -247,15 +247,14 @@ $EDITOR /tmp/corrupted_wal ### Processor and Memory -While actual specs vary depending on the load and validators count, -minimal requirements are: +While actual specs vary depending on the load and validators count, minimal +requirements are: - 1GB RAM - 25GB of disk space - 1.4 GHz CPU -SSD disks are preferable for applications with high transaction -throughput. +SSD disks are preferable for applications with high transaction throughput. Recommended: @@ -263,21 +262,34 @@ Recommended: - 100GB SSD - x64 2.0 GHz 2v CPU -While for now, Tendermint stores all the history and it may require -significant disk space over time, we are planning to implement state -syncing (See -[this issue](https://github.com/tendermint/tendermint/issues/828)). So, -storing all the past blocks will not be necessary. +While for now, Tendermint stores all the history and it may require significant +disk space over time, we are planning to implement state syncing (See [this +issue](https://github.com/tendermint/tendermint/issues/828)). So, storing all +the past blocks will not be necessary. + +### Validator signing on 32 bit architectures (or ARM) + +Both our `ed25519` and `secp256k1` implementations require constant time +`uint64` multiplication. Non-constant time crypto can (and has) leaked +private keys on both `ed25519` and `secp256k1`. This doesn't exist in hardware +on 32 bit x86 platforms ([source](https://bearssl.org/ctmul.html)), and it +depends on the compiler to enforce that it is constant time. It's unclear at +this point whenever the Golang compiler does this correctly for all +implementations. + +**We do not support nor recommend running a validator on 32 bit architectures OR +the "VIA Nano 2000 Series", and the architectures in the ARM section rated +"S-".** ### Operating Systems -Tendermint can be compiled for a wide range of operating systems thanks -to Go language (the list of \$OS/\$ARCH pairs can be found +Tendermint can be compiled for a wide range of operating systems thanks to Go +language (the list of \$OS/\$ARCH pairs can be found [here](https://golang.org/doc/install/source#environment)). -While we do not favor any operation system, more secure and stable Linux -server distributions (like Centos) should be preferred over desktop -operation systems (like Mac OS). +While we do not favor any operation system, more secure and stable Linux server +distributions (like Centos) should be preferred over desktop operation systems +(like Mac OS). ### Miscellaneous From be42442e1013525dd626e875be679a709567ea55 Mon Sep 17 00:00:00 2001 From: Tess Rinearson Date: Tue, 28 Apr 2020 14:12:53 +0200 Subject: [PATCH 13/14] .github: fix whitespace for auto-comment (#4750) --- .github/auto-comment.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/auto-comment.yml b/.github/auto-comment.yml index 49425cda5..604c2f878 100644 --- a/.github/auto-comment.yml +++ b/.github/auto-comment.yml @@ -1,4 +1,4 @@ -pullRequestOpened: > +pullRequestOpened: | :wave: Thanks for creating a PR! Before we can merge this PR, please make sure that all the following items have been From 7a87a784bf4d6192734a917183f39e06c043189d Mon Sep 17 00:00:00 2001 From: Marko Date: Tue, 28 Apr 2020 16:07:11 +0200 Subject: [PATCH 14/14] ci: only run when applicable (#4752) - only run tests when .go, .mod, .sum files have been touched - only run proto checks when .proto files have been touched Signed-off-by: Marko Baricevic marbar3778@yahoo.com --- .github/workflows/proto.yml | 9 ++++++++- .github/workflows/tests.yml | 36 ++++++++++++++++++++++++++++++++---- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/.github/workflows/proto.yml b/.github/workflows/proto.yml index ddc9ee4c4..3add52404 100644 --- a/.github/workflows/proto.yml +++ b/.github/workflows/proto.yml @@ -5,8 +5,15 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@master - - uses: docker-practice/actions-setup-docker@master + - uses: technote-space/get-diff-action@v1 + id: git_diff + with: + SUFFIX_FILTER: .proto + SET_ENV_NAME_INSERTIONS: 1 + SET_ENV_NAME_LINES: 1 - name: lint run: make proto-lint + if: "env.GIT_DIFF != ''" - name: check-breakage run: make proto-check-breaking-ci + if: "env.GIT_DIFF != ''" diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 75ca8b4a0..54417c490 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -7,6 +7,23 @@ on: - release/** jobs: + Diff: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - uses: technote-space/get-diff-action@v1 + id: git_diff + with: + SUFFIX_FILTER: | + .go + .mod + .sum + SET_ENV_NAME_INSERTIONS: 1 + SET_ENV_NAME_LINES: 1 + - name: test + run: exit 1 + if: "env.GIT_DIFF == ''" + cleanup-runs: runs-on: ubuntu-latest steps: @@ -14,6 +31,7 @@ jobs: env: GITHUB_TOKEN: "${{ secrets.GITHUB_TOKEN }}" if: "!startsWith(github.ref, 'refs/tags/') && github.ref != 'refs/heads/master'" + build: name: Build runs-on: ubuntu-latest @@ -33,7 +51,7 @@ jobs: test_abci_apps: runs-on: ubuntu-latest - needs: Build + needs: [Build, Diff] steps: - uses: actions/setup-go@v2-beta - name: Set GOBIN @@ -50,7 +68,7 @@ jobs: test_abci_cli: runs-on: ubuntu-latest - needs: Build + needs: [Build, Diff] steps: - uses: actions/setup-go@v2-beta - name: Set GOBIN @@ -68,15 +86,25 @@ jobs: runs-on: ubuntu-latest needs: Build steps: - - uses: actions/setup-go@v2-beta + - uses: actions/checkout@v2 + - uses: technote-space/get-diff-action@v1 + id: git_diff + with: + SUFFIX_FILTER: | + .go + .mod + .sum + SET_ENV_NAME_INSERTIONS: 1 + SET_ENV_NAME_LINES: 1 - name: Set GOBIN run: | echo "::add-path::$(go env GOPATH)/bin" - - uses: actions/checkout@v2 - uses: actions/cache@v1 with: path: ~/go/bin key: ${{ runner.os }}-go-tm-binary + if: "env.GIT_DIFF != ''" - name: test_apps run: test/app/test.sh shell: bash + if: "env.GIT_DIFF != ''"