refactor: Retire fix1623 amendment (#5928)

Amendments activated for more than 2 years can be retired. This change retires the fix1623 amendment.

include/xrpl/protocol/detail/features.macro

@@ -103,7 +103,6 @@ XRPL_FIX    (PayChanRecipientOwnerDir,   Supported::yes, VoteBehavior::DefaultYe
 XRPL_FIX    (MasterKeyAsRegularKey,      Supported::yes, VoteBehavior::DefaultYes)
 XRPL_FEATURE(MultiSignReserve,           Supported::yes, VoteBehavior::DefaultYes)
 XRPL_FEATURE(DepositPreauth,             Supported::yes, VoteBehavior::DefaultYes)
-XRPL_FIX    (1623,                       Supported::yes, VoteBehavior::DefaultYes)
 XRPL_FEATURE(Checks,                     Supported::yes, VoteBehavior::DefaultYes)
 XRPL_FEATURE(DepositAuth,                Supported::yes, VoteBehavior::DefaultYes)
 XRPL_FEATURE(Flow,                       Supported::yes, VoteBehavior::DefaultYes)
@@ -139,6 +138,7 @@ XRPL_RETIRE(fix1528)
 XRPL_RETIRE(fix1543)
 XRPL_RETIRE(fix1571)
 XRPL_RETIRE(fix1578)
+XRPL_RETIRE(fix1623)
 XRPL_RETIRE(fix1781)
 XRPL_RETIRE(fixCheckThreading)
 XRPL_RETIRE(fixRmSmallIncreasedQOffers)

src/test/app/Check_test.cpp

@@ -1867,17 +1867,11 @@ class Check_test : public beast::unit_test::suite
     }
 
     void
-    testFix1623Enable(FeatureBitset features)
+    testDeliveredAmountForCheckCashTxn(FeatureBitset features)
     {
-        testcase("Fix1623 enable");
+        testcase("DeliveredAmount For CheckCash Txn");
 
         using namespace test::jtx;
-
-        auto testEnable = [this](
-                              FeatureBitset const& features, bool hasFields) {
-            // Unless fix1623 is enabled a "tx" RPC command should return
-            // neither "DeliveredAmount" nor "delivered_amount" on a CheckCash
-            // transaction.
         Account const alice{"alice"};
         Account const bob{"bob"};
 
@@ -1896,20 +1890,12 @@ class Check_test : public beast::unit_test::suite
         std::string const txHash{
             env.tx()->getJson(JsonOptions::none)[jss::hash].asString()};
 
-            // DeliveredAmount and delivered_amount are either present or
-            // not present in the metadata returned by "tx" based on fix1623.
         env.close();
-            Json::Value const meta =
+        Json::Value const meta = env.rpc("tx", txHash)[jss::result][jss::meta];
-                env.rpc("tx", txHash)[jss::result][jss::meta];
 
-            BEAST_EXPECT(
+        // DeliveredAmount and delivered_amount are present.
-                meta.isMember(sfDeliveredAmount.jsonName) == hasFields);
+        BEAST_EXPECT(meta.isMember(sfDeliveredAmount.jsonName));
-            BEAST_EXPECT(meta.isMember(jss::delivered_amount) == hasFields);
+        BEAST_EXPECT(meta.isMember(jss::delivered_amount));
-        };
-
-        // Run both the disabled and enabled cases.
-        testEnable(features - fix1623, false);
-        testEnable(features, true);
     }
 
     void
@@ -2711,7 +2697,7 @@ class Check_test : public beast::unit_test::suite
         testCashInvalid(features);
         testCancelValid(features);
         testCancelInvalid(features);
-        testFix1623Enable(features);
+        testDeliveredAmountForCheckCashTxn(features);
         testWithTickets(features);
     }
 

src/xrpld/app/tx/detail/CashCheck.cpp

@@ -276,7 +276,6 @@ CashCheck::doApply()
     // work to do...
     auto viewJ = ctx_.app.journal("View");
     auto const optDeliverMin = ctx_.tx[~sfDeliverMin];
-    bool const doFix1623{psb.rules().enabled(fix1623)};
 
     if (srcId != account_)
     {
@@ -311,7 +310,7 @@ CashCheck::doApply()
                 return tecUNFUNDED_PAYMENT;
             }
 
-            if (optDeliverMin && doFix1623)
+            if (optDeliverMin)
                 // Set the DeliveredAmount metadata.
                 ctx_.deliver(xrpDeliver);
 
@@ -461,7 +460,7 @@ CashCheck::doApply()
                         << "flow did not produce DeliverMin.";
                     return tecPATH_PARTIAL;
                 }
-                if (doFix1623 && !checkCashMakesTrustLine)
+                if (!checkCashMakesTrustLine)
                     // Set the delivered_amount metadata.
                     ctx_.deliver(result.actualAmountOut);
             }

src/xrpld/rpc/detail/DeliveredAmount.cpp

@@ -78,51 +78,25 @@ getDeliveredAmount(
 }
 
 // Returns true if transaction meta could contain a delivered amount field,
-// based on transaction type, transaction result and whether fix1623 is enabled
+// based on transaction type and transaction result
-template <class GetFix1623Enabled>
 bool
-canHaveDeliveredAmountHelp(
+canHaveDeliveredAmount(
-    GetFix1623Enabled const& getFix1623Enabled,
     std::shared_ptr<STTx const> const& serializedTx,
     TxMeta const& transactionMeta)
 {
     if (!serializedTx)
         return false;
 
-    {
     TxType const tt{serializedTx->getTxnType()};
-        if (tt != ttPAYMENT && tt != ttCHECK_CASH && tt != ttACCOUNT_DELETE)
+    // Transaction type should be ttPAYMENT, ttACCOUNT_DELETE or ttCHECK_CASH
-            return false;
+    // and if the transaction failed nothing could have been delivered.
-
+    if ((tt == ttPAYMENT || tt == ttCHECK_CASH || tt == ttACCOUNT_DELETE) &&
-        if (tt == ttCHECK_CASH && !getFix1623Enabled())
+        transactionMeta.getResultTER() == tesSUCCESS)
-            return false;
+    {
+        return true;
     }
 
-    // if the transaction failed nothing could have been delivered.
-    if (transactionMeta.getResultTER() != tesSUCCESS)
     return false;
-
-    return true;
-}
-
-// Returns true if transaction meta could contain a delivered amount field,
-// based on transaction type, transaction result and whether fix1623 is enabled
-bool
-canHaveDeliveredAmount(
-    RPC::Context const& context,
-    std::shared_ptr<STTx const> const& serializedTx,
-    TxMeta const& transactionMeta)
-{
-    // These lambdas are used to compute the values lazily
-    auto const getFix1623Enabled = [&context]() -> bool {
-        auto const view = context.app.openLedger().current();
-        if (!view)
-            return false;
-        return view->rules().enabled(fix1623);
-    };
-
-    return canHaveDeliveredAmountHelp(
-        getFix1623Enabled, serializedTx, transactionMeta);
 }
 
 void
@@ -133,12 +107,8 @@ insertDeliveredAmount(
     TxMeta const& transactionMeta)
 {
     auto const info = ledger.info();
-    auto const getFix1623Enabled = [&ledger] {
-        return ledger.rules().enabled(fix1623);
-    };
 
-    if (canHaveDeliveredAmountHelp(
+    if (canHaveDeliveredAmount(serializedTx, transactionMeta))
-            getFix1623Enabled, serializedTx, transactionMeta))
     {
         auto const getLedgerIndex = [&info] { return info.seq; };
         auto const getCloseTime = [&info] { return info.closeTime; };
@@ -167,7 +137,7 @@ getDeliveredAmount(
     TxMeta const& transactionMeta,
     GetLedgerIndex const& getLedgerIndex)
 {
-    if (canHaveDeliveredAmount(context, serializedTx, transactionMeta))
+    if (canHaveDeliveredAmount(serializedTx, transactionMeta))
     {
         auto const getCloseTime =
             [&context,
@@ -212,7 +182,7 @@ insertDeliveredAmount(
     std::shared_ptr<STTx const> const& transaction,
     TxMeta const& transactionMeta)
 {
-    if (canHaveDeliveredAmount(context, transaction, transactionMeta))
+    if (canHaveDeliveredAmount(transaction, transactionMeta))
     {
         auto amt = getDeliveredAmount(
             context, transaction, transactionMeta, [&transactionMeta]() {