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.

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.

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

Fix channel reserve calculation on the sending side

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.

Merge pull request #990 from TheBlueMatt/2021-07-0.0.99

Cut 0.0.99

Bump most crate versions to 0.0.99 and lightning-invoice to 0.7.0

Add documentation for all PRs slated to land for 0.0.99

Merge pull request #975 from TheBlueMatt/2021-06-fix-fee-calc

Make the base fee configurable in ChannelConfig

Change serialization backwards compat in Channel to use new version

Instead of interpreting the backwards compatibility data in Channel
serialization, use the serialization version bump present in 0.0.99
as the flag to indicate if a channel should be read in backwards
compatibility.

Add a note clarifying the API guarantees of create_channel

Optionally reject HTLC forwards over priv chans with a new config

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

Update full_stack_target demo input to match new, fewer, fee gets

Make the base fee configurable in ChannelConfig

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.

Update ChannelConfig serialization to be TLV-based

This was missed prior to 0.0.98, so requires a
backwards-compatibility wrapper inside the `Channel` serialization
logic, but it's not very complicated to do so.

Merge pull request #988 from TheBlueMatt/2021-07-chan-details-usability

Improve ChannelDetails readability significantly.

Improve ChannelDetails readability significantly.

After the merge of #984, Jeff pointed out that `ChannelDetails` has
become a bit of a "bag of variables", and that a few of the variable
names in #984 were more confusing than necessary in context.

This addresses several issues by:
 * Splitting counterparty parameters into a separate
   `ChannelCounterpartyParameters` struct,
 * using the name `unspendable_punishment_reserve` for both outbound
   and inbound channel reserves, differentiating them based on their
   position in the counterparty parameters struct or not,
 * Using the name `force_close_spend_delay` instead of
   `spend_csv_on_our_commitment_funds` to better communicate what
   is occurring.

Merge pull request #961 from TheBlueMatt/2021-06-workaround-broken-cln

Use the query start block for ReplyChannelRange response messages

Use the query start block for ReplyChannelRange response messages

C-Lightning versions prior to 0.10 (incorrectly) enforce that the
reply_channel_range first_blocknum field is set to at least the
value they sent in their query_channel_range message. Sending a 0
results in them responding with an Error message, closing open
channels spuriously.

Further, C-Lightning versions prior to 0.10 require that the
reply_channel_range first_blocknum is either the same block implied
as the last block of the previous reply_channel_range or one
greater. This is not only a creative interpretation of the spec,
but a perfectly reasonable implementation might still receive an
Error message in the case of replies split by an empty block.

This code is extracted and modified from a previous version of
the original query_channel_range PR in commit
44ba52ccf10bb0362ed2964b66ec2ae51e388161. The original commit is by
`bmancini55 <bmancini@gmail.com>`.

Merge pull request #949 from TheBlueMatt/2021-06-send-priv-update

Send channel_update messages to direct peers on private channels

Ignore our own gossip if it is sent to us from our counterparty

If our channel party sends us our own channel_update message, we'll
erroneously use the information in that message to update our view
of the forwarding parameters our counterparty requires of us,
ultimately generating invoices with bogus forwarding information.

This fixes that behavior by checking the channel_update's
directionality before handling it.

Fix spelling in ChannelManager comment

Send channel_update messages to direct peers on private channels

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.