In our scoring logic we have a handful of unnecessary bounds,
leading to extra diff in the bindings branch when those bounds have
to be removed as well as a few cases where bindings generation
simply gets confused.
Here we remove a number of bounds across the scoring traits and
impls, cleaning things up and simplifying bindings changes.
In 6b0d94a302 we switched most tests
to `Default::default()` for scoring parameters passed to
route-fetching. Here we do the same for the scoring parameters when
passed to score-updating.
The reason for having a separate `parse_onion_address` from
`FromStr` is to have an onion parsing function in `no-std`, but
when we added it we forgot to make it public. We do this here, as
well as fix a few compilation warnings in `no-std`.
`UtxoLookup` doesn't strictly need to be referenced from the
`PeerManager`, and in fact the new `GossipVerifier` in
`lightning-block-sync` requires itself to be owned by the
`PeerManager` (for circular type reasons).
This allows us to use `lightning-block-sync`'s `GossipVerifier`
with `SimpleArcPeerManager` in ldk-sample.
We now support separate R/W locks in `LockableScore`, which allow
us to do routefinding in parallel, however in order to support
`WriteableScore` for such users we need to implement `Writeable`
for `RwLock` wrappers around `Writeable` types, which we do here.
Motivation: When there is an error while applying monitor_update to a
channel_monitor, we don't want to persist a 'monitor_update' which
results in a failure to apply later while reading 'channel_monitor' with
updates from storage. Instead, we should persist the entire 'channel_monitor'
here.
We were incorrectly marking updates as chain_sync
or not in test_utils based on whether monitor_update
is None or not. Instead, use UpdateOrigin to determine it.
We incorrectly assumed that the descriptor's output index from
second-stage HTLC transaction would always match the HTLC's output index
in the commitment transaction. This doesn't make any sense though, we
need to make sure we map the descriptor to it's corresponding HTLC in
the commitment. Instead, we check that the transaction from which the
descriptor originated from spends the HTLC in question.
Note that pre-anchors, second-stage HTLC transactions are always 1
input-1 output, so previously we would only match if the HTLC was the
first output in the commitment transaction. Post-anchors, they are
malleable, so we can aggregate multiple HTLC claims into a single
transaction making this even more likely to happen. Unfortunately, we
lacked proper coverage in this area so the bug went unnoticed. To
address this, we aim to extend our existing coverage of
`get_claimable_balances` to anchor outputs channels in the following
commits.
`to_remote` outputs on commitment transactions with anchor outputs have
an additional `1 CSV` constraint on its spending condition,
transitioning away from the previous P2WPKH script to a P2WSH.
Since our `ChannelMonitor` was never updated to track the proper
`to_remote` script on anchor outputs channels, we also missed updating
our signer to handle the new script changes.
While our commitment transactions did use the correct `to_remote`
script, the `ChannelMonitor`'s was not as it is tracked separately. This
would lead to users never receiving an `Event::SpendableOutputs` with a
`StaticPaymentOutput` descriptor to claim the funds.
Luckily, any users affected which had channel closures confirmed by a
counterparty commitment just need to replay the closing transaction to
receive the event.
This early return is only possible if the channel requires a single
confirmation, allowing a `channel_ready` message to go out. This can be
problematic though if a commitment transaction (specifically from the
counterparty, as the channel would be immediately closed if a local
commitment is broadcast) also confirms within the same block. The
`ChannelMonitor` will detect both, but it won't inform the
`ChannelManager` at all. Luckily, while the channel still is considered
open to the `ChannelManager`, the `ChannelMonitor` will reject any
further updates to the channel state.
While removing the `balance_msat` field absolutely makes sense -
it is, at best, confusing - we really need a solid replacement for
it before we can do so. While one such replacement is in progress,
it is not complete and we'd like to not block our current release
on its completion.
This reverts commit ef5be580f5.
Previously, we would only consider route hints if the entry point was
in our first hops or in the network graph. We fixed this by also
considering hints if our own node ID was the first src.
Here, we add test coverage for this behavior.
We previously added logic that would avoid adding superflous candidates
for route hints if we detect that we have a first hop for this channel.
Here we add test coverage that we actually prefer the first hop over the
route hint, but still consider the remaining hints.
Three levels of descendant transactions starting from the channel's
funding transaction should cover all potential spendable outputs.
The first level covers the commitment transaction.
The second level covers the to_self claims, to_remote claims,
second-stage HTLC claims and justice transactions.
The third levels covers the justice transactions on second-stage HTLCs,
and to_self claims on second-stage HTLCs.
This test adds coverage for receiving a preimage after seeing a
counterparty commitment confirm, followed by a reorg and the
confirmation of a different commitment instead.
The first test covers the case where a holder commitment confirms after
the counterparty commitment reorg.
The second test covers the case where a previous counterparty commitment
confirms after the latest counterparty commitment reorg.
We should always claim HTLCs from the currently confirmed commitment,
rather than always claiming from the latest or previous counterparty
commitment if we've seen either confirm onchain at a prior point.
Some nodes may rebroadcast their `ChannelUpdate` to their counterparty
on every connection establishment, which leads to us doing an additional
persist most of the time when nothing has changed. Now, we'll only
persist if we receive an update that changes anything.
