Merge pull request #136 from DeFiCh/account-mining-solution

Prevent transactions in blocks from going below balance.

src/masternodes/accountshistory.cpp

@@ -6,6 +6,8 @@
 #include <masternodes/accounts.h>
 #include <key_io.h>
 
+#include <limits>
+
 /// @attention make sure that it does not overlap with those in masternodes.cpp/tokens.cpp/undos.cpp/accounts.cpp !!!
 const unsigned char CAccountsHistoryView::ByAccountHistoryKey::prefix = 'h'; // don't intersects with CMintedHeadersView::MintedHeaders::prefix due to different DB
 
@@ -32,7 +34,8 @@ Res CAccountsHistoryView::SetAccountHistory(const CScript & owner, uint32_t heig
 }
 
 bool CAccountsHistoryView::TrackAffectedAccounts(CStorageKV const & before, MapKV const & diff, uint32_t height, uint32_t txn, const uint256 & txid, unsigned char category) {
-    if (!gArgs.GetBoolArg("-acindex", false))
+    // txn set to max if called from CreateNewBlock to check account balances, do not track here.
+    if (!gArgs.GetBoolArg("-acindex", false) || txn == std::numeric_limits<uint32_t>::max())
         return false;
 
     std::map<CScript, TAmounts> balancesDiff;

src/masternodes/mn_checks.cpp

@@ -113,7 +113,7 @@ bool HasFoundationAuth(CTransaction const & tx, CCoinsViewCache const & coins, C
     return false;
 }
 
-Res ApplyCustomTx(CCustomCSView & base_mnview, CCoinsViewCache const & coins, CTransaction const & tx, Consensus::Params const & consensusParams, uint32_t height, uint32_t txn, bool isCheck)
+Res ApplyCustomTx(CCustomCSView & base_mnview, CCoinsViewCache const & coins, CTransaction const & tx, Consensus::Params const & consensusParams, uint32_t height, uint32_t txn, bool isCheck, bool skipAuth)
 {
     Res res = Res::Ok();
 
@@ -145,7 +145,7 @@ Res ApplyCustomTx(CCustomCSView & base_mnview, CCoinsViewCache const & coins, CT
                 res = ApplyUpdateTokenAnyTx(mnview, coins, tx, height, metadata, consensusParams);
                 break;
             case CustomTxType::MintToken:
-                res = ApplyMintTokenTx(mnview, coins, tx, height, metadata, consensusParams);
+                res = ApplyMintTokenTx(mnview, coins, tx, height, metadata, consensusParams, skipAuth);
                 break;
             case CustomTxType::CreatePoolPair:
                 res = ApplyCreatePoolPairTx(mnview, coins, tx, height, metadata, consensusParams);
@@ -166,7 +166,7 @@ Res ApplyCustomTx(CCustomCSView & base_mnview, CCoinsViewCache const & coins, CT
                 res = ApplyUtxosToAccountTx(mnview, tx, height, metadata, consensusParams);
                 break;
             case CustomTxType::AccountToUtxos:
-                res = ApplyAccountToUtxosTx(mnview, coins, tx, height, metadata, consensusParams);
+                res = ApplyAccountToUtxosTx(mnview, coins, tx, height, metadata, consensusParams, skipAuth);
                 break;
             case CustomTxType::AccountToAccount:
                 res = ApplyAccountToAccountTx(mnview, coins, tx, height, metadata, consensusParams);
@@ -423,7 +423,7 @@ Res ApplyUpdateTokenAnyTx(CCustomCSView & mnview, CCoinsViewCache const & coins,
     return Res::Ok(base);
 }
 
-Res ApplyMintTokenTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams)
+Res ApplyMintTokenTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams, bool skipAuth)
 {
     if((int)height < consensusParams.AMKHeight) { return Res::Err("Token tx before AMK height (block %d)", consensusParams.AMKHeight); }
 
@@ -473,7 +473,7 @@ Res ApplyMintTokenTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTra
                 return Res::Err("%s: token not mintable!", tokenId.ToString());
             }
 
-            if (!HasAuth(tx, coins, auth.out.scriptPubKey)) { // in the case of DAT, it's ok to do not check foundation auth cause exact DAT owner is foundation member himself
+            if (!skipAuth && !HasAuth(tx, coins, auth.out.scriptPubKey)) { // in the case of DAT, it's ok to do not check foundation auth cause exact DAT owner is foundation member himself
                 if (!tokenImpl.IsDAT())
                     return Res::Err("%s: %s", base, "tx must have at least one input from token owner");
 
@@ -696,7 +696,7 @@ Res ApplyUtxosToAccountTx(CCustomCSView & mnview, CTransaction const & tx, uint3
     return Res::Ok(base);
 }
 
-Res ApplyAccountToUtxosTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams)
+Res ApplyAccountToUtxosTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams, bool skipAuth)
 {
     if((int)height < consensusParams.AMKHeight) { return Res::Err("Token tx before AMK height (block %d)", consensusParams.AMKHeight); }
 
@@ -710,7 +710,7 @@ Res ApplyAccountToUtxosTx(CCustomCSView & mnview, CCoinsViewCache const & coins,
     const auto base = strprintf("Transfer AccountToUtxos: %s", msg.ToString());
 
     // check auth
-    if (!HasAuth(tx, coins, msg.from)) {
+    if (!skipAuth && !HasAuth(tx, coins, msg.from)) {
         return Res::Err("%s: %s", base, "tx must have at least one input from account owner");
     }
     // check that all tokens are minted, and no excess tokens are minted

src/masternodes/mn_checks.h

@@ -113,15 +113,15 @@ bool HasAuth(CTransaction const & tx, CKeyID const & auth);
 bool HasAuth(CTransaction const & tx, CCoinsViewCache const & coins, CScript const & auth);
 bool HasCollateralAuth(CTransaction const & tx, CCoinsViewCache const & coins, uint256 const & collateralTx);
 
-Res ApplyCustomTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, const Consensus::Params& consensusParams, uint32_t height, uint32_t txn, bool isCheck = true);
+Res ApplyCustomTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, const Consensus::Params& consensusParams, uint32_t height, uint32_t txn, bool isCheck = true, bool skipAuth = false);
 //! Deep check (and write)
 Res ApplyCreateMasternodeTx(CCustomCSView & mnview, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata);
 Res ApplyResignMasternodeTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata);
 
 Res ApplyCreateTokenTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams);
 Res ApplyUpdateTokenTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams);
 Res ApplyUpdateTokenAnyTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams);
-Res ApplyMintTokenTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams);
+Res ApplyMintTokenTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams, bool skipAuth = false);
 
 Res ApplyCreatePoolPairTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams);
 Res ApplyUpdatePoolPairTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams);
@@ -130,7 +130,7 @@ Res ApplyAddPoolLiquidityTx(CCustomCSView & mnview, CCoinsViewCache const & coin
 Res ApplyRemovePoolLiquidityTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams);
 
 Res ApplyUtxosToAccountTx(CCustomCSView & mnview, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams);
-Res ApplyAccountToUtxosTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams);
+Res ApplyAccountToUtxosTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams, bool skipAuth = false);
 Res ApplyAccountToAccountTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams);
 Res ApplyAnyAccountsToAccountsTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams);
 

src/miner.cpp

@@ -226,7 +226,7 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
 
     int nPackagesSelected = 0;
     int nDescendantsUpdated = 0;
-    addPackageTxs(nPackagesSelected, nDescendantsUpdated);
+    addPackageTxs(nPackagesSelected, nDescendantsUpdated, nHeight);
 
     int64_t nTime1 = GetTimeMicros();
 
@@ -421,7 +421,7 @@ void BlockAssembler::SortForBlock(const CTxMemPool::setEntries& package, std::ve
 // Each time through the loop, we compare the best transaction in
 // mapModifiedTxs with the next transaction in the mempool to decide what
 // transaction package to work on next.
-void BlockAssembler::addPackageTxs(int &nPackagesSelected, int &nDescendantsUpdated)
+void BlockAssembler::addPackageTxs(int &nPackagesSelected, int &nDescendantsUpdated, int nHeight)
 {
     // mapModifiedTx will store sorted packages after they are modified
     // because some of their txs are already in the block
@@ -442,6 +442,12 @@ void BlockAssembler::addPackageTxs(int &nPackagesSelected, int &nDescendantsUpda
     const int64_t MAX_CONSECUTIVE_FAILURES = 1000;
     int64_t nConsecutiveFailed = 0;
 
+    // Custom TXs to undo at the end
+    std::set<uint256> checkedTX;
+
+    // Copy of the view
+    CCustomCSView view(*pcustomcsview);
+
     while (mi != mempool.mapTx.get<ancestor_score>().end() || !mapModifiedTx.empty())
     {
         // First try to find a new transaction in mapTx to evaluate.
@@ -492,7 +498,7 @@ void BlockAssembler::addPackageTxs(int &nPackagesSelected, int &nDescendantsUpda
 
         if (packageFees < blockMinFeeRate.GetFee(packageSize)) {
             // Everything else we might consider has a lower fee rate
-            return;
+            break;
         }
 
         if (!TestPackage(packageSize, packageSigOpsCost)) {
@@ -538,6 +544,48 @@ void BlockAssembler::addPackageTxs(int &nPackagesSelected, int &nDescendantsUpda
         std::vector<CTxMemPool::txiter> sortedEntries;
         SortForBlock(ancestors, sortedEntries);
 
+        // Account check
+        bool customTxPassed{true};
+
+        // Apply and check custom TXs in order
+        for (size_t i = 0; i < sortedEntries.size(); ++i) {
+            const CTransaction& tx = sortedEntries[i]->GetTx();
+
+            // Do not double check already checked custom TX. This will be an ancestor of current TX.
+            if (checkedTX.find(tx.GetHash()) != checkedTX.end()) {
+                continue;
+            }
+
+            std::vector<unsigned char> metadata;
+            CustomTxType txType = GuessCustomTxType(tx, metadata);
+
+            // Only check custom TXs
+            if (txType != CustomTxType::None) {
+                auto res = ApplyCustomTx(view, ::ChainstateActive().CoinsTip(), tx, chainparams.GetConsensus(), nHeight, std::numeric_limits<uint32_t>::max(), false, true);
+
+                // Not okay invalidate, undo and skip
+                if (!res.ok && NotAllowedToFail(txType)) {
+                    customTxPassed = false;
+
+                    LogPrintf("%s: Failed %s TX %s: %s\n", __func__, ToString(txType), tx.GetHash().GetHex(), res.msg);
+
+                    break;
+                }
+
+                // Track checked TXs to avoid double applying
+                checkedTX.insert(tx.GetHash());
+            }
+        }
+
+        // Failed, let's move on!
+        if (!customTxPassed) {
+            if (fUsingModified) {
+                mapModifiedTx.get<ancestor_score>().erase(modit);
+                failedTx.insert(iter);
+            }
+            continue;
+        }
+
         for (size_t i=0; i<sortedEntries.size(); ++i) {
             AddToBlock(sortedEntries[i]);
             // Erase from the modified set, if present

src/miner.h

@@ -180,7 +180,7 @@ class BlockAssembler
     /** Add transactions based on feerate including unconfirmed ancestors
       * Increments nPackagesSelected / nDescendantsUpdated with corresponding
       * statistics from the package selection (for logging statistics). */
-    void addPackageTxs(int &nPackagesSelected, int &nDescendantsUpdated) EXCLUSIVE_LOCKS_REQUIRED(mempool.cs);
+    void addPackageTxs(int &nPackagesSelected, int &nDescendantsUpdated, int nHeight) EXCLUSIVE_LOCKS_REQUIRED(mempool.cs);
 
     // helper functions for addPackageTxs()
     /** Remove confirmed (inBlock) entries from given set */

src/txmempool.cpp

@@ -5,6 +5,7 @@
 
 #include <txmempool.h>
 
+#include <chainparams.h>
 #include <consensus/consensus.h>
 #include <consensus/tx_verify.h>
 #include <consensus/validation.h>
@@ -577,6 +578,49 @@ void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigne
         removeConflicts(*tx);
         ClearPrioritisation(tx->GetHash());
     }
+
+    bool accountConflict{false};
+
+    // Check if any custom TXs are in mempool with conflict
+    for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); ++it) {
+        std::vector<unsigned char> metadata;
+        CustomTxType txType = GuessCustomTxType(it->GetTx(), metadata);
+        if (NotAllowedToFail(txType)) {
+            auto res = ApplyCustomTx(*pcustomcsview, g_chainstate->CoinsTip(), it->GetTx(), Params().GetConsensus(), nBlockHeight, 0, true);
+            if (!res.ok && (res.code & CustomTxErrCodes::Fatal)) {
+                accountConflict = true;
+                break;
+            }
+        }
+    }
+
+    // Account conflict, check entire mempool
+    if (accountConflict) {
+        std::set<CTransactionRef> txsToRemove;
+        CCoinsViewCache mempoolDuplicate(const_cast<CCoinsViewCache*>(&::ChainstateActive().CoinsTip()));
+        CAmount txfee = 0;
+
+        // Check custom TX consensus types are now not in conflict with account layer
+        for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); ++it) {
+            CValidationState state;
+            if (!Consensus::CheckTxInputs(it->GetTx(), state, mempoolDuplicate, pcustomcsview.get(), nBlockHeight, txfee, Params())) {
+                LogPrintf("%s: Remove conflicting TX: %s\n", __func__, it->GetTx().GetHash().GetHex());
+                txsToRemove.insert(it->GetSharedTx());
+            }
+        }
+
+        for (auto& tx : txsToRemove) {
+            txiter it = mapTx.find(tx->GetHash());
+            if (it != mapTx.end()) {
+                setEntries stage;
+                stage.insert(it);
+                RemoveStaged(stage, true, MemPoolRemovalReason::CONFLICT);
+            }
+            removeConflicts(*tx);
+            ClearPrioritisation(tx->GetHash());
+        }
+    }
+
     lastRollingFeeUpdate = GetTime();
     blockSinceLastRollingFeeBump = true;
 }

src/validation.cpp

@@ -1835,7 +1835,6 @@ Res ApplyGeneralCoinbaseTx(CCustomCSView & mnview, CTransaction const & tx, int
         return Res::ErrDbg("bad-cb-wrong-tokens", "coinbase should pay only Defi coins");
 
     if (height >= consensus.AMKHeight) {
-        LogPrintf("ApplyGeneralCoinbaseTx() post AMK logic\n");
         // check classic UTXO foundation share:
         if (!consensus.foundationShareScript.empty() && consensus.foundationShareDFIP1 != 0) {
             CAmount foundationReward = blockReward * consensus.foundationShareDFIP1 / COIN;

test/functional/feature_account_mining.py

@@ -0,0 +1,47 @@
+#!/usr/bin/env python3
+# Copyright (c) 2014-2019 The Bitcoin Core developers
+# Copyright (c) DeFi Blockchain Developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""Test account mining behaviour"""
+
+from test_framework.test_framework import DefiTestFramework
+from test_framework.util import assert_equal
+
+class AccountMiningTest(DefiTestFramework):
+    def set_test_params(self):
+        self.num_nodes = 1
+        self.setup_clean_chain = True
+        self.extra_args = [['-txnotokens=0', '-amkheight=50']]
+
+    def run_test(self):
+        node = self.nodes[0]
+        node.generate(120)
+
+        # Get addresses and set up account
+        account = node.getnewaddress()
+        destination = node.getnewaddress()
+        node.utxostoaccount({account: "10@0"})
+        node.generate(1)
+
+        # Check we have expected balance
+        assert_equal(node.getaccount(account)[0], "10.00000000@DFI")
+
+        # Send double the amount we have in account
+        for _ in range(100):
+            node.accounttoutxos(account, {destination: "2@DFI"})
+
+        # Store block height
+        blockcount = node.getblockcount()
+
+        # Generate a block
+        node.generate(1)
+
+        # Check the blockchain has extended as expected
+        assert_equal(node.getblockcount(), blockcount + 1)
+
+        # Account should now be empty
+        assert_equal(node.getaccount(account), [])
+
+if __name__ == '__main__':
+    AccountMiningTest().main ()

test/functional/test_runner.py

@@ -229,6 +229,7 @@
     'p2p_permissions.py',
     'feature_blocksdir.py',
     'feature_config_args.py',
+    'feature_account_mining.py',
     'rpc_help.py',
     'feature_help.py',
     'feature_shutdown.py',

test/lint/lint-circular-dependencies.sh

@@ -49,6 +49,7 @@ EXPECTED_CIRCULAR_DEPENDENCIES=(
     "validation -> wallet/wallet -> validation"
     "policy/fees -> txmempool -> validation -> wallet/wallet -> policy/fees"
     "policy/fees -> txmempool -> validation -> wallet/wallet -> util/fees -> policy/fees"
+    "chainparams -> masternodes/mn_checks -> txmempool -> chainparams"
 )
 
 EXIT_CODE=0