feature: define option_zero_htlc_tx_fee (feature 22/23) #824

halseth · 2020-12-08T13:51:17Z

Extension to the anchor output format that hinders the fee siphoning attack described by @ariard in https://lists.linuxfoundation.org/pipermail/lightning-dev/2020-September/002796.html

The fix itself, making the HTLC second stage transactions zero fee, was proposed by @ariard in the same thread.

halseth · 2020-12-08T13:52:00Z

09-features.md

@@ -39,6 +39,7 @@ The Context column decodes as follows:
 | 16/17 | `basic_mpp`                      | Node can receive basic multi-part payments                | IN9      | `payment_secret`  | [BOLT #4][bolt04-mpp]                 |
 | 18/19 | `option_support_large_channel`   | Can create large channels                                 | IN       |                   | [BOLT #2](02-peer-protocol.md#the-open_channel-message) |
 | 20/21 | `option_anchor_outputs`          | Anchor outputs                                            | IN       | `option_static_remotekey` | [BOLT #3](03-transactions.md)         |
+| 22/23 | `option_zero_fee_second_stage`   | Anchor commitment type with zero fee second-stage HTLC txs| IN       | `option_anchor_outputs` | [BOLT #3](03-transactions.md)         |


We can discuss what bit to use, but to me it sounds preferable to just one-up the anchor bit.

Or, depending on how widely anchors have been rolled out (on mainnet not a lot apart from the devs I would guess) we could just say that option_anchor_outputs means also this new zero-fee requirement.

t-bast · 2020-12-08T17:10:56Z

You went through all the painful steps of #815 to find the right tx weight, just to multiply that by 0? ¯_(ツ)_/¯

I'll think about this more thoroughly, thanks for sharing! What feature bit are you currently using in lnd if we want to do some testing?

ariard

I prefer this approach over the one proposed in your 11/23 mail :)

Advantages:

it doesn't make the number of HTLCs function of channel_reserve, you might have a trusted relation with your counterparty thus not caring at all about reserve but willingly to have high HTLC-throughput
it avoids adverserial, congestioned mempools where a commitment tx with 10sat/b doesn't propagate, and confirm, even if the proposed feerate is likely fine for now
less channel liquidity provisioned as likely-not-used onchain feerate

Disadvantages:

increase the cost of any "sad path" channel closure by encumbering HTLC txn, even aggregated, with a bumping utxo spent

For a full-BYOF future, it could be super interesting to model when BYOF is a satoshi loss compare to update_fee. You should take as parameters a channel liquidity usage rate over a block period, a routing fees rate and a % of unilateral channel closure. At some high-level of mempool congestion it will be theoritically more interesting to switch back to update_fee as the bumping utxo cost you more that the marginal routing fees gain from the non-feerate committed liquidity 🤔

03-transactions.md

ariard · 2020-12-08T23:53:46Z

09-features.md

@@ -39,6 +39,7 @@ The Context column decodes as follows:
 | 16/17 | `basic_mpp`                      | Node can receive basic multi-part payments                | IN9      | `payment_secret`  | [BOLT #4][bolt04-mpp]                 |
 | 18/19 | `option_support_large_channel`   | Can create large channels                                 | IN       |                   | [BOLT #2](02-peer-protocol.md#the-open_channel-message) |
 | 20/21 | `option_anchor_outputs`          | Anchor outputs                                            | IN       | `option_static_remotekey` | [BOLT #3](03-transactions.md)         |
+| 22/23 | `option_zero_fee_second_stage`   | Anchor commitment type with zero fee second-stage HTLC txs| IN       | `option_anchor_outputs` | [BOLT #3](03-transactions.md)         |


What about adding the original vulnerability mail as a footnote, let's ease understanding of this cryptic option rational by future LN devs ?

Good idea, added!

halseth · 2020-12-09T09:32:28Z

You went through all the painful steps of #815 to find the right tx weight, just to multiply that by 0? ¯_(ツ)_/¯

@t-bast Haha. Yeah, I have to do actual weight estimation now when bumping the fees instead of relying on magic constants, so had to go through to make sure it was correct 😀

I'll think about this more thoroughly, thanks for sharing! What feature bit are you currently using in lnd if we want to do some testing?

I have a branch here if you want to test, using feature bits 22/23: lightningnetwork/lnd#4840

halseth · 2020-12-09T14:34:37Z

I prefer this approach over the one proposed in your 11/23 mail :)

@ariard I prefer this also :)

Thanks for your suggestions, I will update the PR!

The main thing to agree on at this point I think, is the feature bit. I see three avenues to take:

Amend the spec to just say anchors == zero_fee_second_stage. Since anchors haven't been widely rolled out, this should be okay. The implementations should make sure that the few, if any, channels that are already deployed will be closed or continue operate when one of the peers update (because sigs won't match).
Assign a new feature bit (as done in the current proposal as of c35e12e), and start encouraging all new channels to use this one. Feature bits are cheap (?) 😀
Assign an experimental feature bit.

I prefer 1 or 2, they should be similar in complexity. 1 is perhaps better for avoiding the spec to sprawl too much, but harder to guarantee that it won't have unintended consequences for existing deployments of anchors.

ariard · 2020-12-09T21:54:14Z

From RL-side, I favor 1., our anchor implementation doesn't bump if feerate is still gauged as fast-confirm according to the fee-estimator at first-broadcast so just have to remove this check and also it would let remove some feerate negotiation code.

I don't think anchors outputs has been rolled out on mainet beyond LND so OG anchor channels aren't that much a concern but I would let other implems confirm this point?

rustyrussell · 2020-12-10T02:03:22Z

No, you have to make this a new bit, don't roll it into anchors (though this should require anchors, ofc). Anchors is already deployed, and this will cause massive confusion. Plus if there are bugs it's easier to disable a single feature.

Secondly, please call it "option_zero_htlc_tx_fee" since the term "htlc transaction" is used in the spec.

halseth · 2020-12-10T11:56:51Z

Renamed to option_zero_htlc_tx_fee, also added some explanation and requirements to Bolt#5.

cfromknecht · 2020-12-11T02:34:25Z

ACK on the behavioral changes in this PR.

My only concern is with the interaction of the new feature bit with option_anchor_output. IMO we should structure this not as an amendment to option_anchor_outputs, it should be its own completely distinct commitment format that just happens to share a lot in common with option_anchor_outputs. For now we'll call the new format option_anchors_zero_fee_htlc.

The primary distinction is that IMO option_anchors_zero_fee_htlc should depend directly on option_static_remotekey and not option_anchor_outputs, which means that nodes aren't forced to advertise support for the broken anchor format in order to signal support for the better one. By extension, this also gives us the flexibility to completely remove option_anchor_outputs from code/spec in the future.

If we don't take this approach, consider Alice and Bob advertising the following:

Alice: option_static_remotekey, option_anchor_outputs (reject), option_anchors_zero_fee_htlc
Bob: option_static_remotekey, option_anchor_outputs (for real)

where Alice will reject any attempt to make a broken anchor channel.

When Bob is proposing, he will ignore the optional option_anchors_zero_fee_htlc and attempt to make a broken anchor channel, which Alice will reject. If Alice is proposing, she should try to make an option_anchor_outputs channel but doesn't want to. If she tries to make an option_static_remotekey channel, there will be a commitment disagreement since Bob is expecting option_anchor_outputs.

So, by forcing Alice to signal option_anchor_outputs as a dependency, there is no way for either to downgrade to option_static_remotekey which is the correct behavior based on each node's actual preferences and support.

As far as it pertains to the spec, it means we would need to update any existing references of option_anchor_outputs to option_anchor_* (aka option_anchor_outputs or option_anchor_zero_fee_htlc) and only keep the divergence as it pertains to the second-level fees.

NOTE: There is a similar discussion to be had as to whether option_anchor_zero_fee_htlc should depend on option_static_remotekey, but I think that's much less of a practical concern seeing as we aren't looking to phase it out any time soon.

EDIT: On second thought, maybe it is a good idea to make all commitment formats dependency free...

t-bast · 2020-12-14T13:51:36Z

I think @cfromknecht raises a very good point here.

If we want Alice to be able to advertize support for anchor_output_with_0_fee_htlc (and not the current anchor_outputs) and another commitment format (for example the 1.0 commitment), it's simply not possible with the current state of the PR. Alice is forced to set option_anchors_zero_fee_htlc to mandatory which prevents her from connecting to nodes that don't support any flavor of anchor outputs.

halseth · 2020-12-15T19:00:27Z

I think that makes sense. The lnd imlementation has been updated to only set this new bit, and not the "old" anchor bit. Will update this PR as well.

halseth · 2020-12-16T11:10:59Z

Updated to remove the dependency on previous feature bits, and added definition of option_anchors: https://github.com/lightningnetwork/lightning-rfc/pull/824/files#diff-c3e160eeb369e80b50a24ceee86928786ae3b67a0687a7aea80e8cefac0145daR46, which is the OR of the two anchor formats.

This is used everywhere in the spec where the two formats now should be handled equally.

halseth · 2020-12-17T10:48:31Z

If anybody wants to test, lnd has merged support for what is outlined in this PR to master.

EDIT: and it is also included with v0.12 RC1.

t-bast

ACK e4f1579.

We'll need some time to implement this in eclair and do proper cross-implementation tests, but it looks reasonable.

ariard · 2021-01-04T19:15:31Z

ACK e4f1579

Will jump directly to this new commitment transaction format for RL.

ysangkok · 2021-02-02T21:06:28Z

02-peer-protocol.md

@@ -378,10 +378,14 @@ This message introduces the `channel_id` to identify the channel. It's derived f
 #### Requirements

 Both peers:
-  - if `option_static_remotekey` or `option_anchor_outputs` was negotiated:
-    - `option_static_remotekey` or `option_anchor_outputs` applies to all commitment transactions
+  - if `option_static_remotekey`, `option_anchor_outputs` or


since option_anchors has been added to cover _outputs and _zero_fee_htlc_tx, why is it not used here?

Worse, you don't define what the effect is if both are negotiated!

- if `option_anchors_zero_fee_htlc_tx` was negotiated: - `option_anchor_outputs` is ignored. ... Rationale: `option_anchors_zero_fee_htlc_tx` is considered superior to `option_anchor_outputs`, so is favored if both are negotiated.

If both are negotiated, then both can apply which I don't think is a problem since the latter is a superset of features.

I added a rationale and also specified option_anchors the places where option_static_remotekey defines behavior (since technically option_anchors_zero_fee_htlc_tx does no longer depend on option_static_remotekey): 75a4cde

rustyrussell

Ack with minor feedback on details.

rustyrussell · 2021-02-22T00:28:18Z

09-features.md

@@ -39,6 +39,11 @@ The Context column decodes as follows:
 | 16/17 | `basic_mpp`                      | Node can receive basic multi-part payments                | IN9      | `payment_secret`  | [BOLT #4][bolt04-mpp]                 |
 | 18/19 | `option_support_large_channel`   | Can create large channels                                 | IN       |                   | [BOLT #2](02-peer-protocol.md#the-open_channel-message) |
 | 20/21 | `option_anchor_outputs`          | Anchor outputs                                            | IN       | `option_static_remotekey` | [BOLT #3](03-transactions.md)         |
+| 22/23 | `option_anchors_zero_fee_htlc_tx` | Anchor commitment type with zero fee HTLC txs             | IN       |                   | [BOLT #3][bolt03-htlc-tx], [lightning-dev][ml-zero-fee-vuln]|


Depends on option_static_remotekey, please.

I guess this was deliberate because of #824 (comment) :

On second thought, maybe it is a good idea to make all commitment formats dependency free...

No, it's a terrible idea because it blows up the test matrix. This without static_remotekey is unsupported, just make that clear in the spec and we can all sleep easy.

rustyrussell · 2021-02-22T00:32:24Z

02-peer-protocol.md

@@ -378,10 +378,14 @@ This message introduces the `channel_id` to identify the channel. It's derived f
 #### Requirements

 Both peers:
-  - if `option_static_remotekey` or `option_anchor_outputs` was negotiated:
-    - `option_static_remotekey` or `option_anchor_outputs` applies to all commitment transactions
+  - if `option_static_remotekey`, `option_anchor_outputs` or


Worse, you don't define what the effect is if both are negotiated!

- if `option_anchors_zero_fee_htlc_tx` was negotiated: - `option_anchor_outputs` is ignored. ... Rationale: `option_anchors_zero_fee_htlc_tx` is considered superior to `option_anchor_outputs`, so is favored if both are negotiated.

Roasbeef · 2021-08-30T19:38:48Z

I realize now this never was merged in, lnd has been using this feature bit for the last major version (0.13) in the wild. Have other implementations shipped a release that uses this new bit?

t-bast · 2021-08-30T19:59:06Z

Not yet for eclair, but it is on the short term todo list ;)

ariard

See the IRC point on clarifying trimmed HTLC outputs must be computed only against dust_limit_satoshis, HTLC-timeout/HTLC-success fees are 0 with option_zero_htlc_tx_fee.

ariard · 2021-08-30T20:11:41Z

02-peer-protocol.md

+
+`option_anchors_zero_fee_htlc_tx` is considered superior to
+`option_anchor_outputs`, which again is considered superior to
+`option_static_remotekey`, and the superior one is is favored if more than one


nit: s/is//g

ariard · 2021-08-30T20:21:37Z

05-onchain.md

+  2. if `option_anchors_zero_fee_htlc_tx` applies:
+    - MUST combine it with inputs contributing sufficient fee to ensure timely
+      inclusion in a block.
+    - MAY combine it with other transactions.


This MAY is really unsafe, if the combined-with transaction includes an input from a third-party transaction you might have feerate manipulation (like big witness data). Or same if it pays to a third-party (pinning the first-broadcast HTLC-txn and blocking any replacement)

Roasbeef · 2021-08-30T20:47:00Z

09-features.md

@@ -41,6 +41,10 @@ The Context column decodes as follows:
 | 20/21 | `option_anchor_outputs`          | Anchor outputs                                            | IN       | `option_static_remotekey` | [BOLT #3](03-transactions.md)         |
 | 22/23 | `option_anchors_zero_fee_htlc_tx` | Anchor commitment type with zero fee HTLC transactions   | IN       |                   | [BOLT #3][bolt03-htlc-tx], [lightning-dev][ml-sighash-single-harmful]|

+## Definitions
+
+We define `option_anchors` as `option_anchor_outputs || option_anchors_zero_fee_htlc_tx`.


Commenting here that we want the table row above to explicitly add the static key dependancy.

Roasbeef · 2021-08-30T20:49:50Z

Will squash this and have it land now, then make a smol follow up PR with the additions.

t-bast · 2021-08-30T20:59:55Z

@halseth enjoy a glass of champagne, it eventually landed! 😆

Add support for lightning/bolts#824 When the channel type is anchor outputs with zero fee htlc txs, we set the fees for the htlc txs to 0. An important side-effect is that it changes the trimmed to dust calculation, and outputs that were previously dust can now be included in the commit tx.

halseth commented Dec 8, 2020

View reviewed changes

t-bast mentioned this pull request Dec 8, 2020

Lightning Specification Meeting 2020/12/07 #821

Closed

14 tasks

ariard reviewed Dec 8, 2020

View reviewed changes

halseth changed the title ~~feature: define option_zero_fee_second_stage~~ feature: define option_zero_htlc_tx_fee Dec 10, 2020

halseth mentioned this pull request Dec 10, 2020

[anchors] zero-fee HTLC secondlevel transactions lightningnetwork/lnd#4840

Merged

1 task

t-bast mentioned this pull request Dec 14, 2020

Lightning Specification Meeting 2020/12/21 #827

Closed

18 tasks

t-bast reviewed Dec 21, 2020

View reviewed changes

This was referenced Dec 22, 2020

BOLT 2: option_shutdown_anysegwit (Feature 26/27) #672

Merged

bolt-09: reserve feature bits for option_anchors_zero_fee_htlc_tx #828

Merged

t-bast mentioned this pull request Jan 11, 2021

Lightning Specification Meeting 2021/01/18 #831

Closed

17 tasks

t-bast mentioned this pull request Jan 27, 2021

Lightning Specification Meeting 2021/02/01 #838

Closed

21 tasks

ysangkok suggested changes Feb 2, 2021

View reviewed changes

t-bast mentioned this pull request Feb 9, 2021

Lightning Specification Meeting 2021/02/15 #840

Closed

23 tasks

rustyrussell approved these changes Feb 22, 2021

View reviewed changes

feature: define option_zero_fee_second_stage

77dfee2

halseth force-pushed the zero-fee-second-stage branch from e4f1579 to 77dfee2 Compare February 25, 2021 14:19

fixup! feature: define option_zero_fee_second_stage

75a4cde

rustyrussell changed the title ~~feature: define option_zero_htlc_tx_fee~~ feature: define option_zero_htlc_tx_fee (feature 22/23) Mar 1, 2021

rustyrussell mentioned this pull request Mar 1, 2021

Public key-based routing (Feature **NEEDS NUMBER***) #814

Open

Roasbeef mentioned this pull request Aug 30, 2021

BOLT 2: send channel_type in open/accept #880

Merged

ariard reviewed Aug 30, 2021

View reviewed changes

Roasbeef reviewed Aug 30, 2021

View reviewed changes

Roasbeef merged commit fdc078f into lightning:master Aug 30, 2021

t-bast mentioned this pull request Sep 1, 2021

Implement anchor outputs zero fee htlc txs ACINQ/eclair#1932

Merged

bitromortac mentioned this pull request Sep 27, 2021

Anchor commitments, new payment basepoint spesmilo/electrum#7509

Open

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: define option_zero_htlc_tx_fee (feature 22/23) #824

feature: define option_zero_htlc_tx_fee (feature 22/23) #824

halseth commented Dec 8, 2020 •

edited

Loading

halseth Dec 8, 2020

halseth Dec 9, 2020

t-bast commented Dec 8, 2020

ariard left a comment •

edited

Loading

ariard Dec 8, 2020

halseth Dec 10, 2020

halseth commented Dec 9, 2020

halseth commented Dec 9, 2020

ariard commented Dec 9, 2020

rustyrussell commented Dec 10, 2020

halseth commented Dec 10, 2020

cfromknecht commented Dec 11, 2020 •

edited

Loading

t-bast commented Dec 14, 2020

halseth commented Dec 15, 2020

halseth commented Dec 16, 2020

halseth commented Dec 17, 2020 •

edited

Loading

t-bast left a comment

ariard commented Jan 4, 2021

ysangkok Feb 2, 2021

rustyrussell Feb 22, 2021

halseth Feb 25, 2021

rustyrussell left a comment

rustyrussell Feb 22, 2021

SomberNight Feb 22, 2021 •

edited

Loading

rustyrussell May 7, 2021 •

edited

Loading

rustyrussell Feb 22, 2021

Roasbeef commented Aug 30, 2021

t-bast commented Aug 30, 2021

ariard left a comment

ariard Aug 30, 2021

ariard Aug 30, 2021

Roasbeef Aug 30, 2021

Roasbeef commented Aug 30, 2021

t-bast commented Aug 30, 2021

feature: define option_zero_htlc_tx_fee (feature 22/23) #824

feature: define option_zero_htlc_tx_fee (feature 22/23) #824

Conversation

halseth commented Dec 8, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

t-bast commented Dec 8, 2020

ariard left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

halseth commented Dec 9, 2020

halseth commented Dec 9, 2020

ariard commented Dec 9, 2020

rustyrussell commented Dec 10, 2020

halseth commented Dec 10, 2020

cfromknecht commented Dec 11, 2020 • edited Loading

t-bast commented Dec 14, 2020

halseth commented Dec 15, 2020

halseth commented Dec 16, 2020

halseth commented Dec 17, 2020 • edited Loading

t-bast left a comment

Choose a reason for hiding this comment

ariard commented Jan 4, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rustyrussell left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SomberNight Feb 22, 2021 • edited Loading

Choose a reason for hiding this comment

rustyrussell May 7, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Roasbeef commented Aug 30, 2021

t-bast commented Aug 30, 2021

ariard left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Roasbeef commented Aug 30, 2021

t-bast commented Aug 30, 2021

halseth commented Dec 8, 2020 •

edited

Loading

ariard left a comment •

edited

Loading

cfromknecht commented Dec 11, 2020 •

edited

Loading

halseth commented Dec 17, 2020 •

edited

Loading

SomberNight Feb 22, 2021 •

edited

Loading

rustyrussell May 7, 2021 •

edited

Loading