PayChan and Escrow should ignore DisallowXRP (RIPD-1462)

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

@@ -235,7 +235,11 @@ EscrowCreate::doApply()
         if (((*sled)[sfFlags] & lsfRequireDestTag) &&
                 ! ctx_.tx[~sfDestinationTag])
             return tecDST_TAG_NEEDED;
-        if ((*sled)[sfFlags] & lsfDisallowXRP)
+
+        // Obeying the lsfDissalowXRP flag was a bug.  Piggyback on
+        // featureDepositAuth to remove the bug.
+        if (! ctx_.view().rules().enabled(featureDepositAuth) &&
+                ((*sled)[sfFlags] & lsfDisallowXRP))
             return tecNO_TARGET;
     }
 

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

@@ -206,7 +206,11 @@ PayChanCreate::preclaim(PreclaimContext const &ctx)
         if (((*sled)[sfFlags] & lsfRequireDestTag) &&
             !ctx.tx[~sfDestinationTag])
             return tecDST_TAG_NEEDED;
-        if ((*sled)[sfFlags] & lsfDisallowXRP)
+
+        // Obeying the lsfDisallowXRP flag was a bug.  Piggyback on
+        // featureDepositAuth to remove the bug.
+        if (!ctx.view.rules().enabled(featureDepositAuth) &&
+            ((*sled)[sfFlags] & lsfDisallowXRP))
             return tecNO_TARGET;
     }
 
@@ -457,15 +461,19 @@ PayChanClaim::doApply()
         if (!sled)
             return terNO_ACCOUNT;
 
-        if (txAccount == src && (sled->getFlags() & lsfDisallowXRP))
+        // Obeying the lsfDisallowXRP flag was a bug.  Piggyback on
+        // featureDepositAuth to remove the bug.
+        bool const depositAuth {ctx_.view().rules().enabled(featureDepositAuth)};
+        if (!depositAuth &&
+            (txAccount == src && (sled->getFlags() & lsfDisallowXRP)))
             return tecNO_TARGET;
 
         // Check whether the destination account requires deposit authorization
         if (txAccount != dst)
         {
-            if (ctx_.view().rules().enabled(featureDepositAuth) &&
+            if (depositAuth &&
                 ((sled->getFlags() & lsfDepositAuth) == lsfDepositAuth))
-                    return tecNO_PERMISSION;
+                return tecNO_PERMISSION;
         }
 
         (*slep)[sfBalance] = ctx_.tx[sfBalance];

src/test/app/Escrow_test.cpp

@@ -253,6 +253,35 @@ struct Escrow_test : public beast::unit_test::suite
         BEAST_EXPECT((*sle)[sfDestinationTag] == 2);
     }
 
+    void
+    testDisallowXRP()
+    {
+        testcase ("Disallow XRP");
+
+        using namespace jtx;
+        using namespace std::chrono;
+
+        {
+            // Respect the "asfDisallowXRP" account flag:
+            Env env(*this, supported_amendments() - featureDepositAuth);
+
+            env.fund(XRP(5000), "bob", "george");
+            env(fset("george", asfDisallowXRP));
+            env(lockup("bob", "george", XRP(10), env.now() + 1s),
+                ter (tecNO_TARGET));
+        }
+        {
+            // Ignore the "asfDisallowXRP" account flag, which we should
+            // have been doing before.
+            Env env(*this);
+
+            env.fund(XRP(5000), "bob", "george");
+            env(fset("george", asfDisallowXRP));
+            env(lockup("bob", "george", XRP(10), env.now() + 1s));
+        }
+
+    }
+
     void
     testFails()
     {
@@ -331,11 +360,6 @@ struct Escrow_test : public beast::unit_test::suite
         env.fund (accountReserve, "frank");
         env(lockup("frank", "bob", XRP(1), env.now() + 1s),   ter (tecINSUFFICIENT_RESERVE));
 
-        // Respect the "asfDisallowXRP" account flag:
-        env.fund (accountReserve + accountIncrement, "george");
-        env(fset("george", asfDisallowXRP));
-        env(lockup("bob", "george", XRP(10), env.now() + 1s),  ter (tecNO_TARGET));
-
         { // Specify incorrect sequence number
             env.fund (XRP(5000), "hannah");
             auto const seq = env.seq("hannah");
@@ -980,6 +1004,7 @@ struct Escrow_test : public beast::unit_test::suite
     {
         testEnablement();
         testTags();
+        testDisallowXRP();
         testFails();
         testLockup();
         testEscrowConditions();

src/test/app/PayChan_test.cpp

@@ -629,40 +629,57 @@ struct PayChan_test : public beast::unit_test::suite
         testcase ("Disallow XRP");
         using namespace jtx;
         using namespace std::literals::chrono_literals;
+
+        auto const alice = Account ("alice");
+        auto const bob = Account ("bob");
         {
             // Create a channel where dst disallows XRP
-            Env env (*this);
+            Env env (*this, supported_amendments() - featureDepositAuth);
-            auto const alice = Account ("alice");
-            auto const bob = Account ("bob");
             env.fund (XRP (10000), alice, bob);
             env (fset (bob, asfDisallowXRP));
-            auto const pk = alice.pk ();
+            env (create (alice, bob, XRP (1000), 3600s, alice.pk()),
-            auto const settleDelay = 3600s;
-            auto const channelFunds = XRP (1000);
-            env (create (alice, bob, channelFunds, settleDelay, pk),
                 ter (tecNO_TARGET));
             auto const chan = channel (*env.current (), alice, bob);
             BEAST_EXPECT (!channelExists (*env.current (), chan));
         }
+        {
+            // Create a channel where dst disallows XRP.  Ignore that flag,
+            // since it's just advisory.
+            Env env (*this);
+            env.fund (XRP (10000), alice, bob);
+            env (fset (bob, asfDisallowXRP));
+            env (create (alice, bob, XRP (1000), 3600s, alice.pk()));
+            auto const chan = channel (*env.current (), alice, bob);
+            BEAST_EXPECT (channelExists (*env.current (), chan));
+        }
+
         {
             // Claim to a channel where dst disallows XRP
             // (channel is created before disallow xrp is set)
-            Env env (*this);
+            Env env (*this, supported_amendments() - featureDepositAuth);
-            auto const alice = Account ("alice");
-            auto const bob = Account ("bob");
             env.fund (XRP (10000), alice, bob);
-            auto const pk = alice.pk ();
+            env (create (alice, bob, XRP (1000), 3600s, alice.pk()));
-            auto const settleDelay = 3600s;
-            auto const channelFunds = XRP (1000);
-            env (create (alice, bob, channelFunds, settleDelay, pk));
             auto const chan = channel (*env.current (), alice, bob);
             BEAST_EXPECT (channelExists (*env.current (), chan));
 
             env (fset (bob, asfDisallowXRP));
-            auto const preBob = env.balance (bob);
             auto const reqBal = XRP (500).value();
             env (claim (alice, chan, reqBal, reqBal), ter(tecNO_TARGET));
         }
+        {
+            // Claim to a channel where dst disallows XRP (channel is
+            // created before disallow xrp is set).  Ignore that flag
+            // since it is just advisory.
+            Env env (*this);
+            env.fund (XRP (10000), alice, bob);
+            env (create (alice, bob, XRP (1000), 3600s, alice.pk()));
+            auto const chan = channel (*env.current (), alice, bob);
+            BEAST_EXPECT (channelExists (*env.current (), chan));
+
+            env (fset (bob, asfDisallowXRP));
+            auto const reqBal = XRP (500).value();
+            env (claim (alice, chan, reqBal, reqBal));
+        }
     }
 
     void