Use the `hex-conservative` crate directly from `bitcoin` instead of from
`hashes`. Although it makes no real difference it is slightly more clear
and more terse.
The `rust-bitcoin` project is working towards making the public API
separate from the directory structure; eventually the
`bitcoin::blockdata` will go away, to make maintenance easier here stop
using the `blockdata` module.
Do not run the formatter, so as to make review easier. This patch was
created mechanically using:
search-and-replace bitcoin::blockdata bitcoin
and having defined
```bash
search-and-replace () {
if (($# != 2))
then
echo "Usage: $0 <this> <that>"
return
fi
local this="$1"
local that="$2"
for file in $(git grep -l "$this")
do
perl -pi -e "s/$this/$that/g" "$file"
done
}
```
Now that `ChannelManager::send_payment_with_route` is deprecated,
we don't care too much about making it as effecient as possible, so
there's not much cost to making it take `Route` by value. This
avoids bindings being unsure if the by-reference `Route` passed
needs to outlive the `ChannelManager` itself or if it only needs to
outlive the method call, creating some call overhead by forcing a
`Route::clone`, but avoiding a memory leak.
When we update a channel, then while connecting a block persist a
full `ChannelMonitor` prior to persisting the
`ChannelMonitorUpdate`, users can end up seeing a full
`ChannelMonitor` with a given `latest_update_id` prior to seeing
the `ChannelMonitorUpdate` with the same `update_id`. This
could cause users to have a full `ChannelMonitor` on disk as well
as a `ChannelMonitorUpdate` which was already applied. While this
isn't an issue for the LDK-provided update-based `Persist`, its
somewhat surprising for users so we avoid it.
Previously, we would require our users to handle all events
successfully inline or panic will trying to do so. If they would exit
the `EventHandler` any other way we'd forget about the event and
wouldn't replay them after restart.
Here, we implement fallible event handling, allowing the user to return
`Err(())` which signals to our event providers they should abort event
processing and replay any unhandled events later (i.e., in the next
invocation).
In cc78b77c71 it was discovered that
`impl_writeable_tlv_based_enum_upgradable` wasn't actually
upgradable - tuple variants weren't written with length-prefixes,
causing downgrades with new tuple variants to be unreadable by
older clients as they wouldn't know where to stop reading.
This was fixed by simply assuming that any new variants will be
non-tuple variants with a length prefix, but no code write-side
changes were made, allowing new code to freely continue to use the
broken tuple-variant serialization.
Here we address this be defining yet more serialization macros
which aren't broken, and convert existing usage of the existing
macros using non-length-prefixed tuple variants to renamed
`*_legacy` macros.
Note that this changes the serialization format of
`impl_writeable_tlv_based_enum[_upgradable]` when tuple fields are
written, and as such deliberately changes the call semantics for
such tuples.
Only the serialization format of `MessageContext` is changed here
which is fine as it has not yet reached a release of LDK.
Currently, every block connection triggers the persistence of all
ChannelMonitors with an updated best_block. This approach poses
challenges for large node operators managing thousands of channels.
Furthermore, it leads to a thundering herd problem
(https://en.wikipedia.org/wiki/Thundering_herd_problem), overwhelming
the storage with simultaneous requests.
To address this issue, we now persist ChannelMonitors at a
regular cadence, spreading their persistence across blocks to
mitigate spikes in write operations.
Outcome: After doing this, Ldk's IO footprint should be reduced
by ~50 times. The processing time required to sync each block
will be significantly reduced, particularly for nodes with 1000s
of channels, as write latency plays a significant role in this process.
As a result, the Node/ChainMonitor will be blocked for a shorter
duration, leading to further efficiency gains.
This removes dependency of watched_outputs from
per_commitment_claimable_outpoints, it is required since we will
no longer have direct access to per_commitment_claimable_outpoints
once we start publishing PersistClaimInfo as part of #3049.
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.
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.
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.
Fixes#1069
When we receive a new block we may generate
`Event::SpendableOutputs` in `ChannelMonitor`s which then need to
be processed by the background processor. While it will do so
eventually when its normal loop goes around, this may cause user
tests to be delayed in finding events, so we should notify the BP
immediately to wake it on new blocks.
We implement that here, unconditionally notifying the
`background-processor` whenever we receive a new block or confirmed
transactions.
The `PaymentHash`, `PaymentSecret`, `PaymentPreimage`, and
`ChannelId` types are all small wrappers around `[u8; 32]` and are
used throughout the codebase but were defined in the top-level
`ln/mod.rs` file and the relatively sparsely-populated
`ln/channel_id.rs` file.
Here we move them to a common `types` module and go ahead and
update all our in-crate `use` statements to refer to the new
module for bindings. We do, however, leave a `pub use` alias for
the old paths to avoid upgrade hassle for users.
MonitorUpdateId was an opaque abstraction for id's generated by
UpdateOrigin:Offchain and UpdateOrigin::ChainSync monitor updates.
It was mainly needed to map calls made to
ChainMonitor::channel_monitor_updated. We no longer track
UpdateOrigin::ChainSync MonitorUpdates and can directly use
ChannelMonitor::get_latest_update_id() for tracking
UpdateOrigin::Offchain monitor updates.
We only used to store last_chain_persist_height to release
events held for more than LATENCY_GRACE_PERIOD_BLOCKS due to
pending monitor update with UpdateOrigin::ChainSync. Since we no
longer pause events for ChainSync persistence, we no longer need to
store last_chain_persist_height.
We used to wait on ChannelMonitor persistence to avoid
duplicate payment events. But this can still happen in cases where
ChannelMonitor handed the event to ChannelManager and we did not persist
ChannelManager after event handling.
It is expected to receive payment duplicate events and clients should handle these
events in an idempotent manner. Removing this hold-up of events simplifies
the logic and makes it easier to not persist ChannelMonitors on every block connect.
When a `ChannelMonitor[Update]` persistence completes, we rely on
logging in `ChannelManager` to hear about it. However, this won't
happen at all if there's still pending updates as no `MonitorEvent`
will be generated.
Thus, here, we add logging directly in `ChainMonitor`, ensuring we
can deduce when individual updates completed from debug logs.
