ChainSync client: simplify a correctness argument

[amesgen]

This is a single-commit PR; see its message.

Migrated from IntersectMBO/ouroboros-network#4313

[amesgen]

For direct reference, the comments from IntersectMBO/ouroboros-network#4313:

---

@nfrisby:

Hmm. Actually: does "average-case = worst-case" apply here, meaning that we don't want to share the selection's prefix amongst all the ChainSync clients' theirFrag prefixes?

---

@amesgen

I think this is a nice change, I could even see defining an abstract function like

cross ::
     forall blk. HasHeader blk
  => AnchoredFragment blk
  -> AnchoredFragment blk
  -> Maybe (Point blk, AnchoredFragment blk)
cross c1 c2 = do
    (p1, _p2, _s1, s2) <- AF.intersect c1 c2
    let crossed = fromMaybe (error "impossible") $ AF.join p1 s2
    pure (AF.anchorPoint s2, crossed)

to make it more clear that we don't rely on any domain-specific properties of the ChainSync fragments here. Of course, we should keep the fairbairn threshold in mind.

W.r.t. performance, this new implementation should have the same worst-case performance; and I don't immediately see how it might have a better average-case performance (splitAfterPoint and join both perform sharing with previous fragments). In any case, it should be easy to write a small benchmark (and if we use the cross function above, it is easy to swap out the implementation if we intentionally want to make the average case slower again).

---

@nfrisby

splitAfterPoint and join both perform sharing with previous fragments

cross causes all clients to share the selection's prefix. join causes each client to reuse its own copy of that prefix. So splitAfterPoint causes the clients to share less, right?

[nc6]

We reviewed this in a call, and agree that it makes sense, and that the increased sharing in a good case is unlikely to be significant, especially compared to the ledger state.

[nfrisby]

The local definition is a good balance 👍