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.
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 .
* 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
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>
ECDSA signatures in Bitcoin are DER-encoded but public keys are not.
The compressed format for public keys is for example standardized in
Sections 2.3.3 and 2.3.4 of
Standards for Efficient Cryptography, SEC 1: Elliptic Curve
Cryptography, Certicom Research, Version 2, 2009,
https://www.secg.org/sec1-v2.pdf
This separates out the static remotekey changes from the more ambitious
option_simplified_commitment (which also included pushme outputs and
bring-your-own-fee for HTLC outputs).
As per http://www.erisian.com.au/meetbot/lightning-dev/2019/lightning-dev.2019-09-02-20.06.html
Thanks to everyone for feedback: @araspitzu @roasbeef @bitconner
Suggested-by: @roasbeef
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
* Added descriptions of how a 2-of-2 multisignature verification is used for enforcing timelocks when timing out on-chain offered HTLCs as well as spending on-chain received HTLCs in the success case.
OP_CHECKLOCKTIMEVERIFY and OP_CSV use an inconsistent naming convention.
Update OP_CSV to match the OP_CHECKLOCKTIMEVERIFY convention as OP_CHECKSEQUENCEVERIFY.
We express it has how the outputs are ordered, but the only way you can
detect that is by the htlc_signatures order, which is the part which really
matters.
I finally reproduced this, BTW, which is why I'm digging it up!
Closes: #448
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Make it clear what kind of key we're talking about. We use the abbreviation
pubkey for public key (as it's quite common to use in field names), but
generally spell out 'private'.
(I generally prefer 'secret' to 'private' but we use private far more often
already, and we use 'secret' for things which don't directly derive keys).
Fixes: #368
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It misrenders on GitHub. Matt had a patch which changed the actual
form of the requiremnt, this uses ``` instead.
Based-on-Patch-By: Matt Corallo @TheBlueMatt
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
In this commit, we modify the cooperative closing transaction to use
version 2. Currently eclair and lnd already use version 2, while
c-lightning uses version 1. The commitment transaction already uses
version 2, so making this additional transaction (which spends the
funding output) also use version 2 would be consistent. Additionally,
as a best practice, we should be using the highest currently
defined/use transaction version.