refactor: Add [[nodiscard]] to functions returning bool+mutable ref #34520

1. 
   maflcko commented at 4:58 pm on February 5, 2026: member

   Legacy code often used a function signature of the form bool Get...(const& in, mut& in_out); to implement a getter that optionally returns a value.

   In modern code, this can be replaced by std::optional<_> Get...(const& in);.

   However, some legacy code remains. To ensure that the “imaginary optional” is not unwrapped when the getter returns false, add a C++17 [[nodiscard]] attribute to all those legacy functions.

   There were only a few places that ignored the return value. I’ve fixed them in separate commits.

   This should be easy to review via --word-diff-regex=. and then confirming that the attribute was added to such a getter.

2. refactor: Add [[nodiscard]] to GetOp/GetScriptOp

   Also, use the return value in the test. Otherwise, compilation warns:
   
   src/test/transaction_tests.cpp:906:9: warning: ignoring return value of function declared with 'nodiscard' attribute [-Wunused-result]
     906 |         t.vin[0].scriptSig.GetOp(pc, opcode); // advance to next op
         |         ^~~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~
   1 warning generated.

   fa3a40a4a8
3. refactor: Add [[nodiscard]] to GetCachedLastHardenedExtPubKey

   Also, mark the return value as unused in one place, which is checked in
   the line after. This avoids a compile warning:
   
   src/script/descriptor.cpp:552:13: warning: ignoring return value of function declared with 'nodiscard' attribute [-Wunused-result]
     552 |             cache->GetCachedLastHardenedExtPubKey(m_expr_index, xpub);
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~
   1 warning generated.

   fa18a0c2d9
4. DrahtBot renamed this:
   refactor: Add [[nodiscard]] to functions returning bool+mutable ref
   refactor: Add [[nodiscard]] to functions returning bool+mutable ref
   on Feb 5, 2026
5. DrahtBot added the label Refactoring on Feb 5, 2026
6. 
   DrahtBot commented at 4:58 pm on February 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.

   Type	Reviewers
   ACK	HowHsu

   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:

   • #21283 (Implement BIP 370 PSBTv2 by achow101)

   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.

   LLM Linter (✨ experimental)

   Possible places where named args for integral literals may be used (e.g. func(x, /*named_arg=*/0) in C++, and func(x, named_arg=0) in Python):

   • [GetScriptOp(pc, end(), opcodeRet, nullptr)] in src/script/script.h

   2026-02-05 18:21:43

7. refactor: Add [[nodiscard]] to GetProxy

   Also, refactor the rpc code to check the return value (which is
   identical to proxy.IsValid()). Otherwise, the compiler would warn:
   
   src/rpc/net.cpp:622:9: warning: ignoring return value of function declared with 'nodiscard' attribute [-Wunused-result]
     622 |         GetProxy(network, proxy);
         |         ^~~~~~~~ ~~~~~~~~~~~~~~
   1 warning generated.

   fa17ce6465
8. refactor: Add [[nodiscard]] to functions returning bool+mutable ref fac5685e86
9. maflcko force-pushed on Feb 5, 2026
10. 
    HowHsu commented at 11:15 am on February 6, 2026: none

    ACK fac5685

Contributors
maflcko DrahtBot HowHsu

Labels
Refactoring