Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to sign commitments at various feerates (FEAT 32/33) #1036

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 84 additions & 26 deletions 02-peer-protocol.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ operation, and closing.
* [Committing Updates So Far: `commitment_signed`](#committing-updates-so-far-commitment_signed)
* [Completing the Transition to the Updated State: `revoke_and_ack`](#completing-the-transition-to-the-updated-state-revoke_and_ack)
* [Updating Fees: `update_fee`](#updating-fees-update_fee)
* [Signing alternative feerates](#signing-alternative-feerates)
* [Message Retransmission: `channel_reestablish` message](#message-retransmission)
* [Authors](#authors)

Expand Down Expand Up @@ -783,20 +784,25 @@ Once both nodes have exchanged `channel_ready` (and optionally [`announcement_si
Changes are sent in batches: one or more `update_` messages are sent before a
`commitment_signed` message, as in the following diagram:

+-------+ +-------+
| |--(1)---- update_add_htlc ---->| |
| |--(2)---- update_add_htlc ---->| |
| |<-(3)---- update_add_htlc -----| |
| | | |
| |--(4)--- commitment_signed --->| |
| A |<-(5)---- revoke_and_ack ------| B |
| | | |
| |<-(6)--- commitment_signed ----| |
| |--(7)---- revoke_and_ack ----->| |
| | | |
| |--(8)--- commitment_signed --->| |
| |<-(9)---- revoke_and_ack ------| |
+-------+ +-------+
+-------+ +-------+
| |--(1)---- update_add_htlc ---->| |
| |--(2)---- update_add_htlc ---->| |
| |<-(3)---- update_add_htlc -----| |
| | | |
| |--(4)--- commitment_signed --->| |
| |--(4a)--- alternative_commitment_signed --->| |
| |--(4b)--- alternative_commitment_signed --->| |
| A |<-(5)---- revoke_and_ack ------| B |
| | | |
| |<-(6)--- commitment_signed ----| |
| |<-(6a)--- alternative_commitment_signed ----| |
| |<-(6b)--- alternative_commitment_signed ----| |
| |<-(6c)--- alternative_commitment_signed ----| |
| |--(7)---- revoke_and_ack ----->| |
| | | |
| |--(8)--- commitment_signed --->| |
| |<-(9)---- revoke_and_ack ------| |
+-------+ +-------+

Counter-intuitively, these updates apply to the *other node's*
commitment transaction; the node only adds those updates to its own
Expand Down Expand Up @@ -1183,42 +1189,53 @@ sign the resulting transaction (as defined in [BOLT #3](03-transactions.md)), an
* [`u16`:`num_htlcs`]
* [`num_htlcs*signature`:`htlc_signature`]

1. `tlv_stream`: `commitment_signed_tlvs`
2. types:
1. type: 1 (`alternative_feerates`)
2. data:
* [`...*u32`:`feerates_per_kw`]

#### Requirements

A sending node:
- MUST NOT send a `commitment_signed` message that does not include any
updates.
- MAY send a `commitment_signed` message that only
alters the fee.
- MAY send a `commitment_signed` message that doesn't
change the commitment transaction aside from the new revocation number
(due to dust, identical HTLC replacement, or insignificant or multiple
fee changes).
updates.
- MAY send a `commitment_signed` message that only alters the fee.
- MAY send a `commitment_signed` message that doesn't change the
commitment transaction aside from the new revocation number (due to dust,
identical HTLC replacement, or insignificant or multiple fee changes).
- MUST include one `htlc_signature` for every HTLC transaction corresponding
to the ordering of the commitment transaction (see [BOLT #3](03-transactions.md#transaction-input-and-output-ordering)).
- If `option_anchors` and `option_alternative_feerates` were negotiated:
- MUST send `alternative_feerates` containing other feerates for which it
will send signatures.
- MUST send one `alternative_commitment_signed` for each of those feerates.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find it weird that Alice decides what feerates will be available to Bob in Bob's commitment tx, and vice-versa. Ideally it would be Alice who can decide for herself that she wants feerates [3,10,30,100,300] (sat/vbyte). What if Alice sends some nice rates to Bob but Bob does not give any alternatives to Alice (alternative_feerates=[])? Is Alice supposed to force-close because of not having enough feerate options?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, that's one of the decisions I wanted to have on this PR, thanks for raising it 😉. What we do here depends on whether we do "broadcaster pays" or not and whether we re-sign commitment transactions or not.

One issue with Alice deciding what feerates Bob should sign for her commit transaction is that she ask Bob to pay a feerate so high that HTLCs all go to dust, and then she could broadcast the corresponding commitment once it's revoked and Bob won't be able to claim anything. Similarly to what we have with update_fee, Bob shouldn't sign feerates that it feels are absurdly high. But we can probably find a good middle ground that works well enough in practice.

- SHOULD include feerates that are higher than the current commitment feerate.
- if it has not recently received a message from the remote node:
- SHOULD use `ping` and await the reply `pong` before sending `commitment_signed`.
- SHOULD use `ping` and await the reply `pong` before sending `commitment_signed`.

A receiving node:
- once all pending updates are applied:
- if `signature` is not valid for its local commitment transaction OR non-compliant with LOW-S-standard rule <sup>[LOWS](https://github.com/bitcoin/bitcoin/pull/6769)</sup>:
- MUST send a `warning` and close the connection, or send an
`error` and fail the channel.
- if `num_htlcs` is not equal to the number of HTLC outputs in the local
commitment transaction:
commitment transaction:
- MUST send a `warning` and close the connection, or send an
`error` and fail the channel.
- if any `htlc_signature` is not valid for the corresponding HTLC transaction OR non-compliant with LOW-S-standard rule <sup>[LOWS](https://github.com/bitcoin/bitcoin/pull/6769)</sup>:
- MUST send a `warning` and close the connection, or send an
`error` and fail the channel.
- MUST respond with a `revoke_and_ack` message.
- If the message contains `alternative_feerates`:
- MUST wait for the corresponding `alternative_commitment_signed`
before responding with a `revoke_and_ack` message.
- Otherwise:
- MUST respond with a `revoke_and_ack` message.

#### Rationale

There's little point offering spam updates: it implies a bug.

The `num_htlcs` field is redundant, but makes the packet length check fully self-contained.

The recommendation to require recent messages recognizes the reality
that networks are unreliable: nodes might not realize their peers are
offline until after sending `commitment_signed`. Once
Expand Down Expand Up @@ -1342,6 +1359,47 @@ it's simplest to only allow it to set fee levels; however, as the same
fee rate applies to HTLC transactions, the receiving node must also
care about the reasonableness of the fee.

### Signing alternative feerates

When a node sends `commitment_signed`, it may also sign alternative versions
of the commitment using different feerates.

1. type: 137 (`alternative_commitment_signed`)
2. data:
* [`channel_id`:`channel_id`]
* [`u32`:`feerate_per_kw`]
* [`signature`:`signature`]
* [`u16`:`num_htlcs`]
* [`num_htlcs*signature`:`htlc_signature`]

#### Requirements

A sending node:
- If `option_alternative_feerates` was negotiated:
- MUST send one `alternative_commitment_signed` for each feerate listed
in the corresponding `commitment_signed.alternative_feerates`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that HTLC-timeout transactions are CLTV locked (with independent CLTVs between htlcs). A CLTV might be hundreds of blocks into the future. Broadcasting a commitment tx at alternative_feerate1 locks in that same feerate for all HTLCs too. (because the sighash in the htlc tx's input commits to the txid of the parent commit tx)

Unless we make it quadratic and pre-sign htlc txs at each alternative_feerate for commit txs at each alternative_feerate. That would mean creating num_htlcs * len(alternative_feerates)**2 signatures at every committed channel state update.

Copy link
Collaborator Author

@t-bast t-bast Oct 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very good remark. I went back and forth on that, the other option I had in mind that doesn't have this issue is to only sign alternative HTLC txs and not the commit tx. We keep a single commit tx (the one signed in commitment_signed, which will likely need some CPFP using the anchor) and sign alternative HTLC txs for each pending HTLC (and all these alternatives would spend the same output of the commit tx).

That means we need to handle the case where an HTLC tx doesn't make sense to be signed at some feerates (because its value goes entirely to fees): there are different ways of doing that (either skip them entirely or provide a dummy signature like 0x00...00), it's not necessarily hard but is a bit messy.

Another option is to double the number of signatures (instead of quadratically increasing them), by providing for each alternative_commitment one HTLC signature with sighash_all at the given feerate and one with sighash_single | sighash_anyonecanpay. But I'm not a big fan...

- MUST use `SIGHASH_ALL` for the `htlc_signature`s.
Copy link
Contributor

@SomberNight SomberNight Oct 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this help with the HTLC txs? You still need to bring your own utxo

No you won't need to, sorry if that was unclear, but the HTLC txs from alternative_commitment_signed use SIGHASH_ALL, not SIGHASH_SINGLE | SIGHASH_ANYONECANPAY, and they do pay the feerate announced.

I see. The fee part was not clear to me from the text. I guess I could have deduced it from the using sighash_all requirement. I think it would be better to explicitly mention this, e.g.:

Suggested change
- MUST use `SIGHASH_ALL` for the `htlc_signature`s.
- MUST use `SIGHASH_ALL` for the `htlc_signature`s, and apply `feerate_per_kw` for each HTLC transaction even if `option_anchors_zero_fee_htlc_tx` applies to the channel.

(to counterbalance bolt-03 being very specific about the contrary)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes definitely, if there is generally a good feeling about the proposal, I'll make that PR much more detailed, since there are quite a few subtleties that are currently hard to grasp.

- Otherwise:
- MUST NOT send `alternative_commitment_signed`.

A receiving node:
- MUST validate signatures using the same requirements as `commitment_signed`.
- If this is the last expected `alternative_commitment_signed`:
- MUST respond with a `revoke_and_ack` message.

#### Rationale

When using `option_anchors`, nodes must keep a pool of utxos available
to set the fees of commitment and HTLC transactions at broadcast time.
This is a complex task that requires sophisticated utxo management to
efficiently protect against malicious peers.

The `option_alternative_feerates` feature reduces that complexity, by
making it possible for nodes to have versions of HTLC transactions at
various feerates, which may remove the need to add external inputs in
most cases and keep them for exceptional occasions, at the expense of
slightly more latency, bandwidth and storage usage.

## Message Retransmission

Because communication transports are unreliable, and may need to be
Expand Down
2 changes: 2 additions & 0 deletions 09-features.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ 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 | `option_static_remotekey` | [BOLT #3][bolt03-htlc-tx], [lightning-dev][ml-sighash-single-harmful]|
| 26/27 | `option_shutdown_anysegwit` | Future segwit versions allowed in `shutdown` | IN | | [BOLT #2][bolt02-shutdown] |
| 32/33 | `option_alternative_feerates` | Node can sign transactions at multiple feerates | IN | | [BOLT #2](bolt02-alternative-feerates) |
| 44/45 | `option_channel_type` | Node supports the `channel_type` field in open/accept | IN | | [BOLT #2](02-peer-protocol.md#the-open_channel-message) |
| 46/47 | `option_scid_alias` | Supply channel aliases for routing | IN | | [BOLT #2][bolt02-channel-ready] |
| 48/49 | `option_payment_metadata` | Payment metadata in tlv record | 9 | | [BOLT #11](11-payment-encoding.md#tagged-fields)
Expand Down Expand Up @@ -96,6 +97,7 @@ This work is licensed under a [Creative Commons Attribution 4.0 International Li
[bolt03-htlc-tx]: 03-transactions.md#htlc-timeout-and-htlc-success-transactions
[bolt02-shutdown]: 02-peer-protocol.md#closing-initiation-shutdown
[bolt02-channel-ready]: 02-peer-protocol.md#the-channel_ready-message
[bolt02-alternative-feerates]: 02-peer-protocol.md#signing-alternative-feerates
[bolt04]: 04-onion-routing.md
[bolt07-sync]: 07-routing-gossip.md#initial-sync
[bolt07-query]: 07-routing-gossip.md#query-messages
Expand Down