Mempool: remove disconnected orphans #1152
Conversation
iljakuklic
requested review from
alfiedotwtf,
stanislav-tkach,
OBorce,
TheQuantumPhysicist and
ImplOfAnImpl
| /// Handle a p2p event. | ||
| // TODO This is here because of awkward inter-crate dependencies. A nicer solution would be to | ||
| // factor out subsystem interface traits into separate crates and subscribe to p2p events | ||
| // together with the other mempool setup code | ||
| fn handle_p2p_event(&mut self, evt: p2p_types::p2p_event::P2pEvent); |
There was a problem hiding this comment.
Not very happy about exposing this via a public interface. See the PR description for details.
iljakuklic
force-pushed
the
mempool-remove-disconnected-orphans
branch
from
5063911
to
4a4f557
Compare
This has now been changed to a subsystem call rather than event subscribtion. The issue that the event handler for processing peer disconnect events by mempool is now a part of the public API still remains.
Abandoning the approach mentioned here for now. |
iljakuklic
force-pushed
the
mempool-remove-disconnected-orphans
branch
from
4a4f557
to
f3b06b1
Compare
Remove orphan transactions sent by a peer when the peer disconnects.
I am not satisfied with the way the events are wired up. Since p2p already listens for mempool events (orphans being processed) and now mempool listens to p2p events, there is a bit of circularity. Usually, the subsystem subscribes to events it wants to listen to during its setup. However in this case, p2p setup code subscribes mempool to p2p events. Another unfortunate side effect of this is that mempool handler that processes p2p events is now exposed via public API.
A follow up PR that addresses these issues is in the works. It involves separating p2p interface and mempool interface into their own crates. As a side effect, we get a better interface / implementation separation for these two subsystems at crate level.Came up with an alternative solution. Still suffers from the issue of exposing event handlers via a public interface but good enough for now.