Refactor -txordering to remove multiple GetArg calls (#1761)

* Refactor txordering to use global variables * Fix build * Switch arg processing order * Fix lint * Fix build, deprecate blocktimeordering
DeFiCh · Feb 16, 2023 · 380dceb · 380dceb
1 parent 2d68dc4
commit 380dceb
Show file tree

Hide file tree

Showing 7 changed files with 18 additions and 15 deletions.
diff --git a/src/init.cpp b/src/init.cpp
@@ -512,8 +512,8 @@ void SetupServerArgs()
     gArgs.AddArg("-regtest-minttoken-simulate-mainnet", "Simulate mainnet for minttokens on regtest -  default behavior on regtest is to allow anyone to mint mintable tokens for ease of testing", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::OPTIONS);
     gArgs.AddArg("-simulatemainnet", "Configure the regtest network to mainnet target timespan and spacing ", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::OPTIONS);
     gArgs.AddArg("-dexstats", strprintf("Enable storing live dex data in DB (default: %u)", DEFAULT_DEXSTATS), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
-    gArgs.AddArg("-blocktimeordering", strprintf("Whether to order transactions by time, otherwise ordered by fee (default: %u)", DEFAULT_FEE_ORDERING), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
-    gArgs.AddArg("-txordering", strprintf("Whether to order transactions by entry time, fee or both randomly (0: mixed, 1: fee based, 2: entry time) (default: %u)", DEFAULT_AUTO_FEE_ORDERING), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
+    gArgs.AddArg("-blocktimeordering", strprintf("(Deprecated) Whether to order transactions by time, otherwise ordered by fee (default: %u)", false), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
+    gArgs.AddArg("-txordering", strprintf("Whether to order transactions by entry time, fee or both randomly (0: mixed, 1: fee based, 2: entry time) (default: %u)", DEFAULT_TX_ORDERING), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
 #ifdef USE_UPNP
 #if USE_UPNP
     gArgs.AddArg("-upnp", "Use UPnP to map the listening port (default: 1 when listening and no -proxy)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
@@ -962,6 +962,11 @@ void InitParameterInteraction()
             LogPrintf("%s: parameter interaction: -masternode_operator -> setting -leveldbchecksum='true'\n", __func__);
         }
     }
+
+    txOrdering = static_cast<TxOrderings>(gArgs.GetArg("-txordering", DEFAULT_TX_ORDERING));
+
+    if (gArgs.GetBoolArg("-blocktimeordering", false))
+        txOrdering = TxOrderings::ENTRYTIME_ORDERING;
 }
 
 /**

diff --git a/src/miner.cpp b/src/miner.cpp
@@ -218,9 +218,7 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
         UpdateTime(pblock, consensus, pindexPrev); // update time before tx packaging
     }
 
-    const auto txOrdering = gArgs.GetArg("-txordering", DEFAULT_AUTO_FEE_ORDERING);
-    bool timeOrdering = gArgs.GetBoolArg("-blocktimeordering", DEFAULT_FEE_ORDERING);
-
+    bool timeOrdering{false};
     if (txOrdering == MIXED_ORDERING) {
         std::random_device rd;
         std::mt19937_64 gen(rd());
@@ -232,9 +230,9 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
             timeOrdering = true;
     } else if (txOrdering == ENTRYTIME_ORDERING) {
         timeOrdering = true;
-    } else if (gArgs.IsArgSet("-txordering") && txOrdering == FEE_ORDERING) {
+    } else if (txOrdering == FEE_ORDERING) {
         timeOrdering = false;
-    } // falls back to blocktimeordering arg if txordering is not set
+    }
 
     if (timeOrdering) {
         addPackageTxs<entry_time>(nPackagesSelected, nDescendantsUpdated, nHeight, mnview);

diff --git a/src/miner.h b/src/miner.h
@@ -30,6 +30,8 @@ static const bool DEFAULT_GENERATE = false;
 
 static const bool DEFAULT_PRINTPRIORITY = false;
 
+extern TxOrderings txOrdering;
+
 struct CBlockTemplate
 {
     CBlock block;

diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp
@@ -9,6 +9,7 @@
 #include <primitives/transaction.h>
 #include <streams.h>
 #include <txmempool.h>
+#include <miner.h>
 #include <util/system.h>
 #include <validation.h>
 
@@ -789,9 +790,7 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, FeeCalculation
     LOCK(m_cs_fee_estimator);
 
     // If block ordering by time is enabled return 0 to let fallback or discard fee be used.
-    if (gArgs.GetBoolArg("-blocktimeordering", DEFAULT_FEE_ORDERING) ||
-        gArgs.GetArg("-txordering", DEFAULT_AUTO_FEE_ORDERING) == MIXED_ORDERING ||
-        gArgs.GetArg("-txordering", DEFAULT_AUTO_FEE_ORDERING) == ENTRYTIME_ORDERING) {
+    if (txOrdering == MIXED_ORDERING || txOrdering == ENTRYTIME_ORDERING) {
         return 0;
     }
 

diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
@@ -36,6 +36,7 @@
 #include <memory>
 #include <stdint.h>
 
+TxOrderings txOrdering;
 /**
  * Return average network hashes per second based on the last 'lookup' blocks,
  * or from the last difficulty change if 'lookup' is nonpositive.
@@ -979,9 +980,7 @@ static UniValue estimatesmartfee(const JSONRPCRequest& request)
     CFeeRate feeRate = ::feeEstimator.estimateSmartFee(conf_target, &feeCalc, conservative);
     if (feeRate != CFeeRate(0)) {
         result.pushKV("feerate", ValueFromAmount(feeRate.GetFeePerK()));
-    } else if (gArgs.GetBoolArg("-blocktimeordering", DEFAULT_FEE_ORDERING) ||
-               gArgs.GetArg("-txordering", DEFAULT_AUTO_FEE_ORDERING) == MIXED_ORDERING ||
-               gArgs.GetArg("-txordering", DEFAULT_AUTO_FEE_ORDERING) == ENTRYTIME_ORDERING) {
+    } else if (txOrdering == MIXED_ORDERING || txOrdering == ENTRYTIME_ORDERING) {
         result.pushKV("feerate", ValueFromAmount(DEFAULT_TRANSACTION_MINFEE));
     } else {
         errors.push_back("Insufficient data or no feerate found");

diff --git a/src/validation.h b/src/validation.h
@@ -138,8 +138,7 @@ static const bool DEFAULT_DEXSTATS = false;
 /** Default for tracking amount negated by negative interest in attributes */
 static const bool DEFAULT_NEGATIVE_INTEREST = false;
 /** Default for using TX fee ordering in blocks */
-static const bool DEFAULT_FEE_ORDERING = false;
-static const TxOrderings DEFAULT_AUTO_FEE_ORDERING = FEE_ORDERING;
+static const TxOrderings DEFAULT_TX_ORDERING = FEE_ORDERING;
 
 /** Maximum number of headers to announce when relaying blocks with headers message.*/
 static const unsigned int MAX_BLOCKS_TO_ANNOUNCE = 8;

diff --git a/test/lint/lint-circular-dependencies.sh b/test/lint/lint-circular-dependencies.sh
@@ -63,6 +63,7 @@ EXPECTED_CIRCULAR_DEPENDENCIES=(
     "masternodes/mn_checks -> validation -> masternodes/mn_checks"
     "masternodes/mn_checks -> validation -> wallet/wallet -> masternodes/mn_checks"
     "masternodes/validation -> validation -> masternodes/validation"
+    "miner -> wallet/wallet -> policy/fees -> miner"
     "net_processing -> validation -> net_processing"
     "policy/fees -> txmempool -> policy/fees"
     "policy/fees -> validation -> policy/fees"