put tests back

Signed-off-by: Pratik Mankawde <pmankawde@ripple.com>

include/xrpl/protocol/detail/features.macro

@@ -99,7 +99,6 @@ XRPL_FEATURE(HardenedValidations,        Supported::yes, VoteBehavior::DefaultYe
 XRPL_FEATURE(RequireFullyCanonicalSig,   Supported::yes, VoteBehavior::DefaultYes)
 XRPL_FIX    (QualityUpperBound,          Supported::yes, VoteBehavior::DefaultYes)
 XRPL_FEATURE(DeletableAccounts,          Supported::yes, VoteBehavior::DefaultYes)
-XRPL_FIX    (PayChanRecipientOwnerDir,   Supported::yes, VoteBehavior::DefaultYes)
 XRPL_FIX    (MasterKeyAsRegularKey,      Supported::yes, VoteBehavior::DefaultYes)
 XRPL_FEATURE(MultiSignReserve,           Supported::yes, VoteBehavior::DefaultYes)
 XRPL_FEATURE(DepositPreauth,             Supported::yes, VoteBehavior::DefaultYes)
@@ -141,6 +140,7 @@ XRPL_RETIRE(fix1571)
 XRPL_RETIRE(fix1578)
 XRPL_RETIRE(fix1781)
 XRPL_RETIRE(fixCheckThreading)
+XRPL_RETIRE(fixPayChanRecipientOwnerDir)
 XRPL_RETIRE(fixRmSmallIncreasedQOffers)
 XRPL_RETIRE(fixSTAmountCanonicalize)
 XRPL_RETIRE(fixTakerDryOfferRemoval)

src/test/app/AccountDelete_test.cpp

@@ -287,12 +287,8 @@ public:
 
         testcase("Owned types");
 
-        // We want to test both...
-        //  o Old-style PayChannels without a recipient backlink as well as
-        //  o New-styled PayChannels with the backlink.
-        // So we start the test using old-style PayChannels.  Then we pass
-        // the amendment to get new-style PayChannels.
-        Env env{*this, testable_amendments() - fixPayChanRecipientOwnerDir};
+        // We want to test PayChannels with the backlink.
+        Env env{*this, testable_amendments()};
         Account const alice("alice");
         Account const becky("becky");
         Account const gw("gw");
@@ -393,16 +389,14 @@ public:
             alice, becky, XRP(57), 4s, env.now() + 2s, alice.pk()));
         env.close();
 
-        // An old-style PayChannel does not add a back link from the
-        // destination.  So with the PayChannel in place becky should be
-        // able to delete her account, but alice should not.
+        // With the PayChannel in place becky and alice should not be
+        // able to delete her account
         auto const beckyBalance{env.balance(becky)};
         env(acctdelete(alice, gw), fee(acctDelFee), ter(tecHAS_OBLIGATIONS));
-        env(acctdelete(becky, gw), fee(acctDelFee));
-        verifyDeliveredAmount(env, beckyBalance - acctDelFee);
+        env(acctdelete(becky, gw), fee(acctDelFee), ter(tecHAS_OBLIGATIONS));
         env.close();
 
-        // Alice cancels her PayChannel which will leave her with only offers
+        // Alice cancels her PayChannel, which will leave her with only offers
         // in her directory.
 
         // Lambda to close a PayChannel.
@@ -420,14 +414,8 @@ public:
         env(payChanClose(alice, alicePayChanKey, alice.pk()));
         env.close();
 
-        // Now enable the amendment so PayChannels add a backlink from the
-        // destination.
-        env.enableFeature(fixPayChanRecipientOwnerDir);
-        env.close();
-
-        // gw creates a PayChannel with alice as the destination.  With the
-        // amendment passed this should prevent alice from deleting her
-        // account.
+        // gw creates a PayChannel with alice as the destination, this should
+        // prevent alice from deleting her account.
         Keylet const gwPayChanKey{keylet::payChan(gw, alice, env.seq(gw))};
 
         env(payChanCreate(gw, alice, XRP(68), 4s, env.now() + 2s, alice.pk()));
@@ -449,84 +437,6 @@ public:
         env.close();
     }
 
-    void
-    testResurrection()
-    {
-        // Create an account with an old-style PayChannel.  Delete the
-        // destination of the PayChannel then resurrect the destination.
-        // The PayChannel should still work.
-        using namespace jtx;
-
-        testcase("Resurrection");
-
-        // We need an old-style PayChannel that doesn't provide a backlink
-        // from the destination.  So don't enable the amendment with that fix.
-        Env env{*this, testable_amendments() - fixPayChanRecipientOwnerDir};
-        Account const alice("alice");
-        Account const becky("becky");
-
-        env.fund(XRP(10000), alice, becky);
-        env.close();
-
-        // Verify that becky's account root is present.
-        Keylet const beckyAcctKey{keylet::account(becky.id())};
-        BEAST_EXPECT(env.closed()->exists(beckyAcctKey));
-
-        using namespace std::chrono_literals;
-        Keylet const payChanKey{keylet::payChan(alice, becky, env.seq(alice))};
-        auto const payChanXRP = XRP(37);
-
-        env(payChanCreate(
-            alice, becky, payChanXRP, 4s, env.now() + 1h, alice.pk()));
-        env.close();
-        BEAST_EXPECT(env.closed()->exists(payChanKey));
-
-        // Close enough ledgers to be able to delete becky's account.
-        incLgrSeqForAccDel(env, becky);
-
-        auto const beckyPreDelBalance{env.balance(becky)};
-
-        auto const acctDelFee{drops(env.current()->fees().increment)};
-        env(acctdelete(becky, alice), fee(acctDelFee));
-        verifyDeliveredAmount(env, beckyPreDelBalance - acctDelFee);
-        env.close();
-
-        // Verify that becky's account root is gone.
-        BEAST_EXPECT(!env.closed()->exists(beckyAcctKey));
-
-        // All it takes is a large enough XRP payment to resurrect
-        // becky's account.  Try too small a payment.
-        env(pay(alice,
-                becky,
-                drops(env.current()->fees().accountReserve(0)) - XRP(1)),
-            ter(tecNO_DST_INSUF_XRP));
-        env.close();
-
-        // Actually resurrect becky's account.
-        env(pay(alice, becky, XRP(10)));
-        env.close();
-
-        // becky's account root should be back.
-        BEAST_EXPECT(env.closed()->exists(beckyAcctKey));
-        BEAST_EXPECT(env.balance(becky) == XRP(10));
-
-        // becky's resurrected account can be the destination of alice's
-        // PayChannel.
-        auto payChanClaim = [&]() {
-            Json::Value jv;
-            jv[jss::TransactionType] = jss::PaymentChannelClaim;
-            jv[jss::Account] = alice.human();
-            jv[sfChannel.jsonName] = to_string(payChanKey.key);
-            jv[sfBalance.jsonName] =
-                payChanXRP.value().getJson(JsonOptions::none);
-            return jv;
-        };
-        env(payChanClaim());
-        env.close();
-
-        BEAST_EXPECT(env.balance(becky) == XRP(10) + payChanXRP);
-    }
-
     void
     testAmendmentEnable()
     {
@@ -1257,7 +1167,6 @@ public:
         testBasics();
         testDirectories();
         testOwnedTypes();
-        testResurrection();
         testAmendmentEnable();
         testTooManyOffers();
         testImplicitlyCreatedTrustline();

src/test/app/PayChan_test.cpp

@@ -1841,36 +1841,6 @@ struct PayChan_test : public beast::unit_test::suite
             return std::distance(ownerDir.begin(), ownerDir.end());
         };
 
-        {
-            // Test without adding the paychan to the recipient's owner
-            // directory
-            Env env(*this, features - fixPayChanRecipientOwnerDir);
-            env.fund(XRP(10000), alice, bob);
-            env(create(alice, bob, XRP(1000), settleDelay, pk));
-            env.close();
-            auto const [chan, chanSle] =
-                channelKeyAndSle(*env.current(), alice, bob);
-            BEAST_EXPECT(inOwnerDir(*env.current(), alice, chanSle));
-            BEAST_EXPECT(ownerDirCount(*env.current(), alice) == 1);
-            BEAST_EXPECT(!inOwnerDir(*env.current(), bob, chanSle));
-            BEAST_EXPECT(ownerDirCount(*env.current(), bob) == 0);
-            if (features[fixIncludeKeyletFields])
-            {
-                BEAST_EXPECT((*chanSle)[sfSequence] == env.seq(alice) - 1);
-            }
-            else
-            {
-                BEAST_EXPECT(!chanSle->isFieldPresent(sfSequence));
-            }
-            // close the channel
-            env(claim(bob, chan), txflags(tfClose));
-            BEAST_EXPECT(!channelExists(*env.current(), chan));
-            BEAST_EXPECT(!inOwnerDir(*env.current(), alice, chanSle));
-            BEAST_EXPECT(ownerDirCount(*env.current(), alice) == 0);
-            BEAST_EXPECT(!inOwnerDir(*env.current(), bob, chanSle));
-            BEAST_EXPECT(ownerDirCount(*env.current(), bob) == 0);
-        }
-
         {
             // Test with adding the paychan to the recipient's owner directory
             Env env{*this, features};
@@ -1895,7 +1865,7 @@ struct PayChan_test : public beast::unit_test::suite
         {
             // Test removing paychans created before adding to the recipient's
             // owner directory
-            Env env(*this, features - fixPayChanRecipientOwnerDir);
+            Env env(*this, features);
             env.fund(XRP(10000), alice, bob);
             // create the channel before the amendment activates
             env(create(alice, bob, XRP(1000), settleDelay, pk));
@@ -1904,21 +1874,9 @@ struct PayChan_test : public beast::unit_test::suite
                 channelKeyAndSle(*env.current(), alice, bob);
             BEAST_EXPECT(inOwnerDir(*env.current(), alice, chanSle));
             BEAST_EXPECT(ownerDirCount(*env.current(), alice) == 1);
-            BEAST_EXPECT(!inOwnerDir(*env.current(), bob, chanSle));
-            BEAST_EXPECT(ownerDirCount(*env.current(), bob) == 0);
-            env.enableFeature(fixPayChanRecipientOwnerDir);
-            env.close();
-            BEAST_EXPECT(
-                env.current()->rules().enabled(fixPayChanRecipientOwnerDir));
-            // These checks look redundant, but if you don't `close` after the
-            // `create` these checks will fail. I believe this is due to the
-            // create running with one set of amendments initially, then with a
-            // different set with the ledger closes (tho I haven't dug into it)
-            BEAST_EXPECT(inOwnerDir(*env.current(), alice, chanSle));
-            BEAST_EXPECT(!inOwnerDir(*env.current(), bob, chanSle));
-            BEAST_EXPECT(ownerDirCount(*env.current(), bob) == 0);
+            BEAST_EXPECT(inOwnerDir(*env.current(), bob, chanSle));
+            BEAST_EXPECT(ownerDirCount(*env.current(), bob) == 1);
 
-            // close the channel after the amendment activates
             env(claim(bob, chan), txflags(tfClose));
             BEAST_EXPECT(!channelExists(*env.current(), chan));
             BEAST_EXPECT(!inOwnerDir(*env.current(), alice, chanSle));
@@ -1960,12 +1918,8 @@ struct PayChan_test : public beast::unit_test::suite
         auto const bob = Account("bob");
         auto const carol = Account("carol");
 
-        for (bool const withOwnerDirFix : {false, true})
         {
-            auto const amd = withOwnerDirFix
-                ? features
-                : features - fixPayChanRecipientOwnerDir;
-            Env env{*this, amd};
+            Env env{*this, features};
             env.fund(XRP(10000), alice, bob, carol);
             env.close();
 
@@ -1980,11 +1934,7 @@ struct PayChan_test : public beast::unit_test::suite
 
             rmAccount(env, alice, carol, tecHAS_OBLIGATIONS);
             // can only remove bob if the channel isn't in their owner direcotry
-            rmAccount(
-                env,
-                bob,
-                carol,
-                withOwnerDirFix ? TER(tecHAS_OBLIGATIONS) : TER(tesSUCCESS));
+            rmAccount(env, bob, carol, TER(tecHAS_OBLIGATIONS));
 
             auto const feeDrops = env.current()->fees().base;
             auto chanBal = channelBalance(*env.current(), chan);
@@ -1999,152 +1949,21 @@ struct PayChan_test : public beast::unit_test::suite
             assert(reqBal <= chanAmt);
 
             // claim should fail if the dst was removed
-            if (withOwnerDirFix)
-            {
-                env(claim(alice, chan, reqBal, authAmt));
-                env.close();
-                BEAST_EXPECT(channelBalance(*env.current(), chan) == reqBal);
-                BEAST_EXPECT(channelAmount(*env.current(), chan) == chanAmt);
-                BEAST_EXPECT(env.balance(bob) == preBob + delta);
-                chanBal = reqBal;
-            }
-            else
-            {
-                auto const preAlice = env.balance(alice);
-                env(claim(alice, chan, reqBal, authAmt), ter(tecNO_DST));
-                env.close();
-                BEAST_EXPECT(channelBalance(*env.current(), chan) == chanBal);
-                BEAST_EXPECT(channelAmount(*env.current(), chan) == chanAmt);
-                BEAST_EXPECT(env.balance(bob) == preBob);
-                BEAST_EXPECT(env.balance(alice) == preAlice - feeDrops);
-            }
+            env(claim(alice, chan, reqBal, authAmt));
+            env.close();
+            BEAST_EXPECT(channelBalance(*env.current(), chan) == reqBal);
+            BEAST_EXPECT(channelAmount(*env.current(), chan) == chanAmt);
+            BEAST_EXPECT(env.balance(bob) == preBob + delta);
+            chanBal = reqBal;
 
             // fund should fail if the dst was removed
-            if (withOwnerDirFix)
-            {
-                auto const preAlice = env.balance(alice);
-                env(fund(alice, chan, XRP(1000)));
-                env.close();
-                BEAST_EXPECT(
-                    env.balance(alice) == preAlice - XRP(1000) - feeDrops);
-                BEAST_EXPECT(
-                    channelAmount(*env.current(), chan) == chanAmt + XRP(1000));
-                chanAmt = chanAmt + XRP(1000);
-            }
-            else
-            {
-                auto const preAlice = env.balance(alice);
-                env(fund(alice, chan, XRP(1000)), ter(tecNO_DST));
-                env.close();
-                BEAST_EXPECT(env.balance(alice) == preAlice - feeDrops);
-                BEAST_EXPECT(channelAmount(*env.current(), chan) == chanAmt);
-            }
-
-            {
-                // Owner closes, will close after settleDelay
-                env(claim(alice, chan), txflags(tfClose));
-                env.close();
-                // settle delay hasn't ellapsed. Channels should exist.
-                BEAST_EXPECT(channelExists(*env.current(), chan));
-                auto const closeTime = env.current()->info().parentCloseTime;
-                auto const minExpiration = closeTime + settleDelay;
-                env.close(minExpiration);
-                env(claim(alice, chan), txflags(tfClose));
-                BEAST_EXPECT(!channelExists(*env.current(), chan));
-            }
-        }
-
-        {
-            // test resurrected account
-            Env env{*this, features - fixPayChanRecipientOwnerDir};
-            env.fund(XRP(10000), alice, bob, carol);
+            auto const preAlice = env.balance(alice);
+            env(fund(alice, chan, XRP(1000)));
             env.close();
-
-            // Create a channel from alice to bob
-            auto const pk = alice.pk();
-            auto const settleDelay = 100s;
-            auto const chan = channel(alice, bob, env.seq(alice));
-            env(create(alice, bob, XRP(1000), settleDelay, pk));
-            env.close();
-            BEAST_EXPECT(channelBalance(*env.current(), chan) == XRP(0));
-            BEAST_EXPECT(channelAmount(*env.current(), chan) == XRP(1000));
-
-            // Since `fixPayChanRecipientOwnerDir` is not active, can remove bob
-            rmAccount(env, bob, carol);
-            BEAST_EXPECT(!env.closed()->exists(keylet::account(bob.id())));
-
-            auto const feeDrops = env.current()->fees().base;
-            auto chanBal = channelBalance(*env.current(), chan);
-            auto chanAmt = channelAmount(*env.current(), chan);
-            BEAST_EXPECT(chanBal == XRP(0));
-            BEAST_EXPECT(chanAmt == XRP(1000));
-            auto preBob = env.balance(bob);
-            auto const delta = XRP(50);
-            auto reqBal = chanBal + delta;
-            auto authAmt = reqBal + XRP(100);
-            assert(reqBal <= chanAmt);
-
-            {
-                // claim should fail, since bob doesn't exist
-                auto const preAlice = env.balance(alice);
-                env(claim(alice, chan, reqBal, authAmt), ter(tecNO_DST));
-                env.close();
-                BEAST_EXPECT(channelBalance(*env.current(), chan) == chanBal);
-                BEAST_EXPECT(channelAmount(*env.current(), chan) == chanAmt);
-                BEAST_EXPECT(env.balance(bob) == preBob);
-                BEAST_EXPECT(env.balance(alice) == preAlice - feeDrops);
-            }
-
-            {
-                // fund should fail, sincebob doesn't exist
-                auto const preAlice = env.balance(alice);
-                env(fund(alice, chan, XRP(1000)), ter(tecNO_DST));
-                env.close();
-                BEAST_EXPECT(env.balance(alice) == preAlice - feeDrops);
-                BEAST_EXPECT(channelAmount(*env.current(), chan) == chanAmt);
-            }
-
-            // resurrect bob
-            env(pay(alice, bob, XRP(20)));
-            env.close();
-            BEAST_EXPECT(env.closed()->exists(keylet::account(bob.id())));
-
-            {
-                // alice should be able to claim
-                preBob = env.balance(bob);
-                reqBal = chanBal + delta;
-                authAmt = reqBal + XRP(100);
-                env(claim(alice, chan, reqBal, authAmt));
-                BEAST_EXPECT(channelBalance(*env.current(), chan) == reqBal);
-                BEAST_EXPECT(channelAmount(*env.current(), chan) == chanAmt);
-                BEAST_EXPECT(env.balance(bob) == preBob + delta);
-                chanBal = reqBal;
-            }
-
-            {
-                // bob should be able to claim
-                preBob = env.balance(bob);
-                reqBal = chanBal + delta;
-                authAmt = reqBal + XRP(100);
-                auto const sig =
-                    signClaimAuth(alice.pk(), alice.sk(), chan, authAmt);
-                env(claim(bob, chan, reqBal, authAmt, Slice(sig), alice.pk()));
-                BEAST_EXPECT(channelBalance(*env.current(), chan) == reqBal);
-                BEAST_EXPECT(channelAmount(*env.current(), chan) == chanAmt);
-                BEAST_EXPECT(env.balance(bob) == preBob + delta - feeDrops);
-                chanBal = reqBal;
-            }
-
-            {
-                // alice should be able to fund
-                auto const preAlice = env.balance(alice);
-                env(fund(alice, chan, XRP(1000)));
-                BEAST_EXPECT(
-                    env.balance(alice) == preAlice - XRP(1000) - feeDrops);
-                BEAST_EXPECT(
-                    channelAmount(*env.current(), chan) == chanAmt + XRP(1000));
-                chanAmt = chanAmt + XRP(1000);
-            }
+            BEAST_EXPECT(env.balance(alice) == preAlice - XRP(1000) - feeDrops);
+            BEAST_EXPECT(
+                channelAmount(*env.current(), chan) == chanAmt + XRP(1000));
+            chanAmt = chanAmt + XRP(1000);
 
             {
                 // Owner closes, will close after settleDelay

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

@@ -135,8 +135,7 @@ closeChannel(
     }
 
     // Remove PayChan from recipient's owner directory, if present.
-    if (auto const page = (*slep)[~sfDestinationNode];
-        page && view.rules().enabled(fixPayChanRecipientOwnerDir))
+    if (auto const page = (*slep)[~sfDestinationNode])
     {
         auto const dst = (*slep)[sfDestination];
         if (!view.dirRemove(keylet::ownerDir(dst), *page, key, true))
@@ -303,7 +302,6 @@ PayChanCreate::doApply()
     }
 
     // Add PayChan to the recipient's owner directory
-    if (ctx_.view().rules().enabled(fixPayChanRecipientOwnerDir))
     {
         auto const page = ctx_.view().dirInsert(
             keylet::ownerDir(dst), payChanKeylet, describeOwnerDir(dst));