server: Use local addr var in version handler. #1256
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This adds a test to ensure duplicate version messages are rejected. 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 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 server connection code to reject outbound peers that do not offer full node services.
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 exposes a new method named SetServices to the address manager which can be used to update the services for a known address. This will be useful to keep known services up to date. Backported from Decred.
This adds code to update the address manager services for a known address to the services advertised by peers when they are connected to via an outbound connection. It is only done for outbound connections to help prevent malicious behavior from inbound connections. Backported from Decred.
This modifies the OnVersion handler for server peers to use a local variable for the inbound status of the peer in order to avoid grabbing the mutex multiple times. While here, it also does some light cleanup. There are no functional changes. Backported from Decred.
This changes the server peers OnVersion handler to only advertise the server as a viable target for inbound connections when the server believes it is close the best known tip. Backported from Decred.
This modifies the OnVersion handler for server peers to use a local variable for the remote address of the peer in order to avoid grabbing the mutex multiple times. There are no functional changes. Backported from Decred.
davecgh
force-pushed
the
server_onversion_use_local_for_net_addr
branch
from
e0b9c2b
to
8c981e4
Compare
|
Rebased. |
Roasbeef
approved these changes
Oct 13, 2018
| return err | ||
| } | ||
| case <-time.After(negotiateTimeout): | ||
| p.Disconnect() |
There was a problem hiding this comment.
Nice! I think this might explain some issues I've seen where the connection count keeps rising (towards max connected), yet the number of stable peers queryable is much lower.
| } | ||
|
|
||
| // Update the services if needed. | ||
| if ka.na.Services != services { |
There was a problem hiding this comment.
FWIW, this isn't yet useful as we don't currently store the services on disk within the addrmgr. However, it does pave the way for us properly updating the services for a peer as they change overtime once we start to store the services on disk properly.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This requires #1255.
Backported from decred/dcrd#1258.
This modifies the
OnVersionhandler for server peers to use a local variable for the remote address of the peer in order to avoid grabbing the mutex multiple times.There are no functional changes.