When a node retires a failed path as part of a larger MPP payment,
the node may wish to use a path which is constrained by an
`htlc_minimum_msat` value. In this case, the node is forced to
overpay, likely overshooting the `total_msat` it set in the earlier
onions for the same MPP payment.
There are two possible solutions to this - either allow the
`total_msat` value to change in later HTLCs or allow the node to
(slightly) overshoot the `total_msat` value.
Allowing `total_msat` to change across HTLCs is nontrivial to
implement - HTLCs may arrive out-of-order, causing the receiving
node to have to track all seen `total_msat` values and accept a
set of HTLCs which meet any of the seen `total_msat` values.
Instead, this commit changes the MPP logic to simply allow a sender
to overshoot the stated `total_msat`.
Sadly the backwards-compatibility story for this is not great.
There doesn't seem to be a good way to resolve this issue in a
backwards-compatible way. Instead we just bite the bullet and make
the incompatible change, hoping the overshooting is rare enough
that it's not a major issue.
Requirements for the htlc_maximum_msat field in channel_update were
inadvertently removed by #999 (this PR meant to make this field mandatory,
not removed explanations about what it does).
To only use valid tlv payloads instead of fixed-size legacy ones and
invalid tlv streams.
[ Minor typo change: third payload is 275 not 256 bytes long --RR ]
My measurements a few weeks ago reveal that only 5 nodes do not
advertize this feature, of over 17000. I have a patch to
remove support from c-lightning, too.
[ 6 months later: t-bast notes that they only see 0.2% of htlcs using
legacy, and my node hasn't seen one for 2 months w/ 12000 htlcs --RR ]
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The order of operations is now more clearly specified as:
HTLC output amount = (`amount_msat` / 1000) - (fees in satoshis) where all
divisions are rounded down.
This is required to avoid issues in rounding if we were to take
HTLC output amount = (`amount_msat` - (`feerate_per_kw` * weight)) / 1000 and
then rounded down.
Since #910, nodes are allowed to use aliases instead of real scids. It is
helpful to make it explicit that updates using such aliases must not be
forwarded to other nodes by setting a flag in `channel_update`.
This flag is also generally useful for unannounced channels, regardless
of whether they use an scid alias or not.
We also make the `htlc_maximum_msat` field mandatory: every node on the
network currently sets it, so we can simplify the spec.
This commit ensures closing_signed can only begin if there are
no dangling commitments. It also clarifies update_fee requirements
if it is sent after shutdown.
When a node creates a new `channel_update` to change its channel parameters,
it will take some time to propagate through the network and payers may use
older parameters. It is recommended to keep accepting older parameters for a
while to improve payment latency and reliability.
And `next_per_commitment_point` to explictly `second_per_commitment_point`;
this is particularly important since `channel_ready` can be retransmitted
after the channel has been in use, for example.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
And weaken it: the opener doesn't need to respect it.
Note also that the `funding_locked`-can-change-alias refers to the same peer.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This lets you add your brand new channel to routehints, and also
means you can use a routehinted channel even if you (later?) have a
real channel.
This supports both trusted and untrusted zero-conf channels: in the
trusted case you can use it immediately like any other channel,
and for the untrusted case you simply use any push_msat they gave you
for outgoing payments, but fail incoming.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The rationale for this is to avoid bad cases like the following one
which was previously allowed:
* sender -> shutdown(script_one) -> receiver
* sender -> shutdown(script_two) -> receiver
* sender <- shutdown(script_one) <- receiver
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
It had a blank line and invalid format for tools/extract-formats.py.
And move the format information into the requirements section
(and fix spelling: `node_announement` -> `node_announcement`
Diff for extract-formats.py before and after:
```diff
--- /tmp/before 2022-05-17 10:47:01.583086352 +0930
+++ /tmp/after 2022-05-17 10:51:59.166850111 +0930
@@ -6,6 +6,8 @@
msgdata,init,tlvs,init_tlvs,
tlvtype,init_tlvs,networks,1
tlvdata,init_tlvs,networks,chains,chain_hash,...
+tlvtype,init_tlvs,remote_addr,3
+tlvdata,init_tlvs,remote_addr,data,byte,...
msgtype,error,17
msgdata,error,channel_id,channel_id,
msgdata,error,len,u16,
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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>
