Address my review feedback on (#6017)

- Shortcut on issuer match in canTransfer.
- An asset can transfer if either trustline enabled rippling, so both
  must have it disabled for the transfer to fail with terNO_RIPPLE.
- Remove unnecessary asfDefaultRipple sets in unit tests.
- Add a new unit test helper class: testline, and a macro: THISLINE.
  When included as a parameter to Env::operator(), will include the line
  number of the transaction that didn't get the expected result. Works
  similarly to BEAST_EXPECT. I didn't do the same for the file name,
  because that can be deduced from the testcase name.

src/libxrpl/ledger/View.cpp

@@ -3288,11 +3288,11 @@ canTransfer(
         return tesSUCCESS;
 
     auto const issuerId = issue.getIssuer();
+    if (issuerId == from || issuerId == to)
+        return tesSUCCESS;
     auto const sleIssuer = view.read(keylet::account(issuerId));
     if (sleIssuer == nullptr)
         return tefINTERNAL;  // LCOV_EXCL_LINE
-    if (issuerId == from || issuerId == to)
-        return tesSUCCESS;
 
     auto const isRippleDisabled = [&](AccountID account) -> bool {
         // Line might not exist, but some transfers can create it. If this
@@ -3306,8 +3306,8 @@ canTransfer(
         return sleIssuer->isFlag(lsfDefaultRipple) == false;
     };
 
-    // Fail if rippling disabled on either trust line
-    if (isRippleDisabled(from) || isRippleDisabled(to))
+    // Fail if rippling disabled on both trust lines
+    if (isRippleDisabled(from) && isRippleDisabled(to))
         return terNO_RIPPLE;
 
     return tesSUCCESS;

src/test/app/Loan_test.cpp

@@ -4516,8 +4516,8 @@ protected:
         // preclaim
         Env env(*this);
         env.fund(XRP(1'000), lender, issuer, borrower);
-        env(trust(lender, IOU(10'000'000)));
-        env(pay(issuer, lender, IOU(5'000'000)));
+        env(trust(lender, IOU(10'000'000)), THISLINE);
+        env(pay(issuer, lender, IOU(5'000'000)), THISLINE);
         BrokerInfo brokerInfo{createVaultAndBroker(env, issuer["IOU"], lender)};
 
         auto const loanSetFee = fee(env.current()->fees().base * 2);
@@ -4525,21 +4525,24 @@ protected:
 
         env(set(borrower, brokerInfo.brokerID, debtMaximumRequest),
             sig(sfCounterpartySignature, lender),
-            loanSetFee);
+            loanSetFee,
+            THISLINE);
 
         env.close();
 
         std::uint32_t const loanSequence = 1;
         auto const loanKeylet = keylet::loan(brokerInfo.brokerID, loanSequence);
 
-        env(fset(issuer, asfGlobalFreeze));
+        env(fset(issuer, asfGlobalFreeze), THISLINE);
         env.close();
 
         // preclaim: tecFROZEN
-        env(pay(borrower, loanKeylet.key, debtMaximumRequest), ter(tecFROZEN));
+        env(pay(borrower, loanKeylet.key, debtMaximumRequest),
+            ter(tecFROZEN),
+            THISLINE);
         env.close();
 
-        env(fclear(issuer, asfGlobalFreeze));
+        env(fclear(issuer, asfGlobalFreeze), THISLINE);
         env.close();
 
         auto const pseudoBroker = [&]() -> std::optional<Account> {
@@ -4559,37 +4562,51 @@ protected:
 
         // Lender and pseudoaccount must both be frozen
         env(trust(
-            issuer,
-            lender["IOU"](1'000),
-            lender,
-            tfSetFreeze | tfSetDeepFreeze));
+                issuer,
+                lender["IOU"](1'000),
+                lender,
+                tfSetFreeze | tfSetDeepFreeze),
+            THISLINE);
         env(trust(
-            issuer,
-            (*pseudoBroker)["IOU"](1'000),
-            *pseudoBroker,
-            tfSetFreeze | tfSetDeepFreeze));
+                issuer,
+                (*pseudoBroker)["IOU"](1'000),
+                *pseudoBroker,
+                tfSetFreeze | tfSetDeepFreeze),
+            THISLINE);
         env.close();
 
         // preclaim: tecFROZEN due to deep frozen
-        env(pay(borrower, loanKeylet.key, debtMaximumRequest), ter(tecFROZEN));
+        env(pay(borrower, loanKeylet.key, debtMaximumRequest),
+            ter(tecFROZEN),
+            THISLINE);
         env.close();
 
         // Only one needs to be unfrozen
         env(trust(
-            issuer, lender["IOU"](1'000), tfClearFreeze | tfClearDeepFreeze));
+                issuer,
+                lender["IOU"](1'000),
+                tfClearFreeze | tfClearDeepFreeze),
+            THISLINE);
         env.close();
 
         // The payment is late by this point
-        env(pay(borrower, loanKeylet.key, debtMaximumRequest), ter(tecEXPIRED));
+        env(pay(borrower, loanKeylet.key, debtMaximumRequest),
+            ter(tecEXPIRED),
+            THISLINE);
         env.close();
-        env(pay(
-            borrower, loanKeylet.key, debtMaximumRequest, tfLoanLatePayment));
+        env(pay(borrower,
+                loanKeylet.key,
+                debtMaximumRequest,
+                tfLoanLatePayment),
+            THISLINE);
         env.close();
 
         // preclaim: tecKILLED
         // note that tecKILLED in loanMakePayment()
         // doesn't happen because of the preclaim check.
-        env(pay(borrower, loanKeylet.key, debtMaximumRequest), ter(tecKILLED));
+        env(pay(borrower, loanKeylet.key, debtMaximumRequest),
+            ter(tecKILLED),
+            THISLINE);
     }
 
     void

src/test/app/Vault_test.cpp

@@ -3,6 +3,7 @@
 #include <test/jtx/Env.h>
 #include <test/jtx/amount.h>
 #include <test/jtx/mpt.h>
+#include <test/jtx/testline.h>
 
 #include <xrpl/basics/base_uint.h>
 #include <xrpl/beast/unit_test/suite.h>
@@ -588,7 +589,6 @@ class Vault_test : public beast::unit_test::suite
             Vault vault{env};
             env.fund(XRP(1000), issuer, owner, depositor, charlie, dave);
             env.close();
-            env(fset(issuer, asfDefaultRipple));
             env(fset(issuer, asfAllowTrustLineClawback));
             env(fset(issuer, asfRequireAuth));
             env(fset(dave, asfRequireDest));
@@ -659,7 +659,6 @@ class Vault_test : public beast::unit_test::suite
             env.fund(XRP(1000), issuer, owner);
             env.close();
 
-            env(fset(issuer, asfDefaultRipple));
             env(fset(issuer, asfAllowTrustLineClawback));
             env(fset(issuer, asfRequireAuth));
             env.close();
@@ -2567,7 +2566,6 @@ class Vault_test : public beast::unit_test::suite
                 Account const charlie{"charlie"};
                 Vault vault{env};
                 env.fund(XRP(args.initialXRP), issuer, owner, charlie);
-                env(fset(issuer, asfDefaultRipple));
                 env(fset(issuer, asfAllowTrustLineClawback));
                 env.close();
 
@@ -2971,7 +2969,7 @@ class Vault_test : public beast::unit_test::suite
                 Account const& owner,
                 Account const& issuer,
                 Account const& charlie,
-                auto,
+                auto vaultAccount,
                 Vault& vault,
                 PrettyAsset const& asset,
                 std::function<MPTID(ripple::Keylet)> issuanceId) {
@@ -2983,13 +2981,18 @@ class Vault_test : public beast::unit_test::suite
                 env(tx);
                 env.close();
 
+                // Turn on noripple on the pseudo account's trust line.
+                // Charlie's is already set.
+                env(trust(issuer, vaultAccount(keylet)["IOU"], tfSetNoRipple),
+                    THISLINE);
+
                 {
                     // Charlie cannot deposit
                     auto tx = vault.deposit(
                         {.depositor = charlie,
                          .id = keylet.key,
                          .amount = asset(100)});
-                    env(tx, ter{terNO_RIPPLE});
+                    env(tx, ter{terNO_RIPPLE}, THISLINE);
                     env.close();
                 }
 
@@ -2999,7 +3002,7 @@ class Vault_test : public beast::unit_test::suite
                         {.depositor = owner,
                          .id = keylet.key,
                          .amount = asset(100)});
-                    env(tx1);
+                    env(tx1, THISLINE);
                     env.close();
 
                     // Charlie cannot receive funds
@@ -3008,7 +3011,7 @@ class Vault_test : public beast::unit_test::suite
                          .id = keylet.key,
                          .amount = shares(100)});
                     tx2[sfDestination] = charlie.human();
-                    env(tx2, ter{terNO_RIPPLE});
+                    env(tx2, ter{terNO_RIPPLE}, THISLINE);
                     env.close();
 
                     {
@@ -3021,7 +3024,7 @@ class Vault_test : public beast::unit_test::suite
                         env(tx);
                         env.close();
                     }
-                    env(pay(owner, charlie, shares(100)));
+                    env(pay(owner, charlie, shares(100)), THISLINE);
                     env.close();
 
                     // Charlie cannot withdraw
@@ -3032,7 +3035,7 @@ class Vault_test : public beast::unit_test::suite
                     env(tx3, ter{terNO_RIPPLE});
                     env.close();
 
-                    env(pay(charlie, owner, shares(100)));
+                    env(pay(charlie, owner, shares(100)), THISLINE);
                     env.close();
                 }
 
@@ -3040,11 +3043,11 @@ class Vault_test : public beast::unit_test::suite
                     {.depositor = owner,
                      .id = keylet.key,
                      .amount = asset(100)});
-                env(tx);
+                env(tx, THISLINE);
                 env.close();
 
                 // Delete vault with zero balance
-                env(vault.del({.owner = owner, .id = keylet.key}));
+                env(vault.del({.owner = owner, .id = keylet.key}), THISLINE);
             },
             {.charlieRipple = false});
 
@@ -3366,7 +3369,6 @@ class Vault_test : public beast::unit_test::suite
             credIssuer1,
             credIssuer2);
         env.close();
-        env(fset(issuer, asfDefaultRipple));
         env(fset(issuer, asfAllowTrustLineClawback));
         env.close();
         env.require(flags(issuer, asfAllowTrustLineClawback));
@@ -3796,7 +3798,6 @@ class Vault_test : public beast::unit_test::suite
             Account const depositor{"depositor"};
             Vault vault{env};
             env.fund(XRP(1000), issuer, owner, depositor);
-            env(fset(issuer, asfDefaultRipple));
             env(fset(issuer, asfAllowTrustLineClawback));
             env.close();
 

src/test/jtx.h

@@ -52,6 +52,7 @@
 #include <test/jtx/tag.h>
 #include <test/jtx/tags.h>
 #include <test/jtx/ter.h>
+#include <test/jtx/testline.h>
 #include <test/jtx/ticket.h>
 #include <test/jtx/token.h>
 #include <test/jtx/trust.h>

src/test/jtx/JTx.h

@@ -40,6 +40,9 @@ struct JTx
     // Functions that sign something else after the mainSigners, such as
     // sfCounterpartySignature
     std::vector<std::function<void(Env&, JTx&)>> postSigners;
+    // Metadata about the unit test itself
+    // The line where the JTx was constructed
+    std::optional<int> testLine = std::nullopt;
 
     JTx() = default;
     JTx(JTx const&) = default;

src/test/jtx/impl/Env.cpp

@@ -427,14 +427,16 @@ Env::postconditions(
     ParsedResult const& parsed,
     Json::Value const& jr)
 {
-    bool bad = !test.expect(parsed.ter, "apply: No ter result!");
+    auto const line = jt.testLine ? " (" + to_string(*jt.testLine) + ")" : "";
+    bool bad = !test.expect(parsed.ter, "apply: No ter result!" + line);
     bad =
         (jt.ter && parsed.ter &&
          !test.expect(
              *parsed.ter == *jt.ter,
              "apply: Got " + transToken(*parsed.ter) + " (" +
                  transHuman(*parsed.ter) + "); Expected " +
-                 transToken(*jt.ter) + " (" + transHuman(*jt.ter) + ")"));
+                 transToken(*jt.ter) + " (" + transHuman(*jt.ter) + ")" +
+                 line));
     using namespace std::string_literals;
     bad = (jt.rpcCode &&
            !test.expect(
@@ -446,21 +448,21 @@ Env::postconditions(
                         : "NO RESULT") +
                    " (" + parsed.rpcMessage + "); Expected " +
                    RPC::get_error_info(jt.rpcCode->first).token.c_str() + " (" +
-                   jt.rpcCode->second + ")")) ||
+                   jt.rpcCode->second + ")" + line)) ||
         bad;
     // If we have an rpcCode (just checked), then the rpcException check is
     // optional - the 'error' field may not be defined, but if it is, it must
     // match rpcError.
-    bad =
-        (jt.rpcException &&
-         !test.expect(
-             (jt.rpcCode && parsed.rpcError.empty()) ||
-                 (parsed.rpcError == jt.rpcException->first &&
-                  (!jt.rpcException->second ||
-                   parsed.rpcException == *jt.rpcException->second)),
-             "apply: Got RPC result "s + parsed.rpcError + " (" +
-                 parsed.rpcException + "); Expected " + jt.rpcException->first +
-                 " (" + jt.rpcException->second.value_or("n/a") + ")")) ||
+    bad = (jt.rpcException &&
+           !test.expect(
+               (jt.rpcCode && parsed.rpcError.empty()) ||
+                   (parsed.rpcError == jt.rpcException->first &&
+                    (!jt.rpcException->second ||
+                     parsed.rpcException == *jt.rpcException->second)),
+               "apply: Got RPC result "s + parsed.rpcError + " (" +
+                   parsed.rpcException + "); Expected " +
+                   jt.rpcException->first + " (" +
+                   jt.rpcException->second.value_or("n/a") + ")" + line)) ||
         bad;
     if (bad)
     {

src/test/jtx/impl/testline.cpp

@@ -0,0 +1,15 @@
+#include <test/jtx/testline.h>
+
+namespace ripple {
+namespace test {
+namespace jtx {
+
+void
+testline::operator()(Env&, JTx& jt) const
+{
+    jt.testLine = line_;
+}
+
+}  // namespace jtx
+}  // namespace test
+}  // namespace ripple

src/test/jtx/testline.h

@@ -0,0 +1,34 @@
+#ifndef XRPL_TEST_JTX_TESTLINE_H_INCLUDED
+#define XRPL_TEST_JTX_TESTLINE_H_INCLUDED
+
+#include <test/jtx/Env.h>
+
+namespace ripple {
+namespace test {
+namespace jtx {
+
+/** Store the line number of the current test in a JTx.
+
+    Intended to help debug failing transaction submission tests.
+*/
+class testline
+{
+private:
+    int line_;
+
+public:
+    explicit testline(int line) : line_(line)
+    {
+    }
+
+    void
+    operator()(Env&, JTx& jt) const;
+};
+
+#define THISLINE testline(__LINE__)
+
+}  // namespace jtx
+}  // namespace test
+}  // namespace ripple
+
+#endif