harden simulation tests (#4423)

* Minor follow up to #4353 to harden tests
* Bump medida

Author: sisuresh

lib/libmedida

@@ -2325,12 +2325,13 @@ TEST_CASE("peer is purged from database after few failures",
     REQUIRE(!peerManager.load(localhost(cfg2.PEER_PORT)).second);
 }
 
-TEST_CASE("disconnected topology recovery")
+TEST_CASE("disconnected topology recovery", "[overlay][simulation]")
 {
+    auto initCfg = getTestConfig();
     auto cfgs = std::vector<Config>{};
     auto peers = std::vector<std::string>{};
 
-    for (int i = 0; i < 8; ++i)
+    for (int i = 0; i < 7; ++i)
     {
         auto cfg = getTestConfig(i + 1);
         cfgs.push_back(cfg);
@@ -2340,8 +2341,12 @@ TEST_CASE("disconnected topology recovery")
     auto doTest = [&](bool usePreferred) {
         auto simulation = Topologies::separate(
             7, 0.5, Simulation::OVER_LOOPBACK,
-            sha256(getTestConfig().NETWORK_PASSPHRASE), 0, [&](int i) {
-                auto cfg = cfgs[i];
+            sha256(initCfg.NETWORK_PASSPHRASE), 0, [&](int i) {
+                if (i == 0)
+                {
+                    return initCfg;
+                }
+                auto cfg = cfgs[i - 1];
                 cfg.TARGET_PEER_CONNECTIONS = 1;
                 if (usePreferred)
                 {

src/overlay/test/OverlayTests.cpp

@@ -109,10 +109,10 @@ setupStaticNetworkTopology(std::vector<Config>& configList,
     }
 
     // D->A->B->C B->E
-    simulation->addConnection(keyList[D], keyList[A]);
-    simulation->addConnection(keyList[A], keyList[B]);
-    simulation->addConnection(keyList[B], keyList[C]);
-    simulation->addConnection(keyList[B], keyList[E]);
+    simulation->addPendingConnection(keyList[D], keyList[A]);
+    simulation->addPendingConnection(keyList[A], keyList[B]);
+    simulation->addPendingConnection(keyList[B], keyList[C]);
+    simulation->addPendingConnection(keyList[B], keyList[E]);
 
     simulation->startAllNodes();
 
@@ -402,7 +402,7 @@ TEST_CASE("survey request process order", "[overlay][survey][topology]")
     {
         for (int j = i + 1; j < numberOfNodes; j++)
         {
-            simulation->addConnection(keyList[i], keyList[j]);
+            simulation->addPendingConnection(keyList[i], keyList[j]);
         }
     }
 
@@ -731,10 +731,10 @@ TEST_CASE("Time sliced dynamic topology survey", "[overlay][survey][topology]")
     }
 
     // D->A->B->C B->E (F not connected)
-    simulation->addConnection(keyList[D], keyList[A]);
-    simulation->addConnection(keyList[A], keyList[B]);
-    simulation->addConnection(keyList[B], keyList[C]);
-    simulation->addConnection(keyList[B], keyList[E]);
+    simulation->addPendingConnection(keyList[D], keyList[A]);
+    simulation->addPendingConnection(keyList[A], keyList[B]);
+    simulation->addPendingConnection(keyList[B], keyList[C]);
+    simulation->addPendingConnection(keyList[B], keyList[E]);
 
     simulation->startAllNodes();
 
@@ -785,10 +785,13 @@ TEST_CASE("Time sliced dynamic topology survey", "[overlay][survey][topology]")
     checkSurveyState(nonce, /*isReporting*/ false, {A, B, C, D, E});
 
     // Add F to the simulation and connect to B
-    simulation->addNode(configList.at(F).NODE_SEED, qSet, &configList.at(F));
-    simulation->getNode(keyList[F])->start();
+    simulation->addNode(configList.at(F).NODE_SEED, qSet, &configList.at(F))
+        ->start();
     simulation->addConnection(keyList[F], keyList[B]);
 
+    // Let survey run for a bit to establish connection
+    crankForSurvey();
+
     // Disconnect E from B
     simulation->dropConnection(keyList[B], keyList[E]);
 

src/overlay/test/SurveyManagerTests.cpp

@@ -45,12 +45,6 @@ Simulation::Simulation(Mode mode, Hash const& networkID, ConfigGen confGen,
     parallel = parallel && mVirtualClockMode == VirtualClock::REAL_TIME;
     mIdleApp = Application::create(mClock, cfg);
     mPeerMap.emplace(mIdleApp->getConfig().PEER_PORT, mIdleApp);
-
-    // mIdleApp can end up connected to some nodes in the simulation, but never
-    // starts Herder. Therefore, it needs to initialize the max tx size so that
-    // an assertion doesn't fail when it initializes flow control.
-    Herder& herder = mIdleApp->getHerder();
-    herder.setMaxTxSize(herder.getMaxClassicTxSize());
 }
 
 Simulation::~Simulation()
@@ -152,18 +146,6 @@ Simulation::addNode(SecretKey nodeKey, SCPQuorumSet qSet, Config const* cfg2,
 
     mPeerMap.emplace(app->getConfig().PEER_PORT,
                      std::weak_ptr<Application>(app));
-
-    Herder& herder = app->getHerder();
-    if (herder.getState() == Herder::HERDER_BOOTING_STATE)
-    {
-        // Application creation did not start Herder.  To protect against cases
-        // where the test using this new node run the node's OverlayManager
-        // without starting Herder, we initialize the max tx size here. This
-        // prevents an assertion failure on flow control initialization. This is
-        // harmless if the test later starts Herder as `HerderImpl::start` will
-        // overwrite this value.
-        herder.setMaxTxSize(herder.getMaxClassicTxSize());
-    }
     return app;
 }
 

src/simulation/Simulation.cpp

@@ -47,6 +47,8 @@ class Simulation
     void setCurrentVirtualTime(VirtualClock::time_point t);
     void setCurrentVirtualTime(VirtualClock::system_time_point t);
 
+    // Add new node to the simulation. This function does not start the node.
+    // Callers are expected to call `start` or `startAllNodes` manually.
     Application::pointer addNode(SecretKey nodeKey, SCPQuorumSet qSet,
                                  Config const* cfg = nullptr, bool newDB = true,
                                  uint32_t startAtLedger = 0,
@@ -55,6 +57,9 @@ class Simulation
     std::vector<Application::pointer> getNodes();
     std::vector<NodeID> getNodeIDs();
 
+    // Add a pending connection to an unstarted node. Typically called after
+    // `addNode`, but before `startAllNodes`. No-op if the simulation is already
+    // started.
     void addPendingConnection(NodeID const& initiator, NodeID const& acceptor);
     // Returns LoopbackPeerConnection given initiator, acceptor pair or nullptr
     std::shared_ptr<LoopbackPeerConnection>
@@ -80,6 +85,8 @@ class Simulation
     void crankUntil(VirtualClock::system_time_point timePoint, bool finalCrank);
     std::string metricsSummary(std::string domain = "");
 
+    // Add a real (not pending) connection to the simulation. Works even if the
+    // simulation has started.
     void addConnection(NodeID initiator, NodeID acceptor);
     void dropConnection(NodeID initiator, NodeID acceptor);
     Config newConfig(); // generates a new config