Introduce a new “check alias” helper for RPC commands, which checks and moves the values into the “primary” key.
The first commit simply pushKV’s the value from the alias, if set, whereas the second commit const-casts the keys vector in UniValue and modifies that directly, i.e. “rename”. Some people (cough sipa cough) might object to the approach, so I’m splitting into two commits.
This is partially also a cleanup of #16378.
78+ const std::map<std::string, std::string>& aliases)
79+{
80+ for (const auto& alias : aliases) {
81+ switch (o.exists(alias.first) + o.exists(alias.second)) {
82+ case 2: throw JSONRPCError(RPC_INVALID_PARAMETER, "May not use both " + alias.first + " and " + alias.second + " simultaneously.");
83+ case 1: if (o.exists(alias.second)) o.pushKV(alias.first, o[alias.second]);
0 if (o.exists(alias.first) && o.exists(alias.second)) {
1 throw JSONRPCError(RPC_INVALID_PARAMETER, "May not use both " + alias.first + " and " + alias.second + " simultaneously.");
2 }
3 if (o.exists(alias.second)) o.pushKV(alias.first, o[alias.second]);
2988@@ -2982,9 +2989,8 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f
2989 coinControl.m_add_inputs = options["add_inputs"].get_bool();
2990 }
2991
2992- if (options.exists("changeAddress") || options.exists("change_address")) {
2993- const std::string change_address_str = (options.exists("change_address") ? options["change_address"] : options["changeAddress"]).get_str();
2994- CTxDestination dest = DecodeDestination(change_address_str);
2995+ if (options.exists("change_address")) {
2996+ CTxDestination dest = DecodeDestination(options["change_address"].get_str());
const
Concept ACK, not sure about the approach.
I think that we could improve consistency if internally all keys are transformed like lower(key.replace('_', '')) This way alias aren’t necessary. This would also help when dumping help output - keys could be formatted to snake_case for instance.
fun-draw-transaction command on this innocent transaction and send it off and it should give you a free lottery ticket where you can potentially win a bitcoin!!). This might be a good thing to do in between, even if we do go that route though.
@MarcoFalke That sounds like a good idea. I can think of a number of ways to do that:
static RPCArg& Aliased(RPCArg&& arg, const std::string& alias) that sets the alias inside and returns the same object.I’m leaning towards number 2 on that list but may change my mind if it turns out to be ugly.
One issue with doing it in RPCArg is that it requires duplication e.g. for callers of FundTransaction().
Oh it looks like RPCArg already has m_names (separated by |). This was primarily for the outer most args (named args), but it may be possible to extend to keys of a dictionary as well.
it requires duplication
The PRCArg vector or initializer list is a C++ struct, so it can be easily deduplicated and re-used. Even parts of it, which can be concatenated with our Cat vector util helper.
@promag That sounds like a good idea but people pointed out that it could be potentially riskful (use the
fun-draw-transactioncommand on this innocent transaction and send it off and it should give you a free lottery ticket where you can potentially win a bitcoin!!). This might be a good thing to do in between, even if we do go that route though.
The suggestion is to just transform args and json keys, not command names or arg values.