Because `ChannelManager` doesn't have a corresponding `Channel`
after the channels are closed, we'd always used an `update_id` of
`u64::MAX` for any `ChannelMonitorUpdate`s we need to build after
the channel is closed.
This completely breaks the abstraction of `update_id`s and leaks
into persistence logic - because we might have more than one
`ChannelMonitorUpdate` with the same (`u64::MAX`) value, suddenly
instead of being able to safely use `update_id` as IDs, the
`MonitorUpdatingPersister` has to have special logic to handle
this.
Worse, because we don't have a unique ID with which to refer to
post-close `ChannelMonitorUpdate`s we cannot track when they
complete async persistence. This means we cannot properly support
async persist for forwarded payments where the inbound edge has hit
the chain prior to the preimage coming to us.
Here we rectify this by using consistent `update_id`s even after a
channel has closed. In order to do so we have to keep some state
for all channels for which the `ChannelMonitor` has not been
archived (after which point we can be confident we will not need to
update them). While this violates our long-standing policy of
having no state at all in `ChannelManager`s for closed channels,
its only a `(ChannelId, u64)` pair per channel, so shouldn't be
problematic for any of our users (as they already store a whole
honkin `ChannelMonitor` for these channels anyway).
While limited changes are made to the connection-count-limiting
logic, reviewers should carefully analyze the interactions the new
map created here has with that logic.
This uses the newly introduced conditional configuration checks that are
now configurable withint Cargo (beta).
This allows us to get rid of our custom python script that checks for
expected features and cfgs.
This does introduce a warning regarding the unknown lint in Cargo
versions prior to the current beta, but since these are not rustc errors,
they won't break any builds with the "-D warnings" RUSTFLAG.
Moving to this lint actually exposed the "strict" feature not being
present in the lightning-invoice crate, as our python script didnt
correctly parse the cfg_attr where it appeared.
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.
In this commit i added additional parameter `error_message` to
`force_close_sending_error`. This parameter will allow users to
configure error message and send to peers during the force closing
of channel.I have also updated the tests for this updated function.
To avoid confusion and for accuracy going forward, we remove this method
as it is inconsistent with channel IDs generated during V2 channel
establishment. If one wants to create a V1, funding outpoint-based
channel ID, then `ChannelId::v1_from_funding_outpoint` should be used
instead.
A large portion of the library has always made the assumption that having
the funding outpoint will always allow us to generate the channel ID.
This will not be the case anymore and we need to pass the channel ID along
where appropriate. All channels that could have been persisted up to this
point could only have used V1 establishment, so if some structures don't
store a channel ID for them they can safely fall back to the funding
outpoint-based version.
While there is no great way to handle a true failure to persist a
`ChannelMonitorUpdate`, it is confusing for users for there to be
no error variant at all on an I/O operation.
Thus, here we re-add the error variant removed over the past
handful of commits, but rather than handle it in a truly unsafe
way, we simply panic, optimizing for maximum mutex poisoning to
ensure any future operations fail and return immediately.
In the future, we may consider changing the handling of this to
instead set some "disconnect all peers and fail all operations"
bool to give the user a better chance to shutdown in a semi-orderly
fashion, but there's only so much that can be done in lightning if
we truly cannot persist new updates.
When a `ChannelMonitorUpdate` fails to apply, it generally means
we cannot reach our storage backend. This, in general, is a
critical issue, but is often only a transient issue.
Sadly, users see the failure variant and return it on any I/O
error, resulting in channel force-closures due to transient issues.
Users don't generally expect force-closes in most cases, and
luckily with async `ChannelMonitorUpdate`s supported we don't take
any risk by "delaying" the `ChannelMonitorUpdate` indefinitely.
Thus, here we drop the `PermanentFailure` variant entirely, making
all failures instead be "the update is in progress, but won't ever
complete", which is equivalent if we do not close the channel
automatically.
Firstly, we switch our BP over to use `FilesystemStore`, which also gives us test
coverage and ensures the compatibility.
Then, we remove the superseded `KVStorePersister` trait and
the `FilesystemPersister` code.
Rather than using the std benchmark framework (which isn't
maintained and is unlikely to get any further maintenance), we swap
for criterion, which at least gets us a variable number of test
runs so our benchmarks don't take forever.
We also fix the RGS benchmark to pass now that the file in use is
stale compared to today's date.
`rust-bitcoin v0.30.0` introduces concrete variants for data members of
block `Header`s. To avoid having to update these across every use, we
introduce new helpers to create dummy blocks and headers, such that the
update process is a bit more straight-forward.
