diff --git a/src/test/app/HashRouter_test.cpp b/src/test/app/HashRouter_test.cpp index a6609a225a3..691c3d534a3 100644 --- a/src/test/app/HashRouter_test.cpp +++ b/src/test/app/HashRouter_test.cpp @@ -38,6 +38,7 @@ class HashRouter_test : public beast::unit_test::suite void testNonExpiration() { + testcase("Non-expiration"); using namespace std::chrono_literals; TestStopwatch stopwatch; HashRouter router(getSetup(2s, 1s), stopwatch); @@ -75,6 +76,7 @@ class HashRouter_test : public beast::unit_test::suite void testExpiration() { + testcase("Expiration"); using namespace std::chrono_literals; TestStopwatch stopwatch; HashRouter router(getSetup(2s, 1s), stopwatch); @@ -152,6 +154,7 @@ class HashRouter_test : public beast::unit_test::suite void testSuppression() { + testcase("Suppression"); // Normal HashRouter using namespace std::chrono_literals; TestStopwatch stopwatch; @@ -181,6 +184,7 @@ class HashRouter_test : public beast::unit_test::suite void testSetFlags() { + testcase("Set Flags"); using namespace std::chrono_literals; TestStopwatch stopwatch; HashRouter router(getSetup(2s, 1s), stopwatch); @@ -194,6 +198,7 @@ class HashRouter_test : public beast::unit_test::suite void testRelay() { + testcase("Relay"); using namespace std::chrono_literals; TestStopwatch stopwatch; HashRouter router(getSetup(50s, 1s), stopwatch); @@ -237,6 +242,7 @@ class HashRouter_test : public beast::unit_test::suite void testProcess() { + testcase("Process"); using namespace std::chrono_literals; TestStopwatch stopwatch; HashRouter router(getSetup(5s, 1s), stopwatch); @@ -251,6 +257,111 @@ class HashRouter_test : public beast::unit_test::suite BEAST_EXPECT(router.shouldProcess(key, peer, flags, 1s)); } + void + testSetup() + { + testcase("setup_HashRouter"); + + using namespace std::chrono_literals; + { + Config cfg; + // default + auto const setup = setup_HashRouter(cfg); + BEAST_EXPECT(setup.holdTime == 300s); + BEAST_EXPECT(setup.relayTime == 30s); + } + { + Config cfg; + // non-default + auto& h = cfg.section("hashrouter"); + h.set("hold_time", "600"); + h.set("relay_time", "15"); + auto const setup = setup_HashRouter(cfg); + BEAST_EXPECT(setup.holdTime == 600s); + BEAST_EXPECT(setup.relayTime == 15s); + } + { + Config cfg; + // equal + auto& h = cfg.section("hashrouter"); + h.set("hold_time", "400"); + h.set("relay_time", "400"); + auto const setup = setup_HashRouter(cfg); + BEAST_EXPECT(setup.holdTime == 400s); + BEAST_EXPECT(setup.relayTime == 400s); + } + { + Config cfg; + // wrong order + auto& h = cfg.section("hashrouter"); + h.set("hold_time", "60"); + h.set("relay_time", "120"); + try + { + auto const setup = setup_HashRouter(cfg); + fail(); + } + catch (std::exception const& e) + { + std::string expected = + "HashRouter relay time must be less than or equal to hold " + "time"; + BEAST_EXPECT(e.what() == expected); + } + } + { + Config cfg; + // too small hold + auto& h = cfg.section("hashrouter"); + h.set("hold_time", "10"); + h.set("relay_time", "120"); + try + { + auto const setup = setup_HashRouter(cfg); + fail(); + } + catch (std::exception const& e) + { + std::string expected = + "HashRouter hold time must be at least 12 seconds (the " + "approximate validation time for three " + "ledgers)."; + BEAST_EXPECT(e.what() == expected); + } + } + { + Config cfg; + // too small relay + auto& h = cfg.section("hashrouter"); + h.set("hold_time", "500"); + h.set("relay_time", "6"); + try + { + auto const setup = setup_HashRouter(cfg); + fail(); + } + catch (std::exception const& e) + { + std::string expected = + "HashRouter relay time must be at least 8 seconds (the " + "approximate validation time for two ledgers)."; + BEAST_EXPECT(e.what() == expected); + } + } + { + Config cfg; + // garbage + auto& h = cfg.section("hashrouter"); + h.set("hold_time", "alice"); + h.set("relay_time", "bob"); + auto const setup = setup_HashRouter(cfg); + // The set function ignores values that don't covert, so the + // defaults are left unchanged + BEAST_EXPECT(setup.holdTime == 300s); + BEAST_EXPECT(setup.relayTime == 30s); + } + } + public: void run() override @@ -261,6 +372,7 @@ class HashRouter_test : public beast::unit_test::suite testSetFlags(); testRelay(); testProcess(); + testSetup(); } }; diff --git a/src/xrpld/app/main/Tuning.h b/src/xrpld/app/main/Tuning.h index 2eb25c9c62d..c6ca81643e3 100644 --- a/src/xrpld/app/main/Tuning.h +++ b/src/xrpld/app/main/Tuning.h @@ -20,6 +20,8 @@ #ifndef RIPPLE_APP_MAIN_TUNING_H_INCLUDED #define RIPPLE_APP_MAIN_TUNING_H_INCLUDED +#include + namespace ripple { constexpr std::size_t fullBelowTargetSize = 524288; diff --git a/src/xrpld/app/misc/HashRouter.cpp b/src/xrpld/app/misc/HashRouter.cpp index 5e2b9c4162a..8aea90d2061 100644 --- a/src/xrpld/app/misc/HashRouter.cpp +++ b/src/xrpld/app/misc/HashRouter.cpp @@ -142,7 +142,7 @@ setup_HashRouter(Config const& config) if (set(tmp, "hold_time", section)) { - if (tmp < 8) + if (tmp < 12) Throw( "HashRouter hold time must be at least 12 seconds (the " "approximate validation time for three ledgers)."); diff --git a/src/xrpld/app/misc/HashRouter.h b/src/xrpld/app/misc/HashRouter.h index 37c7b30cf78..a13bcb9f8fb 100644 --- a/src/xrpld/app/misc/HashRouter.h +++ b/src/xrpld/app/misc/HashRouter.h @@ -61,8 +61,11 @@ class HashRouter /** Structure used to customize @ref HashRouter behavior. * - * Even though these items are configurable, don't change them unless there - * is a good reason, and network-wide coordination to do it. + * Even though these items are configurable, they are undocumented. Don't + * change them unless there is a good reason, and network-wide coordination + * to do it. + * + * Configuration is processed in setup_HashRouter. */ struct Setup { @@ -132,9 +135,9 @@ class HashRouter bool shouldRelay( Stopwatch::time_point const& now, - std::chrono::seconds holdTime) + std::chrono::seconds relayTime) { - if (relayed_ && *relayed_ + holdTime > now) + if (relayed_ && *relayed_ + relayTime > now) return false; relayed_.emplace(now); return true; diff --git a/src/xrpld/app/misc/NetworkOPs.cpp b/src/xrpld/app/misc/NetworkOPs.cpp index d238fac5a65..77dd213750e 100644 --- a/src/xrpld/app/misc/NetworkOPs.cpp +++ b/src/xrpld/app/misc/NetworkOPs.cpp @@ -1418,7 +1418,9 @@ NetworkOPsImp::processTransactionSet(CanonicalTXSet const& set) } doTransactionSyncBatch(lock, [&](std::unique_lock const&) { - assert(lock.owns_lock()); + XRPL_ASSERT( + lock.owns_lock(), + "ripple::NetworkOPsImp::processTransactionSet has lock"); return std::any_of( mTransactions.begin(), mTransactions.end(), [](auto const& t) { return t.transaction->getApplying();