test: Shorten the ValidatorSite request timeout case (#8632)

API-CHANGELOG.md

@@ -40,7 +40,6 @@ Version 3.5.0 is not yet released.
 
 - `channel_authorize`: The `channel_id` field now returns an `invalidParams` error if the value is not a string. [#7582](https://github.com/XRPLF/rippled/pull/7582)
 - `channel_verify`: The `channel_id` and `signature` fields now return an `invalidParams` error if the value is not a string. [#7582](https://github.com/XRPLF/rippled/pull/7582)
-- `gateway_balances`: `locked` now counts only escrows of tokens the queried account issues; an escrow of another issuer's token that the account sends or receives is no longer added to its totals. [#8646](https://github.com/XRPLF/rippled/pull/8646)
 - `account_currencies`, `account_nfts`, `account_offers`: A request with more than one problem may now report a different one of them. An invalid `ledger_hash` or `ledger_index` is reported before an invalid or missing `account`. An invalid `limit` or `marker` is reported before `lgrNotFound` or `actNotFound`, and for `account_offers` also before `actMalformed`. A request with a single problem returns the same error as before. [#8561](https://github.com/XRPLF/rippled/pull/8561)
 - `deposit_authorized`: A `credentials` entry that is all zeros (for example `"0"` or 64 `0` characters) now returns a `badCredentials` error instead of looking up a credential that can never exist.
 - `account_info`, `account_lines`, `account_channels`: A request with more than one problem may now report a different one of them. An invalid `ledger_hash` or `ledger_index` is reported before a missing or invalid `account`, or `ident` for `account_info`. For `account_lines`, a `peer` that is not a string, or an invalid `limit` or `marker`, is reported before `lgrNotFound`, `actNotFound` or a malformed `account`, and an invalid `limit` or `marker` also before a malformed `peer`. For `account_channels`, a malformed `account` is reported before `lgrNotFound`, and an invalid `destination_account`, `limit` or `marker` before `lgrNotFound` or `actNotFound`. A request with a single problem returns the same error as before. [#8573](https://github.com/XRPLF/rippled/pull/8573)

src/libxrpl/tx/transactors/escrow/EscrowCreate.cpp

@@ -21,6 +21,7 @@
 #include <xrpl/protocol/Issue.h>
 #include <xrpl/protocol/Keylet.h>
 #include <xrpl/protocol/LedgerFormats.h>
+#include <xrpl/protocol/MPTAmount.h>
 #include <xrpl/protocol/MPTIssue.h>
 #include <xrpl/protocol/Protocol.h>
 #include <xrpl/protocol/Rate.h>
@@ -124,7 +125,7 @@ escrowCreatePreflightHelper<MPTIssue>(PreflightContext const& ctx)
         return temDISABLED;
 
     auto const amount = ctx.tx[sfAmount];
-    if (amount.native() || amount <= beast::kZero)
+    if (amount.native() || amount.mpt() > MPTAmount{kMaxMpTokenAmount} || amount <= beast::kZero)
         return temBAD_AMOUNT;
 
     return tesSUCCESS;

src/libxrpl/tx/transactors/token/Clawback.cpp

@@ -10,7 +10,9 @@
 #include <xrpl/protocol/Indexes.h>
 #include <xrpl/protocol/Issue.h>
 #include <xrpl/protocol/LedgerFormats.h>
+#include <xrpl/protocol/MPTAmount.h>
 #include <xrpl/protocol/MPTIssue.h>
+#include <xrpl/protocol/Protocol.h>
 #include <xrpl/protocol/SField.h>
 #include <xrpl/protocol/STAmount.h>
 #include <xrpl/protocol/STLedgerEntry.h>
@@ -65,7 +67,7 @@ preflightHelper<MPTIssue>(PreflightContext const& ctx)
     if (ctx.tx[sfAccount] == *mptHolder)
         return temMALFORMED;
 
-    if (clawAmount <= beast::kZero)
+    if (clawAmount.mpt() > MPTAmount{kMaxMpTokenAmount} || clawAmount <= beast::kZero)
         return temBAD_AMOUNT;
 
     return tesSUCCESS;

src/test/app/ValidatorSite_test.cpp

@@ -132,8 +132,21 @@ private:
         std::chrono::seconds effectiveOverlap = detail::kDefaultEffectiveOverlap;
         int expectedRefreshMin = 0;
     };
+
+    /**
+     * Serve each configured list from its own publisher server, fetch them all
+     * through one ValidatorSite and check what it reports for each site.
+     *
+     * @param good Directory the cache files are written to.
+     * @param paths One server per entry, with what to serve and what to expect.
+     * @param requestTimeout How long the ValidatorSite waits for a response
+     * before it reports that the request took too long.
+     */
     void
-    testFetchList(detail::DirGuard const& good, std::vector<FetchListConfig> const& paths)
+    testFetchList(
+        detail::DirGuard const& good,
+        std::vector<FetchListConfig> const& paths,
+        std::chrono::seconds requestTimeout = std::chrono::seconds{12})
     {
         testcase << "Fetch list - "
                  << boost::algorithm::join(
@@ -211,10 +224,10 @@ private:
 
         BEAST_EXPECT(trustedKeys.load({}, emptyCfgKeys, cfgPublishers));
 
-        // Normally, tests will only need a fraction of this time,
-        // but sometimes DNS resolution takes an inordinate amount
-        // of time, so the test will just wait.
-        auto sites = std::make_unique<ValidatorSite>(env.app(), journal, 12s);
+        // Normally, tests will only need a fraction of the default timeout,
+        // but sometimes DNS resolution takes an inordinate amount of time, so
+        // the test will just wait.
+        auto sites = std::make_unique<ValidatorSite>(env.app(), journal, requestTimeout);
 
         std::vector<std::string> uris;
         for (auto const& u : servers)
@@ -586,18 +599,19 @@ public:
                         .failApply = true,
                     },
                 });
-            // timeout
+            // timeout: the server answers 1s after the request times out
             testFetchList(
                 good,
                 {
                     {
-                        .path = "/sleep/13",
+                        .path = "/sleep/2",
                         .msg = "took too long",
                         .ssl = ssl,
                         .failFetch = true,
                         .failApply = true,
                     },
-                });
+                },
+                std::chrono::seconds{1});
             // bad manifest format using known versions
             // * Retrieves a v1 formatted list claiming version 2
             testFetchList(

src/test/rpc/GatewayBalances_test.cpp

@@ -3,7 +3,6 @@
 #include <test/jtx/WSClient.h>
 #include <test/jtx/amount.h>
 #include <test/jtx/escrow.h>
-#include <test/jtx/flags.h>
 #include <test/jtx/mpt.h>
 #include <test/jtx/pay.h>
 #include <test/jtx/trust.h>
@@ -321,58 +320,6 @@ public:
         expect(jv[jss::status] == "success");
     }
 
-    void
-    testGWBEscrowOtherIssuer()
-    {
-        testcase("Gateway Balances with escrows of another issuer's token");
-        using namespace std::chrono_literals;
-        using namespace jtx;
-
-        Env env(*this, testableAmendments());
-        Account const gw{"gw"};
-        Account const issuer{"issuer"};
-        Account const alice{"alice"};
-        Account const bob{"bob"};
-        env.fund(XRP(10000), gw, issuer, alice, bob);
-        env.close();
-        env(fset(gw, asfAllowTrustLineLocking));
-        env(fset(issuer, asfAllowTrustLineLocking));
-        env.close();
-        env(trust(alice, gw["USD"](10000)));
-        env(trust(gw, issuer["USD"](10000)));
-        env(trust(bob, issuer["USD"](10000)));
-        env.close();
-        env(pay(gw, alice, gw["USD"](1000)));
-        env(pay(issuer, gw, issuer["USD"](1000)));
-        env(pay(issuer, bob, issuer["USD"](1000)));
-        env.close();
-
-        auto wsc = makeWSClient(env.app().config());
-        auto const gatewayBalances = [&] {
-            json::Value qry;
-            qry[jss::account] = gw.human();
-            return wsc->invoke("gateway_balances", qry)[jss::result];
-        };
-
-        // gw escrows another issuer's USD: not part of gw's locked total.
-        env(escrow::create(gw, bob, issuer["USD"](500)), escrow::kFinishTime(env.now() + 10s));
-        env.close();
-        auto result = gatewayBalances();
-        BEAST_EXPECT(result[jss::obligations]["USD"] == "1000");
-        BEAST_EXPECT(!result.isMember(jss::locked));
-
-        // An escrow of another issuer's USD to gw is not counted either.
-        env(escrow::create(bob, gw, issuer["USD"](200)), escrow::kFinishTime(env.now() + 10s));
-        env.close();
-        BEAST_EXPECT(!gatewayBalances().isMember(jss::locked));
-
-        // An escrow of gw's own USD is counted.
-        env(escrow::create(alice, bob, gw["USD"](100)), escrow::kFinishTime(env.now() + 10s));
-        env.close();
-        result = gatewayBalances();
-        BEAST_EXPECT(result[jss::locked]["USD"] == "100");
-    }
-
     void
     run() override
     {
@@ -385,7 +332,6 @@ public:
             testGWBInvalidAccount(feature);
         }
         testGWBWithMPT();
-        testGWBEscrowOtherIssuer();
         testGWBOverflow();
     }
 };

src/xrpld/rpc/handlers/account/GatewayBalances.cpp

@@ -154,9 +154,8 @@ doGatewayBalances(rpc::JsonContext& context)
             if (sle->getType() == ltESCROW)
             {
                 auto const& escrow = sle->getFieldAmount(sfAmount);
-                // Gateway Balance should not include MPTs, nor escrows of
-                // tokens another account issues.
-                if (escrow.holds<MPTIssue>() || escrow.getIssuer() != accountID)
+                // Gateway Balance should not include MPTs
+                if (escrow.holds<MPTIssue>())
                     return;
 
                 auto& bal = locked[escrow.get<Issue>().currency];