Add field name to output of invalidParams for OneOf (#906)

Fixes #901

src/rpc/RPCHelpers.cpp

@@ -1149,21 +1149,15 @@ parseBook(ripple::Currency pays, ripple::AccountID payIssuer, ripple::Currency g
 {
     if (isXRP(pays) && !isXRP(payIssuer))
         return Status{
-            RippledError::rpcSRC_ISR_MALFORMED,
+            RippledError::rpcSRC_ISR_MALFORMED, "Unneeded field 'taker_pays.issuer' for XRP currency specification."};
-            "Unneeded field 'taker_pays.issuer' for XRP currency "
-            "specification."};
 
     if (!isXRP(pays) && isXRP(payIssuer))
         return Status{
-            RippledError::rpcSRC_ISR_MALFORMED,
+            RippledError::rpcSRC_ISR_MALFORMED, "Invalid field 'taker_pays.issuer', expected non-XRP issuer."};
-            "Invalid field 'taker_pays.issuer', expected non-XRP "
-            "issuer."};
 
     if (ripple::isXRP(gets) && !ripple::isXRP(getIssuer))
         return Status{
-            RippledError::rpcDST_ISR_MALFORMED,
+            RippledError::rpcDST_ISR_MALFORMED, "Unneeded field 'taker_gets.issuer' for XRP currency specification."};
-            "Unneeded field 'taker_gets.issuer' for XRP currency "
-            "specification."};
 
     if (!ripple::isXRP(gets) && ripple::isXRP(getIssuer))
         return Status{
@@ -1233,15 +1227,11 @@ parseBook(boost::json::object const& request)
 
     if (isXRP(pay_currency) && !isXRP(pay_issuer))
         return Status{
-            RippledError::rpcSRC_ISR_MALFORMED,
+            RippledError::rpcSRC_ISR_MALFORMED, "Unneeded field 'taker_pays.issuer' for XRP currency specification."};
-            "Unneeded field 'taker_pays.issuer' for XRP currency "
-            "specification."};
 
     if (!isXRP(pay_currency) && isXRP(pay_issuer))
         return Status{
-            RippledError::rpcSRC_ISR_MALFORMED,
+            RippledError::rpcSRC_ISR_MALFORMED, "Invalid field 'taker_pays.issuer', expected non-XRP issuer."};
-            "Invalid field 'taker_pays.issuer', expected non-XRP "
-            "issuer."};
 
     if ((!isXRP(pay_currency)) && (!taker_pays.contains("issuer")))
         return Status{RippledError::rpcSRC_ISR_MALFORMED, "Missing non-XRP issuer."};
@@ -1258,9 +1248,7 @@ parseBook(boost::json::object const& request)
 
         if (get_issuer == ripple::noAccount())
             return Status{
-                RippledError::rpcDST_ISR_MALFORMED,
+                RippledError::rpcDST_ISR_MALFORMED, "Invalid field 'taker_gets.issuer', bad issuer account one."};
-                "Invalid field 'taker_gets.issuer', bad issuer account "
-                "one."};
     }
     else
     {
@@ -1269,9 +1257,7 @@ parseBook(boost::json::object const& request)
 
     if (ripple::isXRP(get_currency) && !ripple::isXRP(get_issuer))
         return Status{
-            RippledError::rpcDST_ISR_MALFORMED,
+            RippledError::rpcDST_ISR_MALFORMED, "Unneeded field 'taker_gets.issuer' for XRP currency specification."};
-            "Unneeded field 'taker_gets.issuer' for XRP currency "
-            "specification."};
 
     if (!ripple::isXRP(get_currency) && ripple::isXRP(get_issuer))
         return Status{

src/rpc/common/Validators.cpp

@@ -146,11 +146,7 @@ CustomValidator IssuerValidator =
 
         if (issuer == ripple::noAccount())
             return Error{Status{
-                RippledError::rpcINVALID_PARAMS,
+                RippledError::rpcINVALID_PARAMS, fmt::format("Invalid field '{}', bad issuer account one.", key)}};
-                fmt::format(
-                    "Invalid field '{}', bad issuer account "
-                    "one.",
-                    key)}};
 
         return MaybeError{};
     }};

src/rpc/common/Validators.h

@@ -417,7 +417,7 @@ public:
 
         auto const res = value_to<Type>(value.as_object().at(key.data()));
         if (std::find(std::begin(options_), std::end(options_), res) == std::end(options_))
-            return Error{Status{RippledError::rpcINVALID_PARAMS}};
+            return Error{Status{RippledError::rpcINVALID_PARAMS, fmt::format("Invalid field '{}'.", key)}};
 
         return {};
     }

src/rpc/handlers/AccountTx.cpp

@@ -105,10 +105,9 @@ AccountTxHandler::process(AccountTxHandler::Input input, Context const& ctx) con
     if (input.ledgerHash || input.ledgerIndex || input.usingValidatedLedger)
     {
         if (ctx.apiVersion > 1u && (input.ledgerIndexMax || input.ledgerIndexMin))
-        {
             return Error{Status{RippledError::rpcINVALID_PARAMS, "containsLedgerSpecifierAndRange"}};
-        }
+
-        else if (!input.ledgerIndexMax && !input.ledgerIndexMin)
+        if (!input.ledgerIndexMax && !input.ledgerIndexMin)
         {
             // mimic rippled, when both range and index specified, respect the range.
             // take ledger from ledgerHash or ledgerIndex only when range is not specified

src/rpc/handlers/BookOffers.h

@@ -95,7 +95,7 @@ public:
             // return INVALID_PARAMS if account format is wrong for "taker"
             {JS(taker),
              meta::WithCustomError{
-                 validation::AccountValidator, Status(RippledError::rpcINVALID_PARAMS, "Invalid field 'taker'")}},
+                 validation::AccountValidator, Status(RippledError::rpcINVALID_PARAMS, "Invalid field 'taker'.")}},
             {JS(limit),
              validation::Type<uint32_t>{},
              validation::Min(1u),

src/rpc/handlers/LedgerData.h

@@ -100,9 +100,7 @@ public:
              meta::WithCustomError{
                  validation::Type<std::string>{},
                  Status{ripple::rpcINVALID_PARAMS, "Invalid field 'type', not string."}},
-             meta::WithCustomError{
+             validation::OneOf<std::string>(TYPES_KEYS.cbegin(), TYPES_KEYS.cend())},
-                 validation::OneOf<std::string>(TYPES_KEYS.cbegin(), TYPES_KEYS.cend()),
-                 Status{ripple::rpcINVALID_PARAMS, "Invalid field 'type'."}}},
 
         };
         return rpcSpec;

unittests/rpc/handlers/AccountObjectsTests.cpp

@@ -85,7 +85,7 @@ generateTestValuesForParametersTest()
             "TypeInvalid",
             R"({"account":"rLEsXccBGNR3UPuPu2hUXPjziKC3qKSBun", "type":"wrong"})",
             "invalidParams",
-            "Invalid parameters."},
+            "Invalid field 'type'."},
         AccountObjectsParamTestCaseBundle{
             "LedgerHashInvalid",
             R"({"account":"rLEsXccBGNR3UPuPu2hUXPjziKC3qKSBun", "ledger_hash":"1"})",

unittests/rpc/handlers/BookOffersTests.cpp

@@ -256,7 +256,7 @@ generateParameterBookOffersTestBundles()
                 "taker": "123"
             })",
             "invalidParams",
-            "Invalid field 'taker'"},
+            "Invalid field 'taker'."},
         ParameterTestBundle{
             "TakerNotString",
             R"({
@@ -272,7 +272,7 @@ generateParameterBookOffersTestBundles()
                 "taker": 123
             })",
             "invalidParams",
-            "Invalid field 'taker'"},
+            "Invalid field 'taker'."},
         ParameterTestBundle{
             "LimitNotInt",
             R"({
@@ -384,8 +384,7 @@ generateParameterBookOffersTestBundles()
                 }
             })",
             "srcIsrMalformed",
-            "Unneeded field 'taker_pays.issuer' for XRP currency "
+            "Unneeded field 'taker_pays.issuer' for XRP currency specification."},
-            "specification."},
         ParameterTestBundle{
             "PaysCurrencyWithXRPIssuer",
             R"({
@@ -430,8 +429,7 @@ generateParameterBookOffersTestBundles()
                 }            
             })",
             "dstIsrMalformed",
-            "Unneeded field 'taker_gets.issuer' for XRP currency "
+            "Unneeded field 'taker_gets.issuer' for XRP currency specification."},
-            "specification."},
         ParameterTestBundle{
             "BadMarket",
             R"({

unittests/rpc/handlers/SubscribeTests.cpp

@@ -410,8 +410,7 @@ generateTestValuesForParametersTest()
                 ]
             })",
             "dstIsrMalformed",
-            "Unneeded field 'taker_gets.issuer' for XRP currency "
+            "Unneeded field 'taker_gets.issuer' for XRP currency specification."},
-            "specification."},
         SubscribeParamTestCaseBundle{
             "BooksItemTakerPaysXRPHasIssuer",
             R"({
@@ -430,8 +429,7 @@ generateTestValuesForParametersTest()
                 ]
             })",
             "srcIsrMalformed",
-            "Unneeded field 'taker_pays.issuer' for XRP currency "
+            "Unneeded field 'taker_pays.issuer' for XRP currency specification."},
-            "specification."},
         SubscribeParamTestCaseBundle{
             "BooksItemBadMartket",
             R"({

unittests/rpc/handlers/UnsubscribeTests.cpp

@@ -405,8 +405,7 @@ generateTestValuesForParametersTest()
                 ]
             })",
             "dstIsrMalformed",
-            "Unneeded field 'taker_gets.issuer' for XRP currency "
+            "Unneeded field 'taker_gets.issuer' for XRP currency specification."},
-            "specification."},
         UnsubscribeParamTestCaseBundle{
             "BooksItemTakerPaysXRPHasIssuer",
             R"({
@@ -426,8 +425,7 @@ generateTestValuesForParametersTest()
                 ]
             })",
             "srcIsrMalformed",
-            "Unneeded field 'taker_pays.issuer' for XRP currency "
+            "Unneeded field 'taker_pays.issuer' for XRP currency specification."},
-            "specification."},
         UnsubscribeParamTestCaseBundle{
             "BooksItemBadMartket",
             R"({