refactor: Include function info in runtime errors #34398

pull Chand-ra wants to merge 1 commits into bitcoin:master from Chand-ra:runtime_error changing 1 files +2 −2

Chand-ra commented at 5:15 AM on January 24, 2026: none

Some runtime errors in wallet/scriptpubkeyman.cpp include function information while others do not. Refactor the code to consistently include function info throughout the file.

refactor: Include function info in runtime errors

Some runtime errors in `wallet/scriptpubkeyman.cpp` include function
information while others do not. Refactor the code to consistently
include function info throughout the file.

1769114751

DrahtBot added the label Refactoring on Jan 24, 2026
DrahtBot commented at 5:15 AM on January 24, 2026: contributor



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



Code Coverage & Benchmarks

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



Reviews

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

in src/wallet/scriptpubkeyman.cpp:1438 in 1769114751

1434 | @@ -1435,7 +1435,7 @@ void DescriptorScriptPubKeyMan::SetCache(const DescriptorCache& cache)
1435 |          FlatSigningProvider out_keys;
1436 |          std::vector<CScript> scripts_temp;
1437 |          if (!m_wallet_descriptor.descriptor->ExpandFromCache(i, m_wallet_descriptor.cache, scripts_temp, out_keys)) {
1438 | -            throw std::runtime_error("Error: Unable to expand wallet descriptor from cache");
1439 | +            throw std::runtime_error(std::string(__func__) + ": Unable to expand wallet descriptor from cache");

maflcko commented at 9:35 AM on January 26, 2026:

Not sure what the goal or benefit here is.

I don't think there is any rule to mention the function name. And you are not even making it more consistent, because just 5 lines below, it is "missing" as well.

Also, there is not test coverage or steps to test this.

maflcko commented at 9:36 AM on January 26, 2026: member

Not sure about this. I think for internal checks, util/check should be used and for external checks, a unique error message should be use. Neither should manually include a function name.

maflcko commented at 9:51 AM on January 26, 2026: member

Also, the two are already unique:

sh-5.2$ git grep -W '"Unable to expand descriptor"' | grep --perl-regexp '^\S+='
src/wallet/scriptpubkeyman.cpp=void DescriptorScriptPubKeyMan::UpgradeDescriptorCache()

sh-5.2$ git grep -W '"Error: Unable to expand wallet descriptor from cache"' | grep --perl-regexp '^\S+='
src/wallet/scriptpubkeyman.cpp=void DescriptorScriptPubKeyMan::SetCache(const DescriptorCache& cache)

sedited commented at 12:40 PM on January 27, 2026: contributor

I think this PR should be closed it doesn't seem like it solves a need. If there is clear motivation for such a change it should address the problem more comprehensively.
fanquake commented at 12:52 PM on January 27, 2026: member

Closing for now.
fanquake closed this on Jan 27, 2026

Contributors

Labels

refactor: Include function info in runtime errors #34398

Code Coverage & Benchmarks

Reviews