A prior change to address accidental DNS lookups introduced the
SocketAddr on peer, which was then used to add it to the addressbook.
Which in turn swallowed the self reported port of the peer, which is
important on a reconnect. This change revives the NetAddress on NodeInfo
which the Peer carries, but now returns an error to avoid nil
dereferencing another issue observed in the past. Additionally we could
potentially address #3532, yet the original problem statemenf of that
issue stands.
As a drive-by optimisation `MarkAsGood` now takes only a `p2p.ID` which
makes it interface a bit stricter and leaner.
We are swapping the exisiting listener implementation with the newly
introduced Transport and its default implementation MultiplexTransport,
removing a large chunk of old connection setup and handling scattered
over the Peer and Switch code. The Switch requires a Transport now and
handles externally passed Peer filters.
This is the implementation for the design described in ADR 12[0]. It's
the first step of a larger refactor of the p2p package as tracked in
interface bundling all concerns of low-level connection handling and
isolating the rest of peer lifecycle management from the specifics of
the low-level internet protocols. Even if the swappable implementation
will never be utilised, already the isolation of conn related code in
one place will help with the reasoning about execution path and
addressation of security sensitive issues surfaced through bounty
programs and audits.
We deliberately decided to not have Peer filtering and other management
in the Transport, its sole responsibility is the translation of
connections to Peers, handing those to the caller fully setup. It's the
responsibility of the caller to reject those and or keep track. Peer
filtering will take place in the Switch and can be inspected in a the
following commit.
This changeset additionally is an exercise in clean separation of logic
and other infrastructural concerns like logging and instrumentation. By
leveraging a clean and minimal interface. How this looks can be seen in
a follow-up change.
Design #2069[2]
Refs #2067[3]
Fixes #2047[4]
Fixes #2046[5]
changes:
* describe Transport interface
* implement new default Transport: MultiplexTransport
* test MultiplexTransport with new constraints
* implement ConnSet for concurrent management of net.Conn, synchronous
to PeerSet
* implement and expose duplicate IP filter
* implemnt TransportOption for optional parametirisation
[0] https://github.com/tendermint/tendermint/blob/master/docs/architecture/adr-012-peer-transport.md
[1] https://github.com/tendermint/tendermint/issues/2067
[2] https://github.com/tendermint/tendermint/pull/2069
[3] https://github.com/tendermint/tendermint/issues/2067
[4] https://github.com/tendermint/tendermint/issues/2047
[5] https://github.com/tendermint/tendermint/issues/2046
* p2p: Propose PeerTransport ADR
* adr: Set status to in review
* adr: Add high-level decision
* adr: Extend on the idea of guards
* adr: Rework guards into transport specific filters
* adr: Rename to nodeAddr
* adr: Incorporate review
Instead of mutating the passed in MConnConfig part of P2PConfig we just
use the default and override the values, the same as before as it was
always the default version. This is yet another good reason to not embed
information and access to config structs in our components and will go
away with the ongoing refactoring in #1325.
As both configs are concerned with the p2p packaage and PeerConfig is
only used inside of the package there is no good reason to keep the
couple of fields separate, therefore it is collapsed into the more
general P2PConifg. This is a stepping stone towards a setup where the
components inside of p2p do not have any knowledge about the config.
follow-up to #1325
This is a maintenance change to move the private validator package out
of the types and to a top-level location. There is no good reason to
keep it under the types and it will more clearly coommunicate where
additions related to the privval belong. It leaves the interface and the
mock in types for now as it would introduce circular dependency between
privval and types, this should be resolved eventually.
* mv priv_validator to privval pkg
* use consistent `privval` as import
Follow-up to #1255
As we didn't hear any voices requesting this feature, we removed the
option to disable it and always have peer connection auth encrypted.
closes#1518
follow-up #1325
We can make the implementation more robust by adjusting our assumptions
and leverage explicit file modes for syncing. Additionally we going to
assume that we want to clean up and can't really recover if thos
operations (file close and removal) fail.
* utilise file mode for majority of concerns
* improve test coverage by covering more assumptions
* signature parity with ioutil.WriteFile
* always clean up
Replaces #160
We can make the implementation more robust by adjusting our assumptions
and leverage explicit file modes for syncing. Additionally we going to
assume that we want to clean up and can't really recover if thos
operations (file close and removal) fail.
* utilise file mode for majority of concerns
* improve test coverage by covering more assumptions
* signature parity with ioutil.WriteFile
* always clean up
Replaces #160
Follow-up to feedback from #1286, this change simplifies the connection
handling in the SocketClient and makes the communication via TCP more
robust. It introduces the tcpTimeoutListener to encapsulate accept and
i/o timeout handling as well as connection keep-alive, this type could
likely be upgraded to handle more fine-grained tuning of the tcp stack
(linger, nodelay, etc.) according to the properties we desire. The same
methods should be applied to the RemoteSigner which will be overhauled
when the priv_val_server is fleshed out.
* require private key
* simplify connect logic
* break out conn upgrades to tcpTimeoutListener
* extend test coverage and simplify component setup
Follow-up to #1255 aligning with the expectation that the external
signing process connects to the node. The SocketClient will block on
start until one connection has been established, support for multiple
signers connected simultaneously is a planned future extension.
* SocketClient accepts connection
* PrivValSocketServer renamed to RemoteSigner
* extend tests
To achieve faster feedback cycles for our feature PRs this change
reduces the average buildtime from 35 to ~6min by utilising their new
2.0 offering based on docker and nomad. We make use of parallel build
steps wherever possible so that the duration is determined by the
slowest test suite (p2p).
This is an intermediate step until we move our CI/CD completely
on-premise for more control and added security.
In order to improve the operator experience we want the node to dial
seeds immediately if there are no peers to connect to. Until now the
routine responsible for ensuring peers are connected to would wait
a random amount of time up to 30s (if not configured otherwise).
Following ADDR 008 the node will connect to an external
process to handle signing requests. Operation of the external process is
left to the user.
* introduce alias for PrivValidator interface on socket client
* integrate socket client in node
* structure tests
* remove unnecessary flag
Alexander SimmerlandGitHub
