rpc: Use type-safe exception to pass RPC help #32660

The current "catch-all" catch (const std::exception& e) in CRPCTable::help is problematic, because it could catch exceptions unrelated to passing the help string up.

Fix this by using a dedicated exception type.

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--006a51241073e994b41acfe9ec718e94-->

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32660.

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline for information on the review process.

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

<!--5faf32d7da4f0f540f40219e4f7537a3-->

Can be tested by adding a bug, like

diff --git a/src/rpc/signmessage.cpp b/src/rpc/signmessage.cpp
index 5597f8d237..d62893bf0c 100644
--- a/src/rpc/signmessage.cpp
+++ b/src/rpc/signmessage.cpp
@@ -19,7 +19,7 @@ static RPCHelpMan verifymessage()
     return RPCHelpMan{"verifymessage",
         "Verify a signed message.",
         {
-            {"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The bitcoin address to use for the signature."},
+            {"addres\ns", RPCArg::Type::STR, RPCArg::Optional::NO, "The bitcoin address to use for the signature."},
             {"signature", RPCArg::Type::STR, RPCArg::Optional::NO, "The signature provided by the signer in base 64 encoding (see signmessage)."},
             {"message", RPCArg::Type::STR, RPCArg::Optional::NO, "The message that was signed."},
         },

On master, the bug is hidden. On this pull, it is more obvious. For example, ./test/functional/rpc_help.py or the help RPC fail.

code review ACK fa0cf42380dec17e73d38aa69dd70662a0bc344a

It doesn't seem like this is catching the issue in #29136 (review)? I tried making a similar change to a RPC in this PR and the test still passed.

It doesn't seem like this is catching the issue in #29136 (comment)? I tried making a similar change to a RPC in this PR and the test still passed.

Are you sure? What are the exact steps to reproduce?

Locally such a bug passes for me on current master:

$ git checkout bitcoin-core/master && git diff && cmakeb && ./bld-cmake/test/functional/rpc_help.py 
M	src/rpc/signmessage.cpp
HEAD is now at 88b22acc3d Merge bitcoin/bitcoin#32528: rpc: Round verificationprogress to 1 for a recent tip


diff --git a/src/rpc/signmessage.cpp b/src/rpc/signmessage.cpp
index 5597f8d237..18af6d110e 100644
--- a/src/rpc/signmessage.cpp
+++ b/src/rpc/signmessage.cpp
@@ -17,7 +17,7 @@
 static RPCHelpMan verifymessage()
 {
     return RPCHelpMan{"verifymessage",
-        "Verify a signed message.",
+        "\nVerify a signed message.",
         {
             {"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The bitcoin address to use for the signature."},
             {"signature", RPCArg::Type::STR, RPCArg::Optional::NO, "The signature provided by the signer in base 64 encoding (see signmessage)."},

whereas on this pull, the same diff fails.

whereas on this pull, the same diff fails.

Work for me:

./build/test/functional/rpc_help.py                        
2025-06-03T07:21:05.244000Z TestFramework (INFO): PRNG seed is: 8715551814237508387
2025-06-03T07:21:05.244000Z TestFramework (INFO): Initializing test directory /var/folders/sq/z88fhjzj0b19ftsd2_bjrmjm0000gn/T/bitcoin_func_test_ks0xf8o9
2025-06-03T07:21:06.291000Z TestFramework (ERROR): JSONRPC error
Traceback (most recent call last):
  File "/bitcoin/test/functional/test_framework/test_framework.py", line 189, in main
    self.run_test()
    ~~~~~~~~~~~~~^^
  File "/bitcoin/./build/test/functional/rpc_help.py", line 53, in run_test
    self.test_categories()
    ~~~~~~~~~~~~~~~~~~~~^^
  File "/bitcoin/./build/test/functional/rpc_help.py", line 102, in test_categories
    titles = [line[3:-3] for line in node.help().splitlines() if line.startswith('==')]
                                     ~~~~~~~~~^^
  File "/bitcoin/test/functional/test_framework/coverage.py", line 50, in __call__
    return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
  File "/bitcoin/test/functional/test_framework/authproxy.py", line 151, in __call__
    raise JSONRPCException(response['error'], status)
test_framework.authproxy.JSONRPCException: Internal bug detected: !m_description.starts_with('\n')
../../src/rpc/util.cpp:796 (ToString)
Bitcoin Core v29.99.0-6497cef9bdc8-dirty
Please report this issue here: https://github.com/bitcoin/bitcoin/issues
 (-1)
2025-06-03T07:21:06.343000Z TestFramework (INFO): Not stopping nodes as test failed. The dangling processes will be cleaned up later.

./build/bin/bitcoin-cli help    
error code: -1
error message:
Internal bug detected: !m_description.starts_with('\n')
../../src/rpc/util.cpp:796 (ToString)
Bitcoin Core v29.99.0-6497cef9bdc8-dirty
Please report this issue here: https://github.com/bitcoin/bitcoin/issues

ACK fa0cf42380dec17e73d38aa69dd70662a0bc344a

<details> <summary> Diff for testing </summary>

➜  bitcoin git:(2506-rpc-help) ✗ git diff --color | cat
diff --git a/src/rpc/signmessage.cpp b/src/rpc/signmessage.cpp
index 5597f8d237..18af6d110e 100644
--- a/src/rpc/signmessage.cpp
+++ b/src/rpc/signmessage.cpp
@@ -17,7 +17,7 @@
 static RPCHelpMan verifymessage()
 {
     return RPCHelpMan{"verifymessage",
-        "Verify a signed message.",
+        "\nVerify a signed message.",
         {
             {"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The bitcoin address to use for the signature."},
             {"signature", RPCArg::Type::STR, RPCArg::Optional::NO, "The signature provided by the signer in base 64 encoding (see signmessage)."},

</details>

<details> <summary> Branch CLI behaviour: proper error is thrown in the RPC </summary>

➜  bitcoin git:(2506-rpc-help) ✗ bitcoinclireg help verifymessage  
error code: -1
error message:
Internal bug detected: !m_description.starts_with('\n')
rpc/util.cpp:796 (ToString)
Bitcoin Core v29.99.0-fa0cf42380de-dirty
Please report this issue here: https://github.com/bitcoin/bitcoin/issues

➜  bitcoin git:(2506-rpc-help) ✗ bitcoinclireg help
error code: -1
error message:
Internal bug detected: !m_description.starts_with('\n')
rpc/util.cpp:796 (ToString)
Bitcoin Core v29.99.0-fa0cf42380de-dirty
Please report this issue here: https://github.com/bitcoin/bitcoin/issues

</details>

<details> <summary> Master CLI behaviour: error message is treated as the RPC response </summary>

➜  bitcoin git:(master) ✗ bitcoinclireg help verifymessage
Internal bug detected: !m_description.starts_with('\n')
rpc/util.cpp:796 (ToString)
Bitcoin Core v29.99.0-4b1d48a6866b-dirty
Please report this issue here: https://github.com/bitcoin/bitcoin/issues

➜  bitcoin git:(master) ✗ bitcoinclireg help
...
...
signmessagewithprivkey "privkey" "message"
validateaddress "address"
Internal bug detected: !m_description.starts_with('\n')

== Wallet ==
abandontransaction "txid"

</details>

The rpc_help functional test also fails with the diff in the branch but not in master.

Are you sure? What are the exact steps to reproduce?

Nope.

Not sure what I did wrong yesterday, but tried again and it does seem to work.

I retested the scenario with "address\n;)", running

cmake -B build -G Ninja -DCMAKE_BUILD_TYPE=Debug && ninja -C build && build/test/functional/test_runner.py  rpc_help

and got:

Internal bug detected: s.m_left.find('\n') == std::string::npos

on master it's:

rpc_help.py | ✓ Passed

👍

tested ACK fa946520d229ae45b30519bccc9eaa2c47b4a093 (edited)

Reapplied the tests from #32660 (comment) - passes without the changes, fails with the change as expected!

<details> <summary>the diff compared to previous ack is indeed only the rebase and the exception rename + commit message update</summary>

git fetch origin fa0cf42380dec17e73d38aa69dd70662a0bc344a fa946520d229ae45b30519bccc9eaa2c47b4a093 git range-diff master fa946520d229ae45b30519bccc9eaa2c47b4a093 fa0cf42380dec17e73d38aa69dd70662a0bc344a

1:  fa97f9b78e ! 1:  eeeec1579e rpc: Use type-safe HelpException
@@ Metadata
 Author: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
 
## Commit message ##
-    rpc: Use type-safe HelpException
+    rpc: Use type-safe exception to pass RPC help
 
## src/rpc/server.cpp ##
@@ src/rpc/server.cpp: std::string CRPCTable::help(const std::string& strCommand, const JSONRPCRequest&
@@ src/rpc/server.cpp: std::string CRPCTable::help(const std::string& strCommand, c
 -        {
 -            // Help text is returned in an exception
 -            std::string strHelp = std::string(e.what());
-+        } catch (const HelpException& e) {
++        } catch (const HelpResult& e) {
 +            std::string strHelp{e.what()};
              if (strCommand == "")
              {
@@ src/rpc/util.cpp: UniValue RPCHelpMan::HandleRequest(const JSONRPCRequest& reque
       */
      if (request.mode == JSONRPCRequest::GET_HELP || !IsValidNumArgs(request.params.size())) {
 -        throw std::runtime_error(ToString());
-+        throw HelpException{ToString()};
++        throw HelpResult{ToString()};
      }
      UniValue arg_mismatch{UniValue::VOBJ};
      for (size_t i{0}; i < m_args.size(); ++i) {
@@ src/rpc/util.h: class FillableSigningProvider;
  class CScript;
  struct Sections;
  
-+struct HelpException : std::runtime_error {
-+    explicit HelpException(const std::string& msg) : std::runtime_error{msg} {}
++struct HelpResult : std::runtime_error {
++    explicit HelpResult(const std::string& msg) : std::runtime_error{msg} {}
 +};
 +
  /**
2:  fa0cf42380 = 2:  fa946520d2 refactor: Use structured binding for-loop

</details>

re-ACK fa946520d229ae45b30519bccc9eaa2c47b4a093

git range-diff fa0cf42...fa94652

Thanks for accepting the naming suggestion.

ACK fa946520d229ae45b30519bccc9eaa2c47b4a093