* 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
* 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
* 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:
592bf6a59c/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
* 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
```
* base verifier: bc->bv and check chainid
* improve some comments
* comments in dynamic verifier
* fix comment in doc about BaseVerifier
It requires the validator set to perfectly match.
* failing test for #2862
* move errTooMuchChange to types. fixes#2862
* changelog, comments
* ic -> dv
* update comment, link to issue
* more proposer priority tests
- test that we don't reset to zero when updating / adding
- test that same power validators alternate
* add another test to track / simulate similar behaviour as in #2960
* address some of Chris' review comments
* address some more of Chris' review comments
* temporarily pushing branch with the following changes:
The total power might change if:
- a validator is added
- a validator is removed
- a validator is updated
Decrement the accums (of all validators) directly after any of these events
(by the inverse of the change)
* Fix 2960 by re-normalizing / scaling priorities to be in bounds of total
power, additionally:
- remove heap where it doesn't make sense
- avg. only at the end of IncrementProposerPriority instead of each
iteration
- update (and slightly improve)
TestAveragingInIncrementProposerPriorityWithVotingPower to reflect
above changes
* Fix 2960 by re-normalizing / scaling priorities to be in bounds of total
power, additionally:
- remove heap where it doesn't make sense
- avg. only at the end of IncrementProposerPriority instead of each
iteration
- update (and slightly improve)
TestAveragingInIncrementProposerPriorityWithVotingPower to reflect
above changes
* fix tests
* add comment
* update changelog pending & some minor changes
* comment about division will floor the result & fix typo
* Update TestLargeGenesisValidator:
- remove TODO and increase large genesis validator's voting power
accordingly
* move changelog entry to P2P Protocol
* Ceil instead of flooring when dividing & update test
* quickly fix failing TestProposerPriorityDoesNotGetResetToZero:
- divide by Ceil((maxPriority - minPriority) / 2*totalVotingPower)
* fix typo: rename getValWitMostPriority -> getValWithMostPriority
* test proposer frequencies
* return absolute value for diff. keep testing
* use for loop for div
* cleanup, more tests
* spellcheck
* get rid of using floats: manually ceil where necessary
* Remove float, simplify, fix tests to match chris's proof (#3157)
* 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 <ethan@coinculture.info>
* 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
* 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
* 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
* 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
* 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 <quadrispro@ubuntu.com>
* 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
* 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
* 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
* 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 24c1094ebc.
* 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
(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().
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.
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
* types: ValidatorSet.Update preserves ProposerPriority
This solves the other issue discovered as part of #2718,
where Accum (now called ProposerPriority) is reset to
0 every time a validator is updated.
* update changelog
* add test
* update comment
* Update types/validator_set_test.go
Co-Authored-By: ebuchman <ethan@coinculture.info>
Thane Thomson
Ethan Buchman
Ismail Khoffi
Anton Kaliaev
cong
Dev Ojha
Gian Felipe
Alessio Treglia
yutianwu
Zach
Jae Kwon
JamesRay
mircea-c
Daniil Lashin