Require to claim revoked local output in its own penalty tx post-anchor

[ariard]

If option_anchor_outputs applies, the cheating node can pin spends of its
HTLC-timeout/HTLC-success outputs thanks to SIGHASH_SINGLE malleability.
Using a single penalty transaction for all revoked outputs is thus unsafe as it
could be blocked to propagate long enough for the `local node's main output 's
relative timelock to expire and the cheating party escaping the penalty on this
output.

I think we may just make this an unconditional requirement, indifferently of anchors.

[halseth]

I think we can allow spending them all in a single tx, but perhaps recommend something like "if confirmation doesn't happen within X blocks from expiry, to_local should be swept separately as it might be pinned etc".

[t-bast]

I agree with @halseth, it's worth mentioning this potential attack (good find!) but we should be more permissive with implementers.

[Roasbeef]

IMO it's worth nothing more of the subtly here. If they're pinning, then in order to even get into the mempool, they're likely paying a relatively high fee, which means that if they do confirm, then there's the CSV period on the second level. Also the defender can mix things up themselves, by only sweeping one of the HTLC outputs, which would then invalidate the entire "tree" (possibly needing to pay high enough fees?) of the attacker.

[ariard]

As usual they might pay a high fee but low-feerate, which isn't a real deterrence. Updated with the reference to CSV period. You can sweep one of the HTLC output thus invalidating the batched pinning of the attacker, but the attacker can pinning each commitment HTLC output at first broadcast as you have full-malleability for each of them. Generally I don't think we should successively "react" compared on guessed attacker moves as our transaction broadcast algorithm is public but not attacker one.

[Roasbeef]

Perhaps this should be "should" instead? As there're other ways to defend against this, for example: sweep every output in its own transaction.

[ariard]

Reworded, it's used now a MAY use a single transaction to *resolve* all the outputs which by the negative allow to sweep every output in its own transaction as first broadcast to mitigate. I kept the MUST if the confirmation isn't reached near to CSV expiration as suggested by Johan. Thus you're free to aggregate as you like until some point to save on fees but after it's a matter of security and should you care to confirm.

[ariard]

Updated following Johan suggestion, making the new requirement conditional on reaching a new security_delay, i.e "if confirmation doesn't happen before reaching X blocks from expiry". I poured a recommendation for its default value but defining a variable sounds better as its exact value can be a matter of implementation policy (and the confidence if your fee-bumping logic).

@Roasbeef I was hesitant to introduce a note on a scorched earth approach as we discussed on IRC. I'm still fearful of this being used as a double-edge by an attacker to burn more of the victim actual balance than it would loss by the punishment of its channel reserve.

[rustyrussell]

OK, lots of good discussion at meeting about this. The spec says MAY, because if you do make one bit tx, you need to split (as HTLC txs can race you), but you also need to split because your big tx might be pinned.

It's worth noting this, for sure, but I think we can be simpler; also why pick on the to-local output? In theory any output could be largest, and you want that one.

[ariard]

also why pick on the to-local output? In theory any output could be largest, and you want that one.

If "you want" designates the cheating party, it's right that you can pin any largest HTLC outputs but those ones must be spent by a 2nd-level HTLC txn sooner or latter. Thus for a profitable attack to makes sense you will target the to_local output. That said, a cheating party may take hostage HTLC outputs, according to mempool conditions so better to split-off there too. Extended wording to them.

[t-bast]

LGTM

[ariard]

Updated with nit fix.

[halseth]

ACK

[halseth]

Feels like SHOULD is more correct here, since the sweeping node is aware of the risk and should be allowed to do as it pleases.

[t-bast]

I agree, @ariard I think this is the last pending point before we merge, what do you think?

[niftynei]

We've got the ACKs on this, should we wait til this is addressed? paging @ariard

[ariard]

Sounds good to me, it's up to the implementer to gauge risk-vs-fee-saving.

[ariard]

Sorry for delay, updated at aeb19de!

[t-bast]

Merging as agreed during the last spec meeting.