Expose async monitor persistence for tests

[tnull]

Downstream tests need to exercise the same async monitor persistence path used by ChainMonitor::new_async_beta without reimplementing Persist. Add test-only constructors that let TestChainMonitor and AsyncPersister share the wake notifier so async completions drive the monitor update future.

Co-Authored-By: HAL 9000

[ldk-reviews-bot]

👋 I see @joostjager was un-assigned.
If you'd like another reviewer assignment, please click here.

[ldk-claude-review-bot]

I've now reviewed the entire PR diff thoroughly — all three files, all hunks, all new methods, the test, and the cfg gating logic.

Summary of findings:

The refactoring of ChainMonitor::new into new + with_event_notifier is correct and preserves the same behavior. The new test-only constructors (AsyncPersister::new_test, ChainMonitor::new_with_event_notifier, TestChainMonitor::new_with_event_notifier) are properly cfg-gated and correctly plumb the shared Arc<Notifier> through. The with_deferred_and_event_notifier private helper handles both _test_utils and non-_test_utils compilation paths correctly, with consistent cfg guards ensuring no caller can pass Some(notifier) when the feature isn't enabled. The test correctly verifies that the shared notifier connects AsyncPersister and ChainMonitor.

No issues found.

[TheBlueMatt]

Hmm, its not entirely clear to me why this is needed? Can you spell out what kind of test you want to write and why it needs this? In general downstream tests (and, really, any tests) should just block the KVStore write they want to block, IMO.

[tnull]

Hmm, its not entirely clear to me why this is needed? Can you spell out what kind of test you want to write and why it needs this? In general downstream tests (and, really, any tests) should just block the KVStore write they want to block, IMO.

Yes see here: lightningdevkit/ldk-node#919 (comment)

This will allow us to do:

  type TestMonitorPersister<K> = MonitorUpdatingPersisterAsync<
        TestStoreRef<K>,
        TestFutureSpawner,
        &'static test_utils::TestLogger,
        &'static test_utils::TestKeysInterface,
        &'static test_utils::TestKeysInterface,
        &'static test_utils::TestBroadcaster,
        &'static test_utils::TestFeeEstimator,
  >;

  type TestAsyncPersister<K> = lightning::chain::chainmonitor::AsyncPersister<
        TestStoreRef<K>,
        TestFutureSpawner,
        &'static test_utils::TestLogger,
        &'static test_utils::TestKeysInterface,
        &'static test_utils::TestKeysInterface,
        &'static test_utils::TestBroadcaster,
        &'static test_utils::TestFeeEstimator,
  >;
(..)
  let monitor_persister = TestMonitorPersister::new(
        store,
        TestFutureSpawner::new(Arc::clone(&runtime)),
        &chanmon_cfg.logger,
        max_pending_updates,
        &chanmon_cfg.keys_manager,
        &chanmon_cfg.keys_manager,
        &chanmon_cfg.tx_broadcaster,
        &chanmon_cfg.fee_estimator,
  );

  let event_notifier = Arc::new(Notifier::new());
  let persister = lightning::chain::chainmonitor::AsyncPersister::new_test(
        monitor_persister,
        Arc::clone(&event_notifier),
  );

(...)

  test_utils::TestChainMonitor::new_with_event_notifier(
        Some(&chanmon_cfg.chain_source),
        &chanmon_cfg.tx_broadcaster,
        &chanmon_cfg.logger,
        &chanmon_cfg.fee_estimator,
        &persister.persister,
        &chanmon_cfg.keys_manager,
        Arc::clone(&persister.event_notifier),
  )

.. rather than duplicating code via our custom TestMonitorUpdatePersister we have in that PR branch currently (which doesn't actually exercise the MonitorUpdatingPersister we have in prod):

https://github.com/tnull/ldk-node/blob/9e095d154babc6a37ecbe8a967eceadf4bf4a3a6/src/io/test_utils.rs#L41-L158

[TheBlueMatt]

Are there some specific tests in LDK Node that need that struct vs just using the MonitorUpdatingPersister you use in prod and hooking any writes to pause/resume them at the KVStore layer?