The excess delta is included in the final RouteHop::cltv_expiry_delta, so by
adding it explicitly to cur_cltv we were erroneously including it twice in the
total cltv expiry.
This could've add up to an extra MAX_SHADOW_CLTV_DELTA_OFFSET (432) blocks to
the total cltv expiry.
Previously, we were setting the final blinded hop's CLTV expiry height to
best_block_height + total_blinded_path_cltv_delta + shadow_cltv_offset. This is
incorrect, it should instead be set to best_block_height + shadow_cltv_offset
only -- it doesn't make sense to include the delta for the other blinded hops
in the final hop's expiry.
The reason this too-high final cltv value didn't cause test failures previously
is because of a 2nd bug that is fixed in an upcoming commit where the sender
adds the shadow offset twice to the total path CLTV expiry. This 2nd offset
meant that intermediate nodes had some buffer CLTV to subtract their delta from
while still (usually) have enough leftover to meet the expiry in the final hop's
onion.
When we originally added the `onion_message` module, there weren't
a lot of public items in it, and it didn't make a lot of sense to
export the whole sub-module structure publicly. So, instead, we
exported the public items via re-exports directly in the
`onion_message` top-level module. However, as time went on, more
and more things entered the module, which left the top-level module
rather cluttered.
Worse, in 0.0.119, we exposed
`onion_message::messenger::SendSuccess` via the return type of
`send_message`, but forgot to re-export the enum itself, making
it impossible to actually use from external code.
Here we address both issues and simply replace the re-export with
the underlying sub-module structure.
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.
When users do async monitor updating, it may not be the case that
all pending monitors will complete updating at once. Thus, we
should fuzz monitor updates completing out of order, which we do
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.
In e06484b0f4, we added specific
handling for outbound-channel initial monitor updates failing -
in such a case we have a counterparty who tried to open a second
channel with the same funding info we just gave them, causing us
to force-close our outbound channel as it shows up as
duplicate-funding. Its largely harmless as it leads to a spurious
force-closure of a channel with a peer doing something absurd,
however it causes the `full_stack_target` fuzzer to fail.
Sadly, in 574c77e7bc, as we were
dropping handling of `PermanentFailure` handling for updates, we
accidentally dropped handling for initial updates as well.
Here we fix the issue (again) and add a test.
We'd previously assumed that LDK would receive
`funding_transaction_generated` prior to our peer learning the txid
and panicked if the peer tried to open a redundant channel to us
with the same funding outpoint.
While this assumption is generally safe, some users may have
out-of-band protocols where they notify their LSP about a funding
outpoint first, or this may be violated in the future with
collaborative transaction construction protocols, i.e. the upcoming
dual-funding protocol.
Currently the channel shutdown sequence has a number of steps which
all the shutdown callsites have to call. Because many shutdown
cases are rare error cases, its relatively easy to miss a call and
leave users without `Event`s or miss some important cleanup.
One of those steps, calling `issue_channel_close_events`, is rather
easy to remove, as it only generates two events, which can simply
be moved to another shutdown step.
Here we remove `issue_channel_close_events` by moving
`ChannelClosed` event generation into `finish_force_close_channel`.
Currently the channel shutdown sequence has a number of steps which
all the shutdown callsites have to call. Because many shutdown
cases are rare error cases, its relatively easy to miss a call and
leave users without `Event`s or miss some important cleanup.
One of those steps, calling `issue_channel_close_events`, is rather
easy to remove, as it only generates two events, which can simply
be moved to another shutdown step.
Here we move the first of the two events, `DiscardFunding`, into
`finish_force_close_channel`.
