Remove zlib compression gossip query support #981
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
23 tasks
Gossip query compression is not very useful - it was added for mobile clients to, in theory, sync the gossip data directly from P2P peers, but to my knowledge no mobile clients actually use it for that, or at least use it where the gossip *query* data is a substantial portion of their overall bandwidth usage. Further, because of the semantics of `gossip_timestamp_filter`, its impractical to ensure you receive a reliable, full view of the gossip data without re-downloading large portions of the gossip data on startup. Ultimately, gossip queries are a pretty non-optimal method of synchronizing the gossip data. If someone wants highly optimized gossip data synchronization a new method based on set reconciliation needs to be propose. Finally, the current gossip query encoding semantics do not allow for negotiation and instead require all lightning implementations take a zlib dependency in some form or another. Given the recent zlib decoding memory corruption vulnerability, this seems like an opportune time to simply remove the zlib support, requiring that nodes stop sending compressed gossip query data (though they can support reading such gossip query data as long as they wish). This is an alternative to the suggested gossip query encoding support in lightning#825.
TheBlueMatt
force-pushed
the
2022-4-no-zlib
branch
from
f2c8588 to
2e8f209
Compare
Collaborator
|
So lnd doesn't send the zlib encoded scids, but we do decode it. So if we remove it then not much things we need to do on our end. |
Collaborator
|
ACK 2e8f209 |
t-bast
added a commit
to ACINQ/eclair
that referenced
this pull request
Apr 26, 2022
While zlib provides good compression results for gossip, it's a dependency that had a few important CVEs in the past. Some implementations are reluctant to import it, so we decided to remove it from the specification in lightning/bolts#981 We stop compressing with zlib and only send uncompressed results, while still supporting receiving compressed data. We will remove support for decompression once our monitoring indicates that we stopped receiving compressed data.
t-bast
added a commit
to ACINQ/eclair
that referenced
this pull request
May 10, 2022
While zlib provides good compression results for gossip, it's a dependency that had a few important CVEs in the past. Some implementations are reluctant to import it, so we decided to remove it from the specification in lightning/bolts#981 We stop compressing with zlib and only send uncompressed results, while still supporting receiving compressed data. We will remove support for decompression once our monitoring indicates that we stopped receiving compressed data.
t-bast
added a commit
to ACINQ/eclair
that referenced
this pull request
May 11, 2022
While zlib provides good compression results for gossip, it's a dependency that had a few important CVEs in the past. Some implementations are reluctant to import it, so we decided to remove it from the specification in lightning/bolts#981 We stop compressing with zlib and only send uncompressed results, while still supporting receiving compressed data. We will remove support for decompression once our monitoring indicates that we stopped receiving compressed data. We also reduce the maximum allowed chunk size. The previous calculation was wrong and could lead to messages bigger than 65kB.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Gossip query compression is not very useful - it was added for
mobile clients to, in theory, sync the gossip data directly from
P2P peers, but to my knowledge no mobile clients actually use it
for that, or at least use it where the gossip query data is a
substantial portion of their overall bandwidth usage.
Further, because of the semantics of
gossip_timestamp_filter, itsimpractical to ensure you receive a reliable, full view of the
gossip data without re-downloading large portions of the gossip
data on startup.
Ultimately, gossip queries are a pretty non-optimal method of
synchronizing the gossip data. If someone wants highly optimized
gossip data synchronization a new method based on set
reconciliation needs to be propose.
Finally, the current gossip query encoding semantics do not allow
for negotiation and instead require all lightning implementations
take a zlib dependency in some form or another. Given the recent
zlib decoding memory corruption vulnerability, this seems like an
opportune time to simply remove the zlib support, requiring that
nodes stop sending compressed gossip query data (though they can
support reading such gossip query data as long as they wish).
This is an alternative to the suggested gossip query encoding
support in #825.