Clarify onion failure data encoding #979
Conversation
TheBlueMatt
force-pushed
the
2022-04-onion-fixes
branch
from
cf1e881
to
842a62e
Compare
04-onion-routing.md
Outdated
| @@ -853,7 +853,9 @@ The top byte of `failure_code` can be read as a set of flags: | |||
| * 0x1000 (UPDATE): new channel update enclosed | |||
|
|
|||
| Please note that the `channel_update` field is mandatory in messages whose | |||
| `failure_code` includes the `UPDATE` flag. | |||
| `failure_code` includes the `UPDATE` flag. It is encoded as only the bytes | |||
| which would appear in the message body, *not* including the `0x0102` message | |||
There was a problem hiding this comment.
Right now eclair supports decoding both cases, but encodes with the 0x0102 message prefix (IIRC one implementation only supported reading that case at some point so it was the right choice to maximize compatibility). I'm happy to change it if everyone supports it today.
There was a problem hiding this comment.
I believe lnd and we do not use the message type prefix, CLN and y'all do. I'm happy with either, I don't really care, I'm also happy to flip a coin.
There was a problem hiding this comment.
I'd rather drop the message prefix as you propose, I think it makes more sense without it, I just need to make sure that latest versions of all implementations will know how to decode that.
There was a problem hiding this comment.
Whether they do or not, no-type-prefix is, from what I've seen, by far the most common version on the network, likely because its what lnd does.
| * [`u16`:`len`] | ||
| * [`len*byte`:`channel_update`] | ||
|
|
||
| The channel from the processing node has been disabled. | ||
| No flags for `disabled_flags` are currently defined, thus it is currently |
There was a problem hiding this comment.
In eclair this is actually the channel_update.message_flags byte followed by the channel_update.channel_flags byte, so it won't be 0x0000. It is redundant with the contents of the channel_update though, so I really don't know why a separate field was included in that error.
There was a problem hiding this comment.
Yea, honestly it seems like an oversight that there's extra bytes here, but given its otherwise redundant I see no reason to copy the channel update flags, better to just set them to 0 and move on.
|
lnd doesn't write the type byte today, but all nodes are able to decode both. So either way, if we start to write the type byte, we shouldn't have any propagation/decoding issues. |
Apparently not all implementations implemented the onion encoding the same, causing vastly differing onion failure packets. This should unify them somewhat. CC ElementsProject/lightning#5154
TheBlueMatt
force-pushed
the
2022-04-onion-fixes
branch
from
842a62e
to
ef8cead
Compare
|
After meeting discussion I swapped it - the change now says we have to add the message type bytes, not skip them. |
...well, okay, do today, but by the time anyone is reading this it'll be "used to".
|
Okay, added a little historical note. |
|
Should we merge this, as discussed during the last spec meeting? @Roasbeef is the historical note good enough for you? |
|
You can also close #545 when you close this PR. |
For about a year [1], the spec has prescribed encoding channel_updates with their type prefix (0x0102) in onion failure messages. LND can decode correctly with or without the prefix but hasn't been writing the prefix during encoding. This commit starts writing the prefix. [1] lightning/bolts#979
For about a year [1], the spec has prescribed encoding channel_updates with their type prefix (0x0102) in onion failure messages. LND can decode correctly with or without the prefix but hasn't been writing the prefix during encoding. This commit starts writing the prefix. [1] lightning/bolts#979
No description provided.