Account index tests (#120)

* fix `sfImportSequence` & decrease account count

* add account count test

* add import account delete test

* add account index for payment

* add guards

* revert decrease account count on delete

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

@@ -201,10 +201,6 @@ DeleteAccount::preclaim(PreclaimContext const& ctx)
     if (!sleDst)
         return tecNO_DST;
 
-    // accounts created via Import are blocked from deletion
-    if (sleDst->isFieldPresent(sfImportSequence))
-        return tecHAS_OBLIGATIONS;
-
     if ((*sleDst)[sfFlags] & lsfRequireDestTag && !ctx.tx[~sfDestinationTag])
         return tecDST_TAG_NEEDED;
 
@@ -221,6 +217,13 @@ DeleteAccount::preclaim(PreclaimContext const& ctx)
     if (!sleAccount)
         return terNO_ACCOUNT;
 
+    // accounts created via Import are blocked from deletion
+    if (ctx.view.rules().enabled(featureImport))
+    {
+        if (sleAccount->isFieldPresent(sfImportSequence))
+            return tecHAS_OBLIGATIONS;
+    }
+
     if (ctx.view.rules().enabled(featureNonFungibleTokensV1))
     {
         // If an issuer has any issued NFTs resident in the ledger then it
@@ -253,8 +256,11 @@ DeleteAccount::preclaim(PreclaimContext const& ctx)
     // do not allow the account to be removed if there are hooks installed or one or more hook states
     // when these fields are completely empty the field is made absent so this test is sufficient
     // these fields cannot be populated unless hooks is enabled so the rules do not need to be checked
-    if (sleAccount->isFieldPresent(sfHookNamespaces) || sleAccount->isFieldPresent(sfHooks))
-        return tecHAS_OBLIGATIONS;
+    if (ctx.view.rules().enabled(featureHooks))
+    {
+        if (sleAccount->isFieldPresent(sfHookNamespaces) || sleAccount->isFieldPresent(sfHooks))
+            return tecHAS_OBLIGATIONS;
+    }
     
     // When fixNFTokenRemint is enabled, we don't allow an account to be
     // deleted if <FirstNFTokenSequence + MintedNFTokens> is within 256 of the

src/test/app/Import_test.cpp

@@ -225,6 +225,23 @@ class Import_test : public beast::unit_test::suite
         return slep != nullptr;
     }
 
+    void
+    incLgrSeqForAccDel(
+        jtx::Env& env,
+        jtx::Account const& acc,
+        std::uint32_t margin = 0)
+    {
+        int const delta = [&]() -> int {
+            if (env.seq(acc) + 255 > env.current()->seq())
+                return env.seq(acc) - env.current()->seq() + 255 - margin;
+            return 0;
+        }();
+        BEAST_EXPECT(margin == 0 || delta >= 0);
+        for (int i = 0; i < delta; ++i)
+            env.close();
+        BEAST_EXPECT(env.current()->seq() == env.seq(acc) + 255 - margin);
+    }
+
     void
     testComputeStartingBalance(FeatureBitset features)
     {
@@ -4348,13 +4365,14 @@ class Import_test : public beast::unit_test::suite
     }
 
     void
-    testAccountCount(FeatureBitset features)
+    testAccountIndex(FeatureBitset features)
     {
-        testcase("account count");
+        testcase("account index");
 
         using namespace test::jtx;
         using namespace std::literals;
 
+        // Account Index from Import
         {
             test::jtx::Env env{*this, makeNetworkVLConfig(21337, keys)};
             auto const feeDrops = env.current()->fees().base;
@@ -4390,6 +4408,42 @@ class Import_test : public beast::unit_test::suite
             auto const [fee, feeSle] = feesKeyAndSle(*env.current());
             BEAST_EXPECT((*feeSle)[sfAccountCount] == 1);
         }
+
+        // Account Index from Payment
+        {
+            test::jtx::Env env{*this, makeNetworkVLConfig(21337, keys)};
+            auto const feeDrops = env.current()->fees().base;
+
+            auto const alice = Account("alice");
+            auto const bob = Account("bob");
+            auto const carol = Account("carol");
+            env.fund(XRP(1000), alice, bob, carol);
+            env.close();
+
+            struct TestAccountData
+            {
+                Account acct;
+                std::uint64_t index;
+            };
+
+            std::array<TestAccountData, 3> acctTests = {{
+                {alice, 0},
+                {bob, 1},
+                {carol, 2},
+            }};
+
+            for (auto const& t : acctTests)
+            {
+                // confirm index was set
+                auto const [acct, acctSle] =
+                    accountKeyAndSle(*env.current(), t.acct);
+                BEAST_EXPECT((*acctSle)[sfAccountIndex] == t.index);
+            }
+
+            // confirm count was updated
+            auto const [fee, feeSle] = feesKeyAndSle(*env.current());
+            BEAST_EXPECT((*feeSle)[sfAccountCount] == 3);
+        }
     }
 
     void
@@ -4484,6 +4538,44 @@ class Import_test : public beast::unit_test::suite
         }
     }
 
+    void
+    testAccountDelete(FeatureBitset features)
+    {
+        testcase("account delete");
+
+        using namespace test::jtx;
+        using namespace std::literals;
+
+        {
+            test::jtx::Env env{*this, makeNetworkVLConfig(21337, keys)};
+
+            auto const feeDrops = env.current()->fees().base;
+
+            auto const alice = Account("alice");
+            auto const bob = Account("bob");
+            env.fund(XRP(1000), alice, bob);
+            env.close();
+
+            // burn 10'000 xrp
+            auto const master = Account("masterpassphrase");
+            env(noop(master), fee(10'000'000'000), ter(tesSUCCESS));
+            env.close();
+
+            Json::Value const xpop = loadXpop(ImportTCAccountSet::w_seed);
+            Json::Value const tx = import(alice, xpop);
+            env(tx, fee(feeDrops * 10), ter(tesSUCCESS));
+
+            // Close enough ledgers to be able to delete alices's account.
+            incLgrSeqForAccDel(env, alice);
+
+            // alice cannot delete account after import
+            auto const acctDelFee{drops(env.current()->fees().increment)};
+            env(acctdelete(alice, bob),
+                fee(acctDelFee),
+                ter(tecHAS_OBLIGATIONS));
+        }
+    }
+
     void
     testImportSequence(FeatureBitset features)
     {
@@ -5398,9 +5490,10 @@ public:
         testSetRegularKey(features);
         testSetRegularKeyFlags(features);
         testSignersListSet(features);
-        testAccountCount(features);
+        testAccountIndex(features);
         testHookIssuer(features);
         testImportSequence(features);
+        testAccountDelete(features);
         testMaxSupply(features);
         testMinMax(features);
         testHalving(features - featureOwnerPaysFee);