remove temp vector

The tests that ensure `tfInnerBatchTxn` won't block delegated transactions silently fail in `Delegate_test.cpp`. This change removes these cases from that file and adds them to `Batch_test.cpp` instead where they do not silently fail, because there the batch delegate results are explicitly checked. Moving them to that file further avoids refactoring many helper functions.

.github/actions/build/action.yml

@@ -27,7 +27,6 @@ runs:
     - name: build
       shell: bash
       run: |
-        echo Running $(nproc) processes
         cmake \
           --build ${build_dir} \
           --config ${{ inputs.configuration }} \

cmake/RippledSanity.cmake

@@ -2,16 +2,6 @@
    convenience variables and sanity checks
 #]===================================================================]
 
-include(ProcessorCount)
-
-if (NOT ep_procs)
-  ProcessorCount(ep_procs)
-  if (ep_procs GREATER 1)
-    # never use more than half of cores for EP builds
-    math (EXPR ep_procs "${ep_procs} / 2")
-    message (STATUS "Using ${ep_procs} cores for ExternalProject builds.")
-  endif ()
-endif ()
 get_property(is_multiconfig GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG)
 
 set (CMAKE_CONFIGURATION_TYPES "Debug;Release" CACHE STRING "" FORCE)

src/libxrpl/protocol/BuildInfo.cpp

@@ -36,7 +36,7 @@ namespace BuildInfo {
 //  and follow the format described at http://semver.org/
 //------------------------------------------------------------------------------
 // clang-format off
-char const* const versionString = "2.5.0-rc1"
+char const* const versionString = "2.5.0-rc2"
 // clang-format on
 
 #if defined(DEBUG) || defined(SANITIZER)

src/libxrpl/protocol/STAmount.cpp

@@ -581,7 +581,10 @@ canAdd(STAmount const& a, STAmount const& b)
 
         return true;
     }
+    // LCOV_EXCL_START
+    UNREACHABLE("STAmount::canAdd : unexpected STAmount type");
     return false;
+    // LCOV_EXCL_STOP
 }
 
 /**
@@ -653,8 +656,10 @@ canSubtract(STAmount const& a, STAmount const& b)
             return false;
         return true;
     }
-
+    // LCOV_EXCL_START
+    UNREACHABLE("STAmount::canSubtract : unexpected STAmount type");
     return false;
+    // LCOV_EXCL_STOP
 }
 
 void

src/test/app/Batch_test.cpp

@@ -3765,6 +3765,8 @@ class Batch_test : public beast::unit_test::suite
         }
 
         // delegated non atomic inner (AccountSet)
+        // this also makes sure tfInnerBatchTxn won't block delegated AccountSet
+        // with granular permission
         {
             test::jtx::Env env{*this, envconfig()};
 
@@ -3810,6 +3812,145 @@ class Batch_test : public beast::unit_test::suite
             BEAST_EXPECT(env.balance(alice) == preAlice - XRP(2) - batchFee);
             BEAST_EXPECT(env.balance(bob) == preBob + XRP(2));
         }
+
+        // delegated non atomic inner (MPTokenIssuanceSet)
+        // this also makes sure tfInnerBatchTxn won't block delegated
+        // MPTokenIssuanceSet with granular permission
+        {
+            test::jtx::Env env{*this, envconfig()};
+            Account alice{"alice"};
+            Account bob{"bob"};
+            env.fund(XRP(100000), alice, bob);
+            env.close();
+
+            auto const mptID = makeMptID(env.seq(alice), alice);
+            MPTTester mpt(env, alice, {.fund = false});
+            env.close();
+            mpt.create({.flags = tfMPTCanLock});
+            env.close();
+
+            // alice gives granular permission to bob of MPTokenIssuanceLock
+            env(delegate::set(
+                alice, bob, {"MPTokenIssuanceLock", "MPTokenIssuanceUnlock"}));
+            env.close();
+
+            auto const seq = env.seq(alice);
+            auto const batchFee = batch::calcBatchFee(env, 0, 2);
+
+            Json::Value jv1;
+            jv1[sfTransactionType] = jss::MPTokenIssuanceSet;
+            jv1[sfAccount] = alice.human();
+            jv1[sfDelegate] = bob.human();
+            jv1[sfSequence] = seq + 1;
+            jv1[sfMPTokenIssuanceID] = to_string(mptID);
+            jv1[sfFlags] = tfMPTLock;
+
+            Json::Value jv2;
+            jv2[sfTransactionType] = jss::MPTokenIssuanceSet;
+            jv2[sfAccount] = alice.human();
+            jv2[sfDelegate] = bob.human();
+            jv2[sfSequence] = seq + 2;
+            jv2[sfMPTokenIssuanceID] = to_string(mptID);
+            jv2[sfFlags] = tfMPTUnlock;
+
+            auto const [txIDs, batchID] = submitBatch(
+                env,
+                tesSUCCESS,
+                batch::outer(alice, seq, batchFee, tfAllOrNothing),
+                batch::inner(jv1, seq + 1),
+                batch::inner(jv2, seq + 2));
+            env.close();
+
+            std::vector<TestLedgerData> testCases = {
+                {0, "Batch", "tesSUCCESS", batchID, std::nullopt},
+                {1, "MPTokenIssuanceSet", "tesSUCCESS", txIDs[0], batchID},
+                {2, "MPTokenIssuanceSet", "tesSUCCESS", txIDs[1], batchID},
+            };
+            validateClosedLedger(env, testCases);
+        }
+
+        // delegated non atomic inner (TrustSet)
+        // this also makes sure tfInnerBatchTxn won't block delegated TrustSet
+        // with granular permission
+        {
+            test::jtx::Env env{*this, envconfig()};
+            Account gw{"gw"};
+            Account alice{"alice"};
+            Account bob{"bob"};
+            env.fund(XRP(10000), gw, alice, bob);
+            env(fset(gw, asfRequireAuth));
+            env.close();
+            env(trust(alice, gw["USD"](50)));
+            env.close();
+
+            env(delegate::set(
+                gw, bob, {"TrustlineAuthorize", "TrustlineFreeze"}));
+            env.close();
+
+            auto const seq = env.seq(gw);
+            auto const batchFee = batch::calcBatchFee(env, 0, 2);
+
+            auto jv1 = trust(gw, gw["USD"](0), alice, tfSetfAuth);
+            jv1[sfDelegate] = bob.human();
+            auto jv2 = trust(gw, gw["USD"](0), alice, tfSetFreeze);
+            jv2[sfDelegate] = bob.human();
+
+            auto const [txIDs, batchID] = submitBatch(
+                env,
+                tesSUCCESS,
+                batch::outer(gw, seq, batchFee, tfAllOrNothing),
+                batch::inner(jv1, seq + 1),
+                batch::inner(jv2, seq + 2));
+            env.close();
+
+            std::vector<TestLedgerData> testCases = {
+                {0, "Batch", "tesSUCCESS", batchID, std::nullopt},
+                {1, "TrustSet", "tesSUCCESS", txIDs[0], batchID},
+                {2, "TrustSet", "tesSUCCESS", txIDs[1], batchID},
+            };
+            validateClosedLedger(env, testCases);
+        }
+
+        // inner transaction not authorized by the delegating account.
+        {
+            test::jtx::Env env{*this, envconfig()};
+            Account gw{"gw"};
+            Account alice{"alice"};
+            Account bob{"bob"};
+            env.fund(XRP(10000), gw, alice, bob);
+            env(fset(gw, asfRequireAuth));
+            env.close();
+            env(trust(alice, gw["USD"](50)));
+            env.close();
+
+            env(delegate::set(
+                gw, bob, {"TrustlineAuthorize", "TrustlineFreeze"}));
+            env.close();
+
+            auto const seq = env.seq(gw);
+            auto const batchFee = batch::calcBatchFee(env, 0, 2);
+
+            auto jv1 = trust(gw, gw["USD"](0), alice, tfSetFreeze);
+            jv1[sfDelegate] = bob.human();
+            auto jv2 = trust(gw, gw["USD"](0), alice, tfClearFreeze);
+            jv2[sfDelegate] = bob.human();
+
+            auto const [txIDs, batchID] = submitBatch(
+                env,
+                tesSUCCESS,
+                batch::outer(gw, seq, batchFee, tfIndependent),
+                batch::inner(jv1, seq + 1),
+                // tecNO_DELEGATE_PERMISSION: not authorized to clear freeze
+                batch::inner(jv2, seq + 2));
+            env.close();
+
+            std::vector<TestLedgerData> testCases = {
+                {0, "Batch", "tesSUCCESS", batchID, std::nullopt},
+                {1, "TrustSet", "tesSUCCESS", txIDs[0], batchID},
+                {2, "TrustSet", "tecNO_DELEGATE_PERMISSION", txIDs[1], batchID},
+            };
+            validateClosedLedger(env, testCases);
+        }
     }
 
     void

src/test/app/Delegate_test.cpp

@@ -913,37 +913,6 @@ class Delegate_test : public beast::unit_test::suite
                     gw, gw["USD"](0), alice, tfSetfAuth | tfFullyCanonicalSig),
                 delegate::as(bob));
         }
-
-        // tfInnerBatchTxn won't block delegated transaction
-        {
-            Env env(*this);
-            Account gw{"gw"};
-            Account alice{"alice"};
-            Account bob{"bob"};
-            env.fund(XRP(10000), gw, alice, bob);
-            env(fset(gw, asfRequireAuth));
-            env.close();
-            env(trust(alice, gw["USD"](50)));
-            env.close();
-
-            env(delegate::set(
-                gw, bob, {"TrustlineAuthorize", "TrustlineFreeze"}));
-            env.close();
-
-            auto const seq = env.seq(gw);
-            auto const batchFee = batch::calcBatchFee(env, 0, 2);
-            auto jv1 = trust(gw, gw["USD"](0), alice, tfSetfAuth);
-            jv1[sfDelegate] = bob.human();
-            auto jv2 = trust(gw, gw["USD"](0), alice, tfSetFreeze);
-            jv2[sfDelegate] = bob.human();
-
-            // batch::inner will set tfInnerBatchTxn, this should not
-            // block delegated transaction
-            env(batch::outer(gw, seq, batchFee, tfAllOrNothing),
-                batch::inner(jv1, seq + 1),
-                batch::inner(jv2, seq + 2));
-            env.close();
-        }
     }
 
     void
@@ -1234,53 +1203,6 @@ class Delegate_test : public beast::unit_test::suite
             env(jt);
             BEAST_EXPECT((*env.le(alice))[sfDomain] == makeSlice(domain));
         }
-
-        // tfInnerBatchTxn won't block delegated transaction
-        {
-            Env env(*this);
-            Account alice{"alice"};
-            Account bob{"bob"};
-            env.fund(XRP(10000), alice, bob);
-            env.close();
-
-            env(delegate::set(
-                alice, bob, {"AccountDomainSet", "AccountEmailHashSet"}));
-            env.close();
-
-            auto const seq = env.seq(alice);
-            auto const batchFee = batch::calcBatchFee(env, 0, 3);
-
-            auto jv1 = noop(alice);
-            std::string const domain1 = "example1.com";
-            jv1[sfDomain] = strHex(domain1);
-            jv1[sfDelegate] = bob.human();
-            jv1[sfSequence] = seq + 1;
-
-            auto jv2 = noop(alice);
-            std::string const domain2 = "example2.com";
-            jv2[sfDomain] = strHex(domain2);
-            jv2[sfDelegate] = bob.human();
-            jv2[sfSequence] = seq + 2;
-
-            // bob set domain back and add email hash for alice
-            auto jv3 = noop(alice);
-            std::string const mh("5F31A79367DC3137FADA860C05742EE6");
-            jv3[sfDomain] = strHex(domain1);
-            jv3[sfEmailHash] = mh;
-            jv3[sfDelegate] = bob.human();
-            jv3[sfSequence] = seq + 3;
-
-            // batch::inner will set tfInnerBatchTxn, this should not
-            // block delegated transaction
-            env(batch::outer(alice, seq, batchFee, tfAllOrNothing),
-                batch::inner(jv1, seq + 1),
-                batch::inner(jv2, seq + 2),
-                batch::inner(jv3, seq + 3));
-            env.close();
-
-            BEAST_EXPECT((*env.le(alice))[sfDomain] == makeSlice(domain1));
-            BEAST_EXPECT(to_string((*env.le(alice))[sfEmailHash]) == mh);
-        }
     }
 
     void
@@ -1411,52 +1333,6 @@ class Delegate_test : public beast::unit_test::suite
                  .flags = tfMPTLock | tfFullyCanonicalSig,
                  .delegate = bob});
         }
-
-        // tfInnerBatchTxn won't block delegated transaction
-        {
-            Env env(*this);
-            Account alice{"alice"};
-            Account bob{"bob"};
-            env.fund(XRP(100000), alice, bob);
-            env.close();
-
-            auto const mptID = makeMptID(env.seq(alice), alice);
-            MPTTester mpt(env, alice, {.fund = false});
-            env.close();
-            mpt.create({.flags = tfMPTCanLock});
-            env.close();
-
-            // alice gives granular permission to bob of MPTokenIssuanceLock
-            env(delegate::set(
-                alice, bob, {"MPTokenIssuanceLock", "MPTokenIssuanceUnlock"}));
-            env.close();
-
-            auto const seq = env.seq(alice);
-            auto const batchFee = batch::calcBatchFee(env, 0, 2);
-
-            Json::Value jv1;
-            jv1[sfTransactionType] = jss::MPTokenIssuanceSet;
-            jv1[sfAccount] = alice.human();
-            jv1[sfDelegate] = bob.human();
-            jv1[sfSequence] = seq + 1;
-            jv1[sfMPTokenIssuanceID] = to_string(mptID);
-            jv1[sfFlags] = tfMPTLock;
-
-            Json::Value jv2;
-            jv2[sfTransactionType] = jss::MPTokenIssuanceSet;
-            jv2[sfAccount] = alice.human();
-            jv2[sfDelegate] = bob.human();
-            jv2[sfSequence] = seq + 2;
-            jv2[sfMPTokenIssuanceID] = to_string(mptID);
-            jv2[sfFlags] = tfMPTUnlock;
-
-            // batch::inner will set tfInnerBatchTxn, this should not
-            // block delegated transaction
-            env(batch::outer(alice, seq, batchFee, tfAllOrNothing),
-                batch::inner(jv1, seq + 1),
-                batch::inner(jv2, seq + 2));
-            env.close();
-        }
     }
 
     void

src/test/app/EscrowToken_test.cpp

@@ -21,9 +21,11 @@
 
 #include <xrpld/app/tx/applySteps.h>
 #include <xrpld/ledger/Dir.h>
+#include <xrpld/ledger/Sandbox.h>
 
 #include <xrpl/protocol/Feature.h>
 #include <xrpl/protocol/Indexes.h>
+#include <xrpl/protocol/STAmount.h>
 #include <xrpl/protocol/TxFlags.h>
 #include <xrpl/protocol/jss.h>
 
@@ -56,27 +58,39 @@ struct EscrowToken_test : public beast::unit_test::suite
         return 0;
     }
 
-    void
-    issuerIOUEscrowed(
+    jtx::PrettyAmount
+    issuerBalance(
         jtx::Env& env,
         jtx::Account const& account,
-        Currency const& currency,
-        int const& outstanding,
-        int const& locked)
+        Issue const& issue)
     {
         Json::Value params;
         params[jss::account] = account.human();
         auto jrr = env.rpc("json", "gateway_balances", to_string(params));
         auto const result = jrr[jss::result];
-        auto const actualOutstanding =
-            result[jss::obligations][to_string(currency)];
-        BEAST_EXPECT(actualOutstanding == to_string(outstanding));
-        if (locked != 0)
-        {
-            auto const actualEscrowed =
-                result[jss::locked][to_string(currency)];
-            BEAST_EXPECT(actualEscrowed == to_string(locked));
-        }
+        auto const obligations =
+            result[jss::obligations][to_string(issue.currency)];
+        if (obligations.isNull())
+            return {STAmount(issue, 0), account.name()};
+        STAmount const amount = amountFromString(issue, obligations.asString());
+        return {amount, account.name()};
+    }
+
+    jtx::PrettyAmount
+    issuerEscrowed(
+        jtx::Env& env,
+        jtx::Account const& account,
+        Issue const& issue)
+    {
+        Json::Value params;
+        params[jss::account] = account.human();
+        auto jrr = env.rpc("json", "gateway_balances", to_string(params));
+        auto const result = jrr[jss::result];
+        auto const locked = result[jss::locked][to_string(issue.currency)];
+        if (locked.isNull())
+            return {STAmount(issue, 0), account.name()};
+        STAmount const amount = amountFromString(issue, locked.asString());
+        return {amount, account.name()};
     }
 
     void
@@ -136,6 +150,37 @@ struct EscrowToken_test : public beast::unit_test::suite
             env(escrow::cancel(bob, alice, seq2), finishResult);
             env.close();
         }
+
+        for (bool const withTokenEscrow : {false, true})
+        {
+            auto const amend =
+                withTokenEscrow ? features : features - featureTokenEscrow;
+            Env env{*this, amend};
+            auto const baseFee = env.current()->fees().base;
+            auto const alice = Account("alice");
+            auto const bob = Account("bob");
+            auto const gw = Account{"gateway"};
+            auto const USD = gw["USD"];
+            env.fund(XRP(5000), alice, bob, gw);
+            env(fset(gw, asfAllowTrustLineLocking));
+            env.close();
+            env.trust(USD(10'000), alice, bob);
+            env.close();
+            env(pay(gw, alice, USD(5000)));
+            env(pay(gw, bob, USD(5000)));
+            env.close();
+
+            auto const seq1 = env.seq(alice);
+            env(escrow::finish(bob, alice, seq1),
+                escrow::condition(escrow::cb1),
+                escrow::fulfillment(escrow::fb1),
+                fee(baseFee * 150),
+                ter(tecNO_TARGET));
+            env.close();
+
+            env(escrow::cancel(bob, alice, seq1), ter(tecNO_TARGET));
+            env.close();
+        }
     }
 
     void
@@ -865,34 +910,76 @@ struct EscrowToken_test : public beast::unit_test::suite
         env.close();
         env.trust(USD(10'000), alice, bob);
         env.close();
-        env(pay(gw, alice, USD(5000)));
-        env(pay(gw, bob, USD(5000)));
+        env(pay(gw, alice, USD(5'000)));
+        env(pay(gw, bob, USD(5'000)));
         env.close();
 
+        auto const outstandingUSD = USD(10'000);
+
+        // Create & Finish Escrow
         auto const seq1 = env.seq(alice);
-        env(escrow::create(alice, bob, USD(1'000)),
-            escrow::condition(escrow::cb1),
-            escrow::finish_time(env.now() + 1s),
-            fee(baseFee * 150),
-            ter(tesSUCCESS));
-        env.close();
-        env(escrow::finish(bob, alice, seq1),
-            escrow::condition(escrow::cb1),
-            escrow::fulfillment(escrow::fb1),
-            fee(baseFee * 150),
-            ter(tesSUCCESS));
-        env.close();
+        {
+            auto const preAliceUSD = env.balance(alice, USD);
+            auto const preBobUSD = env.balance(bob, USD);
+            env(escrow::create(alice, bob, USD(1'000)),
+                escrow::condition(escrow::cb1),
+                escrow::finish_time(env.now() + 1s),
+                fee(baseFee * 150),
+                ter(tesSUCCESS));
+            env.close();
 
+            BEAST_EXPECT(env.balance(alice, USD) == preAliceUSD - USD(1'000));
+            BEAST_EXPECT(env.balance(bob, USD) == preBobUSD);
+            BEAST_EXPECT(
+                issuerBalance(env, gw, USD) == outstandingUSD - USD(1'000));
+            BEAST_EXPECT(issuerEscrowed(env, gw, USD) == USD(1'000));
+        }
+        {
+            auto const preAliceUSD = env.balance(alice, USD);
+            auto const preBobUSD = env.balance(bob, USD);
+            env(escrow::finish(bob, alice, seq1),
+                escrow::condition(escrow::cb1),
+                escrow::fulfillment(escrow::fb1),
+                fee(baseFee * 150),
+                ter(tesSUCCESS));
+            env.close();
+
+            BEAST_EXPECT(env.balance(alice, USD) == preAliceUSD);
+            BEAST_EXPECT(env.balance(bob, USD) == preBobUSD + USD(1'000));
+            BEAST_EXPECT(issuerBalance(env, gw, USD) == outstandingUSD);
+            BEAST_EXPECT(issuerEscrowed(env, gw, USD) == USD(0));
+        }
+
+        // Create & Cancel Escrow
         auto const seq2 = env.seq(alice);
-        env(escrow::create(alice, bob, USD(1'000)),
-            escrow::condition(escrow::cb2),
-            escrow::finish_time(env.now() + 1s),
-            escrow::cancel_time(env.now() + 2s),
-            fee(baseFee * 150),
-            ter(tesSUCCESS));
-        env.close();
-        env(escrow::cancel(bob, alice, seq2), ter(tesSUCCESS));
-        env.close();
+        {
+            auto const preAliceUSD = env.balance(alice, USD);
+            auto const preBobUSD = env.balance(bob, USD);
+            env(escrow::create(alice, bob, USD(1'000)),
+                escrow::condition(escrow::cb2),
+                escrow::finish_time(env.now() + 1s),
+                escrow::cancel_time(env.now() + 2s),
+                fee(baseFee * 150),
+                ter(tesSUCCESS));
+            env.close();
+
+            BEAST_EXPECT(env.balance(alice, USD) == preAliceUSD - USD(1'000));
+            BEAST_EXPECT(env.balance(bob, USD) == preBobUSD);
+            BEAST_EXPECT(
+                issuerBalance(env, gw, USD) == outstandingUSD - USD(1'000));
+            BEAST_EXPECT(issuerEscrowed(env, gw, USD) == USD(1'000));
+        }
+        {
+            auto const preAliceUSD = env.balance(alice, USD);
+            auto const preBobUSD = env.balance(bob, USD);
+            env(escrow::cancel(bob, alice, seq2), ter(tesSUCCESS));
+            env.close();
+
+            BEAST_EXPECT(env.balance(alice, USD) == preAliceUSD + USD(1'000));
+            BEAST_EXPECT(env.balance(bob, USD) == preBobUSD);
+            BEAST_EXPECT(issuerBalance(env, gw, USD) == outstandingUSD);
+            BEAST_EXPECT(issuerEscrowed(env, gw, USD) == USD(0));
+        }
     }
 
     void
@@ -2430,7 +2517,6 @@ struct EscrowToken_test : public beast::unit_test::suite
             mptGw.authorize({.account = alice});
             mptGw.authorize({.account = bob});
             auto const MPT = mptGw["MPT"];
-            env(pay(gw, alice, MPT(10)));
             env(pay(gw, bob, MPT(10)));
             env.close();
 
@@ -2521,6 +2607,39 @@ struct EscrowToken_test : public beast::unit_test::suite
             env.close();
         }
 
+        // tecOBJECT_NOT_FOUND: MPT issuance does not exist
+        {
+            Env env{*this, features};
+            auto const baseFee = env.current()->fees().base;
+            auto const alice = Account("alice");
+            auto const bob = Account("bob");
+            env.fund(XRP(10'000), alice, bob);
+            env.close();
+
+            auto const seq1 = env.seq(alice);
+            env.app().openLedger().modify(
+                [&](OpenView& view, beast::Journal j) {
+                    Sandbox sb(&view, tapNONE);
+                    auto sleNew =
+                        std::make_shared<SLE>(keylet::escrow(alice, seq1));
+                    MPTIssue const mpt{
+                        MPTIssue{makeMptID(1, AccountID(0x4985601))}};
+                    STAmount amt(mpt, 10);
+                    sleNew->setAccountID(sfDestination, bob);
+                    sleNew->setFieldAmount(sfAmount, amt);
+                    sb.insert(sleNew);
+                    sb.apply(view);
+                    return true;
+                });
+
+            env(escrow::finish(bob, alice, seq1),
+                escrow::condition(escrow::cb1),
+                escrow::fulfillment(escrow::fb1),
+                fee(baseFee * 150),
+                ter(tecOBJECT_NOT_FOUND));
+            env.close();
+        }
+
         // tecLOCKED: issuer has locked the dest
         {
             Env env{*this, features};
@@ -2726,6 +2845,36 @@ struct EscrowToken_test : public beast::unit_test::suite
             env(escrow::cancel(bob, alice, seq1), ter(tecNO_AUTH));
             env.close();
         }
+
+        // tecOBJECT_NOT_FOUND: MPT issuance does not exist
+        {
+            Env env{*this, features};
+            auto const baseFee = env.current()->fees().base;
+            auto const alice = Account("alice");
+            auto const bob = Account("bob");
+            env.fund(XRP(10'000), alice, bob);
+
+            auto const seq1 = env.seq(alice);
+            env.app().openLedger().modify(
+                [&](OpenView& view, beast::Journal j) {
+                    Sandbox sb(&view, tapNONE);
+                    auto sleNew =
+                        std::make_shared<SLE>(keylet::escrow(alice, seq1));
+                    MPTIssue const mpt{
+                        MPTIssue{makeMptID(1, AccountID(0x4985601))}};
+                    STAmount amt(mpt, 10);
+                    sleNew->setAccountID(sfDestination, bob);
+                    sleNew->setFieldAmount(sfAmount, amt);
+                    sb.insert(sleNew);
+                    sb.apply(view);
+                    return true;
+                });
+
+            env(escrow::cancel(bob, alice, seq1),
+                fee(baseFee),
+                ter(tecOBJECT_NOT_FOUND));
+            env.close();
+        }
     }
 
     void
@@ -3603,12 +3752,14 @@ struct EscrowToken_test : public beast::unit_test::suite
                 fee(baseFee * 150));
             env.close();
 
+            env(pay(alice, gw, MPT(10'000)), ter(tecPATH_PARTIAL));
             env(pay(alice, gw, MPT(9'990)));
             env(pay(bob, gw, MPT(10'000)));
             BEAST_EXPECT(env.balance(alice, MPT) == MPT(0));
             BEAST_EXPECT(mptEscrowed(env, alice, MPT) == 10);
             BEAST_EXPECT(env.balance(bob, MPT) == MPT(0));
             BEAST_EXPECT(mptEscrowed(env, bob, MPT) == 0);
+            BEAST_EXPECT(env.balance(gw, MPT) == MPT(10));
             mptGw.authorize({.account = bob, .flags = tfMPTUnauthorize});
             mptGw.destroy(
                 {.id = mptGw.issuanceID(),

src/test/app/Escrow_test.cpp

@@ -369,6 +369,12 @@ struct Escrow_test : public beast::unit_test::suite
         env.fund(XRP(5000), "alice", "bob", "gw");
         env.close();
 
+        // temINVALID_FLAG
+        env(escrow::create("alice", "bob", XRP(1000)),
+            escrow::finish_time(env.now() + 5s),
+            txflags(tfPassive),
+            ter(temINVALID_FLAG));
+
         // Finish time is in the past
         env(escrow::create("alice", "bob", XRP(1000)),
             escrow::finish_time(env.now() - 5s),

src/test/ledger/Invariants_test.cpp

@@ -762,6 +762,153 @@ class Invariants_test : public beast::unit_test::suite
                 ac.view().insert(sleNew);
                 return true;
             });
+
+        // IOU < 0
+        doInvariantCheck(
+            {{"escrow specifies invalid amount"}},
+            [](Account const& A1, Account const&, ApplyContext& ac) {
+                // escrow with too-little iou
+                auto const sle = ac.view().peek(keylet::account(A1.id()));
+                if (!sle)
+                    return false;
+                auto sleNew = std::make_shared<SLE>(
+                    keylet::escrow(A1, (*sle)[sfSequence] + 2));
+
+                Issue const usd{
+                    Currency(0x5553440000000000), AccountID(0x4985601)};
+                STAmount amt(usd, -1);
+                sleNew->setFieldAmount(sfAmount, amt);
+                ac.view().insert(sleNew);
+                return true;
+            });
+
+        // IOU bad currency
+        doInvariantCheck(
+            {{"escrow specifies invalid amount"}},
+            [](Account const& A1, Account const&, ApplyContext& ac) {
+                // escrow with bad iou currency
+                auto const sle = ac.view().peek(keylet::account(A1.id()));
+                if (!sle)
+                    return false;
+                auto sleNew = std::make_shared<SLE>(
+                    keylet::escrow(A1, (*sle)[sfSequence] + 2));
+
+                Issue const bad{badCurrency(), AccountID(0x4985601)};
+                STAmount amt(bad, 1);
+                sleNew->setFieldAmount(sfAmount, amt);
+                ac.view().insert(sleNew);
+                return true;
+            });
+
+        // MPT < 0
+        doInvariantCheck(
+            {{"escrow specifies invalid amount"}},
+            [](Account const& A1, Account const&, ApplyContext& ac) {
+                // escrow with too-little mpt
+                auto const sle = ac.view().peek(keylet::account(A1.id()));
+                if (!sle)
+                    return false;
+                auto sleNew = std::make_shared<SLE>(
+                    keylet::escrow(A1, (*sle)[sfSequence] + 2));
+
+                MPTIssue const mpt{
+                    MPTIssue{makeMptID(1, AccountID(0x4985601))}};
+                STAmount amt(mpt, -1);
+                sleNew->setFieldAmount(sfAmount, amt);
+                ac.view().insert(sleNew);
+                return true;
+            });
+
+        // MPT OutstandingAmount < 0
+        doInvariantCheck(
+            {{"escrow specifies invalid amount"}},
+            [](Account const& A1, Account const&, ApplyContext& ac) {
+                // mpissuance outstanding is negative
+                auto const sle = ac.view().peek(keylet::account(A1.id()));
+                if (!sle)
+                    return false;
+
+                MPTIssue const mpt{
+                    MPTIssue{makeMptID(1, AccountID(0x4985601))}};
+                auto sleNew =
+                    std::make_shared<SLE>(keylet::mptIssuance(mpt.getMptID()));
+                sleNew->setFieldU64(sfOutstandingAmount, -1);
+                ac.view().insert(sleNew);
+                return true;
+            });
+
+        // MPT LockedAmount < 0
+        doInvariantCheck(
+            {{"escrow specifies invalid amount"}},
+            [](Account const& A1, Account const&, ApplyContext& ac) {
+                // mpissuance locked is less than locked
+                auto const sle = ac.view().peek(keylet::account(A1.id()));
+                if (!sle)
+                    return false;
+
+                MPTIssue const mpt{
+                    MPTIssue{makeMptID(1, AccountID(0x4985601))}};
+                auto sleNew =
+                    std::make_shared<SLE>(keylet::mptIssuance(mpt.getMptID()));
+                sleNew->setFieldU64(sfLockedAmount, -1);
+                ac.view().insert(sleNew);
+                return true;
+            });
+
+        // MPT OutstandingAmount < LockedAmount
+        doInvariantCheck(
+            {{"escrow specifies invalid amount"}},
+            [](Account const& A1, Account const&, ApplyContext& ac) {
+                // mpissuance outstanding is less than locked
+                auto const sle = ac.view().peek(keylet::account(A1.id()));
+                if (!sle)
+                    return false;
+
+                MPTIssue const mpt{
+                    MPTIssue{makeMptID(1, AccountID(0x4985601))}};
+                auto sleNew =
+                    std::make_shared<SLE>(keylet::mptIssuance(mpt.getMptID()));
+                sleNew->setFieldU64(sfOutstandingAmount, 1);
+                sleNew->setFieldU64(sfLockedAmount, 10);
+                ac.view().insert(sleNew);
+                return true;
+            });
+
+        // MPT MPTAmount < 0
+        doInvariantCheck(
+            {{"escrow specifies invalid amount"}},
+            [](Account const& A1, Account const&, ApplyContext& ac) {
+                // mptoken amount is negative
+                auto const sle = ac.view().peek(keylet::account(A1.id()));
+                if (!sle)
+                    return false;
+
+                MPTIssue const mpt{
+                    MPTIssue{makeMptID(1, AccountID(0x4985601))}};
+                auto sleNew =
+                    std::make_shared<SLE>(keylet::mptoken(mpt.getMptID(), A1));
+                sleNew->setFieldU64(sfMPTAmount, -1);
+                ac.view().insert(sleNew);
+                return true;
+            });
+
+        // MPT LockedAmount < 0
+        doInvariantCheck(
+            {{"escrow specifies invalid amount"}},
+            [](Account const& A1, Account const&, ApplyContext& ac) {
+                // mptoken locked amount is negative
+                auto const sle = ac.view().peek(keylet::account(A1.id()));
+                if (!sle)
+                    return false;
+
+                MPTIssue const mpt{
+                    MPTIssue{makeMptID(1, AccountID(0x4985601))}};
+                auto sleNew =
+                    std::make_shared<SLE>(keylet::mptoken(mpt.getMptID(), A1));
+                sleNew->setFieldU64(sfLockedAmount, -1);
+                ac.view().insert(sleNew);
+                return true;
+            });
     }
 
     void

src/test/rpc/Simulate_test.cpp

@@ -720,7 +720,11 @@ class Simulate_test : public beast::unit_test::suite
                                       Json::Value const& tx) {
                 auto result = resp[jss::result];
                 checkBasicReturnValidity(
-                    result, tx, env.seq(alice), env.current()->fees().base * 2);
+                    result,
+                    tx,
+                    env.seq(alice),
+                    tx.isMember(jss::Signers) ? env.current()->fees().base * 2
+                                              : env.current()->fees().base);
 
                 BEAST_EXPECT(result[jss::engine_result] == "tesSUCCESS");
                 BEAST_EXPECT(result[jss::engine_result_code] == 0);
@@ -762,6 +766,10 @@ class Simulate_test : public beast::unit_test::suite
             tx[jss::Account] = alice.human();
             tx[jss::TransactionType] = jss::AccountSet;
             tx[sfDomain] = newDomain;
+
+            // test with autofill
+            testTx(env, tx, validateOutput, false);
+
             tx[sfSigners] = Json::arrayValue;
             {
                 Json::Value signer;
@@ -771,7 +779,7 @@ class Simulate_test : public beast::unit_test::suite
                 tx[sfSigners].append(signerOuter);
             }
 
-            // test with autofill
+            // test with just signer accounts
             testTx(env, tx, validateOutput, false);
 
             tx[sfSigningPubKey] = "";
@@ -780,8 +788,7 @@ class Simulate_test : public beast::unit_test::suite
             // transaction requires a non-base fee
             tx[sfFee] =
                 (env.current()->fees().base * 2).jsonClipped().asString();
-            tx[sfSigners][0u][sfSigner][jss::SigningPubKey] =
-                strHex(becky.pk().slice());
+            tx[sfSigners][0u][sfSigner][jss::SigningPubKey] = "";
             tx[sfSigners][0u][sfSigner][jss::TxnSignature] = "";
 
             // test without autofill
@@ -830,11 +837,12 @@ class Simulate_test : public beast::unit_test::suite
             tx[jss::Account] = env.master.human();
             tx[jss::TransactionType] = jss::AccountSet;
             tx[sfDomain] = newDomain;
+            // master key is disabled, so this is invalid
+            tx[jss::SigningPubKey] = strHex(env.master.pk().slice());
 
             // test with autofill
             testTx(env, tx, testSimulation);
 
-            tx[sfSigningPubKey] = "";
             tx[sfTxnSignature] = "";
             tx[sfSequence] = env.seq(env.master);
             tx[sfFee] = env.current()->fees().base.jsonClipped().asString();
@@ -844,6 +852,79 @@ class Simulate_test : public beast::unit_test::suite
         }
     }
 
+    void
+    testInvalidSingleAndMultiSigningTransaction()
+    {
+        testcase(
+            "Transaction with both single-signing SigningPubKey and "
+            "multi-signing Signers");
+
+        using namespace jtx;
+        Env env(*this);
+        static auto const newDomain = "123ABC";
+        Account const alice("alice");
+        Account const becky("becky");
+        Account const carol("carol");
+        env.fund(XRP(10000), alice);
+        env.close();
+
+        // set up valid multisign
+        env(signers(alice, 1, {{becky, 1}, {carol, 1}}));
+        env.close();
+
+        {
+            std::function<void(Json::Value const&, Json::Value const&)> const&
+                testSimulation = [&](Json::Value const& resp,
+                                     Json::Value const& tx) {
+                    auto result = resp[jss::result];
+                    checkBasicReturnValidity(
+                        result,
+                        tx,
+                        env.seq(env.master),
+                        env.current()->fees().base * 2);
+
+                    BEAST_EXPECT(result[jss::engine_result] == "temINVALID");
+                    BEAST_EXPECT(result[jss::engine_result_code] == -277);
+                    BEAST_EXPECT(
+                        result[jss::engine_result_message] ==
+                        "The transaction is ill-formed.");
+
+                    BEAST_EXPECT(
+                        !result.isMember(jss::meta) &&
+                        !result.isMember(jss::meta_blob));
+                };
+
+            Json::Value tx;
+
+            tx[jss::Account] = env.master.human();
+            tx[jss::TransactionType] = jss::AccountSet;
+            tx[sfDomain] = newDomain;
+            // master key is disabled, so this is invalid
+            tx[jss::SigningPubKey] = strHex(env.master.pk().slice());
+            tx[sfSigners] = Json::arrayValue;
+            {
+                Json::Value signer;
+                signer[jss::Account] = becky.human();
+                Json::Value signerOuter;
+                signerOuter[sfSigner] = signer;
+                tx[sfSigners].append(signerOuter);
+            }
+
+            // test with autofill
+            testTx(env, tx, testSimulation, false);
+
+            tx[sfTxnSignature] = "";
+            tx[sfSequence] = env.seq(env.master);
+            tx[sfFee] = env.current()->fees().base.jsonClipped().asString();
+            tx[sfSigners][0u][sfSigner][jss::SigningPubKey] =
+                strHex(becky.pk().slice());
+            tx[sfSigners][0u][sfSigner][jss::TxnSignature] = "";
+
+            // test without autofill
+            testTx(env, tx, testSimulation);
+        }
+    }
+
     void
     testMultisignedBadPubKey()
     {
@@ -1117,6 +1198,7 @@ public:
         testTransactionTecFailure();
         testSuccessfulTransactionMultisigned();
         testTransactionSigningFailure();
+        testInvalidSingleAndMultiSigningTransaction();
         testMultisignedBadPubKey();
         testDeleteExpiredCredentials();
         testSuccessfulTransactionNetworkID();

src/xrpld/app/ledger/detail/InboundLedger.cpp

@@ -502,15 +502,17 @@ InboundLedger::trigger(std::shared_ptr<Peer> const& peer, TriggerReason reason)
 
     if (auto stream = journal_.debug())
     {
-        stream << "Trigger acquiring ledger " << hash_;
+        std::stringstream ss;
+        ss << "Trigger acquiring ledger " << hash_;
         if (peer)
-            stream << " from " << peer;
+            ss << " from " << peer;
 
         if (complete_ || failed_)
-            stream << "complete=" << complete_ << " failed=" << failed_;
+            ss << " complete=" << complete_ << " failed=" << failed_;
         else
-            stream << "header=" << mHaveHeader << " tx=" << mHaveTransactions
-                   << " as=" << mHaveState;
+            ss << " header=" << mHaveHeader << " tx=" << mHaveTransactions
+               << " as=" << mHaveState;
+        stream << ss.str();
     }
 
     if (!mHaveHeader)

src/xrpld/app/ledger/detail/InboundLedgers.cpp

@@ -384,17 +384,10 @@ public:
     {
         auto const start = m_clock.now();
 
-        // Make a list of things to sweep, while holding the lock
-        std::vector<MapType::mapped_type> stuffToSweep;
-        std::size_t total;
-
         {
             ScopedLockType sl(mLock);
             MapType::iterator it(mLedgers.begin());
-            total = mLedgers.size();
-
-            stuffToSweep.reserve(total);
-
+            
             while (it != mLedgers.end())
             {
                 auto const la = it->second->getLastAction();
@@ -404,11 +397,8 @@ public:
                     it->second->touch();
                     ++it;
                 }
-                else if ((la + std::chrono::minutes(1)) < start)
+                else if ((la + std::chrono::seconds(10)) < start)
                 {
-                    stuffToSweep.push_back(it->second);
-                    // shouldn't cause the actual final delete
-                    // since we are holding a reference in the vector.
                     it = mLedgers.erase(it);
                 }
                 else
@@ -419,14 +409,6 @@ public:
 
             beast::expire(mRecentFailures, kReacquireInterval);
         }
-
-        JLOG(j_.debug())
-            << "Swept " << stuffToSweep.size() << " out of " << total
-            << " inbound ledgers. Duration: "
-            << std::chrono::duration_cast<std::chrono::milliseconds>(
-                   m_clock.now() - start)
-                   .count()
-            << "ms";
     }
 
     void

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

@@ -595,7 +595,7 @@ EscrowCreate::doApply()
                 },
                 amount.asset().value());
             !isTesSuccess(ret))
-            return ret;
+            return ret;  // LCOV_EXCL_LINE
     }
 
     // increment owner count
@@ -766,26 +766,26 @@ EscrowFinish::preclaim(PreclaimContext const& ctx)
             return err;
     }
 
-    auto const k = keylet::escrow(ctx.tx[sfOwner], ctx.tx[sfOfferSequence]);
-    auto const slep = ctx.view.read(k);
-    if (!slep)
-        return tecNO_TARGET;
-
-    AccountID const dest = (*slep)[sfDestination];
-    STAmount const amount = (*slep)[sfAmount];
-
-    if (!isXRP(amount))
+    if (ctx.view.rules().enabled(featureTokenEscrow))
     {
-        if (!ctx.view.rules().enabled(featureTokenEscrow))
-            return temDISABLED;  // LCOV_EXCL_LINE
+        auto const k = keylet::escrow(ctx.tx[sfOwner], ctx.tx[sfOfferSequence]);
+        auto const slep = ctx.view.read(k);
+        if (!slep)
+            return tecNO_TARGET;
 
-        if (auto const ret = std::visit(
-                [&]<typename T>(T const&) {
-                    return escrowFinishPreclaimHelper<T>(ctx, dest, amount);
-                },
-                amount.asset().value());
-            !isTesSuccess(ret))
-            return ret;
+        AccountID const dest = (*slep)[sfDestination];
+        STAmount const amount = (*slep)[sfAmount];
+
+        if (!isXRP(amount))
+        {
+            if (auto const ret = std::visit(
+                    [&]<typename T>(T const&) {
+                        return escrowFinishPreclaimHelper<T>(ctx, dest, amount);
+                    },
+                    amount.asset().value());
+                !isTesSuccess(ret))
+                return ret;
+        }
     }
     return tesSUCCESS;
 }
@@ -1015,7 +1015,12 @@ EscrowFinish::doApply()
     auto const k = keylet::escrow(ctx_.tx[sfOwner], ctx_.tx[sfOfferSequence]);
     auto const slep = ctx_.view().peek(k);
     if (!slep)
+    {
+        if (ctx_.view().rules().enabled(featureTokenEscrow))
+            return tecINTERNAL;  // LCOV_EXCL_LINE
+
         return tecNO_TARGET;
+    }
 
     // If a cancel time is present, a finish operation should only succeed prior
     // to that time. fix1571 corrects a logic error in the check that would make
@@ -1245,7 +1250,7 @@ escrowCancelPreclaimHelper<MPTIssue>(
         keylet::mptIssuance(amount.get<MPTIssue>().getMptID());
     auto const sleIssuance = ctx.view.read(issuanceKey);
     if (!sleIssuance)
-        return tecOBJECT_NOT_FOUND;  // LCOV_EXCL_LINE
+        return tecOBJECT_NOT_FOUND;
 
     // If the issuer has requireAuth set, check if the account is
     // authorized
@@ -1261,26 +1266,27 @@ escrowCancelPreclaimHelper<MPTIssue>(
 TER
 EscrowCancel::preclaim(PreclaimContext const& ctx)
 {
-    auto const k = keylet::escrow(ctx.tx[sfOwner], ctx.tx[sfOfferSequence]);
-    auto const slep = ctx.view.read(k);
-    if (!slep)
-        return tecNO_TARGET;
-
-    AccountID const account = (*slep)[sfAccount];
-    STAmount const amount = (*slep)[sfAmount];
-
-    if (!isXRP(amount))
+    if (ctx.view.rules().enabled(featureTokenEscrow))
     {
-        if (!ctx.view.rules().enabled(featureTokenEscrow))
-            return temDISABLED;  // LCOV_EXCL_LINE
+        auto const k = keylet::escrow(ctx.tx[sfOwner], ctx.tx[sfOfferSequence]);
+        auto const slep = ctx.view.read(k);
+        if (!slep)
+            return tecNO_TARGET;
 
-        if (auto const ret = std::visit(
-                [&]<typename T>(T const&) {
-                    return escrowCancelPreclaimHelper<T>(ctx, account, amount);
-                },
-                amount.asset().value());
-            !isTesSuccess(ret))
-            return ret;
+        AccountID const account = (*slep)[sfAccount];
+        STAmount const amount = (*slep)[sfAmount];
+
+        if (!isXRP(amount))
+        {
+            if (auto const ret = std::visit(
+                    [&]<typename T>(T const&) {
+                        return escrowCancelPreclaimHelper<T>(
+                            ctx, account, amount);
+                    },
+                    amount.asset().value());
+                !isTesSuccess(ret))
+                return ret;
+        }
     }
     return tesSUCCESS;
 }
@@ -1291,7 +1297,12 @@ EscrowCancel::doApply()
     auto const k = keylet::escrow(ctx_.tx[sfOwner], ctx_.tx[sfOfferSequence]);
     auto const slep = ctx_.view().peek(k);
     if (!slep)
+    {
+        if (ctx_.view().rules().enabled(featureTokenEscrow))
+            return tecINTERNAL;  // LCOV_EXCL_LINE
+
         return tecNO_TARGET;
+    }
 
     if (ctx_.view().rules().enabled(fix1571))
     {

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

@@ -271,26 +271,6 @@ NoZeroEscrow::visitEntry(
     std::shared_ptr<SLE const> const& after)
 {
     auto isBad = [](STAmount const& amount) {
-        // IOU case
-        if (amount.holds<Issue>())
-        {
-            if (amount <= beast::zero)
-                return true;
-
-            if (badCurrency() == amount.getCurrency())
-                return true;
-        }
-
-        // MPT case
-        if (amount.holds<MPTIssue>())
-        {
-            if (amount <= beast::zero)
-                return true;
-
-            if (amount.mpt() > MPTAmount{maxMPTokenAmount})
-                return true;
-        }
-
         // XRP case
         if (amount.native())
         {
@@ -300,7 +280,28 @@ NoZeroEscrow::visitEntry(
             if (amount.xrp() >= INITIAL_XRP)
                 return true;
         }
+        else
+        {
+            // IOU case
+            if (amount.holds<Issue>())
+            {
+                if (amount <= beast::zero)
+                    return true;
 
+                if (badCurrency() == amount.getCurrency())
+                    return true;
+            }
+
+            // MPT case
+            if (amount.holds<MPTIssue>())
+            {
+                if (amount <= beast::zero)
+                    return true;
+
+                if (amount.mpt() > MPTAmount{maxMPTokenAmount})
+                    return true;  // LCOV_EXCL_LINE
+            }
+        }
         return false;
     };
 

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

@@ -59,7 +59,7 @@ MPTokenIssuanceDestroy::preclaim(PreclaimContext const& ctx)
         return tecHAS_OBLIGATIONS;
 
     if ((*sleMPT)[~sfLockedAmount].value_or(0) != 0)
-        return tecHAS_OBLIGATIONS;
+        return tecHAS_OBLIGATIONS;  // LCOV_EXCL_LINE
 
     return tesSUCCESS;
 }

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

@@ -184,6 +184,12 @@ preflight2(PreflightContext const& ctx)
                 return temINVALID;  // LCOV_EXCL_LINE
             }
         }
+
+        if (!ctx.tx.getSigningPubKey().empty())
+        {
+            // trying to single-sign _and_ multi-sign a transaction
+            return temINVALID;
+        }
         return tesSUCCESS;
     }
 
@@ -297,9 +303,9 @@ Transactor::checkFee(PreclaimContext const& ctx, XRPAmount baseFee)
 
     if (balance < feePaid)
     {
-        JLOG(ctx.j.trace()) << "Insufficient balance:"
-                            << " balance=" << to_string(balance)
-                            << " paid=" << to_string(feePaid);
+        JLOG(ctx.j.trace())
+            << "Insufficient balance:" << " balance=" << to_string(balance)
+            << " paid=" << to_string(feePaid);
 
         if ((balance > beast::zero) && !ctx.view.open())
         {
@@ -571,13 +577,13 @@ Transactor::apply()
 NotTEC
 Transactor::checkSign(PreclaimContext const& ctx)
 {
+    auto const pkSigner = ctx.tx.getSigningPubKey();
     // Ignore signature check on batch inner transactions
     if (ctx.tx.isFlag(tfInnerBatchTxn) &&
         ctx.view.rules().enabled(featureBatch))
     {
         // Defensive Check: These values are also checked in Batch::preflight
-        if (ctx.tx.isFieldPresent(sfTxnSignature) ||
-            !ctx.tx.getSigningPubKey().empty() ||
+        if (ctx.tx.isFieldPresent(sfTxnSignature) || !pkSigner.empty() ||
             ctx.tx.isFieldPresent(sfSigners))
         {
             return temINVALID_FLAG;  // LCOV_EXCL_LINE
@@ -585,25 +591,30 @@ Transactor::checkSign(PreclaimContext const& ctx)
         return tesSUCCESS;
     }
 
+    if ((ctx.flags & tapDRY_RUN) && pkSigner.empty() &&
+        !ctx.tx.isFieldPresent(sfSigners))
+    {
+        // simulate: skip signature validation when neither SigningPubKey nor
+        // Signers are provided
+        return tesSUCCESS;
+    }
+
     auto const idAccount = ctx.tx[~sfDelegate].value_or(ctx.tx[sfAccount]);
 
     // If the pk is empty and not simulate or simulate and signers,
     // then we must be multi-signing.
-    if ((ctx.flags & tapDRY_RUN && ctx.tx.isFieldPresent(sfSigners)) ||
-        (!(ctx.flags & tapDRY_RUN) && ctx.tx.getSigningPubKey().empty()))
+    if (ctx.tx.isFieldPresent(sfSigners))
     {
         STArray const& txSigners(ctx.tx.getFieldArray(sfSigners));
         return checkMultiSign(ctx.view, idAccount, txSigners, ctx.flags, ctx.j);
     }
 
     // Check Single Sign
-    auto const pkSigner = ctx.tx.getSigningPubKey();
-    // This ternary is only needed to handle `simulate`
     XRPL_ASSERT(
-        (ctx.flags & tapDRY_RUN) || !pkSigner.empty(),
+        !pkSigner.empty(),
         "ripple::Transactor::checkSingleSign : non-empty signer or simulation");
 
-    if (!(ctx.flags & tapDRY_RUN) && !publicKeyType(makeSlice(pkSigner)))
+    if (!publicKeyType(makeSlice(pkSigner)))
     {
         JLOG(ctx.j.trace())
             << "checkSingleSign: signing public key type is unknown";
@@ -798,14 +809,15 @@ Transactor::checkMultiSign(
         // public key.
         auto const spk = txSigner.getFieldVL(sfSigningPubKey);
 
-        if (!(flags & tapDRY_RUN) && !publicKeyType(makeSlice(spk)))
+        // spk being non-empty in non-simulate is checked in
+        // STTx::checkMultiSign
+        if (!spk.empty() && !publicKeyType(makeSlice(spk)))
         {
             JLOG(j.trace())
                 << "checkMultiSign: signing public key type is unknown";
             return tefBAD_SIGNATURE;
         }
 
-        // This ternary is only needed to handle `simulate`
         XRPL_ASSERT(
             (flags & tapDRY_RUN) || !spk.empty(),
             "ripple::Transactor::checkMultiSign : non-empty signer or "

src/xrpld/ledger/detail/View.cpp

@@ -2747,18 +2747,18 @@ rippleLockEscrowMPT(
     auto const mptID = keylet::mptIssuance(mptIssue.getMptID());
     auto sleIssuance = view.peek(mptID);
     if (!sleIssuance)
-    {
+    {  // LCOV_EXCL_START
         JLOG(j.error()) << "rippleLockEscrowMPT: MPT issuance not found for "
                         << mptIssue.getMptID();
-        return tecOBJECT_NOT_FOUND;  // LCOV_EXCL_LINE
-    }
+        return tecOBJECT_NOT_FOUND;
+    }  // LCOV_EXCL_STOP
 
     if (amount.getIssuer() == sender)
-    {
+    {  // LCOV_EXCL_START
         JLOG(j.error())
             << "rippleLockEscrowMPT: sender is the issuer, cannot lock MPTs.";
-        return tecINTERNAL;  // LCOV_EXCL_LINE
-    }
+        return tecINTERNAL;
+    }  // LCOV_EXCL_STOP
 
     // 1. Decrease the MPT Holder MPTAmount
     // 2. Increase the MPT Holder EscrowedAmount
@@ -2766,23 +2766,23 @@ rippleLockEscrowMPT(
         auto const mptokenID = keylet::mptoken(mptID.key, sender);
         auto sle = view.peek(mptokenID);
         if (!sle)
-        {
+        {  // LCOV_EXCL_START
             JLOG(j.error())
                 << "rippleLockEscrowMPT: MPToken not found for " << sender;
-            return tecOBJECT_NOT_FOUND;  // LCOV_EXCL_LINE
-        }
+            return tecOBJECT_NOT_FOUND;
+        }  // LCOV_EXCL_STOP
 
         auto const amt = sle->getFieldU64(sfMPTAmount);
         auto const pay = amount.mpt().value();
 
         // Underflow check for subtraction
         if (!canSubtract(STAmount(mptIssue, amt), STAmount(mptIssue, pay)))
-        {
+        {  // LCOV_EXCL_START
             JLOG(j.error())
                 << "rippleLockEscrowMPT: insufficient MPTAmount for "
                 << to_string(sender) << ": " << amt << " < " << pay;
-            return tecINTERNAL;  // LCOV_EXCL_LINE
-        }
+            return tecINTERNAL;
+        }  // LCOV_EXCL_STOP
 
         (*sle)[sfMPTAmount] = amt - pay;
 
@@ -2790,12 +2790,12 @@ rippleLockEscrowMPT(
         uint64_t const locked = (*sle)[~sfLockedAmount].value_or(0);
 
         if (!canAdd(STAmount(mptIssue, locked), STAmount(mptIssue, pay)))
-        {
+        {  // LCOV_EXCL_START
             JLOG(j.error())
                 << "rippleLockEscrowMPT: overflow on locked amount for "
                 << to_string(sender) << ": " << locked << " + " << pay;
-            return tecINTERNAL;  // LCOV_EXCL_LINE
-        }
+            return tecINTERNAL;
+        }  // LCOV_EXCL_STOP
 
         if (sle->isFieldPresent(sfLockedAmount))
             (*sle)[sfLockedAmount] += pay;
@@ -2815,13 +2815,13 @@ rippleLockEscrowMPT(
         // Overflow check for addition
         if (!canAdd(
                 STAmount(mptIssue, issuanceEscrowed), STAmount(mptIssue, pay)))
-        {
+        {  // LCOV_EXCL_START
             JLOG(j.error()) << "rippleLockEscrowMPT: overflow on issuance "
                                "locked amount for "
                             << mptIssue.getMptID() << ": " << issuanceEscrowed
                             << " + " << pay;
-            return tecINTERNAL;  // LCOV_EXCL_LINE
-        }
+            return tecINTERNAL;
+        }  // LCOV_EXCL_STOP
 
         if (sleIssuance->isFieldPresent(sfLockedAmount))
             (*sleIssuance)[sfLockedAmount] += pay;
@@ -2846,21 +2846,21 @@ rippleUnlockEscrowMPT(
     auto const mptID = keylet::mptIssuance(mptIssue.getMptID());
     auto sleIssuance = view.peek(mptID);
     if (!sleIssuance)
-    {
+    {  // LCOV_EXCL_START
         JLOG(j.error()) << "rippleUnlockEscrowMPT: MPT issuance not found for "
                         << mptIssue.getMptID();
-        return tecOBJECT_NOT_FOUND;  // LCOV_EXCL_LINE
-    }
+        return tecOBJECT_NOT_FOUND;
+    }  // LCOV_EXCL_STOP
 
     // Decrease the Issuance EscrowedAmount
     {
         if (!sleIssuance->isFieldPresent(sfLockedAmount))
-        {
+        {  // LCOV_EXCL_START
             JLOG(j.error())
                 << "rippleUnlockEscrowMPT: no locked amount in issuance for "
                 << mptIssue.getMptID();
-            return tecINTERNAL;  // LCOV_EXCL_LINE
-        }
+            return tecINTERNAL;
+        }  // LCOV_EXCL_STOP
 
         auto const locked = sleIssuance->getFieldU64(sfLockedAmount);
         auto const redeem = amount.mpt().value();
@@ -2868,12 +2868,12 @@ rippleUnlockEscrowMPT(
         // Underflow check for subtraction
         if (!canSubtract(
                 STAmount(mptIssue, locked), STAmount(mptIssue, redeem)))
-        {
+        {  // LCOV_EXCL_START
             JLOG(j.error())
                 << "rippleUnlockEscrowMPT: insufficient locked amount for "
                 << mptIssue.getMptID() << ": " << locked << " < " << redeem;
-            return tecINTERNAL;  // LCOV_EXCL_LINE
-        }
+            return tecINTERNAL;
+        }  // LCOV_EXCL_STOP
 
         auto const newLocked = locked - redeem;
         if (newLocked == 0)
@@ -2889,23 +2889,23 @@ rippleUnlockEscrowMPT(
         auto const mptokenID = keylet::mptoken(mptID.key, receiver);
         auto sle = view.peek(mptokenID);
         if (!sle)
-        {
+        {  // LCOV_EXCL_START
             JLOG(j.error())
                 << "rippleUnlockEscrowMPT: MPToken not found for " << receiver;
             return tecOBJECT_NOT_FOUND;  // LCOV_EXCL_LINE
-        }
+        }  // LCOV_EXCL_STOP
 
         auto current = sle->getFieldU64(sfMPTAmount);
         auto delta = amount.mpt().value();
 
         // Overflow check for addition
         if (!canAdd(STAmount(mptIssue, current), STAmount(mptIssue, delta)))
-        {
+        {  // LCOV_EXCL_START
             JLOG(j.error())
                 << "rippleUnlockEscrowMPT: overflow on MPTAmount for "
                 << to_string(receiver) << ": " << current << " + " << delta;
-            return tecINTERNAL;  // LCOV_EXCL_LINE
-        }
+            return tecINTERNAL;
+        }  // LCOV_EXCL_STOP
 
         (*sle)[sfMPTAmount] += delta;
         view.update(sle);
@@ -2919,55 +2919,56 @@ rippleUnlockEscrowMPT(
         // Underflow check for subtraction
         if (!canSubtract(
                 STAmount(mptIssue, outstanding), STAmount(mptIssue, redeem)))
-        {
+        {  // LCOV_EXCL_START
             JLOG(j.error())
                 << "rippleUnlockEscrowMPT: insufficient outstanding amount for "
                 << mptIssue.getMptID() << ": " << outstanding << " < "
                 << redeem;
-            return tecINTERNAL;  // LCOV_EXCL_LINE
-        }
+            return tecINTERNAL;
+        }  // LCOV_EXCL_STOP
 
         sleIssuance->setFieldU64(sfOutstandingAmount, outstanding - redeem);
         view.update(sleIssuance);
     }
 
     if (issuer == sender)
-    {
+    {  // LCOV_EXCL_START
         JLOG(j.error()) << "rippleUnlockEscrowMPT: sender is the issuer, "
                            "cannot unlock MPTs.";
-        return tecINTERNAL;  // LCOV_EXCL_LINE
-    }
+        return tecINTERNAL;
+    }  // LCOV_EXCL_STOP
     else
     {
         // Decrease the MPT Holder EscrowedAmount
         auto const mptokenID = keylet::mptoken(mptID.key, sender);
         auto sle = view.peek(mptokenID);
         if (!sle)
-        {
+        {  // LCOV_EXCL_START
             JLOG(j.error())
                 << "rippleUnlockEscrowMPT: MPToken not found for " << sender;
-            return tecOBJECT_NOT_FOUND;  // LCOV_EXCL_LINE
-        }
+            return tecOBJECT_NOT_FOUND;
+        }  // LCOV_EXCL_STOP
 
         if (!sle->isFieldPresent(sfLockedAmount))
-        {
+        {  // LCOV_EXCL_START
             JLOG(j.error())
                 << "rippleUnlockEscrowMPT: no locked amount in MPToken for "
                 << to_string(sender);
-            return tecINTERNAL;  // LCOV_EXCL_LINE
-        }
+            return tecINTERNAL;
+        }  // LCOV_EXCL_STOP
 
         auto const locked = sle->getFieldU64(sfLockedAmount);
         auto const delta = amount.mpt().value();
 
         // Underflow check for subtraction
+        // LCOV_EXCL_START
         if (!canSubtract(STAmount(mptIssue, locked), STAmount(mptIssue, delta)))
-        {
+        {  // LCOV_EXCL_START
             JLOG(j.error())
                 << "rippleUnlockEscrowMPT: insufficient locked amount for "
                 << to_string(sender) << ": " << locked << " < " << delta;
-            return tecINTERNAL;  // LCOV_EXCL_LINE
-        }
+            return tecINTERNAL;
+        }  // LCOV_EXCL_STOP
 
         auto const newLocked = locked - delta;
         if (newLocked == 0)