activate tsan and both on gcc

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

.github/scripts/strategy-matrix/generate.py

@@ -20,8 +20,8 @@ class Config:
 Generate a strategy matrix for GitHub Actions CI.
 
 On each PR commit we will build a selection of Debian, RHEL, Ubuntu, MacOS, and
-Windows configurations, while upon merge into the develop or release branches,
-we will build all configurations, and test most of them.
+Windows configurations, while upon merge into the develop, release, or master
+branches, we will build all configurations, and test most of them.
 
 We will further set additional CMake arguments as follows:
 - All builds will have the `tests`, `werr`, and `xrpld` options.
@@ -242,10 +242,12 @@ def generate_strategy_matrix(all: bool, config: Config) -> list:
         # names get truncated.
         # Add Address and Thread (both coupled with UB) sanitizers for specific bookworm distros.
         # GCC-Asan rippled-embedded tests are failing because of https://github.com/google/sanitizers/issues/856
-        if (
-            os["distro_version"] == "bookworm"
-            and f"{os['compiler_name']}-{os['compiler_version']}" == "clang-20"
-        ):
+        if os[
+            "distro_version"
+        ] == "bookworm" and f"{os['compiler_name']}-{os['compiler_version']}" in [
+            "clang-20",
+            "gcc-15",
+        ]:
             # Add ASAN + UBSAN configuration.
             configurations.append(
                 {
@@ -260,7 +262,7 @@ def generate_strategy_matrix(all: bool, config: Config) -> list:
                 }
             )
             # TSAN is deactivated due to seg faults with latest compilers.
-            activate_tsan = False
+            activate_tsan = True
             if activate_tsan:
                 configurations.append(
                     {

.github/workflows/on-pr.yml

@@ -125,7 +125,7 @@ jobs:
     needs:
       - should-run
       - build-test
-    if: ${{ needs.should-run.outputs.go == 'true' && startsWith(github.ref, 'refs/heads/release') }}
+    if: ${{ needs.should-run.outputs.go == 'true' && (startsWith(github.base_ref, 'release') || github.base_ref == 'master') }}
     uses: ./.github/workflows/reusable-notify-clio.yml
     secrets:
       clio_notify_token: ${{ secrets.CLIO_NOTIFY_TOKEN }}

.github/workflows/on-trigger.yml

@@ -1,8 +1,9 @@
-# This workflow runs all workflows to build and test the code on various Linux
-# flavors, as well as on MacOS and Windows, on a scheduled basis, on merge into
-# the 'develop' or 'release*' branches, or when requested manually. Upon
-# successful completion, it also uploads the built libxrpl package to the Conan
-# remote.
+# This workflow runs all workflows to build the dependencies required for the
+# project on various Linux flavors, as well as on MacOS and Windows, on a
+# scheduled basis, on merge into the 'develop', 'release', or 'master' branches,
+# or manually. The missing commits check is only run when the code is merged
+# into the 'develop' or 'release' branches, and the documentation is built when
+# the code is merged into the 'develop' branch.
 name: Trigger
 
 on:
@@ -10,6 +11,7 @@ on:
     branches:
       - "develop"
       - "release*"
+      - "master"
     paths:
       # These paths are unique to `on-trigger.yml`.
       - ".github/workflows/on-trigger.yml"
@@ -68,10 +70,10 @@ jobs:
     with:
       # Enable ccache only for events targeting the XRPLF repository, since
       # other accounts will not have access to our remote cache storage.
-      # However, we do not enable ccache for events targeting a release branch,
-      # to protect against the rare case that the output produced by ccache is
-      # not identical to a regular compilation.
-      ccache_enabled: ${{ github.repository_owner == 'XRPLF' && !startsWith(github.ref, 'refs/heads/release') }}
+      # However, we do not enable ccache for events targeting the master or a
+      # release branch, to protect against the rare case that the output
+      # produced by ccache is not identical to a regular compilation.
+      ccache_enabled: ${{ github.repository_owner == 'XRPLF' && !(github.base_ref == 'master' || startsWith(github.base_ref, 'release')) }}
       os: ${{ matrix.os }}
       strategy_matrix: ${{ github.event_name == 'schedule' && 'all' || 'minimal' }}
     secrets:

.github/workflows/pre-commit.yml

@@ -3,9 +3,7 @@ name: Run pre-commit hooks
 on:
   pull_request:
   push:
-    branches:
-      - "develop"
-      - "release*"
+    branches: [develop, release, master]
   workflow_dispatch:
 
 jobs:

.github/workflows/reusable-build-test-config.yml

@@ -203,10 +203,10 @@ jobs:
       - name: Set sanitizer options
         if: ${{ !inputs.build_only && env.SANITIZERS_ENABLED == 'true' }}
         run: |
-          echo "ASAN_OPTIONS=print_stacktrace=1:detect_container_overflow=0:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/asan.supp" >> ${GITHUB_ENV}
-          echo "TSAN_OPTIONS=second_deadlock_stack=1:halt_on_error=0:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/tsan.supp" >> ${GITHUB_ENV}
-          echo "UBSAN_OPTIONS=suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/ubsan.supp" >> ${GITHUB_ENV}
-          echo "LSAN_OPTIONS=suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/lsan.supp" >> ${GITHUB_ENV}
+          echo "ASAN_OPTIONS=print_stacktrace=1:detect_container_overflow=0" >> ${GITHUB_ENV}
+          echo "TSAN_OPTIONS=second_deadlock_stack=1:halt_on_error=0" >> ${GITHUB_ENV}
+        # echo "UBSAN_OPTIONS=suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/ubsan.supp" >> ${GITHUB_ENV}
+        # echo "LSAN_OPTIONS=suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/lsan.supp" >> ${GITHUB_ENV}
 
       - name: Run the separate tests
         if: ${{ !inputs.build_only }}

cmake/XrplSanitizers.cmake

@@ -151,13 +151,13 @@ if(is_gcc)
 elseif(is_clang)
     # Add ignorelist for Clang (GCC doesn't support this)
     # Use CMAKE_SOURCE_DIR to get the path to the ignorelist
-    set(IGNORELIST_PATH "${CMAKE_SOURCE_DIR}/sanitizers/suppressions/sanitizer-ignorelist.txt")
-    if(NOT EXISTS "${IGNORELIST_PATH}")
-        message(FATAL_ERROR "Sanitizer ignorelist not found: ${IGNORELIST_PATH}")
-    endif()
+    # set(IGNORELIST_PATH "${CMAKE_SOURCE_DIR}/sanitizers/suppressions/sanitizer-ignorelist.txt")
+    # if(NOT EXISTS "${IGNORELIST_PATH}")
+    #     message(FATAL_ERROR "Sanitizer ignorelist not found: ${IGNORELIST_PATH}")
+    # endif()
 
-    list(APPEND SANITIZERS_COMPILE_FLAGS "-fsanitize-ignorelist=${IGNORELIST_PATH}")
-    message(STATUS "  Using sanitizer ignorelist: ${IGNORELIST_PATH}")
+    # list(APPEND SANITIZERS_COMPILE_FLAGS "-fsanitize-ignorelist=${IGNORELIST_PATH}")
+    # message(STATUS "  Using sanitizer ignorelist: ${IGNORELIST_PATH}")
 
     # Join sanitizer flags with commas for -fsanitize option
     list(JOIN SANITIZER_TYPES "," SANITIZER_TYPES_STR)

src/libxrpl/beast/core/CurrentThreadName.cpp

@@ -89,14 +89,14 @@ setCurrentThreadNameImpl(std::string_view name)
 
     pthread_setname_np(pthread_self(), boundedName);
 
-#ifdef TRUNCATED_THREAD_NAME_LOGS
-    if (name.size() > maxThreadNameLength)
-    {
-        std::cerr << "WARNING: Thread name \"" << name << "\" (length "
-                  << name.size() << ") exceeds maximum of "
-                  << maxThreadNameLength << " characters on Linux.\n";
-    }
-#endif
+    // #ifdef TRUNCATED_THREAD_NAME_LOGS
+    //     if (name.size() > maxThreadNameLength)
+    //     {
+    //         std::cerr << "WARNING: Thread name \"" << name << "\" (length "
+    //                   << name.size() << ") exceeds maximum of "
+    //                   << maxThreadNameLength << " characters on Linux.\n";
+    //     }
+    // #endif
 }
 
 }  // namespace beast::detail

src/test/rpc/LedgerEntry_test.cpp

@@ -5,8 +5,6 @@
 #include <test/jtx/multisign.h>
 #include <test/jtx/xchain_bridge.h>
 
-#include <xrpld/app/tx/apply.h>
-
 #include <xrpl/beast/unit_test.h>
 #include <xrpl/json/json_value.h>
 #include <xrpl/protocol/AccountID.h>
@@ -32,7 +30,6 @@ enum class FieldType {
     CurrencyField,
     HashField,
     HashOrObjectField,
-    FixedHashField,
     IssueField,
     ObjectField,
     StringField,
@@ -89,7 +86,6 @@ getTypeName(FieldType typeID)
         case FieldType::CurrencyField:
             return "Currency";
         case FieldType::HashField:
-        case FieldType::FixedHashField:
             return "hex string";
         case FieldType::HashOrObjectField:
             return "hex string or object";
@@ -206,7 +202,6 @@ class LedgerEntry_test : public beast::unit_test::suite
         static auto const& badBlobValues = remove({3, 7, 8, 16});
         static auto const& badCurrencyValues = remove({14});
         static auto const& badHashValues = remove({2, 3, 7, 8, 16});
-        static auto const& badFixedHashValues = remove({1, 2, 3, 4, 7, 8, 16});
         static auto const& badIndexValues = remove({12, 16, 18, 19});
         static auto const& badUInt32Values = remove({2, 3});
         static auto const& badUInt64Values = remove({2, 3});
@@ -227,8 +222,6 @@ class LedgerEntry_test : public beast::unit_test::suite
                 return badHashValues;
             case FieldType::HashOrObjectField:
                 return badIndexValues;
-            case FieldType::FixedHashField:
-                return badFixedHashValues;
             case FieldType::IssueField:
                 return badIssueValues;
             case FieldType::UInt32Field:
@@ -724,12 +717,7 @@ class LedgerEntry_test : public beast::unit_test::suite
         }
 
         // negative tests
-        testMalformedField(
-            env,
-            Json::Value{},
-            jss::amendments,
-            FieldType::FixedHashField,
-            "malformedRequest");
+        runLedgerEntryTest(env, jss::amendments);
     }
 
     void
@@ -1550,12 +1538,7 @@ class LedgerEntry_test : public beast::unit_test::suite
         }
 
         // negative tests
-        testMalformedField(
-            env,
-            Json::Value{},
-            jss::fee,
-            FieldType::FixedHashField,
-            "malformedRequest");
+        runLedgerEntryTest(env, jss::fee);
     }
 
     void
@@ -1578,12 +1561,7 @@ class LedgerEntry_test : public beast::unit_test::suite
         }
 
         // negative tests
-        testMalformedField(
-            env,
-            Json::Value{},
-            jss::hashes,
-            FieldType::FixedHashField,
-            "malformedRequest");
+        runLedgerEntryTest(env, jss::hashes);
     }
 
     void
@@ -1708,12 +1686,7 @@ class LedgerEntry_test : public beast::unit_test::suite
         }
 
         // negative tests
-        testMalformedField(
-            env,
-            Json::Value{},
-            jss::nunl,
-            FieldType::FixedHashField,
-            "malformedRequest");
+        runLedgerEntryTest(env, jss::nunl);
     }
 
     void
@@ -2370,438 +2343,6 @@ class LedgerEntry_test : public beast::unit_test::suite
         }
     }
 
-    /// Test the ledger entry types that don't take parameters
-    void
-    testFixed()
-    {
-        using namespace test::jtx;
-
-        Account const alice{"alice"};
-        Account const bob{"bob"};
-
-        Env env{*this, envconfig([](auto cfg) {
-                    cfg->START_UP = Config::FRESH;
-                    return cfg;
-                })};
-
-        env.close();
-
-        /** Verifies that the RPC result has the expected data
-         *
-         * @param good: Indicates that the request should have succeeded
-         *   and returned a ledger object of `expectedType` type.
-         * @param jv: The RPC result Json value
-         * @param expectedType: The type that the ledger object should
-         *   have if "good".
-         * @param expectedError: Optional. The expected error if not
-         *   good. Defaults to "entryNotFound".
-         */
-        auto checkResult =
-            [&](bool good,
-                Json::Value const& jv,
-                Json::StaticString const& expectedType,
-                std::optional<std::string> const& expectedError = {}) {
-                if (good)
-                {
-                    BEAST_EXPECTS(
-                        jv.isObject() && jv.isMember(jss::result) &&
-                            !jv[jss::result].isMember(jss::error) &&
-                            jv[jss::result].isMember(jss::node) &&
-                            jv[jss::result][jss::node].isMember(
-                                sfLedgerEntryType.jsonName) &&
-                            jv[jss::result][jss::node]
-                              [sfLedgerEntryType.jsonName] == expectedType,
-                        to_string(jv));
-                }
-                else
-                {
-                    BEAST_EXPECTS(
-                        jv.isObject() && jv.isMember(jss::result) &&
-                            jv[jss::result].isMember(jss::error) &&
-                            !jv[jss::result].isMember(jss::node) &&
-                            jv[jss::result][jss::error] ==
-                                expectedError.value_or("entryNotFound"),
-                        to_string(jv));
-                }
-            };
-
-        /** Runs a series of tests for a given fixed-position ledger
-         * entry.
-         *
-         * @param field: The Json request field to use.
-         * @param expectedType: The type that the ledger object should
-         *   have if "good".
-         * @param expectedKey: The keylet of the fixed object.
-         * @param good: Indicates whether the object is expected to
-         *   exist.
-         */
-        auto test = [&](Json::StaticString const& field,
-                        Json::StaticString const& expectedType,
-                        Keylet const& expectedKey,
-                        bool good) {
-            testcase << expectedType.c_str() << (good ? "" : " not")
-                     << " found";
-
-            auto const hexKey = strHex(expectedKey.key);
-
-            {
-                // Test bad values
-                // "field":null
-                Json::Value params;
-                params[jss::ledger_index] = jss::validated;
-                params[field] = Json::nullValue;
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                checkResult(false, jv, expectedType, "malformedRequest");
-                BEAST_EXPECT(!jv[jss::result].isMember(jss::index));
-            }
-
-            {
-                Json::Value params;
-                // "field":"string"
-                params[jss::ledger_index] = jss::validated;
-                params[field] = "arbitrary string";
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                checkResult(false, jv, expectedType, "malformedRequest");
-                BEAST_EXPECT(!jv[jss::result].isMember(jss::index));
-            }
-
-            {
-                Json::Value params;
-                // "field":false
-                params[jss::ledger_index] = jss::validated;
-                params[field] = false;
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                checkResult(false, jv, expectedType, "invalidParams");
-                BEAST_EXPECT(!jv[jss::result].isMember(jss::index));
-            }
-
-            {
-                Json::Value params;
-
-                // "field":[incorrect index hash]
-                auto const badKey = strHex(expectedKey.key + uint256{1});
-                params[jss::ledger_index] = jss::validated;
-                params[field] = badKey;
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                checkResult(false, jv, expectedType, "entryNotFound");
-                BEAST_EXPECTS(
-                    jv[jss::result][jss::index] == badKey, to_string(jv));
-            }
-
-            {
-                Json::Value params;
-                // "index":"field" using API 2
-                params[jss::ledger_index] = jss::validated;
-                params[jss::index] = field;
-                params[jss::api_version] = 2;
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                checkResult(false, jv, expectedType, "malformedRequest");
-                BEAST_EXPECT(!jv[jss::result].isMember(jss::index));
-            }
-
-            std::string const pdIdx = [&]() {
-                {
-                    Json::Value params;
-                    // Test good values
-                    // Use the "field":true notation
-                    params[jss::ledger_index] = jss::validated;
-                    params[field] = true;
-                    auto const jv =
-                        env.rpc("json", "ledger_entry", to_string(params));
-                    // Index will always be returned for valid parameters.
-                    std::string const pdIdx =
-                        jv[jss::result][jss::index].asString();
-                    BEAST_EXPECTS(hexKey == pdIdx, to_string(jv));
-                    checkResult(good, jv, expectedType);
-
-                    return pdIdx;
-                }
-            }();
-
-            {
-                Json::Value params;
-                // "field":"[index hash]"
-                params[jss::ledger_index] = jss::validated;
-                params[field] = hexKey;
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                checkResult(good, jv, expectedType);
-                BEAST_EXPECT(jv[jss::result][jss::index].asString() == hexKey);
-            }
-
-            {
-                // Bad value
-                // Use the "index":"field" notation with API 2
-                Json::Value params;
-                params[jss::ledger_index] = jss::validated;
-                params[jss::index] = field;
-                params[jss::api_version] = 2;
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                checkResult(false, jv, expectedType, "malformedRequest");
-                BEAST_EXPECT(!jv[jss::result].isMember(jss::index));
-            }
-
-            {
-                Json::Value params;
-                // Use the "index":"field" notation with API 3
-                params[jss::ledger_index] = jss::validated;
-                params[jss::index] = field;
-                params[jss::api_version] = 3;
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                // Index is correct either way
-                BEAST_EXPECT(jv[jss::result][jss::index].asString() == hexKey);
-                checkResult(good, jv, expectedType);
-            }
-
-            {
-                Json::Value params;
-                // Use the "index":"[index hash]" notation
-                params[jss::ledger_index] = jss::validated;
-                params[jss::index] = pdIdx;
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                // Index is correct either way
-                BEAST_EXPECT(jv[jss::result][jss::index].asString() == hexKey);
-                checkResult(good, jv, expectedType);
-            }
-        };
-
-        test(jss::amendments, jss::Amendments, keylet::amendments(), true);
-        test(jss::fee, jss::FeeSettings, keylet::fees(), true);
-        // There won't be an nunl
-        test(jss::nunl, jss::NegativeUNL, keylet::negativeUNL(), false);
-        // Can only get the short skip list this way
-        test(jss::hashes, jss::LedgerHashes, keylet::skip(), true);
-    }
-
-    void
-    testHashes()
-    {
-        using namespace test::jtx;
-
-        Account const alice{"alice"};
-        Account const bob{"bob"};
-
-        Env env{*this, envconfig([](auto cfg) {
-                    cfg->START_UP = Config::FRESH;
-                    return cfg;
-                })};
-
-        env.close();
-
-        /** Verifies that the RPC result has the expected data
-         *
-         * @param good: Indicates that the request should have succeeded
-         *   and returned a ledger object of `expectedType` type.
-         * @param jv: The RPC result Json value
-         * @param expectedCount: The number of Hashes expected in the
-         *   object if "good".
-         * @param expectedError: Optional. The expected error if not
-         *   good. Defaults to "entryNotFound".
-         */
-        auto checkResult =
-            [&](bool good,
-                Json::Value const& jv,
-                int expectedCount,
-                std::optional<std::string> const& expectedError = {}) {
-                if (good)
-                {
-                    BEAST_EXPECTS(
-                        jv.isObject() && jv.isMember(jss::result) &&
-                            !jv[jss::result].isMember(jss::error) &&
-                            jv[jss::result].isMember(jss::node) &&
-                            jv[jss::result][jss::node].isMember(
-                                sfLedgerEntryType.jsonName) &&
-                            jv[jss::result][jss::node]
-                              [sfLedgerEntryType.jsonName] == jss::LedgerHashes,
-                        to_string(jv));
-                    BEAST_EXPECTS(
-                        jv[jss::result].isMember(jss::node) &&
-                            jv[jss::result][jss::node].isMember("Hashes") &&
-                            jv[jss::result][jss::node]["Hashes"].size() ==
-                                expectedCount,
-                        to_string(jv[jss::result][jss::node]["Hashes"].size()));
-                }
-                else
-                {
-                    BEAST_EXPECTS(
-                        jv.isObject() && jv.isMember(jss::result) &&
-                            jv[jss::result].isMember(jss::error) &&
-                            !jv[jss::result].isMember(jss::node) &&
-                            jv[jss::result][jss::error] ==
-                                expectedError.value_or("entryNotFound"),
-                        to_string(jv));
-                }
-            };
-
-        /** Runs a series of tests for a given ledger index.
-         *
-         * @param ledger: The ledger index value of the "hashes" request
-         *   parameter. May not necessarily be a number.
-         * @param expectedKey: The expected keylet of the object.
-         * @param good: Indicates whether the object is expected to
-         *   exist.
-         * @param expectedCount: The number of Hashes expected in the
-         *   object if "good".
-         */
-        auto test = [&](Json::Value ledger,
-                        Keylet const& expectedKey,
-                        bool good,
-                        int expectedCount = 0) {
-            testcase << "LedgerHashes: seq: " << env.current()->header().seq
-                     << " \"hashes\":" << to_string(ledger)
-                     << (good ? "" : " not") << " found";
-
-            auto const hexKey = strHex(expectedKey.key);
-
-            {
-                // Test bad values
-                // "hashes":null
-                Json::Value params;
-                params[jss::ledger_index] = jss::validated;
-                params[jss::hashes] = Json::nullValue;
-                auto jv = env.rpc("json", "ledger_entry", to_string(params));
-                checkResult(false, jv, 0, "malformedRequest");
-                BEAST_EXPECT(!jv[jss::result].isMember(jss::index));
-            }
-
-            {
-                Json::Value params;
-                // "hashes":"non-uint string"
-                params[jss::ledger_index] = jss::validated;
-                params[jss::hashes] = "arbitrary string";
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                checkResult(false, jv, 0, "malformedRequest");
-                BEAST_EXPECT(!jv[jss::result].isMember(jss::index));
-            }
-
-            {
-                Json::Value params;
-                // "hashes":"uint string" is invalid, too
-                params[jss::ledger_index] = jss::validated;
-                params[jss::hashes] = "10";
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                checkResult(false, jv, 0, "malformedRequest");
-                BEAST_EXPECT(!jv[jss::result].isMember(jss::index));
-            }
-
-            {
-                Json::Value params;
-                // "hashes":false
-                params[jss::ledger_index] = jss::validated;
-                params[jss::hashes] = false;
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                checkResult(false, jv, 0, "invalidParams");
-                BEAST_EXPECT(!jv[jss::result].isMember(jss::index));
-            }
-
-            {
-                Json::Value params;
-                // "hashes":-1
-                params[jss::ledger_index] = jss::validated;
-                params[jss::hashes] = -1;
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                checkResult(false, jv, 0, "internal");
-                BEAST_EXPECT(!jv[jss::result].isMember(jss::index));
-            }
-
-            // "hashes":[incorrect index hash]
-            {
-                Json::Value params;
-                auto const badKey = strHex(expectedKey.key + uint256{1});
-                params[jss::ledger_index] = jss::validated;
-                params[jss::hashes] = badKey;
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                checkResult(false, jv, 0, "entryNotFound");
-                BEAST_EXPECT(jv[jss::result][jss::index] == badKey);
-            }
-
-            {
-                Json::Value params;
-                // Test good values
-                // Use the "hashes":ledger notation
-                params[jss::ledger_index] = jss::validated;
-                params[jss::hashes] = ledger;
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                checkResult(good, jv, expectedCount);
-                // Index will always be returned for valid parameters.
-                std::string const pdIdx =
-                    jv[jss::result][jss::index].asString();
-                BEAST_EXPECTS(hexKey == pdIdx, strHex(pdIdx));
-            }
-
-            {
-                Json::Value params;
-                // "hashes":"[index hash]"
-                params[jss::ledger_index] = jss::validated;
-                params[jss::hashes] = hexKey;
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                checkResult(good, jv, expectedCount);
-                // Index is correct either way
-                BEAST_EXPECTS(
-                    hexKey == jv[jss::result][jss::index].asString(),
-                    strHex(jv[jss::result][jss::index].asString()));
-            }
-
-            {
-                Json::Value params;
-                // Use the "index":"[index hash]" notation
-                params[jss::ledger_index] = jss::validated;
-                params[jss::index] = hexKey;
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                checkResult(good, jv, expectedCount);
-                // Index is correct either way
-                BEAST_EXPECTS(
-                    hexKey == jv[jss::result][jss::index].asString(),
-                    strHex(jv[jss::result][jss::index].asString()));
-            }
-        };
-
-        // short skip list
-        test(true, keylet::skip(), true, 2);
-        // long skip list at index 0
-        test(1, keylet::skip(1), false);
-        // long skip list at index 1
-        test(1 << 17, keylet::skip(1 << 17), false);
-
-        // Close more ledgers, but stop short of the flag ledger
-        for (auto i = env.current()->seq(); i <= 250; ++i)
-            env.close();
-
-        // short skip list
-        test(true, keylet::skip(), true, 249);
-        // long skip list at index 0
-        test(1, keylet::skip(1), false);
-        // long skip list at index 1
-        test(1 << 17, keylet::skip(1 << 17), false);
-
-        // Close a flag ledger so the first "long" skip list is created
-        for (auto i = env.current()->seq(); i <= 260; ++i)
-            env.close();
-
-        // short skip list
-        test(true, keylet::skip(), true, 256);
-        // long skip list at index 0
-        test(1, keylet::skip(1), true, 1);
-        // long skip list at index 1
-        test(1 << 17, keylet::skip(1 << 17), false);
-    }
-
     void
     testCLI()
     {
@@ -2859,8 +2400,6 @@ public:
         testOracleLedgerEntry();
         testMPT();
         testPermissionedDomain();
-        testFixed();
-        testHashes();
         testCLI();
     }
 };

src/xrpld/rpc/handlers/LedgerEntry.cpp

@@ -18,32 +18,6 @@
 
 namespace xrpl {
 
-using FunctionType = std::function<Expected<uint256, Json::Value>(
-    Json::Value const&,
-    Json::StaticString const,
-    unsigned const apiVersion)>;
-
-static Expected<uint256, Json::Value>
-parseFixed(
-    Keylet const& keylet,
-    Json::Value const& params,
-    Json::StaticString const& fieldName,
-    unsigned const apiVersion);
-
-// Helper function to return FunctionType for objects that have a fixed
-// location. That is, they don't take parameters to compute the index.
-// e.g. amendments, fees, negative UNL, etc.
-static FunctionType
-fixed(Keylet const& keylet)
-{
-    return [keylet](
-               Json::Value const& params,
-               Json::StaticString const fieldName,
-               unsigned const apiVersion) -> Expected<uint256, Json::Value> {
-        return parseFixed(keylet, params, fieldName, apiVersion);
-    };
-}
-
 static Expected<uint256, Json::Value>
 parseObjectID(
     Json::Value const& params,
@@ -59,33 +33,13 @@ parseObjectID(
 }
 
 static Expected<uint256, Json::Value>
-parseIndex(
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    unsigned const apiVersion)
+parseIndex(Json::Value const& params, Json::StaticString const fieldName)
 {
-    if (apiVersion > 2u && params.isString())
-    {
-        std::string const index = params.asString();
-        if (index == jss::amendments.c_str())
-            return keylet::amendments().key;
-        if (index == jss::fee.c_str())
-            return keylet::fees().key;
-        if (index == jss::nunl)
-            return keylet::negativeUNL().key;
-        if (index == jss::hashes)
-            // Note this only finds the "short" skip list. Use "hashes":index to
-            // get the long list.
-            return keylet::skip().key;
-    }
     return parseObjectID(params, fieldName, "hex string");
 }
 
 static Expected<uint256, Json::Value>
-parseAccountRoot(
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
+parseAccountRoot(Json::Value const& params, Json::StaticString const fieldName)
 {
     if (auto const account = LedgerEntryHelpers::parse<AccountID>(params))
     {
@@ -96,13 +50,14 @@ parseAccountRoot(
         "malformedAddress", fieldName, "AccountID");
 }
 
-auto const parseAmendments = fixed(keylet::amendments());
+static Expected<uint256, Json::Value>
+parseAmendments(Json::Value const& params, Json::StaticString const fieldName)
+{
+    return parseObjectID(params, fieldName, "hex string");
+}
 
 static Expected<uint256, Json::Value>
-parseAMM(
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
+parseAMM(Json::Value const& params, Json::StaticString const fieldName)
 {
     if (!params.isObject())
     {
@@ -130,10 +85,7 @@ parseAMM(
 }
 
 static Expected<uint256, Json::Value>
-parseBridge(
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
+parseBridge(Json::Value const& params, Json::StaticString const fieldName)
 {
     if (!params.isMember(jss::bridge))
     {
@@ -164,19 +116,13 @@ parseBridge(
 }
 
 static Expected<uint256, Json::Value>
-parseCheck(
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
+parseCheck(Json::Value const& params, Json::StaticString const fieldName)
 {
     return parseObjectID(params, fieldName, "hex string");
 }
 
 static Expected<uint256, Json::Value>
-parseCredential(
-    Json::Value const& cred,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
+parseCredential(Json::Value const& cred, Json::StaticString const fieldName)
 {
     if (!cred.isObject())
     {
@@ -207,10 +153,7 @@ parseCredential(
 }
 
 static Expected<uint256, Json::Value>
-parseDelegate(
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
+parseDelegate(Json::Value const& params, Json::StaticString const fieldName)
 {
     if (!params.isObject())
     {
@@ -301,10 +244,7 @@ parseAuthorizeCredentials(Json::Value const& jv)
 }
 
 static Expected<uint256, Json::Value>
-parseDepositPreauth(
-    Json::Value const& dp,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
+parseDepositPreauth(Json::Value const& dp, Json::StaticString const fieldName)
 {
     if (!dp.isObject())
     {
@@ -357,10 +297,7 @@ parseDepositPreauth(
 }
 
 static Expected<uint256, Json::Value>
-parseDID(
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
+parseDID(Json::Value const& params, Json::StaticString const fieldName)
 {
     auto const account = LedgerEntryHelpers::parse<AccountID>(params);
     if (!account)
@@ -375,8 +312,7 @@ parseDID(
 static Expected<uint256, Json::Value>
 parseDirectoryNode(
     Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
+    Json::StaticString const fieldName)
 {
     if (!params.isObject())
     {
@@ -429,10 +365,7 @@ parseDirectoryNode(
 }
 
 static Expected<uint256, Json::Value>
-parseEscrow(
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
+parseEscrow(Json::Value const& params, Json::StaticString const fieldName)
 {
     if (!params.isObject())
     {
@@ -451,53 +384,20 @@ parseEscrow(
     return keylet::escrow(*id, *seq).key;
 }
 
-auto const parseFeeSettings = fixed(keylet::fees());
-
 static Expected<uint256, Json::Value>
-parseFixed(
-    Keylet const& keylet,
-    Json::Value const& params,
-    Json::StaticString const& fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
+parseFeeSettings(Json::Value const& params, Json::StaticString const fieldName)
 {
-    if (!params.isBool())
-    {
-        return parseObjectID(params, fieldName, "hex string");
-    }
-    if (!params.asBool())
-    {
-        return LedgerEntryHelpers::invalidFieldError(
-            "invalidParams", fieldName, "true");
-    }
-
-    return keylet.key;
+    return parseObjectID(params, fieldName, "hex string");
 }
 
 static Expected<uint256, Json::Value>
-parseLedgerHashes(
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    unsigned const apiVersion)
+parseLedgerHashes(Json::Value const& params, Json::StaticString const fieldName)
 {
-    if (params.isUInt() || params.isInt())
-    {
-        // If the index doesn't parse as a UInt, throw
-        auto const index = params.asUInt();
-
-        // Return the "long" skip list for the given ledger index.
-        auto const keylet = keylet::skip(index);
-        return keylet.key;
-    }
-    // Return the key in `params` or the "short" skip list, which contains
-    // hashes since the last flag ledger.
-    return parseFixed(keylet::skip(), params, fieldName, apiVersion);
+    return parseObjectID(params, fieldName, "hex string");
 }
 
 static Expected<uint256, Json::Value>
-parseLoanBroker(
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
+parseLoanBroker(Json::Value const& params, Json::StaticString const fieldName)
 {
     if (!params.isObject())
     {
@@ -517,10 +417,7 @@ parseLoanBroker(
 }
 
 static Expected<uint256, Json::Value>
-parseLoan(
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
+parseLoan(Json::Value const& params, Json::StaticString const fieldName)
 {
     if (!params.isObject())
     {
@@ -540,10 +437,7 @@ parseLoan(
 }
 
 static Expected<uint256, Json::Value>
-parseMPToken(
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
+parseMPToken(Json::Value const& params, Json::StaticString const fieldName)
 {
     if (!params.isObject())
     {
@@ -566,8 +460,7 @@ parseMPToken(
 static Expected<uint256, Json::Value>
 parseMPTokenIssuance(
     Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
+    Json::StaticString const fieldName)
 {
     auto const mptIssuanceID = LedgerEntryHelpers::parse<uint192>(params);
     if (!mptIssuanceID)
@@ -578,30 +471,25 @@ parseMPTokenIssuance(
 }
 
 static Expected<uint256, Json::Value>
-parseNFTokenOffer(
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
+parseNFTokenOffer(Json::Value const& params, Json::StaticString const fieldName)
 {
     return parseObjectID(params, fieldName, "hex string");
 }
 
 static Expected<uint256, Json::Value>
-parseNFTokenPage(
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
+parseNFTokenPage(Json::Value const& params, Json::StaticString const fieldName)
 {
     return parseObjectID(params, fieldName, "hex string");
 }
 
-auto const parseNegativeUNL = fixed(keylet::negativeUNL());
+static Expected<uint256, Json::Value>
+parseNegativeUNL(Json::Value const& params, Json::StaticString const fieldName)
+{
+    return parseObjectID(params, fieldName, "hex string");
+}
 
 static Expected<uint256, Json::Value>
-parseOffer(
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
+parseOffer(Json::Value const& params, Json::StaticString const fieldName)
 {
     if (!params.isObject())
     {
@@ -622,10 +510,7 @@ parseOffer(
 }
 
 static Expected<uint256, Json::Value>
-parseOracle(
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
+parseOracle(Json::Value const& params, Json::StaticString const fieldName)
 {
     if (!params.isObject())
     {
@@ -646,10 +531,7 @@ parseOracle(
 }
 
 static Expected<uint256, Json::Value>
-parsePayChannel(
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
+parsePayChannel(Json::Value const& params, Json::StaticString const fieldName)
 {
     return parseObjectID(params, fieldName, "hex string");
 }
@@ -657,8 +539,7 @@ parsePayChannel(
 static Expected<uint256, Json::Value>
 parsePermissionedDomain(
     Json::Value const& pd,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
+    Json::StaticString const fieldName)
 {
     if (pd.isString())
     {
@@ -687,8 +568,7 @@ parsePermissionedDomain(
 static Expected<uint256, Json::Value>
 parseRippleState(
     Json::Value const& jvRippleState,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
+    Json::StaticString const fieldName)
 {
     Currency uCurrency;
 
@@ -738,19 +618,13 @@ parseRippleState(
 }
 
 static Expected<uint256, Json::Value>
-parseSignerList(
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
+parseSignerList(Json::Value const& params, Json::StaticString const fieldName)
 {
     return parseObjectID(params, fieldName, "hex string");
 }
 
 static Expected<uint256, Json::Value>
-parseTicket(
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
+parseTicket(Json::Value const& params, Json::StaticString const fieldName)
 {
     if (!params.isObject())
     {
@@ -771,10 +645,7 @@ parseTicket(
 }
 
 static Expected<uint256, Json::Value>
-parseVault(
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
+parseVault(Json::Value const& params, Json::StaticString const fieldName)
 {
     if (!params.isObject())
     {
@@ -797,8 +668,7 @@ parseVault(
 static Expected<uint256, Json::Value>
 parseXChainOwnedClaimID(
     Json::Value const& claim_id,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
+    Json::StaticString const fieldName)
 {
     if (!claim_id.isObject())
     {
@@ -823,8 +693,7 @@ parseXChainOwnedClaimID(
 static Expected<uint256, Json::Value>
 parseXChainOwnedCreateAccountClaimID(
     Json::Value const& claim_id,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
+    Json::StaticString const fieldName)
 {
     if (!claim_id.isObject())
     {
@@ -848,6 +717,10 @@ parseXChainOwnedCreateAccountClaimID(
     return keylet.key;
 }
 
+using FunctionType = Expected<uint256, Json::Value> (*)(
+    Json::Value const&,
+    Json::StaticString const);
+
 struct LedgerEntry
 {
     Json::StaticString fieldName;
@@ -880,7 +753,7 @@ doLedgerEntry(RPC::JsonContext& context)
         {jss::ripple_state, parseRippleState, ltRIPPLE_STATE},
     });
 
-    auto const hasMoreThanOneMember = [&]() {
+    auto hasMoreThanOneMember = [&]() {
         int count = 0;
 
         for (auto const& ledgerEntry : ledgerEntryParsers)
@@ -924,8 +797,8 @@ doLedgerEntry(RPC::JsonContext& context)
                 Json::Value const& params = ledgerEntry.fieldName == jss::bridge
                     ? context.params
                     : context.params[ledgerEntry.fieldName];
-                auto const result = ledgerEntry.parseFunction(
-                    params, ledgerEntry.fieldName, context.apiVersion);
+                auto const result =
+                    ledgerEntry.parseFunction(params, ledgerEntry.fieldName);
                 if (!result)
                     return result.error();
 
@@ -956,13 +829,9 @@ doLedgerEntry(RPC::JsonContext& context)
             throw;
     }
 
-    // Return the computed index regardless of whether the node exists.
-    jvResult[jss::index] = to_string(uNodeIndex);
-
     if (uNodeIndex.isZero())
     {
-        RPC::inject_error(rpcENTRY_NOT_FOUND, jvResult);
-        return jvResult;
+        return RPC::make_error(rpcENTRY_NOT_FOUND);
     }
 
     auto const sleNode = lpLedger->read(keylet::unchecked(uNodeIndex));
@@ -974,14 +843,12 @@ doLedgerEntry(RPC::JsonContext& context)
     if (!sleNode)
     {
         // Not found.
-        RPC::inject_error(rpcENTRY_NOT_FOUND, jvResult);
-        return jvResult;
+        return RPC::make_error(rpcENTRY_NOT_FOUND);
     }
 
     if ((expectedType != ltANY) && (expectedType != sleNode->getType()))
     {
-        RPC::inject_error(rpcUNEXPECTED_LEDGER_TYPE, jvResult);
-        return jvResult;
+        return RPC::make_error(rpcUNEXPECTED_LEDGER_TYPE);
     }
 
     if (bNodeBinary)
@@ -991,10 +858,12 @@ doLedgerEntry(RPC::JsonContext& context)
         sleNode->add(s);
 
         jvResult[jss::node_binary] = strHex(s.peekData());
+        jvResult[jss::index] = to_string(uNodeIndex);
     }
     else
     {
         jvResult[jss::node] = sleNode->getJson(JsonOptions::none);
+        jvResult[jss::index] = to_string(uNodeIndex);
     }
 
     return jvResult;