Fix more problems introduced by 6adb2ec

- Renamed canSendToAccount to canWithdraw, because the semantics are
  a little different from a payment. Notably, if withdrawing to self,
  you can still include a destination tag.
- Simplified the interface to canWithdraw to just pass in the
  STTx.
- preflightDestinationAndTag is pretty pointless now, so removed it.

include/xrpl/ledger/View.h

@@ -702,16 +702,6 @@ isPseudoAccount(ReadView const& view, AccountID const& accountId)
 [[nodiscard]] TER
 canAddHolding(ReadView const& view, Asset const& asset);
 
-/** Checks that the destination and destination tag are valid.
-
-   - If destination is set, it must not be zero.
-   - If not set, there must not be a destination tag.
-*/
-[[nodiscard]] NotTEC
-preflightDestinationAndTag(
-    std::optional<AccountID> const& destination,
-    bool hasDestinationTag);
-
 /** Validates that the destination SLE and tag are valid
 
    - Checks that the SLE is not null.
@@ -720,34 +710,60 @@ preflightDestinationAndTag(
 [[nodiscard]] TER
 checkDestinationAndTag(SLE::const_ref toSle, bool hasDestinationTag);
 
-/** Checks that from can sent to to (with an SLE)
-
-   - Does the checkDestinationAndTag checks, plus:
-   - if the destination requires deposit auth, checks that the from account has
-     that auth.
-*/
+/** Checks that can withdraw funds from an object to itself or a destination.
+ *
+ * The receiver may be either the submitting account (sfAccount) or a different
+ * destination account (sfDestination).
+ *
+ *    - Checks that the receiver account exists.
+ *    - If the receiver requires a destination tag, check that one exists, even
+ *      if withdrawing to self.
+ *    - If withdrawing to self, succeed.
+ *    - If not, checks if the receiver requires deposit authorization, and if
+ *      the sender has it.
+ */
 [[nodiscard]] TER
-canSendToAccount(
+canWithdraw(
     AccountID const& from,
     ReadView const& view,
     AccountID const& to,
     SLE::const_ref toSle,
     bool hasDestinationTag);
 
-/** Checks that from can sent to to (with an AccountID)
-
-   - Loads the SLE for "to"
-   - Checks whether the account is trying to send to itself.
-     Succeeds if the SLE exists, fails if not.
-   - Then does the checkDestinationAndTag check with an SLE.
-*/
+/** Checks that can withdraw funds from an object to itself or a destination.
+ *
+ * The receiver may be either the submitting account (sfAccount) or a different
+ * destination account (sfDestination).
+ *
+ *    - Checks that the receiver account exists.
+ *    - If the receiver requires a destination tag, check that one exists, even
+ *      if withdrawing to self.
+ *    - If withdrawing to self, succeed.
+ *    - If not, checks if the receiver requires deposit authorization, and if
+ *      the sender has it.
+ */
 [[nodiscard]] TER
-canSendToAccount(
+canWithdraw(
     AccountID const& from,
     ReadView const& view,
     AccountID const& to,
     bool hasDestinationTag);
 
+/** Checks that can withdraw funds from an object to itself or a destination.
+ *
+ * The receiver may be either the submitting account (sfAccount) or a different
+ * destination account (sfDestination).
+ *
+ *    - Checks that the receiver account exists.
+ *    - If the receiver requires a destination tag, check that one exists, even
+ *      if withdrawing to self.
+ *    - If withdrawing to self, succeed.
+ *    - If not, checks if the receiver requires deposit authorization, and if
+ *      the sender has it.
+ */
+[[nodiscard]] TER
+canWithdraw(ReadView const& view, STTx const& tx);
+
 /// Any transactors that call addEmptyHolding() in doApply must call
 /// canAddHolding() in preflight with the same View and Asset
 [[nodiscard]] TER

src/libxrpl/ledger/View.cpp

@@ -1340,26 +1340,6 @@ canAddHolding(ReadView const& view, Asset const& asset)
         asset.value());
 }
 
-[[nodiscard]] NotTEC
-preflightDestinationAndTag(
-    std::optional<AccountID> const& destination,
-    bool hasDestinationTag)
-{
-    if (destination)
-    {
-        if (*destination == beast::zero)
-        {
-            return temMALFORMED;
-        }
-    }
-    else if (hasDestinationTag)
-    {
-        return temMALFORMED;
-    }
-
-    return tesSUCCESS;
-}
-
 [[nodiscard]] TER
 checkDestinationAndTag(SLE::const_ref toSle, bool hasDestinationTag)
 {
@@ -1375,7 +1355,7 @@ checkDestinationAndTag(SLE::const_ref toSle, bool hasDestinationTag)
 }
 
 [[nodiscard]] TER
-canSendToAccount(
+canWithdraw(
     AccountID const& from,
     ReadView const& view,
     AccountID const& to,
@@ -1398,7 +1378,7 @@ canSendToAccount(
 }
 
 [[nodiscard]] TER
-canSendToAccount(
+canWithdraw(
     AccountID const& from,
     ReadView const& view,
     AccountID const& to,
@@ -1406,7 +1386,16 @@ canSendToAccount(
 {
     auto const toSle = view.read(keylet::account(to));
 
-    return canSendToAccount(from, view, to, toSle, hasDestinationTag);
+    return canWithdraw(from, view, to, toSle, hasDestinationTag);
+}
+
+[[nodiscard]] TER
+canWithdraw(ReadView const& view, STTx const& tx)
+{
+    auto const from = tx[sfAccount];
+    auto const to = tx[~sfDestination].value_or(from);
+
+    return canWithdraw(from, view, to, tx.isFieldPresent(sfDestinationTag));
 }
 
 [[nodiscard]] TER

src/test/app/LoanBroker_test.cpp

@@ -384,13 +384,6 @@ class LoanBroker_test : public beast::unit_test::suite
                 destination(AccountID{}),
                 ter(temMALFORMED));
 
-            // If a destination tag is specified, a destination must be
-            // specified, too
-            env(coverWithdraw(alice, keylet.key, vault.asset(1)),
-                dtag(123),
-                ter(temMALFORMED));
-            verifyCoverAmount(10);
-
             // Withdraw some of the cover amount
             env(coverWithdraw(alice, keylet.key, vault.asset(7)));
             env.close();

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

@@ -43,9 +43,13 @@ LoanBrokerCoverWithdraw::preflight(PreflightContext const& ctx)
     if (!isLegalNet(dstAmount))
         return temBAD_AMOUNT;
 
-    if (auto const ret = preflightDestinationAndTag(
-            ctx.tx[~sfDestination], ctx.tx.isFieldPresent(sfDestinationTag)))
-        return ret;
+    if (auto const destination = ctx.tx[~sfDestination])
+    {
+        if (*destination == beast::zero)
+        {
+            return temMALFORMED;
+        }
+    }
 
     return tesSUCCESS;
 }
@@ -85,11 +89,7 @@ LoanBrokerCoverWithdraw::preclaim(PreclaimContext const& ctx)
     AuthType authType = AuthType::Legacy;
     if (account != dstAcct)
     {
-        if (auto const ret = canSendToAccount(
-                account,
-                ctx.view,
-                dstAcct,
-                tx.isFieldPresent(sfDestinationTag)))
+        if (auto const ret = canWithdraw(ctx.view, tx))
             return ret;
 
         // The destination account must have consented to receive the asset by

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

@@ -42,9 +42,13 @@ VaultWithdraw::preflight(PreflightContext const& ctx)
     if (ctx.tx[sfAmount] <= beast::zero)
         return temBAD_AMOUNT;
 
-    if (auto const ret = preflightDestinationAndTag(
-            ctx.tx[~sfDestination], ctx.tx.isFieldPresent(sfDestinationTag)))
-        return ret;
+    if (auto const destination = ctx.tx[~sfDestination])
+    {
+        if (*destination == beast::zero)
+        {
+            return temMALFORMED;
+        }
+    }
 
     return tesSUCCESS;
 }
@@ -105,11 +109,7 @@ VaultWithdraw::preclaim(PreclaimContext const& ctx)
     auto const account = ctx.tx[sfAccount];
     auto const dstAcct = ctx.tx[~sfDestination].value_or(account);
 
-    if (auto const ret = canSendToAccount(
-            account,
-            ctx.view,
-            dstAcct,
-            ctx.tx.isFieldPresent(sfDestinationTag)))
+    if (auto const ret = canWithdraw(ctx.view, ctx.tx))
         return ret;
 
     // If sending to Account (i.e. not a transfer), we will also create (only