Skip to content

Commit

Permalink
Add MonitorUpdatingPersister
Browse files Browse the repository at this point in the history
MonitorUpdatingPersister is an implementation of Persister that stores ChannelMonitorUpdates separately from ChannelMonitors. Its RFC is in #2545, at https://github.com/orgs/lightningdevkit/discussions/2545.

In addition, some earlier commits are squashed in this one:

squash fixups

first pass of documentation

f various nits, make the struct public also

f single allocation for monitor_bytes vec

f use update name for sequence; refactor update block

f use update storage key for sequence

f coupla nits

add draft MonitorUpdatingPersister

no-list init algorithm

f no left padding

f - squashed fixups

f clarify migration example is just fs

maybe fix lock order issue in CI

f docfixes

f debug assertion to check updates are what they say they are

f refactor persist_new_channel flow control

f add logging and minor refactors to update_persisted_channel

correct comparison

fix duplicate line picked up in rebase

f qualify persist_new_channel

f qualify update_persisted_channel

f qualify read_channel_monitors_with_updates

f grammatical nit

f more grammar

f shorter first line

f qualify ErrorKind::NotFound

f linking in docs

f remove monitor_update_namespace as we no longer need to do this

f decrement the examples

f inline list_monitors

f no ref to release notes

f align doc bullets

f s/deserialize/read

f more map_err

f rename start/end to first/last

f clarify cleanup_stale_updates docs

f remove redundant todo

f less allocation writing monitor

f further disambiguate update_ids

f little more elegant into outpoint

f use our own conversion

f add a constructor for the MUP

f doc examples less elite

f remove trailing whitespace

f revert sentinel to 0xFFs

f more expressive debug assertion

f s/PermanentFailure/InProgress/g

f confirm no funding txo collision on write

f a little more trailing whitespace

f use different update windows for two test persisters

f remove superfluous if

f add test, remove avoidable bad deletes, fix nits

Adds a test for `clean_stale_updates`. Also fixes a bug that the test
uncovered, where the debug assertion would fail due to off-by-one
offsets and the special case of a closed channel, both of which
caused it to try reading updates that weren't present. I was able to
cure the offsets of course, but it's hard to avoid some deletes of
non-existent files in the closed channel case, since the update
fast-forwards to u64::MAX.

Add `clean_stale_updates` function.

This adds a public function to the MonitorUpdatingPersister for
out-of-band cleanup of updates. This is mostly here for convenience
in the event users need it, as it is pretty straightforward for us to
read `ChannelMonitors` and evaluate whether updates are stale.
More advanced users might want to parallelize the work, but in that
case they may use this as a reference but implement it in the form
that best suits their design.

In doing this, I also in-lined the `lazy_delete_updates` work into the
`persist_new_channel` function, as it ends up not being helpful for
the cleanup as one is driving deletes from iterating over a listing, so
this is the only place it's used.

f remove list-less cleanup_stale_updates, to be replaced later

Revert "f Error on checked add"

This reverts commit b358454.

f add a line for readability

f Error on checked add

The only way we could overflow here is if we actually read an update at u64::MAX, and that is no longer allowed to happen, so this is an error.

f s/update_id/current_update_id

f revert failures to UnrecoverableError

f fix downgrade guidance to path of least resistance

Add logic to reduce no-op delete requests.

We know that there will be no updates to clean up if the monitor we're writing is at the same update_id as the old one, or if it's one greater.

f erroneous variable name in comment

f remove superfluous delete at u64::MAX

f add monitor name to cleanup error message

f tweak comments and log error if overwriting a higher monitor_id

f inline ns/subns/key for the most part

f wider code formatting

f documentation copy errors

f doctest attribute to avoid a failed doctest

f reduce test iterations

update to use new UnrecoverableError variant instead of PermanentFailure

f more documentation changes

f use chanmon_cfg keys manager instead of node_cfg keys manager

f cleaner build of test fixture while still maintaining lock order

f less loopy tests, rename tests fns, a few other simplifications

f no magic number required

f make MonitorName and UpdateName pub(crate)

note this makes the UpdateName::new function unused, but I expect to use it when switching the cleanup back to a listing design.

f remove convenience method for making test persisters, as it was no longer very convenient

f it's okay to always derive debug

f use infallible conversions for CM/CMU in Persist, and touch up logging messages for error too

f log full path on CMU read failure

f fix allocation, variable names in read_channel_monitors_with_updates

f edits and fixes to documentation for MUP

f fix reference to outpoint in docs

f docfixes on cleanup_stale_updates

f minimize derived trait impls

f docfixes

f do checked add and break instead of sat add

fix doc error

fix lock order violation (finally)

remove accidental use of std

make unit test do more payments to force a lot of updates

also use a prime number so the pending updates don't align with the same channel state over and over.

one more refactor of update_persisted_channel

complete docs todos, reflow to 100 columns

f make persist_new_channel check for an existing monitor at greater update_id

further refactored update_persisted_channel for more readability

change to using stored channel monitor instead of hashmap for last update id

f clearer assertion code/comment

Co-Authored-By: Elias Rohrer <[email protected]>
  • Loading branch information
domZippilli and tnull committed Sep 26, 2023
1 parent 7ea280e commit 5783f8e
Show file tree
Hide file tree
Showing 2 changed files with 1,005 additions and 4 deletions.
Loading

0 comments on commit 5783f8e

Please sign in to comment.