Proposed 2.2.0-b3

.codecov.yml

@@ -1,6 +1,37 @@
+codecov:
+  require_ci_to_pass: true
+
+comment:
+  behavior: default
+  layout: reach,diff,flags,tree,reach
+  show_carryforward_flags: false
+
 coverage:
+  range: "60..80"
+  precision: 1
+  round: nearest
   status:
     project:
       default:
         target: 60%
         threshold: 2%
+    patch:
+      default:
+        target: auto
+        threshold: 2%
+    changes: false
+
+github_checks:
+  annotations: true
+
+parsers:
+  cobertura:
+    partials_as_hits: true
+    handle_missing_conditions : true
+
+slack_app: false
+
+ignore:
+  - "src/test/"
+  - "src/ripple/beast/test/"
+  - "src/ripple/beast/unit_test/"

.github/workflows/nix.yml

@@ -179,6 +179,8 @@ jobs:
         run: |
           mkdir -p ~/.conan
           tar -xzf conan.tar -C ~/.conan
+      - name: install gcovr
+        run: pip install "gcovr>=7,<8"
       - name: check environment
         run: |
           echo ${PATH} | tr ':' '\n'
@@ -207,7 +209,7 @@ jobs:
             -DCMAKE_CXX_FLAGS="-O0"
             -DCMAKE_C_FLAGS="-O0"
           cmake-target: coverage
-      - name: build
+      - name: move coverage report
         shell: bash
         run: |
           mv "${build_dir}/coverage.xml" ./
@@ -218,13 +220,15 @@ jobs:
           path: coverage.xml
           retention-days: 30
       - name: upload coverage report
-        uses: wandalen/wretry.action@v1.3.0
+        uses: wandalen/wretry.action@v1.4.10
         with:
-          action: codecov/codecov-action@v4
+          action: codecov/codecov-action@v4.3.0
           with: |
             files: coverage.xml
             fail_ci_if_error: true
+            disable_search: true
             verbose: true
+            plugin: noop
             token: ${{ secrets.CODECOV_TOKEN }}
           attempt_limit: 5
-          attempt_delay: 35000 # in milliseconds
+          attempt_delay: 210000 # in milliseconds

Builds/CMake/CodeCoverage.cmake

@@ -95,6 +95,9 @@
 #     - replace both functions setup_target_for_coverage_gcovr_* with a single setup_target_for_coverage_gcovr
 #     - add support for all gcovr output formats
 #
+# 2024-04-03, Bronek Kozicki
+#     - add support for output formats: jacoco, clover, lcov
+#
 # USAGE:
 #
 # 1. Copy this file into your cmake modules path.
@@ -256,10 +259,10 @@ endif()
 #     BASE_DIRECTORY "../"                   # Base directory for report
 #                                            #  (defaults to PROJECT_SOURCE_DIR)
 #     FORMAT "cobertura"                     # Output format, one of:
-#                                            #  xml cobertura sonarqube json-summary
-#                                            #  json-details coveralls csv txt
-#                                            #  html-single html-nested html-details
-#                                            #  (xml is an alias to cobertura;
+#                                            #  xml cobertura sonarqube jacoco clover
+#                                            #  json-summary json-details coveralls csv
+#                                            #  txt html-single html-nested html-details
+#                                            #  lcov (xml is an alias to cobertura;
 #                                            #  if no format is set, defaults to xml)
 #     EXCLUDE "src/dir1/*" "src/dir2/*"      # Patterns to exclude (can be relative
 #                                            #  to BASE_DIRECTORY, with CMake 3.4+)
@@ -308,6 +311,8 @@ function(setup_target_for_coverage_gcovr)
             set(GCOVR_OUTPUT_FILE ${Coverage_NAME}.txt)
         elseif(Coverage_FORMAT STREQUAL "csv")
             set(GCOVR_OUTPUT_FILE ${Coverage_NAME}.csv)
+        elseif(Coverage_FORMAT STREQUAL "lcov")
+            set(GCOVR_OUTPUT_FILE ${Coverage_NAME}.lcov)
         else()
             set(GCOVR_OUTPUT_FILE ${Coverage_NAME}.xml)
         endif()
@@ -320,6 +325,14 @@ function(setup_target_for_coverage_gcovr)
         set(Coverage_FORMAT cobertura) # overwrite xml
     elseif(Coverage_FORMAT STREQUAL "sonarqube")
         list(APPEND GCOVR_ADDITIONAL_ARGS --sonarqube "${GCOVR_OUTPUT_FILE}" )
+    elseif(Coverage_FORMAT STREQUAL "jacoco")
+        list(APPEND GCOVR_ADDITIONAL_ARGS --jacoco "${GCOVR_OUTPUT_FILE}" )
+        list(APPEND GCOVR_ADDITIONAL_ARGS --jacoco-pretty )
+    elseif(Coverage_FORMAT STREQUAL "clover")
+        list(APPEND GCOVR_ADDITIONAL_ARGS --clover "${GCOVR_OUTPUT_FILE}" )
+        list(APPEND GCOVR_ADDITIONAL_ARGS --clover-pretty )
+    elseif(Coverage_FORMAT STREQUAL "lcov")
+        list(APPEND GCOVR_ADDITIONAL_ARGS --lcov "${GCOVR_OUTPUT_FILE}" )
     elseif(Coverage_FORMAT STREQUAL "json-summary")
         list(APPEND GCOVR_ADDITIONAL_ARGS --json-summary "${GCOVR_OUTPUT_FILE}" )
         list(APPEND GCOVR_ADDITIONAL_ARGS --json-summary-pretty)

src/ripple/app/misc/impl/AMMUtils.cpp

@@ -60,9 +60,12 @@ ammHolds(
                     *optIssue2,
                     std::make_optional(std::make_pair(issue1, issue2))))
             {
+                // This error can only be hit if the AMM is corrupted
+                // LCOV_EXCL_START
                 JLOG(j.debug()) << "ammHolds: Invalid optIssue1 or optIssue2 "
                                 << *optIssue1 << " " << *optIssue2;
                 return std::nullopt;
+                // LCOV_EXCL_STOP
             }
             return std::make_optional(std::make_pair(*optIssue1, *optIssue2));
         }
@@ -74,17 +77,21 @@ ammHolds(
                 return std::make_optional(std::make_pair(issue1, issue2));
             else if (checkIssue == issue2)
                 return std::make_optional(std::make_pair(issue2, issue1));
+            // Unreachable unless AMM corrupted.
+            // LCOV_EXCL_START
             JLOG(j.debug())
                 << "ammHolds: Invalid " << label << " " << checkIssue;
             return std::nullopt;
+            // LCOV_EXCL_STOP
         };
         if (optIssue1)
         {
             return singleIssue(*optIssue1, "optIssue1");
         }
         else if (optIssue2)
         {
-            return singleIssue(*optIssue2, "optIssue2");
+            // Cannot have Amount2 without Amount.
+            return singleIssue(*optIssue2, "optIssue2");  // LCOV_EXCL_LINE
         }
         return std::make_optional(std::make_pair(issue1, issue2));
     }();
@@ -210,19 +217,23 @@ deleteAMMTrustLines(
             // Should only have the trustlines
             if (nodeType != LedgerEntryType::ltRIPPLE_STATE)
             {
+                // LCOV_EXCL_START
                 JLOG(j.error())
                     << "deleteAMMTrustLines: deleting non-trustline "
                     << nodeType;
                 return {tecINTERNAL, SkipEntry::No};
+                // LCOV_EXCL_STOP
             }
 
             // Trustlines must have zero balance
             if (sleItem->getFieldAmount(sfBalance) != beast::zero)
             {
+                // LCOV_EXCL_START
                 JLOG(j.error())
                     << "deleteAMMTrustLines: deleting trustline with "
                        "non-zero balance.";
                 return {tecINTERNAL, SkipEntry::No};
+                // LCOV_EXCL_STOP
             }
 
             return {
@@ -243,18 +254,22 @@ deleteAMMAccount(
     auto ammSle = sb.peek(keylet::amm(asset, asset2));
     if (!ammSle)
     {
+        // LCOV_EXCL_START
         JLOG(j.error()) << "deleteAMMAccount: AMM object does not exist "
                         << asset << " " << asset2;
         return tecINTERNAL;
+        // LCOV_EXCL_STOP
     }
 
     auto const ammAccountID = (*ammSle)[sfAccount];
     auto sleAMMRoot = sb.peek(keylet::account(ammAccountID));
     if (!sleAMMRoot)
     {
+        // LCOV_EXCL_START
         JLOG(j.error()) << "deleteAMMAccount: AMM account does not exist "
                         << to_string(ammAccountID);
         return tecINTERNAL;
+        // LCOV_EXCL_STOP
     }
 
     if (auto const ter =
@@ -266,14 +281,18 @@ deleteAMMAccount(
     if (!sb.dirRemove(
             ownerDirKeylet, (*ammSle)[sfOwnerNode], ammSle->key(), false))
     {
+        // LCOV_EXCL_START
         JLOG(j.error()) << "deleteAMMAccount: failed to remove dir link";
         return tecINTERNAL;
+        // LCOV_EXCL_STOP
     }
     if (sb.exists(ownerDirKeylet) && !sb.emptyDirDelete(ownerDirKeylet))
     {
+        // LCOV_EXCL_START
         JLOG(j.error()) << "deleteAMMAccount: cannot delete root dir node of "
                         << toBase58(ammAccountID);
         return tecINTERNAL;
+        // LCOV_EXCL_STOP
     }
 
     sb.erase(ammSle);
@@ -322,11 +341,11 @@ initializeFeeAuctionVote(
     if (tfee != 0)
         ammSle->setFieldU16(sfTradingFee, tfee);
     else if (ammSle->isFieldPresent(sfTradingFee))
-        ammSle->makeFieldAbsent(sfTradingFee);
+        ammSle->makeFieldAbsent(sfTradingFee);  // LCOV_EXCL_LINE
     if (auto const dfee = tfee / AUCTION_SLOT_DISCOUNTED_FEE_FRACTION)
         auctionSlot.setFieldU16(sfDiscountedFee, dfee);
     else if (auctionSlot.isFieldPresent(sfDiscountedFee))
-        auctionSlot.makeFieldAbsent(sfDiscountedFee);
+        auctionSlot.makeFieldAbsent(sfDiscountedFee);  // LCOV_EXCL_LINE
 }
 
 }  // namespace ripple

src/ripple/app/paths/AMMOffer.h

@@ -74,9 +74,6 @@ class AMMOffer
     Issue const&
     issueIn() const;
 
-    Issue const&
-    issueOut() const;
-
     AccountID const&
     owner() const;
 

src/ripple/app/paths/impl/AMMOffer.cpp

@@ -44,13 +44,6 @@ AMMOffer<TIn, TOut>::issueIn() const
     return ammLiquidity_.issueIn();
 }
 
-template <typename TIn, typename TOut>
-Issue const&
-AMMOffer<TIn, TOut>::issueOut() const
-{
-    return ammLiquidity_.issueOut();
-}
-
 template <typename TIn, typename TOut>
 AccountID const&
 AMMOffer<TIn, TOut>::owner() const

src/ripple/app/tx/impl/AMMDeposit.cpp

@@ -186,16 +186,18 @@ AMMDeposit::preclaim(PreclaimContext const& ctx)
         FreezeHandling::fhIGNORE_FREEZE,
         ctx.j);
     if (!expected)
-        return expected.error();
+        return expected.error();  // LCOV_EXCL_LINE
     auto const [amountBalance, amount2Balance, lptAMMBalance] = *expected;
     if (ctx.tx.getFlags() & tfTwoAssetIfEmpty)
     {
         if (lptAMMBalance != beast::zero)
             return tecAMM_NOT_EMPTY;
         if (amountBalance != beast::zero || amount2Balance != beast::zero)
         {
+            // LCOV_EXCL_START
             JLOG(ctx.j.debug()) << "AMM Deposit: tokens balance is not zero.";
             return tecINTERNAL;
+            // LCOV_EXCL_STOP
         }
     }
     else
@@ -205,9 +207,11 @@ AMMDeposit::preclaim(PreclaimContext const& ctx)
         if (amountBalance <= beast::zero || amount2Balance <= beast::zero ||
             lptAMMBalance < beast::zero)
         {
+            // LCOV_EXCL_START
             JLOG(ctx.j.debug())
                 << "AMM Deposit: reserves or tokens balance is zero.";
             return tecINTERNAL;
+            // LCOV_EXCL_STOP
         }
     }
 
@@ -254,10 +258,12 @@ AMMDeposit::preclaim(PreclaimContext const& ctx)
             if (auto const ter =
                     requireAuth(ctx.view, amount->issue(), accountID))
             {
+                // LCOV_EXCL_START
                 JLOG(ctx.j.debug())
                     << "AMM Deposit: account is not authorized, "
                     << amount->issue();
                 return ter;
+                // LCOV_EXCL_STOP
             }
             // AMM account or currency frozen
             if (isFrozen(ctx.view, ammAccountID, amount->issue()))
@@ -339,7 +345,7 @@ AMMDeposit::applyGuts(Sandbox& sb)
     auto const lpTokensDeposit = ctx_.tx[~sfLPTokenOut];
     auto ammSle = sb.peek(keylet::amm(ctx_.tx[sfAsset], ctx_.tx[sfAsset2]));
     if (!ammSle)
-        return {tecINTERNAL, false};
+        return {tecINTERNAL, false};  // LCOV_EXCL_LINE
     auto const ammAccountID = (*ammSle)[sfAccount];
 
     auto const expected = ammHolds(
@@ -350,7 +356,7 @@ AMMDeposit::applyGuts(Sandbox& sb)
         FreezeHandling::fhZERO_IF_FROZEN,
         ctx_.journal);
     if (!expected)
-        return {expected.error(), false};
+        return {expected.error(), false};  // LCOV_EXCL_LINE
     auto const [amountBalance, amount2Balance, lptAMMBalance] = *expected;
     auto const tfee = (lptAMMBalance == beast::zero)
         ? ctx_.tx[~sfTradingFee].value_or(0)
@@ -421,8 +427,10 @@ AMMDeposit::applyGuts(Sandbox& sb)
                 lptAMMBalance.issue(),
                 tfee);
         // should not happen.
+        // LCOV_EXCL_START
         JLOG(j_.error()) << "AMM Deposit: invalid options.";
         return std::make_pair(tecINTERNAL, STAmount{});
+        // LCOV_EXCL_STOP
     }();
 
     if (result == tesSUCCESS)
@@ -621,10 +629,12 @@ AMMDeposit::equalDepositTokens(
     }
     catch (std::exception const& e)
     {
+        // LCOV_EXCL_START
         JLOG(j_.error()) << "AMMDeposit::equalDepositTokens exception "
                          << e.what();
+        return {tecINTERNAL, STAmount{}};
+        // LCOV_EXCL_STOP
     }
-    return {tecINTERNAL, STAmount{}};
 }
 
 /** Proportional deposit of pool assets with the constraints on the maximum