Fail loadgen when pending accounts are found in available accounts li…

…st (#4372)

# Description

Although `mAccountsAvailable` shouldn't contain pending accounts, it is
possible when the network is overloaded. Consider the following
scenario:
1. A node generates a transaction `t` using account `a` and broadcasts
it on. In doing so, loadgen marks `a` as in use, removing it from
`mAccountsAvailable.
2. For whatever reason, `t` never makes it out of the queue and the node
bans it.
3. After some period of time, the node unbans `t` because bans only last
for so many ledgers.
4. Loadgen marks `a` available, moving it back into
`mAccountsAvailable`.
5. The node hears about `t` again on the network and (as it is no longer
banned) adds it back to the queue
6. getNextAvailableAccount draws `a` from `mAccountsAvailable`. However,
`a` is no longer available as `t` is in the transaction queue!

In this scenario, returning `a` results in an assertion failure later.
This change instead detects this and marks the loadgen run as failed.

# Checklist
- [x] Reviewed the
[contributing](https://github.com/stellar/stellar-core/blob/master/CONTRIBUTING.md#submitting-changes)
document
- [x] Rebased on top of master (no merge commits)
- [x] Ran `clang-format` v8.0.0 (via `make format` or the Visual Studio
extension)
- [x] Compiles
- [x] Ran all tests
- [ ] If change impacts performance, include supporting evidence per the
[performance
document](https://github.com/stellar/stellar-core/blob/master/performance-eval/performance-eval.md)

src/simulation/LoadGenerator.cpp

@@ -684,7 +684,7 @@ LoadGenerator::generateLoad(GeneratedLoadConfig cfg)
                 return;
             }
 
-            uint64_t sourceAccountId = getNextAvailableAccount();
+            uint64_t sourceAccountId = getNextAvailableAccount(ledgerNum);
 
             std::function<
                 std::pair<LoadGenerator::TestAccountPtr, TransactionFramePtr>()>
@@ -904,17 +904,45 @@ LoadGenerator::submitTx(GeneratedLoadConfig const& cfg,
 }
 
 uint64_t
-LoadGenerator::getNextAvailableAccount()
-{
-    releaseAssert(!mAccountsAvailable.empty());
+LoadGenerator::getNextAvailableAccount(uint32_t ledgerNum)
+{
+    uint64_t sourceAccountId;
+    do
+    {
+        releaseAssert(!mAccountsAvailable.empty());
+
+        auto sourceAccountIdx =
+            rand_uniform<uint64_t>(0, mAccountsAvailable.size() - 1);
+        auto it = mAccountsAvailable.begin();
+        std::advance(it, sourceAccountIdx);
+        sourceAccountId = *it;
+        mAccountsAvailable.erase(it);
+        releaseAssert(mAccountsInUse.insert(sourceAccountId).second);
+
+        // Although mAccountsAvailable shouldn't contain pending accounts, it is
+        // possible when the network is overloaded. Consider the following
+        // scenario:
+        // 1. This node generates a transaction `t` using account `a` and
+        //    broadcasts it on. In doing so, loadgen marks `a` as in use,
+        //    removing it from `mAccountsAvailable.
+        // 2. For whatever reason, `t` never makes it out of the queue and this
+        //    node bans it.
+        // 3. After some period of time, this node unbans `t` because bans only
+        //    last for so many ledgers.
+        // 4. Loadgen marks `a` available, moving it back into
+        //    `mAccountsAvailable`.
+        // 5. This node hears about `t` again on the network and (as it is no
+        //    longer banned) adds it back to the queue
+        // 6. getNextAvailableAccount draws `a` from `mAccountsAvailable`.
+        //    However, `a` is no longer available as `t` is in the transaction
+        //    queue!
+        //
+        // In this scenario, returning `a` results in an assertion failure
+        // later. To resolve this, we resample a new account by simply looping
+        // here.
+    } while (mApp.getHerder().sourceAccountPending(
+        findAccount(sourceAccountId, ledgerNum)->getPublicKey()));
 
-    auto sourceAccountIdx =
-        rand_uniform<uint64_t>(0, mAccountsAvailable.size() - 1);
-    auto it = mAccountsAvailable.begin();
-    std::advance(it, sourceAccountIdx);
-    uint64_t sourceAccountId = *it;
-    mAccountsAvailable.erase(it);
-    releaseAssert(mAccountsInUse.insert(sourceAccountId).second);
     return sourceAccountId;
 }
 
@@ -990,7 +1018,7 @@ LoadGenerator::creationTransaction(uint64_t startAccount, uint64_t numItems,
 {
     TestAccountPtr sourceAcc =
         mInitialAccountsCreated
-            ? findAccount(getNextAvailableAccount(), ledgerNum)
+            ? findAccount(getNextAvailableAccount(ledgerNum), ledgerNum)
             : mRoot;
     vector<Operation> creationOps = createAccounts(
         startAccount, numItems, ledgerNum, !mInitialAccountsCreated);

src/simulation/LoadGenerator.h

@@ -306,7 +306,9 @@ class LoadGenerator
     // queue (to avoid source account collisions during tx submission)
     std::unordered_set<uint64_t> mAccountsInUse;
     std::unordered_set<uint64_t> mAccountsAvailable;
-    uint64_t getNextAvailableAccount();
+
+    // Get an account ID not currently in use.
+    uint64_t getNextAvailableAccount(uint32_t ledgerNum);
 
     // For account creation only: allocate a few accounts for creation purposes
     // (with sufficient balance to create new accounts) to avoid source account