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

Integration with peer2peer networking #3363

Merged
merged 19 commits into from
Nov 16, 2021
Merged

Integration with peer2peer networking #3363

merged 19 commits into from
Nov 16, 2021

Conversation

coot
Copy link
Contributor

@coot coot commented Nov 12, 2021

This PR integrates with peer2peer networking and allows to run a node in either mode:

  • non-p2p: the only officially supported version
  • p2p: unverified and unsupported p2p mode

This also includes:

  • a new topology file format for p2p nodes
  • a mechanism to reload topology file on SIGHUP signal
  • network trace instances: use ToJSON or ToObject instances
  • network trace instances: peer selection traces
  • network trace instances: ToJSON instances
  • consensus trace instances: trace termination reason
  • P2P / NonP2P cardano-node
  • Update p2p topology configuration via the SIGHUP signal
  • Removed MonoLocalBinds extension
  • topology file: improved parsing error message
  • Improve NetworkTopology generator
  • byron-mainnet configuration: added p2p trace options
  • p2p topology file for cardano mainnet
  • Document the p2p topology file
  • Updated cardano-node-chairman
  • Updated trace-dispatcher and trace-forward libraries
  • Updated tx-generator
  • Updated cardano-testnet package
  • Code cleanup in Tracing.Config
  • nixos service: add p2p topology support.
  • nixos-service: Add space-cost profiling support
  • nixos-service: Add space-heap profiling support

@coot
Copy link
Contributor Author

coot commented Nov 12, 2021

Although p2p-master in ouroboros-network hasn't been merged yet (and thus the git dependencies check failure), this is ready for review.

Copy link
Contributor

@karknu karknu left a comment

Choose a reason for hiding this comment

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

LGTM.
Just some minor comments.

cardano-node/src/Cardano/Tracing/Tracers.hs Show resolved Hide resolved
cardano-node/src/Cardano/Tracing/Tracers.hs Show resolved Hide resolved
cardano-node/src/Cardano/Tracing/Tracers.hs Show resolved Hide resolved
cardano-node/src/Cardano/Tracing/Tracers.hs Show resolved Hide resolved
@coot coot force-pushed the p2p-master branch 2 times, most recently from c1e8140 to 8f80f31 Compare November 15, 2021 10:29
Copy link
Contributor

@newhoggy newhoggy left a comment

Choose a reason for hiding this comment

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

Approved with comments.

@coot coot force-pushed the p2p-master branch 2 times, most recently from 719b19c to b568136 Compare November 15, 2021 15:50
Copy link
Contributor

@Jimbo4350 Jimbo4350 left a comment

Choose a reason for hiding this comment

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

Looking good but a few comments

cabal.project Outdated Show resolved Hide resolved
cardano-node/src/Cardano/Node/Run.hs Outdated Show resolved Hide resolved
cardano-testnet/src/Testnet/Byron.hs Show resolved Hide resolved
cardano-testnet/src/Testnet/Byron.hs Show resolved Hide resolved
cardano-testnet/src/Testnet/Shelley.hs Outdated Show resolved Hide resolved
@coot coot force-pushed the p2p-master branch 3 times, most recently from 37ab9a8 to 7f37808 Compare November 15, 2021 21:50
@coot coot force-pushed the p2p-master branch 2 times, most recently from 39c11bf to 757e891 Compare November 16, 2021 18:14
coot and others added 15 commits November 16, 2021 19:48
This commit provides:
* p2p options
* p2p traces
* p2p topology file format
* integration with changes in `ouroboros-consensus` and
  `ouroboros-network`.
* the provided default p2p config values are for a relay node

As of now, using p2p mode is not as thoroughly verified as the current
implementation, and thus it is __not__ supported.  We also do not
guarantee, at this stage, that the p2p mode is forward compatible,
specifically the version of `NodeToNodeV_8` might entirely change its
scope in the future which can break p2p nodes.

Co-authored-by: Armando Santos <bolt12@users.noreply.github.com>
Co-authored-by: Karl Knutsson <karknu@users.noreply.github.com>
Co-authored-by: Marcin Szamotulski <coot@users.noreply.github.com>
It also removes `MockNodeTopolgy` (todo: why?)

Co-authored-by: Armando Santos <bolt12@users.noreply.github.com>
Co-authored-by: Karl Knutsson <karknu@users.noreply.github.com>
Co-authored-by: Marcin Szamotulski <coot@users.noreply.github.com>
TODO: do we want to commit it or keep it in a branch?

Co-authored-by: Armando Santos <bolt12@users.noreply.github.com>
Co-authored-by: Karl Knutsson <karknu@users.noreply.github.com>
Co-authored-by: Marcin Szamotulski <coot@users.noreply.github.com>
- Fixed chairman tests for P2P and NonP2P modes
- Fixed topology format generation in chairman tests.
- Updated config file and topology file
- Added P2P Switch flag to chairman config files
- Added mkTopologyConfig function to switch accordingly as well
- Disable P2P Mode for Byron and Shelley tests
- General chairman-tests refactoring to support P2P Switch
- Fix valency in ByronShelley
- Updated config files
@coot
Copy link
Contributor Author

coot commented Nov 16, 2021

bors merge

iohk-bors bot added a commit that referenced this pull request Nov 16, 2021
3363: Integration with peer2peer networking r=coot a=coot

This PR integrates with peer2peer networking and allows to run a node in either mode:

* non-p2p: the only officially supported version
* p2p: unverified and unsupported p2p mode

This also includes:

* a new topology file format for p2p nodes
* a mechanism to reload topology file on SIGHUP signal

- network trace instances: use ToJSON or ToObject instances
- network trace instances: peer selection traces
- network trace instances: ToJSON instances
- consensus trace instances: trace termination reason
- P2P / NonP2P cardano-node
- Update p2p topology configuration via the SIGHUP signal
- Removed MonoLocalBinds extension
- topology file: improved parsing error message
- Improve NetworkTopology generator
- byron-mainnet configuration: added p2p trace options
- p2p topology file for cardano mainnet
- Document the p2p topology file
- Updated cardano-node-chairman
- Updated trace-dispatcher and trace-forward libraries
- Updated tx-generator
- Updated cardano-testnet package
- Code cleanup in Tracing.Config
- nixos service: add p2p topology support.
- nixos-service: Add space-cost profiling support
- nixos-service: Add space-heap profiling support


Co-authored-by: Marcin Szamotulski <profunctor@pm.me>
Co-authored-by: Armando Santos <armandoifsantos@gmail.com>
Co-authored-by: Jean-Baptiste Giraudeau <jean-baptiste.giraudeau@iohk.io>
Co-authored-by: John Lotoski <john.lotoski@iohk.io>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Nov 16, 2021

This PR was included in a batch that successfully built, but then failed to merge into master. It will not be retried.

Additional information:

{"message":"Waiting on code owner review from deepfire, denisshevchenko, jutaro, and/or MarcFontaine.","documentation_url":"https://docs.github.com/articles/about-protected-branches"}

@coot
Copy link
Contributor Author

coot commented Nov 16, 2021

bors merge

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Nov 16, 2021

Build succeeded:

@iohk-bors iohk-bors bot merged commit 5939f80 into master Nov 16, 2021
@iohk-bors iohk-bors bot deleted the p2p-master branch November 16, 2021 20:50
@coot coot added the peer2peer Issues / PRs related to peer-2-peer label Nov 19, 2021
@coot coot restored the p2p-master branch November 30, 2021 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
networking Issues and PRs related to networking peer2peer Issues / PRs related to peer-2-peer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants