hold_invoice_paid guard is read-then-act, not an atomic claim

[grunch]

Follow-up from the review of #853.

hold_invoice_paid (src/flow.rs) now guards on the order's status and invoice_held_at, but it reads them and then acts without atomicity. A concurrent writer on the main loop — cancel_order_by_maker in src/app/cancel.rs is the concrete one — can interleave between the read and the write, so the guard can pass against state that is already stale by the time the flow commits.

Adding invoice_held_at == 0 to the guard (done in #853) closes re-entry from LND redelivering Accepted. It does not close this race — that is a separate problem and needs a conditional write.

The codebase already has the right pattern: on_bond_invoice_accepted in src/app/bond/flow.rs claims the transition with an atomic UPDATE ... WHERE and branches on rows_affected, bailing cleanly when the claim misses. hold_invoice_paid should do the same rather than read-then-act.

Low severity — it needs an exact interleaving — but the fix is a known-good pattern already in the tree.

[grunch]

Important constraint for whoever picks this up, found while reviewing a suggestion to fix it inside #853.

An atomic claim cannot simply be moved ahead of the message sends. enqueue_order_msg (src/util.rs:1355) pushes onto MESSAGE_QUEUES.queue_order_msg, an in-memory RwLock<Vec<...>>. Nothing about it is durable.

That makes the current ordering — send messages, then write invoice_held_at — accidentally crash-safe in the useful direction. If the process dies in between, the queued messages are lost but invoice_held_at is still 0, so the replay on the next restart re-runs the flow and re-sends them.

Invert it to claim-first, and a crash after the claim loses those messages permanently and suppresses the replay that would have regenerated them. The buyer never receives AddInvoice and the order stalls with the escrow locked. That trades a duplicate-message bug for a lost-message bug, which is the worse of the two.

So this issue is really two pieces of work, in order:

1. A durable outbox (or writing the claim and the messages in one transaction), so work survives a crash after claiming.
2. The atomic UPDATE ... WHERE claim itself, branching on rows_affected as on_bond_invoice_accepted does.

Doing 2 without 1 is a regression.

One scoping note: the reachable race is the subscriber task versus a cancel on the main loop. Two Accepted deliveries racing each other does not appear reachable — invoice_subscribe has exactly two production call sites, show_hold_invoice (src/util.rs:1121) and the restart resubscribe (src/main.rs:248), which belong to different process lifetimes for the same order.

[ToRyVand]

I'd like to take this one, along with #810 and #809 — all three are the same read-then-act-instead-of-CAS defect, so the context carries across.

Verified against current main before claiming: hold_invoice_paid (src/flow.rs:54) still reads the status/invoice_held_at pair and then acts unconditionally.

Agreed on the fix direction you point at — on_bond_invoice_accepted (src/app/bond/flow.rs:278, claiming with rows_affected() == 1) is the right template, and admin_settle's guard is a second example in-tree.

Assignment request is on #810.

[grunch]

@ToRyVand I assigned to you #810 and #809 but please do each one in a different PR

[ToRyVand]

Confirmed the outbox is still needed after #910: claim the status, crash before the messages, and the replay early-returns on !is_pre_payment — messages gone. enqueue_order_msg is still in-memory.

Two questions: does the outbox belong inside #855 or its own issue (one PR per issue makes this one PR carrying both)? And #910 rewrites hold_invoice_paid, so #855 waits for it — confirm?