## 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
Sam Kleinman
Callum Waters
Marko
Anton Kaliaev
Erik Grinaker
Stevan Ognjanovic
Alessio Treglia
Anca Zamfir
Gustavo Chaín
Ethan Buchman
Phil Salant
Thane Thomson