From f4c5b419f25cedfef6c9f56497c7f321bd77d86b Mon Sep 17 00:00:00 2001 From: Daniel Kraft Date: Wed, 21 Oct 2020 16:51:13 +0200 Subject: [PATCH 1/3] Tweak sigTime as necessary for delayed broadcast. When creating a delayed masternode broadcast, check the sigtime against the collateral confirmation block and make sure that it meets the network criterion (signature must not be earlier than the timestamp of the block that confirmed the collateral 15 times). If the condition is violated, just use "now + one hour" as sigtime. --- divi/src/MasternodeBroadcastFactory.cpp | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/divi/src/MasternodeBroadcastFactory.cpp b/divi/src/MasternodeBroadcastFactory.cpp index 02723e0b9..afbceb311 100644 --- a/divi/src/MasternodeBroadcastFactory.cpp +++ b/divi/src/MasternodeBroadcastFactory.cpp @@ -348,7 +348,23 @@ void CMasternodeBroadcastFactory::createWithoutSignatures( ? createDelayedMasternodePing(mnbRet) : createCurrentPing(txin)); mnbRet.lastPing = mnp; - mnbRet.sigTime = mnp.sigTime; + + /* We have to ensure that the sigtime of the broadcast itself is later + than when the collateral gets its 15 confirmations (and it might not + even be confirmed once yet). */ + const auto* pindexConf = ComputeMasternodeConfirmationBlockIndex(mnbRet); + if (pindexConf == nullptr) { + /* The signature time is accepted on the network up to one hour + into the future; we use 45 minutes here to ensure this is still + fine even with some clock drift. That will also be more than + enough to be later than the confirmation block time assuming the + collateral is going to be mined "now". */ + mnbRet.sigTime = GetAdjustedTime() + 45 * 60; + LogPrint("masternode", "Using future sigtime for masternode broadcast: %d\n", mnbRet.sigTime); + } else { + mnbRet.sigTime = pindexConf->GetBlockTime(); + LogPrint("masternode", "Using collateral confirmation time for broadcast: %d\n", mnbRet.sigTime); + } } bool CMasternodeBroadcastFactory::Create( From 603efb9290cea64a910622d7c61b3ee4d355dcb4 Mon Sep 17 00:00:00 2001 From: Daniel Kraft Date: Wed, 13 Jan 2021 16:33:06 +0100 Subject: [PATCH 2/3] Allow just refreshing ping in mn broadcast. broadcaststartmasternode has a feature to refresh the ping's timestamp if an explicit signature is passed. This extends that feature to allow passing "update_ping" in place of the signature, in which case the signature contained in the mnb will be kept but the ping will still be refreshed. Not passing a second argument at all is still a way to just broadcast the message as-is. --- divi/src/RpcMasternodeFeatures.cpp | 7 +++++-- divi/src/RpcMasternodeFeatures.h | 7 +++++-- divi/src/rpcmasternode.cpp | 11 ++++++++++- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/divi/src/RpcMasternodeFeatures.cpp b/divi/src/RpcMasternodeFeatures.cpp index a55d9816f..173df116c 100644 --- a/divi/src/RpcMasternodeFeatures.cpp +++ b/divi/src/RpcMasternodeFeatures.cpp @@ -69,16 +69,19 @@ MasternodeCountData::MasternodeCountData( { } -bool RelayMasternodeBroadcast(std::string hexData, std::string signature) +bool RelayMasternodeBroadcast(const std::string& hexData, const std::string& signature, const bool updatePing) { auto& activeMasternode = mnModule.getActiveMasternode(); auto& mnodeman = mnModule.getMasternodeManager(); auto& masternodeSync = mnModule.getMasternodeSynchronization(); CMasternodeBroadcast mnb = readFromHex(hexData); + if(!signature.empty()) - { mnb.signature = ParseHex(signature); + + if (updatePing) + { if(activeMasternode.IsOurBroadcast(mnb,true)) { if(activeMasternode.UpdatePing(mnb.lastPing)) diff --git a/divi/src/RpcMasternodeFeatures.h b/divi/src/RpcMasternodeFeatures.h index 2288fc390..0262f4493 100644 --- a/divi/src/RpcMasternodeFeatures.h +++ b/divi/src/RpcMasternodeFeatures.h @@ -51,9 +51,12 @@ struct MasternodeCountData MasternodeCountData(); }; -bool RelayMasternodeBroadcast(std::string hexData,std::string signature = ""); +/** Relays a broadcast given in serialised form as hex string. If the signature + * is present, then it will replace the signature in the broadcast. If + * updatePing is true, then the masternode ping is re-signed freshly. */ +bool RelayMasternodeBroadcast(const std::string& hexData, const std::string& signature, bool updatePing); MasternodeStartResult StartMasternode(const CKeyStore& keyStore, std::string alias, bool deferRelay); ActiveMasternodeStatus GetActiveMasternodeStatus(); std::vector GetMasternodeList(std::string strFilter); MasternodeCountData GetMasternodeCounts(const CBlockIndex* chainTip); -#endif// RPC_MASTERNODE_FEATURES_H \ No newline at end of file +#endif// RPC_MASTERNODE_FEATURES_H diff --git a/divi/src/rpcmasternode.cpp b/divi/src/rpcmasternode.cpp index d210e0b73..62de9cd00 100644 --- a/divi/src/rpcmasternode.cpp +++ b/divi/src/rpcmasternode.cpp @@ -452,8 +452,17 @@ Value broadcaststartmasternode(const Array& params, bool fHelp) "\nResult:\n" "\"status\" (string) status of broadcast\n"); + bool updatePing = false; + std::string signature; + if(params.size()==2) + { + if (params[1].get_str() != "update_ping") + signature = params[1].get_str(); + updatePing = true; + } + Object result; - if(RelayMasternodeBroadcast(params[0].get_str(), (params.size()==2)? params[1].get_str():"" )) + if(RelayMasternodeBroadcast(params[0].get_str(), signature, updatePing)) { result.push_back(Pair("status", "success")); } From b1b58be3545b2e7b2fcd8c46e1327f12b03b75e9 Mon Sep 17 00:00:00 2001 From: Daniel Kraft Date: Mon, 26 Oct 2020 15:44:05 +0100 Subject: [PATCH 3/3] Regtest for proposed mn vaults. This adds a new regtest mnvaults.py, which manually implements the proposal for masternode vaults with a temporary key and a pre-signed unvault transaction. It sets up a masternode with a collateral at a temporary address, for which we need not retain the private key. Instead we just keep a pre-signed transaction to withdraw the funds to a specified address and the signed masternode broadcast in storage, and use them to first run the masternode and then unvault the funds. --- divi/qa/rpc-tests/mnvaults.py | 271 +++++++++++++++++++++++++++++++ divi/qa/rpc-tests/test_runner.py | 1 + 2 files changed, 272 insertions(+) create mode 100755 divi/qa/rpc-tests/mnvaults.py diff --git a/divi/qa/rpc-tests/mnvaults.py b/divi/qa/rpc-tests/mnvaults.py new file mode 100755 index 000000000..d86461fae --- /dev/null +++ b/divi/qa/rpc-tests/mnvaults.py @@ -0,0 +1,271 @@ +#!/usr/bin/env python3 +# Copyright (c) 2020 The DIVI developers +# Distributed under the MIT/X11 software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +# Tests the workflow for setting up a masternode vault (with prepared +# unvault tx and destroyed private key), running the masternode with it +# and unvaulting the funds later. +# +# We use seven nodes: +# - node 0 is used to fund and unvault the masternode +# - node 1 is the "hot" masternode +# - node 2 holds the "temporary" vault key and can sign with it +# (but we use it sparingly) +# - nodes 3-6 are just used to get above the "three full nodes" threshold + +from test_framework import BitcoinTestFramework +from util import * +from messages import * +from masternode import * + +from binascii import unhexlify +import time + + +class MnVaultsTest (BitcoinTestFramework): + + def __init__ (self): + super (MnVaultsTest, self).__init__ () + self.base_args = ["-debug", "-nolistenonion"] + self.cfg = None + + def setup_chain (self): + for i in range (7): + initialize_datadir (self.options.tmpdir, i) + + def setup_network (self, config_line=None, extra_args=[]): + # The masternode starts off, the others are online initially. + self.nodes = [ + start_node (0, self.options.tmpdir, extra_args=self.base_args), + None, + ] + [ + start_node (i, self.options.tmpdir, extra_args=self.base_args) + for i in [2, 3, 4, 5, 6] + ] + + # We want to work with mock times that are beyond the genesis + # block timestamp but before current time (so that nodes being + # started up and before they get on mocktime aren't rejecting + # the on-disk blockchain). + self.time = 1580000000 + assert self.time < time.time () + set_node_times (self.nodes, self.time) + + # Nodes 3-5 are connected between each other, and the cluster is + # also connected to nodes 0-2. + connect_nodes (self.nodes[3], 4) + connect_nodes (self.nodes[3], 5) + connect_nodes (self.nodes[3], 6) + connect_nodes (self.nodes[4], 5) + connect_nodes (self.nodes[4], 6) + connect_nodes (self.nodes[5], 6) + for i in [0, 2]: + connect_nodes (self.nodes[i], 3) + connect_nodes (self.nodes[i], 4) + connect_nodes (self.nodes[i], 5) + connect_nodes (self.nodes[i], 6) + + self.is_network_split = False + + def start_node (self, n): + """Starts node n with the proper arguments + and masternode config for it.""" + + args = self.base_args + if n == 1: + args.append ("-masternode") + args.append ("-masternodeprivkey=%s" % self.cfg.privkey) + + if self.cfg: + cfg = [self.cfg.line] + else: + cfg = [] + + self.nodes[n] = start_node (n, self.options.tmpdir, + extra_args=args, mn_config_lines=cfg) + self.nodes[n].setmocktime (self.time) + + for i in [3, 4, 5, 6]: + connect_nodes (self.nodes[n], i) + + sync_blocks (self.nodes) + + def stop_node (self, n): + stop_node (self.nodes[n], n) + self.nodes[n] = None + + def advance_time (self, dt=1): + """Advances mocktime by the given number of seconds.""" + + self.time += dt + set_node_times (self.nodes, self.time) + + def mine_blocks (self, n): + """Mines blocks with node 3.""" + + sync_mempools (self.nodes) + self.nodes[3].setgenerate(True, n) + sync_blocks (self.nodes) + + def run_test (self): + self.fund_vault () + self.start_masternode () + self.get_payments () + self.unvault () + + def fund_vault (self): + print ("Funding masternode vault...") + + self.nodes[0].setgenerate (True, 5) + sync_blocks (self.nodes) + self.mine_blocks (20) + + addr = self.nodes[2].getnewaddress () + privkey = self.nodes[2].dumpprivkey (addr) + + amount = 100 + txid = self.nodes[0].sendtoaddress (addr, amount) + raw = self.nodes[0].getrawtransaction (txid, 1) + vout = None + for i in range (len (raw["vout"])): + o = raw["vout"][i] + if addr in o["scriptPubKey"]["addresses"]: + vout = i + break + assert vout is not None + + unvaultAddr = self.nodes[0].getnewaddress ("unvaulted") + data = self.nodes[0].validateaddress (unvaultAddr) + + tx = CTransaction () + tx.vin.append (CTxIn (COutPoint (txid=txid, n=vout))) + tx.vout.append (CTxOut (amount * COIN, unhexlify (data["scriptPubKey"]))) + unsigned = ToHex (tx) + + validated = self.nodes[0].validateaddress (addr) + script = validated["scriptPubKey"] + prevtx = [{"txid": txid, "vout": vout, "scriptPubKey": script}] + signed = self.nodes[0].signrawtransaction (unsigned, prevtx, [privkey], + "SINGLE|ANYONECANPAY") + assert_equal (signed["complete"], True) + self.unvaultTx = signed["hex"] + + self.cfg = fund_masternode (self.nodes[0], "mn", "copper", txid, + "localhost:%d" % p2p_port (1)) + # FIXME: Use reward address from node 0. + self.cfg.rewardAddr = addr + + for i in [0, 2]: + self.stop_node (i) + self.start_node (i) + + # Prepare the masternode activation broadcast, without actually + # relaying it to the network. After this is done, node 2 with the + # "temporary" private key is no longer needed at all, and can be + # shut down for the rest of the test. + bc = self.nodes[2].startmasternode ("mn", True) + assert_equal (bc["status"], "success") + self.broadcast = bc["broadcastData"] + self.stop_node (2) + + self.mine_blocks (20) + + def start_masternode (self): + print ("Starting masternode from vault...") + + # Advance some time to simulate starting the node later (e.g. also when + # restarting it as necessary during operation). + for _ in range (100): + self.advance_time (100) + + # Due to advancing the time without having any masternodes, sync will + # have failed on the nodes that are up. Reset the sync now to make + # sure they will then properly sync together with the other nodes + # after we start our masternode. + for n in self.nodes: + if n is not None: + n.mnsync ("reset") + + # Now start and activate the masternode based on the stored + # broadcast message. + self.start_node (1) + bc = self.nodes[1].broadcaststartmasternode (self.broadcast, "update_ping") + assert_equal (bc["status"], "success") + + # Finish masternode sync. + for _ in range (100): + self.advance_time () + for n in self.nodes: + if n is not None: + status = n.mnsync ("status") + assert_equal (status["RequestedMasternodeAssets"], 999) + + # Check that the masternode is indeed active. + data = self.nodes[1].getmasternodestatus () + assert_equal (data["status"], 4) + assert_equal (data["message"], "Masternode successfully started") + + def get_payments (self): + print ("Receiving masternode payments...") + + # For payments, the masternode needs to be active at least 8000 seconds + # and we also need at least 100 blocks. We also need some extra + # leeway in the time due to the one hour we add to the current time + # when signing a collateral that is not yet 15 times confirmed. + self.mine_blocks (100) + for _ in range (150): + self.advance_time (100) + + cnt = self.nodes[3].getmasternodecount () + assert_equal (cnt["total"], 1) + assert_equal (cnt["enabled"], 1) + assert_equal (cnt["inqueue"], 1) + + # Mine some blocks, but advance the time in between and do it + # one by one so the masternode winners can get broadcast between + # blocks and such. + for _ in range (10): + self.mine_blocks (1) + self.advance_time (10) + + # Check that some payments were made. + winners = self.nodes[3].getmasternodewinners () + found = False + for w in winners: + if w["winner"]["address"] == self.cfg.rewardAddr: + found = True + break + assert_equal (found, True) + + # FIXME: Check in wallet when we have a custom reward address. + + def unvault (self): + print ("Unvaulting the funds...") + + # The prepared unvaulting tx is just a single input/output pair + # with no fee attached. To add the transaction fee, we add another + # input and output, which is fine due to the SINGLE|ANYONECANPAY signature + # that we used. + + fee = Decimal ('0.10000000') + inp = self.nodes[0].listunspent ()[0] + change = int ((inp["amount"] - fee) * COIN) + assert_greater_than (change, 0) + changeAddr = self.nodes[0].getnewaddress () + data = self.nodes[0].validateaddress (changeAddr) + + tx = FromHex (CTransaction (), self.unvaultTx) + tx.vin.append (CTxIn (COutPoint (txid=inp["txid"], n=inp["vout"]))) + tx.vout.append (CTxOut (change, unhexlify (data["scriptPubKey"]))) + partial = ToHex (tx) + + signed = self.nodes[0].signrawtransaction (partial) + assert_equal (signed["complete"], True) + self.nodes[0].sendrawtransaction (signed["hex"]) + self.mine_blocks (1) + assert_equal (self.nodes[0].getbalance ("unvaulted"), 100) + + +if __name__ == '__main__': + MnVaultsTest ().main () diff --git a/divi/qa/rpc-tests/test_runner.py b/divi/qa/rpc-tests/test_runner.py index 88d870741..735fc7fb6 100755 --- a/divi/qa/rpc-tests/test_runner.py +++ b/divi/qa/rpc-tests/test_runner.py @@ -90,6 +90,7 @@ 'MnAreSafeToRestart.py', 'mncollateral.py', 'mnoperation.py', + 'mnvaults.py', 'op_meta.py', 'proxy_test.py', 'receivedby.py',