Attributable errors 2025 #3611
Attributable errors 2025 #3611
Conversation
joostjager
force-pushed
the
attr-errors-2025
branch
from
97480b6 to
72a8397
Compare
|
Okay, you got totally bitten by some really old serialization logic that was written when upgrade/downgrade support was...not super well thought-through (and which was not upgraded when we made the transition, sadly). Luckily there's fairly little of this code left, but you jumped right into an area where there's a good bit of it :(. I went ahead and implemented the serialization logic at https://git.bitcoin.ninja/?p=rust-lightning;a=log;h=refs/heads/2025-02-3611-ser-impl note that the |
joostjager
force-pushed
the
attr-errors-2025
branch
from
d2a3b6a to
936a2b9
Compare
| @@ -9973,7 +9970,21 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider | |||
| }, | |||
| &InboundHTLCState::LocalRemoved(ref removal_reason) => { | |||
| 4u8.write(writer)?; | |||
| removal_reason.write(writer)?; | |||
| match removal_reason { | |||
There was a problem hiding this comment.
Pending htlc initiated the removal (send update fail back upstream) but no response yet from peer to lock into their commit tx. See InboundHTLCState docs.
This code needs to work because of resends. Fetch fail message but don't deliver. Then check if after restart message is identical. Other node just sends chan reestablish.
| 4 => InboundHTLCState::LocalRemoved(Readable::read(reader)?), | ||
| 4 => { | ||
| let reason = match <u8 as Readable>::read(reader)? { | ||
| 0 => InboundHTLCRemovalReason::FailRelay(msgs::OnionErrorPacket { |
There was a problem hiding this comment.
Update in holding cell, still waiting for next round of commitment dance. Should be tests that test holding cell persistence.
Strategy: break main, see if tests fail.
There was a problem hiding this comment.
expect_pending_htlcs_forwardable!(nodes[1]); - in here let node 0 do something else like add another htlc, so that the failure gets into the holding cell
lightning/src/ln/channelmanager.rs
Outdated
| 0u8.write(writer)?; | ||
| channel_id.write(writer)?; | ||
| htlc_id.write(writer)?; | ||
| reason.write(writer)?; | ||
| attribution_data.write(writer)?; // TODO: Make TLV? |
There was a problem hiding this comment.
Try remove serialization logic, see where it used.
Look at call site, see what context is, and find how to add more data at a higher level (tlv?)
lightning/src/ln/channelmanager.rs
Outdated
| @@ -12881,6 +12887,7 @@ impl Readable for HTLCFailureMsg { | |||
| channel_id: Readable::read(reader)?, | |||
| htlc_id: Readable::read(reader)?, | |||
| reason: Readable::read(reader)?, | |||
| attribution_data: Readable::read(reader)?, | |||
There was a problem hiding this comment.
change write side to use version 2/3 and it will have the length descriptor. can add additional content at the end.
maybe intro tlv stream here?
joostjager
force-pushed
the
attr-errors-2025
branch
2 times, most recently
from
33c4368 to
dc4b841
Compare
joostjager
force-pushed
the
attr-errors-2025
branch
10 times, most recently
from
636570a to
898cd51
Compare
Prepares for extending OnionErrorPacket with attribution data.
Co-authored-by: Matt Corallo <[email protected]>
joostjager
force-pushed
the
attr-errors-2025
branch
from
898cd51 to
21791b2
Compare
Work PR for reviving #2256