## Description
This PR aims to make the crypto.PubKey interface more intuitive.
Changes:
- `VerfiyBytes` -> `VerifySignature`
Before `Bytes()` was amino encoded, now since it is the byte representation should we get rid of it entirely?
EDIT: decided to keep `Bytes()` as it is useful if you are using the interface instead of the concrete key
Closes: #XXX
Fixes#5192.
@liamsi Can you verify that the test vectors match the Rust implementation? I updated `ProofsFromByteSlices()` as well, anything else that should be updated?
Solves #5138 in the way that if a validatorSet is nil or empty it will not try to transform it to protobug
Co-authored-by: Callum Michael Waters <cmwaters19@gmail.com>
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.
## Description
This PR removes simple prefix from all types in the crypto/merkle directory.
The two proto types `Proof` & `ProofOp` have been moved to the `proto/crypto/merkle` directory.
proto messge `Proof` was renamed to `ProofOps` and `SimpleProof` message to `Proof`.
Closes: #2755
Creates Amnesia Evidence which is formed from Potential Amnesia Evidence with either a matching proof or after a period of time denoted as the Amnesia Trial Period. This also adds the code necessary so that Amnesia Evidence can be validated and committed on a block
Closes#4603
Commands used (VIM):
```
:args `rg -l errors.Wrap`
:argdo normal @q | update
```
where q is a macros rewriting the `errors.Wrap` to `fmt.Errorf`.
Closes#4783
It looks like we're validating Commit twice. Also, height and blockID params were coming from the commit, so no need to pass them separately.
* 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
* 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
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
* 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
* 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
* init of (2/2) common errors
* Remove instances of cmn.Error (2/2)
- Replace usage of cmnError and errorWrap
- ref #3862
Signed-off-by: Marko Baricevic <marbar3778@yahoo.com>
* comment wording
* simplify IsErrXXX functions
* log panic along with stopping the MConnection
Add gocritic as a linter
The linting is not complete, but should i complete in this PR or in a following.
23 files have been touched so it may be better to do in a following PR
Commits:
* Add gocritic to linting
- Added gocritic to linting
Signed-off-by: Marko Baricevic <marbar3778@yahoo.com>
* gocritic
* pr comments
* remove switch in cmdBatch
cleanup to add linter
grpc change:
https://godoc.org/google.golang.org/grpc#WithContextDialerhttps://godoc.org/google.golang.org/grpc#WithDialer
grpc/grpc-go#2627
prometheous change:
due to UninstrumentedHandler, being deprecated in the future
empty branch = empty if or else statement
didn't delete them entirely but commented
couldn't find a reason to have them
could not replicate the issue #3406
but if want to keep it commented then we should comment out the if statement as well
* Expose priv validators for use in testing
* Generalize block header validation test past height 1
* Remove ineffectual assignment
* Remove redundant SaveState call
* Reorder comment for clarity
* Use the block executor ApplyBlock function instead of implementing a stripped-down version of it
* Remove commented-out code
* Remove unnecessary test
The required tests already appear to be implemented (implicitly) through
the TestValidateBlockHeader test.
* Allow for catching of specific error types during TestValidateBlockCommit
* Make return error testable
* Clean up and add TestValidateBlockCommit code
* Fix formatting
* Extract function to create a new mock test app
* Update comment for clarity
* Fix comment
* Add skeleton code for evidence-related test
* Allow for addressing priv val by address
* Generalize test beyond a single validator
* Generalize TestValidateBlockEvidence past first height
* Reorder code to clearly separate tests and utility code
* Use a common constant for stop height for testing in state/validation_test.go
* Refactor errors to resemble existing conventions
* Fix formatting
* Extract common helper functions
Having the tests littered with helper functions makes them less easily
readable imho, so I've pulled them out into a separate file. This also
makes it easier to see what helper functions are available during
testing, so we minimize the chance of duplication when writing new
tests.
* Remove unused parameter
* Remove unused parameters
* Add field keys
* Remove unused height constant
* Fix typo
* Fix incorrect return error
* Add field keys
* Use separate package for tests
This refactors all of the state package's tests into a state_test
package, so as to keep any usage of the state package's internal methods
explicit.
Any internal methods/constants used by tests are now explicitly exported
in state/export_test.go
* Refactor: extract helper function to make, validate, execute and commit a block
* Rename state function to makeState
* Remove redundant constant for number of validators
* Refactor mock evidence registration into TestMain
* Remove extraneous nVals variable
* Replace function-level TODOs with file-level TODO and explanation
* Remove extraneous comment
* Fix linting issues brought up by GolangCI (pulled in from latest merge from develop)
* VoteSignBytes builds CanonicalVote
* CommitVotes implements VoteSetReader
- new CommitVotes struct holds both the Commit and the ValidatorSet and
implements VoteSetReader
- ToVote takes a ValidatorSet
* fix TestCommit
* use CommitSig.BlockID
Commits may include votes for a different BlockID, could be nil,
or different altogether. This means we can't use `commit.BlockID`
for reconstructing the sign bytes, since up to -1/3 of the commits
might be for independent BlockIDs. This means CommitSig will need to
include an indicator for what BlockID it signed - if it's not the
committed one or nil, it will need to include it fully in order to be
verified. This is unfortunate but unavoidable so long as we include
votes for non-committed BlockIDs (which we do to track validator
liveness)
* fixes from review
* remove CommitVotes. CommitSig contains address
* remove commit.canonicalVote method
* toVote -> getVote, takes valIdx
* update adr-025
* commit.ToVoteSet -> CommitToVoteSet
* add test
* fix from review
* fix failure in TestProposerFrequency
* Add test to check priority order after updates
* Changed applyRemovals() and removed Remove()
Changed applyRemovals() similar to applyUpdates()
Removed function Remove()
Updated comments
* review comments
* simplify applyRemovals and add more comments
* small correction in comment
* Fix check in test
* Fix priority check for centering, address review comments
* fix assert for priority centering
* review comments
* review comments
* cleanup and review comments
added upper limit check for validator voting power
moved check for empty validator set earlier
moved panic on potential negative set length in verifyRemovals
added more tests
* review comments
* Initial commit for 3181..still early
* unit test updates
* unit test updates
* fix check of dups accross updates and deletes
* simplify the processChange() func
* added overflow check utest
* Added checks for empty valset, new utest
* deepcopy changes in processUpdate()
* moved to new API, fixed tests
* test cleanup
* address review comments
* make sure votePower > 0
* gofmt fixes
* handle duplicates and invalid values
* more work on tests, review comments
* Renamed and explained K
* make TestVal private
* split verifyUpdatesAndComputeNewPriorities.., added check for deletes
* return error if validator set is empty after processing changes
* address review comments
* lint err
* Fixed the total voting power and added comments
* fix lint
* fix lint
* types: memoize height/round in commit instead of first vote
* types: commit.ValidateBasic in VerifyCommit
* types: new CommitSig alias for Vote
In preparation for reducing the redundancy in Commits, we introduce the
CommitSig as an alias for Vote. This is non-breaking on the protocol,
and minor breaking on the Go API, as Commit now contains a list of
CommitSig instead of Vote.
* remove dependence on ToVote
* update some comments
* fix tests
* fix tests
* fixes from review
* base verifier: bc->bv and check chainid
* improve some comments
* comments in dynamic verifier
* fix comment in doc about BaseVerifier
It requires the validator set to perfectly match.
* failing test for #2862
* move errTooMuchChange to types. fixes#2862
* changelog, comments
* ic -> dv
* update comment, link to issue
* more proposer priority tests
- test that we don't reset to zero when updating / adding
- test that same power validators alternate
* add another test to track / simulate similar behaviour as in #2960
* address some of Chris' review comments
* address some more of Chris' review comments
* temporarily pushing branch with the following changes:
The total power might change if:
- a validator is added
- a validator is removed
- a validator is updated
Decrement the accums (of all validators) directly after any of these events
(by the inverse of the change)
* Fix 2960 by re-normalizing / scaling priorities to be in bounds of total
power, additionally:
- remove heap where it doesn't make sense
- avg. only at the end of IncrementProposerPriority instead of each
iteration
- update (and slightly improve)
TestAveragingInIncrementProposerPriorityWithVotingPower to reflect
above changes
* Fix 2960 by re-normalizing / scaling priorities to be in bounds of total
power, additionally:
- remove heap where it doesn't make sense
- avg. only at the end of IncrementProposerPriority instead of each
iteration
- update (and slightly improve)
TestAveragingInIncrementProposerPriorityWithVotingPower to reflect
above changes
* fix tests
* add comment
* update changelog pending & some minor changes
* comment about division will floor the result & fix typo
* Update TestLargeGenesisValidator:
- remove TODO and increase large genesis validator's voting power
accordingly
* move changelog entry to P2P Protocol
* Ceil instead of flooring when dividing & update test
* quickly fix failing TestProposerPriorityDoesNotGetResetToZero:
- divide by Ceil((maxPriority - minPriority) / 2*totalVotingPower)
* fix typo: rename getValWitMostPriority -> getValWithMostPriority
* test proposer frequencies
* return absolute value for diff. keep testing
* use for loop for div
* cleanup, more tests
* spellcheck
* get rid of using floats: manually ceil where necessary
* Remove float, simplify, fix tests to match chris's proof (#3157)
* types: ValidatorSet.Update preserves ProposerPriority
This solves the other issue discovered as part of #2718,
where Accum (now called ProposerPriority) is reset to
0 every time a validator is updated.
* update changelog
* add test
* update comment
* Update types/validator_set_test.go
Co-Authored-By: ebuchman <ethan@coinculture.info>
* set the accum of a new validator to (-total voting power):
- disincentivize validators to unbond, then rebon to reset their
negative Accum to zero
additional unrelated changes:
- do not capitalize error msgs
- fix typo
* review comments: (re)capitalize errors & delete obsolete comments
* More changes suggested by @melekes
* WIP: do not batch clip (#2809)
* substract avgAccum on each iteration
- temporarily skip test
* remove unused method safeMulClip / safeMul
* always substract the avg accum
- temp. skip another test
* remove overflow / underflow tests & add tests for avgAccum:
- add test for computeAvgAccum
- as we substract the avgAccum now we will not trivially over/underflow
* address @cwgoes' comments
* shift by avg at the end of IncrementAccum
* Add comment to MaxTotalVotingPower
* Guard inputs to not exceed MaxTotalVotingPower
* Address review comments:
- do not fetch current validator from set again
- update error message
* Address a few review comments:
- fix typo
- extract variable
* address more review comments:
- clarify 1.125*totalVotingPower == totalVotingPower + (totalVotingPower >> 3)
* review comments: panic instead of "clipping":
- total voting power is guarded to not exceed MaxTotalVotingPower ->
panic if this invariant is violated
* fix failing test
Marko
Erik Grinaker
Anton Kaliaev
Callum Waters
Stevan Ognjanovic
Alessio Treglia
Anca Zamfir
Gustavo Chaín
Ethan Buchman
Phil Salant
Thane Thomson
Ismail Khoffi