diff --git a/src/ripple/app/tx/impl/DeleteAccount.cpp b/src/ripple/app/tx/impl/DeleteAccount.cpp index 6447c510b..9b7b57169 100644 --- a/src/ripple/app/tx/impl/DeleteAccount.cpp +++ b/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 is within 256 of the diff --git a/src/test/app/Import_test.cpp b/src/test/app/Import_test.cpp index d7a69df4d..c706bc054 100644 --- a/src/test/app/Import_test.cpp +++ b/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 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);