cli: Return more correct error on -norpccookiefile without -rpcpassword

maflcko commented at 9:05 am on March 27, 2026: member

This clarifies an error message to be more precise. When neither a cookie file, nor password are provided, the bitcoin-cli error message would be:

Incorrect rpcuser or rpcpassword.

Implying that the user can only fix the error by setting an rpcpassword. However, the user can also set -rpccookiesfile to fix the error. So the new error message makes more sense:

Could not locate RPC credentials. No authentication cookie could be found, and RPC password is not set.

Also, refactor to std::optional, which makes the code clearer.

DrahtBot renamed this:
~~cli: Return more correct error on -norpccookiefile without -rpcpassword~~
cli: Return more correct error on -norpccookiefile without -rpcpassword
on Mar 27, 2026

DrahtBot added the label Scripts and tools on Mar 27, 2026

maflcko force-pushed on Mar 27, 2026

DrahtBot commented at 9:05 am 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. A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#34520 (refactor: Add [[nodiscard]] to functions returning bool+mutable ref by maflcko)
#34342 (cli: Replace libevent usage with simple http client by fjahr)
#32297 (bitcoin-cli: Add -ipcconnect option by ryanofsky)

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.

DrahtBot added the label CI failed on Mar 27, 2026

maflcko commented at 9:11 am on March 27, 2026: member

I haven’t checked this, but I think the “regression” was introduced in commit 39cbd4f37c3d3a32cd993cbc78052d53f700989b, however, the commit does not mention whether it is a refactor or behavior change. cc @hodlinator as the author of that commit, you may be qualified to nack/ack this here.

Probably doesn’t matter much either way, but I think the prior message was minimally clearer.

DrahtBot removed the label CI failed on Mar 27, 2026

hodlinator commented at 10:43 am on March 27, 2026: contributor

I haven’t checked this, but I think the “regression” was introduced in commit 39cbd4f, however, the commit does not mention whether it is a refactor or behavior change. cc @hodlinator as the author of that commit, you may be qualified to nack/ack this here.

Probably doesn’t matter much either way, but I think the prior message was minimally clearer.

I think the reasoning for return true; // -norpccookiefile in GetAuthCookie() was that if the user intentionally disables the authentication cookie, we should not set failedToGetAuthCookie = true; in bitcoin-cli.cpp. The error it results in, “Authorization failed: Incorrect rpcuser or rpcpassword” is fairly correct, as we only call GetAuthCookie() if the password is empty - Explicitly disabling the cookie file means you instead need to provide a password.

Your PR description states:

This clarifies an error message to be more precise. When neither a cookie file, nor password are provided, the bitcoin-cli error message would be:

Incorrect rpcuser or rpcpassword.

That is incorrect/imprecise. With default settings we try to read from the default cookie file location and would output “Could not locate RPC credentials.” etc if it does not exist. Only when the user passes -norpccookiefile or sets -rpcpassword to non-empty do we get the error message.

maflcko commented at 11:02 am on March 27, 2026: member

Only when the user passes -norpccookiefile or sets -rpcpassword to non-empty do we get the error message.

I don’t think this is true, see also the pre-existing test cases: As soon as -rpcpassword is non-empty, the error will be Incorrect rpcuser or rpcpassword. The goal here is to restore the error message to say “missing credentials …”, when neither credential has been set at all. (Possibly due to a stale and forgotten norpccookiefile=1 in the config file, or so)

I know it is a bit confusing, and that is why I created this pull request :sweat_smile:

maflcko force-pushed on Mar 27, 2026

DrahtBot added the label Needs rebase on Mar 31, 2026

cli: Return more correct error on -norpccookiefile without -rpcpassword

This is a minor change of the error message. Previously it was:

> Incorrect rpcuser or rpcpassword.

Implying that the user can only fix the error by setting an rpcpassword.
However, the user can also set -rpccookiesfile to fix the error. So the
new error message makes more sense:

> Could not locate RPC credentials. No authentication cookie could be found, and RPC password is not set.

faeeb965de

refactor: Use std::optional<std::string> for RPC credentials in bitcoin-cli

Previously, a separate boolean and string was used, which is brittle and
hard to understand (see the prior commit that fixes a minor issue)

fac6754157

maflcko force-pushed on Mar 31, 2026

maflcko commented at 7:53 am on March 31, 2026: member

rebased

hodlinator commented at 8:27 am on March 31, 2026: contributor

Only when the user passes -norpccookiefile or sets -rpcpassword to non-empty do we get the error message.

I don’t think this is true

Do you mean with the changes in this PR or on master? I was thinking about behavior on master.

see also the pre-existing test cases: As soon as -rpcpassword is non-empty, the error will be Incorrect rpcuser or rpcpassword. The goal here is to restore the error message to say “missing credentials …”, when neither credential has been set at all. (Possibly due to a stale and forgotten norpccookiefile=1 in the config file, or so)

I know it is a bit confusing, and that is why I created this pull request 😅

The current version on master does not take into account a user forgetting norpccookiefile=1 in the config file.

“Missing credentials” upon failing to read the cookie file is arguably incorrect both on master and in this PR.

I would prefer something closer to this branch with repurposes GenerateAuthCookieResult and has more fine grained informative error messages: https://github.com/bitcoin/bitcoin/compare/master...hodlinator:bitcoin:pr/34935_suggestions

DrahtBot removed the label Needs rebase on Mar 31, 2026

maflcko commented at 9:25 am on March 31, 2026: member

Yeah, I guess when touching this, may as well also clarify the error message in all cases. Happy to ack your branch, if you create a pull :)

cli: Return more correct error on -norpccookiefile without -rpcpassword #34935

Reviews

Conflicts