This commit is intended to fix an ambiguity in the spec that led to a
divergence in the sorting tie breaker between implementations, that can
lead to force closed transaction in practice. BIP 69 operates on the
output level, therefore it examines the _satoshi_ amount of a output
when sorting. The spec however, references BIP 69, but states that an
"identical" HTLC output may have the same `amount_msat` value.
In the wild this led to some implementations checking the _sat_ value of
an HTLC while others checked the _msat_ value. In the scenario where an
pair HTLC has the same _sat_ value, but differing _msat_ values, then
one will fall through to the tie-breaker, while the other while sort
them according to their _msat_ values.
In this commit, we attempt to make this requirement more explicit by
removing the reference to `msat`, and more explicitly describing when an
HTLC pair is to be considered identical.
This is the only one, so I simply removed it. We'd notice if a new field
was introduced which didn't change the output these days, but this has been
here since 2017.
Here's the difference in extract-formats.py's output:
```diff
@@ -177,6 +177,9 @@
msgtype,final_incorrect_htlc_amount,19
msgdata,final_incorrect_htlc_amount,incoming_htlc_amt,u64,
msgtype,channel_disabled,UPDATE|20
+msgdata,channel_disabled,flags,u16,
+msgdata,channel_disabled,len,u16,
+msgdata,channel_disabled,channel_update,byte,len
msgtype,expiry_too_far,21
msgtype,invalid_onion_payload,PERM|22
msgdata,invalid_onion_payload,type,bigsize,
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Add a serialized transactions test vector for the edge case of sorting htlc-timeout-tx
when there are multiple offered htlc with the same amount and preimage.
The test vector reuses previous preimages and creates a case scenario with 1 received htlc
and 2 offered, the two offered will have same scriptPubKey and redeemScript, but different CLTV value.
It is asserted the order in which the htlc transactions should be kept internally
and we assume the same order is used to construct the commitment_signed message.
This complements #491 .
Judging from the comment
https://github.com/bitcoin/bitcoin/pull/18267/files#r491150895 in the
Signet PR all test networks should have the same bech32_hrp prefix (even
regtest). That's why 'tb' was chosen for Signet as well.
This is not optimal for LN as invoices shouldn't be vague in
what network they were issued for.
Therefore we add the explicit prefix 'lntbs' for Signet invoices.
Routing nodes have an incentive to use low fees when opening channels to
ensure their activity is economically viable.
However, when a funding transaction takes too long to confirm, the fundee
may have forgotten the channel. In that case the funder is forced to
broadcast the first commit tx to get his funds back and then open a new
channel, which is costly.
We can avoid this issue by simply knowing how long the fundee will wait,
and ensuring the funding tx confirms before that period ends. We set this
timeout to 2016 blocks (2 weeks).
Some ChaCha20 implementations API's support both 64- and 96-bit nonces, while
others only support a single one.
Functionally, both nonce sizes are equivalent for LN usage, since the
nonce is always zeroed. However, while evaluating spec compliance of
ChaCha20 libraries, the fact that some do not support the 8 byte nonce
variant prompted a closer investigation about the nonce requirement.
Since RFC8439 is the one linked to in the current BOLT0004 spec and that
RFC only specifies the 96-bit nonce variant, that requirement is made
more explicit by this commit.
We previously insisted that `reply_channel_range` messages were not
overlapping: blocks content could not be split across multiple messages.
This made it possible to implicitly figure out when sync was complete, so we
re-purposed the previous `complete` field to a `full_information` field.
We now revert that change to allow blocks to be split across multiple
messages. An explicit flag is thus needed to signal that sync is complete.
Fixes#804
* BOLT#3: use 4 bytes for cltv_expiry in accepted_htlc_script
* BOLT#3: correct success_witness size
* BOLT#3: note HTLC tx weights differ a bit from actual weights
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.
The existing requirements were not specifying the case where both a
`commitment_signed` and `revoke_and_ack` need to be retransmitted.
This is an important case to specify because if the relative order is not
preserved, the channel will close.
Fixes#794
This script was previously used in Electrum but no more.
So since this script doesn't even parse the output of
extract-formats.py any more, just remove it.
Many channels use a value below 6, which is really insecure (there are
more than 2k such channels on mainnet).
While less risky, there are more than 7k channels with a value below 12.
This indicates that the spec should probably make the risks a bit more
clear to help guide node operators.
This commit extends the specification with a new commitment format that
adds two anchor outputs to the commitment transaction. Anchor outputs
are a safety feature that allows a channel party to unilaterally increase
the fee of the commitment transaction using CPFP and ensure timely
confirmation on the chain. There is no cooperation required from the
remote party.
It turns out everyone does `P[B / 8] ^= (1 << (P % 8))`,
which is not what the spec says to do (it implies you
would treat P as a bitstring numbered 255 to 0).
See this stackoverflow question:
https://stackoverflow.com/questions/49928131/lightning-secret-generation-from-seed
Reported-by: Janus Troelsen @ysangkok (on Twitter)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
* Rename all the 'varint' to 'bigsize'.
Having both is confusing; we chose the name bigsize, so use it
explicitly.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
* BOLT 7: use `byte` instead of `u8`.
`u8` isn't a type; see BOLT #1 "Fundamental Types".
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
* BOLT 1: promote bigsize to a Fundamental Type.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We added a requirement on the writer, not the reader. We can't really add
a test vector without a new requirement, though.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
One for uppercase, and one with should-be-ignored fields.
The first of these addresses #659 (#677 directly changes the text
to make it clear this is allowed, and should also be applied).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The introductory paragraph describes node discovery and channel
discovery, but changes the ordering. Keep the same ordering throughout
the paragraph for readability.
Add an additional "reserve" for funders on top of the real reserve to
avoid getting in a state where the channel is unusable because
of the increased commit tx cost of a new HTLC.
Requirements are only added for the funder sending an HTLC.
Fundee receiving HTLCs may choose to verify that funders apply
this, but it may lead to an unusable UX.
Fixes#728.
The spec already prepared a hook to add additional information to existing
messages (additional bytes at the end of a message must be ignored).
Since we're using TLV in many places, it makes sense to use that optional
additional space at the end of each message to allow an optional tlv stream.
This requires making a few previously optional fields mandatory:
- channel_reestablish commitment points: it makes sense to always include those
regardless of whether `option_dataloss_protect` or `option_static_remotekey` are set.
- option_upfront_shutdown_script: if you're not using one, just set the length to 0.
That field is moved to a TLV record because luckily, the resulting bytes are the same.
This provides more flexibility to later remove the requirement of making this field mandatory.
No need to change the `channel_update`'s `htlc_maximum_msat` because
the `message_flags` encode its presence/absence.
It can still be either included or omitted without causing issues to the extension stream.
* Rename `complete` flag to `full_information` and provide rationale
This was confusing: the flag name made implementers *think* they
knew what it was for.
* Insist that reply_channel_range be adjacent and ordered
The current loose constraints causes confusion (and now all major
implementations meet the stricter requirements anyway).
You are allowed to provide more blocks than requested, but you have
to be complete and in order, and each reply has to have some overlap
with the requested range.
