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>
