Add a recoverFromPanic method that catches panics in peer goroutines,
logs the error with a stack trace, and disconnects the offending peer
instead of crashing the entire node. This provides defense-in-depth
against future unknown panics in message parsing or handling.
Protect two goroutine boundaries:
- The negotiation goroutine in start(), which handles pre-handshake
message decoding via BtcDecode.
- The inHandler goroutine, which processes all post-handshake messages.
Refactor inHandler cleanup from manual end-of-function calls to
defer-based cleanup, ensuring close(p.inQuit) always runs even on
panic, which prevents stallHandler from hanging.
Address review feedback on the peer add/done race fix:
- Make peerLifecycleHandler (renamed from peerDoneHandler) the sole
sender of both peerAdd and peerDone events for each peer. OnVerAck
now closes a signal channel (verAckCh) instead of sending directly,
and peerLifecycleHandler selects on verAckCh vs peer.Done() to
decide whether to send peerAdd before peerDone. This guarantees
ordering by construction: a single goroutine sends both events
sequentially, eliminating the negotiateTimeout race window.
- Add Done() method to peer.Peer exposing the quit channel read-only,
enabling select-based disconnect detection from server code.
- Remove the now-unused AddPeer method.
- Address style feedback: 80-char line limit, empty lines between
switch cases, break long function calls, use require.GreaterOrEqualf
instead of if+Fatalf, bump syncRaceConcurrency to 300 for
backpressure testing, add TestPreVerackDisconnect for disconnect
prior to verack.
In this commit, we reduce the size of the v2transport module by shedding
the dep on the wire package. This allows us to only depend on btcec/v2,
which itself is a sub module, instead of the entire btcd module.
In this commit, we refactor the v2 -> v1 downgrade logic to: simplify
the code in the server for reconnection, use a functional option to
avoid breaking the connmgr API, and we also encapsulate the downgrade
state in a new struct so it can be re-used elsewhere.
This commit adds a new boolean field, `V2Connection`, to the
`getpeerinfo` RPC result. This field indicates whether the connection
to the peer is using the v2 encrypted transport protocol.
The `peer.StatsSnap` struct is updated to include this `V2Connection`
field, which is populated based on the `UsingV2Conn` field in the peer's
configuration. The `btcjson.GetPeerInfoResult` struct is also updated
to include the corresponding JSON field `v2_connection`. Finally, the
`handleGetPeerInfo` RPC handler is modified to copy this value from the
peer's stats snapshot into the RPC response.
When ShouldDowngradeToV1() returns true, we'll:
- mark in pendingReconnects that we should attempt v1 bitcoin p2p transport
when the connmgr successfully re-establishes the outbound TCP connection.
- set a bit on the associated *serverPeer that will cause us to tell the
connmgr to reconnect. This is needed because not all outbound peers are
permanent and we might otherwise not reconnect to them if the initial v2
transport negotiation failed.
IBD for new nodes were broken due to the version handshake failing
between nodes that recognized wtxid based relays. Reverting the changes
that were made so that the node is able to connect to those nodes.
* Add support for wtxidrelay message.
This adds support for the wtxidrelay (BIP339) message in wire.
While here, add tests for sendaddrv2 and rename AddrV2Version to
SendAddrV2Version in order to make the code consistent with all other
messages. This diff does keep the old AddrV2Version constant for
backwards compatibility.
* Bump copyrights
The doc formatting changes introduced in the recent go version is
increasing the diff for all of the new commits. Formatting it all in
this commit will help the readability of future PRs by reducing the
diff.
In this commit, we add a new config options that allows one to start
`btcd` in an operating mode that disables the stall detection. This can
be useful in simnet/regtest integration tests settings where it's
important that `btcd` holds on to its possibly sole connection to the
only other node in the test harness.
A new config flag has been added to gate this behavior, which is off by
default.
The previous use of allowSelfConns prevented this, as users aren't able
to invoke peer.TstAllowSelfConns themselves due to being part of a test
file, which aren't exported at the library level, leading to a
"disconnecting peer connected to self" error upon establishing a mock
connection between two peers. By including the option at the config
level instead (false by default, prevents connections to self) we enable
users of the peer library to properly test the behavior of the peer.Peer
struct externally.
This modifies the UpdateLastBlockHeight function to ensure the new
height is after the existing height before updating it in order to
prevent it from going backwards so it properly matches the intent of the
function which is to report the latest known block height for the peer.
Without this change, the value will properly start out at the latest
known block height reported by the peer during version negotiation,
however, it will be set to lower values when syncing from the peer due
to requesting old blocks and blindly updating the height.
It also adds a test to ensure proper functionality.
This is a backport of https://github.com/decred/dcrd/pull/1747
It was possible for connections to bitcoind nodes to be closed from
their side if the OnVersion callback queued a message to send. This is
because bitcoind expects a verack message before attempting to process
any other messages. To address this, we ensure the verack is sent as
part of the handshake process, such that any queued messages happen
after the fact.
This modifies the peer code which deals with advertising service flags
via the net address fields of the version message as follows:
- For outgoing connections:
- Set the local netaddress services to what the local peer supports
- Set the remote netaddress services to 0 to indicate no services as
they are still unknown
- For incoming connections:
- Set the local netaddress services to what the local peer supports
- Set the remote netaddress services to the what was advertised by the
remote peer in its version message
This modifies the OnVersion callback to allow a reject message to be
returned in which case the message will be sent to the peer and the peer
will be disconnected.
Backported from Decred.
This modifies the negotiation logic to ensure the callback has the
opportunity to see the message before the peer is disconnected and
improves the error handling when reading the remote version message.
It also has the side effect of ensuring the protocol version is
negotiated before sending reject messages with the exception of the
first message not being a version message since negotiation is not
possible in that case.
This is being changed because it is useful for the server to see the
message regardless in order to have the opportunity to things such as
update the address manager and reject peers that don't have desired
services.
Backported from Decred.
This rearranges some of the function definitions that pertain to initial
peer version negotiation and bringup so they are more consistent with
the preferred order used throughout the codebase. In particular, the
functions are defined before they're first used and generally as close
as possible to the first use when they're defined in the same file.
There are no functional changes.
Backported from Decred.
In this commit, we patch a goroutine leak within the peer struct. This
goroutine leak can happen, if the remote side fails to actually finish
negotiation the protocol before our timeout ticker ticks. In this case,
the goroutine will be blocked on a send, as the channel is unfired. An
example trace from a btcd node I had on testnet showed:
```
3183 @ 0x42e78a 0x42e83e 0x40540b 0x4051a5 0x872f76 0x45bfc1
```
So, all instances of the goroutine failing to exit due to the remote
peer not finishing the p2p version negotiation handshake.
Our fix is simple: make the `negotiateErr' channel unbuffered. With this
simple change, we ensure that the goroutine will always exit even in the
case that the parent goroutine exists due to a timeout. # Please enter
the commit message for your changes. Lines starting
The cfilter BIP specifies that the filter type is a uint8. The
current code encodes it correctly on the wire, but everywhere else,
it's treated as a boolean (false for basic filter, true for
extended). This commit corrects that to account for possible
additional filter types in the future. All package changes are
done in one commit as they're all interdependent. The following
packages are updated:
* blockchain/indexers
* btcjson
* peer
* wire
* main (server.go and rpcserver.go)
