bitcoin: Inconsistent RPC error handling

Expected behavior

The behavior of returning error from RPC call should be consistent between these cases:

• Single RPC call fails
• Single forbidden RPC call
• Batched failed RPC call
• One (or more) method of the batched call is forbidden

Actual behavior

• Single failed RPC call returns HTTP error code
• Single forbidden RPC call returns HTTP error code - HTTP_FORBIDDEN (403)
• Batched failed RPC call leaks an internal error code
• Batched call containing a forbidden method returns HTTP_FORBIDDEN (403) for the whole batch.

To reproduce

Disclaimer: the bug was discovered by code analysis when trying to come up with a correct solution to https://github.com/MetacoSA/NBitcoin/issues/864 I’m fairly confident that using curl to call methods and then observing output would reproduce the issue.

Another way to see a problem might be running BTCPayServer/NBXplorer with the bitcoind version from master with some users restricted. (Example: restrict getpeerinfo and see that Payjoin stops working.) I didn’t try this with newest Core, but I hit it with btc-rpc-proxy, which returns HTTP errors for authentication just like what bitcoind does according to the code.

System information

Not relevant.

Discussion

I’m opening this issue in order to decide on proper handling of RPC errors. I see these possible approaches:

• RPC_METHOD_NOT_FOUND becomes public, the comment is removed and the rest of the code stays the same, the behavior doesn’t change.
• JSONErrorReply function is modified to return HTTP errors for batched calls too.
• New public error codes are defined and used for every method independently, the mapping to HTTP codes is removed (at least for batched calls)

I personally like the last one because it allows efficient inspection of node capabilities, but I’m worried that it might break some clients. I’d love to hear arguments from people who are more experienced in this area.