From bf41b31b2b0d91c5b56e7d463256607eda9b2293 Mon Sep 17 00:00:00 2001 From: gladcow Date: Mon, 3 Sep 2018 17:00:21 +0300 Subject: [PATCH 01/25] support set of keys to sign spork --- src/spork.cpp | 16 ++++++++++++---- src/spork.h | 4 ++-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/spork.cpp b/src/spork.cpp index cc60cf7ff2b2..6305c2bab5df 100644 --- a/src/spork.cpp +++ b/src/spork.cpp @@ -102,7 +102,13 @@ void CSporkManager::ProcessSpork(CNode* pfrom, const std::string& strCommand, CD } } - if(!spork.CheckSignature(sporkPubKeyID, IsSporkActive(SPORK_6_NEW_SIGS))) { + bool found = false; + for(const auto& keyid: sporkPubKeyIDs) { + if(spork.CheckSignature(keyid, IsSporkActive(SPORK_6_NEW_SIGS))) { + found = true; + } + } + if(!found) { LOCK(cs_main); LogPrintf("CSporkManager::ProcessSpork -- ERROR: invalid signature\n"); Misbehaving(pfrom->GetId(), 100); @@ -260,10 +266,12 @@ bool CSporkManager::GetSporkByHash(const uint256& hash, CSporkMessage &sporkRet) bool CSporkManager::SetSporkAddress(const std::string& strAddress) { LOCK(cs); CBitcoinAddress address(strAddress); - if (!address.IsValid() || !address.GetKeyID(sporkPubKeyID)) { + CKeyID keyid; + if (!address.IsValid() || !address.GetKeyID(keyid)) { LogPrintf("CSporkManager::SetSporkAddress -- Failed to parse spork address\n"); return false; } + sporkPubKeyIDs.insert(keyid); return true; } @@ -276,8 +284,8 @@ bool CSporkManager::SetPrivKey(const std::string& strPrivKey) return false; } - if (pubKey.GetID() != sporkPubKeyID) { - LogPrintf("CSporkManager::SetPrivKey -- New private key does not belong to spork address\n"); + if (sporkPubKeyIDs.find(pubKey.GetID()) == sporkPubKeyIDs.end()) { + LogPrintf("CSporkManager::SetPrivKey -- New private key does not belong to spork addresses\n"); return false; } diff --git a/src/spork.h b/src/spork.h index efccd232f276..13225f007dcd 100644 --- a/src/spork.h +++ b/src/spork.h @@ -93,7 +93,7 @@ class CSporkManager std::map mapSporksByHash; std::map mapSporksActive; - CKeyID sporkPubKeyID; + std::set sporkPubKeyIDs; CKey sporkPrivKey; public: @@ -114,7 +114,7 @@ class CSporkManager strVersion = SERIALIZATION_VERSION_STRING; READWRITE(strVersion); } - READWRITE(sporkPubKeyID); + READWRITE(sporkPubKeyIDs); READWRITE(mapSporksByHash); READWRITE(mapSporksActive); // we don't serialize private key to prevent its leakage From d449c00614c0f07f3286f3cc999ab190282316b5 Mon Sep 17 00:00:00 2001 From: gladcow Date: Mon, 3 Sep 2018 17:24:24 +0300 Subject: [PATCH 02/25] several addresses support in -sporkkey --- src/init.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index d33dfd96db25..d56629ffc8cc 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1398,8 +1398,13 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler) threadGroup.create_thread(&ThreadScriptCheck); } - if (!sporkManager.SetSporkAddress(GetArg("-sporkaddr", Params().SporkAddress()))) - return InitError(_("Invalid spork address specified with -sporkaddr")); + std::string strSporkAddresses = GetArg("-sporkaddr", Params().SporkAddress()); + std::vector vSporkAddresses; + boost::split(vSporkAddresses, strSporkAddresses, boost::is_any_of(":")); + for(const auto& address: vSporkAddresses) { + if (!sporkManager.SetSporkAddress(address)) + return InitError(_("Invalid spork address specified with -sporkaddr")); + } if (IsArgSet("-sporkkey")) // spork priv key { From 47249e531fc9e7142b661dc54cd217b998bb83d5 Mon Sep 17 00:00:00 2001 From: gladcow Date: Wed, 5 Sep 2018 18:49:57 +0300 Subject: [PATCH 03/25] tests for multykey sporks --- qa/rpc-tests/multikeysporks.py | 153 +++++++++++++++++++++++++++++++++ 1 file changed, 153 insertions(+) create mode 100755 qa/rpc-tests/multikeysporks.py diff --git a/qa/rpc-tests/multikeysporks.py b/qa/rpc-tests/multikeysporks.py new file mode 100755 index 000000000000..7520c3eb1fa7 --- /dev/null +++ b/qa/rpc-tests/multikeysporks.py @@ -0,0 +1,153 @@ +#!/usr/bin/env python3 +# Copyright (c) 2018 The Dash Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +from test_framework.mininode import * +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import * +from time import * +''' +''' + +class MultiKeySporkTest(BitcoinTestFramework): + def __init__(self): + super().__init__() + self.num_nodes = 3 + self.setup_clean_chain = True + self.is_network_split = False + + def setup_network(self): + self.nodes = [] + + # secret(base58): 931wyuRNVYvhg18Uu9bky5Qg1z4QbxaJ7fefNBzjBPiLRqcd33F + # keyid(hex): 60f0f57f71f0081f1aacdd8432340a33a526f91b + # address(base58): yNsMZhEhYqv14TgdYb1NS2UmNZjE8FSJxa + # secret(base58): 91vbXGMSWKGHom62986XtL1q2mQDA12ngcuUNNe5NfMSj44j7g3 + # keyid(hex): 43dff2b09de2f904f688ec14ee6899087b889ad0 + # address(base58): yfLSXFfipnkgYioD6L8aUNyfRgEBuJv48h + # secret(base58): 92bxUjPT5AhgXuXJwfGGXqhomY2SdQ55MYjXyx9DZNxCABCSsRH + # keyid(hex): d9aa5fa00cce99101a4044e65dc544d1579890de + # address(base58): ygcG5S2pQz2U1UAaHvU6EznKZW7yapKMA7 + # secret(base58): 934yPXiVGf4RCY2qTs2Bt5k3TEtAiAg12sMxCt8yVWbSU7p3fuD + # keyid(hex): 0b23935ce0bea3b997a334f6fa276c9fa17687b2 + # address(base58): ycbRQWbovrhQMTuxg9p4LAuW5SCMAKqPrn + # secret(base58): 92Cxwia363Wg2qGF1fE5z4GKi8u7r1nrWQXdtsj2ACZqaDPSihD + # keyid(hex): 1d1098b2b1f759b678a0a7a098637a9b898adcac + # address(base58): yc5TGfcHYoLCrcbVy4umsiDjsYUn39vLui + # secret(base58): 92CKXK5dbysc4zQnNsvzgDpiBGgo67VPsRDAFsF4gGwbiZpgLMv + # keyid(hex): 0e9b6bdc15dc3afb63d1ffc68cc5534a4caaeaa6 + # address(base58): ybY28sZzX3Jy3UNcZ72MNubcVEQXcAN3Vh + # secret(base58): 91tDdZPR5gfCNfkVN8Pym1YyaaMHhf9MyVS12LRuN2MoAgSeDcE + # keyid(hex): 99a001480e0332d1a0d30078bde4d6fe3a178cfa + # address(base58): yjADbqTD649DaJnRGZYrXW1fnfQ4ciDcXH + # secret(base58): 928dGxqesKbqKpfAJXRnGd4gy3GtbtfM1WHTQBisvWDVJZ1Vxyi + # keyid(hex): 3ae21cfc6017c65e80d7ec45310c22f6dc705bdd + # address(base58): ygVsj29xcd1B3iMbs6A2CY9mscC8cBXEZX + # secret(base58): 91ynY1Q7iaXSbuNq9nQ2u6K2i2ryX3em5wEKEofXTjhsrDL2jY8 + # keyid(hex): 551a3c676574d355fc3c314955f3b440cf57b245 + # address(base58): ySfy7vpGjiG27PSr8XKCKr4y3mY1D9U76y + # secret(base58): 92ffBcNsSLyEKT5CmqGMEmYHJBUTHoGHjFy9Y3XNPSLbxi4DVWD + # keyid(hex): 8df696c8d4e69a898ce7d599559e32268b28ccbb + # address(base58): ydSRnFKrboF3L544AWQfG79mcXMC7ZtShL + # secret(base58): 92cDaZ9GGSLnJsaEbCDewrXA3RcmDCG9MZAeJZTmMnpgLQAsyHr + # keyid(hex): 92af1b1ebe0107dcc648aa150905e1229777225d + # address(base58): yUotyrzs4H9DJ7K3Y7ccgeAUh442FEkrKw + # secret(base58): 92PGUYidWoq9qBoyEEAeYQ12ZHWBCR5YusLenpa7vUGFXPfgqXx + # keyid(hex): 093a9963ac777502251dd98c22ea2c71aaa2fee3 + # address(base58): yh6yH8LJ4eTFWAWVNdCDaQa53nXpddvmZp + # secret(base58): 91tbFNAjdSeUYZAG3QhF91Zh1LGgZcFpq8zrGEH1MHKycFLtDrL + # keyid(hex): 5fcfae286651d85e6ad2eff5a38b148d806f9beb + # address(base58): yhoDr8n8DzfLR7VQGcLqpmq29WJ5Z2eYNi + # secret(base58): 92EMsUxGgk1372edF7iNNzQSN3iLsRkUSmZzQKGwLd4CrC3YUUH + # keyid(hex): 4287e28642ebfd3d48e86888a6bb6e7a9af22910 + # address(base58): yMnuur7LEVgrYFczsNfx8W61eAgGYAmTVG + # secret(base58): 92VRomWTLSRRHWR3NwbUAhrPuwfMyciLcDWR9ebJRbSFoEj2RLg + # keyid(hex): 6d3fcee4b4b92488ed20171b61c2951ea51cbeb4 + # address(base58): yco8DwPVN6Wgk1nCmakkLYCGzXy9fgu5zC + # secret(base58): 92swaJmVB2c4kLvKt7CihNneE7MxfQnv7eRV248F7t4CmVWsaLe + # keyid(hex): ab87f6995077f4bdd6233bc2992611674d7c6594 + # address(base58): yZr6Rw8GiZwD6GMdC2fuP7c8qkFQPgWVRX + # secret(base58): 923NQ12YSeVs4RfaCiCqTE9iNAD4dobSmuEhnV7x6q7styQYfKb + # keyid(hex): 2318b0dab38d676aae483ab0f55f2d839a1031b2 + # address(base58): ycZduSBR8W8Gpb9ec1AocYF1c4Q3E6wjeM + # secret(base58): 92tMn4LCFGTthXpdiygJGjQbJYmYY78TiXHFWYTUu9uZiLnHn8W + # keyid(hex): ed73329177896a951b0eb30dd962d3e661c0476e + # address(base58): yWNZ8gibbLMAyuXX4i1ZNaxmADW59Rz3zJ + # secret(base58): 92RgZKa9TYVRJZmj4Nme3jBRn77rD2BmQNJZJkWmN67UVpRXWjW + # keyid(hex): a7845c35a6b608c7c3a9a9388e06370fa3f5d32b + # address(base58): yQKBpf5bCZ3ctmYKFEPqdeLTg4SwuciMo7 + # secret(base58): 92mNuCHJ2kWcxGkAus1P6UDPHPFZDPNTzMMf72YujjXtdmbiUCk + # keyid(hex): dfe5fcd9bee0b871ad221a4cf3532c2848314701 + # address(base58): yLSCowJMnKfrmniiX9RD2bxvbHwvzgyKvQ + # secret(base58): 92cyrqe2iYqw6ni5MQvBAcSQNAAApmeToeA7qzawEcxLDHufGK3 + # keyid(hex): 088f9e1ee1a06e0b3fed714beee97e90c035ed76 + # address(base58): yXAGkUbnEtEeY2cdZnt9zzD4mCnmLeCZfU + # secret(base58): 93UMgtWGffnog5yYyyiSwzHCP7FX8wmURQa19f7oT95JQpLu3LN + # keyid(hex): 98396e03c6e20e44ce776e07e92cb533cb450e7f + # address(base58): yXuFmFBfipN4pDWbiQLLNAaT2HcTGLLAkJ + + + self.nodes.append(start_node(0, self.options.tmpdir, + ["-debug", "-sporkkey=931wyuRNVYvhg18Uu9bky5Qg1z4QbxaJ7fefNBzjBPiLRqcd33F", + "-sporkaddr=ygcG5S2pQz2U1UAaHvU6EznKZW7yapKMA7:yfLSXFfipnkgYioD6L8aUNyfRgEBuJv48h:yNsMZhEhYqv14TgdYb1NS2UmNZjE8FSJxa", + "-minsporkkeys=2"])) + self.nodes.append(start_node(1, self.options.tmpdir, + ["-debug", "-sporkkey=91vbXGMSWKGHom62986XtL1q2mQDA12ngcuUNNe5NfMSj44j7g3", + "-sporkaddr=ygcG5S2pQz2U1UAaHvU6EznKZW7yapKMA7:yfLSXFfipnkgYioD6L8aUNyfRgEBuJv48h:yNsMZhEhYqv14TgdYb1NS2UmNZjE8FSJxa", + "-minsporkkeys=2"])) + self.nodes.append(start_node(2, self.options.tmpdir, + ["-debug", + "-sporkaddr=ygcG5S2pQz2U1UAaHvU6EznKZW7yapKMA7:yfLSXFfipnkgYioD6L8aUNyfRgEBuJv48h:yNsMZhEhYqv14TgdYb1NS2UmNZjE8FSJxa", + "-minsporkkeys=2"])) + # connect nodes at start + connect_nodes(self.nodes[0], 1) + connect_nodes(self.nodes[0], 2) + connect_nodes(self.nodes[1], 2) + + def get_test_spork_state(self, node): + info = node.spork('active') + # use InstantSend spork for tests + return info['SPORK_2_INSTANTSEND_ENABLED'] + + def set_test_spork_state(self, node, state): + if state: + value = 0 + else: + value = 4070908800 + # use InstantSend spork for tests + node.spork('SPORK_2_INSTANTSEND_ENABLED', value) + + def wait_for_test_spork_state(self, node, state): + start = time() + got_state = False + while True: + if self.get_test_spork_state(node) == state: + got_state = True + break + if time() > start + 10: + break + sleep(0.1) + return got_state + + def run_test(self): + # check test spork default state + for node in self.nodes: + assert(self.get_test_spork_state(node)) + + # first signer turns off spork + self.set_test_spork_state(self.nodes[0], False) + # spork change requires at least 2 signers + for node in self.nodes: + assert(not self.wait_for_test_spork_state(node, False)) + + # second signer turns off spork + self.set_test_spork_state(self.nodes[1], False) + # now spork state is changed + for node in self.nodes: + assert(self.wait_for_test_spork_state(node, False)) + + + +if __name__ == '__main__': + MultiKeySporkTest().main() From 6fc399803066b0c5dbbc151a0d34b0f7d204658f Mon Sep 17 00:00:00 2001 From: gladcow Date: Thu, 6 Sep 2018 15:13:35 +0300 Subject: [PATCH 04/25] command line option -minsporkkeys --- src/chainparams.cpp | 12 ++++++++---- src/chainparams.h | 6 ++++-- src/init.cpp | 8 +++++++- src/spork.cpp | 10 ++++++++++ src/spork.h | 2 ++ 5 files changed, 31 insertions(+), 7 deletions(-) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index d2a9da6db168..221fc7e7960c 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -237,7 +237,8 @@ class CMainParams : public CChainParams { nPoolMaxTransactions = 3; nFulfilledRequestExpireTime = 60*60; // fulfilled requests expire in 1 hour - strSporkAddress = "Xgtyuk76vhuFW2iT7UAiHgNdWXCf3J34wh"; + strSporkAddresses = "Xgtyuk76vhuFW2iT7UAiHgNdWXCf3J34wh"; + nMinSporkKeys = 1; checkpointData = (CCheckpointData) { boost::assign::map_list_of @@ -391,7 +392,8 @@ class CTestNetParams : public CChainParams { nPoolMaxTransactions = 3; nFulfilledRequestExpireTime = 5*60; // fulfilled requests expire in 5 minutes - strSporkAddress = "yjPtiKh2uwk3bDutTEA2q9mCtXyiZRWn55"; + strSporkAddresses = "yjPtiKh2uwk3bDutTEA2q9mCtXyiZRWn55"; + nMinSporkKeys = 1; checkpointData = (CCheckpointData) { boost::assign::map_list_of @@ -535,7 +537,8 @@ class CDevNetParams : public CChainParams { nPoolMaxTransactions = 3; nFulfilledRequestExpireTime = 5*60; // fulfilled requests expire in 5 minutes - strSporkAddress = "yjPtiKh2uwk3bDutTEA2q9mCtXyiZRWn55"; + strSporkAddresses = "yjPtiKh2uwk3bDutTEA2q9mCtXyiZRWn55"; + nMinSporkKeys = 1; checkpointData = (CCheckpointData) { boost::assign::map_list_of @@ -641,7 +644,8 @@ class CRegTestParams : public CChainParams { nFulfilledRequestExpireTime = 5*60; // fulfilled requests expire in 5 minutes // privKey: cP4EKFyJsHT39LDqgdcB43Y3YXjNyjb5Fuas1GQSeAtjnZWmZEQK - strSporkAddress = "yj949n1UH6fDhw6HtVE5VMj2iSTaSWBMcW"; + strSporkAddresses = "yj949n1UH6fDhw6HtVE5VMj2iSTaSWBMcW"; + nMinSporkKeys = 1; checkpointData = (CCheckpointData){ boost::assign::map_list_of diff --git a/src/chainparams.h b/src/chainparams.h index fa67c9b73a94..33af093c6ef7 100644 --- a/src/chainparams.h +++ b/src/chainparams.h @@ -88,7 +88,8 @@ class CChainParams const ChainTxData& TxData() const { return chainTxData; } int PoolMaxTransactions() const { return nPoolMaxTransactions; } int FulfilledRequestExpireTime() const { return nFulfilledRequestExpireTime; } - const std::string& SporkAddress() const { return strSporkAddress; } + const std::string& SporkAddresses() const { return strSporkAddresses; } + int MinSporkKeys() const { return nMinSporkKeys; } protected: CChainParams() {} @@ -116,7 +117,8 @@ class CChainParams ChainTxData chainTxData; int nPoolMaxTransactions; int nFulfilledRequestExpireTime; - std::string strSporkAddress; + std::string strSporkAddresses; + int nMinSporkKeys; }; /** diff --git a/src/init.cpp b/src/init.cpp index d56629ffc8cc..27fbfe849433 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -570,6 +570,7 @@ std::string HelpMessage(HelpMessageMode mode) AppendParamsHelpMessages(strUsage, showDebug); strUsage += HelpMessageOpt("-litemode=", strprintf(_("Disable all Dash specific functionality (Masternodes, PrivateSend, InstantSend, Governance) (0-1, default: %u)"), 0)); strUsage += HelpMessageOpt("-sporkaddr=", strprintf(_("Override spork address. Only useful for regtest and devnet. Using this on mainnet or testnet will ban you."))); + strUsage += HelpMessageOpt("-minsporkkeys=", strprintf(_("Overrides minimum spork signers to change spork value. Only useful for regtest and devnet. Using this on mainnet or testnet will ban you."))); strUsage += HelpMessageGroup(_("Masternode options:")); strUsage += HelpMessageOpt("-masternode=", strprintf(_("Enable the client to act as a masternode (0-1, default: %u)"), 0)); @@ -1398,7 +1399,7 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler) threadGroup.create_thread(&ThreadScriptCheck); } - std::string strSporkAddresses = GetArg("-sporkaddr", Params().SporkAddress()); + std::string strSporkAddresses = GetArg("-sporkaddr", Params().SporkAddresses()); std::vector vSporkAddresses; boost::split(vSporkAddresses, strSporkAddresses, boost::is_any_of(":")); for(const auto& address: vSporkAddresses) { @@ -1406,6 +1407,11 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler) return InitError(_("Invalid spork address specified with -sporkaddr")); } + int minsporkkeys = GetArg("-minsporkkeys", Params().MinSporkKeys()); + if(!sporkManager.SetMinSporkKeys(minsporkkeys)) + return InitError(_("Invalid minimum number of spork signers specified with -minsporkkeys")); + + if (IsArgSet("-sporkkey")) // spork priv key { if (!sporkManager.SetPrivKey(GetArg("-sporkkey", ""))) diff --git a/src/spork.cpp b/src/spork.cpp index 6305c2bab5df..59d40a2efe13 100644 --- a/src/spork.cpp +++ b/src/spork.cpp @@ -275,6 +275,16 @@ bool CSporkManager::SetSporkAddress(const std::string& strAddress) { return true; } +bool CSporkManager::SetMinSporkKeys(int minSporkKeys) +{ + if ((minSporkKeys < 1) || (minSporkKeys > sporkPubKeyIDs.size())) { + LogPrintf("CSporkManager::SetSporkAddress -- Invalid min spork signers number: %d\n", minSporkKeys); + return false; + } + nMinSporkKeys = minSporkKeys; + return true; +} + bool CSporkManager::SetPrivKey(const std::string& strPrivKey) { CKey key; diff --git a/src/spork.h b/src/spork.h index 13225f007dcd..d62f6cbc9e79 100644 --- a/src/spork.h +++ b/src/spork.h @@ -94,6 +94,7 @@ class CSporkManager std::map mapSporksActive; std::set sporkPubKeyIDs; + int nMinSporkKeys; CKey sporkPrivKey; public: @@ -135,6 +136,7 @@ class CSporkManager bool GetSporkByHash(const uint256& hash, CSporkMessage &sporkRet); bool SetSporkAddress(const std::string& strAddress); + bool SetMinSporkKeys(int minSporkKeys); bool SetPrivKey(const std::string& strPrivKey); std::string ToString() const; From 656fad27ee3fb2d45d7283954373f66bfa390f14 Mon Sep 17 00:00:00 2001 From: gladcow Date: Fri, 7 Sep 2018 08:55:55 +0300 Subject: [PATCH 05/25] make spork active only after given number of signers --- src/spork.cpp | 105 +++++++++++++++++++++++++++++++++++++------------- src/spork.h | 3 +- 2 files changed, 81 insertions(+), 27 deletions(-) diff --git a/src/spork.cpp b/src/spork.cpp index 59d40a2efe13..e282b36d0675 100644 --- a/src/spork.cpp +++ b/src/spork.cpp @@ -31,6 +31,31 @@ std::map mapSporkDefaults = { {SPORK_16_INSTANTSEND_AUTOLOCKS, 4070908800ULL}, // OFF }; +bool CSporkManager::SporkValueIsActive(int sporkID, int64_t &activeValue) const +{ + LOCK(cs); + if (!mapSporksActive.count(sporkID)) + return false; + + // calc how many values we have and how many signers vote for every value + std::map value_counts; + for (const auto& pair: mapSporksActive.at(sporkID)) { + value_counts[pair.second.nValue]++; + } + + // check if any value has enough signer votes + bool found = false; + for (const auto& value_data: value_counts) { + if(value_data.second >= nMinSporkKeys) { + if (found) return false; // several active values, no consensus among signers + found = true; + activeValue = value_data.first; + } + } + + return found; +} + void CSporkManager::Clear() { LOCK(cs); @@ -88,24 +113,13 @@ void CSporkManager::ProcessSpork(CNode* pfrom, const std::string& strCommand, CD if(!chainActive.Tip()) return; strLogMsg = strprintf("SPORK -- hash: %s id: %d value: %10d bestHeight: %d peer=%d", hash.ToString(), spork.nSporkID, spork.nValue, chainActive.Height(), pfrom->id); } - { - LOCK(cs); // make sure to not lock this together with cs_main - if (mapSporksActive.count(spork.nSporkID)) { - if (mapSporksActive[spork.nSporkID].nTimeSigned >= spork.nTimeSigned) { - LogPrint("spork", "%s seen\n", strLogMsg); - return; - } else { - LogPrintf("%s updated\n", strLogMsg); - } - } else { - LogPrintf("%s new\n", strLogMsg); - } - } bool found = false; + CKeyID signer; for(const auto& keyid: sporkPubKeyIDs) { if(spork.CheckSignature(keyid, IsSporkActive(SPORK_6_NEW_SIGS))) { found = true; + signer = keyid; } } if(!found) { @@ -115,20 +129,44 @@ void CSporkManager::ProcessSpork(CNode* pfrom, const std::string& strCommand, CD return; } + { + LOCK(cs); // make sure to not lock this together with cs_main + if (mapSporksActive.count(spork.nSporkID)) { + if (mapSporksActive[spork.nSporkID].count(signer)) { + if (mapSporksActive[spork.nSporkID][signer].nTimeSigned >= spork.nTimeSigned) { + LogPrint("spork", "%s seen\n", strLogMsg); + return; + } else { + LogPrintf("%s updated\n", strLogMsg); + } + } else { + LogPrintf("%s new signer\n", strLogMsg); + } + } else { + LogPrintf("%s new\n", strLogMsg); + } + } + + { LOCK(cs); // make sure to not lock this together with cs_main mapSporksByHash[hash] = spork; - mapSporksActive[spork.nSporkID] = spork; + mapSporksActive[spork.nSporkID][signer] = spork; } spork.Relay(connman); //does a task if needed - ExecuteSpork(spork.nSporkID, spork.nValue); + int64_t activeValue = 0; + if (SporkValueIsActive(spork.nSporkID, activeValue)) { + ExecuteSpork(spork.nSporkID, activeValue); + } } else if (strCommand == NetMsgType::GETSPORKS) { LOCK(cs); // make sure to not lock this together with cs_main for (const auto& pair : mapSporksActive) { - connman.PushMessage(pfrom, CNetMsgMaker(pfrom->GetSendVersion()).Make(NetMsgType::SPORK, pair.second)); + for (const auto& signer_spork: pair.second) { + connman.PushMessage(pfrom, CNetMsgMaker(pfrom->GetSendVersion()).Make(NetMsgType::SPORK, signer_spork.second)); + } } } @@ -168,10 +206,22 @@ bool CSporkManager::UpdateSpork(int nSporkID, int64_t nValue, CConnman& connman) CSporkMessage spork = CSporkMessage(nSporkID, nValue, GetAdjustedTime()); if(spork.Sign(sporkPrivKey, IsSporkActive(SPORK_6_NEW_SIGS))) { + bool found = false; + CKeyID signer; + for(const auto& keyid: sporkPubKeyIDs) { + if(spork.CheckSignature(keyid, IsSporkActive(SPORK_6_NEW_SIGS))) { + found = true; + signer = keyid; + } + } + if (!found) { + LogPrintf("CSporkManager::UpdateSpork: failed to find keyid for private key\n"); + return false; + } spork.Relay(connman); LOCK(cs); mapSporksByHash[spork.GetHash()] = spork; - mapSporksActive[nSporkID] = spork; + mapSporksActive[nSporkID][signer] = spork; return true; } @@ -184,15 +234,16 @@ bool CSporkManager::IsSporkActive(int nSporkID) LOCK(cs); int64_t r = -1; - if(mapSporksActive.count(nSporkID)){ - r = mapSporksActive[nSporkID].nValue; - } else if (mapSporkDefaults.count(nSporkID)) { - r = mapSporkDefaults[nSporkID]; - } else { - LogPrint("spork", "CSporkManager::IsSporkActive -- Unknown Spork ID %d\n", nSporkID); - r = 4070908800ULL; // 2099-1-1 i.e. off by default + if(SporkValueIsActive(nSporkID, r)){ + return r < GetAdjustedTime(); } + if (mapSporkDefaults.count(nSporkID)) { + return mapSporkDefaults[nSporkID] < GetAdjustedTime(); + } + + LogPrint("spork", "CSporkManager::IsSporkActive -- Unknown Spork ID %d\n", nSporkID); + r = 4070908800ULL; // 2099-1-1 i.e. off by default return r < GetAdjustedTime(); } @@ -200,8 +251,10 @@ bool CSporkManager::IsSporkActive(int nSporkID) int64_t CSporkManager::GetSporkValue(int nSporkID) { LOCK(cs); - if (mapSporksActive.count(nSporkID)) - return mapSporksActive[nSporkID].nValue; + int64_t r = -1; + if(SporkValueIsActive(nSporkID, r)){ + return r; + } if (mapSporkDefaults.count(nSporkID)) { return mapSporkDefaults[nSporkID]; diff --git a/src/spork.h b/src/spork.h index d62f6cbc9e79..2f849558cd26 100644 --- a/src/spork.h +++ b/src/spork.h @@ -91,12 +91,13 @@ class CSporkManager mutable CCriticalSection cs; std::map mapSporksByHash; - std::map mapSporksActive; + std::map > mapSporksActive; std::set sporkPubKeyIDs; int nMinSporkKeys; CKey sporkPrivKey; + bool SporkValueIsActive(int sporkID, int64_t& activeValue) const; public: CSporkManager() {} From e327194a22a5cce2b17a9d2b2f598439cbe8f332 Mon Sep 17 00:00:00 2001 From: gladcow Date: Fri, 7 Sep 2018 17:37:03 +0300 Subject: [PATCH 06/25] use signature in spork hash calculation --- src/spork.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/spork.h b/src/spork.h index 2f849558cd26..86f1bedfd86b 100644 --- a/src/spork.h +++ b/src/spork.h @@ -70,9 +70,7 @@ class CSporkMessage READWRITE(nSporkID); READWRITE(nValue); READWRITE(nTimeSigned); - if (!(s.GetType() & SER_GETHASH)) { - READWRITE(vchSig); - } + READWRITE(vchSig); } uint256 GetHash() const; From 9724bb8d93e34346ac2aea0c5d3565e6749fee35 Mon Sep 17 00:00:00 2001 From: gladcow Date: Wed, 12 Sep 2018 14:18:56 +0300 Subject: [PATCH 07/25] test for new and old spork messages interaction --- qa/rpc-tests/multikeysporks.py | 96 +++++++++++++--------------------- 1 file changed, 36 insertions(+), 60 deletions(-) diff --git a/qa/rpc-tests/multikeysporks.py b/qa/rpc-tests/multikeysporks.py index 7520c3eb1fa7..77c794b979e7 100755 --- a/qa/rpc-tests/multikeysporks.py +++ b/qa/rpc-tests/multikeysporks.py @@ -7,13 +7,23 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import * from time import * + ''' +multikeysporks.py + +Test logic for several signer keys usage for spork broadcast. + +We set 5 possible keys for sporks signing and set minimum +required signers to 2. We check 1 siger can't set the spork +value, any 2 signers can change spork value and other 2 signers +can change it again. ''' + class MultiKeySporkTest(BitcoinTestFramework): def __init__(self): super().__init__() - self.num_nodes = 3 + self.num_nodes = 5 self.setup_clean_chain = True self.is_network_split = False @@ -23,87 +33,47 @@ def setup_network(self): # secret(base58): 931wyuRNVYvhg18Uu9bky5Qg1z4QbxaJ7fefNBzjBPiLRqcd33F # keyid(hex): 60f0f57f71f0081f1aacdd8432340a33a526f91b # address(base58): yNsMZhEhYqv14TgdYb1NS2UmNZjE8FSJxa + # secret(base58): 91vbXGMSWKGHom62986XtL1q2mQDA12ngcuUNNe5NfMSj44j7g3 # keyid(hex): 43dff2b09de2f904f688ec14ee6899087b889ad0 # address(base58): yfLSXFfipnkgYioD6L8aUNyfRgEBuJv48h + # secret(base58): 92bxUjPT5AhgXuXJwfGGXqhomY2SdQ55MYjXyx9DZNxCABCSsRH # keyid(hex): d9aa5fa00cce99101a4044e65dc544d1579890de # address(base58): ygcG5S2pQz2U1UAaHvU6EznKZW7yapKMA7 + # secret(base58): 934yPXiVGf4RCY2qTs2Bt5k3TEtAiAg12sMxCt8yVWbSU7p3fuD # keyid(hex): 0b23935ce0bea3b997a334f6fa276c9fa17687b2 # address(base58): ycbRQWbovrhQMTuxg9p4LAuW5SCMAKqPrn + # secret(base58): 92Cxwia363Wg2qGF1fE5z4GKi8u7r1nrWQXdtsj2ACZqaDPSihD # keyid(hex): 1d1098b2b1f759b678a0a7a098637a9b898adcac # address(base58): yc5TGfcHYoLCrcbVy4umsiDjsYUn39vLui - # secret(base58): 92CKXK5dbysc4zQnNsvzgDpiBGgo67VPsRDAFsF4gGwbiZpgLMv - # keyid(hex): 0e9b6bdc15dc3afb63d1ffc68cc5534a4caaeaa6 - # address(base58): ybY28sZzX3Jy3UNcZ72MNubcVEQXcAN3Vh - # secret(base58): 91tDdZPR5gfCNfkVN8Pym1YyaaMHhf9MyVS12LRuN2MoAgSeDcE - # keyid(hex): 99a001480e0332d1a0d30078bde4d6fe3a178cfa - # address(base58): yjADbqTD649DaJnRGZYrXW1fnfQ4ciDcXH - # secret(base58): 928dGxqesKbqKpfAJXRnGd4gy3GtbtfM1WHTQBisvWDVJZ1Vxyi - # keyid(hex): 3ae21cfc6017c65e80d7ec45310c22f6dc705bdd - # address(base58): ygVsj29xcd1B3iMbs6A2CY9mscC8cBXEZX - # secret(base58): 91ynY1Q7iaXSbuNq9nQ2u6K2i2ryX3em5wEKEofXTjhsrDL2jY8 - # keyid(hex): 551a3c676574d355fc3c314955f3b440cf57b245 - # address(base58): ySfy7vpGjiG27PSr8XKCKr4y3mY1D9U76y - # secret(base58): 92ffBcNsSLyEKT5CmqGMEmYHJBUTHoGHjFy9Y3XNPSLbxi4DVWD - # keyid(hex): 8df696c8d4e69a898ce7d599559e32268b28ccbb - # address(base58): ydSRnFKrboF3L544AWQfG79mcXMC7ZtShL - # secret(base58): 92cDaZ9GGSLnJsaEbCDewrXA3RcmDCG9MZAeJZTmMnpgLQAsyHr - # keyid(hex): 92af1b1ebe0107dcc648aa150905e1229777225d - # address(base58): yUotyrzs4H9DJ7K3Y7ccgeAUh442FEkrKw - # secret(base58): 92PGUYidWoq9qBoyEEAeYQ12ZHWBCR5YusLenpa7vUGFXPfgqXx - # keyid(hex): 093a9963ac777502251dd98c22ea2c71aaa2fee3 - # address(base58): yh6yH8LJ4eTFWAWVNdCDaQa53nXpddvmZp - # secret(base58): 91tbFNAjdSeUYZAG3QhF91Zh1LGgZcFpq8zrGEH1MHKycFLtDrL - # keyid(hex): 5fcfae286651d85e6ad2eff5a38b148d806f9beb - # address(base58): yhoDr8n8DzfLR7VQGcLqpmq29WJ5Z2eYNi - # secret(base58): 92EMsUxGgk1372edF7iNNzQSN3iLsRkUSmZzQKGwLd4CrC3YUUH - # keyid(hex): 4287e28642ebfd3d48e86888a6bb6e7a9af22910 - # address(base58): yMnuur7LEVgrYFczsNfx8W61eAgGYAmTVG - # secret(base58): 92VRomWTLSRRHWR3NwbUAhrPuwfMyciLcDWR9ebJRbSFoEj2RLg - # keyid(hex): 6d3fcee4b4b92488ed20171b61c2951ea51cbeb4 - # address(base58): yco8DwPVN6Wgk1nCmakkLYCGzXy9fgu5zC - # secret(base58): 92swaJmVB2c4kLvKt7CihNneE7MxfQnv7eRV248F7t4CmVWsaLe - # keyid(hex): ab87f6995077f4bdd6233bc2992611674d7c6594 - # address(base58): yZr6Rw8GiZwD6GMdC2fuP7c8qkFQPgWVRX - # secret(base58): 923NQ12YSeVs4RfaCiCqTE9iNAD4dobSmuEhnV7x6q7styQYfKb - # keyid(hex): 2318b0dab38d676aae483ab0f55f2d839a1031b2 - # address(base58): ycZduSBR8W8Gpb9ec1AocYF1c4Q3E6wjeM - # secret(base58): 92tMn4LCFGTthXpdiygJGjQbJYmYY78TiXHFWYTUu9uZiLnHn8W - # keyid(hex): ed73329177896a951b0eb30dd962d3e661c0476e - # address(base58): yWNZ8gibbLMAyuXX4i1ZNaxmADW59Rz3zJ - # secret(base58): 92RgZKa9TYVRJZmj4Nme3jBRn77rD2BmQNJZJkWmN67UVpRXWjW - # keyid(hex): a7845c35a6b608c7c3a9a9388e06370fa3f5d32b - # address(base58): yQKBpf5bCZ3ctmYKFEPqdeLTg4SwuciMo7 - # secret(base58): 92mNuCHJ2kWcxGkAus1P6UDPHPFZDPNTzMMf72YujjXtdmbiUCk - # keyid(hex): dfe5fcd9bee0b871ad221a4cf3532c2848314701 - # address(base58): yLSCowJMnKfrmniiX9RD2bxvbHwvzgyKvQ - # secret(base58): 92cyrqe2iYqw6ni5MQvBAcSQNAAApmeToeA7qzawEcxLDHufGK3 - # keyid(hex): 088f9e1ee1a06e0b3fed714beee97e90c035ed76 - # address(base58): yXAGkUbnEtEeY2cdZnt9zzD4mCnmLeCZfU - # secret(base58): 93UMgtWGffnog5yYyyiSwzHCP7FX8wmURQa19f7oT95JQpLu3LN - # keyid(hex): 98396e03c6e20e44ce776e07e92cb533cb450e7f - # address(base58): yXuFmFBfipN4pDWbiQLLNAaT2HcTGLLAkJ - self.nodes.append(start_node(0, self.options.tmpdir, ["-debug", "-sporkkey=931wyuRNVYvhg18Uu9bky5Qg1z4QbxaJ7fefNBzjBPiLRqcd33F", - "-sporkaddr=ygcG5S2pQz2U1UAaHvU6EznKZW7yapKMA7:yfLSXFfipnkgYioD6L8aUNyfRgEBuJv48h:yNsMZhEhYqv14TgdYb1NS2UmNZjE8FSJxa", + "-sporkaddr=ygcG5S2pQz2U1UAaHvU6EznKZW7yapKMA7:yfLSXFfipnkgYioD6L8aUNyfRgEBuJv48h:yNsMZhEhYqv14TgdYb1NS2UmNZjE8FSJxa:ycbRQWbovrhQMTuxg9p4LAuW5SCMAKqPrn:yc5TGfcHYoLCrcbVy4umsiDjsYUn39vLui", "-minsporkkeys=2"])) self.nodes.append(start_node(1, self.options.tmpdir, ["-debug", "-sporkkey=91vbXGMSWKGHom62986XtL1q2mQDA12ngcuUNNe5NfMSj44j7g3", - "-sporkaddr=ygcG5S2pQz2U1UAaHvU6EznKZW7yapKMA7:yfLSXFfipnkgYioD6L8aUNyfRgEBuJv48h:yNsMZhEhYqv14TgdYb1NS2UmNZjE8FSJxa", + "-sporkaddr=ygcG5S2pQz2U1UAaHvU6EznKZW7yapKMA7:yfLSXFfipnkgYioD6L8aUNyfRgEBuJv48h:yNsMZhEhYqv14TgdYb1NS2UmNZjE8FSJxa:ycbRQWbovrhQMTuxg9p4LAuW5SCMAKqPrn:yc5TGfcHYoLCrcbVy4umsiDjsYUn39vLui", "-minsporkkeys=2"])) self.nodes.append(start_node(2, self.options.tmpdir, - ["-debug", - "-sporkaddr=ygcG5S2pQz2U1UAaHvU6EznKZW7yapKMA7:yfLSXFfipnkgYioD6L8aUNyfRgEBuJv48h:yNsMZhEhYqv14TgdYb1NS2UmNZjE8FSJxa", + ["-debug", "-sporkkey=92bxUjPT5AhgXuXJwfGGXqhomY2SdQ55MYjXyx9DZNxCABCSsRH", + "-sporkaddr=ygcG5S2pQz2U1UAaHvU6EznKZW7yapKMA7:yfLSXFfipnkgYioD6L8aUNyfRgEBuJv48h:yNsMZhEhYqv14TgdYb1NS2UmNZjE8FSJxa:ycbRQWbovrhQMTuxg9p4LAuW5SCMAKqPrn:yc5TGfcHYoLCrcbVy4umsiDjsYUn39vLui", + "-minsporkkeys=2"])) + self.nodes.append(start_node(3, self.options.tmpdir, + ["-debug", "-sporkkey=934yPXiVGf4RCY2qTs2Bt5k3TEtAiAg12sMxCt8yVWbSU7p3fuD", + "-sporkaddr=ygcG5S2pQz2U1UAaHvU6EznKZW7yapKMA7:yfLSXFfipnkgYioD6L8aUNyfRgEBuJv48h:yNsMZhEhYqv14TgdYb1NS2UmNZjE8FSJxa:ycbRQWbovrhQMTuxg9p4LAuW5SCMAKqPrn:yc5TGfcHYoLCrcbVy4umsiDjsYUn39vLui", + "-minsporkkeys=2"])) + self.nodes.append(start_node(4, self.options.tmpdir, + ["-debug", "-sporkkey=92Cxwia363Wg2qGF1fE5z4GKi8u7r1nrWQXdtsj2ACZqaDPSihD", + "-sporkaddr=ygcG5S2pQz2U1UAaHvU6EznKZW7yapKMA7:yfLSXFfipnkgYioD6L8aUNyfRgEBuJv48h:yNsMZhEhYqv14TgdYb1NS2UmNZjE8FSJxa:ycbRQWbovrhQMTuxg9p4LAuW5SCMAKqPrn:yc5TGfcHYoLCrcbVy4umsiDjsYUn39vLui", "-minsporkkeys=2"])) # connect nodes at start - connect_nodes(self.nodes[0], 1) - connect_nodes(self.nodes[0], 2) - connect_nodes(self.nodes[1], 2) + for i in range(0, 5): + for j in range(i, 5): + connect_nodes(self.nodes[i], j) def get_test_spork_state(self, node): info = node.spork('active') @@ -147,6 +117,12 @@ def run_test(self): for node in self.nodes: assert(self.wait_for_test_spork_state(node, False)) + # now turn on the spork again with other signers to test + # old and new spork messages interaction + self.set_test_spork_state(self.nodes[3], True) + self.set_test_spork_state(self.nodes[4], True) + for node in self.nodes: + assert(self.wait_for_test_spork_state(node, True)) if __name__ == '__main__': From 633313cf01a129c14faf94a4ebc4e1b9d2c49f0d Mon Sep 17 00:00:00 2001 From: gladcow Date: Wed, 12 Sep 2018 18:34:46 +0300 Subject: [PATCH 08/25] add multikeyspork.py to integration tests --- qa/pull-tester/rpc-tests.py | 1 + 1 file changed, 1 insertion(+) diff --git a/qa/pull-tester/rpc-tests.py b/qa/pull-tester/rpc-tests.py index e248d80859fc..b40da3c13271 100755 --- a/qa/pull-tester/rpc-tests.py +++ b/qa/pull-tester/rpc-tests.py @@ -124,6 +124,7 @@ 'wallet-accounts.py', 'wallet-dump.py', 'listtransactions.py', + 'multikeysporks.py', # vv Tests less than 60s vv 'sendheaders.py', # NOTE: needs dash_hash to pass 'zapwallettxes.py', From a3129ebefff7e49623a6d582cf1d5ddfde8ff682 Mon Sep 17 00:00:00 2001 From: gladcow Date: Fri, 14 Sep 2018 18:56:49 +0300 Subject: [PATCH 09/25] change test to have ability to distinguish default spork value --- qa/rpc-tests/multikeysporks.py | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/qa/rpc-tests/multikeysporks.py b/qa/rpc-tests/multikeysporks.py index 77c794b979e7..be2f25e05e80 100755 --- a/qa/rpc-tests/multikeysporks.py +++ b/qa/rpc-tests/multikeysporks.py @@ -76,23 +76,19 @@ def setup_network(self): connect_nodes(self.nodes[i], j) def get_test_spork_state(self, node): - info = node.spork('active') + info = node.spork('show') # use InstantSend spork for tests return info['SPORK_2_INSTANTSEND_ENABLED'] - def set_test_spork_state(self, node, state): - if state: - value = 0 - else: - value = 4070908800 + def set_test_spork_state(self, node, value): # use InstantSend spork for tests node.spork('SPORK_2_INSTANTSEND_ENABLED', value) - def wait_for_test_spork_state(self, node, state): + def wait_for_test_spork_state(self, node, value): start = time() got_state = False while True: - if self.get_test_spork_state(node) == state: + if self.get_test_spork_state(node) == value: got_state = True break if time() > start + 10: @@ -103,26 +99,26 @@ def wait_for_test_spork_state(self, node, state): def run_test(self): # check test spork default state for node in self.nodes: - assert(self.get_test_spork_state(node)) + assert(self.get_test_spork_state(node) == 0) - # first signer turns off spork - self.set_test_spork_state(self.nodes[0], False) + # first signer set spork value + self.set_test_spork_state(self.nodes[0], 1) # spork change requires at least 2 signers for node in self.nodes: - assert(not self.wait_for_test_spork_state(node, False)) + assert(not self.wait_for_test_spork_state(node, 1)) - # second signer turns off spork - self.set_test_spork_state(self.nodes[1], False) + # second signer set spork value + self.set_test_spork_state(self.nodes[1], 1) # now spork state is changed for node in self.nodes: - assert(self.wait_for_test_spork_state(node, False)) + assert(self.wait_for_test_spork_state(node, 1)) - # now turn on the spork again with other signers to test + # now set the spork again with other signers to test # old and new spork messages interaction - self.set_test_spork_state(self.nodes[3], True) - self.set_test_spork_state(self.nodes[4], True) + self.set_test_spork_state(self.nodes[3], 2) + self.set_test_spork_state(self.nodes[4], 2) for node in self.nodes: - assert(self.wait_for_test_spork_state(node, True)) + assert(self.wait_for_test_spork_state(node, 2)) if __name__ == '__main__': From e8da6a578f5d0b0dab3be3bf22e8f21261708a77 Mon Sep 17 00:00:00 2001 From: gladcow Date: Fri, 14 Sep 2018 19:07:26 +0300 Subject: [PATCH 10/25] require min spork keys number to be more than the half of the common spork keys number --- qa/rpc-tests/multikeysporks.py | 18 ++++++++++-------- src/spork.cpp | 3 ++- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/qa/rpc-tests/multikeysporks.py b/qa/rpc-tests/multikeysporks.py index be2f25e05e80..77dd7ef86d1a 100755 --- a/qa/rpc-tests/multikeysporks.py +++ b/qa/rpc-tests/multikeysporks.py @@ -53,23 +53,23 @@ def setup_network(self): self.nodes.append(start_node(0, self.options.tmpdir, ["-debug", "-sporkkey=931wyuRNVYvhg18Uu9bky5Qg1z4QbxaJ7fefNBzjBPiLRqcd33F", "-sporkaddr=ygcG5S2pQz2U1UAaHvU6EznKZW7yapKMA7:yfLSXFfipnkgYioD6L8aUNyfRgEBuJv48h:yNsMZhEhYqv14TgdYb1NS2UmNZjE8FSJxa:ycbRQWbovrhQMTuxg9p4LAuW5SCMAKqPrn:yc5TGfcHYoLCrcbVy4umsiDjsYUn39vLui", - "-minsporkkeys=2"])) + "-minsporkkeys=3"])) self.nodes.append(start_node(1, self.options.tmpdir, ["-debug", "-sporkkey=91vbXGMSWKGHom62986XtL1q2mQDA12ngcuUNNe5NfMSj44j7g3", "-sporkaddr=ygcG5S2pQz2U1UAaHvU6EznKZW7yapKMA7:yfLSXFfipnkgYioD6L8aUNyfRgEBuJv48h:yNsMZhEhYqv14TgdYb1NS2UmNZjE8FSJxa:ycbRQWbovrhQMTuxg9p4LAuW5SCMAKqPrn:yc5TGfcHYoLCrcbVy4umsiDjsYUn39vLui", - "-minsporkkeys=2"])) + "-minsporkkeys=3"])) self.nodes.append(start_node(2, self.options.tmpdir, ["-debug", "-sporkkey=92bxUjPT5AhgXuXJwfGGXqhomY2SdQ55MYjXyx9DZNxCABCSsRH", "-sporkaddr=ygcG5S2pQz2U1UAaHvU6EznKZW7yapKMA7:yfLSXFfipnkgYioD6L8aUNyfRgEBuJv48h:yNsMZhEhYqv14TgdYb1NS2UmNZjE8FSJxa:ycbRQWbovrhQMTuxg9p4LAuW5SCMAKqPrn:yc5TGfcHYoLCrcbVy4umsiDjsYUn39vLui", - "-minsporkkeys=2"])) + "-minsporkkeys=3"])) self.nodes.append(start_node(3, self.options.tmpdir, ["-debug", "-sporkkey=934yPXiVGf4RCY2qTs2Bt5k3TEtAiAg12sMxCt8yVWbSU7p3fuD", "-sporkaddr=ygcG5S2pQz2U1UAaHvU6EznKZW7yapKMA7:yfLSXFfipnkgYioD6L8aUNyfRgEBuJv48h:yNsMZhEhYqv14TgdYb1NS2UmNZjE8FSJxa:ycbRQWbovrhQMTuxg9p4LAuW5SCMAKqPrn:yc5TGfcHYoLCrcbVy4umsiDjsYUn39vLui", - "-minsporkkeys=2"])) + "-minsporkkeys=3"])) self.nodes.append(start_node(4, self.options.tmpdir, ["-debug", "-sporkkey=92Cxwia363Wg2qGF1fE5z4GKi8u7r1nrWQXdtsj2ACZqaDPSihD", "-sporkaddr=ygcG5S2pQz2U1UAaHvU6EznKZW7yapKMA7:yfLSXFfipnkgYioD6L8aUNyfRgEBuJv48h:yNsMZhEhYqv14TgdYb1NS2UmNZjE8FSJxa:ycbRQWbovrhQMTuxg9p4LAuW5SCMAKqPrn:yc5TGfcHYoLCrcbVy4umsiDjsYUn39vLui", - "-minsporkkeys=2"])) + "-minsporkkeys=3"])) # connect nodes at start for i in range(0, 5): for j in range(i, 5): @@ -101,20 +101,22 @@ def run_test(self): for node in self.nodes: assert(self.get_test_spork_state(node) == 0) - # first signer set spork value + # first and second signers set spork value self.set_test_spork_state(self.nodes[0], 1) + self.set_test_spork_state(self.nodes[1], 1) # spork change requires at least 2 signers for node in self.nodes: assert(not self.wait_for_test_spork_state(node, 1)) - # second signer set spork value - self.set_test_spork_state(self.nodes[1], 1) + # third signer set spork value + self.set_test_spork_state(self.nodes[2], 1) # now spork state is changed for node in self.nodes: assert(self.wait_for_test_spork_state(node, 1)) # now set the spork again with other signers to test # old and new spork messages interaction + self.set_test_spork_state(self.nodes[2], 2) self.set_test_spork_state(self.nodes[3], 2) self.set_test_spork_state(self.nodes[4], 2) for node in self.nodes: diff --git a/src/spork.cpp b/src/spork.cpp index e282b36d0675..f9c6d20ea6ec 100644 --- a/src/spork.cpp +++ b/src/spork.cpp @@ -330,7 +330,8 @@ bool CSporkManager::SetSporkAddress(const std::string& strAddress) { bool CSporkManager::SetMinSporkKeys(int minSporkKeys) { - if ((minSporkKeys < 1) || (minSporkKeys > sporkPubKeyIDs.size())) { + int maxKeysNumber = sporkPubKeyIDs.size(); + if ((minSporkKeys <= maxKeysNumber / 2) || (minSporkKeys > maxKeysNumber)) { LogPrintf("CSporkManager::SetSporkAddress -- Invalid min spork signers number: %d\n", minSporkKeys); return false; } From 187b87af7717ec7f3753f38bad86dba137e77418 Mon Sep 17 00:00:00 2001 From: gladcow Date: Sat, 15 Sep 2018 00:21:12 +0300 Subject: [PATCH 11/25] calc current spork value with majority of signers --- src/spork.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/spork.cpp b/src/spork.cpp index f9c6d20ea6ec..a69a9f984673 100644 --- a/src/spork.cpp +++ b/src/spork.cpp @@ -44,16 +44,17 @@ bool CSporkManager::SporkValueIsActive(int sporkID, int64_t &activeValue) const } // check if any value has enough signer votes - bool found = false; + int max_count = 0; for (const auto& value_data: value_counts) { if(value_data.second >= nMinSporkKeys) { - if (found) return false; // several active values, no consensus among signers - found = true; - activeValue = value_data.first; + if (value_data.second > max_count) { + activeValue = value_data.first; + max_count = value_data.second; + } } } - return found; + return (max_count > 0); } void CSporkManager::Clear() From 17afe2ad974eab2d63477e9ae222042b3211513d Mon Sep 17 00:00:00 2001 From: gladcow Date: Sat, 15 Sep 2018 00:26:02 +0300 Subject: [PATCH 12/25] set test nodes time in integration test --- qa/rpc-tests/multikeysporks.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/qa/rpc-tests/multikeysporks.py b/qa/rpc-tests/multikeysporks.py index 77dd7ef86d1a..420306aaf570 100755 --- a/qa/rpc-tests/multikeysporks.py +++ b/qa/rpc-tests/multikeysporks.py @@ -101,6 +101,8 @@ def run_test(self): for node in self.nodes: assert(self.get_test_spork_state(node) == 0) + set_mocktime(get_mocktime() + 1) + set_node_times(self.nodes, get_mocktime()) # first and second signers set spork value self.set_test_spork_state(self.nodes[0], 1) self.set_test_spork_state(self.nodes[1], 1) @@ -114,6 +116,8 @@ def run_test(self): for node in self.nodes: assert(self.wait_for_test_spork_state(node, 1)) + set_mocktime(get_mocktime() + 1) + set_node_times(self.nodes, get_mocktime()) # now set the spork again with other signers to test # old and new spork messages interaction self.set_test_spork_state(self.nodes[2], 2) From d51ceae00c31e029e1762958bced090a50718234 Mon Sep 17 00:00:00 2001 From: gladcow Date: Mon, 17 Sep 2018 14:06:49 +0300 Subject: [PATCH 13/25] extract keyid from signed spork message directly --- src/spork.cpp | 43 +++++++++++++++++++++++++++---------------- src/spork.h | 1 + 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/src/spork.cpp b/src/spork.cpp index a69a9f984673..3a960893f9e7 100644 --- a/src/spork.cpp +++ b/src/spork.cpp @@ -115,15 +115,9 @@ void CSporkManager::ProcessSpork(CNode* pfrom, const std::string& strCommand, CD strLogMsg = strprintf("SPORK -- hash: %s id: %d value: %10d bestHeight: %d peer=%d", hash.ToString(), spork.nSporkID, spork.nValue, chainActive.Height(), pfrom->id); } - bool found = false; CKeyID signer; - for(const auto& keyid: sporkPubKeyIDs) { - if(spork.CheckSignature(keyid, IsSporkActive(SPORK_6_NEW_SIGS))) { - found = true; - signer = keyid; - } - } - if(!found) { + if(!(spork.GetSignerKeyID(signer, IsSporkActive(SPORK_6_NEW_SIGS)) + && sporkPubKeyIDs.count(signer))) { LOCK(cs_main); LogPrintf("CSporkManager::ProcessSpork -- ERROR: invalid signature\n"); Misbehaving(pfrom->GetId(), 100); @@ -207,15 +201,8 @@ bool CSporkManager::UpdateSpork(int nSporkID, int64_t nValue, CConnman& connman) CSporkMessage spork = CSporkMessage(nSporkID, nValue, GetAdjustedTime()); if(spork.Sign(sporkPrivKey, IsSporkActive(SPORK_6_NEW_SIGS))) { - bool found = false; CKeyID signer; - for(const auto& keyid: sporkPubKeyIDs) { - if(spork.CheckSignature(keyid, IsSporkActive(SPORK_6_NEW_SIGS))) { - found = true; - signer = keyid; - } - } - if (!found) { + if (!(spork.GetSignerKeyID(signer, IsSporkActive(SPORK_6_NEW_SIGS) && sporkPubKeyIDs.count(signer)))) { LogPrintf("CSporkManager::UpdateSpork: failed to find keyid for private key\n"); return false; } @@ -455,6 +442,30 @@ bool CSporkMessage::CheckSignature(const CKeyID& pubKeyId, bool fSporkSixActive) return true; } +bool CSporkMessage::GetSignerKeyID(CKeyID &sporkSignerID, bool fSporkSixActive) +{ + CPubKey pubkeyFromSig; + if (fSporkSixActive) { + if (!pubkeyFromSig.RecoverCompact(GetHash(), vchSig)) { + return false; + } + } else { + std::string strMessage = std::to_string(nSporkID) + std::to_string(nValue) + std::to_string(nTimeSigned); + CHashWriter ss(SER_GETHASH, 0); + ss << strMessageMagic; + ss << strMessage; + if (!pubkeyFromSig.RecoverCompact(ss.GetHash(), vchSig)) { + // Note: unlike for other messages we have to check for new format even with SPORK_6_NEW_SIGS + // inactive because SPORK_6_NEW_SIGS default is OFF and it is not the first spork to sync + // (and even if it would, spork order can't be guaranteed anyway). + return GetSignerKeyID(sporkSignerID, true); + } + } + + sporkSignerID = pubkeyFromSig.GetID(); + return true; +} + void CSporkMessage::Relay(CConnman& connman) { CInv inv(MSG_SPORK, GetHash()); diff --git a/src/spork.h b/src/spork.h index 86f1bedfd86b..6c03b428f2b5 100644 --- a/src/spork.h +++ b/src/spork.h @@ -78,6 +78,7 @@ class CSporkMessage bool Sign(const CKey& key, bool fSporkSixActive); bool CheckSignature(const CKeyID& pubKeyId, bool fSporkSixActive) const; + bool GetSignerKeyID(CKeyID& sporkSignerID, bool fSporkSixActive); void Relay(CConnman& connman); }; From 9ad4485228eea699a2dfcdfab32e24f80fc14dc8 Mon Sep 17 00:00:00 2001 From: gladcow Date: Mon, 17 Sep 2018 15:02:40 +0300 Subject: [PATCH 14/25] change -sporkaddr option syntax to process several addresses --- qa/rpc-tests/multikeysporks.py | 30 +++++++++++++++++++++++++----- src/chainparams.cpp | 8 ++++---- src/chainparams.h | 4 ++-- src/init.cpp | 7 +++++-- 4 files changed, 36 insertions(+), 13 deletions(-) diff --git a/qa/rpc-tests/multikeysporks.py b/qa/rpc-tests/multikeysporks.py index 420306aaf570..8be768991773 100755 --- a/qa/rpc-tests/multikeysporks.py +++ b/qa/rpc-tests/multikeysporks.py @@ -52,23 +52,43 @@ def setup_network(self): self.nodes.append(start_node(0, self.options.tmpdir, ["-debug", "-sporkkey=931wyuRNVYvhg18Uu9bky5Qg1z4QbxaJ7fefNBzjBPiLRqcd33F", - "-sporkaddr=ygcG5S2pQz2U1UAaHvU6EznKZW7yapKMA7:yfLSXFfipnkgYioD6L8aUNyfRgEBuJv48h:yNsMZhEhYqv14TgdYb1NS2UmNZjE8FSJxa:ycbRQWbovrhQMTuxg9p4LAuW5SCMAKqPrn:yc5TGfcHYoLCrcbVy4umsiDjsYUn39vLui", + "-sporkaddr=ygcG5S2pQz2U1UAaHvU6EznKZW7yapKMA7", + "-sporkaddr=yfLSXFfipnkgYioD6L8aUNyfRgEBuJv48h", + "-sporkaddr=yNsMZhEhYqv14TgdYb1NS2UmNZjE8FSJxa", + "-sporkaddr=ycbRQWbovrhQMTuxg9p4LAuW5SCMAKqPrn", + "-sporkaddr=yc5TGfcHYoLCrcbVy4umsiDjsYUn39vLui", "-minsporkkeys=3"])) self.nodes.append(start_node(1, self.options.tmpdir, ["-debug", "-sporkkey=91vbXGMSWKGHom62986XtL1q2mQDA12ngcuUNNe5NfMSj44j7g3", - "-sporkaddr=ygcG5S2pQz2U1UAaHvU6EznKZW7yapKMA7:yfLSXFfipnkgYioD6L8aUNyfRgEBuJv48h:yNsMZhEhYqv14TgdYb1NS2UmNZjE8FSJxa:ycbRQWbovrhQMTuxg9p4LAuW5SCMAKqPrn:yc5TGfcHYoLCrcbVy4umsiDjsYUn39vLui", + "-sporkaddr=ygcG5S2pQz2U1UAaHvU6EznKZW7yapKMA7", + "-sporkaddr=yfLSXFfipnkgYioD6L8aUNyfRgEBuJv48h", + "-sporkaddr=yNsMZhEhYqv14TgdYb1NS2UmNZjE8FSJxa", + "-sporkaddr=ycbRQWbovrhQMTuxg9p4LAuW5SCMAKqPrn", + "-sporkaddr=yc5TGfcHYoLCrcbVy4umsiDjsYUn39vLui", "-minsporkkeys=3"])) self.nodes.append(start_node(2, self.options.tmpdir, ["-debug", "-sporkkey=92bxUjPT5AhgXuXJwfGGXqhomY2SdQ55MYjXyx9DZNxCABCSsRH", - "-sporkaddr=ygcG5S2pQz2U1UAaHvU6EznKZW7yapKMA7:yfLSXFfipnkgYioD6L8aUNyfRgEBuJv48h:yNsMZhEhYqv14TgdYb1NS2UmNZjE8FSJxa:ycbRQWbovrhQMTuxg9p4LAuW5SCMAKqPrn:yc5TGfcHYoLCrcbVy4umsiDjsYUn39vLui", + "-sporkaddr=ygcG5S2pQz2U1UAaHvU6EznKZW7yapKMA7", + "-sporkaddr=yfLSXFfipnkgYioD6L8aUNyfRgEBuJv48h", + "-sporkaddr=yNsMZhEhYqv14TgdYb1NS2UmNZjE8FSJxa", + "-sporkaddr=ycbRQWbovrhQMTuxg9p4LAuW5SCMAKqPrn", + "-sporkaddr=yc5TGfcHYoLCrcbVy4umsiDjsYUn39vLui", "-minsporkkeys=3"])) self.nodes.append(start_node(3, self.options.tmpdir, ["-debug", "-sporkkey=934yPXiVGf4RCY2qTs2Bt5k3TEtAiAg12sMxCt8yVWbSU7p3fuD", - "-sporkaddr=ygcG5S2pQz2U1UAaHvU6EznKZW7yapKMA7:yfLSXFfipnkgYioD6L8aUNyfRgEBuJv48h:yNsMZhEhYqv14TgdYb1NS2UmNZjE8FSJxa:ycbRQWbovrhQMTuxg9p4LAuW5SCMAKqPrn:yc5TGfcHYoLCrcbVy4umsiDjsYUn39vLui", + "-sporkaddr=ygcG5S2pQz2U1UAaHvU6EznKZW7yapKMA7", + "-sporkaddr=yfLSXFfipnkgYioD6L8aUNyfRgEBuJv48h", + "-sporkaddr=yNsMZhEhYqv14TgdYb1NS2UmNZjE8FSJxa", + "-sporkaddr=ycbRQWbovrhQMTuxg9p4LAuW5SCMAKqPrn", + "-sporkaddr=yc5TGfcHYoLCrcbVy4umsiDjsYUn39vLui", "-minsporkkeys=3"])) self.nodes.append(start_node(4, self.options.tmpdir, ["-debug", "-sporkkey=92Cxwia363Wg2qGF1fE5z4GKi8u7r1nrWQXdtsj2ACZqaDPSihD", - "-sporkaddr=ygcG5S2pQz2U1UAaHvU6EznKZW7yapKMA7:yfLSXFfipnkgYioD6L8aUNyfRgEBuJv48h:yNsMZhEhYqv14TgdYb1NS2UmNZjE8FSJxa:ycbRQWbovrhQMTuxg9p4LAuW5SCMAKqPrn:yc5TGfcHYoLCrcbVy4umsiDjsYUn39vLui", + "-sporkaddr=ygcG5S2pQz2U1UAaHvU6EznKZW7yapKMA7", + "-sporkaddr=yfLSXFfipnkgYioD6L8aUNyfRgEBuJv48h", + "-sporkaddr=yNsMZhEhYqv14TgdYb1NS2UmNZjE8FSJxa", + "-sporkaddr=ycbRQWbovrhQMTuxg9p4LAuW5SCMAKqPrn", + "-sporkaddr=yc5TGfcHYoLCrcbVy4umsiDjsYUn39vLui", "-minsporkkeys=3"])) # connect nodes at start for i in range(0, 5): diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 221fc7e7960c..51ae459cda9a 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -237,7 +237,7 @@ class CMainParams : public CChainParams { nPoolMaxTransactions = 3; nFulfilledRequestExpireTime = 60*60; // fulfilled requests expire in 1 hour - strSporkAddresses = "Xgtyuk76vhuFW2iT7UAiHgNdWXCf3J34wh"; + vSporkAddresses = {"Xgtyuk76vhuFW2iT7UAiHgNdWXCf3J34wh"}; nMinSporkKeys = 1; checkpointData = (CCheckpointData) { @@ -392,7 +392,7 @@ class CTestNetParams : public CChainParams { nPoolMaxTransactions = 3; nFulfilledRequestExpireTime = 5*60; // fulfilled requests expire in 5 minutes - strSporkAddresses = "yjPtiKh2uwk3bDutTEA2q9mCtXyiZRWn55"; + vSporkAddresses = {"yjPtiKh2uwk3bDutTEA2q9mCtXyiZRWn55"}; nMinSporkKeys = 1; checkpointData = (CCheckpointData) { @@ -537,7 +537,7 @@ class CDevNetParams : public CChainParams { nPoolMaxTransactions = 3; nFulfilledRequestExpireTime = 5*60; // fulfilled requests expire in 5 minutes - strSporkAddresses = "yjPtiKh2uwk3bDutTEA2q9mCtXyiZRWn55"; + vSporkAddresses = {"yjPtiKh2uwk3bDutTEA2q9mCtXyiZRWn55"}; nMinSporkKeys = 1; checkpointData = (CCheckpointData) { @@ -644,7 +644,7 @@ class CRegTestParams : public CChainParams { nFulfilledRequestExpireTime = 5*60; // fulfilled requests expire in 5 minutes // privKey: cP4EKFyJsHT39LDqgdcB43Y3YXjNyjb5Fuas1GQSeAtjnZWmZEQK - strSporkAddresses = "yj949n1UH6fDhw6HtVE5VMj2iSTaSWBMcW"; + vSporkAddresses = {"yj949n1UH6fDhw6HtVE5VMj2iSTaSWBMcW"}; nMinSporkKeys = 1; checkpointData = (CCheckpointData){ diff --git a/src/chainparams.h b/src/chainparams.h index 33af093c6ef7..29e3b9588474 100644 --- a/src/chainparams.h +++ b/src/chainparams.h @@ -88,7 +88,7 @@ class CChainParams const ChainTxData& TxData() const { return chainTxData; } int PoolMaxTransactions() const { return nPoolMaxTransactions; } int FulfilledRequestExpireTime() const { return nFulfilledRequestExpireTime; } - const std::string& SporkAddresses() const { return strSporkAddresses; } + const std::vector& SporkAddresses() const { return vSporkAddresses; } int MinSporkKeys() const { return nMinSporkKeys; } protected: CChainParams() {} @@ -117,7 +117,7 @@ class CChainParams ChainTxData chainTxData; int nPoolMaxTransactions; int nFulfilledRequestExpireTime; - std::string strSporkAddresses; + std::vector vSporkAddresses; int nMinSporkKeys; }; diff --git a/src/init.cpp b/src/init.cpp index 27fbfe849433..120c4744afc7 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1399,9 +1399,12 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler) threadGroup.create_thread(&ThreadScriptCheck); } - std::string strSporkAddresses = GetArg("-sporkaddr", Params().SporkAddresses()); std::vector vSporkAddresses; - boost::split(vSporkAddresses, strSporkAddresses, boost::is_any_of(":")); + if(mapMultiArgs.count("-sporkaddr")) { + vSporkAddresses = mapMultiArgs.at("-sporkaddr"); + } else { + vSporkAddresses = Params().SporkAddresses(); + } for(const auto& address: vSporkAddresses) { if (!sporkManager.SetSporkAddress(address)) return InitError(_("Invalid spork address specified with -sporkaddr")); From 4adf3346169f3d16d38d3bf43c72e1a1f7765993 Mon Sep 17 00:00:00 2001 From: gladcow Date: Mon, 17 Sep 2018 22:36:50 +0300 Subject: [PATCH 15/25] codestyle fixes --- src/init.cpp | 16 +++++++++------- src/spork.cpp | 17 +++++++++-------- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 120c4744afc7..0cf30ccca109 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1400,25 +1400,27 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler) } std::vector vSporkAddresses; - if(mapMultiArgs.count("-sporkaddr")) { + if (mapMultiArgs.count("-sporkaddr")) { vSporkAddresses = mapMultiArgs.at("-sporkaddr"); } else { vSporkAddresses = Params().SporkAddresses(); } - for(const auto& address: vSporkAddresses) { - if (!sporkManager.SetSporkAddress(address)) + for (const auto& address: vSporkAddresses) { + if (!sporkManager.SetSporkAddress(address)) { return InitError(_("Invalid spork address specified with -sporkaddr")); + } } int minsporkkeys = GetArg("-minsporkkeys", Params().MinSporkKeys()); - if(!sporkManager.SetMinSporkKeys(minsporkkeys)) + if (!sporkManager.SetMinSporkKeys(minsporkkeys)) { return InitError(_("Invalid minimum number of spork signers specified with -minsporkkeys")); + } - if (IsArgSet("-sporkkey")) // spork priv key - { - if (!sporkManager.SetPrivKey(GetArg("-sporkkey", ""))) + if (IsArgSet("-sporkkey")) { // spork priv key + if (!sporkManager.SetPrivKey(GetArg("-sporkkey", ""))) { return InitError(_("Unable to sign spork message, wrong key?")); + } } // Start the lightweight task scheduler thread diff --git a/src/spork.cpp b/src/spork.cpp index 3a960893f9e7..c178b80eb918 100644 --- a/src/spork.cpp +++ b/src/spork.cpp @@ -34,8 +34,8 @@ std::map mapSporkDefaults = { bool CSporkManager::SporkValueIsActive(int sporkID, int64_t &activeValue) const { LOCK(cs); - if (!mapSporksActive.count(sporkID)) - return false; + + if (!mapSporksActive.count(sporkID)) return false; // calc how many values we have and how many signers vote for every value std::map value_counts; @@ -46,7 +46,7 @@ bool CSporkManager::SporkValueIsActive(int sporkID, int64_t &activeValue) const // check if any value has enough signer votes int max_count = 0; for (const auto& value_data: value_counts) { - if(value_data.second >= nMinSporkKeys) { + if (value_data.second >= nMinSporkKeys) { if (value_data.second > max_count) { activeValue = value_data.first; max_count = value_data.second; @@ -116,7 +116,7 @@ void CSporkManager::ProcessSpork(CNode* pfrom, const std::string& strCommand, CD } CKeyID signer; - if(!(spork.GetSignerKeyID(signer, IsSporkActive(SPORK_6_NEW_SIGS)) + if (!(spork.GetSignerKeyID(signer, IsSporkActive(SPORK_6_NEW_SIGS)) && sporkPubKeyIDs.count(signer))) { LOCK(cs_main); LogPrintf("CSporkManager::ProcessSpork -- ERROR: invalid signature\n"); @@ -222,7 +222,7 @@ bool CSporkManager::IsSporkActive(int nSporkID) LOCK(cs); int64_t r = -1; - if(SporkValueIsActive(nSporkID, r)){ + if (SporkValueIsActive(nSporkID, r)){ return r < GetAdjustedTime(); } @@ -239,9 +239,10 @@ bool CSporkManager::IsSporkActive(int nSporkID) int64_t CSporkManager::GetSporkValue(int nSporkID) { LOCK(cs); - int64_t r = -1; - if(SporkValueIsActive(nSporkID, r)){ - return r; + + int64_t sporkValue = -1; + if (SporkValueIsActive(nSporkID, sporkValue)) { + return sporkValue; } if (mapSporkDefaults.count(nSporkID)) { From a09496693f90c83616ab0b52a788857611362c57 Mon Sep 17 00:00:00 2001 From: gladcow Date: Mon, 24 Sep 2018 16:51:25 +0300 Subject: [PATCH 16/25] fix test comments --- qa/rpc-tests/multikeysporks.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/qa/rpc-tests/multikeysporks.py b/qa/rpc-tests/multikeysporks.py index 8be768991773..8d4f8c38e670 100755 --- a/qa/rpc-tests/multikeysporks.py +++ b/qa/rpc-tests/multikeysporks.py @@ -14,8 +14,8 @@ Test logic for several signer keys usage for spork broadcast. We set 5 possible keys for sporks signing and set minimum -required signers to 2. We check 1 siger can't set the spork -value, any 2 signers can change spork value and other 2 signers +required signers to 3. We check 1 and 2 signers can't set the spork +value, any 3 signers can change spork value and other 3 signers can change it again. ''' @@ -126,7 +126,7 @@ def run_test(self): # first and second signers set spork value self.set_test_spork_state(self.nodes[0], 1) self.set_test_spork_state(self.nodes[1], 1) - # spork change requires at least 2 signers + # spork change requires at least 3 signers for node in self.nodes: assert(not self.wait_for_test_spork_state(node, 1)) From ea19a733611e6883bbe6c8ac5a8edb148eeb77a1 Mon Sep 17 00:00:00 2001 From: gladcow Date: Mon, 24 Sep 2018 17:34:24 +0300 Subject: [PATCH 17/25] codestyle fixes --- src/spork.cpp | 41 ++++++++++++++++++++--------------------- src/spork.h | 2 +- 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/src/spork.cpp b/src/spork.cpp index c178b80eb918..ac5cb10fc3c3 100644 --- a/src/spork.cpp +++ b/src/spork.cpp @@ -31,24 +31,24 @@ std::map mapSporkDefaults = { {SPORK_16_INSTANTSEND_AUTOLOCKS, 4070908800ULL}, // OFF }; -bool CSporkManager::SporkValueIsActive(int sporkID, int64_t &activeValue) const +bool CSporkManager::SporkValueIsActive(int nSporkID, int64_t &nActiveValueRet) const { LOCK(cs); - if (!mapSporksActive.count(sporkID)) return false; + if (!mapSporksActive.count(nSporkID)) return false; // calc how many values we have and how many signers vote for every value - std::map value_counts; - for (const auto& pair: mapSporksActive.at(sporkID)) { - value_counts[pair.second.nValue]++; + std::map mapValueCounts; + for (const auto& pair: mapSporksActive.at(nSporkID)) { + mapValueCounts[pair.second.nValue]++; } // check if any value has enough signer votes int max_count = 0; - for (const auto& value_data: value_counts) { + for (const auto& value_data: mapValueCounts) { if (value_data.second >= nMinSporkKeys) { if (value_data.second > max_count) { - activeValue = value_data.first; + nActiveValueRet = value_data.first; max_count = value_data.second; } } @@ -115,9 +115,9 @@ void CSporkManager::ProcessSpork(CNode* pfrom, const std::string& strCommand, CD strLogMsg = strprintf("SPORK -- hash: %s id: %d value: %10d bestHeight: %d peer=%d", hash.ToString(), spork.nSporkID, spork.nValue, chainActive.Height(), pfrom->id); } - CKeyID signer; - if (!(spork.GetSignerKeyID(signer, IsSporkActive(SPORK_6_NEW_SIGS)) - && sporkPubKeyIDs.count(signer))) { + CKeyID keyIDSigner; + if (!(spork.GetSignerKeyID(keyIDSigner, IsSporkActive(SPORK_6_NEW_SIGS)) + && sporkPubKeyIDs.count(keyIDSigner))) { LOCK(cs_main); LogPrintf("CSporkManager::ProcessSpork -- ERROR: invalid signature\n"); Misbehaving(pfrom->GetId(), 100); @@ -127,8 +127,8 @@ void CSporkManager::ProcessSpork(CNode* pfrom, const std::string& strCommand, CD { LOCK(cs); // make sure to not lock this together with cs_main if (mapSporksActive.count(spork.nSporkID)) { - if (mapSporksActive[spork.nSporkID].count(signer)) { - if (mapSporksActive[spork.nSporkID][signer].nTimeSigned >= spork.nTimeSigned) { + if (mapSporksActive[spork.nSporkID].count(keyIDSigner)) { + if (mapSporksActive[spork.nSporkID][keyIDSigner].nTimeSigned >= spork.nTimeSigned) { LogPrint("spork", "%s seen\n", strLogMsg); return; } else { @@ -146,14 +146,14 @@ void CSporkManager::ProcessSpork(CNode* pfrom, const std::string& strCommand, CD { LOCK(cs); // make sure to not lock this together with cs_main mapSporksByHash[hash] = spork; - mapSporksActive[spork.nSporkID][signer] = spork; + mapSporksActive[spork.nSporkID][keyIDSigner] = spork; } spork.Relay(connman); //does a task if needed - int64_t activeValue = 0; - if (SporkValueIsActive(spork.nSporkID, activeValue)) { - ExecuteSpork(spork.nSporkID, activeValue); + int64_t nActiveValue = 0; + if (SporkValueIsActive(spork.nSporkID, nActiveValue)) { + ExecuteSpork(spork.nSporkID, nActiveValue); } } else if (strCommand == NetMsgType::GETSPORKS) { @@ -201,15 +201,15 @@ bool CSporkManager::UpdateSpork(int nSporkID, int64_t nValue, CConnman& connman) CSporkMessage spork = CSporkMessage(nSporkID, nValue, GetAdjustedTime()); if(spork.Sign(sporkPrivKey, IsSporkActive(SPORK_6_NEW_SIGS))) { - CKeyID signer; - if (!(spork.GetSignerKeyID(signer, IsSporkActive(SPORK_6_NEW_SIGS) && sporkPubKeyIDs.count(signer)))) { + CKeyID keyIDSigner; + if (!(spork.GetSignerKeyID(keyIDSigner, IsSporkActive(SPORK_6_NEW_SIGS) && sporkPubKeyIDs.count(keyIDSigner)))) { LogPrintf("CSporkManager::UpdateSpork: failed to find keyid for private key\n"); return false; } spork.Relay(connman); LOCK(cs); mapSporksByHash[spork.GetHash()] = spork; - mapSporksActive[nSporkID][signer] = spork; + mapSporksActive[nSporkID][keyIDSigner] = spork; return true; } @@ -231,8 +231,7 @@ bool CSporkManager::IsSporkActive(int nSporkID) } LogPrint("spork", "CSporkManager::IsSporkActive -- Unknown Spork ID %d\n", nSporkID); - r = 4070908800ULL; // 2099-1-1 i.e. off by default - return r < GetAdjustedTime(); + return false; } // grab the value of the spork on the network, or the default diff --git a/src/spork.h b/src/spork.h index 6c03b428f2b5..4858f50d3a55 100644 --- a/src/spork.h +++ b/src/spork.h @@ -96,7 +96,7 @@ class CSporkManager int nMinSporkKeys; CKey sporkPrivKey; - bool SporkValueIsActive(int sporkID, int64_t& activeValue) const; + bool SporkValueIsActive(int nSporkID, int64_t& nActiveValueRet) const; public: CSporkManager() {} From 985ca1d8dd581d6bdfeaaa7cfa0b759ab0927e8e Mon Sep 17 00:00:00 2001 From: gladcow Date: Mon, 24 Sep 2018 19:25:24 +0300 Subject: [PATCH 18/25] simplify CSporkManager::SporkValueIsActive --- src/spork.cpp | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/spork.cpp b/src/spork.cpp index ac5cb10fc3c3..f34fcdb6b4fc 100644 --- a/src/spork.cpp +++ b/src/spork.cpp @@ -41,20 +41,15 @@ bool CSporkManager::SporkValueIsActive(int nSporkID, int64_t &nActiveValueRet) c std::map mapValueCounts; for (const auto& pair: mapSporksActive.at(nSporkID)) { mapValueCounts[pair.second.nValue]++; - } - - // check if any value has enough signer votes - int max_count = 0; - for (const auto& value_data: mapValueCounts) { - if (value_data.second >= nMinSporkKeys) { - if (value_data.second > max_count) { - nActiveValueRet = value_data.first; - max_count = value_data.second; - } + if (mapValueCounts.at(pair.second.nValue) >= nMinSporkKeys) { + // nMinSporkKeys is always more than the half of the max spork keys number, + // so there is only one such value and we can stop here + nActiveValueRet = pair.second.nValue; + return true; } } - return (max_count > 0); + return false; } void CSporkManager::Clear() From 5c0c2ce6d5ea1ae159c766cdf48f02b5da95abe6 Mon Sep 17 00:00:00 2001 From: gladcow Date: Tue, 25 Sep 2018 18:30:54 +0300 Subject: [PATCH 19/25] Calc signature hash without signature field --- src/spork.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/spork.cpp b/src/spork.cpp index f34fcdb6b4fc..7f5ee6253ea6 100644 --- a/src/spork.cpp +++ b/src/spork.cpp @@ -364,7 +364,11 @@ uint256 CSporkMessage::GetHash() const uint256 CSporkMessage::GetSignatureHash() const { - return GetHash(); + CHashWriter s(SER_GETHASH, 0); + s << nSporkID; + s << nValue; + s << nTimeSigned; + return s.GetHash(); } bool CSporkMessage::Sign(const CKey& key, bool fSporkSixActive) @@ -441,7 +445,7 @@ bool CSporkMessage::GetSignerKeyID(CKeyID &sporkSignerID, bool fSporkSixActive) { CPubKey pubkeyFromSig; if (fSporkSixActive) { - if (!pubkeyFromSig.RecoverCompact(GetHash(), vchSig)) { + if (!pubkeyFromSig.RecoverCompact(GetSignatureHash(), vchSig)) { return false; } } else { From 58da4c5242ab2494ad6709944449a39b44240509 Mon Sep 17 00:00:00 2001 From: gladcow Date: Wed, 26 Sep 2018 14:14:24 +0300 Subject: [PATCH 20/25] do not restore pubkey ids from cach --- src/spork.cpp | 2 +- src/spork.h | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/spork.cpp b/src/spork.cpp index 7f5ee6253ea6..6b3dfc52055f 100644 --- a/src/spork.cpp +++ b/src/spork.cpp @@ -15,7 +15,7 @@ CSporkManager sporkManager; -const std::string CSporkManager::SERIALIZATION_VERSION_STRING = "CMasternodeMan-Version-1"; +const std::string CSporkManager::SERIALIZATION_VERSION_STRING = "CMasternodeMan-Version-2"; std::map mapSporkDefaults = { {SPORK_2_INSTANTSEND_ENABLED, 0}, // ON diff --git a/src/spork.h b/src/spork.h index 4858f50d3a55..38cc59d7200e 100644 --- a/src/spork.h +++ b/src/spork.h @@ -115,7 +115,9 @@ class CSporkManager strVersion = SERIALIZATION_VERSION_STRING; READWRITE(strVersion); } - READWRITE(sporkPubKeyIDs); + // we don't serialize pubkey ids because pubkeys should be + // hardcoded or be setted with cmdline or options, should + // not reuse pubkeys from previous dashd run READWRITE(mapSporksByHash); READWRITE(mapSporksActive); // we don't serialize private key to prevent its leakage From 8aa814a5f72d61823b6d9cdca20121beb61a37da Mon Sep 17 00:00:00 2001 From: gladcow Date: Wed, 26 Sep 2018 14:56:06 +0300 Subject: [PATCH 21/25] Calc different keyids to check signature because not all sporks can be synced at moment --- src/spork.cpp | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/spork.cpp b/src/spork.cpp index 6b3dfc52055f..61857de473c2 100644 --- a/src/spork.cpp +++ b/src/spork.cpp @@ -113,10 +113,16 @@ void CSporkManager::ProcessSpork(CNode* pfrom, const std::string& strCommand, CD CKeyID keyIDSigner; if (!(spork.GetSignerKeyID(keyIDSigner, IsSporkActive(SPORK_6_NEW_SIGS)) && sporkPubKeyIDs.count(keyIDSigner))) { - LOCK(cs_main); - LogPrintf("CSporkManager::ProcessSpork -- ERROR: invalid signature\n"); - Misbehaving(pfrom->GetId(), 100); - return; + // Note: unlike for other messages we have to check for new format even with SPORK_6_NEW_SIGS + // inactive because SPORK_6_NEW_SIGS default is OFF and it is not the first spork to sync + // (and even if it would, spork order can't be guaranteed anyway). + if (!(spork.GetSignerKeyID(keyIDSigner, !IsSporkActive(SPORK_6_NEW_SIGS)) + && sporkPubKeyIDs.count(keyIDSigner))) { + LOCK(cs_main); + LogPrintf("CSporkManager::ProcessSpork -- ERROR: invalid signature\n"); + Misbehaving(pfrom->GetId(), 100); + return; + } } { From 167bf9ff33b7836ec95606e2b5c65d1d880e4427 Mon Sep 17 00:00:00 2001 From: gladcow Date: Wed, 26 Sep 2018 16:03:40 +0300 Subject: [PATCH 22/25] codestyle fixes --- src/spork.cpp | 30 +++++++++++++++--------------- src/spork.h | 4 ++-- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/spork.cpp b/src/spork.cpp index 61857de473c2..bb12b4cdd21c 100644 --- a/src/spork.cpp +++ b/src/spork.cpp @@ -112,12 +112,12 @@ void CSporkManager::ProcessSpork(CNode* pfrom, const std::string& strCommand, CD CKeyID keyIDSigner; if (!(spork.GetSignerKeyID(keyIDSigner, IsSporkActive(SPORK_6_NEW_SIGS)) - && sporkPubKeyIDs.count(keyIDSigner))) { + && setSporkPubKeyIDs.count(keyIDSigner))) { // Note: unlike for other messages we have to check for new format even with SPORK_6_NEW_SIGS // inactive because SPORK_6_NEW_SIGS default is OFF and it is not the first spork to sync // (and even if it would, spork order can't be guaranteed anyway). if (!(spork.GetSignerKeyID(keyIDSigner, !IsSporkActive(SPORK_6_NEW_SIGS)) - && sporkPubKeyIDs.count(keyIDSigner))) { + && setSporkPubKeyIDs.count(keyIDSigner))) { LOCK(cs_main); LogPrintf("CSporkManager::ProcessSpork -- ERROR: invalid signature\n"); Misbehaving(pfrom->GetId(), 100); @@ -203,7 +203,7 @@ bool CSporkManager::UpdateSpork(int nSporkID, int64_t nValue, CConnman& connman) if(spork.Sign(sporkPrivKey, IsSporkActive(SPORK_6_NEW_SIGS))) { CKeyID keyIDSigner; - if (!(spork.GetSignerKeyID(keyIDSigner, IsSporkActive(SPORK_6_NEW_SIGS) && sporkPubKeyIDs.count(keyIDSigner)))) { + if (!(spork.GetSignerKeyID(keyIDSigner, IsSporkActive(SPORK_6_NEW_SIGS) && setSporkPubKeyIDs.count(keyIDSigner)))) { LogPrintf("CSporkManager::UpdateSpork: failed to find keyid for private key\n"); return false; } @@ -221,10 +221,10 @@ bool CSporkManager::UpdateSpork(int nSporkID, int64_t nValue, CConnman& connman) bool CSporkManager::IsSporkActive(int nSporkID) { LOCK(cs); - int64_t r = -1; + int64_t nSporkValue = -1; - if (SporkValueIsActive(nSporkID, r)){ - return r < GetAdjustedTime(); + if (SporkValueIsActive(nSporkID, nSporkValue)) { + return nSporkValue < GetAdjustedTime(); } if (mapSporkDefaults.count(nSporkID)) { @@ -240,9 +240,9 @@ int64_t CSporkManager::GetSporkValue(int nSporkID) { LOCK(cs); - int64_t sporkValue = -1; - if (SporkValueIsActive(nSporkID, sporkValue)) { - return sporkValue; + int64_t nSporkValue = -1; + if (SporkValueIsActive(nSporkID, nSporkValue)) { + return nSporkValue; } if (mapSporkDefaults.count(nSporkID)) { @@ -313,13 +313,13 @@ bool CSporkManager::SetSporkAddress(const std::string& strAddress) { LogPrintf("CSporkManager::SetSporkAddress -- Failed to parse spork address\n"); return false; } - sporkPubKeyIDs.insert(keyid); + setSporkPubKeyIDs.insert(keyid); return true; } bool CSporkManager::SetMinSporkKeys(int minSporkKeys) { - int maxKeysNumber = sporkPubKeyIDs.size(); + int maxKeysNumber = setSporkPubKeyIDs.size(); if ((minSporkKeys <= maxKeysNumber / 2) || (minSporkKeys > maxKeysNumber)) { LogPrintf("CSporkManager::SetSporkAddress -- Invalid min spork signers number: %d\n", minSporkKeys); return false; @@ -337,7 +337,7 @@ bool CSporkManager::SetPrivKey(const std::string& strPrivKey) return false; } - if (sporkPubKeyIDs.find(pubKey.GetID()) == sporkPubKeyIDs.end()) { + if (setSporkPubKeyIDs.find(pubKey.GetID()) == setSporkPubKeyIDs.end()) { LogPrintf("CSporkManager::SetPrivKey -- New private key does not belong to spork addresses\n"); return false; } @@ -447,7 +447,7 @@ bool CSporkMessage::CheckSignature(const CKeyID& pubKeyId, bool fSporkSixActive) return true; } -bool CSporkMessage::GetSignerKeyID(CKeyID &sporkSignerID, bool fSporkSixActive) +bool CSporkMessage::GetSignerKeyID(CKeyID &retKeyidSporkSigner, bool fSporkSixActive) { CPubKey pubkeyFromSig; if (fSporkSixActive) { @@ -463,11 +463,11 @@ bool CSporkMessage::GetSignerKeyID(CKeyID &sporkSignerID, bool fSporkSixActive) // Note: unlike for other messages we have to check for new format even with SPORK_6_NEW_SIGS // inactive because SPORK_6_NEW_SIGS default is OFF and it is not the first spork to sync // (and even if it would, spork order can't be guaranteed anyway). - return GetSignerKeyID(sporkSignerID, true); + return GetSignerKeyID(retKeyidSporkSigner, true); } } - sporkSignerID = pubkeyFromSig.GetID(); + retKeyidSporkSigner = pubkeyFromSig.GetID(); return true; } diff --git a/src/spork.h b/src/spork.h index 38cc59d7200e..85e44a33ee10 100644 --- a/src/spork.h +++ b/src/spork.h @@ -78,7 +78,7 @@ class CSporkMessage bool Sign(const CKey& key, bool fSporkSixActive); bool CheckSignature(const CKeyID& pubKeyId, bool fSporkSixActive) const; - bool GetSignerKeyID(CKeyID& sporkSignerID, bool fSporkSixActive); + bool GetSignerKeyID(CKeyID& retKeyidSporkSigner, bool fSporkSixActive); void Relay(CConnman& connman); }; @@ -92,7 +92,7 @@ class CSporkManager std::map mapSporksByHash; std::map > mapSporksActive; - std::set sporkPubKeyIDs; + std::set setSporkPubKeyIDs; int nMinSporkKeys; CKey sporkPrivKey; From d184ecedd1d3b08dec18ff2544619c64324a5db1 Mon Sep 17 00:00:00 2001 From: gladcow Date: Wed, 26 Sep 2018 19:26:15 +0300 Subject: [PATCH 23/25] Fix CSporkManager::CheckAndRemove to use several keys --- src/spork.cpp | 38 ++++++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/src/spork.cpp b/src/spork.cpp index bb12b4cdd21c..49ce3ff98dce 100644 --- a/src/spork.cpp +++ b/src/spork.cpp @@ -64,29 +64,47 @@ void CSporkManager::Clear() void CSporkManager::CheckAndRemove() { LOCK(cs); - bool fSporkAddressIsSet = !sporkPubKeyID.IsNull(); + bool fSporkAddressIsSet = !setSporkPubKeyIDs.empty(); assert(fSporkAddressIsSet); auto itActive = mapSporksActive.begin(); while (itActive != mapSporksActive.end()) { - if (!itActive->second.CheckSignature(sporkPubKeyID, false)) { - if (!itActive->second.CheckSignature(sporkPubKeyID, true)) { - mapSporksByHash.erase(itActive->second.GetHash()); - mapSporksActive.erase(itActive++); + auto signer_msg_pair = itActive->second.begin(); + while (signer_msg_pair != itActive->second.end()) { + if (setSporkPubKeyIDs.find(signer_msg_pair->first) == setSporkPubKeyIDs.end()) { + mapSporksByHash.erase(signer_msg_pair->second.GetHash()); continue; } + if (!signer_msg_pair->second.CheckSignature(signer_msg_pair->first, false)) { + if (!signer_msg_pair->second.CheckSignature(signer_msg_pair->first, true)) { + mapSporksByHash.erase(signer_msg_pair->second.GetHash()); + itActive->second.erase(signer_msg_pair++); + continue; + } + } + signer_msg_pair++; + } + if (itActive->second.empty()) { + mapSporksActive.erase(itActive++); + continue; } ++itActive; } + auto itByHash = mapSporksByHash.begin(); while (itByHash != mapSporksByHash.end()) { - if (!itByHash->second.CheckSignature(sporkPubKeyID, false)) { - if (!itByHash->second.CheckSignature(sporkPubKeyID, true)) { - mapSporksActive.erase(itByHash->second.nSporkID); - mapSporksByHash.erase(itByHash++); - continue; + bool found = false; + for (const auto& signer: setSporkPubKeyIDs) { + if (itByHash->second.CheckSignature(signer, false) || + itByHash->second.CheckSignature(signer, true)) { + found = true; + break; } } + if (!found) { + mapSporksByHash.erase(itByHash++); + continue; + } ++itByHash; } } From f60fbfb1474c8d19be3cbe383159612d5aae0f4a Mon Sep 17 00:00:00 2001 From: gladcow Date: Thu, 27 Sep 2018 17:29:58 +0300 Subject: [PATCH 24/25] codestyle fixes --- src/spork.cpp | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/spork.cpp b/src/spork.cpp index 49ce3ff98dce..32b6d10896de 100644 --- a/src/spork.cpp +++ b/src/spork.cpp @@ -69,20 +69,20 @@ void CSporkManager::CheckAndRemove() auto itActive = mapSporksActive.begin(); while (itActive != mapSporksActive.end()) { - auto signer_msg_pair = itActive->second.begin(); - while (signer_msg_pair != itActive->second.end()) { - if (setSporkPubKeyIDs.find(signer_msg_pair->first) == setSporkPubKeyIDs.end()) { - mapSporksByHash.erase(signer_msg_pair->second.GetHash()); + auto itSignerPair = itActive->second.begin(); + while (itSignerPair != itActive->second.end()) { + if (setSporkPubKeyIDs.find(itSignerPair->first) == setSporkPubKeyIDs.end()) { + mapSporksByHash.erase(itSignerPair->second.GetHash()); continue; } - if (!signer_msg_pair->second.CheckSignature(signer_msg_pair->first, false)) { - if (!signer_msg_pair->second.CheckSignature(signer_msg_pair->first, true)) { - mapSporksByHash.erase(signer_msg_pair->second.GetHash()); - itActive->second.erase(signer_msg_pair++); + if (!itSignerPair->second.CheckSignature(itSignerPair->first, false)) { + if (!itSignerPair->second.CheckSignature(itSignerPair->first, true)) { + mapSporksByHash.erase(itSignerPair->second.GetHash()); + itActive->second.erase(itSignerPair++); continue; } } - signer_msg_pair++; + ++itSignerPair; } if (itActive->second.empty()) { mapSporksActive.erase(itActive++); @@ -129,13 +129,13 @@ void CSporkManager::ProcessSpork(CNode* pfrom, const std::string& strCommand, CD } CKeyID keyIDSigner; - if (!(spork.GetSignerKeyID(keyIDSigner, IsSporkActive(SPORK_6_NEW_SIGS)) - && setSporkPubKeyIDs.count(keyIDSigner))) { + if (!spork.GetSignerKeyID(keyIDSigner, IsSporkActive(SPORK_6_NEW_SIGS)) + || !setSporkPubKeyIDs.count(keyIDSigner)) { // Note: unlike for other messages we have to check for new format even with SPORK_6_NEW_SIGS // inactive because SPORK_6_NEW_SIGS default is OFF and it is not the first spork to sync // (and even if it would, spork order can't be guaranteed anyway). - if (!(spork.GetSignerKeyID(keyIDSigner, !IsSporkActive(SPORK_6_NEW_SIGS)) - && setSporkPubKeyIDs.count(keyIDSigner))) { + if (!spork.GetSignerKeyID(keyIDSigner, !IsSporkActive(SPORK_6_NEW_SIGS)) + || !setSporkPubKeyIDs.count(keyIDSigner)) { LOCK(cs_main); LogPrintf("CSporkManager::ProcessSpork -- ERROR: invalid signature\n"); Misbehaving(pfrom->GetId(), 100); @@ -178,8 +178,8 @@ void CSporkManager::ProcessSpork(CNode* pfrom, const std::string& strCommand, CD } else if (strCommand == NetMsgType::GETSPORKS) { LOCK(cs); // make sure to not lock this together with cs_main for (const auto& pair : mapSporksActive) { - for (const auto& signer_spork: pair.second) { - connman.PushMessage(pfrom, CNetMsgMaker(pfrom->GetSendVersion()).Make(NetMsgType::SPORK, signer_spork.second)); + for (const auto& signerSporkPair: pair.second) { + connman.PushMessage(pfrom, CNetMsgMaker(pfrom->GetSendVersion()).Make(NetMsgType::SPORK, signerSporkPair.second)); } } } @@ -221,7 +221,7 @@ bool CSporkManager::UpdateSpork(int nSporkID, int64_t nValue, CConnman& connman) if(spork.Sign(sporkPrivKey, IsSporkActive(SPORK_6_NEW_SIGS))) { CKeyID keyIDSigner; - if (!(spork.GetSignerKeyID(keyIDSigner, IsSporkActive(SPORK_6_NEW_SIGS) && setSporkPubKeyIDs.count(keyIDSigner)))) { + if (!spork.GetSignerKeyID(keyIDSigner, IsSporkActive(SPORK_6_NEW_SIGS)) || !setSporkPubKeyIDs.count(keyIDSigner)) { LogPrintf("CSporkManager::UpdateSpork: failed to find keyid for private key\n"); return false; } @@ -339,7 +339,7 @@ bool CSporkManager::SetMinSporkKeys(int minSporkKeys) { int maxKeysNumber = setSporkPubKeyIDs.size(); if ((minSporkKeys <= maxKeysNumber / 2) || (minSporkKeys > maxKeysNumber)) { - LogPrintf("CSporkManager::SetSporkAddress -- Invalid min spork signers number: %d\n", minSporkKeys); + LogPrintf("CSporkManager::SetMinSporkKeys -- Invalid min spork signers number: %d\n", minSporkKeys); return false; } nMinSporkKeys = minSporkKeys; From 8dc93fd6fbae55330a27ef4ab12216a625d3690d Mon Sep 17 00:00:00 2001 From: gladcow Date: Thu, 27 Sep 2018 18:34:06 +0300 Subject: [PATCH 25/25] Correct processing of not actual spork6 value with GetSignerKeyID --- src/spork.cpp | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/spork.cpp b/src/spork.cpp index 32b6d10896de..a9963eb138c2 100644 --- a/src/spork.cpp +++ b/src/spork.cpp @@ -129,12 +129,13 @@ void CSporkManager::ProcessSpork(CNode* pfrom, const std::string& strCommand, CD } CKeyID keyIDSigner; - if (!spork.GetSignerKeyID(keyIDSigner, IsSporkActive(SPORK_6_NEW_SIGS)) + bool fSpork6IsActive = IsSporkActive(SPORK_6_NEW_SIGS); + if (!spork.GetSignerKeyID(keyIDSigner, fSpork6IsActive) || !setSporkPubKeyIDs.count(keyIDSigner)) { // Note: unlike for other messages we have to check for new format even with SPORK_6_NEW_SIGS // inactive because SPORK_6_NEW_SIGS default is OFF and it is not the first spork to sync // (and even if it would, spork order can't be guaranteed anyway). - if (!spork.GetSignerKeyID(keyIDSigner, !IsSporkActive(SPORK_6_NEW_SIGS)) + if (!spork.GetSignerKeyID(keyIDSigner, !fSpork6IsActive) || !setSporkPubKeyIDs.count(keyIDSigner)) { LOCK(cs_main); LogPrintf("CSporkManager::ProcessSpork -- ERROR: invalid signature\n"); @@ -219,9 +220,10 @@ bool CSporkManager::UpdateSpork(int nSporkID, int64_t nValue, CConnman& connman) { CSporkMessage spork = CSporkMessage(nSporkID, nValue, GetAdjustedTime()); - if(spork.Sign(sporkPrivKey, IsSporkActive(SPORK_6_NEW_SIGS))) { + bool fSpork6IsActive = IsSporkActive(SPORK_6_NEW_SIGS); + if(spork.Sign(sporkPrivKey, fSpork6IsActive)) { CKeyID keyIDSigner; - if (!spork.GetSignerKeyID(keyIDSigner, IsSporkActive(SPORK_6_NEW_SIGS)) || !setSporkPubKeyIDs.count(keyIDSigner)) { + if (!spork.GetSignerKeyID(keyIDSigner, fSpork6IsActive) || !setSporkPubKeyIDs.count(keyIDSigner)) { LogPrintf("CSporkManager::UpdateSpork: failed to find keyid for private key\n"); return false; } @@ -478,10 +480,7 @@ bool CSporkMessage::GetSignerKeyID(CKeyID &retKeyidSporkSigner, bool fSporkSixAc ss << strMessageMagic; ss << strMessage; if (!pubkeyFromSig.RecoverCompact(ss.GetHash(), vchSig)) { - // Note: unlike for other messages we have to check for new format even with SPORK_6_NEW_SIGS - // inactive because SPORK_6_NEW_SIGS default is OFF and it is not the first spork to sync - // (and even if it would, spork order can't be guaranteed anyway). - return GetSignerKeyID(retKeyidSporkSigner, true); + return false; } }