This commit fixes an advisory error in the current spec draft. We
currently use `fee-per-kw` where `kw = 1000` weight to determine the
proper fee to pay for commitment transactions. Currently, the spec
advises implementer to take the typical sat/Kb at _multiply_ by 4. This
will result in implementations overpaying for commitment transactions
as the scaling should actually be in the _opposite_ direction. As the
weight is scaled up by 4, for fee-per-kw should be scaled down by 4.
So: sat/Kb * 1/4, instead of sat/Kb * 4.
[Minor fixup: "1/4th" to "1/4", better english, and doesn't trip spellcheck. -- 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>
1. Only one per-hop thing, called `per-hop`, or `hops_data` when in aggregate.
2. Move HMAC to the end of stuff it covers, both of the packet itself, and the per-hop.
3. Use `channel-id` instead of RIPEMD(nodepubkey).
4. Use 4 byte amounts.
5. This is all for realm "0", we can have future realms. We also have 16
bytes of unused padding.
6. No longer need the `gamma` key, but document the `_um_` key used for
errors.
7. Use normal 32-byte HMAC, not truncated 20-bytes, which more than eats
up the room we saved.
The result is that the onion is now 1366 not 1254 bytes, but simpler.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
`channel-id` which identifies , which is derived
=>
`channel-id` to identify the channel, which is derived
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It was never mentioned, but fulfilling a timedout HTLC means a race
between timing out and fulfilling, which is bad.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This commit adds a `chain-hash` field to message that commences a
funding workflow. This field is used to specify a _target_ chain for the
proposed channel. In order to uniquely identity blockchains in a manner
that doesn't require strict coordination between developers, the genesis
hash of the target chain is used. For channels opened on the Bitcoin
blockchain, the `chain-hash` field should _always_ be set to:
000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f.
Introducing this new field _immediately_ allows nodes within the network
to open a channel within any Satoshi-derived blockchain. Nodes can have
channels open across different blockchains globally, but also have many
channels open across distinct blockchains with the same peer.
As per discussion in #115, we now allow `shutdown` immediately following
`commitment_signed`.
This means that revoke-and-ack doesn't *always* ack the shutdown. Rather
than specify that a revoke-and-ack which is caused by an update-commit
following the shutdown acknowledges, we leave this unacknowledged until
we actually start closing.
This means we will retransmit it on every reconnect until then. But
that's not all that wasteful, and fairly robust.
Suggested-by: Pierre-Marie Padiou <pm.padiou@gmail.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Using the receiver's dust limit implies a naive node can be fooled
(by a peer with massive dust limit) into not getting its own output.
Using our own opens the possibility of creating different transactions,
so we explicitly allow the tractable case, while accepting failure on
the case where disagreement is real.
Closes: #128
Reported-by: Pierre-Marie Padiou
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We can no longer allow 511 each way; reduce it to match our calculations
so we can always create a single standard penalty transaction.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
At cost of a few extra bytes between peers, this avoids the whole "oops, we were on a chain fork" problem, and simplifies generation of temporary channel-ids (just pick a random one).
Now we move the announcement_signature exchange to at least 6 confirms, which makes re-xmit tricky; I resolved that by insisting on reconnect that we send if we haven't received, and reply to the first one.
The term "channel shortid" wasn't used anywhere, so I removed it; it's now a gossip-only thing anyway.
One subtle change: pkt_error on unknown channels is now "MUST ignore"; this section was reworked anyway, and we'll want this if the #120 goes through, where one side might have forgotten unformed channels).
Closes: #114
Suggested-by: Olaoluwa Osuntokun <laolu32@gmail.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
* FIXUP! Two bytes for funding-output-index.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
* FIXUP! Channel-id rework, temp ids, 32 bits only.
Re-add the idea of temporary channel ids: far simpler since they're now
big enough we can just fill with noise.
Remove the alignment issues by combining txid and outnum using XOR; we
could reduce to 128 bit if we really wanted to, but we don't.
Error handling is now simple again, but while editing I changed the
behaviour for unknown channels to MUST ignore (this is important for
Change the 8-byte gossip channel id to `short-channel-id`.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
* FIXUP! Minor text tweaks from Pierre-Marie and Christian
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
* BOLT 2&7: Cleaner separation of concerns wrt announcement signatures
So far we did not have any indication on what to do if a node does not
allow announcing the channel and we had a mix of concerns in the
`funding_locked` message, which would also transfer the signatures
needed for the announcement. This is a proposal about splitting the
signatures into their own message, so that simple omission is an
opt-out of announcements, and it does not mix announcement/gossip
stuff into the peer-protocol.
(It also ended up adding a localfeatures flag to opt-into the channel-announcement, and thus creating BOLT 9)
Olaoluwa pointed out that peers can make work for us by starting to open
connections then disconnecting: we need to allow timeouts, since it's
the simplest solution.
The comment about remembering `r` values (ie. `payment-preimage`) from
update_fulfill_htlc was also vague: it was meant to simply note that
it's not completely reversible, since the knowledge is (and probably
should be!) used to fulfill an incoming HTLC.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1) Make it clear that `init` needs to be sent every time.
- This means if you upgrade and no longer support an old connection, it's
clear, plus it simplifies the question of re-transmission of `init`.
2) Spell out the retransmission requirements for reconnection.
- We agreed in Milan to simply use retransmit and ignore-dups.
- This needs actual testing by implementations, but this is my best guess
on exactly how far back to retransmit.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This reduces failure codes to 2 bytes, places them into data itself.
Now we can use the same parsing code for them as we use for normal packets.
BOLT 2 is adjusted to match, and order of args changed to restore sha256
alignment to a nice 8 bytes.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
* BOLT 2: allow more leniancy with forks during channel establishment.
Christoper points out that two nodes with aggressive minimum-depth settings
may see different blocks and the protocol requires they close the channel
since their funding_locked messages will disagree.
This can also happen when only one side has an aggressive minimum-depth
setting: if it sends funding_locked referring to a block which is orphaned,
it can't update it.
There are three changes here, two optional.
- Allow sending of an updated funding_locked. This fixes this case where
one side is on an orphan and uses a v. low minimum-depth.
- Require accepting of an updated funding_locked.
- Allow waiting instead of immediate failure if funding_lock disagrees.
eg. you might wait another block or two to see if one side reorgs.
Reported-by: Christopher Jämthagen
Closes: #73
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If we get sent junk (or unknown version), we can't encrypt the
failure. Currently that's only if the onion version is non-zero, or
the ephemeral key is malformed.
We do check the BADONION bit so the origin can tell that we're
reporting it on the next peer's behalf, in case that ever matters.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Header from folded patch 'fix__feedback_from_roasbeef.patch':
FIX: Feedback from roasbeef.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
* BOLT 1: recommend full tx in error pkt if signature fails.
This will usually be the commitment tx, but could also be the HTLC
tx.
Reported-by: pm47 <pm.padiou@gmail.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
* FIX: Feedback
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
* BOLT 1: tighten error message `len` requirement.
Of course it has to match data length exactly.
Reported-by: pm47 <pm.padiou@gmail.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
* BOLT 1, BOLT 2, BOLT 5: commitsig -> commitment_signed.
Consistency FTW.
Reported-by: pm47 <pm.padiou@gmail.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Specifying that the `onion-routing-packet` commits to the
`payment-key` by setting the associated data. This avoids replay
attacks and specifying it here keeps the onion-routing spec clean.
This commit updates the opening paragraph in the channel establishment
section. The messages referenced are were from the prior iteration of
the funding messages and naming scheme. The section has been updated to
properly reference the latest messages and also to give a bit more
context to the reader to start with.
They're really a blob of bytes, and we weren't aligning them correctly in
two cases anyway. This gets rid of gratuitous padding, too.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Since our cryptopacket limits us to 2 bytes, and since people will
send 1-message-per-crypto-packet and nobody will test the
multiple-messages-in-one-cryptopacket code, let's just restrict to
64k messages.
1. Make cryptopacket length not include the HMAC, so we can actually send
64k messages.
2. Remove len prefix from packet, make type 2 bytes, note alignment properties.
3. Change message internal lengths/counts from 4 to 2 bytes, since more
is nonsensical anyway, and this removes a need to check before allocating:
- init feature bitfield length
- error message length
- shutdown scriptpubkey length
- commit_sig number of HTLC signatures
- revoke_and_ack number of HTLC-timeout signatures
4. Change max-accepted-htlcs to two bytes, and limit it to 511 to ensure
that commit_sig will always be under 64k.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Bitcoin still uses feerate-per-kb, which needs to be multiplied by 4
in segwit. It's more than a little confusing.
Split the paragraphs, too, as it's a little overwhelming.
Reported-by: Olaoluwa Osuntokun <laolu32@gmail.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is what it actually is (and how it's documented): you can't control
the total since both sides can add at the same time, you can only tell
the other side how many *it* can add, and control how many you add.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The MSG_XXX is trivially derived from the name of the message if
people want to do that, so just change it to the actual message name.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We need to supply a pair (a signature using our bitcoin key which signs the
node id, and a signature using our node key which signs the announce message),
and the length was wrong. Move them to make alignment nicer, too (though
it probably doesn't matter, they're 4-byte aligned everywhere else).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
And make it channel-index not channel_index to match the others (markdown
doesn't treat - specially, unlike _).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Make the fee amount by kiloweight, not by kilobyte, and spell out how
to calculate it. Note that we have to trim HTLCs outputs whose second-stage
would also fall below dust limit, and we also make some reasonable
approximations on transaction size (which is unknowable before signatures
anyway).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The fact that a revocation response also serves to acknowledge the
pending changes (and thus indicate the sender should apply them
to itself) is documented in the desciption, but still terribly
subtle. Joseph suggested making it clearer, I think this
does that.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
After much discussion with Tadge and Laolu, I think we have something
which is nicely outsourcable, and yetnot insanely complex.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Calculations are put in 05-onchain.md, and referred to by 02-peer-protocol.
The number is 600, comfortably under the 626 theoretical limit.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
