refactor: Return std::optional over bool+mut& #34938

1. 
   maflcko commented at 1:35 pm on March 27, 2026: member

   Using a bool to indicate whether a mutable in-out param was written to is fine in legacy code, but otherwise confusing and brittle:

   • Sometimes the in-out-param is written to, even when the function returns false, like in ParseDouble.
   • Call sites must manually check the return value

   Fix those issues by returning std::optional<_> from ParseDouble (and a few other functions).

   This refactor is a style cleanup and does not change any behavior.

2. DrahtBot added the label Refactoring on Mar 27, 2026
3. 
   DrahtBot commented at 1:35 pm on March 27, 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.

   Type	Reviewers
   ACK	stickies-v, hodlinator
   Concept ACK	hebasto

   If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #34806 (refactor: logging: Various API improvements by ajtowns)
   • #34520 (refactor: Add [[nodiscard]] to functions returning bool+mutable ref by maflcko)
   • #34038 (logging: replace -loglevel with -trace, expose trace logging via RPC by ajtowns)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

4. 
   stickies-v commented at 1:53 pm on March 27, 2026: contributor

   Concept ACK
5. 
   hebasto commented at 3:02 pm on March 27, 2026: member

   Concept ACK.
6. in src/logging.h:300 in fa6d4d8972

   297@@ -297,6 +298,6 @@ static inline bool LogAcceptCategory(BCLog::LogFlags category, BCLog::Level leve
   298 }
   299 
   300 /** Return true if str parses as a log category and set the flag */
   

   ---

   ---

   

   stickies-v commented at 4:33 pm on March 27, 2026:

   nit: docstring out of date

   ---

   

   maflcko commented at 7:04 am on March 30, 2026:

   thx, fixed up
7. stickies-v approved
8. 
   stickies-v commented at 4:48 pm on March 27, 2026: contributor

   ACK fa6d4d8972e3e57a81f521126cd411f460501c03

   Easier to read and reason, straightforward changes.

9. DrahtBot requested review from hebasto on Mar 27, 2026
10. refactor: Return std::optional from GetLogCategory fafb0c4cbe
11. refactor: Return std::optional from GetWalletNameFromJSONRPCRequest fa0a09441d
12. refactor: Return std::optional from ParseDouble fabab69e9e
13. maflcko force-pushed on Mar 30, 2026
14. 
    stickies-v commented at 12:06 pm on March 30, 2026: contributor

    re-ACK fabab69e9e864053dfd75acf2ab4ba7420970d05

    No changes except fix stale docstring

15. hodlinator approved
16. 
    hodlinator commented at 3:39 pm on March 30, 2026: contributor

    crACK fabab69e9e864053dfd75acf2ab4ba7420970d05

    Straightforward improvement to functions called from few places.

Contributors
maflcko DrahtBot stickies-v hebasto hodlinator

Review Requested
hebasto

Labels
Refactoring