Improve porting documentation for legacy-only wallet RPCs #25363

importaddress (and possibly other legacy-only wallet functions), when used on a descriptor wallet, return a fairly nondescript error:

$ ./bitcoin-cli -regtest importaddress test
error code: -4
error message:
This type of wallet does not support this command

The RPC help of importaddress doesn't mention nor further explain this incompatibility, nor what alternative to use instead (e.g. importdescriptors with addr(X) in this case).

This potentially applies to the following RPCs:

• importaddress (done in #25368)
• importprivkey
• importpubkey
• importmulti
• addmultisigaddress
• dumpprivkey
• getnewaddress
• dumpwallet
• importwallet
• sethdseed

Is the goal here to update the error messages to something more descriptive as well as the documentation for this RPC?

Documentation update should include that the command is incompatible with descriptor wallets as well as mention the alternative to use?

It looks like it is always the case that the specified wallet is a descriptor wallet if the JSONRPCError is thrown within EnsureLegacyScriptPubKeyMan and EnsureConstLegacyScriptPubKeyMan.

I've never touched documentation for this project before, is it just to edit the object returned by importaddress function in backup.cpp that returns a RPCHelpMan object?

Is the goal here to update the error messages to something more descriptive as well as the documentation for this RPC?

There are multiple possible angles here:

• Update the error message. Note that error messages have limited space and are not supposed to be documentation, though, so this can at most be a hint.

• Add RPC documentation. At least mention in the help that the functions don't work with descriptor wallets. Adding instructions on how it can be done with descriptor wallets is even better, though also see next point.

• Add a dedicated "porting guide" to port a code base from legacy wallets to descriptor wallets.

• Make the functions backwards compatible. I don't personally like this very much, as it's just something that will be deprecated and removed again later in the interest of not cluttering the API.

I've never touched documentation for this project before, is it just to edit the object returned by importaddress function in backup.cpp that returns a RPCHelpMan object?

Yes.

Apologies for interrupting, but I feel like lost while reading your comments and stuffs [...] I'm willing to find out "how exactly one is supposed to work on topics and for which purposes?" Please.

Kind regards.

How can I update error messages for one of the remaining rpc's? Do they still need improvement? I've reviewed this PR and relatively know what should I do, but I'm not sure whether the changes are still needed or not. The issue was opened 8 months ago.

@Riahiamirreza seems that #25680 aims to fix this issue. Maybe you could go there and help reviewing. And to #27034 if you like the importaddress compatibility idea.

@laanwj Hello, as #25680 merged, do you think this issue may be closed?

Yes, lgtm, thanks!