From 70f2a87784229d53098adc0e9f144c2cc9b38fe4 Mon Sep 17 00:00:00 2001
From: Brett Boston <brett.boston@stellar.org>
Date: Thu, 8 Aug 2024 14:38:08 -0700
Subject: [PATCH] Ensure unique preimages during loadgen upgrade setup

Currently, loadgen can produce duplicate preimages during
`SOROBAN_UPGRADE_SETUP` if metrics have been cleared between
`SOROBAN_UPGRADE_SETUP` calls. This scenario causes the setup to fail.
This change fixes this by using an internal counter as part of the
preimage instead of using medida metrics.
---
 src/simulation/LoadGenerator.cpp           |  7 +---
 src/simulation/LoadGenerator.h             |  4 ++
 src/simulation/test/LoadGeneratorTests.cpp | 45 ++++++++++++++++++++++
 3 files changed, 51 insertions(+), 5 deletions(-)

diff --git a/src/simulation/LoadGenerator.cpp b/src/simulation/LoadGenerator.cpp
index 040537f9e6..9805934771 100644
--- a/src/simulation/LoadGenerator.cpp
+++ b/src/simulation/LoadGenerator.cpp
@@ -1260,11 +1260,8 @@ LoadGenerator::createContractTransaction(uint32_t ledgerNum, uint64_t accountId,
     createResources.readBytes = mContactOverheadBytes;
     createResources.writeBytes = 300;
 
-    auto salt = sha256(
-        std::to_string(mContractInstanceKeys.size()) + "run" +
-        std::to_string(mApp.getMetrics()
-                           .NewMeter({"loadgen", "run", "complete"}, "run")
-                           .count()));
+    auto salt = sha256("upgrade" +
+                       std::to_string(++mNumCreateContractTransactionCalls));
     auto contractIDPreimage = makeContractIDPreimage(*account, salt);
 
     auto tx = makeSorobanCreateContractTx(
diff --git a/src/simulation/LoadGenerator.h b/src/simulation/LoadGenerator.h
index d33315e703..9e441e5900 100644
--- a/src/simulation/LoadGenerator.h
+++ b/src/simulation/LoadGenerator.h
@@ -327,6 +327,10 @@ class LoadGenerator
     int64_t mPreLoadgenApplySorobanSuccess = 0;
     int64_t mPreLoadgenApplySorobanFailure = 0;
 
+    // Number of times `createContractTransaction` has been called. Used to
+    // ensure unique preimages for all `SOROBAN_UPGRADE_SETUP` runs.
+    uint32_t mNumCreateContractTransactionCalls = 0;
+
     bool mFailed{false};
     bool mStarted{false};
     bool mInitialAccountsCreated{false};
diff --git a/src/simulation/test/LoadGeneratorTests.cpp b/src/simulation/test/LoadGeneratorTests.cpp
index 15515a1436..03865af2f5 100644
--- a/src/simulation/test/LoadGeneratorTests.cpp
+++ b/src/simulation/test/LoadGeneratorTests.cpp
@@ -716,3 +716,48 @@ TEST_CASE("Multi-op mixed transactions are valid", "[loadgen]")
     REQUIRE(dexOps > 0);
     REQUIRE(dexOps + nonDexOps == 3 * 100);
 }
+
+TEST_CASE("Upgrade setup with metrics reset", "[loadgen]")
+{
+    // Create a simulation with two nodes
+    Simulation::pointer sim = Topologies::pair(
+        Simulation::OVER_LOOPBACK, sha256(getTestConfig().NETWORK_PASSPHRASE));
+    sim->startAllNodes();
+    sim->crankUntil([&]() { return sim->haveAllExternalized(3, 1); },
+                    2 * Herder::EXP_LEDGER_TIMESPAN_SECONDS, false);
+
+    Application::pointer app = sim->getNodes().front();
+    LoadGenerator& loadgen = app->getLoadGenerator();
+    medida::Meter& runsComplete =
+        app->getMetrics().NewMeter({"loadgen", "run", "complete"}, "run");
+    medida::Meter& runsFailed =
+        app->getMetrics().NewMeter({"loadgen", "run", "failed"}, "run");
+
+    // Add an account
+    loadgen.generateLoad(GeneratedLoadConfig::createAccountsLoad(
+        /* nAccounts */ 1, /* txRate */ 1));
+    sim->crankUntil([&]() { return runsComplete.count() == 1; },
+                    5 * Herder::EXP_LEDGER_TIMESPAN_SECONDS, false);
+
+    // Clear metrics to reset run count
+    app->clearMetrics("");
+
+    // Setup a soroban limit upgrade that must succeed
+    GeneratedLoadConfig upgradeSetupCfg =
+        GeneratedLoadConfig::createSorobanUpgradeSetupLoad();
+    upgradeSetupCfg.setMinSorobanPercentSuccess(100);
+    loadgen.generateLoad(upgradeSetupCfg);
+    sim->crankUntil([&]() { return runsComplete.count() == 1; },
+                    5 * Herder::EXP_LEDGER_TIMESPAN_SECONDS, false);
+    REQUIRE(runsFailed.count() == 0);
+
+    // Clear metrics again to reset run count
+    app->clearMetrics("");
+
+    // Setup again. This should succeed even though it's the same account with
+    // the same `runsComplete` value performing the setup
+    loadgen.generateLoad(upgradeSetupCfg);
+    sim->crankUntil([&]() { return runsComplete.count() == 1; },
+                    5 * Herder::EXP_LEDGER_TIMESPAN_SECONDS, false);
+    REQUIRE(runsFailed.count() == 0);
+}