From c857636b32a1c325f598c7e050febf6d285cc77b Mon Sep 17 00:00:00 2001 From: random-zebra Date: Mon, 8 Mar 2021 09:09:49 +0100 Subject: [PATCH 1/8] [Tests] Introduce new functional test tiertwo_deterministicmns.py - check dmn list, protx creation and new payment logic --- src/tiertwo_networksync.cpp | 6 + .../test_framework/test_framework.py | 11 +- test/functional/test_runner.py | 1 + test/functional/tiertwo_deterministicmns.py | 255 ++++++++++++++++++ 4 files changed, 270 insertions(+), 3 deletions(-) create mode 100755 test/functional/tiertwo_deterministicmns.py diff --git a/src/tiertwo_networksync.cpp b/src/tiertwo_networksync.cpp index 0183a5ec7f61..8f89402de739 100644 --- a/src/tiertwo_networksync.cpp +++ b/src/tiertwo_networksync.cpp @@ -170,6 +170,12 @@ void CMasternodeSync::RequestDataTo(CNode* pnode, const char* msg, bool forceReq void CMasternodeSync::SyncRegtest(CNode* pnode) { + // skip mn list and winners sync if legacy mn are obsolete + if (deterministicMNManager->LegacyMNObsolete() && + (RequestedMasternodeAssets == MASTERNODE_SYNC_LIST || RequestedMasternodeAssets == MASTERNODE_SYNC_MNW)) { + RequestedMasternodeAssets = MASTERNODE_SYNC_BUDGET; + } + // Initial sync, verify that the other peer answered to all of the messages successfully if (RequestedMasternodeAssets == MASTERNODE_SYNC_SPORKS) { RequestDataTo(pnode, NetMsgType::GETSPORKS, false); diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 3350ba1b83d5..2995c94b6c72 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -1193,7 +1193,7 @@ def not_found(): """ Create a ProReg tx, which has the collateral as one of its outputs """ - def protx_register_fund(self, miner, controller, dmn, collateral_addr, lock=True): + def protx_register_fund(self, miner, controller, dmn, collateral_addr, lock=True, op_rew=None): # send to the owner the collateral tx + some dust for the ProReg and fee funding_txid = miner.sendtoaddress(collateral_addr, Decimal('101')) # confirm and verify reception @@ -1201,8 +1201,13 @@ def protx_register_fund(self, miner, controller, dmn, collateral_addr, lock=True self.sync_blocks([miner, controller]) assert_greater_than(controller.getrawtransaction(funding_txid, True)["confirmations"], 0) # create and send the ProRegTx funding the collateral - dmn.proTx = controller.protx_register_fund(collateral_addr, dmn.ipport, dmn.owner, - dmn.operator, dmn.voting, dmn.payee) + if op_rew is None: + dmn.proTx = controller.protx_register_fund(collateral_addr, dmn.ipport, dmn.owner, + dmn.operator, dmn.voting, dmn.payee) + else: + dmn.proTx = controller.protx_register_fund(collateral_addr, dmn.ipport, dmn.owner, + dmn.operator, dmn.voting, dmn.payee, + op_rew["reward"], op_rew["address"]) dmn.collateral = COutPoint(int(dmn.proTx, 16), get_collateral_vout(controller.getrawtransaction(dmn.proTx, True))) if lock: diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 905a95593f9f..f4f747379c16 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -141,6 +141,7 @@ TIERTWO_SCRIPTS = [ # Longest test should go first, to favor running tests in parallel + 'tiertwo_deterministicmns.py', 'tiertwo_governance_sync_basic.py', 'tiertwo_mn_compatibility.py', 'tiertwo_masternode_activation.py', diff --git a/test/functional/tiertwo_deterministicmns.py b/test/functional/tiertwo_deterministicmns.py new file mode 100755 index 000000000000..dc91ddc64800 --- /dev/null +++ b/test/functional/tiertwo_deterministicmns.py @@ -0,0 +1,255 @@ +#!/usr/bin/env python3 +# Copyright (c) 2021 The PIVX Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +# +# Test deterministic masternodes +# + +from decimal import Decimal +from random import randrange +import time + +from test_framework.test_framework import PivxTestFramework +from test_framework.blocktools import ( + create_block, + create_coinbase, +) +from test_framework.messages import CTxOut, COIN +from test_framework.util import ( + assert_greater_than, + assert_equal, + assert_raises_rpc_error, + bytes_to_hex_str, + create_new_dmn, + hex_str_to_bytes, + spend_mn_collateral, +) + + +class DIP3Test(PivxTestFramework): + + def set_test_params(self): + # 1 miner, 1 controller, 6 remote mns + self.num_nodes = 8 + self.minerPos = 0 + self.controllerPos = 1 + self.setup_clean_chain = True + self.extra_args = [["-nuparams=v5_shield:1", "-nuparams=v6_evo:130"]] * self.num_nodes + self.extra_args[0].append("-sporkkey=932HEevBSujW2ud7RfB1YF91AFygbBRQj3de3LyaCRqNzKKgWXi") + + def add_new_dmn(self, mns, strType, op_keys=None, from_out=None, lock=True): + mns.append(self.register_new_dmn(2 + len(mns), + self.minerPos, + self.controllerPos, + strType, + outpoint=from_out, + op_addr_and_key=op_keys, + fLock=lock)) + + def check_mn_list(self, mns): + for i in range(self.num_nodes): + self.check_mn_list_on_node(i, mns) + self.log.info("Deterministic list contains %d masternodes for all peers." % len(mns)) + + def get_addr_balance(self, node, addr): + rcv = node.listreceivedbyaddress(0, False, False, addr) + return rcv[0]['amount'] if len(rcv) > 0 else 0 + + def get_last_paid_mn(self): + return next(x['proTxHash'] for x in self.nodes[0].listmasternodes() + if x['dmnstate']['lastPaidHeight'] == self.nodes[0].getblockcount()) + + def create_block(self, mn_payee_script, height, prevhash): + coinbase = create_coinbase(height) + coinbase.vout[0].nValue -= 3 * COIN + coinbase.vout.append(CTxOut(int(3 * COIN), hex_str_to_bytes(mn_payee_script))) + coinbase.rehash() + return create_block(int(prevhash, 16), coinbase, nVersion=10) + + def wait_until_mnsync_completed(self): + SYNC_FINISHED = [999] * self.num_nodes + synced = [-1] * self.num_nodes + timeout = time.time() + 120 + while synced != SYNC_FINISHED and time.time() < timeout: + synced = [node.mnsync("status")["RequestedMasternodeAssets"] + for node in self.nodes] + if synced != SYNC_FINISHED: + time.sleep(5) + if synced != SYNC_FINISHED: + raise AssertionError("Unable to complete mnsync: %s" % str(synced)) + + def run_test(self): + self.disable_mocktime() + + # Additional connections to miner and owner + for nodePos in [self.minerPos, self.controllerPos]: + self.connect_to_all(nodePos) + miner = self.nodes[self.minerPos] + controller = self.nodes[self.controllerPos] + + dummy_add = controller.getnewaddress("dummy") + + # Enforce mn payments and reject legacy mns at block 131 + self.activate_spork(0, "SPORK_8_MASTERNODE_PAYMENT_ENFORCEMENT") + assert_equal("success", self.set_spork(self.minerPos, "SPORK_21_LEGACY_MNS_MAX_HEIGHT", 130)) + time.sleep(1) + assert_equal([130] * self.num_nodes, [self.get_spork(x, "SPORK_21_LEGACY_MNS_MAX_HEIGHT") + for x in range(self.num_nodes)]) + mns = [] + + # Mine 100 blocks + self.log.info("Mining...") + miner.generate(110) + self.sync_blocks() + self.assert_equal_for_all(110, "getblockcount") + + # Test rejection before enforcement + self.log.info("Testing rejection of ProRegTx before DIP3 activation...") + assert_raises_rpc_error(-1, "Evo upgrade is not active yet", self.add_new_dmn, mns, "internal") + assert_raises_rpc_error(-1, "Evo upgrade is not active yet", self.add_new_dmn, mns, "fund") + # Can create the raw proReg + dmn = create_new_dmn(2, controller, dummy_add, None) + tx, sig = self.protx_register_ext(miner, controller, dmn, None, False, False) + # but cannot send it + assert_raises_rpc_error(-1, "Evo upgrade is not active yet", miner.protx_register_submit, tx, sig) + self.log.info("Done. Now mine blocks till enforcement...") + + # DIP3 activates at block 130. + miner.generate(130 - miner.getblockcount()) + self.sync_blocks() + self.assert_equal_for_all(130, "getblockcount") + + # -- DIP3 enforced and SPORK_21 active here -- + self.wait_until_mnsync_completed() + + # Create 3 DMNs and init the remote nodes + self.log.info("Initializing masternodes...") + self.add_new_dmn(mns, "internal") + self.add_new_dmn(mns, "external") + self.add_new_dmn(mns, "fund") + for mn in mns: + self.nodes[mn.idx].initmasternode(mn.operator_key, "", True) + time.sleep(1) + miner.generate(1) + self.sync_blocks() + + # Init the other 3 remote nodes before creating the ProReg tx + self.log.info("Initializing more masternodes...") + op_keys = [] + for i in range(3): + idx = 2 + len(mns) + i + add_and_key = [] + add_and_key.append(miner.getnewaddress("oper-%d-key" % idx)) + add_and_key.append(miner.dumpprivkey(add_and_key[0])) + self.nodes[idx].initmasternode(add_and_key[1], "", True) + op_keys.append(add_and_key) + time.sleep(1) + + # Now send the ProReg txes and check list + self.add_new_dmn(mns, "internal", op_keys[0]) + self.add_new_dmn(mns, "external", op_keys[1]) + self.add_new_dmn(mns, "fund", op_keys[2]) + miner.generate(2) + self.sync_blocks() + time.sleep(1) + self.log.info("Masternodes started.") + self.check_mn_list(mns) + + # Check status from remote nodes + assert_equal([self.nodes[idx].getmasternodestatus()['status'] for idx in range(2, self.num_nodes)], + ["Ready"] * (self.num_nodes - 2)) + self.log.info("All masternodes ready.") + + # Test collateral spending + dmn = mns.pop(randrange(len(mns))) # pop one at random + self.log.info("Spending collateral of mn with idx=%d..." % dmn.idx) + spend_txid = spend_mn_collateral(controller, dmn) + self.sync_mempools([miner, controller]) + miner.generate(1) + self.sync_blocks() + assert_greater_than(miner.getrawtransaction(spend_txid, True)["confirmations"], 0) + self.check_mn_list(mns) + + # Register dmn again, with the collateral of dmn2 + # dmn must be added again to the list, and dmn2 must be removed + dmn2 = mns.pop(randrange(len(mns))) # pop one at random + dmn_keys = [dmn.operator, dmn.operator_key] + dmn2_keys = [dmn2.operator, dmn2.operator_key] + self.log.info("Reactivating node %d reusing the collateral of node %d..." % (dmn.idx, dmn2.idx)) + mns.append(self.register_new_dmn(dmn.idx, self.minerPos, self.controllerPos, "external", + outpoint=dmn2.collateral, op_addr_and_key=dmn_keys, + fLock=False)) + miner.generate(1) + self.sync_blocks() + self.check_mn_list(mns) + + # Now try to register dmn2 again with an already-used IP + self.log.info("Trying duplicate IP...") + rand_idx = mns[randrange(len(mns))].idx + assert_raises_rpc_error(-1, "bad-protx-dup-IP-address", + self.register_new_dmn, rand_idx, self.minerPos, self.controllerPos, "fund", + op_addr_and_key=dmn2_keys) + + # Now try with duplicate operator key + self.log.info("Trying duplicate operator key...") + dmn2b = create_new_dmn(dmn2.idx, controller, dummy_add, dmn_keys) + assert_raises_rpc_error(-1, "bad-protx-dup-operator-key", + self.protx_register_fund, miner, controller, dmn2b, dummy_add, False) + + # Now try with duplicate owner key + self.log.info("Trying duplicate owner key...") + dmn2c = create_new_dmn(dmn2.idx, controller, dummy_add, dmn2_keys) + dmn2c.owner = mns[randrange(len(mns))].owner + assert_raises_rpc_error(-1, "bad-protx-dup-owner-key", + self.protx_register_fund, miner, controller, dmn2c, dummy_add, False) + + # Finally, register it properly. This time setting 10% of the reward for the operator + op_rew = {"reward": 10.00, "address": self.nodes[dmn2.idx].getnewaddress()} + self.log.info("Reactivating the node with a new registration (with operator reward)...") + dmn2c = create_new_dmn(dmn2.idx, controller, dummy_add, dmn2_keys) + self.protx_register_fund(miner, controller, dmn2c, dummy_add, True, op_rew) + mns.append(dmn2c) + time.sleep(1) + self.sync_mempools([miner, controller]) + miner.generate(6) + self.sync_blocks() + self.check_mn_list(mns) # 6 masternodes again + + # Test payments. + # Mine 12 blocks and check that each masternode has been paid exactly twice. + # Save last paid masternode. Check that it's the last paid also after the 12 blocks. + # Note: dmn2 sends (2 * 0.3 PIV) to the operator, and (2 * 2.7 PIV) to the owner + self.log.info("Testing masternode payments...") + last_paid_mn = self.get_last_paid_mn() + starting_balances = {"operator": self.get_addr_balance(self.nodes[dmn2c.idx], op_rew["address"])} + for mn in mns: + starting_balances[mn.payee] = self.get_addr_balance(controller, mn.payee) + miner.generate(12) + self.sync_blocks() + for mn in mns: + bal = self.get_addr_balance(controller, mn.payee) + expected = starting_balances[mn.payee] + (Decimal('6.0') if mn.idx != dmn2c.idx else Decimal('5.4')) + if bal != expected: + raise Exception("Invalid balance (%s != %s) for node %d" % (bal, expected, mn.idx)) + self.log.info("All masternodes paid twice.") + assert_equal(self.get_addr_balance(self.nodes[dmn2c.idx], op_rew["address"]), + starting_balances["operator"] + Decimal('0.6')) + self.log.info("Operator paid twice.") + assert_equal(last_paid_mn, self.get_last_paid_mn()) + self.log.info("Order preserved.") + + # Test invalid payment + self.wait_until_mnsync_completed() # just to be sure + self.log.info("Testing invalid masternode payment...") + mn_payee_script = miner.validateaddress(miner.getnewaddress())['scriptPubKey'] + block = self.create_block(mn_payee_script, miner.getblockcount(), miner.getbestblockhash()) + block.solve() + assert_equal(miner.submitblock(bytes_to_hex_str(block.serialize())), "bad-cb-payee") + + self.log.info("All good.") + + +if __name__ == '__main__': + DIP3Test().main() From cc132a81e94cbd930711dbbbdaa1d9f12380f2a6 Mon Sep 17 00:00:00 2001 From: random-zebra Date: Mon, 8 Mar 2021 22:05:29 +0100 Subject: [PATCH 2/8] [Wallet] Implement auto-locking of masternode collaterals --- src/evo/providertx.cpp | 17 +++++++++++++++ src/evo/providertx.h | 4 ++++ src/init.cpp | 10 +++++++++ src/wallet/wallet.cpp | 47 +++++++++++++++++++++++++++++++++++++++++- src/wallet/wallet.h | 24 +++++++++++++++++++++ 5 files changed, 101 insertions(+), 1 deletion(-) diff --git a/src/evo/providertx.cpp b/src/evo/providertx.cpp index 9a7b230af1da..a168344ecf88 100644 --- a/src/evo/providertx.cpp +++ b/src/evo/providertx.cpp @@ -246,3 +246,20 @@ void ProRegPL::ToJson(UniValue& obj) const obj.pushKV("operatorReward", (double)nOperatorReward / 100); obj.pushKV("inputsHash", inputsHash.ToString()); } + +bool GetProRegCollateral(const CTransactionRef& tx, COutPoint& outRet) +{ + if (tx == nullptr) { + return false; + } + if (!tx->IsSpecialTx() || tx->nType != CTransaction::TxType::PROREG) { + return false; + } + ProRegPL pl; + if (!GetTxPayload(*tx, pl)) { + return false; + } + outRet = pl.collateralOutpoint.hash.IsNull() ? COutPoint(tx->GetHash(), pl.collateralOutpoint.n) + : pl.collateralOutpoint; + return true; +} diff --git a/src/evo/providertx.h b/src/evo/providertx.h index a8640259992a..f5875921582d 100644 --- a/src/evo/providertx.h +++ b/src/evo/providertx.h @@ -70,4 +70,8 @@ class ProRegPL bool CheckProRegTx(const CTransaction& tx, const CBlockIndex* pindexPrev, CValidationState& state); +// If tx is a ProRegTx, return the collateral outpoint in outRet. +bool GetProRegCollateral(const CTransactionRef& tx, COutPoint& outRet); + + #endif //PIVX_PROVIDERTX_H diff --git a/src/init.cpp b/src/init.cpp index f00b2fc4d59f..91322bfe4b00 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -22,6 +22,7 @@ #include "checkpoints.h" #include "compat/sanity.h" #include "consensus/upgrades.h" +#include "evo/deterministicmns.h" #include "evo/evonotificationinterface.h" #include "fs.h" #include "httpserver.h" @@ -1860,6 +1861,7 @@ bool AppInitMain() strBudgetMode = gArgs.GetArg("-budgetvotemode", "auto"); #ifdef ENABLE_WALLET + // !TODO: remove after complete transition to DMN // use only the first wallet here. This section can be removed after transition to DMN if (gArgs.GetBoolArg("-mnconflock", DEFAULT_MNCONFLOCK) && !vpwallets.empty() && vpwallets[0]) { LOCK(vpwallets[0]->cs_wallet); @@ -1873,6 +1875,14 @@ bool AppInitMain() mne.getAlias(), mne.getTxHash(), mne.getOutputIndex()); } } + + // automatic lock for DMN + if (gArgs.GetBoolArg("-mnconflock", DEFAULT_MNCONFLOCK)) { + const auto& mnList = deterministicMNManager->GetListAtChainTip(); + for (CWallet* pwallet : vpwallets) { + pwallet->ScanMasternodeCollateralsAndLock(mnList); + } + } #endif // lite mode disables all Masternode related functionality diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 17941b463bcd..1cd7a009d8bf 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -9,6 +9,7 @@ #include "budget/budgetmanager.h" #include "coincontrol.h" +#include "evo/deterministicmns.h" #include "init.h" #include "guiinterfaceutil.h" #include "masternode.h" @@ -963,7 +964,7 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose) { LOCK(cs_wallet); CWalletDB walletdb(*dbw, "r+", fFlushOnClose); - uint256 hash = wtxIn.GetHash(); + const uint256& hash = wtxIn.GetHash(); // Inserts only if not already there, returns tx inserted or tx found std::pair::iterator, bool> ret = mapWallet.emplace(hash, wtxIn); @@ -1164,6 +1165,9 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, const CWallet } } + // If this is a ProRegTx and the wallet owns the collateral, lock the corresponding coin + LockIfMyCollateral(ptx); + bool isFromMe = IsFromMe(ptx); if (fExisted || IsMine(ptx) || isFromMe || (saplingNoteData && !saplingNoteData->empty())) { @@ -4153,6 +4157,47 @@ void CWallet::AutoCombineDust(CConnman* connman) } } +void CWallet::LockOutpointIfMine(const CTransactionRef& ptx, const COutPoint& c) +{ + AssertLockHeld(cs_wallet); + CTxOut txout; + if (ptx && c.hash == ptx->GetHash() && c.n < ptx->vout.size()) { + // the collateral is an output of this tx + txout = ptx->vout[c.n]; + } else { + // the collateral is a reference to an utxo inside this wallet + const auto& it = mapWallet.find(c.hash); + if (it != mapWallet.end()) { + txout = it->second.tx->vout[c.n]; + } + } + if (!txout.IsNull() && IsMine(txout) != ISMINE_NO && !IsSpent(c)) { + LockCoin(c); + } +} + +// Called during Init +void CWallet::ScanMasternodeCollateralsAndLock(const CDeterministicMNList& mnList) +{ + LOCK(cs_wallet); + + LogPrintf("Locking masternode collaterals...\n"); + mnList.ForEachMN(false, [&](const CDeterministicMNCPtr& dmn) { + LockOutpointIfMine(nullptr, dmn->collateralOutpoint); + }); +} + +// Called from AddToWalletIfInvolvingMe +void CWallet::LockIfMyCollateral(const CTransactionRef& ptx) +{ + AssertLockHeld(cs_wallet); + + COutPoint o; + if (GetProRegCollateral(ptx, o)) { + LockOutpointIfMine(ptx, o); + } +} + std::string CWallet::GetWalletHelpString(bool showDebug) { std::string strUsage = HelpMessageGroup(_("Wallet options:")); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 5b4a7e6e0365..383d99f55e5f 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -98,6 +98,7 @@ class CScheduler; class ScriptPubKeyMan; class SaplingScriptPubKeyMan; class SaplingNoteData; +class CDeterministicMNList; /** (client) version numbers for particular wallet features */ enum WalletFeature { @@ -847,6 +848,29 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface bool SetStakeSplitThreshold(const CAmount sst); CAmount GetStakeSplitThreshold() const { LOCK(cs_wallet); return nStakeSplitThreshold; } + /* + * Requires cs_wallet lock. + * Lock for spending the coin c, if it's owned by the wallet, it's unspent, and: + * -- If ptx is not null, c is one of the outputs of *ptx + * -- If ptx is null, c is the output of a transaction in mapWallet + */ + void LockOutpointIfMine(const CTransactionRef& ptx, const COutPoint& c); + + /* + * Locks cs_wallet + * Called during Init. If a DMN collateral is found in the wallet, + * lock the corresponding coin, to prevent accidental spending. + */ + void ScanMasternodeCollateralsAndLock(const CDeterministicMNList& mnList); + + /* + * Requires cs_wallet lock. + * Called from AddToWalletIfInvolvingMe. If ptx is a ProRegTx, and the + * collateral (either referenced or created) is owned by this wallet, + * lock the corresponding coin, to prevent accidental spending. + */ + void LockIfMyCollateral(const CTransactionRef& ptx); + // keystore implementation PairResult getNewAddress(CTxDestination& ret, const std::string addressLabel, const std::string purpose, const CChainParams::Base58Type addrType = CChainParams::PUBKEY_ADDRESS); From 93e84534bc8af3f61818b0e31b23751f138b23a2 Mon Sep 17 00:00:00 2001 From: random-zebra Date: Mon, 8 Mar 2021 23:27:09 +0100 Subject: [PATCH 3/8] [Tests] Update deterministicmns test checking collateral locking --- test/functional/test_framework/messages.py | 3 ++ .../test_framework/test_framework.py | 29 +++++------ test/functional/test_framework/util.py | 10 ++-- test/functional/tiertwo_deterministicmns.py | 49 ++++++++++++++----- 4 files changed, 57 insertions(+), 34 deletions(-) diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index 2b8e684f4c95..168ab658b8c1 100644 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -305,6 +305,9 @@ def deserialize_uniqueness(self, f): def __repr__(self): return "COutPoint(hash=%064x n=%i)" % (self.hash, self.n) + def to_json(self): + return {"txid": "%064x" % self.hash, "vout": self.n} + NullOutPoint = COutPoint(0, 0xffffffff) class CTxIn(): diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 2995c94b6c72..5a5488cb272d 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -48,13 +48,13 @@ connect_nodes_clique, disconnect_nodes, get_collateral_vout, - lock_utxo, Decimal, DEFAULT_FEE, get_datadir_path, hex_str_to_bytes, bytes_to_hex_str, initialize_datadir, + is_coin_locked_by, create_new_dmn, p2p_port, set_node_times, @@ -1193,7 +1193,7 @@ def not_found(): """ Create a ProReg tx, which has the collateral as one of its outputs """ - def protx_register_fund(self, miner, controller, dmn, collateral_addr, lock=True, op_rew=None): + def protx_register_fund(self, miner, controller, dmn, collateral_addr, op_rew=None): # send to the owner the collateral tx + some dust for the ProReg and fee funding_txid = miner.sendtoaddress(collateral_addr, Decimal('101')) # confirm and verify reception @@ -1210,14 +1210,12 @@ def protx_register_fund(self, miner, controller, dmn, collateral_addr, lock=True op_rew["reward"], op_rew["address"]) dmn.collateral = COutPoint(int(dmn.proTx, 16), get_collateral_vout(controller.getrawtransaction(dmn.proTx, True))) - if lock: - lock_utxo(controller, dmn.collateral) """ Create a ProReg tx, which references an 100 PIV UTXO as collateral. The controller node owns the collateral and creates the ProReg tx. """ - def protx_register(self, miner, controller, dmn, collateral_addr, fLock): + def protx_register(self, miner, controller, dmn, collateral_addr): # send to the owner the exact collateral tx amount funding_txid = miner.sendtoaddress(collateral_addr, Decimal('100')) # send another output to be used for the fee of the proReg tx @@ -1231,14 +1229,12 @@ def protx_register(self, miner, controller, dmn, collateral_addr, fLock): dmn.collateral = COutPoint(int(funding_txid, 16), get_collateral_vout(json_tx)) dmn.proTx = controller.protx_register(funding_txid, dmn.collateral.n, dmn.ipport, dmn.owner, dmn.operator, dmn.voting, dmn.payee) - if fLock: - lock_utxo(controller, dmn.collateral) """ Create a ProReg tx, referencing a collateral signed externally (eg. HW wallets). Here the controller node owns the collateral (and signs), but the miner creates the ProReg tx. """ - def protx_register_ext(self, miner, controller, dmn, outpoint, fSubmit, fLock): + def protx_register_ext(self, miner, controller, dmn, outpoint, fSubmit): # send to the owner the collateral tx if the outpoint is not specified if outpoint is None: funding_txid = miner.sendtoaddress(controller.getnewaddress("collateral"), Decimal('100')) @@ -1254,8 +1250,6 @@ def protx_register_ext(self, miner, controller, dmn, outpoint, fSubmit, fLock): dmn.operator, dmn.voting, dmn.payee) sig = controller.signmessage(reg_tx["collateralAddress"], reg_tx["signMessage"]) if fSubmit: - if fLock: - lock_utxo(controller, dmn.collateral) dmn.proTx = miner.protx_register_submit(reg_tx["tx"], sig) else: return reg_tx["tx"], sig @@ -1271,11 +1265,10 @@ def protx_register_ext(self, miner, controller, dmn, outpoint, fSubmit, fLock): If not provided, a new utxo is created, sending it from the miner. op_addr_and_key: (list of strings) List with two entries, operator address (0) and private key (1). If not provided, a new address-key pair is generated. - fLock: (boolean) lock the collateral output :return: dmn: (Masternode) the deterministic masternode object """ def register_new_dmn(self, idx, miner_idx, controller_idx, strType, - payout_addr=None, outpoint=None, op_addr_and_key=None, fLock=True): + payout_addr=None, outpoint=None, op_addr_and_key=None): # Prepare remote node assert idx != miner_idx assert idx != controller_idx @@ -1293,11 +1286,11 @@ def register_new_dmn(self, idx, miner_idx, controller_idx, strType, self.log.info("Creating%s proRegTx for deterministic masternode idx=%d..." % ( " and funding" if strType == "fund" else "", idx)) if strType == "fund": - self.protx_register_fund(miner_node, controller_node, dmn, collateral_addr, fLock) + self.protx_register_fund(miner_node, controller_node, dmn, collateral_addr) elif strType == "internal": - self.protx_register(miner_node, controller_node, dmn, collateral_addr, fLock) + self.protx_register(miner_node, controller_node, dmn, collateral_addr) elif strType == "external": - self.protx_register_ext(miner_node, controller_node, dmn, outpoint, True, fLock) + self.protx_register_ext(miner_node, controller_node, dmn, outpoint, True) else: raise Exception("Type %s not available" % strType) time.sleep(1) @@ -1308,6 +1301,10 @@ def register_new_dmn(self, idx, miner_idx, controller_idx, strType, self.sync_blocks(self.nodes) assert_greater_than(mn_node.getrawtransaction(dmn.proTx, 1)["confirmations"], 0) assert dmn.proTx in mn_node.protx_list(False) + + # check coin locking + assert is_coin_locked_by(controller_node, dmn.collateral) + return dmn def check_mn_list_on_node(self, idx, mns): @@ -1317,7 +1314,7 @@ def check_mn_list_on_node(self, idx, mns): protxs = [x["proTxHash"] for x in mnlist] for mn in mns: if mn.proTx not in protxs: - raise Exception("ProTx for mn %d (%s) not found in the list of node %d", mn.idx, mn.proTx, idx) + raise Exception("ProTx for mn %d (%s) not found in the list of node %d" % (mn.idx, mn.proTx, idx)) ### ------------------------------------------------------ diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index 9006cebb57ef..de17d6fb3ad9 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -583,9 +583,8 @@ def get_coinstake_address(node, expected_utxos=None): return addrs[0] # Deterministic masternodes - -def lock_utxo(node, outpoint): - node.lockunspent(False, [{"txid": "%064x" % outpoint.hash, "vout": outpoint.n}]) +def is_coin_locked_by(node, outpoint): + return outpoint.to_json() in node.listlockunspent() def get_collateral_vout(json_tx): funding_txidn = -1 @@ -599,7 +598,8 @@ def get_collateral_vout(json_tx): # owner and voting keys are created from controller node. # operator key and address are created, if operator_addr_and_key is None. def create_new_dmn(idx, controller, payout_addr, operator_addr_and_key): - ipport = "127.0.0.1:" + str(p2p_port(idx)) + port = p2p_port(idx) if idx <= MAX_NODES else p2p_port(MAX_NODES) + (idx - MAX_NODES) + ipport = "127.0.0.1:" + str(port) owner_addr = controller.getnewaddress("mnowner-%d" % idx) voting_addr = controller.getnewaddress("mnvoting-%d" % idx) if operator_addr_and_key is None: @@ -611,7 +611,7 @@ def create_new_dmn(idx, controller, payout_addr, operator_addr_and_key): return messages.Masternode(idx, owner_addr, operator_addr, voting_addr, ipport, payout_addr, operator_key) def spend_mn_collateral(spender, dmn): - inputs = [{"txid": "%064x" % dmn.collateral.hash, "vout": dmn.collateral.n}] + inputs = [dmn.collateral.to_json()] outputs = {spender.getnewaddress(): Decimal('99.99')} sig_res = spender.signrawtransaction(spender.createrawtransaction(inputs, outputs)) assert_equal(sig_res['complete'], True) diff --git a/test/functional/tiertwo_deterministicmns.py b/test/functional/tiertwo_deterministicmns.py index dc91ddc64800..254576a7bdae 100755 --- a/test/functional/tiertwo_deterministicmns.py +++ b/test/functional/tiertwo_deterministicmns.py @@ -23,7 +23,9 @@ assert_raises_rpc_error, bytes_to_hex_str, create_new_dmn, + connect_nodes, hex_str_to_bytes, + is_coin_locked_by, spend_mn_collateral, ) @@ -39,14 +41,13 @@ def set_test_params(self): self.extra_args = [["-nuparams=v5_shield:1", "-nuparams=v6_evo:130"]] * self.num_nodes self.extra_args[0].append("-sporkkey=932HEevBSujW2ud7RfB1YF91AFygbBRQj3de3LyaCRqNzKKgWXi") - def add_new_dmn(self, mns, strType, op_keys=None, from_out=None, lock=True): + def add_new_dmn(self, mns, strType, op_keys=None, from_out=None): mns.append(self.register_new_dmn(2 + len(mns), self.minerPos, self.controllerPos, strType, outpoint=from_out, - op_addr_and_key=op_keys, - fLock=lock)) + op_addr_and_key=op_keys)) def check_mn_list(self, mns): for i in range(self.num_nodes): @@ -61,12 +62,21 @@ def get_last_paid_mn(self): return next(x['proTxHash'] for x in self.nodes[0].listmasternodes() if x['dmnstate']['lastPaidHeight'] == self.nodes[0].getblockcount()) - def create_block(self, mn_payee_script, height, prevhash): - coinbase = create_coinbase(height) + def create_block(self, mn_payee_script, prev_block): + coinbase = create_coinbase(prev_block["height"] + 1) coinbase.vout[0].nValue -= 3 * COIN coinbase.vout.append(CTxOut(int(3 * COIN), hex_str_to_bytes(mn_payee_script))) coinbase.rehash() - return create_block(int(prevhash, 16), coinbase, nVersion=10) + return create_block(int(prev_block["hash"], 16), + coinbase, + hashFinalSaplingRoot=int(prev_block["finalsaplingroot"], 16), + nVersion=10) + + def restart_controller(self): + self.restart_node(self.controllerPos, extra_args=self.extra_args[self.controllerPos]) + self.connect_to_all(self.controllerPos) + connect_nodes(self.nodes[self.controllerPos], self.minerPos) + self.sync_all() def wait_until_mnsync_completed(self): SYNC_FINISHED = [999] * self.num_nodes @@ -111,11 +121,14 @@ def run_test(self): assert_raises_rpc_error(-1, "Evo upgrade is not active yet", self.add_new_dmn, mns, "fund") # Can create the raw proReg dmn = create_new_dmn(2, controller, dummy_add, None) - tx, sig = self.protx_register_ext(miner, controller, dmn, None, False, False) + tx, sig = self.protx_register_ext(miner, controller, dmn, None, False) # but cannot send it assert_raises_rpc_error(-1, "Evo upgrade is not active yet", miner.protx_register_submit, tx, sig) self.log.info("Done. Now mine blocks till enforcement...") + # Check that no coin has been locked by the controller yet + assert_equal(len(controller.listlockunspent()), 0) + # DIP3 activates at block 130. miner.generate(130 - miner.getblockcount()) self.sync_blocks() @@ -162,6 +175,17 @@ def run_test(self): ["Ready"] * (self.num_nodes - 2)) self.log.info("All masternodes ready.") + # Restart the controller and check that the collaterals are still locked + self.log.info("Restarting controller...") + self.restart_controller() + time.sleep(1) + for mn in mns: + if not is_coin_locked_by(controller, mn.collateral): + raise Exception( + "Collateral %s of mn with idx=%d is not locked" % (mn.collateral, mn.idx) + ) + self.log.info("Collaterals still locked.") + # Test collateral spending dmn = mns.pop(randrange(len(mns))) # pop one at random self.log.info("Spending collateral of mn with idx=%d..." % dmn.idx) @@ -179,8 +203,7 @@ def run_test(self): dmn2_keys = [dmn2.operator, dmn2.operator_key] self.log.info("Reactivating node %d reusing the collateral of node %d..." % (dmn.idx, dmn2.idx)) mns.append(self.register_new_dmn(dmn.idx, self.minerPos, self.controllerPos, "external", - outpoint=dmn2.collateral, op_addr_and_key=dmn_keys, - fLock=False)) + outpoint=dmn2.collateral, op_addr_and_key=dmn_keys)) miner.generate(1) self.sync_blocks() self.check_mn_list(mns) @@ -196,20 +219,20 @@ def run_test(self): self.log.info("Trying duplicate operator key...") dmn2b = create_new_dmn(dmn2.idx, controller, dummy_add, dmn_keys) assert_raises_rpc_error(-1, "bad-protx-dup-operator-key", - self.protx_register_fund, miner, controller, dmn2b, dummy_add, False) + self.protx_register_fund, miner, controller, dmn2b, dummy_add) # Now try with duplicate owner key self.log.info("Trying duplicate owner key...") dmn2c = create_new_dmn(dmn2.idx, controller, dummy_add, dmn2_keys) dmn2c.owner = mns[randrange(len(mns))].owner assert_raises_rpc_error(-1, "bad-protx-dup-owner-key", - self.protx_register_fund, miner, controller, dmn2c, dummy_add, False) + self.protx_register_fund, miner, controller, dmn2c, dummy_add) # Finally, register it properly. This time setting 10% of the reward for the operator op_rew = {"reward": 10.00, "address": self.nodes[dmn2.idx].getnewaddress()} self.log.info("Reactivating the node with a new registration (with operator reward)...") dmn2c = create_new_dmn(dmn2.idx, controller, dummy_add, dmn2_keys) - self.protx_register_fund(miner, controller, dmn2c, dummy_add, True, op_rew) + self.protx_register_fund(miner, controller, dmn2c, dummy_add, op_rew) mns.append(dmn2c) time.sleep(1) self.sync_mempools([miner, controller]) @@ -244,7 +267,7 @@ def run_test(self): self.wait_until_mnsync_completed() # just to be sure self.log.info("Testing invalid masternode payment...") mn_payee_script = miner.validateaddress(miner.getnewaddress())['scriptPubKey'] - block = self.create_block(mn_payee_script, miner.getblockcount(), miner.getbestblockhash()) + block = self.create_block(mn_payee_script, miner.getblock(miner.getbestblockhash(), True)) block.solve() assert_equal(miner.submitblock(bytes_to_hex_str(block.serialize())), "bad-cb-payee") From 5d6bcef8be46a8a6a275741dd0c40d26a8dae889 Mon Sep 17 00:00:00 2001 From: random-zebra Date: Tue, 9 Mar 2021 19:01:51 +0100 Subject: [PATCH 4/8] [RPC][Tests] Include proTx data in json formatted transactions --- .../test_framework/test_framework.py | 23 +++++++++++++++++-- test/functional/tiertwo_deterministicmns.py | 3 +++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 5a5488cb272d..b6ff820b041d 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -1298,13 +1298,17 @@ def register_new_dmn(self, idx, miner_idx, controller_idx, strType, # confirm and verify inclusion in list miner_node.generate(1) - self.sync_blocks(self.nodes) - assert_greater_than(mn_node.getrawtransaction(dmn.proTx, 1)["confirmations"], 0) + self.sync_blocks() + json_tx = mn_node.getrawtransaction(dmn.proTx, 1) + assert_greater_than(json_tx["confirmations"], 0) assert dmn.proTx in mn_node.protx_list(False) # check coin locking assert is_coin_locked_by(controller_node, dmn.collateral) + # check json payload against local dmn object + self.check_proreg_payload(dmn, json_tx) + return dmn def check_mn_list_on_node(self, idx, mns): @@ -1316,6 +1320,21 @@ def check_mn_list_on_node(self, idx, mns): if mn.proTx not in protxs: raise Exception("ProTx for mn %d (%s) not found in the list of node %d" % (mn.idx, mn.proTx, idx)) + def check_proreg_payload(self, dmn, json_tx): + assert "payload" in json_tx + # null hash if funding collateral + collateral_hash = 0 if int(json_tx["txid"], 16) == dmn.collateral.hash \ + else dmn.collateral.hash + pl = json_tx["payload"] + assert_equal(pl["version"], 1) + assert_equal(pl["collateralHash"], "%064x" % collateral_hash) + assert_equal(pl["collateralIndex"], dmn.collateral.n) + assert_equal(pl["service"], dmn.ipport) + assert_equal(pl["ownerAddress"], dmn.owner) + assert_equal(pl["votingAddress"], dmn.voting) + assert_equal(pl["operatorAddress"], dmn.operator) + assert_equal(pl["payoutAddress"], dmn.payee) + ### ------------------------------------------------------ diff --git a/test/functional/tiertwo_deterministicmns.py b/test/functional/tiertwo_deterministicmns.py index 254576a7bdae..bfa3428db81e 100755 --- a/test/functional/tiertwo_deterministicmns.py +++ b/test/functional/tiertwo_deterministicmns.py @@ -238,6 +238,9 @@ def run_test(self): self.sync_mempools([miner, controller]) miner.generate(6) self.sync_blocks() + json_tx = self.nodes[dmn2c.idx].getrawtransaction(dmn2c.proTx, True) + assert_greater_than(json_tx['confirmations'], 0) + self.check_proreg_payload(dmn2c, json_tx) self.check_mn_list(mns) # 6 masternodes again # Test payments. From 9a1bf27e8a5ba3d31840c354cd594b876f00afd5 Mon Sep 17 00:00:00 2001 From: random-zebra Date: Sat, 13 Mar 2021 23:02:26 +0100 Subject: [PATCH 5/8] [BUG] Add special tx processing to RollforwardBlock --- src/validation.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/validation.cpp b/src/validation.cpp index 8784e976ad14..325a07d36982 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3743,6 +3743,13 @@ static bool RollforwardBlock(const CBlockIndex* pindex, CCoinsViewCache& inputs, // Pass check = true as every addition may be an overwrite. AddCoins(inputs, *tx, pindex->nHeight, true, fSkipInvalid); } + + CValidationState state; + if (!ProcessSpecialTxsInBlock(block, pindex, state, false /*fJustCheck*/)) { + return error("%s: Special tx processing failed for block %s with %s", + __func__, pindex->GetBlockHash().ToString(), FormatStateMessage(state)); + } + return true; } From 57fa99f950c620f3fe4e9aba5fcb596432119666 Mon Sep 17 00:00:00 2001 From: random-zebra Date: Mon, 15 Mar 2021 01:44:35 +0100 Subject: [PATCH 6/8] [Tests] Introduce tiertwo_reorg_mempool test --- .../test_framework/test_framework.py | 1 + test/functional/test_runner.py | 11 +- test/functional/tiertwo_reorg_mempool.py | 244 ++++++++++++++++++ 3 files changed, 251 insertions(+), 5 deletions(-) create mode 100755 test/functional/tiertwo_reorg_mempool.py diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index b6ff820b041d..41ee1beabf2c 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -1312,6 +1312,7 @@ def register_new_dmn(self, idx, miner_idx, controller_idx, strType, return dmn def check_mn_list_on_node(self, idx, mns): + self.nodes[idx].syncwithvalidationinterfacequeue() mnlist = self.nodes[idx].listmasternodes() if len(mnlist) != len(mns): raise Exception("Invalid mn list on node %d:\n%s\nExpected:%s" % (idx, str(mnlist), str(mns))) diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index f4f747379c16..91bde2153c35 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -141,11 +141,12 @@ TIERTWO_SCRIPTS = [ # Longest test should go first, to favor running tests in parallel - 'tiertwo_deterministicmns.py', - 'tiertwo_governance_sync_basic.py', - 'tiertwo_mn_compatibility.py', - 'tiertwo_masternode_activation.py', - 'tiertwo_masternode_ping.py', + 'tiertwo_governance_sync_basic.py', # ~ 445 sec + 'tiertwo_mn_compatibility.py', # ~ 413 sec + 'tiertwo_deterministicmns.py', # ~ 366 sec + 'tiertwo_masternode_activation.py', # ~ 352 sec + 'tiertwo_masternode_ping.py', # ~ 293 sec + 'tiertwo_reorg_mempool.py', # ~ 107 sec ] SAPLING_SCRIPTS = [ diff --git a/test/functional/tiertwo_reorg_mempool.py b/test/functional/tiertwo_reorg_mempool.py new file mode 100755 index 000000000000..2fc67fbcc858 --- /dev/null +++ b/test/functional/tiertwo_reorg_mempool.py @@ -0,0 +1,244 @@ +#!/usr/bin/env python3 +# Copyright (c) 2021 The PIVX Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +""" +Test deterministic masternodes conflicts and reorgs. +- Check that in-mempool reuse of mn unique-properties is invalid +- Check mempool eviction after conflict with newly connected block / reorg +- Check deterministic list consensus after reorg +""" + +import random +import time + +from test_framework.test_framework import PivxTestFramework + +from test_framework.util import ( + assert_equal, + assert_raises_rpc_error, + create_new_dmn, + connect_nodes, + disconnect_nodes, +) + +class TiertwoReorgMempoolTest(PivxTestFramework): + + def set_test_params(self): + # two nodes mining on separate chains + self.num_nodes = 2 + self.setup_clean_chain = True + self.extra_args = [["-nuparams=v5_shield:1", "-nuparams=v6_evo:160"]] * self.num_nodes + self.extra_args[0].append("-sporkkey=932HEevBSujW2ud7RfB1YF91AFygbBRQj3de3LyaCRqNzKKgWXi") + + def setup_network(self): + self.setup_nodes() + self.connect_all() + + def connect_all(self): + connect_nodes(self.nodes[0], 1) + connect_nodes(self.nodes[1], 0) + + def disconnect_all(self): + self.log.info("Disconnecting nodes...") + disconnect_nodes(self.nodes[0], 1) + disconnect_nodes(self.nodes[1], 0) + self.log.info("Nodes disconnected") + + def register_masternode(self, from_node, dmn, collateral_addr): + dmn.proTx = from_node.protx_register_fund(collateral_addr, dmn.ipport, dmn.owner, + dmn.operator, dmn.voting, dmn.payee) + + def run_test(self): + self.disable_mocktime() + nodeA = self.nodes[0] + nodeB = self.nodes[1] + free_idx = 1 # unique id for masternodes. first available. + + # Enforce mn payments and reject legacy mns at block 202 + self.activate_spork(0, "SPORK_8_MASTERNODE_PAYMENT_ENFORCEMENT") + assert_equal("success", self.set_spork(0, "SPORK_21_LEGACY_MNS_MAX_HEIGHT", 201)) + time.sleep(1) + assert_equal([201] * self.num_nodes, [self.get_spork(x, "SPORK_21_LEGACY_MNS_MAX_HEIGHT") + for x in range(self.num_nodes)]) + + # Mine 201 blocks + self.log.info("Mining...") + nodeA.generate(25) + self.sync_blocks() + nodeB.generate(25) + self.sync_blocks() + nodeA.generate(50) + self.sync_blocks() + nodeB.generate(101) + self.sync_blocks() + self.assert_equal_for_all(201, "getblockcount") + + # Register one masternode before the split + collateral_addr = nodeA.getnewaddress() # for both collateral and payouts + pre_split_mn = create_new_dmn(100, nodeA, nodeA.getnewaddress(), None) + self.register_masternode(nodeA, pre_split_mn, collateral_addr) + nodeA.generate(1) + self.sync_blocks() + mnsA = [pre_split_mn] + mnsB = [pre_split_mn] + self.check_mn_list_on_node(0, mnsA) + self.check_mn_list_on_node(1, mnsB) + self.log.info("Pre-split masternode registered.") + + # Disconnect the nodes + self.disconnect_all() # network splits at block 203 + + # + # -- CHAIN A -- + # + + # Register 5 masternodes, then mine 5 blocks + self.log.info("Registering masternodes on chain A...") + for _ in range(5): + dmn = create_new_dmn(free_idx, nodeA, collateral_addr, None) + free_idx += 1 + self.register_masternode(nodeA, dmn, collateral_addr) + mnsA.append(dmn) + nodeA.generate(5) + self.check_mn_list_on_node(0, mnsA) + self.log.info("Masternodes registered on chain A.") + + # Lock any utxo with less than 101 confs (e.g. change), so we can resurrect everything + for x in nodeA.listunspent(0, 101): + nodeA.lockunspent(False, [{"txid": x["txid"], "vout": x["vout"]}]) + + # Now send a valid proReg tx to the mempool, without mining it + mempool_dmn1 = create_new_dmn(free_idx, nodeA, collateral_addr, None) + free_idx += 1 + self.register_masternode(nodeA, mempool_dmn1, collateral_addr) + assert mempool_dmn1.proTx in nodeA.getrawmempool() + + # Try sending a proReg tx with same owner + self.log.info("Testing in-mempool duplicate-owner rejection...") + dmn_A1 = create_new_dmn(free_idx, nodeA, collateral_addr, None) + free_idx += 1 + dmn_A1.owner = mempool_dmn1.owner + assert_raises_rpc_error(-26, "protx-dup", + self.register_masternode, nodeA, dmn_A1, collateral_addr) + assert dmn_A1.proTx not in nodeA.getrawmempool() + + # Try sending a proReg tx with same operator + self.log.info("Testing in-mempool duplicate-operator rejection...") + dmn_A2 = create_new_dmn(free_idx, nodeA, collateral_addr, None) + free_idx += 1 + dmn_A2.operator = mempool_dmn1.operator + assert_raises_rpc_error(-26, "protx-dup", + self.register_masternode, nodeA, dmn_A2, collateral_addr) + assert dmn_A2.proTx not in nodeA.getrawmempool() + + # Try sending a proReg tx with same IP + self.log.info("Testing proReg in-mempool duplicate-IP rejection...") + dmn_A3 = create_new_dmn(free_idx, nodeA, collateral_addr, None) + free_idx += 1 + dmn_A3.ipport = mempool_dmn1.ipport + assert_raises_rpc_error(-26, "protx-dup", + self.register_masternode, nodeA, dmn_A3, collateral_addr) + assert dmn_A3.proTx not in nodeA.getrawmempool() + + # Now send other 2 valid proReg tx to the mempool, without mining it + mempool_dmn2 = create_new_dmn(free_idx, nodeA, collateral_addr, None) + free_idx += 1 + mempool_dmn3 = create_new_dmn(free_idx, nodeA, collateral_addr, None) + free_idx += 1 + self.register_masternode(nodeA, mempool_dmn2, collateral_addr) + self.register_masternode(nodeA, mempool_dmn3, collateral_addr) + + # Now nodeA has 3 proReg txes in its mempool + mempoolA = nodeA.getrawmempool() + assert mempool_dmn1.proTx in mempoolA + assert mempool_dmn2.proTx in mempoolA + assert mempool_dmn3.proTx in mempoolA + + assert_equal(nodeA.getblockcount(), 207) + + # + # -- CHAIN B -- + # + collateral_addr = nodeB.getnewaddress() + self.log.info("Registering masternodes on chain B...") + + # Register first the 3 nodes that conflict with the mempool of nodes[0] + # mine one block after each registration + for dmn in [dmn_A1, dmn_A2, dmn_A3]: + self.register_masternode(nodeB, dmn, collateral_addr) + mnsB.append(dmn) + nodeB.generate(1) + self.check_mn_list_on_node(1, mnsB) + + # Pick the proReg for the first MN registered on chain A, and replay it on chain B + self.log.info("Replaying a masternode on a different chain...") + mnsA.remove(pre_split_mn) + replay_mn = mnsA.pop(0) + mnsB.append(replay_mn) # same proTx hash + nodeB.sendrawtransaction(nodeA.getrawtransaction(replay_mn.proTx, False)) + nodeB.generate(1) + self.check_mn_list_on_node(1, mnsB) + + # Now pick a proReg for another MN registered on chain A, and re-register it on chain B + self.log.info("Re-registering a masternode on a different chain...") + rereg_mn = random.choice(mnsA) + mnsA.remove(rereg_mn) + self.register_masternode(nodeB, rereg_mn, collateral_addr) + mnsB.append(rereg_mn) # changed proTx hash + nodeB.generate(1) + self.check_mn_list_on_node(1, mnsB) + + # Register 5 more masternodes. One per block. + for _ in range(5): + dmn = create_new_dmn(free_idx, nodeB, collateral_addr, None) + free_idx += 1 + self.register_masternode(nodeB, dmn, collateral_addr) + mnsB.append(dmn) + nodeB.generate(1) + + # Then mine 10 more blocks on chain B + nodeB.generate(10) + self.check_mn_list_on_node(1, mnsB) + self.log.info("Masternodes registered on chain B.") + + assert_equal(nodeB.getblockcount(), 222) + + # + # -- RECONNECT -- + # + + # Reconnect and sync (give it some more time) + self.log.info("Reconnecting nodes...") + self.connect_all() + # !TODO: FIXME - failing because we check budget/mn payment in CheckBlock + # during a reorg, the previous block hasn't been connected yet, so the dmn list is empty. + self.sync_blocks(wait=3, timeout=180) + + # Both nodes have the same list (mnB) + self.log.info("Checking masternode list...") + self.check_mn_list_on_node(0, mnsB) + self.check_mn_list_on_node(1, mnsB) + self.log.info("Both nodes have %d registered masternodes." % len(mnsB)) + + # The first mempool proReg tx has been removed from nodeA's mempool due to + # conflicts with the masternodes of chain B, now connected. + self.log.info("Checking mempool...") + mempoolA = nodeA.getrawmempool() + assert mempool_dmn1.proTx not in mempoolA + assert mempool_dmn2.proTx in mempoolA + assert mempool_dmn3.proTx in mempoolA + # The mempool contains also all the ProReg from the disconnected blocks, + # except the ones re-registered and replayed on chain B. + for mn in mnsA: + assert mn.proTx in mempoolA + assert rereg_mn.proTx not in mempoolA + assert replay_mn.proTx not in mempoolA + assert pre_split_mn.proTx not in mempoolA + + self.log.info("All good.") + + +if __name__ == '__main__': + TiertwoReorgMempoolTest().main() From f6aa6add3df43f660e9b92d32cbef1afab49b2a9 Mon Sep 17 00:00:00 2001 From: random-zebra Date: Sat, 13 Mar 2021 16:26:59 +0100 Subject: [PATCH 7/8] [BUG] Check masternode/budget payments during block connection As the tiertwo_reorg_mempool shows, the reorganization currently fails. This is because, during a reorg, CheckBlock is called on blocks where the prev is not connected to the chain yet. Since the budget/mn payment is checked there, we call GeListForBlock (requesting a list which has not been built yet) and so we erroneously cache an empty list. Fix this by moving IsBlockPayeeValid from CheckBlock to ConnectBlock (as, to check budget/mn payments, the previous block deterministic list must be fully built). Also, to prevent sneaky bugs like this, in the future, only cache the initial snapshot in GetListForBlock for the very last block before the enforcement of DIP3. After the enforcement block, at least the diff is always written to disk in BuildListFromBlock. --- src/evo/deterministicmns.cpp | 10 +++++++++- src/evo/deterministicmns.h | 1 + src/test/evo_deterministicmns_tests.cpp | 19 +++++++++++++++---- src/test/test_pivx.cpp | 2 ++ src/validation.cpp | 15 +++++++++------ test/functional/tiertwo_reorg_mempool.py | 2 -- 6 files changed, 36 insertions(+), 13 deletions(-) diff --git a/src/evo/deterministicmns.cpp b/src/evo/deterministicmns.cpp index b11841e21b0c..860a50d1d97f 100644 --- a/src/evo/deterministicmns.cpp +++ b/src/evo/deterministicmns.cpp @@ -7,6 +7,7 @@ #include "base58.h" #include "chainparams.h" +#include "consensus/upgrades.h" #include "core_io.h" #include "evo/specialtx.h" #include "guiinterface.h" @@ -762,6 +763,7 @@ CDeterministicMNList CDeterministicMNManager::GetListForBlock(const CBlockIndex* { LOCK(cs); + // Return early before enforcement if (!IsDIP3Enforced(pindex->nHeight)) { return {}; } @@ -792,7 +794,13 @@ CDeterministicMNList CDeterministicMNManager::GetListForBlock(const CBlockIndex* CDeterministicMNListDiff diff; if (!evoDb.Read(std::make_pair(DB_LIST_DIFF, pindex->GetBlockHash()), diff)) { - // no snapshot and no diff on disk means that it's the initial snapshot + // no snapshot and no diff on disk means that it's initial snapshot (empty list) + // If we get here, then this must be the block before the enforcement of DIP3. + if (!IsActivationHeight(pindex->nHeight + 1, Params().GetConsensus(), Consensus::UPGRADE_V6_0)) { + std::string err = strprintf("No masternode list data found for block %s at height %d. " + "Possible corrupt database.", pindex->GetBlockHash().ToString(), pindex->nHeight); + throw std::runtime_error(err); + } snapshot = CDeterministicMNList(pindex->GetBlockHash(), -1, 0); mnListsCache.emplace(pindex->GetBlockHash(), snapshot); break; diff --git a/src/evo/deterministicmns.h b/src/evo/deterministicmns.h index fd9a4fde3abc..69363f07d302 100644 --- a/src/evo/deterministicmns.h +++ b/src/evo/deterministicmns.h @@ -586,6 +586,7 @@ class CDeterministicMNManager bool BuildNewListFromBlock(const CBlock& block, const CBlockIndex* pindexPrev, CValidationState& state, CDeterministicMNList& mnListRet, bool debugLogs); void DecreasePoSePenalties(CDeterministicMNList& mnList); + // to return a valid list, it must have been built first, so never call it with a block not-yet connected (e.g. from CheckBlock). CDeterministicMNList GetListForBlock(const CBlockIndex* pindex); CDeterministicMNList GetListAtChainTip(); diff --git a/src/test/evo_deterministicmns_tests.cpp b/src/test/evo_deterministicmns_tests.cpp index 87e13136667e..39d43e3d6021 100644 --- a/src/test/evo_deterministicmns_tests.cpp +++ b/src/test/evo_deterministicmns_tests.cpp @@ -192,13 +192,21 @@ BOOST_FIXTURE_TEST_CASE(dip3_protx, TestChain400Setup) CBlockIndex* chainTip = chainActive.Tip(); int nHeight = chainTip->nHeight; - UpdateNetworkUpgradeParameters(Consensus::UPGRADE_V6_0, nHeight); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_V6_0, nHeight + 2); + + // load empty list (last block before enforcement) + CreateAndProcessBlock({}, coinbaseKey); + chainTip = chainActive.Tip(); + BOOST_CHECK_EQUAL(chainTip->nHeight, ++nHeight); + deterministicMNManager->UpdatedBlockTip(chainTip); + + // force mnsync complete and enable spork 8 masternodeSync.RequestedMasternodeAssets = MASTERNODE_SYNC_FINISHED; - // enable SPORK_8 int64_t nTime = GetTime() - 10; const CSporkMessage& sporkMnPayment = CSporkMessage(SPORK_8_MASTERNODE_PAYMENT_ENFORCEMENT, nTime + 1, nTime); sporkManager.AddOrUpdateSporkMessage(sporkMnPayment); BOOST_CHECK(sporkManager.IsSporkActive(SPORK_8_MASTERNODE_PAYMENT_ENFORCEMENT)); + int port = 1; std::vector dmnHashes; @@ -403,8 +411,11 @@ BOOST_FIXTURE_TEST_CASE(dip3_protx, TestChain400Setup) pblock->vtx[0] = MakeTransactionRef(invalidCoinbaseTx); pblock->hashMerkleRoot = BlockMerkleRoot(*pblock); CValidationState state; - BOOST_CHECK_MESSAGE(!ProcessNewBlock(state, nullptr, pblock, nullptr), "Error, invalid block paying to an already paid DMN passed"); - BOOST_CHECK(WITH_LOCK(cs_main, return chainActive.Height()) == nHeight); // no block connected + ProcessNewBlock(state, nullptr, pblock, nullptr); + // block not connected + chainTip = WITH_LOCK(cs_main, return chainActive.Tip()); + BOOST_CHECK(chainTip->nHeight == nHeight); + BOOST_CHECK(chainTip->GetBlockHash() != pblock->GetHash()); UpdateNetworkUpgradeParameters(Consensus::UPGRADE_V6_0, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); } diff --git a/src/test/test_pivx.cpp b/src/test/test_pivx.cpp index 46f811cdffcb..856eacc884cd 100644 --- a/src/test/test_pivx.cpp +++ b/src/test/test_pivx.cpp @@ -139,6 +139,8 @@ TestChainSetup::TestChainSetup(int blockCount) : TestingSetup(CBaseChainParams:: CBlock b = CreateAndProcessBlock(noTxns, scriptPubKey); coinbaseTxns.push_back(*b.vtx[0]); } + + deterministicMNManager->UpdatedBlockTip(chainActive.Tip()); } // Create a new block with coinbase paying to scriptPubKey, and try to add it to the current chain. diff --git a/src/validation.cpp b/src/validation.cpp index 325a07d36982..cb8c0db3b442 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1717,6 +1717,15 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd REJECT_INVALID, "bad-blk-amount"); } + // Masternode/Budget payments + // !TODO: after transition to DMN is complete, check this also during IBD + if (!fInitialBlockDownload) { + if (!IsBlockPayeeValid(block, pindex->pprev)) { + mapRejectedBlocks.emplace(block.GetHash(), GetTime()); + return state.DoS(0, false, REJECT_INVALID, "bad-cb-payee", false, "Couldn't find masternode/budget payment"); + } + } + // For blocks v10+: Check that the coinbase pays the exact amount if (isPoSActive && pindex->nVersion >= 10 && !IsCoinbaseValueValid(block.vtx[0], nBudgetAmt, state)) { // pass the state returned by the function above @@ -2834,12 +2843,6 @@ bool CheckBlock(const CBlock& block, CValidationState& state, bool fCheckPOW, bo // set Cold Staking Spork fColdStakingActive = !sporkManager.IsSporkActive(SPORK_19_COLDSTAKING_MAINTENANCE); - // check masternode/budget payment - // !TODO: after transition to DMN is complete, check this also during IBD - if (!IsBlockPayeeValid(block, pindexPrev)) { - mapRejectedBlocks.emplace(block.GetHash(), GetTime()); - return state.DoS(0, false, REJECT_INVALID, "bad-cb-payee", false, "Couldn't find masternode/budget payment"); - } } else { LogPrintf("%s: Masternode/Budget payment checks skipped on sync\n", __func__); } diff --git a/test/functional/tiertwo_reorg_mempool.py b/test/functional/tiertwo_reorg_mempool.py index 2fc67fbcc858..ba8c940a961a 100755 --- a/test/functional/tiertwo_reorg_mempool.py +++ b/test/functional/tiertwo_reorg_mempool.py @@ -212,8 +212,6 @@ def run_test(self): # Reconnect and sync (give it some more time) self.log.info("Reconnecting nodes...") self.connect_all() - # !TODO: FIXME - failing because we check budget/mn payment in CheckBlock - # during a reorg, the previous block hasn't been connected yet, so the dmn list is empty. self.sync_blocks(wait=3, timeout=180) # Both nodes have the same list (mnB) From 4fd564c33a367df547780dfb1882ea5afeb18d88 Mon Sep 17 00:00:00 2001 From: random-zebra Date: Sat, 29 May 2021 11:58:28 +0200 Subject: [PATCH 8/8] [Tests] Add EvoNotificationInterface in the TestingSetup fixture Thus avoid direct calls to `UpdatedBlockTip` in the tests. --- src/test/evo_deterministicmns_tests.cpp | 12 +++++------- src/test/test_pivx.cpp | 12 ++++++++---- src/test/test_pivx.h | 2 ++ 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/test/evo_deterministicmns_tests.cpp b/src/test/evo_deterministicmns_tests.cpp index 39d43e3d6021..8421875c682e 100644 --- a/src/test/evo_deterministicmns_tests.cpp +++ b/src/test/evo_deterministicmns_tests.cpp @@ -19,6 +19,7 @@ #include "script/sign.h" #include "spork.h" #include "validation.h" +#include "validationinterface.h" #include @@ -198,7 +199,6 @@ BOOST_FIXTURE_TEST_CASE(dip3_protx, TestChain400Setup) CreateAndProcessBlock({}, coinbaseKey); chainTip = chainActive.Tip(); BOOST_CHECK_EQUAL(chainTip->nHeight, ++nHeight); - deterministicMNManager->UpdatedBlockTip(chainTip); // force mnsync complete and enable spork 8 masternodeSync.RequestedMasternodeAssets = MASTERNODE_SYNC_FINISHED; @@ -240,8 +240,7 @@ BOOST_FIXTURE_TEST_CASE(dip3_protx, TestChain400Setup) CreateAndProcessBlock({tx}, coinbaseKey); chainTip = chainActive.Tip(); BOOST_CHECK_EQUAL(chainTip->nHeight, nHeight + 1); - - deterministicMNManager->UpdatedBlockTip(chainTip); + SyncWithValidationInterfaceQueue(); BOOST_CHECK(deterministicMNManager->GetListAtChainTip().HasMN(txid)); // Add change to the utxos map @@ -260,10 +259,10 @@ BOOST_FIXTURE_TEST_CASE(dip3_protx, TestChain400Setup) // Mine 20 blocks, checking MN reward payments std::map mapPayments; for (size_t i = 0; i < 20; i++) { + SyncWithValidationInterfaceQueue(); auto dmnExpectedPayee = deterministicMNManager->GetListAtChainTip().GetMNPayee(); CBlock block = CreateAndProcessBlock({}, coinbaseKey); chainTip = chainActive.Tip(); - deterministicMNManager->UpdatedBlockTip(chainTip); BOOST_ASSERT(!block.vtx.empty()); BOOST_CHECK(IsMNPayeeInBlock(block, dmnExpectedPayee->pdmnState->scriptPayout)); mapPayments[dmnExpectedPayee->proTxHash]++; @@ -364,8 +363,7 @@ BOOST_FIXTURE_TEST_CASE(dip3_protx, TestChain400Setup) CreateAndProcessBlock(txns, coinbaseKey); chainTip = chainActive.Tip(); BOOST_CHECK_EQUAL(chainTip->nHeight, nHeight + 1); - - deterministicMNManager->UpdatedBlockTip(chainTip); + SyncWithValidationInterfaceQueue(); auto mnList = deterministicMNManager->GetListAtChainTip(); for (size_t j = 0; j < 3; j++) { BOOST_CHECK(mnList.HasMN(txns[j].GetHash())); @@ -377,10 +375,10 @@ BOOST_FIXTURE_TEST_CASE(dip3_protx, TestChain400Setup) // Mine 30 blocks, checking MN reward payments mapPayments.clear(); for (size_t i = 0; i < 30; i++) { + SyncWithValidationInterfaceQueue(); auto dmnExpectedPayee = deterministicMNManager->GetListAtChainTip().GetMNPayee(); CBlock block = CreateAndProcessBlock({}, coinbaseKey); chainTip = chainActive.Tip(); - deterministicMNManager->UpdatedBlockTip(chainTip); BOOST_ASSERT(!block.vtx.empty()); BOOST_CHECK(IsMNPayeeInBlock(block, dmnExpectedPayee->pdmnState->scriptPayout)); mapPayments[dmnExpectedPayee->proTxHash]++; diff --git a/src/test/test_pivx.cpp b/src/test/test_pivx.cpp index 856eacc884cd..20eb499ff1e4 100644 --- a/src/test/test_pivx.cpp +++ b/src/test/test_pivx.cpp @@ -11,6 +11,7 @@ #include "guiinterface.h" #include "evo/deterministicmns.h" #include "evo/evodb.h" +#include "evo/evonotificationinterface.h" #include "miner.h" #include "net_processing.h" #include "rpc/server.h" @@ -81,6 +82,12 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha // our unit tests aren't testing multiple parts of the code at once. GetMainSignals().RegisterBackgroundSignalScheduler(scheduler); + // Register EvoNotificationInterface + g_connman = std::unique_ptr(new CConnman(0x1337, 0x1337)); // Deterministic randomness for tests. + connman = g_connman.get(); + pEvoNotificationInterface = new EvoNotificationInterface(*connman); + RegisterValidationInterface(pEvoNotificationInterface); + // Ideally we'd move all the RPC tests to the functional testing framework // instead of unit tests, but for now we need these here. RegisterAllCoreRPCCommands(tableRPC); @@ -100,8 +107,6 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha nScriptCheckThreads = 3; for (int i=0; i < nScriptCheckThreads-1; i++) threadGroup.create_thread(&ThreadScriptCheck); - g_connman = std::unique_ptr(new CConnman(0x1337, 0x1337)); // Deterministic randomness for tests. - connman = g_connman.get(); RegisterNodeSignals(GetNodeSignals()); } @@ -114,6 +119,7 @@ TestingSetup::~TestingSetup() UnregisterAllValidationInterfaces(); GetMainSignals().UnregisterBackgroundSignalScheduler(); UnloadBlockIndex(); + delete pEvoNotificationInterface; delete pcoinsTip; delete pcoinsdbview; delete pblocktree; @@ -139,8 +145,6 @@ TestChainSetup::TestChainSetup(int blockCount) : TestingSetup(CBaseChainParams:: CBlock b = CreateAndProcessBlock(noTxns, scriptPubKey); coinbaseTxns.push_back(*b.vtx[0]); } - - deterministicMNManager->UpdatedBlockTip(chainActive.Tip()); } // Create a new block with coinbase paying to scriptPubKey, and try to add it to the current chain. diff --git a/src/test/test_pivx.h b/src/test/test_pivx.h index fb925a0f3a87..9f05ca95f536 100644 --- a/src/test/test_pivx.h +++ b/src/test/test_pivx.h @@ -50,11 +50,13 @@ struct BasicTestingSetup { * and wallet (if enabled) setup. */ class CConnman; +class EvoNotificationInterface; struct TestingSetup: public BasicTestingSetup { CCoinsViewDB *pcoinsdbview; boost::thread_group threadGroup; CConnman* connman; + EvoNotificationInterface* pEvoNotificationInterface; CScheduler scheduler; TestingSetup(const std::string& chainName = CBaseChainParams::MAIN);