Merge #2303: [Backport] 5.1.0 backports

50a5e84 GUI: if the custom fee is disabled, use the minimum required fee and not the stored value. (furszy)
1fc145c Automatically set the lowest possible Custom Fee when user type in fee that is too low. (MishaPozdnikin)
0d2555e net_processing: missing cs_main lock for chainActive.GetLocator() call (furszy)
64bd400 validation: Remove redundant request sync from ProcessNewBlock(). (furszy)
c44b431 validation: missing cs_main lock for `CheckBlock` in `ProcessNewBlock` (furszy)
1f38b90 Removal of the `fAlreadyChecked` flag from the entire `ActivateBestChain` flow. (furszy)
f53c824 Refactor: remove redundant `fAlreadyCheckedBlock` argument from `AcceptBlock` (furszy)
374f6c9 Refactor: move `CheckBlockSignature` function call inside `CheckBlock`. (furszy)
a78dfbe Validation: Remove `CheckBlockSignature` now unneeded enableP2PKH flag. (furszy)
7f244b1 [Tests] Check last_processed_block in getwalletinfo Github-Pull: #2283 Rebased-From: a700fdf (random-zebra)
b3a12b5 [RPC] `getwalletinfo`: Add last_processed_block return value. (furszy)
e51dcee Add unit tests for signals generated by ProcessNewBlock() (furszy)
c4dd07f Validation: rename one of the two instances using "bad-prevblk" to its correct description of "prevblk-not-found" (furszy)
39f6eaf Fix concurrency-related bugs in ActivateBestChain (Jesse Cohen)
fb19c3a Do not unlock cs_main in ABC unless we've actually made progress. (Matt Corallo)
fd79bb7 Optimize ActivateBestChain for long chains (Pieter Wuille)
c2ae5ff Fix fast-shutdown crash if genesis block was not loaded (Matt Corallo)
7ab7112 Hold cs_main while calling UpdatedBlockTip() and ui.NotifyBlockTip (Jesse Cohen)
c202bc1 Update ValidationInterface() documentation to explicitly specify threading and memory model (Jesse Cohen)
f499e6e Update documentation for SingleThreadedSchedulerClient() to specify the memory model (Jesse Cohen)
9a9425f Add Unit Test for SingleThreadedSchedulerClient (Jesse Cohen)
4903f31 Removing blocksizenotify abomination. (furszy)
c865148 [validation] Do not actively wait for cs_main lock in `ActivateBestChain()` (furszy)
a582560 docs: add reduce-memory.md (fanquake)
f806584 test: move sync_blocks and sync_mempool functions to test_framework.py (random-zebra)
d902128 [Test] Fix intermittent sync_blocks failures (random-zebra)
a1d6c0c Move g_is_mempool_loaded into CTxMemPool::m_is_loaded (Ben Woosley)
584bbba rpc: Expose g_is_mempool_loaded via getmempoolinfo and /rest/mempool/info.json (Ben Woosley)
e10c1b6 Set fee to highest possible if input is too big #fixes 2234 (dnchk)
775e532 Fixes double fade-in animation when clicking the question mark next to the 'Available' label in the top bar (Volodia)
0cf4064 Make box of PIVX address return to purple when it's empty (Volodia)

Pull request description:

  Backport the following PRs to the 5.1 branch:

  * #2237
  * #2247
  * #2249
  * #2254
  * #2262
  * #2284
  * #2290

  Missing open PRs that need to get merged to move forward with v5.1.0:

  * [x] #2215
  * [x] #2283
  * [x] #2295
  * [x] #2306

  When every PR get merged, we can move forward with a new release candidate for the v5.1.0 release (rc3). Then one or two weeks of testing (depending on how many testers join the efforts) and we are ready for the production release.

  The heart of v5.1.0 has been already battle tested this past month with the v5.1.0rc2 testing phase. The only remaining point that needs a more broadly usage/testing is #2290 that it's solving a reported issue in the recent release candidate.

ACKs for top commit:
  random-zebra:
    utACK 50a5e84
  Fuzzbawls:
    ACK 50a5e84

Tree-SHA512: 1f77e1601c51f280b3de0d1de9e0b084871701c1af35ad817760ad55ec28c02e2423494b3d189273aacd367b41ba826d63beb73b0259a1c17f019d05711d54ea

doc/README.md

@@ -64,6 +64,7 @@ The PIVX repo's [root README](/README.md) contains relevant information on the d
 ### Miscellaneous
 - [Assets Attribution](assets-attribution.md)
 - [Files](files.md)
+- [Reduce Memory](reduce-memory.md)
 - [Tor Support](tor.md)
 - [Init Scripts (systemd/upstart/openrc)](init.md)
 

doc/REST-interface.md

@@ -79,6 +79,7 @@ $ curl localhost:18332/rest/getutxos/checkmempool/b2cdfd7b89def827ff8af7cd9bff76
 
 Returns various information about the TX mempool.
 Only supports JSON as output format.
+* loaded : (boolean) if the mempool is fully loaded
 * size : (numeric) the number of transactions in the TX mempool
 * bytes : (numeric) size of the TX mempool in bytes
 

doc/reduce-memory.md

@@ -0,0 +1,45 @@
+# Reduce Memory
+
+There are a few parameters that can be dialed down to reduce the memory usage of `pivxd`. This can be useful on embedded systems or small VPSes.
+
+## In-memory caches
+
+The size of some in-memory caches can be reduced. As caches trade off memory usage for performance, reducing these will usually have a negative effect on performance.
+
+- `-dbcache=<n>` - the UTXO database cache size, this defaults to `450`. The unit is MiB (1024).
+  - The minimum value for `-dbcache` is 4.
+  - A lower `-dbcache` makes initial sync time much longer. After the initial sync, the effect is less pronounced for most use-cases, unless fast validation of blocks is important, such as for mining.
+
+## Memory pool
+
+- In PIVX Core there is a memory pool limiter which can be configured with `-maxmempool=<n>`, where `<n>` is the size in MB (1000). The default value is `300`.
+  - The minimum value for `-maxmempool` is 5.
+  - A lower maximum mempool size means that transactions will be evicted sooner. This will affect any uses of `pivxd` that process unconfirmed transactions.
+
+- Unused memory allocated to the mempool (default: 300MB) is shared with the UTXO cache, so when trying to reduce memory usage you should limit the mempool, with the `-maxmempool` command line argument.
+
+## Number of peers
+
+- `-maxconnections=<n>` - the maximum number of connections, this defaults to `125`. Each active connection takes up some memory. Only significant if incoming
+   connections are enabled, otherwise the number of connections will never be more than `8`.
+
+## Thread configuration
+
+For each thread a thread stack needs to be allocated. By default on Linux,
+threads take up 8MiB for the thread stack on a 64-bit system, and 4MiB in a
+32-bit system.
+
+- `-par=<n>` - the number of script verification threads, defaults to the number of cores in the system minus one.
+- `-rpcthreads=<n>` - the number of threads used for processing RPC requests, defaults to `4`.
+
+## Linux specific
+
+By default, since glibc `2.10`, the C library will create up to two heap arenas per core. This is known to cause excessive memory usage in some scenarios. To avoid this make a script that sets `MALLOC_ARENA_MAX` before starting pivxd:
+
+```bash
+#!/usr/bin/env bash
+export MALLOC_ARENA_MAX=1
+pivxd
+```
+
+The behavior was introduced to increase CPU locality of allocated memory and performance with concurrent allocation, so this setting could in theory reduce performance. However, in PIVX Core very little parallel allocation happens, so the impact is expected to be small or absent.

src/Makefile.test.include

@@ -101,7 +101,8 @@ BITCOIN_TESTS =\
   test/univalue_tests.cpp \
   test/util_tests.cpp \
   test/sha256compress_tests.cpp \
-  test/upgrades_tests.cpp
+  test/upgrades_tests.cpp \
+  test/validation_block_tests.cpp
 
 SAPLING_TESTS =\
     test/librust/libsapling_utils_tests.cpp \

src/blockassembler.cpp

@@ -226,7 +226,7 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
         if (chainActive.Tip() != pindexPrev) return nullptr; // new block came in, move on
 
         CValidationState state;
-        if (!TestBlockValidity(state, *pblock, pindexPrev, false, false)) {
+        if (!TestBlockValidity(state, *pblock, pindexPrev, false, false, false)) {
             throw std::runtime_error(
                     strprintf("%s: TestBlockValidity failed: %s", __func__, FormatStateMessage(state)));
         }

src/blockassembler.h

@@ -66,8 +66,8 @@ class BlockAssembler
     BlockAssembler(const CChainParams& chainparams, const bool defaultPrintPriority);
     /** Construct a new block template with coinbase to scriptPubKeyIn */
     std::unique_ptr<CBlockTemplate> CreateNewBlock(const CScript& scriptPubKeyIn,
-                                   CWallet* pwallet,
-                                   bool fProofOfStake,
+                                   CWallet* pwallet = nullptr,
+                                   bool fProofOfStake = false,
                                    std::vector<CStakeableOutput>* availableCoins = nullptr);
 
 private:

src/blocksignature.cpp

@@ -40,7 +40,7 @@ bool SignBlock(CBlock& block, const CKeyStore& keystore)
     return SignBlockWithKey(block, key);
 }
 
-bool CheckBlockSignature(const CBlock& block, const bool enableP2PKH)
+bool CheckBlockSignature(const CBlock& block)
 {
     if (block.IsProofOfWork())
         return block.vchBlockSig.empty();
@@ -64,13 +64,6 @@ bool CheckBlockSignature(const CBlock& block, const bool enableP2PKH)
         if (!Solver(txout.scriptPubKey, whichType, vSolutions))
             return false;
 
-        if (!enableP2PKH) {
-            // Before v5 activation, P2PKH was always failing.
-            if (whichType == TX_PUBKEYHASH) {
-                return false;
-            }
-        }
-
         if (whichType == TX_PUBKEY) {
             valtype& vchPubKey = vSolutions[0];
             pubkey = CPubKey(vchPubKey);

src/blocksignature.h

@@ -11,6 +11,6 @@
 
 bool SignBlockWithKey(CBlock& block, const CKey& key);
 bool SignBlock(CBlock& block, const CKeyStore& keystore);
-bool CheckBlockSignature(const CBlock& block, const bool enableP2PKH);
+bool CheckBlockSignature(const CBlock& block);
 
 #endif //PIVX_BLOCKSIGNATURE_H

src/guiinterface.h

@@ -100,9 +100,6 @@ class CClientUIInterface
     /** New block has been accepted */
     boost::signals2::signal<void(bool fInitialDownload, const CBlockIndex* newTip)> NotifyBlockTip;
 
-    /** New block has been accepted and is over a certain size */
-    boost::signals2::signal<void(int size, const uint256& hash)> NotifyBlockSize;
-
     /** Banlist did change. */
     boost::signals2::signal<void (void)> BannedListChanged;
 };

src/init.cpp

@@ -248,8 +248,8 @@ void PrepareShutdown()
     DumpBudgets(g_budgetman);
     DumpMasternodePayments();
     UnregisterNodeSignals(GetNodeSignals());
-    if (g_is_mempool_loaded && gArgs.GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) {
-        DumpMempool();
+    if (::mempool.IsLoaded() && gArgs.GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) {
+        DumpMempool(::mempool);
     }
 
     if (fFeeEstimatesInitialized) {
@@ -422,7 +422,6 @@ std::string HelpMessage(HelpMessageMode mode)
     strUsage += HelpMessageOpt("-version", _("Print version and exit"));
     strUsage += HelpMessageOpt("-alertnotify=<cmd>", _("Execute command when a relevant alert is received or we see a really long fork (%s in cmd is replaced by message)"));
     strUsage += HelpMessageOpt("-blocknotify=<cmd>", _("Execute command when the best block changes (%s in cmd is replaced by block hash)"));
-    strUsage += HelpMessageOpt("-blocksizenotify=<cmd>", _("Execute command when the best block changes and its size is over (%s in cmd is replaced by block hash, %d with the block size)"));
     strUsage += HelpMessageOpt("-checkblocks=<n>", strprintf(_("How many blocks to check at startup (default: %u, 0 = all)"), DEFAULT_CHECKBLOCKS));
     strUsage += HelpMessageOpt("-conf=<file>", strprintf(_("Specify configuration file (default: %s)"), PIVX_CONF_FILENAME));
     if (mode == HMM_BITCOIND) {
@@ -622,16 +621,6 @@ static void BlockNotifyCallback(bool initialSync, const CBlockIndex *pBlockIndex
     }
 }
 
-static void BlockSizeNotifyCallback(int size, const uint256& hashNewTip)
-{
-    std::string strCmd = gArgs.GetArg("-blocksizenotify", "");
-
-    boost::replace_all(strCmd, "%s", hashNewTip.GetHex());
-    boost::replace_all(strCmd, "%d", std::to_string(size));
-    std::thread t(runCommand, strCmd);
-    t.detach(); // thread runs free
-}
-
 ////////////////////////////////////////////////////
 
 static bool fHaveGenesis = false;
@@ -728,9 +717,9 @@ void ThreadImport(const std::vector<fs::path>& vImportFiles)
     }
 
     if (gArgs.GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) {
-        LoadMempool();
+        LoadMempool(::mempool);
     }
-    g_is_mempool_loaded = !fRequestShutdown;
+    ::mempool.SetIsLoaded(!ShutdownRequested());
 }
 
 /** Sanity checks
@@ -1786,9 +1775,6 @@ bool AppInitMain()
     if (gArgs.IsArgSet("-blocknotify"))
         uiInterface.NotifyBlockTip.connect(BlockNotifyCallback);
 
-    if (gArgs.IsArgSet("-blocksizenotify"))
-        uiInterface.NotifyBlockSize.connect(BlockSizeNotifyCallback);
-
     // scan for better chains in the block chain database, that are not yet connected in the active best chain
     CValidationState state;
     if (!ActivateBestChain(state)) {

src/net_processing.cpp

@@ -1599,16 +1599,17 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR
         CInv inv(MSG_BLOCK, hashBlock);
         LogPrint(BCLog::NET, "received block %s peer=%d\n", inv.hash.ToString(), pfrom->id);
 
-        //sometimes we will be sent their most recent block and its not the one we want, in that case tell where we are
+        // sometimes we will be sent their most recent block and its not the one we want, in that case tell where we are
         if (!mapBlockIndex.count(pblock->hashPrevBlock)) {
+            CBlockLocator locator = WITH_LOCK(cs_main, return chainActive.GetLocator(););
             if (find(pfrom->vBlockRequested.begin(), pfrom->vBlockRequested.end(), hashBlock) != pfrom->vBlockRequested.end()) {
-                //we already asked for this block, so lets work backwards and ask for the previous block
-                connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::GETBLOCKS, chainActive.GetLocator(), pblock->hashPrevBlock));
-                pfrom->vBlockRequested.push_back(pblock->hashPrevBlock);
+                // we already asked for this block, so lets work backwards and ask for the previous block
+                connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::GETBLOCKS, locator, pblock->hashPrevBlock));
+                pfrom->vBlockRequested.emplace_back(pblock->hashPrevBlock);
             } else {
-                //ask to sync to this block
-                connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::GETBLOCKS, chainActive.GetLocator(), hashBlock));
-                pfrom->vBlockRequested.push_back(hashBlock);
+                // ask to sync to this block
+                connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::GETBLOCKS, locator, hashBlock));
+                pfrom->vBlockRequested.emplace_back(hashBlock);
             }
         } else {
             pfrom->AddInventoryKnown(inv);

src/primitives/block.h

@@ -94,7 +94,7 @@ class CBlock : public CBlockHeader
     std::vector<unsigned char> vchBlockSig;
 
     // memory only
-    mutable bool fChecked;
+    mutable bool fChecked{false};
 
     CBlock()
     {

src/qt/pivx/balancebubble.cpp

@@ -44,12 +44,12 @@ void BalanceBubble::showEvent(QShowEvent *event)
 {
     QGraphicsOpacityEffect *eff = new QGraphicsOpacityEffect(this);
     this->setGraphicsEffect(eff);
-    QPropertyAnimation *a = new QPropertyAnimation(eff,"opacity");
-    a->setDuration(400);
-    a->setStartValue(0.1);
-    a->setEndValue(1);
-    a->setEasingCurve(QEasingCurve::InBack);
-    a->start(QPropertyAnimation::DeleteWhenStopped);
+    QPropertyAnimation *anim = new QPropertyAnimation(eff,"opacity");
+    anim->setDuration(400);
+    anim->setStartValue(0);
+    anim->setEndValue(1);
+    anim->setEasingCurve(QEasingCurve::Linear);
+    anim->start(QPropertyAnimation::DeleteWhenStopped);
 
     if (!hideTimer) hideTimer = new QTimer(this);
     connect(hideTimer, &QTimer::timeout, this, &BalanceBubble::hideTimeout);
@@ -77,4 +77,4 @@ void BalanceBubble::hideTimeout()
 BalanceBubble::~BalanceBubble()
 {
     delete ui;
-}
+}

src/qt/pivx/sendcustomfeedialog.cpp

@@ -123,9 +123,17 @@ void SendCustomFeeDialog::accept()
     // Check insane fee
     const CAmount insaneFee = ::minRelayTxFee.GetFeePerK() * 10000;
     if (customFee >= insaneFee) {
+        ui->lineEditCustomFee->setText(BitcoinUnits::format(walletModel->getOptionsModel()->getDisplayUnit(), insaneFee - CWallet::GetRequiredFee(1000)));
         inform(tr("Fee too high. Must be below: %1").arg(
                 BitcoinUnits::formatWithUnit(walletModel->getOptionsModel()->getDisplayUnit(), insaneFee)));
     } else if (customFee < CWallet::GetRequiredFee(1000)) {
+        CAmount nFee = 0;
+        if (walletModel->hasWalletCustomFee()) {
+            walletModel->getWalletCustomFee(nFee);
+        } else {
+            nFee = CWallet::GetRequiredFee(1000);
+        }
+        ui->lineEditCustomFee->setText(BitcoinUnits::format(walletModel->getOptionsModel()->getDisplayUnit(), nFee));
         inform(tr("Fee too low. Must be at least: %1").arg(
                 BitcoinUnits::formatWithUnit(walletModel->getOptionsModel()->getDisplayUnit(), CWallet::GetRequiredFee(1000))));
     } else {

src/qt/pivx/sendmultirow.cpp

@@ -149,6 +149,10 @@ bool SendMultiRow::addressChanged(const QString& str, bool fOnlyValidate)
         updateStyle(ui->lineEditAddress);
         return valid;
     }
+
+    setCssProperty(ui->lineEditAddress, "edit-primary-multi-book");
+    updateStyle(ui->lineEditAddress);
+
     return false;
 }
 

src/rpc/blockchain.cpp

@@ -14,6 +14,7 @@
 #include "kernel.h"
 #include "masternodeman.h"
 #include "policy/feerate.h"
+#include "policy/policy.h"
 #include "rpc/server.h"
 #include "sync.h"
 #include "txdb.h"
@@ -1149,9 +1150,13 @@ UniValue getchaintips(const JSONRPCRequest& request)
 UniValue mempoolInfoToJSON()
 {
     UniValue ret(UniValue::VOBJ);
+    ret.pushKV("loaded", mempool.IsLoaded());
     ret.pushKV("size", (int64_t) mempool.size());
     ret.pushKV("bytes", (int64_t) mempool.GetTotalTxSize());
     ret.pushKV("usage", (int64_t) mempool.DynamicMemoryUsage());
+    size_t maxmempool = gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000;
+    ret.pushKV("mempoolminfee", ValueFromAmount(std::max(mempool.GetMinFee(maxmempool), ::minRelayTxFee).GetFeePerK()));
+    ret.pushKV("minrelaytxfee", ValueFromAmount(::minRelayTxFee.GetFeePerK()));
 
     return ret;
 }
@@ -1165,9 +1170,13 @@ UniValue getmempoolinfo(const JSONRPCRequest& request)
 
             "\nResult:\n"
             "{\n"
+            "  \"loaded\": true|false         (boolean) True if the mempool is fully loaded\n"
             "  \"size\": xxxxx                (numeric) Current tx count\n"
             "  \"bytes\": xxxxx               (numeric) Sum of all tx sizes\n"
             "  \"usage\": xxxxx               (numeric) Total memory usage for the mempool\n"
+            "  \"maxmempool\": xxxxx,         (numeric) Maximum memory usage for the mempool\n"
+            "  \"mempoolminfee\": xxxxx       (numeric) Minimum fee rate in " + CURRENCY_UNIT + "/kB for tx to be accepted. Is the maximum of minrelaytxfee and minimum mempool fee\n"
+            "  \"minrelaytxfee\": xxxxx       (numeric) Current minimum relay fee for transactions\n"
             "}\n"
 
             "\nExamples:\n" +

src/scheduler.h

@@ -87,9 +87,13 @@ class CScheduler
 
 /**
  * Class used by CScheduler clients which may schedule multiple jobs
- * which are required to be run serially. Does not require such jobs
- * to be executed on the same thread, but no two jobs will be executed
- * at the same time.
+ * which are required to be run serially. Jobs may not be run on the
+ * same thread, but no two jobs will be executed
+ * at the same time and memory will be release-acquire consistent
+ * (the scheduler will internally do an acquire before invoking a callback
+ * as well as a release at the end). In practice this means that a callback
+ * B() will be able to observe all of the effects of callback A() which executed
+ * before it.
  */
 class SingleThreadedSchedulerClient {
 private:
@@ -104,6 +108,13 @@ class SingleThreadedSchedulerClient {
 
 public:
     explicit SingleThreadedSchedulerClient(CScheduler *pschedulerIn) : m_pscheduler(pschedulerIn) {}
+
+    /**
+     * Add a callback to be executed. Callbacks are executed serially
+     * and memory is release-acquire consistent between callback executions.
+     * Practially, this means that callbacks can behave as if they are executed
+     * in order by a single thread.
+     */
     void AddToProcessQueue(std::function<void (void)> func);
 
     // Processes all remaining queue members on the calling thread, blocking until queue is empty

src/test/CMakeLists.txt

@@ -133,6 +133,7 @@ set(BITCOIN_TESTS
         ${CMAKE_CURRENT_SOURCE_DIR}/validation_tests.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/sha256compress_tests.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/upgrades_tests.cpp
+        ${CMAKE_CURRENT_SOURCE_DIR}/validation_block_tests.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/librust/sapling_rpc_wallet_tests.cpp
         ${CMAKE_SOURCE_DIR}/src/wallet/test/wallet_tests.cpp
         ${CMAKE_SOURCE_DIR}/src/wallet/test/crypto_tests.cpp