IntersectMBO / IntersectMBO/ouroboros-consensus
Simplify time conversions in Consensus
- Dominant language
- Haskell
- Stars
- 67
- Forks
- 43
- Avg merge
- 5d 13h
- Merged PRs (30d)
- 43
Description
This issue captures some changes that can be implemented to reduce or localize slot-to-wallclock conversions in our code.
- [ ] 1) We could replace[`blockForgeUTCTime`](https://github.com/input-output-hk/ouroboros-network/blob/92778e3bb243ab68015f1f1dc1acf13718e7b1e1/ouroboros-network-api/src/Ouroboros/Network/BlockFetch/ConsensusInterface.hs#L152) with [`headerForgeUTCTime`](https://github.com/input-output-hk/ouroboros-network/blob/92778e3bb243ab68015f1f1dc1acf13718e7b1e1/ouroboros-network-api/src/Ouroboros/Network/BlockFetch/ConsensusInterface.hs#L145), since the equivalent header is in scope.
- [ ] 2) We could annotate headers that have been validated by `ChainSync` with their UTC Time, which is calculated [here](https://github.com/intersectmbo/ouroboros-consensus/blob/f4d877b3f6c3163863582740bf5bba7a6730cbee/ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/MiniProtocol/ChainSync/Client/InFutureCheck.hs#L131).
- [ ] 3) We take the following two independent paths:
- [ ] a) Remove [`HeaderStateWithTime`](https://github.com/intersectmbo/ouroboros-consensus/blob/f4d877b3f6c3163863582740bf5bba7a6730cbee/ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/HeaderStateHistory.hs#L147) and use the header's annotation for the historicity check instead.
The only caveat here is that the `MsgAwaitReply` part of the historicity check currently relies on having a slot time for the anchor in case the fragment is empty. Possible approaches are:
1. Stop relying on this and always disengage (or even disconnect) a peer that sends `MsgAwaitReply` when their candidate fragment is empty (remember that candidate fragments are anchored in a recent immutable tip). This would be more zeleaous than the current historicity check, so it requires proper justification. It seems very plausible to us, but it doesn't seem to immediately follow from any of the "usual" properties like Chain Growth or Existential Chain Quality.
2. Introduce wrapper on top of `AnchoredFragment` that also stores extra data (the slot time of the anchor) for use for the candidate fragments. We would need to implement functions like `intersect` for this that properly update the extra anchor data.[^1]
- [ ] b) Propagate `HeaderWithTime` into the Diffusion Layer's interface so that `headerForgeUTCTime` simply returns the annotation from the header. This will remove an invocation of `slotToWallclock`, instead reusing the header's time annotation.
Observation: Steps 2 and 3b are not really independent. 2 has a significant performance cost without 3b (because discarding the headers requires a lot of allocation). So, both probably need to be in a PR together, but they could be different commits.
[^1]: With infinite time, it would be neat to generalize the `AnchoredFragment` API to allow custom extra data for the anchor; but it is rather questionable that this is a useful thing to pursue right now.
Contributor guide
Research direction
Start with ConsensusInterface.hs, InFutureCheck.hs, and HeaderStateHistory.hs, tracing the existing slot-to-wallclock conversions and HeaderStateWithTime use. Review the ChainSync historicity check and Diffusion Layer interface before choosing between header annotations and an anchored-fragment wrapper. Done means the selected conversion paths are localized without weakening the MsgAwaitReply historicity behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- haskell
- Domain
- blockchain, distributed-systems
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100