rpc: Remove deprecated dummy alias for listtransactions::label #31413

The RPC arg is not a dummy, but a label, so offering an undocumented alias is inconsistent with all other label interfaces and confusing at best, if not entirely unused.

Fix it by removing the deprecated alias.

This pull is a breaking change, but it should be limited, because it only affects someone using the deprecated named arg on this RPC. I can't imagine anyone doing this, because in all other places where label args are accepted, they are called label. If someone really didn't use label here as named arg, it would be trivial and less confusing for them to fix it up.

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--006a51241073e994b41acfe9ec718e94-->

Code Coverage & Benchmarks

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

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline for information on the review process.

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #31250 (wallet: Disable creating and loading legacy wallets by achow101)
• #28710 (Remove the legacy wallet and BDB dependency by achow101)

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.

The code change seems trivial but my review would be helped if you can share some more context because Idk history around this dummy arg/alias.

From #18607 I found

Unrelated to that, there have been a bunch of aliases in the server code that are not present in the source code of the documentation.

Is this dummy alias not used in server code anymore? Can you share the commit for it?

I also found couple dummy RPC args in 2 other RPCs below. I'm guessing they can't be removed because they are not aliases but separate args instead?

src/wallet/rpc/coins.cpp:170:                    {"dummy", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Remains for b
ackward compatibility. Must be excluded or set to \"*\"."},
...
src/wallet/rpc/spend.cpp:330:                    {"dummy", RPCArg::Type::STR, RPCArg::Default{"\"\""}, "Must be set to
\"\" for backwards compatibility.",

Is this dummy alias not used in server code anymore? Can you share the commit for it?

It is used in server code in current master. This pull removes it, so the commit is in this pull.

I also found couple dummy RPC args in 2 other RPCs below. I'm guessing they can't be removed because they are not aliases but separate args instead?

Those args do not represent labels, as you can't send from a label, or calculate the balance of a label. They can be removed, but it would be a breaking change.

This pull is also a breaking change, but it should be limited, because it only affects someone using the deprecated named arg on this RPC. I can't imagine anyone doing this, because in all other places where label args are accepted, they are called label. If someone really didn't use label here as named arg, it would be trivial and less confusing for them to fix it up.

It is used in server code in current master. This pull removes it, so the commit is in this pull.

Ahh I see, I guess I got confused by the comment in the previous PR. Thanks for answering both the questions.

This pull is also a breaking change, but it should be limited, because it only affects someone using the deprecated named arg on this RPC. I can't imagine anyone doing this, because in all other places where label args are accepted, they are called label. If someone really didn't use label here as named arg, it would be trivial and less confusing for them to fix it up.

I believe this is good context and would be helpful to mention it in the PR description so that the merge script picks it up - specially because it's a breaking change although with a smaller impact.

tACK fa8e0956c23789fb819ad1b31377711df091ec1b

Concept ACK. Reading this I was confused what original purpose of adding the dummy alias in fa168d754221a83cab0d2984a02c41cf6819e873 from #18607 was if we don't want the alias now. But actually fa168d754221a83cab0d2984a02c41cf6819e873 was only documenting the alias not adding it, because before #20012 RPC method registration worked differently and aliases were registered and documented separately.

Going back further, the first listtransactions argument was originally account, then #12953 dropped it and changed it to dummy, then #14411 added it back as label. So there should be little reason for anyone to be using the dummy alias today, or ever since it could always be omitted.

Jup, thanks for writing out the historical context!

Code review ACK fa8e0956c23789fb819ad1b31377711df091ec1b

ACK fa8e0956c23789fb819ad1b31377711df091ec1b