Merge branch 'develop' into ximinez/number-fix-comparison

This reverts commit c42a478bb48a578aec670fc1c9ea9dc78208f935.

.github/scripts/strategy-matrix/linux.json

@@ -1,5 +1,5 @@
 {
-  "image_tag": "sha-63ffdc3",
+  "image_tag": "sha-8abe82e",
   "configs": {
     "ubuntu": [
       {
@@ -67,7 +67,7 @@
         "compiler": ["gcc"],
         "build_type": ["Release"],
         "arch": ["amd64"],
-        "image": "ghcr.io/xrplf/xrpld/packaging-debian:sha-63ffdc3"
+        "image": "debian:bookworm"
       }
     ],
 
@@ -76,7 +76,7 @@
         "compiler": ["gcc"],
         "build_type": ["Release"],
         "arch": ["amd64"],
-        "image": "ghcr.io/xrplf/xrpld/packaging-rhel:sha-63ffdc3"
+        "image": "registry.access.redhat.com/ubi9/ubi:latest"
       }
     ]
   }

.github/workflows/build-nix-images.yml

@@ -22,8 +22,7 @@ on:
   workflow_dispatch:
 
 concurrency:
-  # Read `on-trigger.yml` for the rationale behind this concurrency group name.
-  group: ${{ github.workflow }}-${{ github.event_name == 'push' && github.ref == 'refs/heads/develop' && github.sha || github.ref }}
+  group: ${{ github.workflow }}-${{ github.ref }}
   cancel-in-progress: true
 
 defaults:

.github/workflows/build-packaging-images.yml

@@ -20,8 +20,7 @@ on:
   workflow_dispatch:
 
 concurrency:
-  # Read `on-trigger.yml` for the rationale behind this concurrency group name.
-  group: ${{ github.workflow }}-${{ github.event_name == 'push' && github.ref == 'refs/heads/develop' && github.sha || github.ref }}
+  group: ${{ github.workflow }}-${{ github.ref }}
   cancel-in-progress: true
 
 defaults:

.github/workflows/pre-commit.yml

@@ -14,7 +14,7 @@ on:
 jobs:
   # Call the workflow in the XRPLF/actions repo that runs the pre-commit hooks.
   run-hooks:
-    uses: XRPLF/actions/.github/workflows/pre-commit.yml@312aaab296060ff89d7f798dcab59f019bea6e02
+    uses: XRPLF/actions/.github/workflows/pre-commit.yml@cba1f0891650baf1a9c88624dc2d72573be2eb81
     with:
       runs_on: ubuntu-latest
       container: '{ "image": "ghcr.io/xrplf/ci/tools-rippled-pre-commit:sha-41ec7c1" }'

.github/workflows/publish-docs.yml

@@ -41,13 +41,13 @@ env:
 jobs:
   build:
     runs-on: ubuntu-latest
-    container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-63ffdc3
+    container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-8abe82e
     steps:
       - name: Checkout repository
         uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
 
       - name: Prepare runner
-        uses: XRPLF/actions/prepare-runner@c47daebb2f9db64ffbac71b47d68a661498d5ce8
+        uses: XRPLF/actions/prepare-runner@90f11ee655d1687824fb8793db770477d52afbab
         with:
           enable_ccache: false
 

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

@@ -113,7 +113,7 @@ jobs:
         uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
 
       - name: Prepare runner
-        uses: XRPLF/actions/prepare-runner@c47daebb2f9db64ffbac71b47d68a661498d5ce8
+        uses: XRPLF/actions/prepare-runner@90f11ee655d1687824fb8793db770477d52afbab
         with:
           enable_ccache: ${{ inputs.ccache_enabled }}
 
@@ -370,7 +370,7 @@ jobs:
 
       - name: Upload coverage report
         if: ${{ github.repository == 'XRPLF/rippled' && !inputs.build_only && env.COVERAGE_ENABLED == 'true' }}
-        uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f # v7.0.0
+        uses: codecov/codecov-action@e79a6962e0d4c0c17b229090214935d2e33f8354 # v6.0.1
         with:
           disable_search: true
           disable_telem: true

.github/workflows/reusable-clang-tidy.yml

@@ -29,14 +29,14 @@ jobs:
     if: ${{ inputs.check_only_changed }}
     permissions:
       contents: read
-    uses: XRPLF/actions/.github/workflows/determine-tidy-files.yml@312aaab296060ff89d7f798dcab59f019bea6e02
+    uses: XRPLF/actions/.github/workflows/determine-tidy-files.yml@224f3c48d3014d082a1129237b8291ff0b0a331f
 
   run-clang-tidy:
     name: Run clang tidy
     needs: [determine-files]
     if: ${{ always() && !cancelled() && (!inputs.check_only_changed || needs.determine-files.outputs.cpp_changed_files != '' || needs.determine-files.outputs.clang_tidy_config_changed == 'true') }}
     runs-on: ["self-hosted", "Linux", "X64", "heavy"]
-    container: "ghcr.io/xrplf/xrpld/nix-debian:sha-63ffdc3"
+    container: "ghcr.io/xrplf/xrpld/nix-debian:sha-8abe82e"
     permissions:
       contents: read
       issues: write
@@ -45,7 +45,7 @@ jobs:
         uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
 
       - name: Prepare runner
-        uses: XRPLF/actions/prepare-runner@c47daebb2f9db64ffbac71b47d68a661498d5ce8
+        uses: XRPLF/actions/prepare-runner@90f11ee655d1687824fb8793db770477d52afbab
         with:
           enable_ccache: false
 

.github/workflows/reusable-package.yml

@@ -68,6 +68,31 @@ jobs:
     timeout-minutes: 30
 
     steps:
+      # Packaging runs in a vanilla distro image, so the tooling has to come
+      # from the distro's archive: debhelper for deb, rpm-build (and the
+      # systemd / find-debuginfo macros it depends on) for rpm. Run this
+      # before actions/checkout so the latter can use git (real history) for
+      # build_pkg.sh's SOURCE_DATE_EPOCH; otherwise it falls back to a tarball
+      # download and the timestamp comes from wall-clock time.
+      - name: Install packaging tooling (deb)
+        if: ${{ matrix.distro == 'debian' }}
+        run: |
+          export DEBIAN_FRONTEND=noninteractive
+          apt-get update
+          apt-get install -y --no-install-recommends \
+              ca-certificates \
+              debhelper \
+              git
+
+      - name: Install packaging tooling (rpm)
+        if: ${{ matrix.distro == 'rhel' }}
+        run: |
+          dnf install -y --setopt=install_weak_deps=False \
+              git \
+              rpm-build \
+              redhat-rpm-config \
+              systemd-rpm-macros
+
       - name: Checkout repository
         uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
 

.github/workflows/reusable-upload-recipe.yml

@@ -40,7 +40,7 @@ defaults:
 jobs:
   upload:
     runs-on: ubuntu-latest
-    container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-63ffdc3
+    container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-8abe82e
     steps:
       - name: Checkout repository
         uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3

.github/workflows/upload-conan-deps.yml

@@ -67,7 +67,7 @@ jobs:
         uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
 
       - name: Prepare runner
-        uses: XRPLF/actions/prepare-runner@c47daebb2f9db64ffbac71b47d68a661498d5ce8
+        uses: XRPLF/actions/prepare-runner@90f11ee655d1687824fb8793db770477d52afbab
         with:
           enable_ccache: false
 

include/xrpl/basics/Number.h

@@ -408,33 +408,40 @@ public:
     }
 
     friend constexpr bool
-    operator<(Number const& x, Number const& y) noexcept
+    operator<(Number const& l, Number const& r) noexcept
     {
+        bool const lneg = l.negative_;
+        bool const rneg = r.negative_;
+
         // If the two amounts have different signs (zero is treated as positive)
         // then the comparison is true iff the left is negative.
-        bool const lneg = x.negative_;
-        bool const rneg = y.negative_;
-
         if (lneg != rneg)
             return lneg;
 
-        // Both have same sign and the left is zero: the right must be
-        // greater than 0.
-        if (x.mantissa_ == 0)
-            return y.mantissa_ > 0;
+        // Both have same sign and the left is zero: both must be non-negative.
+        // If the right is greater than 0, then it is larger, so the comparison is true.
+        if (l.mantissa_ == 0)
+            return r.mantissa_ > 0;
 
-        // Both have same sign, the right is zero and the left is non-zero.
-        if (y.mantissa_ == 0)
+        // Both have same sign, the right is zero and the left is non-zero, so the left must be
+        // positive, and thus is larger, so the comparison is false.
+        if (r.mantissa_ == 0)
             return false;
 
         // Both have the same sign, compare by exponents:
-        if (x.exponent_ > y.exponent_)
+        if (l.exponent_ > r.exponent_)
             return lneg;
-        if (x.exponent_ < y.exponent_)
+        if (l.exponent_ < r.exponent_)
             return !lneg;
 
-        // If equal exponents, compare mantissas
-        return x.mantissa_ < y.mantissa_;
+        // If equal signs and exponents, compare mantissas.
+        if (lneg)
+        {
+            // If negative, the operator is reversed.
+            return l.mantissa_ > r.mantissa_;
+        }
+
+        return l.mantissa_ < r.mantissa_;
     }
 
     /** Return the sign of the amount */

src/libxrpl/protocol/STIssue.cpp

@@ -11,8 +11,6 @@
 #include <xrpl/protocol/Serializer.h>
 #include <xrpl/protocol/UintTypes.h>
 
-#include <boost/endian/conversion.hpp>
-
 #include <cstddef>
 #include <cstdint>
 #include <cstring>
@@ -47,9 +45,6 @@ STIssue::STIssue(SerialIter& sit, SField const& name) : STBase{name}
         {
             MPTID mptID;
             std::uint32_t sequence = sit.get32();
-            // Preserve the existing LE memcpy result on every host endian.
-            // Wire 04 03 02 01 becomes MPTID bytes 01 02 03 04 on both.
-            sequence = boost::endian::native_to_little(sequence);
             static_assert(MPTID::size() == sizeof(sequence) + sizeof(currencyOrAccount));
             memcpy(mptID.data(), &sequence, sizeof(sequence));
             memcpy(
@@ -105,9 +100,6 @@ STIssue::add(Serializer& s) const
             s.addBitString(noAccount());
             std::uint32_t sequence = 0;
             memcpy(&sequence, issue.getMptID().data(), sizeof(sequence));
-            // Preserve the existing LE ledger bytes on every host endian.
-            // MPTID bytes 01 02 03 04 become wire bytes 04 03 02 01 on both.
-            sequence = boost::endian::native_to_little(sequence);
             s.add32(sequence);
         });
 }

src/test/basics/Number_test.cpp

@@ -10,6 +10,7 @@
 #include <boost/multiprecision/cpp_dec_float.hpp>
 #include <boost/multiprecision/number.hpp>
 
+#include <algorithm>
 #include <array>
 #include <cctype>
 #include <cstdint>
@@ -20,6 +21,8 @@
 #include <stdexcept>
 #include <string>
 #include <tuple>
+#include <utility>
+#include <vector>
 
 namespace xrpl {
 
@@ -1386,10 +1389,103 @@ public:
     testRelationals()
     {
         testcase << "test_relationals " << to_string(Number::getMantissaScale());
-        BEAST_EXPECT(!(Number{100} < Number{10}));
-        BEAST_EXPECT(Number{100} > Number{10});
-        BEAST_EXPECT(Number{100} >= Number{10});
-        BEAST_EXPECT(!(Number{100} <= Number{10}));
+
+        {
+            auto test = [this](auto const& nums) {
+                BEAST_EXPECT(std::ranges::is_sorted(nums));
+
+                for (auto iter1 = nums.begin(); iter1 != nums.end(); ++iter1)
+                {
+                    auto iter2 = iter1;
+                    for (++iter2; iter2 != nums.end(); ++iter2)
+                    {
+                        Number const& smaller = *iter1;
+                        Number const& larger = *iter2;
+                        std::stringstream ss;
+                        ss << smaller << " < " << larger;
+                        auto const str = ss.str();
+
+                        // The ==/!= operators use a completely different code path than <, etc.
+                        // This helps detect a breakage in one but not the other. It also helps
+                        // verify that the values are being ordered correctly.
+                        BEAST_EXPECTS(smaller != larger, str + " (!=)");
+                        BEAST_EXPECTS(!(smaller == larger), str + " (==)");
+
+                        // true results using operator< and derived operators
+                        BEAST_EXPECTS(smaller < larger, str + " (<)");
+                        BEAST_EXPECTS(larger > smaller, str + " (>)");
+                        BEAST_EXPECTS(larger >= smaller, str + " (>=)");
+                        BEAST_EXPECTS(smaller <= larger, str + " (<=)");
+
+                        // false results using operator< and derived operators
+                        BEAST_EXPECTS(!(larger < smaller), str + " (! <)");
+                        BEAST_EXPECTS(!(smaller > larger), str + " (! >)");
+                        BEAST_EXPECTS(!(smaller >= larger), str + " (! >=)");
+                        BEAST_EXPECTS(!(larger <= smaller), str + " (! <=)");
+                    }
+                }
+            };
+
+            auto const intNums = [this]() {
+                // Inequality test cases are built from a list of sorted integers
+                auto const values =
+                    std::to_array<int>({-100, -50, -20, -10, -1, 0, 1, 10, 20, 50, 100});
+                // Check this list is sorted before converting it to Numbers.
+                // That way if any of the other tests fail, we know it's because of code and not the
+                // source data.
+                BEAST_EXPECT(std::ranges::is_sorted(values));
+
+                std::vector<Number> result;
+                result.reserve(values.size());
+                for (auto const v : values)
+                    result.emplace_back(v);
+                return result;
+            }();
+
+            auto const otherNums = std::to_array<Number>({
+                Number{-5, 100},
+                Number{-1, 100},
+                Number{-7, -10},
+                Number{-2, -10},
+                Number{0},
+                Number{2, -10},
+                Number{7, -10},
+                Number{1, 100},
+                Number{5, 100},
+            });
+
+            test(intNums);
+            test(otherNums);
+        }
+
+        {
+            // Equality test cases are <Number, __LINE__>. Number will be compared against itself
+            using Case = std::pair<Number, int>;
+            auto const c = std::to_array<Case>({
+                {700, __LINE__},
+                {50, __LINE__},
+                {1, __LINE__},
+                {0, __LINE__},
+                {-1, __LINE__},
+                {-30, __LINE__},
+                {-600, __LINE__},
+            });
+            for (auto const& [n, line] : c)
+            {
+                auto const str = to_string(n);
+
+                // NOLINTBEGIN(misc-redundant-expression) Explicitly testing operators with
+                // equivalent values
+                expect(n == n, str + " ==", __FILE__, line);
+                expect(!(n != n), str + " !=", __FILE__, line);
+
+                expect(!(n < n), str + " < ", __FILE__, line);
+                expect(!(n > n), str + " >", __FILE__, line);
+                expect(n >= n, str + " >=", __FILE__, line);
+                expect(n <= n, str + " <=", __FILE__, line);
+                // NOLINTEND(misc-redundant-expression)
+            }
+        }
     }
 
     void

src/test/protocol/STIssue_test.cpp

@@ -3,19 +3,14 @@
 #include <test/jtx/amount.h>  // IWYU pragma: keep
 
 #include <xrpl/basics/base_uint.h>
-#include <xrpl/basics/strHex.h>
 #include <xrpl/beast/unit_test/suite.h>
 #include <xrpl/protocol/AccountID.h>
-#include <xrpl/protocol/Indexes.h>
 #include <xrpl/protocol/Issue.h>
-#include <xrpl/protocol/MPTIssue.h>
 #include <xrpl/protocol/SField.h>
 #include <xrpl/protocol/STIssue.h>
 #include <xrpl/protocol/Serializer.h>
 #include <xrpl/protocol/UintTypes.h>
 
-#include <cstdint>
-
 namespace xrpl::test {
 
 class STIssue_test : public beast::unit_test::Suite
@@ -142,69 +137,12 @@ public:
             "000000000000000000000000000000000000000000000002");
     }
 
-    void
-    testMPTSerialization()
-    {
-        testcase("MPT serialization");
-        using namespace jtx;
-        Account const alice{"alice"};
-
-        struct Vector
-        {
-            std::uint32_t sequence;
-            std::uint32_t legacySequence;
-        };
-
-        // 0x01020304 pins canonical MPTID bytes 01 02 03 04 and
-        // preserved STIssue wire bytes 04 03 02 01 on BE and LE.
-        Vector const vectors[] = {
-            {.sequence = 0x00000001, .legacySequence = 0x01000000},
-            {.sequence = 0x01020304, .legacySequence = 0x04030201},
-            {.sequence = 0xa1b2c3d4, .legacySequence = 0xd4c3b2a1},
-        };
-
-        for (auto const& vector : vectors)
-        {
-            MPTID const mptID = makeMptID(vector.sequence, alice);
-            MPTIssue const issue{mptID};
-            STIssue const stIssue(sfAsset, Asset{issue});
-
-            Serializer actual;
-            stIssue.add(actual);
-
-            // STIssue preserves the existing little-endian validator ledger bytes.
-            Serializer expected;
-            expected.addBitString(alice.id());
-            expected.addBitString(noAccount());
-            {
-                std::array<unsigned char, 4> bytes;
-                auto const seq = boost::endian::native_to_little(vector.sequence);
-                memcpy(bytes.data(), &seq, 4);
-                expected.addRaw(bytes.data(), bytes.size());
-            }
-
-            BEAST_EXPECTS(strHex(actual) == strHex(expected), strHex(actual));
-
-            // Decoding the preserved wire format must recover the canonical MPTID.
-            SerialIter iter(expected.slice());
-            STIssue const decoded(iter, sfAsset);
-            BEAST_EXPECT(decoded.holds<MPTIssue>());
-            BEAST_EXPECT(decoded.value().get<MPTIssue>().getMptID() == mptID);
-
-            // A decoded ledger value must serialize back to the same bytes.
-            Serializer roundTrip;
-            decoded.add(roundTrip);
-            BEAST_EXPECTS(strHex(roundTrip) == strHex(expected), strHex(roundTrip));
-        }
-    }
-
     void
     run() override
     {
         // compliments other unit tests to ensure complete coverage
         testConstructor();
         testCompare();
-        testMPTSerialization();
     }
 };
 

src/tests/libxrpl/CMakeLists.txt

@@ -53,4 +53,4 @@ foreach(module IN LISTS test_modules)
     )
 endforeach()
 
-gtest_discover_tests(xrpl_tests DISCOVERY_TIMEOUT 60)
+gtest_discover_tests(xrpl_tests)