Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rpc: Error using NetInfo in RPC client for Cosmos-SDK v0.30.0 #3251

Closed
fkbenjamin opened this issue Feb 5, 2019 · 13 comments
Closed

rpc: Error using NetInfo in RPC client for Cosmos-SDK v0.30.0 #3251

fkbenjamin opened this issue Feb 5, 2019 · 13 comments
Assignees
Labels
C:rpc Component: JSON RPC, gRPC T:bug Type Bug (Confirmed)
Milestone

Comments

@fkbenjamin
Copy link

fkbenjamin commented Feb 5, 2019

Tendermint version :
1809efa

ABCI app:
Cosmos-SDK v0.30.0

What happened:
Using the rpc websocket in Go:

import (
 "fmt"
 "github.com/tendermint/tendermint/rpc/client"
)

cosmosClient := client.NewHTTP(config.GaiaUrl, "/websocket")
netInfo, err := cosmosClient.NetInfo()
		if err != nil {
			fmt.Println(err)
		}

Results in printing the following error:
NetInfo: Error unmarshalling rpc response result: invalid IP address: AAAAAAAAAAAAAP//rB8ktw==

Seems like this was broken by changing the plain text IP addresses field for this command to amino encoded IP address field in Cosmos. Hope this still belongs here.

@ebuchman ebuchman added T:bug Type Bug (Confirmed) help wanted labels Feb 6, 2019
@jacohend
Copy link

jacohend commented Feb 12, 2019

This is also affecting the same RPC call on a plain tendermint network (no cosmos sdk).

melekes added a commit that referenced this issue Feb 14, 2019
Before:
"AAAAAAAAAAAAAP//rB8ktw=="
which is amino-encoded net.IP byte slice

After:
"192.0.2.1"

Fixes #3251
@melekes melekes self-assigned this Feb 14, 2019
melekes added a commit that referenced this issue Feb 16, 2019
* rpc/net_info: change RemoteIP type from net.IP to String

Before:
"AAAAAAAAAAAAAP//rB8ktw=="
which is amino-encoded net.IP byte slice

After:
"192.0.2.1"

Fixes #3251

* rpc/net_info: non-empty response in docs
@melekes
Copy link
Contributor

melekes commented Feb 16, 2019

Merged #3309 to develop. Should land in v0.30.1

@melekes melekes closed this as completed Feb 16, 2019
@jacohend
Copy link

jacohend commented Mar 5, 2019

@melekes That PR does not seem to have fixed the issue here. The net_info endpoint is still returning the same bad encoding. Can you please reopen?

@jacohend
Copy link

jacohend commented Mar 5, 2019

It does look like the rpc endpoint call is succeeding, though, so I guess maybe this issue is fixed, even if the encoding is still odd?

@melekes melekes reopened this Mar 5, 2019
@melekes
Copy link
Contributor

melekes commented Mar 5, 2019

What version of Tendermint are you using?

@jacohend
Copy link

jacohend commented Mar 6, 2019

I'm using both the master and the develop branches as of yesterday afternoon, and using the client rpc library as of e0f8936

@ebuchman ebuchman added this to the v0.31.1 milestone Mar 16, 2019
@melekes
Copy link
Contributor

melekes commented Mar 18, 2019

https://gist.github.com/melekes/00b1cdb76b3994b135b0eef39b05ebea
this is what I am getting when I execute curl localhost:26657/net_info

@ebuchman
Copy link
Contributor

How come this issue isn't/wasn't being caught by the TestNetInfo? https://github.com/tendermint/tendermint/blob/v0.30.0/rpc/client/rpc_test.go#L74

@melekes
Copy link
Contributor

melekes commented Mar 19, 2019

Because we do not create any peers assert.Equal(t, 0, len(netinfo.Peers)).

@melekes
Copy link
Contributor

melekes commented Mar 25, 2019

@jacohend can you post the output of /net_info here?

pinkiebell added a commit to leapdao/tendermint that referenced this issue Mar 28, 2019
* docs: update ecosystem.json: add Rust ABCI (#2945)

* types: ValidatorSet.Update preserves Accum (#2941)

* 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 <[email protected]>

* Add some ProposerPriority tests  (#2946)

* WIP: tests for #2785

* rebase onto develop

* add Bucky's test without changing ValidatorSet.Update

* make TestValidatorSetBasic fail

* add ProposerPriority preserving fix to ValidatorSet.Update to fix
TestValidatorSetBasic

* fix randValidator_ to stay in bounds of MaxTotalVotingPower

* check for expected proposer and remove some duplicate code

* actually limit the voting power of random validator ...

* fix test

* Bucky/v0.27.0 (#2950)

* update changelog

* changelog, upgrading, version

* explicitly type MaxTotalVotingPower to int64 (#2953)

* check if deliverTxResCh is still open, return an err otherwise (#2947)

deliverTxResCh, like any other eventBus (pubsub) channel, is closed when
eventBus is stopped. We must check if the channel is still open. The
alternative approach is to not close any channels, which seems a bit
odd.

Fixes #2408

* docs: add client#Start/Stop to examples in RPC docs (#2939)

follow-up on https://github.com/tendermint/tendermint/pull/2936

* Minor log changes (#2959)

* node: allow state and code to have diff block versions

* node: pex is a log module

* p2p: panic on transport error (#2968)

* p2p: panic on transport error

Addresses #2823. Currently, the acceptRoutine exits if the transport returns
an error trying to accept a new connection. Once this happens, the node
can't accept any new connections. So here, we panic instead. While we
could potentially be more intelligent by rerunning the acceptRoutine, the
error may indicate something more fundamental (eg. file desriptor limit)
that requires a restart anyways. We can leave it to process managers to
handle that restart, and notify operators about the panic.

* changelog

* p2p: fix peer count mismatch #2332 (#2969)

* p2p: test case for peer count mismatch #2332

* p2p: fix peer count mismatch #2332

* changelog

* use httptest.Server to scrape Prometheus metrics

* kv indexer: add separator to start key when matching ranges (#2925)

* kv indexer: add separator to start key when matching ranges

to avoid including false positives

Refs #2908

* refactor code

* add a test case

* update changelog and upgrading (#2974)

* don't ignore key when executing CONTAINS (#2924)

Fixes #2912

* return an error if validator set is empty in genesis file and after InitChain (#2971)

Fixes #2951

* docs: relative links in docs/spec/readme.md, js-amino lib (#2977)

Co-Authored-By: zramsay <[email protected]>

* turn off strict routability every time (#2983)

previously, we're turning it off only when --populate-persistent-peers
flag was used, which is obviously incorrect.

Fixes https://github.com/cosmos/cosmos-sdk/issues/2983

* Make mempool fail txs with negative gas wanted (#2994)

This is only one part of #2989. We also need to fix the application,
and add rules to consensus to ensure this.

* p2p: set MConnection#created during init (#2990)

Fixes #2715

In crawlPeersRoutine, which is performed when seedMode is run, there is
logic that disconnects the peer's state information at 3-hour intervals
through the duration value. The duration value is calculated by
referring to the created value of MConnection. When MConnection is
created for the first time, the created value is not initiated, so it is
not disconnected every 3 hours but every time it is disconnected. So,
normal nodes are connected to seedNode and disconnected immediately, so
address exchange does not work properly.

https://github.com/tendermint/tendermint/blob/master/p2p/pex/pex_reactor.go#L629
This point is not work correctly.
I think,
https://github.com/tendermint/tendermint/blob/master/p2p/conn/connection.go#L148
created variable is missing the current time setting.

* Make testing logger that doesn't write to stdout (#2997)

* add UnconfirmedTxs/NumUnconfirmedTxs methods to HTTP/Local clients (#2964)

* docs: fixes from 'first time' review (#2999)

* docs: enable full-text search (#3004)

* mempool: add a comment and missing changelog entry (#2996)

Refs #2994

* circleci: add a job to automatically update docs (#3005)

* docs: add edit on Github links (#3014)

* docs: update DOCS_README (#3019)

Co-Authored-By: zramsay <[email protected]>

* mempool: notifyTxsAvailable if there're txs left, but recheck=false (#2991)

(left after committing a block)

Fixes #2961

--------------
ORIGINAL ISSUE

Tendermint version : 0.26.4-b771798d

ABCI app : kv-store

Environment:

OS (e.g. from /etc/os-release): macOS 10.14.1 What happened: Set
mempool.recheck = false and create empty block = false in config.toml.
When transactions get added right between a new empty block is being
proposed and committed, the proposer won't propose new block for that
transactions immediately after. That transactions are stuck in the
mempool until a new transaction is added and trigger the proposer.

What you expected to happen: If there is a transaction left in the
mempool, new block should be proposed immediately.

Have you tried the latest version: yes

How to reproduce it (as minimally and precisely as possible): Fire two
transaction using broadcast_tx_sync with specific delay between them.
(You may need to do it multiple time before the right delay is found, on
my machine the delay is 0.98s)

Logs (paste a small part showing an error (< 10 lines) or link a
pastebin, gist, etc. containing more of the log file):
https://pastebin.com/0Wt6uhPF

Config (you can paste only the changes you've made): [mempool] recheck =
false create_empty_block = false

Anything else we need to know: In mempool.go, we found that proposer
will immediately propose new block if

Last committed block has some transaction (causing AppHash to changed)
or mem.notifyTxsAvailable() is called. Our scenario is as followed.

A transaction is fired, it will create 1 block with 1 tx (line 1-4 in
the log) and 1 empty block. After the empty block is proposed but before
it is committed, second transaction is fired and added to mempool. (line
8-16) Now, since the last committed block is empty and
mem.notifyTxsAvailable() will be called only if mempool.recheck = true.
The proposer won't immediately propose new block, causing the second
transaction to stuck in mempool until another transaction is added to
mempool and trigger mem.notifyTxsAvailable().

* During replay, when appHeight==0, only saveState if stateHeight is also 0 (#3006)

* optimize addProposalBlockPart

* optimize addProposalBlockPart

* if ProposalBlockParts and LockedBlockParts both exist,let LockedBlockParts overwrite ProposalBlockParts.

* fix tryAddBlock

* broadcast lockedBlockParts in higher priority

* when appHeight==0, it's better fetch genDoc than state.validators.

* not save state if replay from height 1

* only save state if replay from height 1 when stateHeight is also 1

* only save state if replay from height 1 when stateHeight is also 1

* only save state if replay from height 0 when stateHeight is also 0

* handshake info's response version only update when stateHeight==0

* save the handshake responseInfo appVersion

* 2980 fix cors (#3021)

* config: cors options are arrays of strings, not strings

Fixes #2980

* docs: update tendermint-core/configuration.html page

* set allow_duplicate_ip to false

* in `tendermint testnet`, set allow_duplicate_ip to true

Refs #2712

* fixes after Ismail's review

* Revert "set allow_duplicate_ip to false"

This reverts commit 24c1094ebcf2bd35f2642a44d7a1e5fb5c178fb1.

* #2980 fix cors doc (#3013)

* docs: networks/docker-compose: small fixes (#3017)

* Bucky/v0.27.1 (#3022)

* update changelog

* linkify

* changelog and version

* Revert to using defers in addrbook. (#3025)

* Revert to using defers in addrbook.  ValidateBasic->Validate since it requires DNS

* Update CHANGELOG_PENDING

* makeNodeInfo returns error (#3029)

* makeNodeInfo returns error

* version and changelog

* crypto: revert to mainline Go crypto lib (#3027)

* crypto: revert to mainline Go crypto lib

We used to use a fork for a modified bcrypt so we could pass our own
randomness but this was largely unecessary, unused, and a burden.
So now we just use the mainline Go crypto lib.

* changelog

* fix tests

* version and changelog

* fix docs / proxy app (#2988)

* fix docs / proxy app, closes #2986

* counter_serial

* review comments

* list all possible options

* add changelog entries

* mempool: move tx to back, not front (#3036)

because we pop txs from the front if the cache is full

Refs #3035

* update go version & other cleanup (#3018)

* update go version & other cleanup

* fix lints

* go one.eleven.four

* keep circle on 1.11.3 for now

* set allow_duplicate_ip to false (#2992)

* config: cors options are arrays of strings, not strings

Fixes #2980

* docs: update tendermint-core/configuration.html page

* set allow_duplicate_ip to false

* in `tendermint testnet`, set allow_duplicate_ip to true

Refs #2712

* fixes after Ismail's review

* docs: add rpc link to docs navbar and re-org sidebar (#3041)

* add rpc to docs navbar and close #3000

* Update config.js

* circleci: update go version (#3051)

* mempool: move tx to back, not front (#3036)

because we pop txs from the front if the cache is full

Refs #3035

* changelog and version

* R4R: Split immutable and mutable parts of priv_validator.json (#2870)

* split immutable and mutable parts of priv_validator.json

* fix bugs

* minor changes

* retrig test

* delete scripts/wire2amino.go

* fix test

* fixes from review

* privval: remove mtx

* rearrange priv_validator.go

* upgrade path

* write tests for the upgrade

* fix for unsafe_reset_all

* add test

* add reset test

* Update node_info.go (#3059)

* Remove privval.GetAddress(), memoize pubkey (#2948)

privval: remove GetAddress(), memoize pubkey

* 3070 [docs] unindent text as it is supposed to behave the same as the parts before (#3075)

* add signing spec (#3061)

* add signing spec

* fixes from review

* more fixes

* fixes from review

* fix build scripts (#3085)

* fix build scripts

Search for the right variable when introspecting Go code. `Version` was
renamed to `TMCoreSemVer`.

This is regression introduced in
b95ac688af14d130e6ad0b580ed9a8181f6c487c

* fix all `Version` introspections.

Use `TMCoreSemVer` instead of `Version`

* cs: prettify logging of ignored votes (#3086)

Refs #3038

* Don't use pointer receivers for PubKeyMultisigThreshold (#3100)

* Don't use pointer receivers for PubKeyMultisigThreshold

* test that showcases panic when PubKeyMultisigThreshold are used in sdk:

 - deserialization will fail in `readInfo` which tries to read a
 `crypto.PubKey` into a `localInfo` (called by
  cosmos-sdk/client/keys.GetKeyInfo)

* Update changelog

* Rename routeTable to nameTable, multisig key is no longer a pointer

* sed -i 's/PubKeyAminoRoute/PubKeyAminoName/g' `grep -lrw PubKeyAminoRoute .`

upon Jae's request

* AminoRoutes -> AminoNames

* sed -e 's/PrivKeyAminoRoute/PrivKeyAminoName/g'

* Update crypto/encoding/amino/amino.go

Co-Authored-By: alessio <[email protected]>

* Ensure multisig keys have 20-byte address (#3103)

* Ensure multisig keys have 20-byte address

Use crypto.AddressHash() to avoid returning 32-byte long address.

Closes: #3102

* fix pointer

* fix test

* update README (#3097)

* update README

* fix from review

* rpc: include peer's remote IP in `/net_info` (#3052)

Refs #3047

* docs: fix p2p readme links (#3109)

* Make SecretConnection thread safe (#3111)

* p2p/conn: add failing tests

* p2p/conn: make SecretConnection thread safe

* changelog

* fix from review

* Close and retry a RemoteSigner on err (#2923)

* Close and recreate a RemoteSigner on err

* Update changelog

* Address Anton's comments / suggestions:

 - update changelog
 - restart TCPVal
 - shut down on `ErrUnexpectedResponse`

* re-init remote signer client with fresh connection if Ping fails

- add/update TODOs in secret connection
- rename tcp.go -> tcp_client.go, same with ipc to clarify their purpose

* account for `conn returned by waitConnection can be `nil`

- also add TODO about RemoteSigner conn field

* Tests for retrying: IPC / TCP

 - shorter info log on success
 - set conn and use it in tests to close conn

* Tests for retrying: IPC / TCP

 - shorter info log on success
 - set conn and use it in tests to close conn
 - add rwmutex for conn field in IPC

* comments and doc.go

* fix ipc tests. fixes #2677

* use constants for tests

* cleanup some error statements

* fixes #2784, race in tests

* remove print statement

* minor fixes from review

* update comment on sts spec

* cosmetics

* p2p/conn: add failing tests

* p2p/conn: make SecretConnection thread safe

* changelog

* IPCVal signer refactor

- use a .reset() method
- don't use embedded RemoteSignerClient
- guard RemoteSignerClient with mutex
- drop the .conn
- expose Close() on RemoteSignerClient

* apply IPCVal refactor to TCPVal

* remove mtx from RemoteSignerClient

* consolidate IPCVal and TCPVal, fixes #3104

- done in tcp_client.go
- now called SocketVal
- takes a listener in the constructor
- make tcpListener and unixListener contain all the differences

* delete ipc files

* introduce unix and tcp dialer for RemoteSigner

* rename files

- drop tcp_ prefix
- rename priv_validator.go to file.go

* bring back listener options

* fix node

* fix priv_val_server

* fix node test

* minor cleanup and comments

* Hotfix/validating query result length (#3053)

* Validating that there are txs in the query results before loop throught the array

* Created tests to validate the error has been fixed

* Added comments

* Fixing misspeling

* check if the variable "skipCount" is bigger than zero. If it is not, we set it to 0. If it, we do not do anything.

* using function that validates the skipCount variable

* undo Gopkg.lock changes

* [WIP] Fill in consensus core details in ADR 030 (#2696)

* Initial work towards making ConsensusCore spec complete

* Initial version of executor and complete consensus

* Bucky/v0.28.0 (#3119)

* changelog pending and upgrading

* linkify and version bump

* changelog shuffle

* More ProposerPriority tests  (#2966)

* 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

* fix order of BlockID and Timestamp in Vote and Proposal (#3078)

* Consistent order fields of Timestamp/BlockID fields in CanonicalVote and
CanonicalProposal

* update spec too

* Introduce and use IsZero & IsComplete:
 - update IsZero method according to spec and introduce IsComplete
 - use methods in validate basic to validate: proposals come with a
 "complete" blockId and votes are either complete or empty
 - update spec: BlockID.IsNil() -> BlockID.IsZero() and fix typo

* BlockID comes first

* fix tests

* Simple merkle rfc compatibility (#2713)

* Begin simple merkle compatibility PR

* Fix query_test

* Use trillian test vectors

* Change the split point per RFC 6962

* update spec

* refactor innerhash to match spec

* Update changelog

* Address @liamsi's comments

* Write the comment requested by @liamsi

* Adds tests for Unix sockets

As per #3115, adds simple Unix socket connect/accept deadline tests in
pretty much the same way as the TCP connect/accept deadline tests work.

* update ADR-020 (#3116)

* [log] fix year format (#3125)

Refs #3060

* makefile: fix build-docker-localnode target (#3122)

cd does not work because it's executed in a subprocess
so it has to be either chained by && or ;
See https://stackoverflow.com/q/1789594/820520
for more details.

Fixes #3058

* return maxPerPage (not defaultPerPage) if per_page is greater than max (#3124)

it's more user-friendly.

Refs #3065

* Make privval listener testing generic

This cuts out two tests by constructing test cases and iterating through
them, rather than having separate sets of tests for TCP and Unix listeners.
This is as per the feedback from #3121.

* privval: fixes from review (#3126)

https://github.com/tendermint/tendermint/pull/2923#pullrequestreview-192065694

* docs: update link for rpc docs (#3129)

* Dropping "construct" prefix as per #3121

* add "chain_id" label for all metrics (#3123)

* add "chain_id" label for all metrics

Refs #3082

* fix labels extraction

* Expanding tests to cover Unix sockets version of client (#3132)

* Adds a random suffix to temporary Unix sockets during testing

* Adds Unix domain socket tests for client (FAILING)

This adds Unix domain socket tests for the privval client. Right now,
one of the tests (TestRemoteSignerRetry) fails, probably because the
Unix domain socket state is known instantaneously on both sides by the
OS. Committing this to collaborate on the error.

* Removes extraneous logging

* Completes testing of Unix sockets client version

This completes the testing of the client connecting via Unix sockets.
There are two specific tests (TestSocketPVDeadline and
TestRemoteSignerRetryTCPOnly) that are only relevant to TCP connections.

* Renames test to show TCP-specificity

* Adds testing into closures for consistency (forgot previously)

* Moves test specific to RemoteSigner into own file

As per discussion on #3132, `TestRemoteSignerRetryTCPOnly` doesn't
really belong with the client tests. This moves it into its own file
related to the `RemoteSigner` class.

* update changelog and upgrading (#3133)

* fix changelog fmt (#3134)

* fixes from review (#3137)

* Add comment to simple_merkle get_split_point (#3136)

* Add comment to simple_merkle get_split_point

* fix grammar error

* docs: fix broken link (#3142)

* Consolidates deadline tests for privval Unix/TCP (#3143)

* Consolidates deadline tests for privval Unix/TCP

Following on from #3115 and #3132, this converts fundamental timeout
errors from the client's `acceptConnection()` method so that these can
be detected by the test for the TCP connection.

Timeout deadlines are now tested for both TCP and Unix domain socket
connections.

There is also no need for the additional secret connection code: the
connection will time out at the `acceptConnection()` phase for TCP
connections, and will time out when attempting to obtain the
`RemoteSigner`'s public key for Unix domain socket connections.

* Removes extraneous logging

* Adds IsConnTimeout helper function

This commit adds a helper function to detect whether an error is either
a fundamental networking timeout error, or an `ErrConnTimeout` error
specific to the `RemoteSigner` class.

* Adds a test for the IsConnTimeout() helper function

* Separates tests logically for IsConnTimeout

* docs: fix RPC links (#3141)

* Bucky/fix evidence halt (#34)

* consensus: createProposalBlock function

* blockExecutor.CreateProposalBlock

- factored out of consensus pkg into a method on blockExec
- new private interfaces for mempool ("txNotifier") and evpool with one function each
- consensus tests still require more mempool methods

* failing test for CreateProposalBlock

* Fix bug in include evidece into block

* evidence: change maxBytes to maxSize

* MaxEvidencePerBlock

- changed to return both the max number and the max bytes
- preparation for #2590

* changelog

* fix linter

* Fix from review

Co-Authored-By: ebuchman <[email protected]>

* Add ParadigmCore to ecosystem.json (#3145)

Adding the OrderStream network client (alpha), ParadigmCore, to the `ecosystem.json` file.

* json2wal: increase reader's buffer size (#3147)

```
panic: failed to unmarshal json: unexpected end of JSON input

goroutine 1 [running]:
main.main()
	/root/gelgo/src/github.com/tendermint/tendermint/scripts/json2wal/main.go:66 +0x73f
```

Closes #3146

* changelog and version

* update btcd fork for v0.1.1 (#3164)

* update btcd fork for v0.1.1

* changelog

* Normalize priorities to not exceed total voting power (#3049)

* 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] hash of ConsensusParams includes only a subset of fields (#3165)

* types: dont hash entire ConsensusParams

* update encoding spec

* update blockchain spec

* spec: consensus params hash

* changelog

* mempool: enforce maxMsgSize limit in CheckTx (#3168)

- fixes #3008
- reactor requires encoded messages are less than maxMsgSize
- requires size of tx + amino-overhead to not exceed maxMsgSize

* fix DynamicVerifier for large validator set changes (#3171)

* 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

* update spec for Merkle RFC 6962 (#3175)

* spec: specify when MerkleRoot is on hashes

* remove unnecessary hash methods

* update changelog

* fix test

* Prepare v0.29.0 (#3184)

* update changelog and upgrading

* add note about max voting power in abci spec

* update version

* changelog

* fix changelog indent (#3190)

* p2p: file descriptor leaks (#3150)

* close peer's connection to avoid fd leak

Fixes #2967

* rename peer#Addr to RemoteAddr

* fix test

* fixes after Ethan's review

* bring back the check

* changelog entry

* write a test for switch#acceptRoutine

* increase timeouts? :(

* remove extra assertNPeersWithTimeout

* simplify test

* assert number of peers (just to be safe)

* Cleanup in OnStop

* run tests with verbose flag on CircleCI

* spawn a reading routine to prevent connection from closing

* get port from the listener

random port is faster, but often results in

```
panic: listen tcp 127.0.0.1:44068: bind: address already in use [recovered]
        panic: listen tcp 127.0.0.1:44068: bind: address already in use

goroutine 79 [running]:
testing.tRunner.func1(0xc0001bd600)
        /usr/local/go/src/testing/testing.go:792 +0x387
panic(0x974d20, 0xc0001b0500)
        /usr/local/go/src/runtime/panic.go:513 +0x1b9
github.com/tendermint/tendermint/p2p.MakeSwitch(0xc0000f42a0, 0x0, 0x9fb9cc, 0x9, 0x9fc346, 0xb, 0xb42128, 0x0, 0x0, 0x0, ...)
        /home/vagrant/go/src/github.com/tendermint/tendermint/p2p/test_util.go:182 +0xa28
github.com/tendermint/tendermint/p2p.MakeConnectedSwitches(0xc0000f42a0, 0x2, 0xb42128, 0xb41eb8, 0x4f1205, 0xc0001bed80, 0x4f16ed)
        /home/vagrant/go/src/github.com/tendermint/tendermint/p2p/test_util.go:75 +0xf9
github.com/tendermint/tendermint/p2p.MakeSwitchPair(0xbb8d20, 0xc0001bd600, 0xb42128, 0x2f7, 0x4f16c0)
        /home/vagrant/go/src/github.com/tendermint/tendermint/p2p/switch_test.go:94 +0x4c
github.com/tendermint/tendermint/p2p.TestSwitches(0xc0001bd600)
        /home/vagrant/go/src/github.com/tendermint/tendermint/p2p/switch_test.go:117 +0x58
testing.tRunner(0xc0001bd600, 0xb42038)
        /usr/local/go/src/testing/testing.go:827 +0xbf
created by testing.(*T).Run
        /usr/local/go/src/testing/testing.go:878 +0x353
exit status 2
FAIL    github.com/tendermint/tendermint/p2p    0.350s
```

* docs: explain how someone can run his/her own ABCI app on localnet (#3195)

Closes #3192.

* docs: update pubsub ADR (#3131)

* docs: update pubsub ADR

* third version

* docs: fix lite client formatting (#3198)

Closes #3180

* only log "Reached max attempts to dial" once (#3144)

Closes #3037

* [WIP] fix halting issue (#3197)

fix halting issue

* update changelog

* bump version

* fix changelog

* R4R: Config TestRoot modification for LCD test (#3177)

* add ResetTestRootWithChainID

* modify chainid

* adr: style fixes (#3206)

- links to issues
 - fix a few markdown glitches
 - inline code
 - etc

* add design philosophy doc (#3034)

* note about TmCoreSemVer

* types: comments on user vs internal events

Distinguish between user events and internal consensus events

* pubsub: comments

* adr-033 update

* start eventBus & indexerService before replay and use them while replaying blocks (#3194)

so if we did not index the last block (because of panic or smth else), we index it during replay

Closes #3186

* refactor TestListenerConnectDeadlines to avoid data races (#3201)

Fixes #3179

* add go-deadlock tool to help detect deadlocks (#3218)

* add go-deadlock tool to help detect deadlocks

Run it with `make test_with_deadlock`. After it's done, use Git to
cleanup `git checkout .`

Link: https://github.com/sasha-s/go-deadlock/
Replaces https://github.com/tendermint/tendermint/pull/3148

* add a target to cleanup changes

* alias amino imports (#3219)

As per conversation here: https://github.com/tendermint/tendermint/pull/3218#discussion_r251364041

This is the result of running the following code on the repo:

```bash
find . -name '*.go' | grep -v 'vendor/' | xargs -n 1 goimports -w
```

* docs: fix links (#3220)

Because there's nothing worse than having to copy/paste a link from a
web page to navigate to it :grin:

* hardcode rpc link (#3223)

* mempool: correct args order in the log msg (#3221)

Before: Unexpected tx response from proxy during recheck\n Expected: {r.CheckTx.Data}, got: {memTx.tx}
After: Unexpected tx response from proxy during recheck\n Expected: {memTx.tx}, got: {tx}

Closes #3214

* secret connection: check for low order points (#3040)

> Implement a check for the blacklisted low order points, ala the X25519 has_small_order() function in libsodium

(#3010 (comment))
resolves first half of #3010

* pubsub: fixes after Ethan's review (#3212)

in https://github.com/tendermint/tendermint/pull/3209

* update gometalinter to 3.0.0 (#3233)

in the attempt to fix https://circleci.com/gh/tendermint/tendermint/43165

also

    code is simplified by running gofmt -s .
    remove unused vars
    enable linters we're currently passing
    remove deprecated linters

* Use ethereum's secp256k1 lib (#3234)

* switch from fork (tendermint/btcd) to orig package (btcsuite/btcd); also

 - remove obsolete check in test `size != -1` is always true
 - WIP as the serialization still needs to be wrapped

* WIP: wrap signature & privkey, pubkey needs to be wrapped as well

* wrap pubkey too

* use "github.com/ethereum/go-ethereum/crypto/secp256k1" if cgo is
available, else use "github.com/btcsuite/btcd/btcec" and take care of
lower-S when verifying

Annoyingly, had to disable pruning when importing
github.com/ethereum/go-ethereum/ :-/

* update comment

* update comment

* emulate signature_nocgo.go for additional benchmarks:
https://github.com/ethereum/go-ethereum/blob/592bf6a59cac9697f0491b24e5093cb759d7e44c/crypto/signature_nocgo.go#L60-L76

* use our format (r || s) in lower-s form when in the non-cgo case

* remove comment about using the C library directly

* vendor github.com/btcsuite/btcd too

* Add test for the !cgo case

* update changelog pending

Closes #3162 #3163
Refs #1958, #2091, tendermint/btcd#1

* fix FlushStop (#3247)

* p2p/pex: failing test

* p2p/conn: add stopMtx for FlushStop and OnStop

* changelog

* WAL: better errors and new fail point (#3246)

* privval: more info in errors

* wal: change Debug logs to Info

* wal: log and return error on corrupted wal instead of panicing

* fail: Exit right away instead of sending interupt

* consensus: FAIL before handling our own vote

allows to replicate #3089:
- run using `FAIL_TEST_INDEX=0`
- delete some bytes from the end of the WAL
- start normally

Results in logs like:

```
I[2019-02-03|18:12:58.225] Searching for height module=consensus wal=/Users/ethanbuchman/.tendermint/data/cs.wal/wal height=1 min=0 max=0
E[2019-02-03|18:12:58.225] Error on catchup replay. Proceeding to start ConsensusState anyway module=consensus err="failed to read data: EOF"
I[2019-02-03|18:12:58.225] Started node module=main nodeInfo="{ProtocolVersion:{P2P:6 Block:9 App:1} ID_:35e87e93f2e31f305b65a5517fd2102331b56002 ListenAddr:tcp://0.0.0.0:26656 Network:test-chain-J8JvJH Version:0.29.1 Channels:4020212223303800 Moniker:Ethans-MacBook-Pro.local Other:{TxIndex:on RPCAddress:tcp://0.0.0.0:26657}}"
E[2019-02-03|18:12:58.226] Couldn't connect to any seeds module=p2p
I[2019-02-03|18:12:59.229] Timed out module=consensus dur=998.568ms height=1 round=0 step=RoundStepNewHeight
I[2019-02-03|18:12:59.230] enterNewRound(1/0). Current: 1/0/RoundStepNewHeight module=consensus height=1 round=0
I[2019-02-03|18:12:59.230] enterPropose(1/0). Current: 1/0/RoundStepNewRound module=consensus height=1 round=0
I[2019-02-03|18:12:59.230] enterPropose: Our turn to propose module=consensus height=1 round=0 proposer=AD278B7767B05D7FBEB76207024C650988FA77D5 privValidator="PrivValidator{AD278B7767B05D7FBEB76207024C650988FA77D5 LH:1, LR:0, LS:2}"
E[2019-02-03|18:12:59.230] enterPropose: Error signing proposal module=consensus height=1 round=0 err="Error signing proposal: Step regression at height 1 round 0. Got 1, last step 2"
I[2019-02-03|18:13:02.233] Timed out module=consensus dur=3s height=1 round=0 step=RoundStepPropose
I[2019-02-03|18:13:02.233] enterPrevote(1/0). Current: 1/0/RoundStepPropose module=consensus
I[2019-02-03|18:13:02.233] enterPrevote: ProposalBlock is nil module=consensus height=1 round=0
E[2019-02-03|18:13:02.234] Error signing vote module=consensus height=1 round=0 vote="Vote{0:AD278B7767B0 1/00/1(Prevote) 000000000000 000000000000 @ 2019-02-04T02:13:02.233897Z}" err="Error signing vote: Conflicting data"
```

Notice the EOF, the step regression, and the conflicting data.

* wal: change errors to be DataCorruptionError

* exit on corrupt WAL

* fix log

* fix new line

* Introduce CommitSig alias for Vote in Commit (#3245)

* 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

* Revert "quick fix for CircleCI (#2279)"

This reverts commit 1cf6712a36e8ecc843a68aa373748e89e0afecba.

* switch to golangci-lint from gometalinter :speedboat:

* remove or comment out unused code

* preallocating memory when we can

* comment out clist tests w/ depend on runtime.SetFinalizer

* use nolint label instead of commenting

* remove gotype linter

WARN [runner/nolint] Found unknown linters in //nolint directives: gotype

* rename TestGCRandom to _TestGCRandom

* cmn: GetFreePort (#3255)

* fix non deterministic test failures and race in privval socket (#3258)

* node: decrease retry conn timeout in test

Should fix #3256

The retry timeout was set to the default, which is the same as the
accept timeout, so it's no wonder this would fail. Here we decrease the
retry timeout so we can try many times before the accept timeout.

* p2p: increase handshake timeout in test

This fails sometimes, presumably because the handshake timeout is so low
(only 50ms). So increase it to 1s. Should fix #3187

* privval: fix race with ping. closes #3237

Pings happen in a go-routine and can happen concurrently with other
messages. Since we use a request/response protocol, we expect to send a
request and get back the corresponding response. But with pings
happening concurrently, this assumption could be violated. We were using
a mutex, but only a RWMutex, where the RLock was being held for sending
messages - this was to allow the underlying connection to be replaced if
it fails. Turns out we actually need to use a full lock (not just a read
lock) to prevent multiple requests from happening concurrently.

* node: fix test name. DelayedStop -> DelayedStart

* autofile: Wait() method

In the TestWALTruncate in consensus/wal_test.go we remove the WAL
directory at the end of the test. However the wal.Stop() does not
properly wait for the autofile group to finish shutting down. Hence it
was possible that the group's go-routine is still running when the
cleanup happens, which causes a panic since the directory disappeared.
Here we add a Wait() method to properly wait until the go-routine exits
so we can safely clean up. This fixes #2852.

* p2p/conn: don't hold stopMtx while waiting (#3254)

* p2p/conn: fix deadlock in FlushStop/OnStop

* makefile: set_with_deadlock

* close doneSendRoutine at end of sendRoutine

* conn: initialize channs in OnStart

* Add remote signer test harness (KMS) (#3149)

* WIP: Starts adding remote signer test harness

This commit adds a new command to Tendermint to allow for us to build a
standalone binary to test remote signers such as KMS
(https://github.com/tendermint/kms).

Right now, all it does is test that the local public key matches the
public key reported by the client, and fails at the point where it
attempts to get the client to sign a proposal.

* Fixes typo

* Fixes proposal validation test

This commit fixes the proposal validation test as per #3149. It also
moves the test harness into its own internal package to isolate its
exports from the `privval` package.

* Adds vote signing validation

* Applying recommendations from #3149

* Adds function descriptions for test harness

* Adds ability to ask remote signer to shut down

Prior to this commit, the remote signer needs to manually be shut down,
which is not ideal for automated testing. This commit allows us to send
a poison pill message to the KMS to let it shut down gracefully once
testing is done (whether the tests pass or fail).

* Adds tests for remote signer test harness

This commit makes some minor modifications to a few files to allow for
testing of the remote signer test harness. Two tests are added here:
checking for a fully successful (the ideal) case, and for the case where
the maximum number of retries has been reached when attempting to accept
incoming connections from the remote signer.

* Condenses serialization of proposals and votes using existing Tendermint functions

* Removes now-unnecessary amino import and codec

* Adds error message for vote signing failure

* Adds key extraction command for integration test

Took the code from here:
https://gist.github.com/Liamsi/a80993f24bff574bbfdbbfa9efa84bc7 to
create a simple utility command to extract a key from a local Tendermint
validator for use in KMS integration testing.

* Makes path expansion success non-compulsory

* Fixes segfault on SIGTERM

We need an additional variable to keep track of whether we're
successfully connected, otherwise hitting Ctrl+Break during execution
causes a segmentation fault. This now allows for a clean shutdown.

* Consolidates shutdown checks

* Adds comments indicating codes for easy lookup

* Adds Docker build for remote signer harness

Updates the `DOCKER/build.sh` and `DOCKER/push.sh` files to allow one to
override the image name and Dockerfile using environment variables.
Updates the primary `Makefile` as well as the `DOCKER/Makefile` to allow
for building the `remote_val_harness` Docker image.

* Adds build_remote_val_harness_docker_image to .PHONY

* Removes remote signer poison pill messaging functionality

* Reduces fluff code in command line parsing

As per
https://github.com/tendermint/tendermint/pull/3149#pullrequestreview-196171788,
this reduces the amount of fluff code in the PR down to the bare
minimum.

* Fixes ordering of error check and info log

* Moves remove_val_harness cmd into tools folder

It seems to make sense to rather keep the remote signer test harness in
its own tool folder (now rather named `tm-signer-harness` to keep with
the tool naming convention). It is actually a separate tool, not meant
to be one of the core binaries, but supplementary and supportive.

* Updates documentation for tm-signer-harness

* Refactors flag parsing to be more compact and less redundant

* Adds version sub-command help

* Removes extraneous flags parsing

* Adds CHANGELOG_PENDING entry for tm-signer-harness

* Improves test coverage

Adds a few extra parameters to the `MockPV` type to fake broken vote and
proposal signing. Also adds some more tests for the test harness so as
to increase coverage for failed cases.

* Fixes formatting for CHANGELOG_PENDING.md

* Fix formatting for documentation config

* Point users towards official Tendermint docs for tools documentation

* Point users towards official Tendermint docs for tm-signer-harness

* Remove extraneous constant

* Rename TestHarness.sc to TestHarness.spv for naming consistency

* Refactor to remove redundant goroutine

* Refactor conditional to cleaner switch statement and better error handling for listener protocol

* Remove extraneous goroutine

* Add note about installing tmkms via Cargo

* Fix typo in naming of output signing key

* Add note about where to find chain ID

* Replace /home/user with ~/ for brevity

* Fixes "signer.key" typo

* Minor edits for clarification for tm-signer-harness bulid/setup process

* p2p: fix infinite loop in addrbook (#3232)

* failing test

* fix infinite loop in addrbook

There are cases where we only have a small number of addresses marked
good ("old"), but the selection mechanism keeps trying to select more of these
addresses, and hence ends up in an infinite loop. Here we fix this to
only try and select such "old" addresses if we have enough of them. Note this
means, if we don't have enough of them, we may return more "new"
addresses than otherwise expected by the newSelectionBias.

This whole GetSelectionWithBias method probably needs to be rewritten,
but this is a quick fix for the issue.

* changelog

* fix infinite loop if not enough new addrs

* fix another potential infinite loop

if a.nNew == 0 -> pickFromOldBucket=true, but we don't have enough items
  (a.nOld > len(oldBucketToAddrsMap) false)

* Revert "fix another potential infinite loop"

This reverts commit 146540c1125597162bd89820d611f6531f5e5e4b.

* check num addresses instead of buckets, new test

* fixed the int division

* add slack to bias % in test, lint fixes

* Added checks for selection content in test

* test cleanup

* Apply suggestions from code review

Co-Authored-By: ebuchman <[email protected]>

* address review comments

* change after  Anton's review comments

* use the same docker image we use for testing

when building a binary for localnet

* switch back to circleci classic

* more review comments

* more review comments

* refactor addrbook_test

* build linux binary inside docker

in attempt to fix

```
--> Running dep
+ make build-linux
GOOS=linux GOARCH=amd64 make build
make[1]: Entering directory `/home/circleci/.go_workspace/src/github.com/tendermint/tendermint'
CGO_ENABLED=0 go build -ldflags "-X github.com/tendermint/tendermint/version.GitCommit=`git rev-parse --short=8 HEAD`" -tags 'tendermint' -o build/tendermint ./cmd/tendermint/
p2p/pex/addrbook.go:373:13: undefined: math.Round
```

* change dir from /usr to /go

* use concrete Go version for localnet binary

* check for nil addresses just to be sure

* addrbook_test: preallocate memory for bookSizes (#3268)

Fixes https://circleci.com/gh/tendermint/tendermint/44901

* prepare v0.29.2 (#3272)

* update changelog

* linkify

* bump version

* update main changelog

* final fixes

* entry for wal fix

* changelog preamble

* remove a line

* secp256k1: change build tags (#3277)

* remove MixEntropy (#3278)

* remove MixEntropy

* changelog

* review comment: cleaner constant for N/2, delete secp256k1N and use (#3279)

`secp256k1.S256().N` directly instead

* treat validator updates as set (#3222)

* 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

* Sec/bucky/35 commit duplicate evidence (#36)

Don't add committed evidence to evpool

* Reject blocks with committed evidence (#37)

* evidence: NewEvidencePool takes evidenceDB

* evidence: failing TestStoreCommitDuplicate

tendermint/security#35

* GetEvidence -> GetEvidenceInfo

* fix TestStoreCommitDuplicate

* comment in VerifyEvidence

* add check if evidence was already seen
 - modify EventPool interface (EventStore is not known in ApplyBlock):
   - add IsCommitted method to iface
 - add test

* update changelog

* fix TestStoreMark:
 - priority in evidence info gets reset to zero after evidence gets committed

* review comments: simplify EvidencePool.IsCommitted
 - delete obsolete EvidenceStore.IsCommitted

* add simple test for IsCommitted

* update changelog: this is actually breaking (PR number still missing)

* fix TestStoreMark:
 - priority in evidence info gets reset to zero after evidence gets
 committed

* review suggestion: simplify return

* types.NewCommit (#3275)

* types.NewCommit

* use types.NewCommit everywhere

* fix log in unsafe_reset

* memoize height and round in constructor

* notes about deprecating toVote

* bring back memoizeHeightRound

* Prepare v0.30.0 (#3287)

* changelog, upgrading, version

* update for evidence fixes

* linkify

* fix an entry

* fix failure in TestProposerFrequency (#3293)

```
--- FAIL: TestProposerFrequency (2.50s)

panic: empty validator set [recovered]

        panic: empty validator set



goroutine 91 [running]:

testing.tRunner.func1(0xc000a98c00)

        /usr/local/go/src/testing/testing.go:792 +0x6a7

panic(0xeae7e0, 0x11fbb30)

        /usr/local/go/src/runtime/panic.go:513 +0x1b9

github.com/tendermint/tendermint/types.(*ValidatorSet).RescalePriorities(0xc0000e7380, 0x0)

        /go/src/github.com/tendermint/tendermint/types/validator_set.go:106 +0x1ac

github.com/tendermint/tendermint/state.TestProposerFrequency(0xc000a98c00)

        /go/src/github.com/tendermint/tendermint/state/state_test.go:335 +0xb44

testing.tRunner(0xc000a98c00, 0x111a4d8)

        /usr/local/go/src/testing/testing.go:827 +0x163

created by testing.(*T).Run

        /usr/local/go/src/testing/testing.go:878 +0x65a

FAIL    github.com/tendermint/tendermint/state  3.139s
```

* make govet linter pass (#3292)

* make govet linter pass

Refs #3262

* close PipeReader and check for err

* make gosec linter pass (#3294)

* not related to linter: remove obsolete constants:
 - `Insecure` and `Secure` and type `Security` are not used anywhere

* not related to linter: update example

 - NewInsecure was deleted; change example to NewRemoteDB

* address: Binds to all network interfaces (gosec):

 - bind to localhost instead of 0.0.0.0
 - regenerate test key and cert for this purpose (was valid for ::) and
 otherwise we would see:
 transport: authentication handshake failed: x509: certificate is
 valid for ::, not 127.0.0.1\"

(used https://github.com/google/keytransparency/blob/master/scripts/gen_server_keys.sh
to regenerate certs)

* use sha256 in tests instead of md5; time difference is negligible

* nolint usage of math/rand in test and add comment on its import

 - crypto/rand is slower and we do not need sth more secure in tests

* enable linter in circle-ci

* another nolint math/rand in test

* replace another occurrence of md5

* consistent comment about importing math/rand

* test blockExec does not panic if all vals removed (#3241)

Fix for #3224
Also address #2084

* types: validator set update tests (#3284)

* types: validator set update tests

* docs: abci val updates must not include duplicates

* consensus: flush wal on stop (#3297)

Should fix #3295
Also partial fix of #3043

* update codeowners (#3305)

limit the scope of @zramsay because he's annoyed by notifications

* return an error on `show_validator` (#3316)

* return a command error prior to init

* add a pending log entry

* Update CHANGELOG_PENDING.md

Co-Authored-By: alexanderbez <[email protected]>

closes: #3314

* fix test-vectors to actually match the sign bytes: (#3312)

- sign bytes are length prefixed
 - change test to use SignBytes methods instead of calling amino methods
 directly

Resolves #3311
ref: #2455 #2508

* rpc/net_info: change RemoteIP type from net.IP to String (#3309)

* rpc/net_info: change RemoteIP type from net.IP to String

Before:
"AAAAAAAAAAAAAP//rB8ktw=="
which is amino-encoded net.IP byte slice

After:
"192.0.2.1"

Fixes #3251

* rpc/net_info: non-empty response in docs

* cs/wal: refuse to encode msg that is bigger than maxMsgSizeBytes (#3303)

Earlier this week somebody posted this in GoS Riot chat:

```
E[2019-02-12|10:38:37.596] Corrupted entry. Skipping... module=consensus wal=/home/gaia/.gaiad/data/cs.wal/wal err="DataCorruptionError[length 878916964 exceeded maximum possible value of 1048576 bytes]"
E[2019-02-12|10:38:37.596] Corrupted entry. Skipping... module=consensus wal=/home/gaia/.gaiad/data/cs.wal/wal err="DataCorruptionError[length 825701731 exceeded maximum possible value of 1048576 bytes]"
E[2019-02-12|10:38:37.596] Corrupted entry. Skipping... module=consensus wal=/home/gaia/.gaiad/data/cs.wal/wal err="DataCorruptionError[length 1631073634 exceeded maximum possible value of 1048576 bytes]"
E[2019-02-12|10:38:37.596] Corrupted entry. Skipping... module=consensus wal=/home/gaia/.gaiad/data/cs.wal/wal err="DataCorruptionError[length 912418148 exceeded maximum possible value of 1048576 bytes]"
E[2019-02-12|10:38:37.600] Corrupted entry. Skipping... module=consensus wal=/home/gaia/.gaiad/data/cs.wal/wal err="DataCorruptionError[failed to read data: EOF]"
E[2019-02-12|10:38:37.600] Error on catchup replay. Proceeding to start ConsensusState anyway module=consensus err="Cannot replay height 7242. WAL does not contain #ENDHEIGHT for 7241"
E[2019-02-12|10:38:37.861] Error dialing peer module=p2p err="dial tcp 35.183.126.181:26656: i/o timeout
```

Note the length error messages. What has happened is the length field got corrupted probably. I've looked at the code and noticed that we don't check the msg size during encoding. This PR fixes that. It also improves a few error messages in WALDecoder.

* cs: reset triggered timeout precommit (#3310)

* Reset TriggeredTimeoutPrecommit as part of updateToState

* Add failing test and fix

* fix DATA RACE in TestResetTimeoutPrecommitUponNewHeight

```
WARNING: DATA RACE
Read at 0x00c001691d28 by goroutine 691:
  github.com/tendermint/tendermint/consensus.decideProposal()
      /go/src/github.com/tendermint/tendermint/consensus/common_test.go:133 +0x121
  github.com/tendermint/tendermint/consensus.TestResetTimeoutPrecommitUponNewHeight()
      /go/src/github.com/tendermint/tendermint/consensus/state_test.go:1389 +0x958
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:827 +0x162

Previous write at 0x00c001691d28 by goroutine 931:
  github.com/tendermint/tendermint/consensus.(*ConsensusState).updateToState()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:562 +0x5b2
  github.com/tendermint/tendermint/consensus.(*ConsensusState).finalizeCommit()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:1340 +0x141e
  github.com/tendermint/tendermint/consensus.(*ConsensusState).tryFinalizeCommit()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:1255 +0x66e
  github.com/tendermint/tendermint/consensus.(*ConsensusState).enterCommit.func1()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:1201 +0x135
  github.com/tendermint/tendermint/consensus.(*ConsensusState).enterCommit()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:1232 +0x94b
  github.com/tendermint/tendermint/consensus.(*ConsensusState).addVote()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:1657 +0x132e
  github.com/tendermint/tendermint/consensus.(*ConsensusState).tryAddVote()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:1503 +0x8f
  github.com/tendermint/tendermint/consensus.(*ConsensusState).handleMsg()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:694 +0xa1e
  github.com/tendermint/tendermint/consensus.(*ConsensusState).receiveRoutine()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:642 +0x948
  github.com/tendermint/tendermint/consensus.(*ConsensusState).receiveRoutine()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:642 +0x948
  github.com/tendermint/tendermint/consensus.(*ConsensusState).receiveRoutine()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:642 +0x948
  github.com/tendermint/tendermint/consensus.(*ConsensusState).receiveRoutine()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:655 +0x7dd
  github.com/tendermint/tendermint/consensus.(*ConsensusState).receiveRoutine()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:642 +0x948
  github.com/tendermint/tendermint/consensus.(*ConsensusState).receiveRoutine()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:655 +0x7dd

Goroutine 691 (running) created at:
  testing.(*T).Run()
      /usr/local/go/src/testing/testing.go:878 +0x659
  testing.runTests.func1()
      /usr/local/go/src/testing/testing.go:1119 +0xa8
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:827 +0x162
  testing.runTests()
  testing.(*M).Run()
      /usr/local/go/src/testing/testing.go:1034 +0x2ee
  main.main()
      _testmain.go:216 +0x332
```

* fix another DATA RACE by locking consensus

```
WARNING: DATA RACE
Read at 0x00c009b835a8 by goroutine 871:
  github.com/tendermint/tendermint/consensus.(*ConsensusState).createProposalBlock()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:955 +0x7c
  github.com/tendermint/tendermint/consensus.decideProposal()
      /go/src/github.com/tendermint/tendermint/consensus/common_test.go:127 +0x53
  github.com/tendermint/tendermint/consensus.TestResetTimeoutPrecommitUponNewHeight()
      /go/src/github.com/tendermint/tendermint/consensus/state_test.go:1389 +0x958
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:827 +0x162

Previous write at 0x00c009b835a8 by goroutine 931:
  github.com/tendermint/tendermint/consensus.(*ConsensusState).updateHeight()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:446 +0xb7
  github.com/tendermint/tendermint/consensus.(*ConsensusState).updateToState()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:542 +0x22f
  github.com/tendermint/tendermint/consensus.(*ConsensusState).finalizeCommit()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:1340 +0x141e
  github.com/tendermint/tendermint/consensus.(*ConsensusState).tryFinalizeCommit()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:1255 +0x66e
  github.com/tendermint/tendermint/consensus.(*ConsensusState).enterCommit.func1()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:1201 +0x135
  github.com/tendermint/tendermint/consensus.(*ConsensusState).enterCommit()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:1232 +0x94b
  github.com/tendermint/tendermint/consensus.(*ConsensusState).addVote()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:1657 +0x132e
  github.com/tendermint/tendermint/consensus.(*ConsensusState).tryAddVote()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:1503 +0x8f
  github.com/tendermint/tendermint/consensus.(*ConsensusState).handleMsg()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:694 +0xa1e
  github.com/tendermint/tendermint/consensus.(*ConsensusState).receiveRoutine()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:642 +0x948
  github.com/tendermint/tendermint/consensus.(*ConsensusState).receiveRoutine()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:642 +0x948
  github.com/tendermint/tendermint/consensus.(*ConsensusState).receiveRoutine()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:642 +0x948
  github.com/tendermint/tendermint/consensus.(*ConsensusState).receiveRoutine()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:655 +0x7dd
  github.com/tendermint/tendermint/consensus.(*ConsensusState).receiveRoutine()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:642 +0x948
  github.com/tendermint/tendermint/consensus.(*ConsensusState).receiveRoutine()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:655 +0x7dd
```

* Fix failing test

* Delete profile.out

* fix data races

* improve ResetTestRootWithChainID() concurrency safety (#3291)

* improve ResetTestRootWithChainID() concurrency safety

Rely on ioutil.TempDir() to create test root directories and ensure
multiple same-chain id test cases can run in parallel.

* Update config/toml.go

Co-Authored-By: alessio <[email protected]>

* clean up test directories after completion

Closes: #1034

* Remove redundant EnsureDir call

* s/PanicSafety()/panic()/s

* Put create dir functionality back in ResetTestRootWithChainID

* Place test directories in OS's tempdir

In modern UNIX and UNIX-like systems /tmp is very often
mounted as tmpfs. This might speed test execution a bit.

* Set 0700 to a const

* rootsDirs -> configRootDirs

* Don't double remove directories

* Avoid global variables

* Fix consensus tests

* Reduce defer stack

* Address review comments

* Try to fix tests

* Update CHANGELOG_PENDING.md

Co-Authored-By: alessio <[email protected]>

* Update consensus/common_test.go

Co-Authored-By: alessio <[email protected]>

* Update consensus/common_test.go

Co-Authored-By: alessio <[email protected]>

* 3291 follow-up (#3323)

* changelog: use issue number instead of PR number

* follow up to #3291

- rpc/test/helpers.go add StopTendermint(node) func
- remove ensureDir(filepath.Dir(walFile), 0700)
- mempool/mempool_test.go add type cleanupFunc func()

* cmd/show_validator: wrap err to make it more clear

* p2p: check secret conn id matches dialed id (#3321)

ref: [#3010 (comment)](https://github.com/tendermint/tendermint/issues/3010#issuecomment-464287627)

> I tried searching for code where we authenticate a peer against its NetAddress.ID and couldn't find it. I don't see a reason to switch to Noise, but a need to ensure that the node's ID is authenticated e.g. after dialing from the address book.

* p2p: check secret conn id matches dialed id

* Fix all p2p tests & make code compile

* add simple test for dialing with wrong ID

* update changelog

* address review comments

* yet another place where to use IDAddressString and fix
testSetupMultiplexTransport

* update changelog and bump version

* cs: sync WAL more frequently (#3300)

As per #3043, this adds a ticker to sync the WAL every 2s while the WAL is running.

* Flush WAL every 2s

This adds a ticker that flushes the WAL every 2s while the WAL is
running. This is related to #3043.

* Fix spelling

* Increase timeout to 2mins for slower build environments

* Make WAL sync interval configurable

* Add TODO to replace testChan with more comprehensive testBus

* Remove extraneous debug statement

* Remove testChan in favour of using system time

As per
https://github.com/tendermint/tendermint/pull/3300#discussion_r255886586,
this removes the `testChan` WAL member and replaces the approach with a
system time-oriented one. In this new approach, we keep track of the
system time at which each flush and periodic flush successfully
occurred.

The naming of the various functions is also updated here to be more
consistent with "flushing" as opposed to "sync'ing".

* Update naming convention and ensure lock for timestamp update

* Add Flush method as part of WAL interface

Adds a `Flush` method as part of the WAL interface to enforce the idea
that we can manually trigger a WAL flush from outside of the WAL. This
is employed in the consensus state management to flush the WAL prior to
signing votes/proposals, as per https://github.com/tendermint/tendermint/issues/3043#issuecomment-453853630

* Update CHANGELOG_PENDING

* Remove mutex approach and replace with DI

The dependency injection approach to dealing with testing concerns could
allow similar effects to some kind of "testing bus"-based approach. This
commit introduces an example of this, where instead of relying on
(potentially fragile) timing of things between the code and the test, we
inject code into the function under test that can signal the test
through a channel.

This allows us to avoid the `time.Sleep()`-based approach previously
employed.

* Update comment on WAL flushing during vote signing

Co-Authored-By: thanethomson <[email protected]>

* Simplify flush interval definition

Co-Authored-By: thanethomson <[email protected]>

* Expand commentary on WAL disk flushing

Co-Authored-By: thanethomson <[email protected]>

* Add broken test to illustrate WAL sync test problem

Removes test-related state (dependency injection code) from the WAL data
structure and adds test code to illustrate the problem with using
`WALGenerateNBlocks` and `wal.SearchForEndHeight` to test periodic
sync'ing.

* Fix test error messages

* Use WAL group buffer size to check for flush

A function is added to `libs/autofile/group.go#Group` in order to return
the size of the buffered data (i.e. data that has not yet been flushed
to disk). The test now checks that, prior to a `time.Sleep`, the group
buffer has data in it. After the `time.Sleep` (during which time the
periodic flush should have been called), the buffer should be empty.

* Remove config root dir removal from #3291

* Add godoc for NewWAL mentioning periodic sync

* docs: fix rpc Tx() method docs (#3331)

* update changelog

* cs: update wal comments (#3334)

* cs: update wal comments

Follow-up to https://github.com/tendermint/tendermint/pull/3300

* Update consensus/wal.go

Co-Authored-By: melekes <[email protected]>

* refactor decideProposal in common_test (#3343)

* fix TestWALPeriodicSync (#3342)

The test was sometimes failing due to processFlushTicks being called too
early. The solution is to call wal#Start later in the test.

* pubsub 2.0 (#3227)

* green pubsub tests :OK:

* get rid of clientToQueryMap

* Subscribe and SubscribeUnbuffered

* start adapting other pkgs to new pubsub

* nope

* rename MsgAndTags to Message

* remove TagMap

it does not bring any additional benefits

* bring back EventSubscriber

* fix test

* fix data race in TestStartNextHeightCorrectly

```
Write at 0x00c0001c7418 by goroutine 796:
  github.com/tendermint/tendermint/consensus.TestStartNextHeightCorrectly()
      /go/src/github.com/tendermint/tendermint/consensus/state_test.go:1296 +0xad
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:827 +0x162

Previous read at 0x00c0001c7418 by goroutine 858:
  github.com/tendermint/tendermint/consensus.(*ConsensusState).addVote()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:1631 +0x1366
  github.com/tendermint/tendermint/consensus.(*ConsensusState).tryAddVote()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:1476 +0x8f
  github.com/tendermint/tendermint/consensus.(*ConsensusState).handleMsg()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:667 +0xa1e
  github.com/tendermint/tendermint/consensus.(*ConsensusState).receiveRoutine()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:628 +0x794

Goroutine 796 (running) created at:
  testing.(*T).Run()
      /usr/local/go/src/testing/testing.go:878 +0x659
  testing.runTests.func1()
      /usr/local/go/src/testing/testing.go:1119 +0xa8
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:827 +0x162
  testing.runTests()
      /usr/local/go/src/testing/testing.go:1117 +0x4ee
  testing.(*M).Run()
      /usr/local/go/src/testing/testing.go:1034 +0x2ee
  main.main()
      _testmain.go:214 +0x332

Goroutine 858 (running) created at:
  github.com/tendermint/tendermint/consensus.(*ConsensusState).startRoutines()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:334 +0x221
  github.com/tendermint/tendermint/consensus.startTestRound()
      /go/src/github.com/tendermint/tendermint/consensus/common_test.go:122 +0x63
  github.com/tendermint/tendermint/consensus.TestStateFullRound…
@melekes
Copy link
Contributor

melekes commented Apr 1, 2019

I don't know what can be done here except #3309. This is the response I'm getting - https://gist.github.com/melekes/00b1cdb76b3994b135b0eef39b05ebea. remote_ip is a string here ("remote_ip": "192.167.10.4").

so I guess maybe this issue is fixed, even if the encoding is still odd?

Have you tried vanilla Tendermint Core (not Cosmos SDK)? Maybe SDK adds some overlay on top of the Tendermint RPC...

@xla xla added the C:rpc Component: JSON RPC, gRPC label Apr 1, 2019
@xla xla changed the title Error using NetInfo in RPC client for Cosmos-SDK v0.30.0 rpc: Error using NetInfo in RPC client for Cosmos-SDK v0.30.0 Apr 1, 2019
@melekes
Copy link
Contributor

melekes commented Apr 12, 2019

Checked our HTTP client https://godoc.org/github.com/tendermint/tendermint/rpc/client response - all good 👍

@melekes
Copy link
Contributor

melekes commented Apr 12, 2019

@jacohend if you're still experiencing this issue, please get in touch either by dropping a msg here or on Riot. Thanks!

@melekes melekes closed this as completed Apr 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:rpc Component: JSON RPC, gRPC T:bug Type Bug (Confirmed)
Projects
None yet
Development

No branches or pull requests

5 participants