Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BOLT 7: clarify how to decode empty arrays #729

Closed

Conversation

sstone
Copy link
Collaborator

@sstone sstone commented Jan 20, 2020

Channel queries use arrays (of short channel ids, timestamps, ...) encoded with a single byte for the encoding type followed by encoded data.
If the encoded data is empty, it always decodes to an empty array, no matter what the encoding type is set to.

Channel queries use arrays (of short channel ids, timestamps, ...) encoded with a single byte for the encoding type followed by encoded data.
If the encoded data is empty, it always decodes to an empty array, no matter what the encoding type is set to.
@t-bast
Copy link
Collaborator

t-bast commented Jan 21, 2020

Actually there's already a pending PR with the same clarification: #560

@sstone
Copy link
Collaborator Author

sstone commented Jan 22, 2020

Good point! Here I'm mostly focused on how to specifically deal with empty arrays, which goes a bit further than #560 (it's not explicit on that point), the biggest issue being: are empty arrays sent wit a zlib prefix valid ? it's ok for eclair and lnd but not c-lightning, and the answer is not obvious yet.

@cdecker
Copy link
Collaborator

cdecker commented Jan 31, 2020

I don't think #560 touches this at all, and no an empty byte string is not a valid zlib compressed value:

>>> import zlib
>>> zlib.compress(b'')
'x\x9c\x03\x00\x00\x00\x00\x01'
>>> zlib.decompress(b'')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
zlib.error: Error -5 while decompressing data: incomplete or truncated stream

As such I think it is invalid to specify an encoding and then providing an invalid value for that encoding, since it might mask other issues internally. We could special-case the empty byte string, but what good would that be? I think the correct way to handle this is ACINQ/eclair#1280, i.e., omitting the fields altogether or specifying the uncompressed encoding if the intention is to communicate an empty byte string.

@sstone
Copy link
Collaborator Author

sstone commented Jan 31, 2020

Ok, I still like the idea of "nothing in means nothing out" but it seems that every zlib encoder out there disagree with me except for the one we use :) so I'll agree that it was rather a bug in our code than a uncertainty in the specs
ps. w'eve chosen to encode empty lists in uncompressed format, it works against c-lighting and lnd

@sstone sstone closed this Jan 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification substantive change or addition around wording or meaning
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants