The only surprise here (maybe?) is that we use the commitment number encoding.
I think that makes sense, but it was unspecified before.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This commit attempts to clarify some ambiguity in the way the
revocation key derivation was formerly described. Rather than framing
the description in terms of local vs remote nodes, we instead frame the
description around the _process_ of creating a new commitment
transaction for a remote node,
[ minor typos and remove weird part-sentence -- RR ]
Plus a few more missing ones, and some consistency fixes in names
as pointed out by Roasbeed and Fabrice.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
As per ElementsProject/lightning#134 Fabrice points out that to calculate
our own commitment tx, we use the *other* side's revocation basepoint:
The revocationkey is a blinded key: the remote node provides the base,
and the local node provides the blinding factor which it later
reveals
Thus, we fix the test vectors by renaming "local_revocation_basepoint"
to "remote_revocation_basepoint", which is what we should be using.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We didn't update it for 394da29189
"BOLT 3: fix test vector derivation."
This doesn't actually change any results, just fixes an intermediate
calculation.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This means more bytes if we need to create a penalty tx, but less for
normal operation:
Witness script for offered htlcs: 139 bytes -> 133 bytes.
Witness script for accepted htlcs: 156 bytes -> 139 bytes.
It's also a little simpler; it's just an OP_IF around the old scripts
to test for the revocation key being the top arg.
Suggested-by: Joseph Poon <joseph@lightning.network>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The weights of HTLCs were corrected by Pierre in "fixed htlc weight
calculation": this adjusts the test vectors to match.
This also means that the feerate thresholds change.
In addition, this fixes feerate on "maximum feerate" tests,
and corrects the fee for the htlc-timeout tx.
Reported-by: Fabrice Drouin <fabrice.drouin@acinq.fr>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The test vectors themselves were fine, but they're supposed to be derived
from known basepoints (and I was actually testing this with some new code).
I accidentally used the *remote* per-commitment-point, instead of the *local*
per-commitment-point to derive the remotekey/remote_privkey; since we are
generating the local transaction, this is wrong. We don't need to know
the remote per-commitment-point at all, in fact.
Thus, the remotekey (and signatures) in the test vectors change.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is a smerge of Fabrice Drouin and my work: he update them, I updated them and enhanced them, he found bugs, I re-derived them, etc.
- rename keys properly
- fix fees
- sign all transactions
- Derive test transaction vectors from minimal subset of parameters.
- rewrite test vectors to test edge cases for trimming.
- add funding tx test vector, make testable on regtest.
This adds a funding transaction test (really, we only specify the output), but importantly adds enough information to duplicate a blockchain with that particular funding tx:
$ rm -rf ~/.bitcoin/regtest/
$ bitcoind -regtest=1 &
$ bitcoin-cli -regtest=1 submitblock 0000002006226e46111a0b59caaf126043eb5bbf28c34f3a5e332a1fc7b2b73cf188910fadbb20ea41a8423ea937e76e8151636bf6093b70eaff942930d20576600521fdc30f9858ffff7f20000000000101000000010000000000000000000000000000000000000000000000000000000000000000ffffffff03510101ffffffff0100f2052a010000001976a9143ca33c2e4446f4a305f23c80df8ad1afdcf652f988ac00000000
$ bitcoin-cli -regtest=1 generate 431
$ bitcoin-cli -regtest=1 sendrawtransaction 0200000001adbb20ea41a8423ea937e76e8151636bf6093b70eaff942930d20576600521fd000000006b48304502210090587b6201e166ad6af0227d3036a9454223d49a1f11839c1a362184340ef0240220577f7cd5cca78719405cbf1de7414ac027f0239ef6e214c90fcaab0454d84b3b012103535b32d5eb0a6ed0982a0479bbadc9868d9836f6ba94dd5a63be16d875069184ffffffff028096980000000000220020c015c4a6be010e21657068fc2e6a9d02b27ebe4d490a25846f7237f104d1a3cd20256d29010000001600143ca33c2e4446f4a305f23c80df8ad1afdcf652f900000000
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Not only is it shorter and cheaper, but the rest of the document (including
test vectors and weight calculation) assumed it already.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
* I add the term "trimmed outputs" for sub-dust outputs; this matters
for both fee calculation and transaction construction.
* Introduce the concept of "base fee": this is what needs to be
extracted from the funder.
* The requirements are spread between the different sections, let's
tie it together in a new section at the end.
* Spell out all the steps in the example which calculates fees.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The most significant byte of the locktime in a commitment transaction
must be set to 0x20. This is to make sure that the locktime value
is always higher than 500,000,000, making it interpreted as a Unix
epoch timestamp, and not a block height. It also makes sure that the
locktime is below the current time, allowing the commitment transaction
to be included in a block.
Since the sequence field in the input of the commitment transaction is
used for the other half of the obscured commitment transaction number,
it will never assume the maxInt value (0xFFFFFFFF) which would disable
locktime checking.
From BIP 68:
If bit (1 << 31) of the sequence number is set, then no consensus
meaning is applied to the sequence number and can be included in any
block under all currently possible circumstances.
Which is what we want.
Reported-by: Olaoluwa Osuntokun <laolu32@gmail.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Now using `<>` on inline code only when part of a script and extra
emphasis is warranted. Outside of scripts the variable names are
already highlighted, so no need to add further emphasis.
