`check_no_inputs_seen_before` callback should not need to manage its own state

[nothingmuch]

check_no_inputs_seen_before implicitly needs to record in some external state that an input was seen.

Secondly it has no indication for whether the related event / state transition was recorded in durable storage.

In principle we can handle this logic entirely within the session event log information given the ability to look at events from all sessions, but that kind of change would be somewhat complex due to being a potential performance bottleneck if every input check ends up replaying the events of all session. Addressing this concern is possible, either by caching the results of session log replay in secondary storage, or by allowing wallets to implement an extended API that that tracks seen inputs explicitly across sessions.

That said I think we should only revisit when we're ready to do more intelligent coin selection choices as well, allowing repeated sender inputs, both in regards to the same payment request or multiple ones, so long as the choice of receiver's inputs in the response is deterministic, so that the state machine only rejects a payjoin when/if that's no longer possible.

Original comment: https://github.com/payjoin/rust-payjoin/pull/883#pullrequestreview-3052252539_

[Johnosezele]

From my reading of #894 and the review thread on PR #883 the goal is to remove the side-effect (recording seen inputs) from check_no_inputs_seen_before and let the library supply that information instead.

Draft approach

1. Introduce a SeenInputsCache derived from the session event log. Cache would be populated lazily/replayed once per session batch and held behind an Arc<RwLock<…>> to avoid replay cost on every callback.
2. Extend the receiver builder with an (optional) seen_inputs_provider trait so integrators can plug in their own fast lookup if they already track this in a DB.
3. Change check_no_inputs_seen_before to a pure validation closure that just gets &seen_inputs and returns Result<(), Error>.

Does that direction align with your expectations, or would you prefer to wait until the broader coin-selection refactor is underway?

[DanGould]

This sounds like the sort of thing we can live with until 2.0. It's not quite a new feature but it's an optimization for something that's also not necessary to disrupt a 1.0, especially since OP mentions revisiting only when we're ready for more intelligent coin selection.