Closes#4926
The dump consensus state had this:
"last_commit": {
"votes": [
"Vote{0:04CBBF43CA3E 385085/00/2(Precommit) 1B73DA9FC4C8 42C97B86D89D @ 2020-05-27T06:46:51.042392895Z}",
"Vote{1:055799E028FA 385085/00/2(Precommit) 652B08AD61EA 0D507D7FA3AB @ 2020-06-28T04:57:29.20793209Z}",
"Vote{2:056024CFA910 385085/00/2(Precommit) 652B08AD61EA C8E95532A4C3 @ 2020-06-28T04:57:29.452696998Z}",
"Vote{3:0741C95814DA 385085/00/2(Precommit) 652B08AD61EA 36D567615F7C @ 2020-06-28T04:57:29.279788593Z}",
Note there's a precommit in there from the first val from May (2020-05-27) while the rest are from today (2020-06-28). It suggests there's a validator from an old instance of the network at this height (they're using the same chain-id!). Obviously a single bad validator shouldn't be an issue. But the Commit refactor work introduced a bug.
When we propose a block, we get the block.LastCommit by calling MakeCommit on the set of precommits we saw for the last height. This set may include precommits for a different block, and hence the block.LastCommit we propose may include precommits that aren't actually for the last block (but of course +2/3 will be). Before v0.33, we just skipped over these precommits during verification. But in v0.33, we expect all signatures for a blockID to be for the same block ID! Thus we end up proposing a block that we can't verify.
Since the light client work introduced in v0.33 it appears full nodes
are no longer fully verifying commit signatures during block execution -
they stop after +2/3. See in VerifyCommit:
0c7fd316eb/types/validator_set.go (L700-L703)
This means proposers can propose blocks that contain valid +2/3
signatures and then the rest of the signatures can be whatever they
want. They can claim that all the other validators signed just by
including a CommitSig with arbitrary signature data. While this doesn't
seem to impact safety of Tendermint per se, it means that Commits may
contain a lot of invalid data. This is already true of blocks, since
they can include invalid txs filled with garbage, but in that case the
application knows they they are invalid and can punish the proposer. But
since applications dont verify commit signatures directly (they trust
tendermint to do that), they won't be able to detect it.
This can impact incentivization logic in the application that depends on
the LastCommitInfo sent in BeginBlock, which includes which validators
signed. For instance, Gaia incentivizes proposers with a bonus for
including more than +2/3 of the signatures. But a proposer can now claim
that bonus just by including arbitrary data for the final -1/3 of
validators without actually waiting for their signatures. There may be
other tricks that can be played because of this.
In general, the full node should be a fully verifying machine. While
it's true that the light client can avoid verifying all signatures by
stopping after +2/3, the full node can not. Thus the light client and
full node should use distinct VerifyCommit functions if one is going to
stop after +2/3 or otherwise perform less validation (for instance light
clients can also skip verifying votes for nil while full nodes can not).
See a commit with a bad signature that verifies here: 56367fd. From what
I can tell, Tendermint will go on to think this commit is valid and
forward this data to the app, so the app will think the second validator
actually signed when it clearly did not.
- Move core stateless validation of the Header type to a ValidateBasic method.
- Call header.ValidateBasic during a SignedHeader validation.
- Call header.ValidateBasic during a PhantomValidatorEvidence validation.
- Call header.ValidateBasic during a LunaticValidatorEvidence validation.
lite tests are skipped since the package is deprecated, no need to waste time on it
closes: #4572
Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
* format: add format cmd & goimport repo
- replaced format command
- added goimports to format command
- ran goimports
Signed-off-by: Marko Baricevic <marbar3778@yahoo.com>
* fix outliers & undo proto file changes
* test functions take time.Now and other minor changes
* updated remaining test files
* Update validation_test.go
* fix typo
* go fmt
* import time
Co-authored-by: Marko <marbar3778@yahoo.com>
* proto: minor linting
minor linting after working with the proto files in the sdk.
there is no logic change just spacing fixes
Signed-off-by: Marko Baricevic <marbar3778@yahoo.com>
* hardcore linting
* evidence: introduce time.Duration to evidence params
- add time.duration to evidence
- this pr is taking pr #2606 and updating it to use both time and height
- closes#2565
Signed-off-by: Marko Baricevic <marbar3778@yahoo.com>
* fix testing and genesis cfg in signer harness
* remove debugging fmt
* change maxageheight to maxagenumblocks, rename other things to block instead of height
* further check of duration
* check duration to not send peers outdated evidence
* change some lines, onward and upward
* refactor evidence package
* add a changelog pending entry
* make mockbadevidence have time and use it
* add what could possibly be called a test case
* remove mockbadevidence and mockgoodevidence in favor of mockevidence
* add a comment for err that is returned
* add a changelog for removal of good & bad evidence
* add a test for adding evidence
* fix test
* add ev to types in testcase
* Update evidence/pool_test.go
Co-Authored-By: Anton Kaliaev <anton.kalyaev@gmail.com>
* Update evidence/pool_test.go
Co-Authored-By: Anton Kaliaev <anton.kalyaev@gmail.com>
* fix tests
* fix linting
Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
* lint: golint issue fixes
- on my local machine golint is a lot stricter than the bot so slowly going through and fixing things.
Signed-off-by: Marko Baricevic <marbar3778@yahoo.com>
* more fixes from golint
* remove isPeerPersistentFn
* add changelog entry
* libs/common: Refactor libs/common 5
- move mathematical functions and types out of `libs/common` to math pkg
- move net functions out of `libs/common` to net pkg
- move string functions out of `libs/common` to strings pkg
- move async functions out of `libs/common` to async pkg
- move bit functions out of `libs/common` to bits pkg
- move cmap functions out of `libs/common` to cmap pkg
- move os functions out of `libs/common` to os pkg
Signed-off-by: Marko Baricevic <marbar3778@yahoo.com>
* fix testing issues
* fix tests
closes#41417
woooooooooooooooooo kill the cmn pkg
Signed-off-by: Marko Baricevic <marbar3778@yahoo.com>
* add changelog entry
* fix goimport issues
* run gofmt
* libs/common: Refactor libs/common 4
- move byte function out of cmn to its own pkg
- move tempfile out of cmn to its own pkg
- move throttletimer to its own pkg
ref #4147
Signed-off-by: Marko Baricevic <marbar3778@yahoo.com>
* add changelog entry
* fix linting issues
* libs/common: refactor libs common 3
- move nil.go into types folder and make private
- move service & baseservice out of common into service pkg
ref #4147
Signed-off-by: Marko Baricevic <marbar3778@yahoo.com>
* add changelog entry
* libs/common: refactor libs/common 2
- move random function to there own pkg
Signed-off-by: Marko Baricevic <marbar3778@yahoo.com>
* change imports and usage throughout repo
* fix goimports
* add changelog entry
* libs/common: Refactor libs/common 01
Signed-off-by: Marko Baricevic <marbar3778@yahoo.com>
* regenerate proto files, move intslice to where its used
* update kv.KVPair(s) to kv.Pair(s)
* add changelog entry
* make intInSlice private
* Rename Tag(s) to Event(s)
- tag was replaced with event, but in some places it still mentions tag, would be easier to understand if we tried to replace it with event to not confuse people.
Signed-off-by: Marko Baricevic <marbar3778@yahoo.com>
* more changes from tag -> event
* rename events to compositeKeys and keys
* Apply suggestions from code review
Co-Authored-By: Anton Kaliaev <anton.kalyaev@gmail.com>
* add minor documentation on how composite keys are constructed
* rename eventkey to compositekey
Co-Authored-By: Anton Kaliaev <anton.kalyaev@gmail.com>
* add changelog entry & add info to regenerate confid to changelog entry
Fix for #4164
The general problem is that in certain conditions an overflow warning is issued when attempting to update a validator set even if the final set's total voting power is not over the maximum allowed.
Root cause is that in verifyUpdates(), updates are verified wrt to total voting power in the order of validator address. It is then possible that a low address validator may increase its power such that the temporary total voting power count goes over MaxTotalVotingPower.
Scenarios where removing and adding/ updating validators with high voting power, in the same update operation, cause the same false warning and the updates are not applied.
Main changes to fix this are in verifyUpdate() that now does the verification starting with the decreases in power. It also takes into account the removals that are part of the update.
## Commits:
* tests for overflow detection and prevention
* test fix
* more tests
* fix the false overflow warnings and golint
* scopelint warning fix
* review comments
* variant with using sort by amount of change in power
* compute separately number new validators in update
* types: use a switch in processChanges
* more review comments
* types: use HasAddress in numNewValidators
* types: refactor verifyUpdates
copy updates, sort them by delta and use resulting slice to calculate
tvpAfterUpdatesBeforeRemovals.
* remove unused structs
* review comments
* update changelog
* types: change `Commit` to consist of just signatures
These are final changes towards removing votes from commit and leaving
only signatures (see ADR-25)
Fixes#1648
* bring back TestCommitToVoteSetWithVotesForAnotherBlockOrNilBlock
+ add absent flag to Vote to indicate that it's for another block
* encode nil votes as CommitSig with BlockIDFlagAbsent
+ make Commit#Precommits array of non-pointers
because precommit will never be nil
* add NewCommitSigAbsent and Absent() funcs
* uncomment validation in CommitSig#ValidateBasic
* add comments to ValidatorSet funcs
* add a changelog entry
* break instead of continue
continue does not make sense in these cases
* types: rename Commit#Precommits to Signatures
* swagger: fix /commit response
* swagger: change block_id_flag type
* fix merge conflicts
Refs #1771
ADR: https://github.com/tendermint/tendermint/blob/master/docs/architecture/adr-044-lite-client-with-weak-subjectivity.md
## Commits:
* add Verifier and VerifyCommitTrusting
* add two more checks
make trustLevel an option
* float32 for trustLevel
* check newHeader time
* started writing lite Client
* unify Verify methods
* ensure h2.Header.bfttime < h1.Header.bfttime + tp
* move trust checks into Verify function
* add more comments
* more docs
* started writing tests
* unbonding period failures
* tests are green
* export ErrNewHeaderTooFarIntoFuture
* make golangci happy
* test for non-adjusted headers
* more precision
* providers and stores
* VerifyHeader and VerifyHeaderAtHeight funcs
* fix compile errors
* remove lastVerifiedHeight, persist new trusted header
* sequential verification
* remove TrustedStore option
* started writing tests for light client
* cover basic cases for linear verification
* bisection tests PASS
* rename BisectingVerification to SkippingVerification
* refactor the code
* add TrustedHeader method
* consolidate sequential verification tests
* consolidate skipping verification tests
* rename trustedVals to trustedNextVals
* start writing docs
* ValidateTrustLevel func and ErrOldHeaderExpired error
* AutoClient and example tests
* fix errors
* update doc
* remove ErrNewHeaderTooFarIntoFuture
This check is unnecessary given existing a) ErrOldHeaderExpired b)
h2.Time > now checks.
* return an error if we're at more recent height
* add comments
* add LastSignedHeaderHeight method to Store
I think it's fine if Store tracks last height
* copy over proxy from old lite package
* make TrustedHeader return latest if height=0
* modify LastSignedHeaderHeight to return an error if no headers exist
* copy over proxy impl
* refactor proxy and start http lite client
* Tx and BlockchainInfo methods
* Block method
* commit method
* code compiles again
* lite client compiles
* extract updateLiteClientIfNeededTo func
* move final parts
* add placeholder for tests
* force usage of lite http client in proxy
* comment out query tests for now
* explicitly mention tp: trusting period
* verify nextVals in VerifyHeader
* refactor bisection
* move the NextValidatorsHash check into updateTrustedHeaderAndVals
+ update the comment
* add ConsensusParams method to RPC client
* add ConsensusParams to rpc/mock/client
* change trustLevel type to a new cmn.Fraction type
+ update SkippingVerification comment
* stress out trustLevel is only used for non-adjusted headers
* fixes after Fede's review
Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
* compare newHeader with a header from an alternative provider
* save pivot header
Refs https://github.com/tendermint/tendermint/pull/3989#discussion_r349122824
* check header can still be trusted in TrustedHeader
Refs https://github.com/tendermint/tendermint/pull/3989#discussion_r349101424
* lite: update Validators and Block endpoints
- Block no longer contains BlockMeta
- Validators now accept two additional params: page and perPage
* make linter happy
Fixes#3986
This pull request is prefixing all the types in proto to avoid conflict.
When a go application is using Tendermint as a library and also define similar types in gogo proto some conflicts might occur (as types is a common package in go).
By prefixing the types with tendermint, this highly reduces the risk of conflicts.
BREAKING CHANGE.
This modification breaks the ABCI Application endpoint.
What was accessible before with `/types.ABCIApplication/Flush` is now accessible with `/tendermint.abci.types.ABCIApplication/Flush`.
## Issue:
Removed BlockMeta in ResultBlock in favor of BlockId for /block
Added block_size to BlockMeta this is reflected in /blockchain
fixes#3188
added breaking as some clients may be using header from blockmeta instead of block in /block
## Commits:
* cleanup block path
Remove duplication of data in `/block`
fixes#3188
Signed-off-by: Marko Baricevic <marbar3778@yahoo.com>
* Remove duplication of data in `/block`
- Created a secondary type to be used for /block
fixes#3188
Signed-off-by: Marko Baricevic <marbar3778@yahoo.com>
* remove commented out code
* fix ci
* add changelog_pending entry
* remove extra variable
* update swagger
* change int to uint for blocksize
* fix swagger
* remove extensive comments
* update changelog
* fix conflicts after merge
* use int for BlockSize and NumTxs in BlockMeta
- with 99.9% guarantee, the size of either will never reach int32
- most of the Go "Size" stdlib functions return int
* Removal of TotalTx & NumTx
- Removed totalTx and numTx
closes#2521
Signed-off-by: Marko Baricevic <marbar3778@yahoo.com>
* abci proto changes
* proto number fix
* txfilter_test fix
* comments on PR
* further changes
* bring back metrics
* fix indexer
* fix TestBlockMaxDataBytes and TestBlockMaxDataBytesUnknownEvidence
* indexer service back to header
* statistics.go fix
* fix ci
* listen for blocks, not headers
to be able to record txs throughput
* fix TestNetworkNewBlock
* fix tests
* fix tests in types package
* fixes after Anton's review
* fix tests
* bring back `consensus_total_txs` metric
I mistakenly thought it was removed.
* improve changelog
* remove LastBlockTotalTx from state
* docs: remove getNumTxs from BeginBlock Java example
* crypto: expose MaxAunts for documentation purposes
* types: update godoc for new maxes
* docs: make hard-coded limits more explicit
* wal: add todo to clarify max size
* shorten lines in test
* cs: panic only when WAL#WriteSync fails
- modify WAL#Write and WAL#WriteSync to return an error
* fix test
* types: validate Part#Proof
add ValidateBasic to crypto/merkle/SimpleProof
* cs: limit max bit array size and block parts count
* cs: test new limits
* cs: only assert important stuff
* update changelog and bump version to 0.32.7
* fixes after Ethan's review
* align max wal msg and max consensus msg sizes
* fix tests
* fix test
* add change log for 31.11
* Fix long line errors in abci, crypto, and libs packages
* Fix long lines in p2p and rpc packages
* Fix long lines in abci, state, and tools packages
* Fix long lines in behaviour and blockchain packages
* Fix long lines in cmd and config packages
* Begin fixing long lines in consensus package
* Finish fixing long lines in consensus package
* Add lll exclusion for lines containing URLs
* Fix long lines in crypto package
* Fix long lines in evidence package
* Fix long lines in mempool and node packages
* Fix long lines in libs package
* Fix long lines in lite package
* Fix new long line in node package
* Fix long lines in p2p package
* Ignore gocritic warning
* Fix long lines in privval package
* Fix long lines in rpc package
* Fix long lines in scripts package
* Fix long lines in state package
* Fix long lines in tools package
* Fix long lines in types package
* Enable lll linter
* Remove omitempty from *pb.go
- remove omitempty from *pb.go files
- added command to makefile for everytime `make protoc_all` is run
- open question:
- Do we want to further remove omitempty from other places
- https://github.com/tendermint/tendermint/blob/master/rpc/lib/types/types.go#L151
- and other places
ref #3882
Signed-off-by: Marko Baricevic <marbar3778@yahoo.com>
* bring back omitempty to *pb.go
* Update types/tx.go
* custom marshlers
* undo benchmark `omitepmty`
* golangci lint fix
* cleanup comments
* changelog_pending entry
* types: add test for block commits with votes for the wrong blockID
* remove
* respond to feedback
* verify the commits as well
* testing table
* Update types/block_test.go
Co-Authored-By: Bot from GolangCI <42910462+golangcibot@users.noreply.github.com>
* Update types/block_test.go
Co-Authored-By: Bot from GolangCI <42910462+golangcibot@users.noreply.github.com>
* gofmt
* test panic case
* Pin range scope vars
* Don't disable scopelint
This PR repairs linter errors seen when running the following commands:
golangci-lint run --no-config --disable-all=true --enable=scopelint
Contributes to #3262
Anton Kaliaev
Marko
Alexander Bezobchuk
Erik Grinaker
Callum Waters
Shivani Joshi
Alessio Treglia
Erik Grinaker
Sunny Aggarwal
Anca Zamfir
Anthony
Gustavo Chaín
Aditya
Ethan Buchman
Phil Salant
Tess Rinearson