index: improve TxoSpenderIndex readability and documentation #34748

1. 
   furszy commented at 9:00 pm on March 5, 2026: member

   This should help future ourselves and other devs to maintain the code. The code here took me a bit to grasp, so I imagine others could be in the same situation.

   First, TxoSpenderIndex::FindSpender returns an Expected<optional<TxoSpender>> but the two levels of the return type were undocumented, making it unclear what a returned nullopt means. So added doc clarifying each return case.

   Second, the result-building loop in gettxspendingprevout was not super readable to me, mainly due to the nested if/else chain and missing comments on what is going on along the process.

   Hope this helps others, if not, I can close it. It was helpful to me.

2. index: document TxoSpenderIndex::FindSpender

   Hard to know what a returned std::Expected(std::nullopt) mean
   if it is not documented anywhere.

   a0718d6fc5
3. DrahtBot added the label UTXO Db and Indexes on Mar 5, 2026
4. 
   DrahtBot commented at 9:01 pm on March 5, 2026: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

5. RPC: make gettxspendingprevout code more maintainable

   Clarifies when the index will be used, renames confusing
   `txospenderindex_ready` flag to `use_index`, and breaks up
   the nested if/else chain into documented sections.

   e40535a66b
6. furszy force-pushed on Mar 5, 2026
7. DrahtBot added the label CI failed on Mar 5, 2026
8. 
   fjahr commented at 9:33 pm on March 5, 2026: contributor

   Did you see my suggested refactoring of the RPC? https://github.com/fjahr/bitcoin/commit/a7b0e950abaae97f191a777c2e167813718a882d

   I just pulled this out of #34635 because it was controversial, see #34635 (review). But I will reopen it as a standalone PR.

   EDIT: Put it here: #34749

9. DrahtBot removed the label CI failed on Mar 5, 2026
10. 
    furszy commented at 11:12 pm on March 5, 2026: member

    Did you see my suggested refactoring of the RPC? fjahr@a7b0e95 I just pulled this out of #34635 because it was controversial, see #34635 (comment).

    Nope, I didn’t see it. Otherwise I would have ACKed the concept and left these changes there. I did it in the context of #34747, took me a bit to grasp the current code during review.

    Side topic, a new PR might not be needed, #34635 seems short as is now. Could add it there. . You already pushed it :p.

    And dunno about the “controversial” argument. Will check your code, but this one at least is purely adding documentation and breaking up the nested blocks. Nothing really serious.

11. 
    fjahr commented at 11:16 pm on March 5, 2026: contributor

    Side topic, a new PR might not be needed, #34635 seems short as is now. Could add it there.

    And dunno about the “controversial” argument.

    Thanks, please read #34635 (review) to see why I pulled it out :) My version does a bit more than your refactoring. Let’s see how we can converge on one improvement that everyone can agree on.

12. 
    furszy commented at 11:18 pm on March 5, 2026: member

    Thanks, please read #34635 (review) to see why I pulled it out :) My version does a bit more than your refactoring. Let’s see how we can converge on one improvement that everyone can agree on.

    Yeah absolutely, will do.

13. 
    furszy commented at 0:23 am on March 6, 2026: member

    Closing. Leaving it in @fjahr’s hands. Let’s continue on your PR.
14. furszy closed this on Mar 6, 2026

    ---

Contributors
furszy DrahtBot fjahr

Labels
UTXO Db and Indexes