RPC: Add getrpcwhitelist method #19117

Revive #18827

This PR implements a new RPC, getrpcwhitelist, that returns whitelisted methods for the calling user.

Example:

./src/bitcoin-cli -regtest getrpcwhitelist
{
  "methods": [
    "getbalance",
    "getrpcwhitelist",
    "getwalletinfo"
  ]
}

A new functional test, rpc_getrpcwhitelist, and a release-notes entry are included.

Fixes #18822

Uses a JSON Object for methods for future extensions.

Not clear what it should do for users that don't have a whitelist (ie, can access all methods). Currently errors I think.

ACK after documenting the circular dep in the linter:

A new circular dependency in the form of "httprpc -> rpc/server -> httprpc" appears to have been introduced.

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #17805 (test: Add test for rpcwhitelistdefault by emilengler)

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.

@MarcoFalke Done

Uses a JSON Object for methods for future extensions

Any idea in mind?

Any idea in mind?

The developer notes recommend it.

@MarcoFalke I mean nothing comes to my mind that could be assigned to each whitelist'ed method.

I think we could name this getrpccredentials and it could return { "whitelist": ["..."] } or {} for when the user doesn't have a whitelist. IMO this is more extensible.

Nothing comes to my mind either, but the key-value pair make the return value a bit more self-documenting. Also, there should be no downside just following the dev notes.

I think we could name this getrpccredentials

I have pushed e8b18af58523635cf5b27b792a14925748030488 with the above suggestion.

Concept ACK. Will review.

I do have an idea for extending the whitelist (I actually wanted to suggest it before I learned about whitelist being extendable!): each RPC call could be versioned using semver (two numbers should be enough) so that then the clients can very precisely detect compatibility. New options are added to various RPC methods over time, so it'd be great for the client to know what the node supports. I'd even suggest starting right away and give each method version "1.0".

The advantage over just versioning whole RPC is higher flexibility - one breaking method doesn't have to affect compatibility with a client that doesn't use it. It increases maintenance burden, but hopefully not by too much? Bumping a number whenever RPC method is changed doesn't seem too bad.

@Kixunil this PR is about returning the whitelist of the current user, not about feature detection nor about interface versioning - IMO server version is enough for the client - so please lets discuss it in the right place.

@promag fair enough, thanks for clarifying!

@promag "Server versioning" is not a good method for feature discovery... Knots for example has many RPCs before they're available in Core.

And if there are multiple ways to do something, now you'd have 2 steps: first, figure out if the version supports the "better" way, then check if the "better" way is not excluded by a whitelist.

Knots for example has many RPCs before they're available in Core.

Then, the help RPC should already return them, no?

Yes, my response was to @promag's proposal that server versioning is a good way to do it. It isn't.

@promag returning the whitelist is kinda fundamentally about feature detection, and that leads to an interesting note...

Whitelists can contain RPCs that don't exist, so it's possible to misuse the output here if you assume the whitelist is valid rpcs.

We probably do want to figure out a robust feature detection call and filter against the whitelists...

@JeremyRubin IIUC this RPC exposes configuration data and, like you say, might reference inexistent RPC, especially if one goes back and forth different versions with same configuration.

We probably do want to figure out a robust feature detection call and filter against the whitelists...

Right, that's why I suggested to discuss feature discovery elsewhere.

I think we shouldn't make an RPC for whitelists potentially, we should just add a feature discovery API (which is why it's relevant to discuss here)

I wouldn't consider them exclusive but in that case it makes sense to discuss here.

What's the use case for purely sending the whitelist anyway? How it could be useful without other things like feature detection?

What's the use case for purely sending the whitelist anyway? How it could be useful without other things like feature detection? @Kixunil please see #12248 (comment) for related discussion.

Looks like we should take a different approach then? (to avoid whitelist usage and combine with feature discovery)

@promag so do I understand correctly the reason why write just whitelist is because it's simple?

Seems like a generic feature discovery method is a better approach, so closing this

@luke-jr do you have a reference on this? I like feature discovery!

When we discussed in core dev meeting everyone was nacking any generic feature discovery feature.