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

Pull configuration values to ouroboros-network #4805

Closed
coot opened this issue Feb 12, 2024 · 4 comments · Fixed by #4808
Closed

Pull configuration values to ouroboros-network #4805

coot opened this issue Feb 12, 2024 · 4 comments · Fixed by #4808
Assignees
Labels
diffusion Issues / PRs related to diffusion layer good first issue Good for newcomers

Comments

@coot
Copy link
Contributor

coot commented Feb 12, 2024

There are various default configuration values defined in different places. I suggest we provide:

Ouroboros.Network.Diffusion.Configuration

module in ouroboros-network which provides them. Some of the defaults are defined in

  • cardano-node and include:
    • pncProtocolIdleTimeout
    • pncTimeWaitTimeout
    • pncAcceptedConnectionsLimit
    • pncTargetNumberOfRootPeers
    • pncTargetNumberOfKnownPeers
    • pncTargetNumberOfEstablishedPeers
    • pncTargetNumberOfActivePeers
    • pncTargetNumberOfKnownBigLedgerPeers
    • pncTargetNumberOfEstablishedBigLedgerPeers
    • pncTargetNumberOfActiveBigLedgerPeers
    • pncEnableP2P
    • pncPeerSharing

from ouroboros-network:

from ouroboros-consensus:

@coot coot added the diffusion Issues / PRs related to diffusion layer label Feb 12, 2024
@coot
Copy link
Contributor Author

coot commented Feb 12, 2024

As of implementation it might be good idea to group all the outbound-governor related targets (e.g. pncTargetNumber...) group in a single record. For other we could provide newtype wrappers, especially if that turns out to be beneficial for ouroboros-network too - the advantage of this is that who ever is doing top level plumbing in cardano-node (or elsewhere, which currently doesn't exist 😁) will be assisted by the compiler.

@coot coot added the good first issue Good for newcomers label Feb 12, 2024
@crocodile-dentist crocodile-dentist self-assigned this Feb 15, 2024
@coot
Copy link
Contributor Author

coot commented Feb 19, 2024

There's possibly one more config value which goes into the outbound-governor default configuration: the default number of bootstrap peers.

@coot
Copy link
Contributor Author

coot commented Feb 21, 2024

Peer sharing configuration values than could be moved too, ref (it's a link to a PR currently in review).

@crocodile-dentist
Copy link
Contributor

will do

@crocodile-dentist crocodile-dentist moved this to In Review in Ouroboros Network Feb 23, 2024
@coot coot moved this from In Review to Done in Ouroboros Network Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
diffusion Issues / PRs related to diffusion layer good first issue Good for newcomers
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants