Account tx v1 api support (#874)

* Don't fail on ledger params for v1

* Different error on invalid ledger indexes for v1

* Allow forward and binary to be not bool for v1

* Minor fixes

* Fix tests

* Don't fail if input ledger index is out of range for v1

* Restore deleted test

* Fix comparison of integers with different signedness

* Updated default api version in README and example config

unittests/rpc/handlers/AccountTxTests.cpp

@@ -45,8 +45,9 @@ struct AccountTxParamTestCaseBundle
 {
     std::string testName;
     std::string testJson;
-    std::string expectedError;
-    std::string expectedErrorMessage;
+    std::optional<std::string> expectedError;
+    std::optional<std::string> expectedErrorMessage;
+    std::uint32_t apiVersion = 2;
 };
 
 // parameterized test cases for parameters check
@@ -58,91 +59,102 @@ struct AccountTxParameterTest : public RPCAccountTxHandlerTest, public WithParam
         std::string
         operator()(const testing::TestParamInfo<ParamType>& info) const
         {
-            auto bundle = static_cast<AccountTxParamTestCaseBundle>(info.param);
-            return bundle.testName;
+            return info.param.testName;
         }
     };
-};
 
-static auto
-generateTestValuesForParametersTest()
-{
-    return std::vector<AccountTxParamTestCaseBundle>{
-        AccountTxParamTestCaseBundle{"MissingAccount", R"({})", "invalidParams", "Required field 'account' missing"},
-        AccountTxParamTestCaseBundle{
-            "BinaryNotBool",
-            R"({"account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", "binary": 1})",
-            "invalidParams",
-            "Invalid parameters."},
-        AccountTxParamTestCaseBundle{
-            "ForwardNotBool",
-            R"({"account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", "forward": 1})",
-            "invalidParams",
-            "Invalid parameters."},
-        AccountTxParamTestCaseBundle{
-            "ledger_index_minNotInt",
-            R"({"account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", "ledger_index_min": "x"})",
-            "invalidParams",
-            "Invalid parameters."},
-        AccountTxParamTestCaseBundle{
-            "ledger_index_maxNotInt",
-            R"({"account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", "ledger_index_max": "x"})",
-            "invalidParams",
-            "Invalid parameters."},
-        AccountTxParamTestCaseBundle{
-            "ledger_indexInvalid",
-            R"({"account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", "ledger_index": "x"})",
-            "invalidParams",
-            "ledgerIndexMalformed"},
-        AccountTxParamTestCaseBundle{
-            "ledger_hashInvalid",
-            R"({"account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", "ledger_hash": "x"})",
-            "invalidParams",
-            "ledger_hashMalformed"},
-        AccountTxParamTestCaseBundle{
-            "ledger_hashNotString",
-            R"({"account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", "ledger_hash": 123})",
-            "invalidParams",
-            "ledger_hashNotString"},
-        AccountTxParamTestCaseBundle{
-            "limitNotInt",
-            R"({"account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", "limit": "123"})",
-            "invalidParams",
-            "Invalid parameters."},
-        AccountTxParamTestCaseBundle{
-            "limitNegative",
-            R"({"account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", "limit": -1})",
-            "invalidParams",
-            "Invalid parameters."},
-        AccountTxParamTestCaseBundle{
-            "limitZero",
-            R"({"account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", "limit": 0})",
-            "invalidParams",
-            "Invalid parameters."},
-        AccountTxParamTestCaseBundle{
-            "MarkerNotObject",
-            R"({"account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", "marker": 101})",
-            "invalidParams",
-            "invalidMarker"},
-        AccountTxParamTestCaseBundle{
-            "MarkerMissingSeq",
-            R"({
+    static auto
+    generateTestValuesForParametersTest()
+    {
+        return std::vector<AccountTxParamTestCaseBundle>{
+            AccountTxParamTestCaseBundle{
+                "MissingAccount", R"({})", "invalidParams", "Required field 'account' missing"},
+            AccountTxParamTestCaseBundle{
+                "BinaryNotBool",
+                R"({"account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", "binary": 1})",
+                "invalidParams",
+                "Invalid parameters."},
+            AccountTxParamTestCaseBundle{
+                "BinaryNotBool_API_v1",
+                R"({"account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", "binary": 1})",
+                std::nullopt,
+                std::nullopt,
+                1u},
+            AccountTxParamTestCaseBundle{
+                "ForwardNotBool",
+                R"({"account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", "forward": 1})",
+                "invalidParams",
+                "Invalid parameters."},
+            AccountTxParamTestCaseBundle{
+                "ForwardNotBool_API_v1",
+                R"({"account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", "forward": 1})",
+                std::nullopt,
+                std::nullopt,
+                1u},
+            AccountTxParamTestCaseBundle{
+                "ledger_index_minNotInt",
+                R"({"account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", "ledger_index_min": "x"})",
+                "invalidParams",
+                "Invalid parameters."},
+            AccountTxParamTestCaseBundle{
+                "ledger_index_maxNotInt",
+                R"({"account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", "ledger_index_max": "x"})",
+                "invalidParams",
+                "Invalid parameters."},
+            AccountTxParamTestCaseBundle{
+                "ledger_indexInvalid",
+                R"({"account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", "ledger_index": "x"})",
+                "invalidParams",
+                "ledgerIndexMalformed"},
+            AccountTxParamTestCaseBundle{
+                "ledger_hashInvalid",
+                R"({"account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", "ledger_hash": "x"})",
+                "invalidParams",
+                "ledger_hashMalformed"},
+            AccountTxParamTestCaseBundle{
+                "ledger_hashNotString",
+                R"({"account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", "ledger_hash": 123})",
+                "invalidParams",
+                "ledger_hashNotString"},
+            AccountTxParamTestCaseBundle{
+                "limitNotInt",
+                R"({"account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", "limit": "123"})",
+                "invalidParams",
+                "Invalid parameters."},
+            AccountTxParamTestCaseBundle{
+                "limitNegative",
+                R"({"account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", "limit": -1})",
+                "invalidParams",
+                "Invalid parameters."},
+            AccountTxParamTestCaseBundle{
+                "limitZero",
+                R"({"account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", "limit": 0})",
+                "invalidParams",
+                "Invalid parameters."},
+            AccountTxParamTestCaseBundle{
+                "MarkerNotObject",
+                R"({"account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", "marker": 101})",
+                "invalidParams",
+                "invalidMarker"},
+            AccountTxParamTestCaseBundle{
+                "MarkerMissingSeq",
+                R"({
                 "account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
                 "marker": {"ledger": 123}
             })",
-            "invalidParams",
-            "Required field 'seq' missing"},
-        AccountTxParamTestCaseBundle{
-            "MarkerMissingLedger",
-            R"({
+                "invalidParams",
+                "Required field 'seq' missing"},
+            AccountTxParamTestCaseBundle{
+                "MarkerMissingLedger",
+                R"({
                 "account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
                 "marker":{"seq": 123}
             })",
-            "invalidParams",
-            "Required field 'ledger' missing"},
-        AccountTxParamTestCaseBundle{
-            "MarkerLedgerNotInt",
-            R"({
+                "invalidParams",
+                "Required field 'ledger' missing"},
+            AccountTxParamTestCaseBundle{
+                "MarkerLedgerNotInt",
+                R"({
                 "account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
                 "marker": 
                 {
@@ -150,11 +162,11 @@ generateTestValuesForParametersTest()
                     "ledger": 1
                 }
             })",
-            "invalidParams",
-            "Invalid parameters."},
-        AccountTxParamTestCaseBundle{
-            "MarkerSeqNotInt",
-            R"({
+                "invalidParams",
+                "Invalid parameters."},
+            AccountTxParamTestCaseBundle{
+                "MarkerSeqNotInt",
+                R"({
                 "account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
                 "marker": 
                 {
@@ -162,77 +174,220 @@ generateTestValuesForParametersTest()
                     "seq": 1
                 }
             })",
-            "invalidParams",
-            "Invalid parameters."},
-        AccountTxParamTestCaseBundle{
-            "LedgerIndexMinLessThanMinSeq",
-            R"({
+                "invalidParams",
+                "Invalid parameters."},
+            AccountTxParamTestCaseBundle{
+                "LedgerIndexMinLessThanMinSeq",
+                R"({
                 "account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
                 "ledger_index_min": 9
             })",
-            "lgrIdxMalformed",
-            "ledgerSeqMinOutOfRange"},
-        AccountTxParamTestCaseBundle{
-            "LedgerIndexMaxLargeThanMaxSeq",
-            R"({
+                "lgrIdxMalformed",
+                "ledgerSeqMinOutOfRange"},
+            AccountTxParamTestCaseBundle{
+                "LedgerIndexMaxLargeThanMaxSeq",
+                R"({
                 "account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
                 "ledger_index_max": 31
             })",
-            "lgrIdxMalformed",
-            "ledgerSeqMaxOutOfRange"},
-        AccountTxParamTestCaseBundle{
-            "LedgerIndexMaxLessThanLedgerIndexMin",
-            R"({
+                "lgrIdxMalformed",
+                "ledgerSeqMaxOutOfRange"},
+            AccountTxParamTestCaseBundle{
+                "LedgerIndexMaxLargeThanMaxSeq_API_v1",
+                R"({
+                "account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
+                "ledger_index_max": 31
+            })",
+                std::nullopt,
+                std::nullopt,
+                1u},
+            AccountTxParamTestCaseBundle{
+                "LedgerIndexMaxSmallerThanMinSeq",
+                R"({
+                "account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
+                "ledger_index_max": 9
+            })",
+                "lgrIdxMalformed",
+                "ledgerSeqMaxOutOfRange"},
+            AccountTxParamTestCaseBundle{
+                "LedgerIndexMaxSmallerThanMinSeq_API_v1",
+                R"({
+                "account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
+                "ledger_index_max": 9
+            })",
+                "lgrIdxsInvalid",
+                "Ledger indexes invalid.",
+                1u},
+            AccountTxParamTestCaseBundle{
+                "LedgerIndexMinSmallerThanMinSeq",
+                R"({
+                "account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
+                "ledger_index_min": 9
+            })",
+                "lgrIdxMalformed",
+                "ledgerSeqMinOutOfRange"},
+            AccountTxParamTestCaseBundle{
+                "LedgerIndexMinSmallerThanMinSeq_API_v1",
+                R"({
+                "account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
+                "ledger_index_min": 9
+            })",
+                std::nullopt,
+                std::nullopt,
+                1u},
+            AccountTxParamTestCaseBundle{
+                "LedgerIndexMinLargerThanMaxSeq",
+                R"({
+                "account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
+                "ledger_index_min": 31
+            })",
+                "lgrIdxMalformed",
+                "ledgerSeqMinOutOfRange"},
+            AccountTxParamTestCaseBundle{
+                "LedgerIndexMinLargerThanMaxSeq_API_v1",
+                R"({
+                "account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
+                "ledger_index_min": 31
+            })",
+                "lgrIdxsInvalid",
+                "Ledger indexes invalid.",
+                1u},
+            AccountTxParamTestCaseBundle{
+                "LedgerIndexMaxLessThanLedgerIndexMin",
+                R"({
                 "account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
                 "ledger_index_max": 11,
                 "ledger_index_min": 20
             })",
-            "invalidLgrRange",
-            "Ledger range is invalid."},
-        AccountTxParamTestCaseBundle{
-            "LedgerIndexMaxMinAndLedgerIndex",
-            R"({
+                "invalidLgrRange",
+                "Ledger range is invalid."},
+            AccountTxParamTestCaseBundle{
+                "LedgerIndexMaxLessThanLedgerIndexMin_API_v1",
+                R"({
+                "account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
+                "ledger_index_max": 11,
+                "ledger_index_min": 20
+            })",
+                "lgrIdxsInvalid",
+                "Ledger indexes invalid.",
+                1u},
+            AccountTxParamTestCaseBundle{
+                "LedgerIndexMaxMinAndLedgerIndex",
+                R"({
                 "account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", 
                 "ledger_index_max": 20,
                 "ledger_index_min": 11,
                 "ledger_index": 10
             })",
-            "invalidParams",
-            "containsLedgerSpecifierAndRange"},
-        AccountTxParamTestCaseBundle{
-            "LedgerIndexMaxMinAndLedgerIndexValidated",
-            R"({
-                "account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", 
+                "invalidParams",
+                "containsLedgerSpecifierAndRange"},
+            AccountTxParamTestCaseBundle{
+                "LedgerIndexMaxMinAndLedgerIndexValidated",
+                R"({
+                "account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
                 "ledger_index_max": 20,
                 "ledger_index_min": 11,
                 "ledger_index": "validated"
             })",
-            "invalidParams",
-            "containsLedgerSpecifierAndRange"},
+                "invalidParams",
+                "containsLedgerSpecifierAndRange"},
+            AccountTxParamTestCaseBundle{
+                "LedgerIndexMaxMinAndLedgerIndex_API_v1",
+                R"({
+                "account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
+                "ledger_index_max": 20,
+                "ledger_index_min": 11,
+                "ledger_index": 10
+            })",
+                std::nullopt,
+                std::nullopt,
+                1u},
+            AccountTxParamTestCaseBundle{
+                "LedgerIndexMaxMinAndLedgerHash",
+                fmt::format(
+                    R"({{
+                "account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", 
+                "ledger_index_max": 20,
+                "ledger_index_min": 11,
+                "ledger_hash": "{}"
+            }})",
+                    LEDGERHASH),
+                "invalidParams",
+                "containsLedgerSpecifierAndRange"},
+            AccountTxParamTestCaseBundle{
+                "LedgerIndexMaxMinAndLedgerHash_API_v1",
+                fmt::format(
+                    R"({{
+                "account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", 
+                "ledger_index_max": 20,
+                "ledger_index_min": 11,
+                "ledger_hash": "{}"
+            }})",
+                    LEDGERHASH),
+                std::nullopt,
+                std::nullopt,
+                1u},
+            AccountTxParamTestCaseBundle{
+                "LedgerIndexMaxMinAndLedgerIndexValidated_API_v1",
+                R"({
+                "account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
+                "ledger_index_max": 20,
+                "ledger_index_min": 11,
+                "ledger_index": "validated"
+            })",
+                std::nullopt,
+                std::nullopt,
+                1u},
+        };
     };
-}
+};
 
 INSTANTIATE_TEST_CASE_P(
     RPCAccountTxGroup1,
     AccountTxParameterTest,
-    ValuesIn(generateTestValuesForParametersTest()),
+    ValuesIn(AccountTxParameterTest::generateTestValuesForParametersTest()),
     AccountTxParameterTest::NameGenerator{});
 
-TEST_P(AccountTxParameterTest, InvalidParams)
+TEST_P(AccountTxParameterTest, CheckParams)
 {
     mockBackendPtr->updateRange(MINSEQ);  // min
     mockBackendPtr->updateRange(MAXSEQ);  // max
     auto const testBundle = GetParam();
-    runSpawn([&, this](auto yield) {
-        auto const handler = AnyHandler{AccountTxHandler{mockBackendPtr}};
-        auto const req = json::parse(testBundle.testJson);
-        auto const output = handler.process(req, Context{yield});
-        ASSERT_FALSE(output);
+    auto* rawBackendPtr = static_cast<MockBackend*>(mockBackendPtr.get());
+    std::cout << "Before parse" << std::endl;
+    auto const req = json::parse(testBundle.testJson);
+    std::cout << "After parse" << std::endl;
+    if (testBundle.expectedError.has_value())
+    {
+        ASSERT_TRUE(testBundle.expectedErrorMessage.has_value());
 
-        auto const err = rpc::makeError(output.error());
-        EXPECT_EQ(err.at("error").as_string(), testBundle.expectedError);
-        EXPECT_EQ(err.at("error_message").as_string(), testBundle.expectedErrorMessage);
-    });
+        runSpawn([&, this](auto yield) {
+            auto const handler = AnyHandler{AccountTxHandler{mockBackendPtr}};
+            auto const output = handler.process(req, Context{.yield = yield, .apiVersion = testBundle.apiVersion});
+            ASSERT_FALSE(output);
+            auto const err = rpc::makeError(output.error());
+            EXPECT_EQ(err.at("error").as_string(), *testBundle.expectedError);
+            EXPECT_EQ(err.at("error_message").as_string(), *testBundle.expectedErrorMessage);
+        });
+    }
+    else
+    {
+        if (req.as_object().contains("ledger_hash"))
+        {
+            EXPECT_CALL(*rawBackendPtr, fetchLedgerByHash).WillOnce(testing::Return(ripple::LedgerHeader{}));
+        }
+        else if (req.as_object().contains("ledger_index"))
+        {
+            EXPECT_CALL(*rawBackendPtr, fetchLedgerBySequence).WillOnce(testing::Return(ripple::LedgerHeader{}));
+        }
+        EXPECT_CALL(*rawBackendPtr, fetchAccountTransactions);
+
+        runSpawn([&, this](auto yield) {
+            auto const handler = AnyHandler{AccountTxHandler{mockBackendPtr}};
+            auto const output = handler.process(req, Context{.yield = yield, .apiVersion = testBundle.apiVersion});
+            EXPECT_TRUE(output);
+        });
+    }
 }
 
 namespace {