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

diffusion: IOError rethrow policy #5067

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

diffusion: IOError rethrow policy #5067

wants to merge 14 commits into from

Conversation

coot
Copy link
Contributor

@coot coot commented Feb 6, 2025

Rebased on top of #5016.

@coot coot requested a review from a team as a code owner February 6, 2025 11:42
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

bolt12 and others added 10 commits February 11, 2025 14:56
(See CHANGELOG for more details)

- Extract Cardano-specific components into separate modules to resolve cyclic dependency issues:
  - `ConsensusModePeerTargets` and `PeerSelectionTargets` were refactored to avoid circular dependencies.

- Introduce extension points in Diffusion data structures:
  - Split P2P functionality into `P2PCardano` (for Cardano Node specifics) and a more general `P2P` module.
  - Added `Minimal/Node.hs` as a placeholder for a minimal Node diffusion instantiation example.

- Refactor Peer Selection:
  - Removed redundant `PeerSelectionActionsArgs`.
  - Reorganized `DNSActions` and `LedgerPeersArgs`.
  - Improved API for `PeerSelectionActions`.

- Rename `Cardano.Node` to `Cardano.Network`.

- Generalize and refactor the following components:
  - `ConsensusModePeerTargets`.
  - `PeerSelection{Views, Counters}` to allow extensibility for third-party users.
  - `PublicRootPeers`.

- Enhance the Outbound Governor:
  - Generalize `MinimalP2P` for diffusion initialization.
  - Update `ArgumentsExtra` with new parameters for a more flexible diffusion setup.
  - Refactor `PeerSelection.Governor.Monitor` to separate Cardano-specific monitoring actions and support third-party custom actions.

- Ensure the `Diffusion` module is fully polymorphic:
  - Removed `CardanoP2P` dependencies.
  - Updated the test suite to align with the new structure.

- Miscellaneous updates:
  - Update CHaP and fix build issues.
  - Remove `daBlockFetchMode`.
  - Address review feedback.
  - Fix churn timeout tests and increase `shortDelay` to stabilize tests.
  - Recover `sigusr1` signal handler.
  - Make addresses polymorphic.
  - Refactor `extraDebugState` and introduce `P2PDecisionType`.
  - Refactor `requestPublicRootPeers` type.
They are passed as a closure, no need to leak them outside of `Cardano`
(or any other 3rd party integration).
Instead of specifying `extraAPI ~ ()` for non-p2p, we can make it
polymorphic.  This way both p2p & non-p2 `Application` type is using the
same type variables, and the wrapper type can be removed.  This is more
in-line with removal of non-p2p code base.
Also adhere to the convention: first `extraFlags`, then `extraPeers`
We can pass `PeerSelectionInterfaces` to `updateWithState` function to
get access to `readUseLedgerPeers`.
@coot coot force-pushed the coot/io-rethrow-policy branch from f1006f9 to 0951ffe Compare February 11, 2025 17:02
coot and others added 3 commits February 12, 2025 13:22
Also edit export lists so that we need fewer imports.
@coot coot mentioned this pull request Feb 13, 2025
7 tasks
@coot coot force-pushed the coot/io-rethrow-policy branch from 0951ffe to 4512e4b Compare February 13, 2025 11:22
@coot coot requested a review from a team as a code owner February 13, 2025 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

3 participants