MessageSendEvent::PaymentFailureNetworkUpdate served as a hack to pass
an HTLCFailChannelUpdate from ChannelManager to NetGraphMsgHandler via
PeerManager. Instead, remove the event entirely and move the contained
data (renamed NetworkUpdate) to Event::PaymentFailed to be processed by
an event handler.
Now that NetworkGraph uses interior mutability, the RwLock used around
it in NetGraphMsgHandler is no longer needed. This allows for shared
ownership without a lock.
Latest rustc nightly compiles are filled with warnings like the
following, which we fix here:
```
warning: trailing semicolon in macro used in expression position
--> lightning/src/util/macro_logger.rs:163:114
|
163 | $logger.log(&$crate::util::logger::Record::new($lvl, format_args!($($arg)+), module_path!(), file!(), line!()));
| ^
|
::: lightning/src/chain/chainmonitor.rs:165:9
|
165 | log_debug!(self.logger, "New best block {} at height {} provided via block_connected", header.block_hash(), height);
| -------------------------------------------------------------------------------------------------------------------- in this macro invocation
|
= note: `#[warn(semicolon_in_expressions_from_macros)]` on by default
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #79813 <https://github.com/rust-lang/rust/issues/79813>
= note: this warning originates in the macro `log_internal` (in Nightly builds, run with -Z macro-backtrace for more info)
```
Because ln::functional_tests if over 9000 LoC long, its useful to
move tests into new modules as we can. Here we move all
cooperative shutdown related tests into a new module entitled
`shutdown_tests`
This adds the new range-based closing_signed negotiation specified
in https://github.com/lightningnetwork/lightning-rfc/pull/847 as
well as cleans up the existing closing_signed negotiation to unify
the new codepaths and the old ones.
Note that because the new range-based closing_signed negotiation
allows the channel fundee to ultimately select the fee out of a
range specified by the funder, which we, of course, always select
the highest allowed amount from. Thus, we've added an extra round
of closing_signed in the common case as we will not simply accept
the first fee we see, always preferring to make the funder pay as
much as they're willing to.
When we added the support for external signing, many of the
signing functions were allowed to return an error, closing the
channel in such a case. `sign_closing_transaction` is one such
function which can now return an error, except instead of handling
it properly we'd simply never send a `closing_signed` message,
hanging the channel until users intervene and force-close it.
Piping the channel-closing error back through the various callsites
(several of which already have pending results by the time they
call `maybe_propose_first_closing_signed`) may be rather
complicated, so instead we simply attempt to propose the initial
`closing_signed` in `get_and_clear_pending_msg_events` like we do
for holding-cell freeing.
Further, since we now (possibly) generate a `ChannelMonitorUpdate`
on `shutdown`, we may need to wait for monitor updating to complete
before we can send a `closing_signed`, meaning we need to handle
the send asynchronously anyway.
This simplifies a few function interfaces and has no impact on
behavior, aside from a few message-ordering edge-cases, as seen in
the two small test changes required.
Previously we'd been expecting to implement anchor outputs before
shipping 0.1, thus reworking our channel fee update process
entirely and leaving it as a future task. However, due to the
difficulty of working with on-chain anchor pools, we are now likely
to ship 0.1 without requiring anchor outputs.
In either case, there isn't a lot of reason to require that users
call an explicit "prevailing feerates have changed" function now
that we have a timer method which is called regularly. Further, we
really should be the ones deciding on the channel feerate in terms
of the users' FeeEstimator, instead of requiring users implement a
second fee-providing interface by calling an update_fee method.
Finally, there is no reason for an update_fee method to be
channel-specific, as we should be updating all (outbound) channel
fees at once.
Thus, we move the update_fee handling to the background, calling it
on the regular 1-minute timer. We also update the regular 1-minute
timer to fire on startup as well as every minute to ensure we get
fee updates even on mobile clients that are rarely, if ever, open
for more than one minute.
When a shutdown script is omitted from open_channel or accept_channel,
it must be provided when sending shutdown. Generate the shutdown script
at channel closing time in this case rather at channel opening.
This requires producing a ChannelMonitorUpdate with the shutdown script
since it is no longer known at ChannelMonitor creation.
It is useful for accounting and informational reasons for users to
be informed when a payment has been successfully forwarded. Thus,
when an HTLC which represents a forwarded leg is claimed, we
generate a new `PaymentForwarded` event.
This requires some additional plumbing to return HTLC values from
`OnchainEvent`s. Further, when we have to go on-chain to claim the
inbound side of the payment, we do not inform the user of the fee
reward, as we cannot calculate it until we see what is confirmed
on-chain.
Substantial code structure rewrites by:
Valentine Wallace <vwallace@protonmail.com>
test_onchain_to_onchain_claim was connecting additional blocks in
order to reach HTLC timeout and broadcast an HTLC-Timeout
transaction, resulting in it not testing whether HTLC preimages are
learned instantly in response to HTLC-Success transactions.
As the variable name implies holder_selected_chan_reserve_msat is
intended to be in millisatoshis, but is instead calculated in
satoshis.
We fix that error here and update the relevant tests to more
accurately calculate the expected reserve value and test both
success and failure cases.
Bug discovered by chanmon_consistency fuzz target.
Private nodes should never wish to forward HTLCs at all, which we
support here by disabling forwards out over private channels by
default. As private nodes should not have any public channels, this
suffices, without allowing users to disable forwarding over
channels announced in the routing graph already.
Closes#969
Currently the base fee we apply is always the expected cost to
claim an HTLC on-chain in case of closure. This results in
significantly higher than market rate fees [1], and doesn't really
match the actual forwarding trust model anyway - as long as
channel counterparties are honest, our HTLCs shouldn't end up
on-chain no matter what the HTLC sender/recipient do.
While some users may wish to use a feerate that implies they will
not lose funds even if they go to chain (assuming no flood-and-loot
style attacks), they should do so by calculating fees themselves;
since they're already charging well above market-rate,
over-estimating some won't have a large impact.
Worse, we current re-calculate fees at forward-time, not based on
the fee we set in the channel_update. This means that the fees
others expect to pay us (and which they calculate their route based
on), is not what we actually want to charge, and that any attempt
to forward through us is inherently race-y.
This commit adds a configuration knob to set the base fee
explicitly, defaulting to 1 sat, which appears to be market-rate
today.
[1] Note that due to an msat-vs-sat bug we currently actually
charge 1000x *less* than the calculated cost.
If we are a public node and have a private channel, our
counterparty needs to know the fees which we will charge to forward
payments to them. Without sending them a channel_update, they have
no way to learn that information, resulting in the channel being
effectively useless for outbound-from-us payments.
This commit fixes our lack of channel_update messages to private
channel counterparties, ensuring we always send them a
channel_update after the channel funding is confirmed.
This adds four new fields in `ChannelDetails`:
1. holder_selected_ and counterparty_selected_channel_reserve_delay
are useful to determine what amount of the channel is
unavailable for payments.
2. confirmations_required is useful when awaiting funding
confirmation to determine how long you will need to wait.
3. to_self_delay is useful to determine how long it will take to
receive funds after a force-close.
Fixes#983.
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.
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.
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.
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/4006Fixes#963
