While working on tendermint my colleague @jinmannwong fixed a few of the unit tests that we found to be flaky in our CI. We thought that you might find this useful, see below for comments.
Closes#1581
This fixes the error in #1581, and also documents the purpose of this line. It ensures that if a peer tells us an address we know about, whose ID is the same as our current ID, we ignore it.
This removes the previous case where the ID's matched, but the IP's did not, which could yield a potential overwrite of the IP associated with the address later on. (This then would yield an eclipse attack)
This was not a vulnerability before though, thanks to a defensive check here 95fc7e58ee/p2p/pex/addrbook.go (L522))
## Description
This PR wraps the stdlib sync.(RW)Mutex & godeadlock.(RW)Mutex. This enables using go-deadlock via a build flag instead of using sed to replace sync with godeadlock in all files
Closes: #3242
## Description
this log message was marked as not useful and in the issue it was proposed to move it to debug. I am going with this path for now. After we have refactored the logger we shold go through the codebase in order to clean our log statements.
Closes: #2101
Closes#4934
* light: do not compare trusted header w/ witnesses
we don't have trusted state to bisect from
* check header before checking height
otherwise you can get nil panic
- drop Height & Base from StatusRequest
It does not make sense nor it's used anywhere currently. Also, there
seem to be no trace of these fields in the ADR-40 (blockchain reactor
v2).
- change PacketMsg#EOF type from int32 to bool
Reorganizes the Protobuf schemas. It is mostly bikeshedding, so if something is contentious or causes a lot of extra work then I'm fine with reverting. Some Protobuf and Go import paths will change.
* Move `abci/types/types.proto` to `abci/types.proto`.
* Move `crypto/keys/types.proto` and `crypto/merkle/types.proto` to `crypto/keys.proto` and `crypto/proof.proto`.
* Drop the use of `msgs` in filenames, as "message" is a very overloaded term (all Protobuf types are messages, and we also have `message Message`). Use `types.proto` as a catch-all, and otherwise name files by conceptual grouping instead of message kind.
Closes#5074
Old code does not work when --consensus.create_empty_blocks=false
(because it only calls tmos.Kill when ApplyBlock fails). New code is
listening ABCI clients for Quit and kills TM process if there were any
errors.
## Description
codecov is having issues on upload so upgrade to 1.0.7 where they claim it works better and dont fail ci on failure to upload coverage file
Closes: #XXX
* fix#5086
* fixes#5082
- run tendermint init on runtime (if necessary)
* Address some feedback:
- restrict the entrypoint to only run `tendermint`
- script into /usr/local/bin
* make it also possible to run ``ith unmodified config again via:
`docker run -v $HOME/.tendermint:/tendermint tendermint/tendermint init
* Update DOCKER/docker-entrypoint.sh
Co-authored-by: Greg Szabo <16846635+greg-szabo@users.noreply.github.com>
Co-authored-by: Greg Szabo <16846635+greg-szabo@users.noreply.github.com>
check bcR.fastSync flag when "OnStop"
fix "service/service.go:161 Not stopping BlockPool -- have not been started yet {"impl": "BlockPool"}" error when kill process
## Description
This adr is meant to weight the pros and cons of gRPC and JSON-RPC. It is fairly incomplete on the JSON-RPC side.
EDIT: Thank you to erik on filling out the pros and cons!!
Work Towards: #3367
## Description
this PR adds test vectors for proto encoding. the main difference from amino was the removal of four bytes due to interface encoding.
should i add more cases?
Closes: #XXX
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.
In order to have more control over the mempool implementation,
introduce a new exported function RemoveTxByKey.
Export also TxKey() and TxKeySize. Use TxKeySize const instead of
sha256.size, so future changes on the hash function won't break the API.
Allows using a TxKey (32 bytes reference) as parameter instead of
the complete array set. So the application layer does not need to
keep track of the whole transaction but only of the sha256 hash (32 bytes).
This function is useful when mempool.Recheck is disabled.
Allows the Application layer to implement its own cleaning mechanism
without having to re-implement the whole mempool interface.
Mempool.Update() would probably also need to change from txBytes to txKey,
but that would require to change the Interface thus will break backwards
compatibility. For now RemoveTxByKey() looks like a good compromise,
it won't break anything and will help to solve some mempool issues from the
application layer.
Signed-off-by: p4u <pau@dabax.net>
## Description
partially cleanup in preparation for errcheck
i ignored a bunch of defer errors in tests but with the update to go 1.14 we can use `t.Cleanup(func() { if err := <>; err != nil {..}}` to cover those errors, I will do this in pr number two of enabling errcheck.
ref #5059
Anton Kaliaev
n-hutton
Dev Ojha
Marko
dependabot-preview[bot]
Tess Rinearson
Callum Waters
Erik Grinaker
Ismail Khoffi
Lei Wang
Ethan Buchman
Pau