Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Commit

Permalink
Problem: AttachedProtocols don't get registered (#7610)
Browse files Browse the repository at this point in the history
I was investigating issues I am having with Whisper support. I've
enabled Whisper on a custom test network and inserted traces into
Whisper handler implementation (Network<T> and NetworkProtocolHandler
for Network<T>) and I noticed that the handler was never invoked.

After further research on this matter, I found out that
AttachedProtocol's register function does nothing:
https://github.com/paritytech/parity/blob/master/sync/src/api.rs#L172
but there was an implementation originally:
99075ad#diff-5212acb6bcea60e9804ba7b50f6fe6ec and it did the actual
expected logic of registering the protocol in the NetworkService.

However, as of 16d84f8#diff-5212acb6bcea60e9804ba7b50f6fe6ec ("finished
removing ipc") this implementation is gone and only the no-op function
is left.

Which leads me to a conclusion that in fact Whisper's handler never gets
registered in the service and therefore two nodes won't communicate
using it.

Solution: Resurrect original non-empty `AttachedProtocols.register`
implementation

Resolves #7566
  • Loading branch information
yrashk authored and debris committed Jan 22, 2018
1 parent 261fd57 commit 470bd6c
Showing 1 changed file with 12 additions and 1 deletion.
13 changes: 12 additions & 1 deletion sync/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,18 @@ pub struct AttachedProtocol {
}

impl AttachedProtocol {
fn register(&self, _network: &NetworkService) {}
fn register(&self, network: &NetworkService) {
let res = network.register_protocol(
self.handler.clone(),
self.protocol_id,
self.packet_count,
self.versions
);

if let Err(e) = res {
warn!(target: "sync", "Error attaching protocol {:?}: {:?}", self.protocol_id, e);
}
}
}

/// EthSync initialization parameters.
Expand Down

0 comments on commit 470bd6c

Please sign in to comment.