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

Peer hashcode fix #194

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

pyckle
Copy link
Collaborator

@pyckle pyckle commented Oct 18, 2021

Separate Peer into InetPeer (for remote peer), LocalPeer (for the local peer information, and ImmutablePeer for lists of peers that are obtained or sent via peer sharing mechanisms (tracker, PEx, etc.)

InetPeer is not suitable for using as a Map key because it is mutable, including the remote port. The remote port for incoming connections is not known until the extended handshake is completed.

Many extension modules implemented must store state on a per peer basis - it is required by many extension specifications. However the implementations that use Map<Peer, State> are clumsy. These maps must be cleaned whenever a peer disconnects.

This has been solved by adding an additional state map in the ConnectionState object that takes a state class and returns its instance for the peer.

Also, many modules have a common check they need to do - check whether the module is supported by the remote peer. The state of supported modules is now stored in the Peer object, to eliminate the need for all of the modules to consume the ExtendedHandshake message. Also, removing support for protocols is now supported.

PEx now shares additional information about peers. It shares whether they are seeding, whether they are using encryption, as well as whether our connection to them is outgoing or incoming. uTP and hole punching are still not implemented.

@codecov-commenter
Copy link

codecov-commenter commented Oct 18, 2021

Codecov Report

Merging #194 (21527b1) into master (7dcb93b) will increase coverage by 2.33%.
The diff coverage is 70.47%.

❗ Current head 21527b1 differs from pull request most recent head d6c7f0b. Consider uploading reports for the commit d6c7f0b to get more accurate results
Impacted file tree graph

@@             Coverage Diff              @@
##             master     #194      +/-   ##
============================================
+ Coverage     65.72%   68.05%   +2.33%     
- Complexity     2378     2512     +134     
============================================
  Files           309      313       +4     
  Lines         10339    10465     +126     
  Branches       1144     1143       -1     
============================================
+ Hits           6795     7122     +327     
+ Misses         2871     2647     -224     
- Partials        673      696      +23     
Impacted Files Coverage Δ
bt-core/src/main/java/bt/event/EventSource.java 0.00% <0.00%> (ø)
...c/main/java/bt/event/PeerBitfieldUpdatedEvent.java 0.00% <ø> (ø)
...ore/src/main/java/bt/event/PeerConnectedEvent.java 50.00% <ø> (ø)
.../src/main/java/bt/event/PeerDisconnectedEvent.java 50.00% <ø> (ø)
bt-core/src/main/java/bt/magnet/UtMetadata.java 69.23% <ø> (ø)
...main/java/bt/net/crypto/MSEHandshakeProcessor.java 67.27% <ø> (+1.10%) ⬆️
...n/java/bt/net/pipeline/ChannelPipelineFactory.java 100.00% <ø> (ø)
...n/java/bt/net/pipeline/DefaultChannelPipeline.java 85.71% <ø> (ø)
.../java/bt/net/pipeline/InboundMessageProcessor.java 77.48% <ø> (ø)
...c/main/java/bt/net/pipeline/MessageSerializer.java 100.00% <ø> (ø)
... and 94 more

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants