Commit graph

2739 commits

Author SHA1 Message Date
Matt Corallo
f2c1712bdd Make channel fields which are from accept_channel Optional
These fields are set with a dummy value, which we should generally
be avoiding since Rust gives us a nice `Option` type to use
instead.

Further, we stop rejecting channel_update messages outright when
the htlc_maximum_msat field includes the reserve values, which
nodes could reasonably do without it meriting a channel closure.
2021-07-06 00:18:17 +00:00
Matt Corallo
9169dfbe5b [router] Use the invoice value for last-hop hint channel capacity
If an invoice contains route hints, we should assume the channel
capacity is sufficient to route the invoice's value.
2021-07-05 18:21:20 +00:00
Matt Corallo
84967faf52
Merge pull request #958 from TheBlueMatt/2021-06-fix-router-panic
Fix panic in router given to bogus last-hop hints
2021-07-05 00:01:43 +00:00
Matt Corallo
2825d65ca9 Fix panic in router given to bogus last-hop hints
See new comments and test cases for more info
2021-07-04 23:26:21 +00:00
Matt Corallo
fbb36a0769 Reject minimum_depth of 0 on channel opens
We don't support turbo channels so this is a pretty clear
indication that there is some incompatibility.
2021-07-04 14:17:26 +00:00
Matt Corallo
422bdcf814 Never generate a BroadcastChannelUpdate for priv channels
Currently we always generate a
`MessageSendEvent::BroadcastChannelUpdate` when a channel is closed
even if the channel is private. Our immediate peers should ignore
such messages as they haven't seen a corresponding
`channel_announcement`, but we are still giving up some privacy by
informing our immediate peers of which channels were ours.

Here we split `ChannelManager::get_channel_update` into a
`get_channel_update_for_broadcast` and
`get_channel_update_for_unicast`. The first is used when we are
broadcasting a `channel_update`, allowing us to refuse to do so
for private channels. The second is used when failing a payment (in
which case the recipient has already shown that they are aware of
the channel so no such privacy concerns exist).
2021-07-02 22:21:32 +00:00
Matt Corallo
0c57018f2f
Merge pull request #970 from TheBlueMatt/2021-06-no-confirmed-csv-delay
Create SpendableOutputs events no matter the chain::Confirm order
2021-07-02 17:55:17 +00:00
Matt Corallo
412cc9f01a Clean up get_broadcasted_holder_claims confirmation height param use 2021-07-02 17:16:12 +00:00
Matt Corallo
1905570358 Clarify when height is the *current* vs a *confirmation* height 2021-07-02 17:16:12 +00:00
Matt Corallo
496eb4526b Create SpendableOutputs events no matter the chain::Confirm order
We had a user who pointed out that we weren't creating
`SpendableOutputs` events when we should have been after they
called `ChannelMonitor::best_block_updated` with a block well
after a CSV locktime and then called
`ChannelMonitor::transactions_confirmed` with the transaction which
we should have been spending (with a block height/hash a ways in
the past).

This was due to `ChannelMonitor::transactions_confirmed` only
calling `ChannelMonitor::block_confirmed` with the height at which
the transactions were confirmed, resulting in all checks being done
against that, not the current height.

Further, in the same scenario, we also would not fail-back and HTLC
where the HTLC-Timeout transaction was confirmed more than
ANTI_REORG_DELAY blocks ago.

To address this, we use the best block height for confirmation
threshold checks in `ChannelMonitor::block_confirmed` and pass both
the confirmation and current heights through to
`OnchainTx::update_claims_view`, using each as appropriate.

Fixes #962.
2021-07-02 17:16:12 +00:00
Matt Corallo
599c74cd42 Update ChannelMonitor::best_block before calling block_confirmed
No matter the context, if we're told about a block which is
guaranteed by our API semantics to be on the best chain, and it has
a higher height than our current understanding of the best chain,
we should update our understanding. This avoids complexity
in `block_confirmed` by never having a height set which is *higher*
than our current best chain, potentially avoiding some bugs in the
rather-complicated code.

It also requires a minor test tweak as we in some cases now no
longer broadcast a conflicting transaction after the original has
reached the ANTI_REORG_DELAY.
2021-07-02 17:16:12 +00:00
Matt Corallo
697033342e Avoid calling OnchainTx::block_disconnected if no block was discon'd
There are no visible effects of this, but it seems like good code
hygiene to not call a disconnect function in a different file if no
disconnect happened.
2021-07-02 17:16:12 +00:00
Matt Corallo
4074909f04 Add new expect_payment_failure_chan_update!() macro in tests
This further DRYs up some functional_test code and increases
coverage.
2021-07-02 17:16:12 +00:00
Matt Corallo
e7d3781dd7
Merge pull request #976 from TheBlueMatt/2021-06-actionable-errors
Make errors actionable when failing to deserialize a ChannelManager
2021-07-01 03:33:15 +00:00
Matt Corallo
4353d4a11c
Merge pull request #954 from TheBlueMatt/2021-06-no-spurious-forward-fails
Consider channels "live" even if they are awaiting a monitor update
2021-07-01 03:28:30 +00:00
Matt Corallo
b58c88430e Consider channels "live" even if they are awaiting a monitor update
We use `Channel::is_live()` to gate inclusion of a channel in
`ChannelManager::list_usable_channels()` and when sending an
HTLC to select whether a channel is available for
forwarding through/sending to.

In both of these cases, we should consider a channel `is_live()` when
they are pending a monitor update. Some clients may update monitors
asynchronously, thus we may simply be waiting a short duration for a
monitor update to complete, and shouldn't fail all forwarding HTLCs
during that time.

After #851, we always ensure any holding cells are free'd when
sending P2P messages, making this change much more trivially
correct - instead of having to ensure that we always free the holding
cell when a channel becomes live again after adding something to the
holding cell, we can simply rely on the fact that it always happens.

Fixes #661.
2021-06-30 23:15:22 +00:00
Matt Corallo
57e813f971
Merge pull request #972 from TheBlueMatt/2021-06-skip-notify-chansync
Do not always persist ChannelManager on channel_update messages
2021-06-30 20:32:35 +00:00
Matt Corallo
803da875dd Fix unused import in peer_handler introduced in 1f592b045f 2021-06-30 16:13:48 +00:00
Matt Corallo
eca6da354b Do not always persist ChannelManager on channel_update messages
If we receive a `channel_update` message for a channel unrelated to
our own, we shouldn't trigger a persistence of our
`ChannelManager`. This avoids significant persistence traffic during
initial node startup.
2021-06-30 16:13:48 +00:00
Matt Corallo
e8110ab33f Correct test log printing due to inverted comparison
We changed the sort order of log levels to be more natural, but this
comparison wasn't updated accordingly. Likely the reason it was
left strange for so long is it also had the comparison argument
ordering flipped.
2021-06-30 16:12:21 +00:00
Matt Corallo
e7560c83b4 Make errors actionable when failing to deserialize a ChannelManager 2021-06-30 16:12:21 +00:00
Matt Corallo
e75d41be7a
Merge pull request #973 from TheBlueMatt/2021-06-fix-broken-event-ser
Fix bogus `Event::PaymentSent` serialization
2021-06-30 01:19:30 +00:00
Matt Corallo
bbda177be6 Clean up check_spendable_outputs!() test macro somewhat 2021-06-29 20:14:37 +00:00
Matt Corallo
f4729075cb
Merge pull request #965 from TheBlueMatt/2021-06-log-cleanups
Cleanup logging
2021-06-29 20:13:50 +00:00
Matt Corallo
6d98aedaf5 Add debug log when we stop tracking confirmed on-chain packages 2021-06-29 19:36:47 +00:00
Matt Corallo
6d446a6249 Correct inbound HTLC upgrade logs on revoke_and_ack receipt 2021-06-29 19:36:47 +00:00
Matt Corallo
74717d390c Increase the log level of several channelmonitor/onchain logs.
ChannelMonitor and related log entries can generally lean towards
being higher log levels than they necessarily need to be, as they
should be exceedingly rare, if only because they require
confirmation of an on-chain transaction.
2021-06-29 19:36:47 +00:00
Matt Corallo
76ea83463b Add additional TRACE-level logging during pathfinding in router 2021-06-29 19:36:47 +00:00
Matt Corallo
7eff56b12f Update logging in channel and channelmanager to better levels
This updates a number of log sites in channel and channelmanager to
 * Be a bit more verbose at the TRACE level,
 * Move some error/useful messages to the ERROR/WARN/INFO level,
 * Add new logs to always log once at the DEBUG level when we
   send/receive a commitment_signed (with some extra data),
 * Include the channel id being operated on in more log messages.
2021-06-29 19:36:47 +00:00
Matt Corallo
1f592b045f Do not log_debug when we receive duplicate gossip messages
We very often receive duplicate gossip messages, which now causes us
to log at the DEBUG level, which is almost certainly not what a
user wants. Instead, we add a new form of ErrorAction which causes
us to only log at the TRACE level.
2021-06-29 19:36:47 +00:00
Matt Corallo
7fa6a7d48e Allow logging to specify an explicit log level instead of a macro
For log entries which may have a variable level, we can't call an
arbitrary macro and need to be able to pass an explicit level. This
does so without breaking the compile-time disabling of certain log
levels.

Further, we "fix" the comparison order of log levels to make more
significant levels sort "higher", which implicitly makes more sense
than sorting "lower".

Finally, we remove the "Off" log level as no log entry should ever
be logged at the "Off" level - that would be nonsensical.
2021-06-29 19:36:47 +00:00
Matt Corallo
d36a875f98 More consistently log in msg handling, incl full msg logging at trace
This much more consistently logs information about messages
sent/received, including logging the full messages being
sent/received at the TRACE log level. Many other log messages which
are more often of interest were moved to the DEBUG log level.
2021-06-29 19:36:47 +00:00
Matt Corallo
3ea4279d55 Unify message sending to use PeerManager::enqueue_message
This makes our logging consistent and somewhat simplifies message
sending code in a few places.
2021-06-29 19:36:47 +00:00
Matt Corallo
133e28ffe6 Add error logs when a ChannelManager as inconsistent monitor state
We had a client application which provided inconsistent monitor
state when deserializing a ChannelManager, resulting in opaque and
generic "InvalidData" deserialization failures. Instead, we log
some informative (and appropriately scary) warning messages in
such cases.
2021-06-29 19:36:47 +00:00
Matt Corallo
3791a7b2a5
Merge pull request #974 from sr-gi/message_signing_borrow
Passes references to the public and secret keys to sign/verify
2021-06-29 16:29:08 +00:00
Matt Corallo
74f10076b2
Merge pull request #966 from TheBlueMatt/2021-06-workaround-broken-lnd
Workaround lnd sending funding_locked before channel_reestablish
2021-06-29 16:28:38 +00:00
Sergi Delgado Segura
c1d2d156c9
Passes references to the public and secret keys to sign/verify
sign/verify should not take ownership of the keys.
2021-06-29 15:26:21 +02:00
Matt Corallo
dec7ec191e Fix bogus Event::PaymentSent serialization
`Event::PaymentSent` serialization has a bug where we
double-serialize the payment_preimage field but do not attempt to
read it twice. This results in a failure to read `ChannelManager`s
from disk if we have a pending `Event::PaymentSent` pending
awaiting handling when we serialize.

Instead of attempting to read both versions, we opt to simply fix
the serialization, assuming it is exceedingly rare for such a
scenario to appear (and if it does, we can assist in manual
recovery).

The release notes have been updated to note this inconsistency.

Found by the chanmon_consistency fuzz target.
2021-06-28 23:38:09 +00:00
Matt Corallo
8df141233f Workaround lnd sending funding_locked before channel_reestablish
lnd has a long-standing bug where, upon reconnection, if the
channel is not yet confirmed they will not send a
channel_reestablish until the channel locks in. Then, they will
send a funding_locked *before* sending the channel_reestablish
(which is clearly a violation of the BOLT specs). We copy
c-lightning's workaround here and simply store the funding_locked
message until we receive a channel_reestablish.

See-also https://github.com/lightningnetwork/lnd/issues/4006

Fixes #963
2021-06-28 02:05:33 +00:00
Matt Corallo
ecddfe1766
Merge pull request #968 from TheBlueMatt/2021-06-no-invoice-dylib
Drop the cdylib export of lightning-invoice
2021-06-25 17:42:40 +00:00
Matt Corallo
b9aa71f056
Merge pull request #964 from valentinewallace/2021-06-tlv-ser
Fix TLV serialization to work with large types
2021-06-24 22:44:41 +00:00
Valentine Wallace
40959b74b7
Fix TLV serialization to work with large types.
Previous to this PR, TLV serialization involved iterating from 0 to the highest
given TLV type. This worked until we decided to implement keysend, which has a
TLV type of ~5.48 billion.

So instead, we now specify the type of whatever is being (de)serialized (which
can be an Option, a Vec type, or a non-Option (specified in the serialization macros as "required").
2021-06-24 16:25:31 -04:00
Matt Corallo
b786adff5b Drop the cdylib export of lightning-invoice
There are ~zero functions in lightning-invoice that are materially
callable from C, so there isn't any reason to tag it as a cdylib
(and make rustc build it as such). Instead, we have C bindings now.
2021-06-24 04:32:25 +00:00
Matt Corallo
bfd9646092
Merge pull request #950 from TheBlueMatt/2021-06-changelog
Add a dummy first CHANGELOG entry for future tracking
2021-06-23 17:22:11 +00:00
Matt Corallo
66726fb508 Add a dummy first CHANGELOG entry for future tracking 2021-06-23 16:43:18 +00:00
Matt Corallo
a146ef2be2 Do not generate error messages when we receive our own gossip
When a peer sends us the routing graph, it may include gossip
messages for our channels, despite it not being a party to them.
This is completely fine, but we currently print a somewhat-scary
looking log messages in these cases, eg:

```
ERROR [lightning::ln::channelmanager:4104] Got a message for a channel from the wrong node!
TRACE [lightning::ln::peer_handler:1267] Handling SendErrorMessage HandleError event in peer_handler for node ... with message Got a message for a channel from the wrong node!
```

Instead, we should simply not consider this an "error" condition
and stay silent.
2021-06-23 01:35:26 +00:00
Matt Corallo
95f9523097 Drop rust-bitcoin crate patches in fuzz now that they're merged
These patches have been merged upstream and are in releases now so
we don't need to patch them locally.
2021-06-23 01:35:26 +00:00
Matt Corallo
c5a6135edc log_debug information about network graph updates from payments 2021-06-23 01:35:26 +00:00
Matt Corallo
d61d698bb8 Don't print file paths in fuzz logger as they can be very long 2021-06-23 01:35:23 +00:00
Matt Corallo
073afbb244
Merge pull request #957 from TheBlueMatt/2021-06-p2p-no-deadlock
Fix P2P Deadlocks
2021-06-23 01:34:55 +00:00