Attributable failures (feature 36/37)

[joostjager]

Failure attribution is important to properly penalize nodes after a payment failure occurs. The goal of the penalty is to give the next attempt a better chance at succeeding. In the happy failure flow, the sender is able to determine the origin of the failure and penalizes a single node or pair of nodes.

Unfortunately it is possible for nodes on the route to hide themselves. If they return random data as the failure message, the sender won't know where the failure happened.

This PR proposes a new failure message format that lets each node commit to the failure message. If one of the nodes corrupts the failure message, the sender will be able to identify that node.

For more information, see https://lists.linuxfoundation.org/pipermail/lightning-dev/2022-October/003723.html.

LND implementation: lightningnetwork/lnd#7139

LDK implementation: lightningdevkit/rust-lightning#3611

Eclair implementation: ACINQ/eclair#2519

[thomash-acinq]

I've started implementing it in eclair, do you have some test vectors so we can check that we are compatible?
The design seems good to me, but as I've said previously, I think keeping hop payloads and hmacs for 8 nodes only (instead of 27) is enough for almost all use cases and would give us huge size savings.

[joostjager]

I don't have test vectors yet, but I can produce them. Will add them to this PR when ready.

Capping the max hops at a lower number is fine to me, but do you have a scenario in mind where this would really make the difference? Or is it to more generally that everything above 8 is wasteful?

[joostjager]

@thomash-acinq added a happy fat error test vector.

[joostjager]

I think this big gap in the bits has emerged here because of tentative spec changes that may or may not make it. Not sure why that is necessary. I thought for unofficial extensions, the custom range is supposed to be used?

I can see that with unofficial features deployed in the wild, it is easier to keep the same bit when something becomes official. But not sure if that is worth creating the gap here? An alternative is to deploy unofficial features in the custom range first, and then later recognize both the official and unofficial bit. Slightly more code, but this feature list remains clean.

[joostjager]

Added fat error signaling to the PR.

[thomash-acinq]

I've spent a lot of time trying to make the test vector pass and I've finally found what was wrong:
In the spec you write that the hmac covers

• failure_len, failuremsg, pad_len and pad.

• The first y+1 payloads in payloads. For example, hmac_0_2 would cover all three payloads.

• y downstream hmacs that correspond to downstream node positions relative to x. For example, hmac_0_2 would cover hmac_1_1 and hmac_2_0.

implying that we need to concatenate them in that order. But in your code you follow a different order:

// Include payloads including our own.
_, _ = hash.Write(payloads[:(NumMaxHops-position)*payloadLen])

// Include downstream hmacs.
var hmacsIdx = position + NumMaxHops
for j := 0; j < NumMaxHops-position-1; j++ {
	_, _ = hash.Write(
		hmacs[hmacsIdx*sha256.Size : (hmacsIdx+1)*sha256.Size],
	)

	hmacsIdx += NumMaxHops - j - 1
}

// Include message.
_, _ = hash.Write(message)

I think the order message + hop payloads + hmacs is more intuitive as it matches the order of the fields in the packet.

[joostjager]

Oh great catch! Will produce a new vector.

[joostjager]

@thomash-acinq updated vector

[joostjager]

Updated LND implementation with sender-picked fat error structure parameters: lightningnetwork/lnd#7139

[GeorgeTsagk]

Great proposal! Quite an elegant idea
There are some elusive details that make this work, which might need some more coverage

[GeorgeTsagk]

could elaborate a bit more on why these hmacs can be discarded / considered irrelevant

[joostjager]

Added a bit more explanation.

[GeorgeTsagk]

should also include the "unhappy" path here, focusing on:

• why origin attributes blame/error on first decoding failure (if that's still the intended way in which things work)
• how an intermediary node that maliciously flips bits cannot shake the blame off themselves

[joostjager]

What are origin attributes?

I think I've elaborated on this more in the latest iteration of the PR.

[GeorgeTsagk]

can the forwarding node always pick the correct pruning positions if it doesn't know its own position in the path?

[joostjager]

Yes, the forwarding node can pick the correct pruning positions. The block of hmacs that the forwarding node receives contains series of hmacs for all possible path lengths up to 20 hops. The forwarding node obviously knows that it is also part of the path, so there can never be an additional 20 hops (this would bring the total to 21). This also means that the last hmac for every hmac series won't ever be useful.

Will try to add some more text to explain this.

[GeorgeTsagk]

to 210 hmacs

there was the (max_hops * (max_hops + 1)) / 2 formula in a previous diff, explaining how this number was produced, seems to have been trimmed

[joostjager]

Re-added explanation of 210.

[joostjager]

One thing I now realize is that even if there's a node on the path that messes with the data and/or hmacs, the upstream nodes will still have valid hmacs and also valid hold_time_ms.

Information that isn't all that useful in that case though. The sender will simply proceed with penalizing the bad node, retry, and only process timing information after "proper" failures.

[joostjager]

If we go for the idea described in #1044 (comment), it may be unnecessary to keep that first payload byte that is either 0 or 1 to indicate intermediate or failing node. Because the 'legacy' hmac is still present, that can also be used to identify the failing node.

[joostjager]

Thanks for your review @GeorgeTsagk. Comments addressed.

[joostjager]

PR rebased and updated to reflect the new approach with a tlv extension to update_fail_htlc.

[joostjager]

Decided to derive a new key rather than continue pulling bits for chacha from the ammag key, to minimize the chance of somehow reusing the stream. Alternatively could use a different nonce, but I believe that would be new in lightning?

[Roasbeef]

Decided to derive a new key rather than continue pulling bits for chacha from the ammag key, to minimize the chance of somehow reusing the stream.

How does deriving a new key do a better job at avoiding key reuse than pull from the stream? The stream is generated via ECDH between the sender and a given hop, you mix in both sources of randomness (sender + node), thereby strengthening the entropy of the stream.

On the other-hand, the strength of this new key would depend only on the entropy sources of that intermediate node (may be missing some context, drive by review here from the GH UI diff).

[joostjager]

No, new key doesn't do a better job than pulling from the stream. It's just that we now have two fields to encrypt and I was thinking of the case of a programming error being made where for each of these fields, the stream is initialized from the same key producing the same bits.

I think it just comes down to personal preference, no strong argument against or in favor of a new key.

[joostjager]

Updated test vectors

[joostjager]

Sentence added that states that nodes may report a zero htlc hold time in case they have no timing information available.

[joostjager]

Open question is how the odd/even feature bits should be interpreted. What does an odd feature bit mean for attributable failures? Nodes supporting it will just always add attribution_data and don't have any requirements regarding their peers. Perhaps even could mean that they'd require attribution_data from their downstream peer?

For sending nodes, signaling even could indicate that if you want to serve this sender as a path node, you'll have to provide attribution_data?

[DerEwige]

@rustyrussell asked me to post my latency data in the attributable failures PR.
I hope this is the right place, if not please just point me in the right direction.

I've been tracking latency for all may payments for over a year now and ban channels that are slow.
This is done mainly to avoid the hightened FC risk that comes with stuck HTLCs.

I do not keep historic data, but rather just have a moving window of the last few days.
The current percentiles look like this:

HTLC processing time per hop

successful payments
p50   0.69s
p90   2.63s
p99  10.75s

failed payments
p50   0.66s
p90   3.73s
p99  14.61s 

If you need more details or info on how the data was collected.
I would be happy to provide

[GeorgeTsagk]

For sending nodes, signaling even could indicate that if you want to serve this sender as a path node, you'll have to provide attribution_data?

Given that supporting nodes will do this by default I think there's no extra value in senders advertising that they prefer attr failures. I think the latest approach where attribution_data are always set by default is sufficient signal-wise.

Also this feature does not introduce any extra risk to your own node, so people will by default use it when it becomes available IMO. No need to create a "push" in feature adoption by having senders require it explicitly

[joostjager]

Yes agreed that sender signaling is probably too much. Was just wondering what that even feature bit could mean and whether we want to define it at all?

[thomash-acinq]

Judging by the test vectors, you're not using the reason field but the "return packet" which is the same but before being xored with the random stream.
I actually think it would make more sense to use the reason field (the serialized packet xored with the pseudo random stream) as it would make computing the attribution data completely independent:

• First you build the "return packet" and xor it to obtain the reason field
• And then, if you have "option_attributable_failure" enabled, you compute the attribution data using the already built reason field.

[joostjager]

That is correct indeed, I use the "unencrypted" reason field as input for the attribution data. That's also how it was done in the 2023 version of this.

We could xor first, but I am not sure what advantage that would give? You mention attribution data being completely independent, but another perspective is that it is actually more dependent, because it is now also dependent on the cypher stream. To me, processing the failure entirely and then encrypting all of it feels more logical, but there is probably not much difference between the two.

[joostjager]

Pushed commit that fixes incorrect references to reason. Thanks for pointing out!

I've kept the protocol to encrypt-after-attribution for now, but open to discussion.