Merge branch 'develop' into ximinez/emptydirectoryinvariant

conan.lock

@@ -6,18 +6,18 @@
         "sqlite3/3.49.1#8631739a4c9b93bd3d6b753bac548a63%1756234266.869",
         "soci/4.0.3#a9f8d773cd33e356b5879a4b0564f287%1756234262.318",
         "snappy/1.1.10#968fef506ff261592ec30c574d4a7809%1756234314.246",
-        "rocksdb/10.5.1#4a197eca381a3e5ae8adf8cffa5aacd0%1759820024.194",
+        "rocksdb/10.5.1#4a197eca381a3e5ae8adf8cffa5aacd0%1762797952.535",
         "re2/20230301#dfd6e2bf050eb90ddd8729cfb4c844a4%1756234257.976",
         "protobuf/3.21.12#d927114e28de9f4691a6bbcdd9a529d1%1756234251.614",
         "openssl/3.5.4#a1d5835cc6ed5c5b8f3cd5b9b5d24205%1760106486.594",
-        "nudb/2.0.9#c62cfd501e57055a7e0d8ee3d5e5427d%1756234237.107",
+        "nudb/2.0.9#fb8dfd1a5557f5e0528114c2da17721e%1763150366.909",
         "lz4/1.10.0#59fc63cac7f10fbe8e05c7e62c2f3504%1756234228.999",
         "libiconv/1.17#1e65319e945f2d31941a9d28cc13c058%1756223727.64",
         "libbacktrace/cci.20210118#a7691bfccd8caaf66309df196790a5a1%1756230911.03",
         "libarchive/3.8.1#5cf685686322e906cb42706ab7e099a8%1756234256.696",
         "jemalloc/5.3.0#e951da9cf599e956cebc117880d2d9f8%1729241615.244",
         "grpc/1.50.1#02291451d1e17200293a409410d1c4e1%1756234248.958",
-        "doctest/2.4.12#eb9fb352fb2fdfc8abb17ec270945165%1749889324.069",
+        "doctest/2.4.12#eb9fb352fb2fdfc8abb17ec270945165%1762797941.757",
         "date/3.0.4#f74bbba5a08fa388256688743136cb6f%1756234217.493",
         "c-ares/1.34.5#b78b91e7cfb1f11ce777a285bbf169c6%1756234217.915",
         "bzip2/1.0.8#00b4a4658791c1f06914e087f0e792f5%1756234261.716",
@@ -53,6 +53,9 @@
         ],
         "lz4/[>=1.9.4 <2]": [
             "lz4/1.10.0#59fc63cac7f10fbe8e05c7e62c2f3504"
+        ],
+        "sqlite3/3.44.2": [
+            "sqlite3/3.49.1"
         ]
     },
     "config_requires": []

include/xrpl/protocol/detail/features.macro

@@ -66,7 +66,6 @@ XRPL_FEATURE(XRPFees,                    Supported::yes, VoteBehavior::DefaultNo
 XRPL_FEATURE(DisallowIncoming,           Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FIX    (RemoveNFTokenAutoTrustLine, Supported::yes, VoteBehavior::DefaultYes)
 XRPL_FEATURE(FlowSortStrands,            Supported::yes, VoteBehavior::DefaultYes)
-XRPL_FEATURE(TicketBatch,                Supported::yes, VoteBehavior::DefaultYes)
 XRPL_FEATURE(NegativeUNL,                Supported::yes, VoteBehavior::DefaultYes)
 XRPL_FEATURE(RequireFullyCanonicalSig,   Supported::yes, VoteBehavior::DefaultYes)
 XRPL_FEATURE(DeletableAccounts,          Supported::yes, VoteBehavior::DefaultYes)
@@ -134,5 +133,6 @@ XRPL_RETIRE_FEATURE(MultiSignReserve)
 XRPL_RETIRE_FEATURE(NonFungibleTokensV1_1)
 XRPL_RETIRE_FEATURE(PayChan)
 XRPL_RETIRE_FEATURE(SortedDirectories)
+XRPL_RETIRE_FEATURE(TicketBatch)
 XRPL_RETIRE_FEATURE(TickSize)
 XRPL_RETIRE_FEATURE(TrustSetAuth)

include/xrpl/protocol/detail/transactions.macro

@@ -155,7 +155,7 @@ TRANSACTION(ttOFFER_CANCEL, 8, OfferCancel,
 #endif
 TRANSACTION(ttTICKET_CREATE, 10, TicketCreate,
     Delegation::delegatable,
-    featureTicketBatch,
+    uint256{},
     noPriv,
     ({
     {sfTicketCount, soeREQUIRED},

src/test/app/Delegate_test.cpp

@@ -1703,7 +1703,6 @@ class Delegate_test : public beast::unit_test::suite
         // NFTokenMint, NFTokenBurn, NFTokenCreateOffer, NFTokenCancelOffer,
         // NFTokenAcceptOffer are not included, they are tested separately.
         std::unordered_map<std::string, uint256> txRequiredFeatures{
-            {"TicketCreate", featureTicketBatch},
             {"CheckCreate", featureChecks},
             {"CheckCash", featureChecks},
             {"CheckCancel", featureChecks},

src/test/app/Ticket_test.cpp

@@ -360,52 +360,6 @@ class Ticket_test : public beast::unit_test::suite
         BEAST_EXPECT(ticketSeq < acctRootSeq);
     }
 
-    void
-    testTicketNotEnabled()
-    {
-        testcase("Feature Not Enabled");
-
-        using namespace test::jtx;
-        Env env{*this, testable_amendments() - featureTicketBatch};
-
-        env(ticket::create(env.master, 1), ter(temDISABLED));
-        env.close();
-        env.require(owners(env.master, 0), tickets(env.master, 0));
-
-        env(noop(env.master), ticket::use(1), ter(temMALFORMED));
-        env(noop(env.master),
-            ticket::use(1),
-            seq(env.seq(env.master)),
-            ter(temMALFORMED));
-
-        // Close enough ledgers that the previous transactions are no
-        // longer retried.
-        for (int i = 0; i < 8; ++i)
-            env.close();
-
-        env.enableFeature(featureTicketBatch);
-        env.close();
-        env.require(owners(env.master, 0), tickets(env.master, 0));
-
-        std::uint32_t ticketSeq{env.seq(env.master) + 1};
-        env(ticket::create(env.master, 2));
-        checkTicketCreateMeta(env);
-        env.close();
-        env.require(owners(env.master, 2), tickets(env.master, 2));
-
-        env(noop(env.master), ticket::use(ticketSeq++));
-        checkTicketConsumeMeta(env);
-        env.close();
-        env.require(owners(env.master, 1), tickets(env.master, 1));
-
-        env(fset(env.master, asfDisableMaster),
-            ticket::use(ticketSeq++),
-            ter(tecNO_ALTERNATIVE_KEY));
-        checkTicketConsumeMeta(env);
-        env.close();
-        env.require(owners(env.master, 0), tickets(env.master, 0));
-    }
-
     void
     testTicketCreatePreflightFail()
     {
@@ -907,70 +861,43 @@ class Ticket_test : public beast::unit_test::suite
     void
     testFixBothSeqAndTicket()
     {
+        using namespace test::jtx;
+
         // It is an error if a transaction contains a non-zero Sequence field
         // and a TicketSequence field.  Verify that the error is detected.
         testcase("Fix both Seq and Ticket");
 
-        // Try the test without featureTicketBatch enabled.
+        Env env{*this, testable_amendments()};
-        using namespace test::jtx;
+        Account alice{"alice"};
-        {
-            Env env{*this, testable_amendments() - featureTicketBatch};
-            Account alice{"alice"};
 
-            env.fund(XRP(10000), alice);
+        env.fund(XRP(10000), alice);
-            env.close();
+        env.close();
 
-            // Fail to create a ticket.
+        // Create a ticket.
-            std::uint32_t const ticketSeq = env.seq(alice) + 1;
+        std::uint32_t const ticketSeq = env.seq(alice) + 1;
-            env(ticket::create(alice, 1), ter(temDISABLED));
+        env(ticket::create(alice, 1));
-            env.close();
+        env.close();
-            env.require(owners(alice, 0), tickets(alice, 0));
+        env.require(owners(alice, 1), tickets(alice, 1));
-            BEAST_EXPECT(ticketSeq == env.seq(alice) + 1);
+        BEAST_EXPECT(ticketSeq + 1 == env.seq(alice));
 
-            // Create a transaction that includes both a ticket and a non-zero
+        // Create a transaction that includes both a ticket and a non-zero
-            // sequence number.  Since a ticket is used and tickets are not yet
+        // sequence number.  The transaction fails with temSEQ_AND_TICKET.
-            // enabled the transaction should be malformed.
+        env(noop(alice),
-            env(noop(alice),
+            ticket::use(ticketSeq),
-                ticket::use(ticketSeq),
+            seq(env.seq(alice)),
-                seq(env.seq(alice)),
+            ter(temSEQ_AND_TICKET));
-                ter(temMALFORMED));
+        env.close();
-            env.close();
-        }
-        // Try the test with featureTicketBatch enabled.
-        {
-            Env env{*this, testable_amendments()};
-            Account alice{"alice"};
 
-            env.fund(XRP(10000), alice);
+        // Verify that the transaction failed by looking at alice's
-            env.close();
+        // sequence number and tickets.
-
+        env.require(owners(alice, 1), tickets(alice, 1));
-            // Create a ticket.
+        BEAST_EXPECT(ticketSeq + 1 == env.seq(alice));
-            std::uint32_t const ticketSeq = env.seq(alice) + 1;
-            env(ticket::create(alice, 1));
-            env.close();
-            env.require(owners(alice, 1), tickets(alice, 1));
-            BEAST_EXPECT(ticketSeq + 1 == env.seq(alice));
-
-            // Create a transaction that includes both a ticket and a non-zero
-            // sequence number.  The transaction fails with temSEQ_AND_TICKET.
-            env(noop(alice),
-                ticket::use(ticketSeq),
-                seq(env.seq(alice)),
-                ter(temSEQ_AND_TICKET));
-            env.close();
-
-            // Verify that the transaction failed by looking at alice's
-            // sequence number and tickets.
-            env.require(owners(alice, 1), tickets(alice, 1));
-            BEAST_EXPECT(ticketSeq + 1 == env.seq(alice));
-        }
     }
 
 public:
     void
     run() override
     {
-        testTicketNotEnabled();
         testTicketCreatePreflightFail();
         testTicketCreatePreclaimFail();
         testTicketInsufficientReserve();

src/test/app/Vault_test.cpp

@@ -1329,7 +1329,7 @@ class Vault_test : public beast::unit_test::suite
                      Vault& vault) {
             auto [tx, keylet] = vault.create({.owner = owner, .asset = asset});
             testcase("insufficient fee");
-            env(tx, fee(env.current()->fees().base), ter(telINSUF_FEE_P));
+            env(tx, fee(env.current()->fees().base - 1), ter(telINSUF_FEE_P));
         });
 
         testCase([this](
@@ -2074,6 +2074,10 @@ class Vault_test : public beast::unit_test::suite
                     auto const sleMPT = env.le(mptoken);
                     BEAST_EXPECT(sleMPT == nullptr);
 
+                    // Use one reserve so the next transaction fails
+                    env(ticket::create(owner, 1));
+                    env.close();
+
                     // No reserve to create MPToken for asset in VaultWithdraw
                     tx = vault.withdraw(
                         {.depositor = owner,
@@ -2091,7 +2095,7 @@ class Vault_test : public beast::unit_test::suite
                 }
             },
             {.requireAuth = false,
-             .initialXRP = acctReserve + incReserve * 4 - 1});
+             .initialXRP = acctReserve + incReserve * 4 + 1});
 
         testCase([this](
                      Env& env,
@@ -2980,6 +2984,9 @@ class Vault_test : public beast::unit_test::suite
                     env.le(keylet::line(owner, asset.raw().get<Issue>()));
                 BEAST_EXPECT(trustline == nullptr);
 
+                env(ticket::create(owner, 1));
+                env.close();
+
                 // Fail because not enough reserve to create trust line
                 tx = vault.withdraw(
                     {.depositor = owner,
@@ -2995,7 +3002,7 @@ class Vault_test : public beast::unit_test::suite
                 env(tx);
                 env.close();
             },
-            CaseArgs{.initialXRP = acctReserve + incReserve * 4 - 1});
+            CaseArgs{.initialXRP = acctReserve + incReserve * 4 + 1});
 
         testCase(
             [&, this](
@@ -3016,8 +3023,7 @@ class Vault_test : public beast::unit_test::suite
                 env(pay(owner, charlie, asset(100)));
                 env.close();
 
-                // Use up some reserve on tickets
+                env(ticket::create(charlie, 3));
-                env(ticket::create(charlie, 2));
                 env.close();
 
                 // Fail because not enough reserve to create MPToken for shares
@@ -3035,7 +3041,7 @@ class Vault_test : public beast::unit_test::suite
                 env(tx);
                 env.close();
             },
-            CaseArgs{.initialXRP = acctReserve + incReserve * 4 - 1});
+            CaseArgs{.initialXRP = acctReserve + incReserve * 4 + 1});
 
         testCase([&, this](
                      Env& env,

src/test/jtx/impl/vault.cpp

@@ -19,7 +19,6 @@ Vault::create(CreateArgs const& args)
     jv[jss::TransactionType] = jss::VaultCreate;
     jv[jss::Account] = args.owner.human();
     jv[jss::Asset] = to_json(args.asset);
-    jv[jss::Fee] = STAmount(env.current()->fees().increment).getJson();
     if (args.flags)
         jv[jss::Flags] = *args.flags;
     return {jv, keylet};

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

@@ -143,14 +143,6 @@ preflightCheckSimulateKeys(
 NotTEC
 Transactor::preflight1(PreflightContext const& ctx, std::uint32_t flagMask)
 {
-    // This is inappropriate in preflight0, because only Change transactions
-    // skip this function, and those do not allow an sfTicketSequence field.
-    if (ctx.tx.isFieldPresent(sfTicketSequence) &&
-        !ctx.rules.enabled(featureTicketBatch))
-    {
-        return temMALFORMED;
-    }
-
     if (ctx.tx.isFieldPresent(sfDelegate))
     {
         if (!ctx.rules.enabled(featurePermissionDelegationV1_1))
@@ -442,8 +434,7 @@ Transactor::checkSeqProxy(
 
     if (t_seqProx.isSeq())
     {
-        if (tx.isFieldPresent(sfTicketSequence) &&
+        if (tx.isFieldPresent(sfTicketSequence))
-            view.rules().enabled(featureTicketBatch))
         {
             JLOG(j.trace()) << "applyTransaction: has both a TicketSequence "
                                "and a non-zero Sequence number";

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

@@ -79,13 +79,6 @@ VaultCreate::preflight(PreflightContext const& ctx)
     return tesSUCCESS;
 }
 
-XRPAmount
-VaultCreate::calculateBaseFee(ReadView const& view, STTx const& tx)
-{
-    // One reserve increment is typically much greater than one base fee.
-    return calculateOwnerReserveFee(view, tx);
-}
-
 TER
 VaultCreate::preclaim(PreclaimContext const& ctx)
 {
@@ -142,8 +135,9 @@ VaultCreate::doApply()
 
     if (auto ter = dirLink(view(), account_, vault))
         return ter;
-    adjustOwnerCount(view(), owner, 1, j_);
+    // We will create Vault and PseudoAccount, hence increase OwnerCount by 2
-    auto ownerCount = owner->at(sfOwnerCount);
+    adjustOwnerCount(view(), owner, 2, j_);
+    auto const ownerCount = owner->at(sfOwnerCount);
     if (mPriorBalance < view().fees().accountReserve(ownerCount))
         return tecINSUFFICIENT_RESERVE;
 

src/xrpld/app/tx/detail/VaultCreate.h

@@ -23,9 +23,6 @@ public:
     static NotTEC
     preflight(PreflightContext const& ctx);
 
-    static XRPAmount
-    calculateBaseFee(ReadView const& view, STTx const& tx);
-
     static TER
     preclaim(PreclaimContext const& ctx);
 

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

@@ -146,7 +146,35 @@ VaultDelete::doApply()
         return tecHAS_OBLIGATIONS;  // LCOV_EXCL_LINE
 
     // Destroy the pseudo-account.
-    view().erase(view().peek(keylet::account(pseudoID)));
+    auto vaultPseudoSLE = view().peek(keylet::account(pseudoID));
+    if (!vaultPseudoSLE || vaultPseudoSLE->at(~sfVaultID) != vault->key())
+        return tefBAD_LEDGER;  // LCOV_EXCL_LINE
+
+    // Making the payment and removing the empty holding should have deleted any
+    // obligations associated with the vault or vault pseudo-account.
+    if (*vaultPseudoSLE->at(sfBalance))
+    {
+        // LCOV_EXCL_START
+        JLOG(j_.error()) << "VaultDelete: pseudo-account has a balance";
+        return tecHAS_OBLIGATIONS;
+        // LCOV_EXCL_STOP
+    }
+    if (vaultPseudoSLE->at(sfOwnerCount) != 0)
+    {
+        // LCOV_EXCL_START
+        JLOG(j_.error()) << "VaultDelete: pseudo-account still owns objects";
+        return tecHAS_OBLIGATIONS;
+        // LCOV_EXCL_STOP
+    }
+    if (view().exists(keylet::ownerDir(pseudoID)))
+    {
+        // LCOV_EXCL_START
+        JLOG(j_.error()) << "VaultDelete: pseudo-account has a directory";
+        return tecHAS_OBLIGATIONS;
+        // LCOV_EXCL_STOP
+    }
+
+    view().erase(vaultPseudoSLE);
 
     // Remove the vault from its owner's directory.
     auto const ownerID = vault->at(sfOwner);
@@ -170,7 +198,9 @@ VaultDelete::doApply()
         return tefBAD_LEDGER;
         // LCOV_EXCL_STOP
     }
-    adjustOwnerCount(view(), owner, -1, j_);
+
+    // We are destroying Vault and PseudoAccount, hence decrease by 2
+    adjustOwnerCount(view(), owner, -2, j_);
 
     // Destroy the vault.
     view().erase(vault);