protocols/gossipsub: Close connection on inbound stream error#24
Closed
mxinden wants to merge 13 commits into
Closed
protocols/gossipsub: Close connection on inbound stream error#24mxinden wants to merge 13 commits into
mxinden wants to merge 13 commits into
Conversation
Author
|
As far as I can tell, one downside would be, that buffered outgoing messages would be dropped when closing the entire connection. |
Author
|
Thinking about this some more and talking to Pierre and Toralf: Instead of closing the connection on an error on the inbound substream (both on In case that is a valid solution, we could consider doing the same for the outbound stream, if not already done closing it, setting it to What do you think @AgeManning? |
* Create gossipsub crate - Basic template, borrowed from floodsub * Add a GossipsubConfig struct and set up basic structures in the Gossipsub struct * Begin implementation of join. Adds get_random_peers helper function and adds tests * Implements gossipsub leave() * Update publishMany to incorporate gossipsub mesh and fanout logic * Use the gossipsub mesh for determining peer subscription * Remove subscribed_topics field from the Gossipsub struct * Rename gossipsubconfig to ProtocolConfig * Implement the gossipsub control messages into the Codec's Encode/Decode and modifies GossipsubRpc * Modify GossipsubActions to enums for succinctness. * Modify the memcache to store Gossipsub messages * Implement control message handling. * Update control message handling to handle multiple messages. * Handle received gossipsub messages using pre-built handlers. * Remove excess connected peer hashmap * Add extra peer mapping and consistent topic naming. * Implement heartbeat, emit_gossip and send_graft_prune. * Group logic in forwarding messages. Add messages to memcache. * Add heartbeat timer and move location of helper function. * Add gossipsub the libp2p workspace, makes layer structs public * Add logging to gossipsub - Adds the log crate and implements logging macros - Specifies versions for external crates * Add example chat for debugging purposes * Implement libp2p#868 for gossipsub. * Add rust documentation to gossipsub crate. - Adds basic documentation, overview and examples to the gossipsub crate. * Re-introduce the initial heartbeat time config. This commit also adds the inject_connected test. * Add subscribe tests. - Modifies `handle_received_subscriptions` to take a reference of subscriptions - Adds `test_subscribe` - Adds `test_handle_received_subscriptions` - Adds tests for the filter in `get_random_peers` * Add Bug fixes and further testing for gossipsub. - Corrects the tuple use of topic_hashes - Corrects JOIN logic around fanout and adding peers to the mesh - Adds test_unsubscribe - Adds test_join * Rename GossipsubMessage::msg_id -> id * Add bug fix for handling disconnected peers. * Implements (partially) libp2p#889 for Gossipsub. * handle_iwant event count tests * handle_ihave event count tests * Move layer.rs tests into separate file. * Implement clippy suggestions for gossipsub. * Modify control message tests for specific types. * Implement builder pattern for GossipsubConfig. As suggested by @twittner - The builder pattern for building GossipsubConfig struct is implemented. * Package version updates as suggested by @twittner. * Correct line lengths in gossipsub. * Correct braces in found by @twittner. * Implement @twittner's suggestions. - Uses `HashSet` where applicable - Update `FnvHashMap` to standard `HashMap` - Uses `min` function in code simplification. * Add NodeList struct to clarify topic_peers. * Cleaner handling of messagelist Co-Authored-By: AgeManning <Age@AgeManning.com> * Cleaner handling of added peers. Co-Authored-By: AgeManning <Age@AgeManning.com> * handle_prune peer removed test * basic grafting tests * multiple topic grafting test * Convert &vec to slice. Co-Authored-By: AgeManning <Age@AgeManning.com> * Convert to lazy insert. Co-Authored-By: AgeManning <Age@AgeManning.com> * Cleaner topic handling. Co-Authored-By: AgeManning <Age@AgeManning.com> * control pool piggybacking using HashMap.drain() in control_pool_flush going to squash this * Add Debug derives to gossipsub and correct tests. * changes from PR squash this all tests passing, but still some that need to be reconsidered test reform * Implements Arc for GossipsubRpc events * Remove support for floodsub nodes * Reconnected to disconnected peers, to mitigate timeout * Use ReadOne WriteOne with configurable max gossip sizes * Remove length delimination from RPC encoding * Prevent peer duplication in mesh * Allow oneshot handler's inactivity_timeout to be configurable * Correct peer duplication in mesh bug * Remove auto-reconnect to allow for user-level disconnects * Single long-lived inbound/outbound streams to match go implementation * Allow gossipsub topics to be optionally hashable * Improves gossipsub stream handling - Corrects the handler's keep alive. - Correct the chat example. - Instantly add peers to the mesh on subscription if the mesh is low. * Allows message validation in gossipsub * Replaces Cuckoofilter with LRUCache The false positive rate was unacceptable for rejecting messages. * Renames configuration parameter and corrects logic * Removes peer from fanout on disconnection * Add publish and fanout tests * Apply @mxinden suggestions * Resend message if outbound stream negotiated - Downgrades log warnings * Implement further reviewer suggestions - Created associated functions to avoid unnecessary cloning - Messages are rejected if their sequence numbers are not u64 - `GossipsbuConfigBuilder` has the same defaults as `GossipsubConfig` - Miscellaneous typos * Add MessageId type and remove unnecessary comments * Add a return value to propagate_message function * Adds user-customised gossipsub message ids * Adds the message id to GossipsubEvent * Implement Debug for GossipsubConfig * protocols/gossipsub: Add basic smoke test Implement a basic smoke test that: 1. Builds a fully connected graph of size N. 2. Subscribes each node to the same topic. 3. Publishes a single message. 4. Waits for all nodes to receive the above message. N and the structure of the graph are reproducibly randomized via Quickcheck. * Corrections pointed out by @mxinden * Add option to remove source id publishing * protocols/gossipsub/tests/smoke: Remove unused variable * Merge latest master * protocols/gossipsub: Move to stable futures * examples/gossipsub-chat.rs: Move to stable futures * protocols/gossipsub/src/behaviour/tests: Update to stable futures * protocols/gossipsub/tests: Update to stable futures * protocols/gossipsub: Log substream errors * protocols/gossipsub: Log outbound substream errors * Remove rust-fmt formatting * Shift to prost for protobuf compiling * Use wasm_timer for wasm compatibility Co-authored-by: Grant Wuerker <gwuerker@gmail.com> Co-authored-by: Toralf Wittner <tw@dtex.org> Co-authored-by: Pawan Dhananjay <pawandhananjay@gmail.com> Co-authored-by: Max Inden <mail@max-inden.de> Co-authored-by: Pierre Krieger <pierre.krieger1708@gmail.com>
* Publish 0.15.0 * Oops, script too efficient
…o pass… (libp2p#1395) * Addressing libp2p#473 ... if I understood the ticket right, we want to pass through whatever the application provides as a topic identifier, leaving hashing (or not hashing) up to the application. * Remove TopicDescriptor and use Topic newtype everywhere * PR feedback Use From<Topic> instead of Into<String> Use impl Into<Topic> instead of Topic in public API Co-authored-by: Peat Bakke <peat@peat.org>
* Add pnet protocol copied from plaintext protocol, since that seems to be the closest match * Minimalize the pnet protocol * WIP private swarms with fixed key * Different nonces for write and read * Use per stream write buffer to avoid allocations * Add parsing and formating of PSKs * Directly call handshake Also remove unneeded InboundUpgrade and OutboundUpgrade * Add HandshakeError * Add dedicated pnet example * Add tests for PSK parsing and formatting * Some more tests for the parsing, fail case * Add fingerprint To be able to check if a go-ipfs and rust-libp2p use the same key without having to dump the actual key. Not sure if there is a spec for this anywhere, but it is basically just copied from go-ipfs. * Minimize dependencies and remove dead code * Rename PSK to PreSharedKey and use pin_project * Add crypt_writer Basically a stripped down and modified version of async_std BufWriter that also encrypts using the given cipher. * cargo fmt * Actually get rid of the Unpin requirement * Rewrite flushing and remove written count from state * Add docs for pnet/lib.rs * Increase library version * Remove pnet example There will be a more elaborate and useful example in a different PR * Return pending on pending... also make doc text less ambiguous * Add debug assertions to check invariants of poll_flush_buf Also, clarify the invariants in the comments of that method
* Add peer id inlining for small public keys * Apply @twittner suggestions * Make PeerId compare equal accross hashes * Fix mDNS * Remove useless functions * Add property test Co-authored-by: Age Manning <Age@AgeManning.com>
* Allow multiple pings over one connection This is so that pinging a rust-libp2p node from ipfs works even without passing -n 1 * Remove timeout
Signed-off-by: koushiro <koushiro.cqx@gmail.com> Co-authored-by: Pierre Krieger <pierre.krieger1708@gmail.com>
Co-authored-by: Pierre Krieger <pierre.krieger1708@gmail.com>
Instead of closing the connection on an error on the inbound substream (both on `poll_next` as well as `poll_close`), one can set it to None and depend on the remote to open an new one in case they have more data to send.
mxinden
force-pushed
the
inbound-close
branch
from
9656776 to
46da52a
Compare
Author
|
Replaced by libp2p#1424. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
To be consistent with the error handling used on the outbound connection
of a GossipSub ProtocolsHandler, this patch makes the handler close the
connection when failing to read from the inbound substream.
Without this rather drastic reaction one could end up with many
unidirectional connections only being able to send, but not receive on
them.
This is a follow up to the discussion here: #22 (comment).
What are your thoughts? I don't have a strong opinion but I think this is worth
further discussing.