Skip to content

Stop compressing with zlib#2244

Merged
t-bast merged 2 commits intomasterfrom
remove-zlib
May 11, 2022
Merged

Stop compressing with zlib#2244
t-bast merged 2 commits intomasterfrom
remove-zlib

Conversation

@t-bast
Copy link
Member

@t-bast t-bast commented 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 t-bast requested review from pm47 and sstone April 26, 2022 07:16
@codecov-commenter
Copy link

Codecov Report

Merging #2244 (91ec0b7) into master (f099409) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #2244      +/-   ##
==========================================
+ Coverage   84.30%   84.31%   +0.01%     
==========================================
  Files         194      194              
  Lines       14383    14381       -2     
  Branches      589      587       -2     
==========================================
+ Hits        12125    12126       +1     
+ Misses       2258     2255       -3     
Impacted Files Coverage Δ
...re/src/main/scala/fr/acinq/eclair/NodeParams.scala 93.12% <100.00%> (+0.32%) ⬆️
...main/scala/fr/acinq/eclair/io/PeerConnection.scala 85.98% <0.00%> (-0.38%) ⬇️
...clair/channel/publish/ReplaceableTxPublisher.scala 74.17% <0.00%> (+1.32%) ⬆️
...src/main/scala/fr/acinq/eclair/db/pg/PgUtils.scala 90.90% <0.00%> (+1.51%) ⬆️

@sstone
Copy link
Member

sstone commented May 10, 2022

LGTM, but how do we make sure that query messages still fit in 65 Kb ?

t-bast added 2 commits May 10, 2022 10:03
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.
The previous calculation was wrong and could lead to messages bigger than 65kB.
@t-bast
Copy link
Member Author

t-bast commented May 10, 2022

LGTM, but how do we make sure that query messages still fit in 65 Kb ?

Thanks for spotting that, there was indeed an issue!
I don't know how we came up with the previous number of 3200, but it was clearly wrong and untested :/
This is fixed in a7653f9

@t-bast t-bast merged commit ee74f10 into master May 11, 2022
@t-bast t-bast deleted the remove-zlib branch May 11, 2022 12:02
t-bast added a commit that referenced this pull request Mar 9, 2026
We removed support for sending compressed data when it was removed from
the spec (see #2244). We meant to
remove support for receiving compressed data as well as a follow-up, but
it slipped for a long time! There's no reason to keep supporting it now.
t-bast added a commit that referenced this pull request Mar 11, 2026
We removed support for sending compressed data when it was removed from
the spec (see #2244). We meant to
remove support for receiving compressed data as well as a follow-up, but
it slipped for a long time! There's no reason to keep supporting it now.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants