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.

Sign up for GitHub

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

More conservative cltv_expiry_delta recommendations #785

Merged
merged 3 commits into from
Aug 20, 2020
Merged

More conservative cltv_expiry_delta recommendations #785

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
07c4493
More conservative cltv_expiry_delta recommendations
t-bast Jun 25, 2020
791801d
Update Bolt 11 default recommendation
t-bast Jul 7, 2020
eb946f0
fixup! Update Bolt 11 default recommendation
t-bast Jul 21, 2020
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
34 changes: 18 additions & 16 deletions 02-peer-protocol.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -740,31 +740,33 @@ the longest possible time to redeem it on-chain:
Thus, the worst case is `3R+2G+2S`, assuming `R` is at least 1. Note that the
chances of three reorganizations in which the other node wins all of them is
low for `R` of 2 or more. Since high fees are used (and HTLC spends can use
almost arbitrary fees), `S` should be small; although, given that block times are
irregular and empty blocks still occur, `S=2` should be considered a
minimum. Similarly, the grace period `G` can be low (1 or 2), as nodes are
required to timeout or fulfill as soon as possible; but if `G` is too low it increases the
risk of unnecessary channel closure due to networking delays.
almost arbitrary fees), `S` should be small during normal operation; although,
given that block times are irregular, empty blocks still occur, fees may vary
greatly, and the fees cannot be bumped on HTLC transactions, `S=12` should be
considered a minimum. `S` is also the parameter that may vary the most under
attack, so a higher value may be desirable when non-negligible amounts are at
Copy link

Choose a reason for hiding this comment

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

Note, I think you can keep this line but overall if you're a meaningful routing node an attacker can exploit by opening either 1 big channel or 10 small ones, even batching opening fees. Really most of attacks are "batchable" :/

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Of course, but that doesn't make these recommendations obsolete, on the contrary! It will need to be coupled with other techniques (e.g. limiting your exposure in the number of channels with nodes you don't trust) but in any case, you'll want high enough deltas.

risk. The grace period `G` can be low (1 or 2), as nodes are required to timeout
or fulfill as soon as possible; but if `G` is too low it increases the risk of
unnecessary channel closure due to networking delays.

There are four values that need be derived:

1. the `cltv_expiry_delta` for channels, `3R+2G+2S`: if in doubt, a
`cltv_expiry_delta` of 12 is reasonable (R=2, G=1, S=2).
`cltv_expiry_delta` of at least 34 is reasonable (R=2, G=2, S=12).

2. the deadline for offered HTLCs: the deadline after which the channel has to be failed
and timed out on-chain. This is `G` blocks after the HTLC's
`cltv_expiry`: 1 block is reasonable.
2. the deadline for offered HTLCs: the deadline after which the channel has to
be failed and timed out on-chain. This is `G` blocks after the HTLC's
`cltv_expiry`: 1 or 2 blocks is reasonable.

3. the deadline for received HTLCs this node has fulfilled: the deadline after which
the channel has to be failed and the HTLC fulfilled on-chain before its
`cltv_expiry`. See steps 4-7 above, which imply a deadline of `2R+G+S`
blocks before `cltv_expiry`: 7 blocks is reasonable.
3. the deadline for received HTLCs this node has fulfilled: the deadline after
which the channel has to be failed and the HTLC fulfilled on-chain before
its `cltv_expiry`. See steps 4-7 above, which imply a deadline of `2R+G+S`
blocks before `cltv_expiry`: 18 blocks is reasonable.

4. the minimum `cltv_expiry` accepted for terminal payments: the
worst case for the terminal node C is `2R+G+S` blocks (as, again, steps
1-3 above don't apply). The default in
[BOLT #11](11-payment-encoding.md) is 9, which is slightly more
conservative than the 7 that this calculation suggests.
1-3 above don't apply). The default in [BOLT #11](11-payment-encoding.md) is
18, which matches this calculation.
Copy link

Choose a reason for hiding this comment

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

Should we subtract one block from deadline for received HTLCs compare to minimum cltv_expiry ? You might evaluate both values with different logics/threads and in-between you may process a block thus breaking the channel without letting a bit of "block"-time for a happy resolution ?

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 think this is more an implementation concern than a spec concern, and all implementations naturally did it the other way around (sending a higher value than what the spec recommends to accommodate for such things).


#### Requirements

Expand Down
9 changes: 5 additions & 4 deletions 11-payment-encoding.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ Currently defined tagged fields are:
* `n` (19): `data_length` 53. 33-byte public key of the payee node
* `h` (23): `data_length` 52. 256-bit description of purpose of payment (SHA256). This is used to commit to an associated description that is over 639 bytes, but the transport mechanism for the description in that case is transport specific and not defined here.
* `x` (6): `data_length` variable. `expiry` time in seconds (big-endian). Default is 3600 (1 hour) if not specified.
* `c` (24): `data_length` variable. `min_final_cltv_expiry` to use for the last HTLC in the route. Default is 9 if not specified.
* `c` (24): `data_length` variable. `min_final_cltv_expiry` to use for the last HTLC in the route. Default is 18 if not specified.
Copy link
Collaborator

Choose a reason for hiding this comment

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

doesn't this need to be conditioned on the context of sender/receiver? i.e., the default is 9 when receiving but 18 when sending? it is also worth asking whether making this distinction is necessary, or just changing to always set explicit values is good enough until we remove the behavior that assumes a CLTV when none is present

Copy link
Collaborator Author

@t-bast t-bast Jul 28, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

doesn't this need to be conditioned on the context of sender/receiver?

This is what's done in eb946f0: the receiver now MUST explicitly specify this value, and the sender should use 18 if it's not specified (backwards-compatibility).

We want the value 9 to completely disappear from the spec (current implementation may be non compliant with that for the moment, but as long as they're working towards compliance in a few releases it's ok).

* `f` (9): `data_length` variable, depending on version. Fallback on-chain address: for Bitcoin, this starts with a 5-bit `version` and contains a witness program or P2PKH or P2SH address.
* `r` (3): `data_length` variable. One or more entries containing extra routing information for a private route; there may be more than one `r` field
* `pubkey` (264 bits)
Expand Down Expand Up @@ -170,11 +170,10 @@ A writer:
- MAY include one `x` field.
- if `x` is included:
- SHOULD use the minimum `data_length` possible.
- MAY include one `c` field.
- MUST include one `c` field (`min_final_cltv_expiry`).
- MUST set `c` to the minimum `cltv_expiry` it will accept for the last
HTLC in the route.
- if `c` is included:
- SHOULD use the minimum `data_length` possible.
- SHOULD use the minimum `data_length` possible.
- MAY include one `n` field. (Otherwise performing signature recovery is required)
- MUST set `n` to the public key used to create the `signature`.
- MAY include one or more `f` fields.
Expand Down Expand Up @@ -212,6 +211,8 @@ A reader:
- MUST use the `n` field to validate the signature instead of performing signature recovery.
- if there is a valid `s` field:
- MUST use that as [`payment_secret`](04-onion-routing.md#tlv_payload-payload-format)
- if the `c` field (`min_final_cltv_expiry`) is not provided:
- MUST use an expiry delta of at least 18 when making the payment
Copy link
Contributor

Choose a reason for hiding this comment

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

The writer now MUST include a min_final_cltv_expiry c field (L173), therefore the reader should reject if c is not provided.

On the one hand, this suggestion makes the spec more consistent. On the other, it may be a non-backwards compatible change. But if the two parties use different defaults payments will fail to go through anyway (if I understand correctly).

Copy link
Collaborator Author

@t-bast t-bast Sep 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

That's exactly why we kept that fallback to 18 when missing, we don't want to make a backwards-incompatible change here. 18 is bigger than the previous default, so it should work for backwards-compatibility.


### Rationale

Expand Down