This is a minor clarification that the `to_self_delay` is enforced
in a 2nd-stage transaction for HTLCs, while it's directly enforced
in the commit tx for the main output.
Gossip query compression is not very useful - it was added for
mobile clients to, in theory, sync the gossip data directly from
P2P peers, but to my knowledge no mobile clients actually use it
for that, or at least use it where the gossip *query* data is a
substantial portion of their overall bandwidth usage.
Further, because of the semantics of `gossip_timestamp_filter`, its
impractical to ensure you receive a reliable, full view of the
gossip data without re-downloading large portions of the gossip
data on startup.
Ultimately, gossip queries are a pretty non-optimal method of
synchronizing the gossip data. If someone wants highly optimized
gossip data synchronization a new method based on set
reconciliation needs to be propose.
Finally, the current gossip query encoding semantics do not allow
for negotiation and instead require all lightning implementations
take a zlib dependency in some form or another. Given the recent
zlib decoding memory corruption vulnerability, this seems like an
opportune time to simply remove the zlib support, requiring that
nodes stop sending compressed gossip query data (though they can
support reading such gossip query data as long as they wish).
This is an alternative to the suggested gossip query encoding
support in #825.
One argument for adding a feature bit for channel types was to make things
more explicit and remove ambiguity.
When sending `open_channel`, we require funders to include a `channel_type`,
so it would make sense to require fundees to echo that `channel_type`
back to explicitly confirm that they're ok with this `channel_type`.
When peers disagree on the closing fee range, disconnecting
doesn't make sense: upon reconnection they will just send the
same `closing_signed` again.
A warning should instead be sent and logged, so that node
operators can decide to update their fee range if they want
this channel close to make progress.
If a node has to fail a channel but knows that its latest commitment transaction is outdated it should not be required to send it but rather wait for the peer to unilaterally close the channel.
The proposed solution is not so clean because it might produce a deadlock in which two peers assume they have outdated state and send `error` back and forth without actually force closing. Maybe in such a scenario we could create a protocol that mutually closes with split balance?
Also replaced the word use with broadcast as it seems more accurate.
Co-authored-by: t-bast <bastuc@hotmail.fr>
Prior to the addition of `warning` messages, BOLT 5 specified a
few cases where users should be warned that funds may have been
lost. However, it used the phrasing "send a warning" which can now
be confused with `warning` messages. Nodes should not generally
inform their counterparty that they have been robbed.
This adds the option to report an remote IP address back to a connecting
peer using the `init` message. A node can decide to use that information
to discover a potential update to its public IPv4 address (NAT) and use
that for a `node_announcement` update message containing the new address.
The proposal includes reporting the IPv4 and IPv6 address,
however in IPv6 there are likely no NAT issues. TOR is skipped for
obvious reasons.
Certain approaches to check and use this information are thinkable:
- Wait for multiple peers or a certain fraction to report the
same new address.
- Check some random node known via gossip to also report the new
address.
- Verify this information by making a test connection to itself.
This introduces a new gossip address descriptor type used for DNS hostnames.
This is particular useful for dynamic DNS users that want to use their home
ISP connection with changing IP addresses without relying only on TOR.
The `len` field is deliberately encoded with just a byte (u8) since
POSIX hostnames do not exceed 255 bytes in total.
There were valid uses for "I don't want to talk to you anymore" apparently!
Also fixed some tabs -> spaces.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
And make most places warn or error. Places where we're operating
on a channel tend to be "warn and close connection" since we want to
forget the mistake they just sent, and closing the connection does that.
We now use the same words everywhere:
1. "fail channel" means to go onchain (if necessary).
2. "send `error`" means to send an error message.
3. "send `warning`" means to send a warning message.
4. "close connection" means close the connection.
These are all spelled out explicitly, rather than having "fail channel"
imply sending an error packet, for example.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Under this spec, an error with an all-zero channel id is ignored.
Warnings, being odd, will be ignored by older nodes too.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
In this commit, we add a new feature bit to gate the new explicit
channel type funding via the new `channel_type` TLV. The addition of
this new bit allows peers to seek out other peers that understand the
new explicit channel negotiation. This is useful in practice, as it
allows peers to avoid needing to "downgrade" the feature bits advertised
at the connection level due to one peer not understanding a new
required feature bit while it has a channel with a connecting peer.
Such a workaround is already deployed on the network between lnd peers
and certain eclair peers, as the `lnd` peers require static key, but the
feature bit is unknown to eclair peers. This situation (forced
downgrade) is undesirable, as until the connected peer updates (or the
channel is closed) and "worst" feature bit set must always be advertised
in order to maintain connectivity.
The other benefit of adding this feature bit is that it allows
implementations to simplify their code by ensuring that the new feature
will be used before sending any messages that include or reference that
feature. Without a feature bit, peers are instead forced to essentially
guess if a peer understands that feature, with logic to be able to "bail
out" of an invalid state.
The addition of this feature bit matches the prior precedent of adding
feature bits when new fields in the channel negotiation message (last
one was upfront shutdown) are added.
The test vectors for invalid `init` messages were invalid since we added
the `networks` tlv extension.
They are now fixed and made more future-proof by using tlvs with high
values.
Before this change, repository gets detected as:
Python 51.1%
Shell 48.9%
After this change, repository gets detected as:
Markdown 98.0%
Other 2.0%
This change was added for a cosmetic effect on GitHub, and the line
this change adds can go away if needed.
None of the project maintainers have access to that website.
It is completely outdated and could be modified without our consent,
so we shouldn't link to it anymore.
Since HTLCs below this amount will not appear in the commitment tx, they
are effectively converted to miner fees. The peer could use this to grief
you by broadcasting its commitment once it contains a lot of dust HTLCs.
Add network dust thresholds computation details, as implemented in Bitcoin
Core's default relay policy.
Drop non-segwit support in shutdown: this allows dust limit to go as low
as 354 sats without creating relay issues with default node policies.
We add a requirement that dust limit cannot be lower than 354 sats.
This ensures implementers don't have to figure this subtlety on their own.
Fixes#696 and #905
There's not a lot of reason to explicitly rate-limit ping messages
on the sending side, hosts on the internet can *always* send you
as much traffic as they want, its up to you whether you want to
talk back to them.
This seems to have been intended as a cutoff where nodes can skip
responding to pings below a certain rate, but in practice 30
seconds is much too long a time to learn that your peer has
disconnected.
We could reduce the threshold, but its not like this is the only
place in the spec where a peer can request a message response, and
that is unlikely to change, making it of highly dubious value.
Bolt 11 invoices must contain a `payment_secret`, which means that the
`features` field must set the `payment_secret` feature (and its dependency,
`var_onion_optin`).
Fixes#897
With anchor outputs, we can keep the commit tx feerate lower than the real
on-chain feerate. That means that when closing the channel, the resulting
fee will not necessarily be lower than the current commit tx fee, this
requirement doesn't make sense to be strict.
Both sides can optionally include a preferred fee range in their
`closing_signed`. This lets their peer know what fees they find acceptable
and simplifies the closing negotiation.
This is extracted from channel_upgrade (#868), but used for opening
negotiation as suggested by @roasbeef on the last spec meeting.
It's a trivial change, fully backwards compatible, but now each channel
has a channel_type, which defines its behavior, rather than an ad-hoc
set of "sticky" feature bits. It also means both peers can *support* a
feature without endorsing it.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Update Bolt 11 test vectors to always include a payment secret.
We want to make it mandatory in invoices which would make the existing
test vectors invalid.
We previously had a restriction on HTLC amounts to avoid big losses during
the early phases of the network, but it shouldn't be necessary anymore.
As long as we honor `max_htlc_value_in_flight_msat` and implementations
provide safe defaults for that parameter, we don't need that additional
restriction.
In bitcoin 0.19.0, standardness rules are going to be relaxed to allow
future witness versions. Once this is widely deployed, it will be safe
to accept them, smoothing use of future segwit versions.
See: https://github.com/bitcoin/bitcoin/pull/15846
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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.
