Big Ledger Peers #4462

coot · 2023-03-20T09:28:13Z

Description

The PR contains a few set of changes:

some initial changes (refactoring)
static big ledger peers
extending peer selection tests
introduction of IsBigLedgerPeer
refactoring of mini-protocol callbacks which we accept: we use different context for initiator / responder sides which arguments provide extra information about the connection (things like ConnectionId or IsBigLedgerPeer for the outbound side): ExpandedMiniProtocolContext, MinimapProtocolContext and ResponderMiniProtocolContext.
refactoring of the diffusion tests (sim-net), which allows us to test big ledger peers
when I was working on that I found a bug in connection-manager which resulted in wrong transitions when a node connects to itself

This PR is addresses most of the task of #3886.

TODO:

add e2e tests for big ledger peers
churn big ledger peers
updated the spec:
- add SelfConn
- add SelfConn^{-1}
- add InboundState^{\tau} Unidirectional -> OutboundState Unidirectional

Checklist

Branch
- Commit sequence broadly makes sense
- Commits have useful messages
- The documentation has been properly updated
- New tests are added if needed and existing tests are updated
- Any changes in the Consensus API (every exposed function, type or module) that has changed its name, has been deleted, has been moved, or altered in some other significant way must leave behind a DEPRECATED warning that notifies downstream consumers. If deprecating a whole module, remember to add it to ./scripts/ci/check-stylish.sh as otherwise stylish-haskell would un-deprecate it.
- If this branch changes Network and has any consequences for downstream repositories or end users, said changes must be documented in interface-CHANGELOG.md
- If serialization changes, user-facing consequences (e.g. replay from genesis) are confirmed to be intentional.
Pull Request
- Self-reviewed the diff
- Useful pull request description at least containing the following information:
  - What does this PR change?
  - Why these changes were needed?
  - How does this affect downstream repositories and/or end-users?
  - Which ticket does this PR close (if any)? If it does, is it linked?
- Reviewer requested

Fixes #3924

ouroboros-network/src/Ouroboros/Network/PeerSelection/LedgerPeers.hs

ouroboros-network/test/Test/LedgerPeers.hs

ouroboros-network/src/Ouroboros/Network/PeerSelection/Governor/Types.hs

coot · 2023-06-06T11:05:45Z

ouroboros-network/src/Ouroboros/Network/PeerSelection/Governor/Types.hs

     | TraceTargetsChanged     PeerSelectionTargets PeerSelectionTargets
+


We should add a TraceInsaneTargets.

This is currently not easy, we update targets in an STM transaction.

🤔 we could move the check outside of the outbound governor, and have a method which submits only if they are sane. But this is outside of this PR.

ouroboros-network/src/Ouroboros/Network/PeerSelection/State/KnownPeers.hs

The core change is adding a lambda in the initiator part of `RunMiniProtocol`. From there it is propagated up to ouroboros-consensus-diffusion.

Also extend its haddocks.

Before this patch we were passing `ConnectionId`, `ControlMessageSTM` and now also `IsBigLedgerPeer` in distinct lambdas. This patch combines them into a single lambda which takes either an initiator or responder context. As in one of the previous patches, the core changes are done to the `RunMiniProtocol` type. We introduces three contexts: * `ExpandedInitiatorContext` - a context used by node-to-node applications. Although non-p2p node-to-node applications receive a context which is created out of `MinimalInitiatorContext` - this is to keep the interface used by `ouroboros-consensus-diffusion` as simple as possible. * `MinimalInitiatorContext` - a context used by node-to-client applications as well as in many non-p2p contexts (e.g. `connectToNode` or `subscribe` from `cardano-client` package) * `ResponderContext` - a context used by all responders.

Currently it only generates numbers which are less than 20, even though the intention was to make it less than 10k. Instead we generate it from the `[0,1k)` interval.

When connecting to itself, the connection manager would create two `TVar`'s with the state of the connection eventually leading to invalid transitions. We both of the following transitions: * `IncludeConnection Outbound → IncludeConnection Inbound` * `IncludeConnection Inbound → IncludeConnection Outbound` are valid. Which one will be executed depends which thread (inbound or outbound) will be executed first after connection is accepted.

Don't use partial function `last`; Quite rarely the refactored tests can fail because of that.

…orContext strict

* `ConnectionId` * `MinimalInitiatorContext` * `ResponderContext`

Two new trace points are added to `TraceLedgerPeers`: * `NotEnoughBigLedgerPeers` * `NotEnoughLedgerPeers` The `FallingBackToBootstrapPeers` is renamed as `FallingBackToPublicRootPeers`.

coot added networking outbound-governor Issues / PRs related to outbound-governor labels Mar 20, 2023

coot self-assigned this Mar 20, 2023

coot linked an issue Mar 20, 2023 that may be closed by this pull request

Eclipse Evasion Implementation #3886

Closed

13 tasks

coot removed a link to an issue Mar 20, 2023

Eclipse Evasion Implementation #3886

Closed

13 tasks

coot force-pushed the coot/eclipse-evasion branch from c11adbc to 3e83ce2 Compare April 27, 2023 18:04

coot force-pushed the coot/eclipse-evasion branch from 3e83ce2 to 2268d58 Compare May 10, 2023 05:47

coot force-pushed the coot/eclipse-evasion branch from 2268d58 to 405fc39 Compare May 22, 2023 13:06

coot marked this pull request as ready for review May 22, 2023 13:07

coot requested review from dcoutts and bolt12 May 22, 2023 13:24

coot changed the title ~~Introduce Big Ledger Peers~~ Big Ledger Peers May 22, 2023

coot force-pushed the coot/eclipse-evasion branch from 405fc39 to 0e22de8 Compare May 25, 2023 08:19

coot requested review from njd42 and peterwthompson as code owners May 25, 2023 08:19

coot removed request for peterwthompson and njd42 May 25, 2023 10:10

coot force-pushed the coot/eclipse-evasion branch 2 times, most recently from f4a8097 to 79dde3a Compare May 31, 2023 05:42

coot linked an issue Jun 1, 2023 that may be closed by this pull request

Eclipse Evasion Implementation #3886

Closed

13 tasks

coot force-pushed the coot/eclipse-evasion branch 2 times, most recently from 73ce1eb to 131532f Compare June 2, 2023 11:25