Context
#4268 gave the manager a registry-owned coordinator lifecycle with Rust-owned FFI callback contexts, and shutdown() / destroy now drain the coordinators, SPV, and the payment hooks. #4185 (in review) adds a per-generation lifecycle gate for the deferred BIP70/BIP270 build → broadcast/release flow: a payment is finalized and registered under one wallet generation, and its later broadcast revalidates that the generation is still live before it touches the network.
These two landed independently and each is coherent on its own. This is a question about how they compose, not a report that either is wrong.
The question
shutdown() drains the coordinators, SPV, and the payment hooks, but as far as I can see it does not seal the per-generation lifecycle gates. A deferred payment that has already passed its liveness check — it observed a live generation and is on its way to the broadcaster — is not, at that point, any of the things the drain waits on. So the sequencing
- deferred payment passes its generation liveness check,
- host calls
shutdown() / destroy, which drains coordinators, SPV and payment hooks and returns,
- the in-flight payment reaches the broadcaster and sends,
appears to leave a window where a transaction can hit the network after the host believes the manager is destroyed.
To be clear about the severity, because I think it is narrower than it sounds:
This is memory-safe under #4268's owned callback contexts — a straggling worker keeps its context alive and releases it on exit, so there is no use-after-free and destroy correctly logs a non-clean join rather than erroring. The problem is not lifetime, it is observable effect: the broadcast is network-visible and irreversible, so a host that has torn the manager down can still have coins move afterwards, with no live object left to attribute it to or reconcile it against. Is a generation-gate drain phase in shutdown() wanted here, or is post-destroy broadcast of an already-authorised payment deliberately out of scope — on the grounds that the payment was authorised while the wallet was live and the reservation semantics already cover it?
Why it might be out of scope
There is a reasonable reading where this is fine by design: the payment was built, signed and authorised while the wallet was live, the merchant ack that triggers the broadcast is the host's own action, and cancelling it at teardown would arguably be worse than completing it — the user asked for it and the reservation was committed. If that is the intent, it would be worth stating, because the natural expectation of destroy() is "nothing further happens".
If it is in scope
The shape that seems to fit #4268's existing drain would be a phase that closes the per-generation gates to new entrants and waits for the ones already past their liveness check, before or alongside the coordinator drain — so that either the broadcast completes before shutdown() returns, or it is refused. I have not tried to implement it; the ordering against the existing coordinator/SPV drain is the part I would not want to guess at.
Happy to be told this is already handled somewhere I did not look, or that #4185's gate should own the whole problem rather than shutdown().
Refs: #4268, #4185.
Context
#4268 gave the manager a registry-owned coordinator lifecycle with Rust-owned FFI callback contexts, and
shutdown()/destroynow drain the coordinators, SPV, and the payment hooks. #4185 (in review) adds a per-generation lifecycle gate for the deferred BIP70/BIP270 build → broadcast/release flow: a payment is finalized and registered under one wallet generation, and its later broadcast revalidates that the generation is still live before it touches the network.These two landed independently and each is coherent on its own. This is a question about how they compose, not a report that either is wrong.
The question
shutdown()drains the coordinators, SPV, and the payment hooks, but as far as I can see it does not seal the per-generation lifecycle gates. A deferred payment that has already passed its liveness check — it observed a live generation and is on its way to the broadcaster — is not, at that point, any of the things the drain waits on. So the sequencingshutdown()/destroy, which drains coordinators, SPV and payment hooks and returns,appears to leave a window where a transaction can hit the network after the host believes the manager is destroyed.
To be clear about the severity, because I think it is narrower than it sounds:
Why it might be out of scope
There is a reasonable reading where this is fine by design: the payment was built, signed and authorised while the wallet was live, the merchant ack that triggers the broadcast is the host's own action, and cancelling it at teardown would arguably be worse than completing it — the user asked for it and the reservation was committed. If that is the intent, it would be worth stating, because the natural expectation of
destroy()is "nothing further happens".If it is in scope
The shape that seems to fit #4268's existing drain would be a phase that closes the per-generation gates to new entrants and waits for the ones already past their liveness check, before or alongside the coordinator drain — so that either the broadcast completes before
shutdown()returns, or it is refused. I have not tried to implement it; the ordering against the existing coordinator/SPV drain is the part I would not want to guess at.Happy to be told this is already handled somewhere I did not look, or that #4185's gate should own the whole problem rather than
shutdown().Refs: #4268, #4185.