👋 Thanks for assigning @TheBlueMatt as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

No issues found.

The implementation changed since my prior review pass (it now compares through the already-held guard via Arc::ptr_eq rather than dropping/re-locking the guard), but the new logic is correct:

• The deadlock is avoided by not re-locking future.state when the pending entry is Arc::ptr_eq to it; the comparison reads replacement_messages (the held guard) directly.
• At compare time async_messages.channel_announce still holds the previous announcement (the new one is written after the check_replace_previous_entry call at utxo.rs:475), so it correctly compares the incoming message against the existing pending one.
• The else branch's unsafe_well_ordered_double_lock_self only runs for a genuinely different mutex, preserving the new→old lock order.
• resolve_single_future runs only under the internal lock held throughout, so no interleaving can lose updates.
• The regression test is valid (shared SCID 0 and fixed bitcoin keys make good_script match the replacement).

One non-blocking observation (not a regression, and strictly better than the deadlock being fixed): with a reused future for the same SCID but a different announcement, there is a single channel_announce slot, so the second announcement overwrites the first and resolve_single_future (utxo.rs:538) processes only the survivor. The nearby comment at utxo.rs:367 ("both results will be handled") only holds when the two requests have distinct future states. This is acceptable for the same-SCID case since the funding output is fixed and only one set of bitcoin keys can match the on-chain script.

Updated to line-wrap commit messages.

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

Not entirely sure anyone would ever implement it this way, but it seems the correct fix is to handle the duplicate with an Arc::ptr_eq check in check_replace_previous_entry, not unlock+relock.

Now updated with this approach. Let me know if you find that preferable, or if we should even consider not fixing this?

IMO we should skip backporting this. I don't believe any of our implementations of the lookup interface will ever return the same future and implementing that interface by caching pending lookups and returning copies of the future seems like a lot of work that implementers aren't gonna do. LMK if you disagree @tnull

IMO we should skip backporting this. I don't believe any of our implementations of the lookup interface will ever return the same future and implementing that interface by caching pending lookups and returning copies of the future seems like a lot of work that implementers aren't gonna do. LMK if you disagree @tnull

Fine by me.