celestia-node
celestia-node copied to clipboard
fix: ensure `HeaderSub` guarantees a contiguous stream of headers
There is an edge case possible here. The HeaderSub doesn't guarantee a contiguous stream of headers, and there might be gaps that we have to handle. It shouldn't be hard to handle those through the header store, though.
Originally posted by @Wondertan in https://github.com/celestiaorg/celestia-node/pull/3539#discussion_r1659115840
I wasn't aware that it doesn't guarantee a contiguous stream of headers - to me, this sounds like something that needs to be fixed in headersub itself. Otherwise, we need to make it public because that is not intuitive and it's how everybody is currently using it. I would be fine holding off on merging this until that is fixed, can we discuss it further?
Originally posted by @distractedm1nd in https://github.com/celestiaorg/celestia-node/pull/3539#discussion_r1660961007
Should we fix this upstream in go-header? @Wondertan suggests that instead of modding HeaderSub
we could alternatively:
Agreed, we can move this issue separately. I think there might be a potential reason to fix this on go-header level with a Subscriber wrapper that does exactly gap filling. This way users relying on header subscriptions would not need to think of that edge case. Need to think of the implications of that tho