Matt Corallo [Tue, 12 Mar 2024 14:29:45 +0000 (14:29 +0000)]
Log available liquidity on each channel when starting routefinding
One of the most common first-steps in troubleshooting routefinding
issues is we ask for the local channel state to determine what the
available HTLC bounds are. While we log first-hop channel details
when we decline to use them, this doesn't tell us if we have
missing channels, and thus here we log all first-hop channels at
the start.
We also take this opportunity to log the limits that were violated
any time we log that we're not using a channel, rather than only
when its a first-hop.
Add a method to BlindedPath that given a network graph will compact the
IntroductionNode as the DirectedShortChannelId variant. Call this method
from DefaultMessageRouter so that Offer paths use the compact
representation (along with reply paths). This leaves payment paths in
Bolt12Invoice using the NodeId variant, as the compact representation
isn't as useful there.
Jeffrey Czyz [Thu, 9 May 2024 22:31:37 +0000 (17:31 -0500)]
Pass ForwardNode when creating BlindedPath
Instead of passing Vec<PublicKey> to MessageRouter::crate_blinded_path,
pass Vec<ForwardNode>. This way callers can include a short_channel_id
for a more compact BlindedPath encoding.
When sending an onion message to a blinded path, the short channel id
between hops isn't need in each hop's encrypted_payload since it is not
a payment. However, using the short channel id instead of the node id
gives a more compact representation. Update BlindedPath::new_for_message
to allow for this.
Matt Corallo [Wed, 1 May 2024 14:32:47 +0000 (14:32 +0000)]
Don't attempt to resume channels if we already exchanged funding
ddf75afd16 introduced the ability to re-exchange our `ChannelOpen`
after a peer disconnects if we didn't complete funding on our end.
It did not implement nor consider what would happen if we
re-connected after we created our own funding transactions, and
currently it panics (and even if it did not it would replay the
`FundingTransactionGenerated` event to users).
While we'd very much like to replay the `open_channel` flow even
if we have already received an `accept_channel` and funded the
channel, we cannot as the peer will likely provide different key
material in their `accept_channel`, causing us to need a different
funding transaction.
Thus, here, we simply close channels which have been funded but not
yet signed when our peer disconnects.
Jeffrey Czyz [Fri, 24 May 2024 16:31:30 +0000 (11:31 -0500)]
Use HashMap::entry instead of HashMap::insert
This allows for obtaining the value without needing to re-look it up. An
upcoming commit will include RecipientOnionFields in the inserted value.
Having it available afterwards prevents needing to clone it.
Matt Corallo [Sun, 19 May 2024 18:35:04 +0000 (18:35 +0000)]
Log how many nodes/channels we have synced when we try to route
A common issue in LN is not having fully synced the network graph
when we attempt to send a payment. This should be improved
substantially with gossip v1.5, but for now we should improve our
debugability by logging how many nodes we have in our graph when
we attempt to find a route.
Filip Gospodinov [Fri, 17 May 2024 15:00:44 +0000 (17:00 +0200)]
Re-export bitcoin crate
For the same reason the `bitcoin` crate is re-exporting
the `secp256k1` crate the `lightning` crate should
re-export the `bitcoin` crate: to ease the burden on
calling code to maintain compatible `bitcoin` versions.
The `lightning` crate makes heavy use of types defined in
(or re-exported by) the `bitcoin` crate. Compilation will
fail if e.g. comparisons or `match` expressions are done with
types from `bitcoin` crate versions with a non-equal minor
version. This forces calling code to depend on a `bitcoin`
crate with a compatible version. This becomes a maintenance
nightmare once two or more crates, that use `bitcoin` types
in their public API, are used in calling code simultaneously.
Matt Corallo [Mon, 13 May 2024 14:36:01 +0000 (14:36 +0000)]
impl `Clone` on various public (mostly BOLT12) types
This is required for bindings as passing types from Rust to GC'd
languages can't map the concept of a type that has a lifetime of
the called function but instead needs to clone for safety.
Matt Corallo [Sat, 11 May 2024 18:51:48 +0000 (18:51 +0000)]
Make `offers::Amount` `Copy` and export it in bindings
`Amount` is less than two pointers long, so there's no reason it
shouldn't be `Copy`. Further, because its an enum, bindings can't
map a reference to it in an `Option`. Thus, here, we simply make it
`Copy` and return it in `Option`s rather than a reference to it.
Jeffrey Czyz [Fri, 10 May 2024 18:58:41 +0000 (13:58 -0500)]
Stop writing EcdsaChannelSigner
EcdsaChannelSigner is no longer deserialized as of version 0.0.113 and
downgrades before version 0.0.113 are no longer supported as of version
0.0.119.
Matt Corallo [Mon, 6 May 2024 01:23:56 +0000 (01:23 +0000)]
Ignore partially-pruned channels during routing
If we prune one side of a channel's `ChannelUpdateInfo` that means
the node hasn't been online for two weeks (as they haven't
generated a new `channel_update` in that time). In such cases, even
if we haven't yet pruned the channel entirely, we should definitely
not be treating these channels as candidates for routing.
Note that this requires some additional `channel_update`s in the
router tests, but all of the new ones are added as disabled
channels.
Matt Corallo [Tue, 30 Apr 2024 17:11:54 +0000 (17:11 +0000)]
Use consistent byte/char offsets when parsing invoice HRPs
When parsing lightning-invoice HRPs we want to read them
char-by-char, tracking at which offset different fields were. Prior
to this commit this was done first by reading char-by-char and then
by indexing using the byte offset which works for ASCII strings but
fails on multi-byte characters.
This commit fixes this issue by simply always walking byte-by-byte
and rejecting multi-byte characters which don't belong in HRPs.
Matt Corallo [Tue, 7 May 2024 16:34:36 +0000 (16:34 +0000)]
Fix the `full_stack_target` breakage test and doc feerate requests
When we added the additional deust exposure checks in 702196819e6445048b803574fcacef77d5ce8c9c we added several
additional feerate fetches which broke the `full_stack_target`
change-detection test.
This updates the hard-coded test to support the new feerate fetches
and also includes a comment on `FeeEstimator` to indicate that
users really need to be caching feerates as otherwise they'll slow
us down.
Matt Corallo [Sun, 5 May 2024 00:44:07 +0000 (00:44 +0000)]
Update default dust exposure limit and the documentation thereof
Now that we're including excess counterparty commitment transaction
fees in our dust calculation, we need to update the docs
accordingly. We do so here, describing some of the considerations
and risks that come with the new changes.
We also take this opportunity to double the default value, as users
have regularly complained that non-anchor channels fail to send
HTLCs with the default settings with some feerates.
Matt Corallo [Tue, 30 Apr 2024 20:42:36 +0000 (20:42 +0000)]
Include excess commitment transaction fees in dust exposure
Transaction fees on counterparty commitment transactions are
ultimately not our money and thus are really "dust" from our PoV -
they're funds that may be ours during off-chain updates but are not
ours once we go on-chain.
Thus, here, we count any such fees in excess of our own fee
estimates towards dust exposure. We don't bother to make an
inbound/outbound channel distinction here as in most cases users
will use `MaxDustExposure::FeeRateMultiplier` which will scale
with the fee we set on outbound channels anyway.
Note that this also enables the dust exposure checks on anchor
channels during feerate updates. We'd previously elided these as
increases in the channel feerates do not change the HTLC dust
exposure, but now do for the fee dust exposure.
Matt Corallo [Sun, 5 May 2024 19:52:58 +0000 (19:52 +0000)]
Provide more color in filter registration methods
A few years ago we had repeated user confusion dealing with the
`Filter` methods and exactly how they differ. Here we try to add a
bit more color in several ways as suggested by users in a few
places - listing examples of why the methods are used as well as
ensuring its clearly communicated that not all parameters need be
used.