Fix handling of inbound uncommitted feerate updates

If we receive an update_fee but do not receive a commitment_signed,
we should not persist the pending fee update to disk or hold on to
it after our peer disconnects.

In order to make the code the most readable, we add a state enum
which matches the relevant states from InboundHTLCState, allowing
for more simple code comparison between inbound HTLC handling and
update_fee handling.

Fix re-sending commitment updates with an outbound fee update

When we send an update_fee to our counterparty on an outbound
channel, if we need to re-send a commitment update after
reconnection, the update_fee must be present in the re-sent
commitment update messages. However, wewere always setting the
update_fee field in the commitment update to None, causing us to
generate invalid commitment signatures and get channel
force-closures.

This fixes the issue by correctly detecting when an update_fee
needs to be re-sent, doing so when required.

Add more logging during chanmon_consistency runs

Add fuzz coverage of (potential) fee update messages

Automatically update fees on outbound channels as fees change

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.

Set cfg=fuzzing when building fuzz crate in CI

We will likely drop the fuzztarget feature soon, and should thus be
setting cfg=fuzzing explicitly anyway.

Add standard derives for ConfirmationTarget

Merge pull request #1009 from ariard/2021-07-add-forward-dust-limit

Add new config setting `max_balance_dust_htlc_msat`

Add test_max_balance_dust_htlc

Enforce `max_balance_dust_htlc_msat` at HTLC reception/forward

At `update_add_htlc()`/`send_htlc()`, we verify that the inbound/
outbound dust or the sum of both, on either sides of the link isn't
above new config setting `max_balance_dust_htlc_msat`.

A dust HTLC is hence defined as a trimmed-to-dust one, i.e including
the fee cost to publish its claiming transaction.

Modify pending inbound/outbound getters to access dust balances

Add new config setting `max_balance_dust_htlc_msat`

Trimmed-to-dust HTLCs are at risk of being burnt as miner fees
at anytime during their lifetime due to the broadcast of either
holder commitment transaction or counterparty's one.

To hedge against this risk, we introduce a new config setting
`max_balance_dust_htlc_msat`, with the initial value of
5_000_000 msat.

Merge pull request #1025 from TheBlueMatt/2021-07-detect-htlcs-on-local-commitment

Merge pull request #1038 from valentinewallace/2021-08-keysend-docs

Add requirement of payment secret for multi path payments

in send_payment_internal

Merge pull request #1019 from jkczyz/2021-07-shutdown-pubkey

Fetch shutdown script based on `commit_upfront_shutdown_pubkey`

Add APIError::IncompatibleShutdownScript

Implement Display for ShutdownScript

Pass InitFeatures by reference to Channel

Remove unreachable BroadcastChannelUpdate

When handling shutdown messages, Channel cannot move to
ChannelState::ShutdownComplete. Remove the code in ChannelManager that
adds a MessageSendEvent::BroadcastChannelUpdate in this case since it is
unreachable.

Generate shutdown script at channel close

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.

Add features to NodeCfg and use in create_network

Connect nodes in remaining tests

Similar to 2745bd5ac776c48950bcb630338538d31a9615d0, this ensures that
ChannelManager knows about the features its peers.

Support all shutdown scripts defined in BOLT 2

KeysInterface::get_shutdown_pubkey is used to form P2WPKH shutdown
scripts. However, BOLT 2 allows for a wider variety of scripts. Refactor
KeysInterface to allow any supported script while still maintaining
serialization backwards compatibility with P2WPKH script pubkeys stored
simply as the PublicKey.

Add an optional TLV field to Channel and ChannelMonitor to support the
new format, but continue to serialize the legacy PublicKey format.

Use ShutdownScript to check scripts from peers

Add ShutdownScript for BOLT 2 acceptable scripts

BOLT 2 enumerates the script formats that may be used for a shutdown
script. KeysInterface::get_shutdown_pubkey returns a PublicKey used to
form one of the acceptable formats (P2WPKH). Add a ShutdownScript
abstraction to encapsulate all accept formats and be backwards
compatible with P2WPKH scripts serialized as the corresponding
PublicKey.

Clean up and add shutdown script functional tests

Merge pull request #1035 from TheBlueMatt/2021-08-faster-pings

Suggest faster ping in `PeerManager::timer_tick_occurred` docs

Handle being asleep for more than double our ping time gracefully

If we've been asleep for double our ping time, for whatever reason,
disconnect all open sockets.

Update lightning-background-processor to ping every five seconds

This updates lightning-background-processor calls to
PeerManager::timer_tick_occurred to match the new suggested rate in
the documentation.

Suggest faster ping in `PeerManager::timer_tick_occurred` docs

This clarifies the docs for `PeerManager::timer_tick_occurred` to
note that the call rate is entirely up to the user, and also
suggests a faster ping rate of "once every five to ten seconds"
instead of "every 30 seconds". There isn't a lot of reason to want
to ping less often, and faster ping means we detect disconnects
sooner, which is important.

Correctly detect missing HTLCs when a local commitment tx was broadcast

If we forward an HTLC to our counterparty, but we force-closed the
channel before our counterparty provides us an updated commitment
transaction, we'll end up with a commitment transaction that does
not contain the HTLC which we attempted to forward. In this case,
we need to wait `ANTI_REORG_DELAY` blocks and then fail back the
HTLC as there is no way for us to learn the preimage and the
confirmed commitment transaction paid us the value of the HTLC.

However, check_spend_holder_transaction did not do this - it
instead only looked for dust HTLCs in the confirmed commitment
transaction, paying no attention to what other HTLCs may exist that
are missed.

This will eventually lead to channel force-closure as the channel
on which we received the inbound HTLC to forward will be closed in
time for the initial sender to claim the HTLC on-chain.

DRY HTLC failure code in check_spend_counterparty_transaction

This extracts the HTLC-not-in-broadcasted-commitment-transaction
code from check_spend_counterparty_transaction and moves it to a
global macro, DRYing up the two very similar codepaths (fixing
some minor logging inconsistencies) in the process.

This macro will be used for local commitment transaction HTLC
failure as well in the next commit.

This commit has no functional change outside of logging.

Update keysend docs

Merge pull request #1029 from TheBlueMatt/2021-07-log-channel-close

Log when a channel is closed on startup due to stale ChannelManager

Make BackgroundProcessor `#[must_use]` to avoid dropping immediately

It is easy for users to have a bug where they drop a
`BackgroundProcessor` immediately, causing it to start and then
immediately stop. Instead, add a `#[must_use]` tag to provide a
compiler warning for such instances.

Log when a channel is closed on startup due to stale ChannelManager

This is one of the riskiest parts of our API from the perspective
of accidental force-closes - if users delay persisting the
ChannelManager much at all after a ChannelMonitor we may hit a
force-close after restart.

The fact that we don't log at all when this happens is criminal.

Add additional TLV serialization type of (default_value, N)

This allows TLV serialization macros to read non-Option-wrapped
types but allow them to be missing, filling them in with the
provided default value as needed.

Merge pull request #1004 from TheBlueMatt/2021-07-forward-event

Add a `PaymentForwarded` Event

Change return value of `claim_funds` to ignore duplicate claims

While we should never reach `ClaimFundsFromHop::DuplicateClaim` in
most cases, if we do, it likely indicates the HTLC was timed out
some time ago and is no longer available to be claimed. Thus, it
does not make sense to imply that we `claimed_any_htlcs`.

Generate a PaymentForwarded event when a forwarded HTLC is claimed

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>

Merge pull request #1022 from TheBlueMatt/2021-07-to-remote-reorg

Fix to_remote SpendableOutputs generation in rare reorg cases

Fix to_remote SpendableOutputs generation in rare reorg cases

If we first see a local commitment transaction, and then a reorg
causes the confirmed channel close transaction to instead be a
remote commitment transaction, we would fail a spurious `if else`
check, resulting in us not generating the correct `SpendableOutput`
event for the to_remote output now confirmed on chain.

This resolves the incorrect logic and adds a regression test.

Merge pull request #1028 from lightning-signer/2021-08-no-std

Actual no_std support

Rename no_std feature to no-std

matches rust-bitcoin

Merge pull request #1033 from TheBlueMatt/2021-07-fix-beta

Fix lightning-persister tests for upcoming rustc changes

Actual no_std support

Check IO errors in test using `raw_os_error()` instead of `kind()`

std::io::ErrorKind is a `#[non_exhaustive]` enum as more specific
error types are to be added in the future. It was unclear in the
docs until very recently, however, that this is to be done by
re-defining `ErrorKind::Other` errors to new enum variants. Thus,
our tests which check explicitly for `ErrorKind::Other` as a
result of trying to access a directory as a file were incorrect.
Sadly, these generated no meaningful feedback from rustc at all,
except that they're suddenly failing in rustc beta!

After some back-and-forth, it seems rustc is moving forward
breaking existing code in future versions, so we move to the
"correct" check here, which is to check the raw IO error.

See rust-lang/rust#86442 and rust-lang/rust#85746 for more info.

Merge pull request #1032 from jkczyz/2021-08-clippy

Fail linter on #[warn(clippy::try_err)]

Some heavily used macros are using ? directly on an Err. Using a return
is easier to read and removes hundreds of linter warnings.

https://rust-lang.github.io/rust-clippy/master/index.html#try_err

Fix #[warn(clippy::try_err)] in ser_macros.rs

Add CI runs on rustc beta on Windows and MacOS

This should catch any platform-specific behavior changes in rustc
before they land in stable.

Merge pull request #1012 from TheBlueMatt/2021-07-bump-deps

Bump dependencies to bitcoin 0.27 and bech32 0.8

Add a `#[macro_use]` on the `alloc` import for `format!()`

Fix no_std warnings due to unused includes

Drop MSRV for no_std to 1.47 as that's what Ubuntu LTS ships with

...but disable it for now given core2 is broken (it claims an MSRV
of 1.47 but does not build).

Bump dependencies to bitcoin 0.27 and bech32 0.8

Merge pull request #1024 from TheBlueMatt/2021-07-always-connect-in-tests

Connect peers on startup in tests

Connect peers on startup in tests

This avoids `ChannelManager` ever being confused by the fact that
it received a message from a peer which it didn't think it was
connected to.

Merge pull request #1021 from TheBlueMatt/2021-07-broken-beta

Disable fast-fail to let CI actually run even though beta is broken

Merge pull request #1007 from jkczyz/2021-07-stop-drop-shutem-down

Stop BackgroundProcessor's thread on drop

Disable fast-fail to let CI actually run even though beta is broken

Merge pull request #1020 from TheBlueMatt/2021-07-log-features-more

Macroize feature printing to ensure we don't miss new flags

Add join method to BackgroundProcessor

The previous commit wraps the background thread's JoinHandle in an
Option. Providing a dedicated method to join hides this implementation
detail from users.

Macroize feature printing to ensure we don't miss new flags

Test preimages are learned instantly in test_onchain_to_onchain_claim

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.

Ignore unknown Events serialized with an odd type value.

This should provide some additional future extensibility, allowing
for new informational events which can be safely ignored to be
ignored by older versions.

Drop single-use macro from check_spend_holder_transaction

The wait_threshold_conf!() macro in check_spend_holder_transaction
was only used once, making it a good candidate for inlining at the
callsite. Further, it incorrectly always logged that we were
failing HTLCs from the "latest" commitment transaction, when it is
sometimes actually failing HTLCs from the previous commitment
transaction.

Merge pull request #977 from TheBlueMatt/2021-06-fix-double-claim-close

Handle double-HTLC-claims without failing the backwards channel

Fail channel if we can't sign a new commitment tx during HTLC claim

Previously, we could fail to generate a new commitment transaction
but it simply indicated we had gone to doule-claim an HTLC. Now
that double-claims are returned instead as Ok(None), we should
handle the error case and fail the channel, as the only way to hit
the error case is if key derivation failed or the user refused to
sign the new commitment transaction.

This also resolves an issue where we wouldn't inform our
ChannelMonitor of the new payment preimage in case we failed to
fetch a signature for the new commitment transaction.

Simplify call graph of get_update_fulfill_htlc since it can't Err.

Handle double-HTLC-claims without failing the backwards channel

When receiving an update_fulfill_htlc message, we immediately
forward the claim backwards along the payment path before waiting
for a full commitment_signed dance. This is great, but can cause
duplicative claims if a node sends an update_fulfill_htlc message,
disconnects, reconnects, and then has to re-send its
update_fulfill_htlc message again.

While there was code to handle this, it treated it as a channel
error on the inbound channel, which is incorrect - this is an
expected, albeit incredibly rare, condition. Instead, we handle
these double-claims correctly, simply ignoring them.

With debug_assertions enabled, we also check that the previous
close of the same HTLC was a fulfill, and that we are not moving
from a HTLC failure to an HTLC claim after its too late.

A test is also added, which hits all three failure cases in
`Channel::get_update_fulfill_htlc`.

Found by the chanmon_consistency fuzzer.

Merge pull request #967 from valentinewallace/2021-06-keysend

Keysend

Clarify decode_update_add_htlc_onion comment

Clearer phrasing

tests: make PaymentSecret optional in pass_along path

and use it to make more keysend tests

Implement utilities for keysending to private nodes

Implement sending keysend payments (to public nodes)

test utils: add optional PaymentPreimage param to pass_along_path

This will allow keysend tests to assert that the PaymentReceived payment preimage is
as expected in upcoming commits.

Implement receiving keysend payments

Refactor PaymentReceived event for keysend receives

Add PendingHTLCRouting variant for receiving keysend payments

Fix indentation in decode_update_add_htlc_onion

Advertise keysend feature

C-Lightning requires us to advertise this feature before they'll
attempt a keysend payment to us.

Parse keysend TLV field in onion.

This doesn't yet use the field, but it will be used in upcoming commits.

Merge pull request #998 from TheBlueMatt/2021-07-fix-chan-reserve-msat-sat

Fix channel reserve calculation on the sending side

Merge pull request #986 from TheBlueMatt/2021-07-route-lasthop-value

[router] Use the invoice value for last-hop hint channel capacity

Merge pull request #1008 from lightning-signer/2021-07-sync-no-std

Dummy sync implementation for no_std

Merge pull request #1010 from sr-gi/enforce_signature_length

Test no_std instead of just hashbrown

Implement dummy Mutex, Condvar and RwLock

Enforces sig_rec length in message_signing

Collect all lightning std::sync imports under crate::sync

in preparation for no-std sync dummies

Stop BackgroundProcessor's thread on drop

Without stopping the thread when BackgroundProcessor is dropped, it will
run free. In the context of language bindings, it is difficult to know
how long references held by the thread should live. Implement Drop to
stop the thread just as is done when explicitly calling stop().

Correctly assert BackgroundProcessor error

The specific error from the ChannelManager persister is not asserted for
in test_persist_error. Rather, any error will do. Update the test to use
BackgroundProcessor::stop and assert for the expected value.

Merge pull request #1003 from jkczyz/2021-known-features-mask

Expand tests for Features::to_context

Merge pull request #1002 from valentinewallace/2021-07-fix-features-index-bounds

Test index-out-of-bounds in Features::to_context

When there are fewer known `from` feature bytes than known `to` feature
bytes, an index-out-of-bounds error can occur if the `from` features
have unknown features set in a byte past the greatest known `from`
feature byte.

Remove unnecessary feature test-only methods

Fix crash due to index-out-of-bounds in feature translation

This was reported by a user when trying to send a payment using the LDK
sample (specifically during route generation when translating a Features
from one context to another)

The problem was we didn't check T::KNOWN_FEATURE_MASK vec length before
indexing into it, due likely to the assumption that known feature vec
lengths are the same across contexts, when they may not be

Support pending update_fail_htlcs in reconnect_nodes test util