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

[Networking] Refinement of message scope; encapsulating the publish logic in the libp2p node #4635

Merged
merged 37 commits into from
Aug 24, 2023

Conversation

yhassanzadeh13
Copy link
Contributor

@yhassanzadeh13 yhassanzadeh13 commented Aug 16, 2023

Introduction

This Pull Request (PR) is a continuation of a larger effort to refactor the networking layer structure within the Flow blockchain, specifically focusing on the removal of the middleware component. It aligns with the strategic approach outlined in Issue #6851.

Objective

The primary goal of this PR is to:

  1. Relocate the Publish Logic: Move the Publish logic of pubsub out of the middleware, placing it within the LibP2P Node, and encapsulating the logic entirely at the LibP2P Node level.
  2. Enhance the Publish Interface: Refactor the Publish interface of the libp2p node to a higher abstraction level that accepts a single outgoing message scope object, rather than a more tightly coupled interface.

Design Principles

The following software engineering principles guide the changes:

  • Encapsulation: By confining the Publish logic to the LibP2P Node level, we are ensuring that the details of the Publish process are hidden from other parts of the system. This promotes modularity and maintainability.
  • Abstraction: The introduction of a higher-level Publish interface that accepts a single outgoing message scope object simplifies interaction with the publishing system, offering a clearer and less complex method of operation.
  • Atomicity: Allowing the upper-level networking component to pass the entire message being published as a single atomic component down to the libp2p level ensures transaction integrity and reduces the risk of inconsistencies.
  • Cohesion: This refactoring strengthens the cohesion within the networking layer, ensuring that related functionalities are kept together, thus making the codebase more understandable and manageable.

Implementation Details

The specific changes are as follows:

  • The Publish logic of pubsub has been moved from the middleware to the LibP2P Node.
  • The existing Publish interface of the libp2p node has been restructured into a higher-level interface that accepts a single outgoing message scope object.

Benefits

The resulting abstraction provides a safer and more maintainable structure. By adhering to established software engineering principles, this PR contributes to the overall robustness and efficiency of the networking layer.

@codecov-commenter
Copy link

codecov-commenter commented Aug 16, 2023

Codecov Report

Merging #4635 (5618fa1) into master (68a75a4) will increase coverage by 0.62%.
The diff coverage is 4.12%.

@@            Coverage Diff             @@
##           master    #4635      +/-   ##
==========================================
+ Coverage   54.69%   55.31%   +0.62%     
==========================================
  Files         917      802     -115     
  Lines       85624    72852   -12772     
==========================================
- Hits        46833    40300    -6533     
+ Misses      35197    29243    -5954     
+ Partials     3594     3309     -285     
Flag Coverage Δ
unittests 55.31% <4.12%> (+0.62%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
cmd/scaffold.go 14.82% <0.00%> (-0.02%) ⬇️
network/p2p/middleware/middleware.go 1.82% <0.00%> (+0.11%) ⬆️
network/p2p/p2pbuilder/libp2pNodeBuilder.go 0.00% <ø> (ø)
network/p2p/test/fixtures.go 30.27% <0.00%> (-1.01%) ⬇️
network/p2p/p2pnode/libp2pNode.go 56.76% <10.52%> (-5.78%) ⬇️

... and 127 files with indirect coverage changes

@yhassanzadeh13 yhassanzadeh13 changed the title Yahya/6851 refactoring middleware [Networking] Refinement of message scope; encapsulating the publish logic in the libp2p node Aug 17, 2023
@yhassanzadeh13 yhassanzadeh13 marked this pull request as ready for review August 18, 2023 18:57
@yhassanzadeh13 yhassanzadeh13 requested review from kc1116 and removed request for peterargue August 18, 2023 19:22
Copy link
Contributor

@gomisha gomisha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Please address comments / suggestions before merging.

yhassanzadeh13 and others added 3 commits August 24, 2023 08:44
@yhassanzadeh13 yhassanzadeh13 added this pull request to the merge queue Aug 24, 2023
Merged via the queue into master with commit a8f28b9 Aug 24, 2023
@yhassanzadeh13 yhassanzadeh13 deleted the yahya/6851-refactoring-middleware branch August 24, 2023 18:28
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.

4 participants