The prior name seems to reference onion decode errors specifically, when in
fact the error contents are generic failure codes for any error that occurs
during HTLC receipt.
935a716cc6 added new wrappers for the
various channel keys, including a payment_key. However, the
`payment_key` has been unused in lightning since the introduction
(and broad requiring) of the `static_remotekey` feature.
Thus, we simply remove it (and an incredibly stale TODO) here.
A previous commit introduced the `time` feature to gate the use of
`SystemTime` dependent APIs in `EsploraSyncClient`. It however omitted
doing the same for the Electrum side of things. Here, we address this
oversight.
If we receive an `OpenChannel` message without a `channel_type`
with `manually_accept_inbound_channels` set, we will `unwrap()`
`None`.
This is uncommon these days as most nodes support `channel_type`,
but sadly is rather trivial for a peer to hit for those with manual
channel acceptance enabled.
Reported in and fixes#2804. Luckily, the updated
`full_stack_target` has no issue reaching this issue quickly.
The bindings generator struggles a bit with the references in enum
variant fields in `CandidateRouteHop`. While we could probably fix
this, its much eaiser (and less risky) to inline the enum variant
fields from `CandidateRouteHop` into structs. This also lets us
make some of the fields non-public, which seems better at least for
the opaque `hint_idx` in the blinded paths.
If a peer provides a feerate which nears `u32::MAX`, we may
overflow calculating the dust buffer feerate, leading to spuriously
keeping non-anchor channels open when they should be force-closed.
If we try to open a channel with a peer that is disconnected (but
with which we have some other channels), we'll end up with an
unfunded channel which will lead to a panic when the peer
reconnects. Here we drop this debug assertion without bother to add
a new test, given this behavior will change in a PR very soon.
When contest delays are >= 0x8000, script pushes require an extra
byte to avoid being interpreted as a negative int. Thus, for
channels with CSV delays longer than ~7.5 months we may generate
transactions with slightly too little fee. This isn't really a huge
deal, but we should prefer to be conservative here, and slightly
too high fee in the general case is better than slightly too little
fee in other cases.
When we or our counterparty are updating the fees on the channel,
we currently check that the resulting balance is sufficient not
only to meet the reserve threshold, but also not push it below
dust. This isn't required in the BOLTs and may lead to spurious
force-closures (which would be a bit safer, but reserve should
always exceed the dust threshold).
Worse, the current logic is broken - it compares the output value
in *billionths of satoshis* to the dust limit in satoshis. Thus,
the code is borderline dead anyway, but can overflow for channels
with several million Bitcoin, causing the fuzzer to get mad (and
lead to spurious force-closures for few-billion-dollar channels).
If we get a `Feature` object which has excess zero bytes, we
shouldn't consider it a different `Feature` from another with the
same bits set, but no excess zero bytes. Here we fix both the
`Hash` and `PartialEq` implementation for `Features` to ignore
excess zero bytes.
62f8669654 added two
`htlc_maximum_msat.unwrap_or`s, but used a default value of 0,
spuriously causing all HTLCs to fail if we don't have an htlc
maximum value. This should be mostly harmless, but we should fix it
anyway.
In 4b5db8c3ce, `channelmanager::PendingHTLCRouting` was made
public, exposing a `FinalOnionHopData` field to the world. However,
`FinalOnionHopData` was left crate-private, making the enum
impossible to construct.
There isn't a strong reason for this (even though the
`FinalOnionHopData` API is somewhat confusing, being separated from
the rest of the onion structs), so we expose it here.
Since `lightning-invoice` now depends on the `bitcoin` crate
directly, also depending on the `bitcoin_hashes` crate is redundant
and just means we confuse users by setting the `std` flag only on
`bitcoin`. Thus, we drop the explicit dependency here and replace
it with `bitcoin::hashes`.
