From 4116ba3253c028559bcc5007f8fc6afe967b7c3e Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Thu, 25 Jul 2024 01:59:11 +0300 Subject: [PATCH 01/21] fix: let internal govobj/vote inv request trackers expire after 60 sec --- src/governance/governance.cpp | 48 +++++++++++++++++++++++++---------- src/governance/governance.h | 6 ++--- 2 files changed, 37 insertions(+), 17 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 6b195ea831..3f6da27ccf 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -55,7 +55,6 @@ CGovernanceManager::CGovernanceManager(CMasternodeMetaMan& mn_metaman, CNetFulfi m_mn_sync{mn_sync}, nTimeLastDiff(0), nCachedBlockHeight(0), - setRequestedObjects(), fRateChecksEnabled(true), votedFundingYesTriggerHash(std::nullopt), mapTrigger{} @@ -453,7 +452,25 @@ void CGovernanceManager::CheckAndRemove() } } - LogPrint(BCLog::GOBJECT, "CGovernanceManager::UpdateCachesAndClean -- %s\n", ToString()); + // forget about expired requests + auto m_ro_it = mapRequestedObjects.begin(); + while (m_ro_it != mapRequestedObjects.end()) { + if (m_ro_it->second < nNow) { + mapRequestedObjects.erase(m_ro_it++); + } else { + ++m_ro_it; + } + } + auto m_rv_it = mapRequestedVotes.begin(); + while (m_rv_it != mapRequestedVotes.end()) { + if (m_rv_it->second < nNow) { + mapRequestedVotes.erase(m_rv_it++); + } else { + ++m_rv_it; + } + } + + LogPrint(BCLog::GOBJECT, "CGovernanceManager::UpdateCachesAndClean -- %s, mapRequestedObjects=%d, mapRequestedVotes=%d\n", ToString(), mapRequestedObjects.size(), mapRequestedVotes.size()); } const CGovernanceObject* CGovernanceManager::FindConstGovernanceObject(const uint256& nHash) const @@ -838,22 +855,25 @@ bool CGovernanceManager::ConfirmInventoryRequest(const CInv& inv) } - hash_s_t* setHash = nullptr; + std::map* mapHash = nullptr; + std::string set_name = "n/a"; switch (inv.type) { case MSG_GOVERNANCE_OBJECT: - setHash = &setRequestedObjects; + mapHash = &mapRequestedObjects; + set_name = "object"; break; case MSG_GOVERNANCE_OBJECT_VOTE: - setHash = &setRequestedVotes; + mapHash = &mapRequestedVotes; + set_name = "vote"; break; default: return false; } - const auto& [_itr, inserted] = setHash->insert(inv.hash); + const auto& [_itr, inserted] = mapHash->emplace(inv.hash, GetTime().count() + RELIABLE_PROPAGATION_TIME); if (inserted) { - LogPrint(BCLog::GOBJECT, "CGovernanceManager::ConfirmInventoryRequest added inv to requested set\n"); + LogPrint(BCLog::GOBJECT, "CGovernanceManager::ConfirmInventoryRequest added %s inv to requested set, size=%d\n", set_name, mapHash->size()); } LogPrint(BCLog::GOBJECT, "CGovernanceManager::ConfirmInventoryRequest reached end, returning true\n"); @@ -1333,24 +1353,24 @@ int CGovernanceManager::RequestGovernanceObjectVotes(const std::vector& bool CGovernanceManager::AcceptObjectMessage(const uint256& nHash) { LOCK(cs); - return AcceptMessage(nHash, setRequestedObjects); + return AcceptMessage(nHash, mapRequestedObjects); } bool CGovernanceManager::AcceptVoteMessage(const uint256& nHash) { LOCK(cs); - return AcceptMessage(nHash, setRequestedVotes); + return AcceptMessage(nHash, mapRequestedVotes); } -bool CGovernanceManager::AcceptMessage(const uint256& nHash, hash_s_t& setHash) +bool CGovernanceManager::AcceptMessage(const uint256& nHash, std::map& mapHash) { - auto it = setHash.find(nHash); - if (it == setHash.end()) { + auto it = mapHash.find(nHash); + if (it == mapHash.end()) { // We never requested this return false; } // Only accept one response - setHash.erase(it); + mapHash.erase(it); return true; } @@ -1580,7 +1600,7 @@ void CGovernanceManager::RemoveInvalidVotes() cmapVoteToObject.Erase(voteHash); cmapInvalidVotes.Erase(voteHash); cmmapOrphanVotes.Erase(voteHash); - setRequestedVotes.erase(voteHash); + mapRequestedVotes.erase(voteHash); } } } diff --git a/src/governance/governance.h b/src/governance/governance.h index 40c69af981..7c385ef99e 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -270,8 +270,8 @@ class CGovernanceManager : public GovernanceStore int nCachedBlockHeight; std::map mapPostponedObjects; hash_s_t setAdditionalRelayObjects; - hash_s_t setRequestedObjects; - hash_s_t setRequestedVotes; + std::map mapRequestedObjects; + std::map mapRequestedVotes; bool fRateChecksEnabled; std::optional votedFundingYesTriggerHash; std::map> mapTrigger; @@ -395,7 +395,7 @@ class CGovernanceManager : public GovernanceStore /// Called to indicate a requested vote has been received bool AcceptVoteMessage(const uint256& nHash); - static bool AcceptMessage(const uint256& nHash, hash_s_t& setHash); + static bool AcceptMessage(const uint256& nHash, std::map& mapHash); void CheckOrphanVotes(CGovernanceObject& govobj, PeerManager& peerman); From b57a9220c1e7495177d675c1b6d08ce817cf6c4e Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Fri, 26 Jul 2024 13:33:29 +0300 Subject: [PATCH 02/21] refactor: sqash 2 hash maps into one, use proper naming --- src/governance/governance.cpp | 65 ++++++++--------------------------- src/governance/governance.h | 12 ++----- 2 files changed, 18 insertions(+), 59 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 3f6da27ccf..479253e569 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -171,7 +171,7 @@ PeerMsgRet CGovernanceManager::ProcessMessage(CNode& peer, CConnman& connman, Pe LogPrint(BCLog::GOBJECT, "MNGOVERNANCEOBJECT -- Received object: %s\n", strHash); - if (!AcceptObjectMessage(nHash)) { + if (!AcceptMessage(nHash)) { LogPrint(BCLog::GOBJECT, "MNGOVERNANCEOBJECT -- Received unrequested object: %s\n", strHash); return {}; } @@ -239,7 +239,7 @@ PeerMsgRet CGovernanceManager::ProcessMessage(CNode& peer, CConnman& connman, Pe std::string strHash = nHash.ToString(); - if (!AcceptVoteMessage(nHash)) { + if (!AcceptMessage(nHash)) { LogPrint(BCLog::GOBJECT, "MNGOVERNANCEOBJECTVOTE -- Received unrequested vote object: %s, hash: %s, peer = %d\n", vote.ToString(tip_mn_list), strHash, peer.GetId()); return {}; @@ -453,24 +453,16 @@ void CGovernanceManager::CheckAndRemove() } // forget about expired requests - auto m_ro_it = mapRequestedObjects.begin(); - while (m_ro_it != mapRequestedObjects.end()) { - if (m_ro_it->second < nNow) { - mapRequestedObjects.erase(m_ro_it++); + auto r_it = m_requested_hash_time.begin(); + while (r_it != m_requested_hash_time.end()) { + if (r_it->second < nNow) { + m_requested_hash_time.erase(r_it++); } else { - ++m_ro_it; - } - } - auto m_rv_it = mapRequestedVotes.begin(); - while (m_rv_it != mapRequestedVotes.end()) { - if (m_rv_it->second < nNow) { - mapRequestedVotes.erase(m_rv_it++); - } else { - ++m_rv_it; + ++r_it; } } - LogPrint(BCLog::GOBJECT, "CGovernanceManager::UpdateCachesAndClean -- %s, mapRequestedObjects=%d, mapRequestedVotes=%d\n", ToString(), mapRequestedObjects.size(), mapRequestedVotes.size()); + LogPrint(BCLog::GOBJECT, "CGovernanceManager::UpdateCachesAndClean -- %s, m_requested_hash_time size=%d\n", ToString(), m_requested_hash_time.size()); } const CGovernanceObject* CGovernanceManager::FindConstGovernanceObject(const uint256& nHash) const @@ -854,26 +846,10 @@ bool CGovernanceManager::ConfirmInventoryRequest(const CInv& inv) return false; } - - std::map* mapHash = nullptr; - std::string set_name = "n/a"; - switch (inv.type) { - case MSG_GOVERNANCE_OBJECT: - mapHash = &mapRequestedObjects; - set_name = "object"; - break; - case MSG_GOVERNANCE_OBJECT_VOTE: - mapHash = &mapRequestedVotes; - set_name = "vote"; - break; - default: - return false; - } - - const auto& [_itr, inserted] = mapHash->emplace(inv.hash, GetTime().count() + RELIABLE_PROPAGATION_TIME); + const auto& [_itr, inserted] = m_requested_hash_time.emplace(inv.hash, GetTime().count() + RELIABLE_PROPAGATION_TIME); if (inserted) { - LogPrint(BCLog::GOBJECT, "CGovernanceManager::ConfirmInventoryRequest added %s inv to requested set, size=%d\n", set_name, mapHash->size()); + LogPrint(BCLog::GOBJECT, "CGovernanceManager::ConfirmInventoryRequest added %s inv hash to m_requested_hash_time, size=%d\n", MSG_GOVERNANCE_OBJECT ? "object" : "vote", m_requested_hash_time.size()); } LogPrint(BCLog::GOBJECT, "CGovernanceManager::ConfirmInventoryRequest reached end, returning true\n"); @@ -1350,27 +1326,16 @@ int CGovernanceManager::RequestGovernanceObjectVotes(const std::vector& return int(vTriggerObjHashes.size() + vOtherObjHashes.size()); } -bool CGovernanceManager::AcceptObjectMessage(const uint256& nHash) +bool CGovernanceManager::AcceptMessage(const uint256& nHash) { LOCK(cs); - return AcceptMessage(nHash, mapRequestedObjects); -} - -bool CGovernanceManager::AcceptVoteMessage(const uint256& nHash) -{ - LOCK(cs); - return AcceptMessage(nHash, mapRequestedVotes); -} - -bool CGovernanceManager::AcceptMessage(const uint256& nHash, std::map& mapHash) -{ - auto it = mapHash.find(nHash); - if (it == mapHash.end()) { + auto it = m_requested_hash_time.find(nHash); + if (it == m_requested_hash_time.end()) { // We never requested this return false; } // Only accept one response - mapHash.erase(it); + m_requested_hash_time.erase(it); return true; } @@ -1600,7 +1565,7 @@ void CGovernanceManager::RemoveInvalidVotes() cmapVoteToObject.Erase(voteHash); cmapInvalidVotes.Erase(voteHash); cmmapOrphanVotes.Erase(voteHash); - mapRequestedVotes.erase(voteHash); + m_requested_hash_time.erase(voteHash); } } } diff --git a/src/governance/governance.h b/src/governance/governance.h index 7c385ef99e..9e54952534 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -270,8 +270,7 @@ class CGovernanceManager : public GovernanceStore int nCachedBlockHeight; std::map mapPostponedObjects; hash_s_t setAdditionalRelayObjects; - std::map mapRequestedObjects; - std::map mapRequestedVotes; + std::map m_requested_hash_time; bool fRateChecksEnabled; std::optional votedFundingYesTriggerHash; std::map> mapTrigger; @@ -389,13 +388,8 @@ class CGovernanceManager : public GovernanceStore bool ProcessVote(CNode* pfrom, const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman); - /// Called to indicate a requested object has been received - bool AcceptObjectMessage(const uint256& nHash); - - /// Called to indicate a requested vote has been received - bool AcceptVoteMessage(const uint256& nHash); - - static bool AcceptMessage(const uint256& nHash, std::map& mapHash); + /// Called to indicate a requested object or vote has been received + bool AcceptMessage(const uint256& nHash); void CheckOrphanVotes(CGovernanceObject& govobj, PeerManager& peerman); From d6fe7146ff9525793def61487404630e9622e828 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Mon, 5 Aug 2024 16:04:11 +0300 Subject: [PATCH 03/21] chore: make clang-format and linter happy --- src/governance/governance.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 479253e569..82eed67a33 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -462,7 +462,8 @@ void CGovernanceManager::CheckAndRemove() } } - LogPrint(BCLog::GOBJECT, "CGovernanceManager::UpdateCachesAndClean -- %s, m_requested_hash_time size=%d\n", ToString(), m_requested_hash_time.size()); + LogPrint(BCLog::GOBJECT, "CGovernanceManager::UpdateCachesAndClean -- %s, m_requested_hash_time size=%d\n", + ToString(), m_requested_hash_time.size()); } const CGovernanceObject* CGovernanceManager::FindConstGovernanceObject(const uint256& nHash) const @@ -846,10 +847,13 @@ bool CGovernanceManager::ConfirmInventoryRequest(const CInv& inv) return false; } - const auto& [_itr, inserted] = m_requested_hash_time.emplace(inv.hash, GetTime().count() + RELIABLE_PROPAGATION_TIME); + const auto valid_until = GetTime().count() + RELIABLE_PROPAGATION_TIME; + const auto& [_itr, inserted] = m_requested_hash_time.emplace(inv.hash, valid_until); if (inserted) { - LogPrint(BCLog::GOBJECT, "CGovernanceManager::ConfirmInventoryRequest added %s inv hash to m_requested_hash_time, size=%d\n", MSG_GOVERNANCE_OBJECT ? "object" : "vote", m_requested_hash_time.size()); + LogPrint(BCLog::GOBJECT, /* Continued */ + "CGovernanceManager::ConfirmInventoryRequest added %s inv hash to m_requested_hash_time, size=%d\n", + MSG_GOVERNANCE_OBJECT ? "object" : "vote", m_requested_hash_time.size()); } LogPrint(BCLog::GOBJECT, "CGovernanceManager::ConfirmInventoryRequest reached end, returning true\n"); From d41d87a5be947fdd980c9fb72613f7f7e440662e Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Mon, 12 Aug 2024 23:32:48 +0300 Subject: [PATCH 04/21] fix: use `std::chrono::seconds` --- src/governance/governance.cpp | 4 ++-- src/governance/governance.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 82eed67a33..8c99b5b27b 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -455,7 +455,7 @@ void CGovernanceManager::CheckAndRemove() // forget about expired requests auto r_it = m_requested_hash_time.begin(); while (r_it != m_requested_hash_time.end()) { - if (r_it->second < nNow) { + if (r_it->second < std::chrono::seconds(nNow)) { m_requested_hash_time.erase(r_it++); } else { ++r_it; @@ -847,7 +847,7 @@ bool CGovernanceManager::ConfirmInventoryRequest(const CInv& inv) return false; } - const auto valid_until = GetTime().count() + RELIABLE_PROPAGATION_TIME; + const auto valid_until = GetTime() + std::chrono::seconds(RELIABLE_PROPAGATION_TIME); const auto& [_itr, inserted] = m_requested_hash_time.emplace(inv.hash, valid_until); if (inserted) { diff --git a/src/governance/governance.h b/src/governance/governance.h index 9e54952534..9e38f411fa 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -270,7 +270,7 @@ class CGovernanceManager : public GovernanceStore int nCachedBlockHeight; std::map mapPostponedObjects; hash_s_t setAdditionalRelayObjects; - std::map m_requested_hash_time; + std::map m_requested_hash_time; bool fRateChecksEnabled; std::optional votedFundingYesTriggerHash; std::map> mapTrigger; From c7c930ece6c15474f939483015634c03653249aa Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Thu, 22 Aug 2024 21:30:26 +0300 Subject: [PATCH 05/21] fix: use correct condition in logs --- src/governance/governance.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 8c99b5b27b..89336190ed 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -853,7 +853,7 @@ bool CGovernanceManager::ConfirmInventoryRequest(const CInv& inv) if (inserted) { LogPrint(BCLog::GOBJECT, /* Continued */ "CGovernanceManager::ConfirmInventoryRequest added %s inv hash to m_requested_hash_time, size=%d\n", - MSG_GOVERNANCE_OBJECT ? "object" : "vote", m_requested_hash_time.size()); + inv.type == MSG_GOVERNANCE_OBJECT ? "object" : "vote", m_requested_hash_time.size()); } LogPrint(BCLog::GOBJECT, "CGovernanceManager::ConfirmInventoryRequest reached end, returning true\n"); From 06b4dba0d36a2f8d62390737f4ee5703cad30547 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Thu, 22 Aug 2024 21:49:40 +0300 Subject: [PATCH 06/21] feat: make CInv python implementation aware of governance invs --- test/functional/test_framework/messages.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index 37e7215595..4ccb3aeb8a 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -59,6 +59,8 @@ MSG_TX = 1 MSG_BLOCK = 2 MSG_FILTERED_BLOCK = 3 +MSG_GOVERNANCE_OBJECT = 17 +MSG_GOVERNANCE_OBJECT_VOTE = 18 MSG_CMPCT_BLOCK = 20 MSG_TYPE_MASK = 0xffffffff >> 2 @@ -350,6 +352,8 @@ class CInv: MSG_TX: "TX", MSG_BLOCK: "Block", MSG_FILTERED_BLOCK: "filtered Block", + MSG_GOVERNANCE_OBJECT: "Governance Object", + MSG_GOVERNANCE_OBJECT_VOTE: "Governance Vote", MSG_CMPCT_BLOCK: "CompactBlock", } From 1f4e1a17edf728f03ef9942e219e354b16ba43de Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Thu, 22 Aug 2024 22:03:48 +0300 Subject: [PATCH 07/21] test: add test for governance inv expiration --- test/functional/p2p_governance_invs.py | 62 ++++++++++++++++++++++++++ test/functional/test_runner.py | 1 + 2 files changed, 63 insertions(+) create mode 100755 test/functional/p2p_governance_invs.py diff --git a/test/functional/p2p_governance_invs.py b/test/functional/p2p_governance_invs.py new file mode 100755 index 0000000000..4679d18cc2 --- /dev/null +++ b/test/functional/p2p_governance_invs.py @@ -0,0 +1,62 @@ +#!/usr/bin/env python3 +# Copyright (c) 2024 The Dash Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +""" +Test inv expiration for governance objects/votes +""" + +from test_framework.messages import ( + CInv, + msg_inv, + MSG_GOVERNANCE_OBJECT, + MSG_GOVERNANCE_OBJECT_VOTE, +) +from test_framework.p2p import P2PInterface +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import force_finish_mnsync + +RELIABLE_PROPAGATION_TIME = 60 # src/governance/governance.cpp +DATA_CLEANUP_TIME = 5 * 60 # src/init.cpp +MSG_INV_ADDED = 'CGovernanceManager::ConfirmInventoryRequest added {} inv hash to m_requested_hash_time' + +class GovernanceInvsTest(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 1 + + def run_test(self): + node = self.nodes[0] + force_finish_mnsync(node) + inv = msg_inv([CInv(MSG_GOVERNANCE_OBJECT, 1)]) + self.test_request_expiration(inv, "object") + inv = msg_inv([CInv(MSG_GOVERNANCE_OBJECT_VOTE, 2)]) + self.test_request_expiration(inv, "vote") + + def test_request_expiration(self, inv, name): + msg = MSG_INV_ADDED.format(name) + node = self.nodes[0] + peer = node.add_p2p_connection(P2PInterface()) + self.log.info(f"Send dummy governance {name} inv and make sure it's added to requested map") + with node.assert_debug_log([msg]): + peer.send_message(inv) + self.log.info(f"Send dummy governance {name} inv again and make sure it's not added because we know about it already") + with node.assert_debug_log([], [msg]): + peer.send_message(inv) + self.log.info("Force internal cleanup") + with node.assert_debug_log(['UpdateCachesAndClean']): + node.mockscheduler(DATA_CLEANUP_TIME + 1) + self.log.info(f"Send dummy governance {name} inv again and make sure it's not added because we still know about it") + with node.assert_debug_log([], [msg]): + peer.send_message(inv) + self.log.info(f"Bump mocktime, force internal cleanup, send dummy governance {name} inv again and make sure it's accepted again") + self.bump_mocktime(RELIABLE_PROPAGATION_TIME + 1, nodes=[node]) + with node.assert_debug_log(['UpdateCachesAndClean']): + node.mockscheduler(DATA_CLEANUP_TIME + 1) + with node.assert_debug_log([msg]): + peer.send_message(inv) + node.disconnect_p2ps() + + +if __name__ == '__main__': + GovernanceInvsTest().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index e2b769def8..9cd58646d7 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -293,6 +293,7 @@ 'feature_governance.py --descriptors', 'feature_governance_cl.py --legacy-wallet', 'feature_governance_cl.py --descriptors', + 'p2p_governance_invs.py', 'rpc_uptime.py', 'feature_discover.py', 'wallet_resendwallettransactions.py --legacy-wallet', From cbc4c63f58f2e17ee4bf21c87a9d1cec9c3e4b89 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 5 Aug 2021 12:14:38 +0200 Subject: [PATCH 08/21] Merge bitcoin/bitcoin#22619: test: refactor: use consistent bytes <-> hex-string conversion in functional test framework MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 5a1bef60a03b57de708a1a751bd90b8245fd8b83 test: refactor: remove binascii from test_framework (Zero-1729) Pull request description: This PR continues the work started in PR #22593, regarding using the `bytes` built-in module. In this PR specifically, instances of `binascii`'s methods `hexlify`, `unhexlify`, and `a2b_hex` have been replaced with the build-in `bytes` module's `hex` and `fromhex` methods where appropriate to make bytes <-> hex-string conversions consistent across the functional test files and test_framework. Additionally, certain changes made are based on the following assumption: ``` bytes.hex(data) == binascii.hexlify(data).decode() bytes.hex(data).encode() == binascii.hexlify(data) ``` Ran the functional tests to ensure behaviour is still consistent and changes didn't break existing tests. closes #22605 ACKs for top commit: theStack: Code-review ACK 5a1bef60a03b57de708a1a751bd90b8245fd8b83 🔢 Tree-SHA512: 8f28076cf0580a0d02a156f3e1e94c9badd3d41c3fbdfb2b87cd8a761dde2c94faa5f4c448d6747b1ccc9111c3ef1a1d7b42a11c806b241fa0410b7529e2445f Signed-off-by: Vijay --- test/functional/feature_addressindex.py | 10 ++++------ test/functional/feature_spentindex.py | 5 ++--- test/functional/feature_txindex.py | 4 +--- test/functional/interface_rest.py | 5 ++--- test/functional/rpc_createmultisig.py | 5 ++--- test/functional/test_framework/bdb.py | 8 ++++---- test/functional/test_framework/blocktools.py | 3 +-- test/functional/test_framework/netutil.py | 3 +-- 8 files changed, 17 insertions(+), 26 deletions(-) diff --git a/test/functional/feature_addressindex.py b/test/functional/feature_addressindex.py index 1cf66fd5d2..7def5b31fb 100755 --- a/test/functional/feature_addressindex.py +++ b/test/functional/feature_addressindex.py @@ -7,8 +7,6 @@ # Test addressindex generation and fetching # -import binascii - from test_framework.messages import COIN, COutPoint, CTransaction, CTxIn, CTxOut from test_framework.test_framework import BitcoinTestFramework from test_framework.test_node import ErrorMatch @@ -132,7 +130,7 @@ def run_test(self): # Check that outputs with the same address will only return one txid self.log.info("Testing for txid uniqueness...") - addressHash = binascii.unhexlify("FE30B718DCF0BF8A2A686BF1820C073F8B2C3B37") + addressHash = bytes.fromhex("FE30B718DCF0BF8A2A686BF1820C073F8B2C3B37") scriptPubKey = CScript([OP_HASH160, addressHash, OP_EQUAL]) unspent = self.nodes[0].listunspent() tx = CTransaction() @@ -159,7 +157,7 @@ def run_test(self): self.log.info("Testing balances after spending...") privkey2 = "cU4zhap7nPJAWeMFu4j6jLrfPmqakDAzy8zn8Fhb3oEevdm4e5Lc" address2 = "yeMpGzMj3rhtnz48XsfpB8itPHhHtgxLc3" - addressHash2 = binascii.unhexlify("C5E4FB9171C22409809A3E8047A29C83886E325D") + addressHash2 = bytes.fromhex("C5E4FB9171C22409809A3E8047A29C83886E325D") scriptPubKey2 = CScript([OP_DUP, OP_HASH160, addressHash2, OP_EQUALVERIFY, OP_CHECKSIG]) self.nodes[0].importprivkey(privkey2) @@ -254,7 +252,7 @@ def run_test(self): privKey3 = "cRyrMvvqi1dmpiCmjmmATqjAwo6Wu7QTjKu1ABMYW5aFG4VXW99K" address3 = "yWB15aAdpeKuSaQHFVJpBDPbNSLZJSnDLA" - addressHash3 = binascii.unhexlify("6C186B3A308A77C779A9BB71C3B5A7EC28232A13") + addressHash3 = bytes.fromhex("6C186B3A308A77C779A9BB71C3B5A7EC28232A13") scriptPubKey3 = CScript([OP_DUP, OP_HASH160, addressHash3, OP_EQUALVERIFY, OP_CHECKSIG]) # address4 = "2N8oFVB2vThAKury4vnLquW2zVjsYjjAkYQ" scriptPubKey4 = CScript([OP_HASH160, addressHash3, OP_EQUAL]) @@ -320,7 +318,7 @@ def run_test(self): # sending and receiving to the same address privkey1 = "cMvZn1pVWntTEcsK36ZteGQXRAcZ8CoTbMXF1QasxBLdnTwyVQCc" address1 = "yM9Eed1bxjy7tYxD3yZDHxjcVT48WdRoB1" - address1hash = binascii.unhexlify("0909C84A817651502E020AAD0FBCAE5F656E7D8A") + address1hash = bytes.fromhex("0909C84A817651502E020AAD0FBCAE5F656E7D8A") address1script = CScript([OP_DUP, OP_HASH160, address1hash, OP_EQUALVERIFY, OP_CHECKSIG]) self.nodes[0].sendtoaddress(address1, 10) diff --git a/test/functional/feature_spentindex.py b/test/functional/feature_spentindex.py index a8837348db..512ea21ee9 100755 --- a/test/functional/feature_spentindex.py +++ b/test/functional/feature_spentindex.py @@ -7,7 +7,6 @@ # Test addressindex generation and fetching # -import binascii from decimal import Decimal from test_framework.messages import COIN, COutPoint, CTransaction, CTxIn, CTxOut @@ -64,7 +63,7 @@ def run_test(self): self.log.info("Testing spent index...") privkey = "cU4zhap7nPJAWeMFu4j6jLrfPmqakDAzy8zn8Fhb3oEevdm4e5Lc" - addressHash = binascii.unhexlify("C5E4FB9171C22409809A3E8047A29C83886E325D") + addressHash = bytes.fromhex("C5E4FB9171C22409809A3E8047A29C83886E325D") scriptPubKey = CScript([OP_DUP, OP_HASH160, addressHash, OP_EQUALVERIFY, OP_CHECKSIG]) unspent = self.nodes[0].listunspent() tx = CTransaction() @@ -103,7 +102,7 @@ def run_test(self): # Check that verbose raw transaction includes address values and input values address2 = "yeMpGzMj3rhtnz48XsfpB8itPHhHtgxLc3" - addressHash2 = binascii.unhexlify("C5E4FB9171C22409809A3E8047A29C83886E325D") + addressHash2 = bytes.fromhex("C5E4FB9171C22409809A3E8047A29C83886E325D") scriptPubKey2 = CScript([OP_DUP, OP_HASH160, addressHash2, OP_EQUALVERIFY, OP_CHECKSIG]) tx2 = CTransaction() tx2.vin = [CTxIn(COutPoint(int(txid, 16), 0))] diff --git a/test/functional/feature_txindex.py b/test/functional/feature_txindex.py index 1419fea409..64f3c76584 100755 --- a/test/functional/feature_txindex.py +++ b/test/functional/feature_txindex.py @@ -7,8 +7,6 @@ # Test txindex generation and fetching # -import binascii - from test_framework.messages import COutPoint, CTransaction, CTxIn, CTxOut from test_framework.script import CScript, OP_CHECKSIG, OP_DUP, OP_EQUALVERIFY, OP_HASH160 from test_framework.test_framework import BitcoinTestFramework @@ -48,7 +46,7 @@ def run_test(self): self.log.info("Testing transaction index...") - addressHash = binascii.unhexlify("C5E4FB9171C22409809A3E8047A29C83886E325D") + addressHash = bytes.fromhex("C5E4FB9171C22409809A3E8047A29C83886E325D") scriptPubKey = CScript([OP_DUP, OP_HASH160, addressHash, OP_EQUALVERIFY, OP_CHECKSIG]) unspent = self.nodes[0].listunspent() tx = CTransaction() diff --git a/test/functional/interface_rest.py b/test/functional/interface_rest.py index 6c6341f547..1d8505fc72 100755 --- a/test/functional/interface_rest.py +++ b/test/functional/interface_rest.py @@ -4,7 +4,6 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test the REST API.""" -import binascii from decimal import Decimal from enum import Enum from io import BytesIO @@ -246,13 +245,13 @@ def run_test(self): response_hex = self.test_rest_request("/block/{}".format(bb_hash), req_type=ReqType.HEX, ret_type=RetType.OBJ) assert_greater_than(int(response_hex.getheader('content-length')), BLOCK_HEADER_SIZE*2) response_hex_bytes = response_hex.read().strip(b'\n') - assert_equal(binascii.hexlify(response_bytes), response_hex_bytes) + assert_equal(response_bytes.hex().encode(), response_hex_bytes) # Compare with hex block header response_header_hex = self.test_rest_request("/headers/1/{}".format(bb_hash), req_type=ReqType.HEX, ret_type=RetType.OBJ) assert_greater_than(int(response_header_hex.getheader('content-length')), BLOCK_HEADER_SIZE*2) response_header_hex_bytes = response_header_hex.read(BLOCK_HEADER_SIZE*2) - assert_equal(binascii.hexlify(response_bytes[:BLOCK_HEADER_SIZE]), response_header_hex_bytes) + assert_equal(response_bytes[:BLOCK_HEADER_SIZE].hex().encode(), response_header_hex_bytes) # Check json format block_json_obj = self.test_rest_request("/block/{}".format(bb_hash)) diff --git a/test/functional/rpc_createmultisig.py b/test/functional/rpc_createmultisig.py index 06a1316535..fd8450567c 100755 --- a/test/functional/rpc_createmultisig.py +++ b/test/functional/rpc_createmultisig.py @@ -3,7 +3,6 @@ # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test multisig RPCs""" -import binascii import decimal import itertools import json @@ -64,9 +63,9 @@ def run_test(self): # decompress pk2 pk_obj = ECPubKey() - pk_obj.set(binascii.unhexlify(pk2)) + pk_obj.set(bytes.fromhex(pk2)) pk_obj.compressed = False - pk2 = binascii.hexlify(pk_obj.get_bytes()).decode() + pk2 = pk_obj.get_bytes().hex() node0.createwallet(wallet_name='wmulti0', disable_private_keys=True) wmulti0 = node0.get_wallet_rpc('wmulti0') diff --git a/test/functional/test_framework/bdb.py b/test/functional/test_framework/bdb.py index 97b9c1d6d0..d623bcdf6e 100644 --- a/test/functional/test_framework/bdb.py +++ b/test/functional/test_framework/bdb.py @@ -24,7 +24,6 @@ `db_dump -da wallet.dat` is useful to see the data in a wallet.dat BDB file """ -import binascii import struct # Important constants @@ -96,7 +95,7 @@ def dump_meta_page(page): metadata['key_count'] = key_count metadata['record_count'] = record_count metadata['flags'] = flags - metadata['uid'] = binascii.hexlify(uid) + metadata['uid'] = uid.hex().encode() assert magic == BTREE_MAGIC, 'bdb magic does not match bdb btree magic' assert pg_type == BTREE_META, 'Metadata page is not a btree metadata page' @@ -110,8 +109,9 @@ def dump_meta_page(page): metadata['re_pad'] = re_pad metadata['root'] = root metadata['crypto_magic'] = crypto_magic - metadata['iv'] = binascii.hexlify(iv) - metadata['chksum'] = binascii.hexlify(chksum) + metadata['iv'] = iv.hex().encode() + metadata['chksum'] = chksum.hex().encode() + return metadata # Given the dict from dump_leaf_page, get the key-value pairs and put them into a dict diff --git a/test/functional/test_framework/blocktools.py b/test/functional/test_framework/blocktools.py index b12a4749ef..e4b407744c 100644 --- a/test/functional/test_framework/blocktools.py +++ b/test/functional/test_framework/blocktools.py @@ -4,7 +4,6 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Utilities for manipulating blocks and transactions.""" -from binascii import a2b_hex from decimal import Decimal import io import struct @@ -46,7 +45,7 @@ def create_block(hashprev=None, coinbase=None, ntime=None, *, version=None, tmpl block.nTime = ntime or tmpl.get('curtime') or int(time.time() + 600) block.hashPrevBlock = hashprev or int(tmpl['previousblockhash'], 0x10) if tmpl and not tmpl.get('bits') is None: - block.nBits = struct.unpack('>I', a2b_hex(tmpl['bits']))[0] + block.nBits = struct.unpack('>I', bytes.fromhex(tmpl['bits']))[0] else: block.nBits = 0x207fffff # difficulty retargeting is disabled in REGTEST chainparams if coinbase is None: diff --git a/test/functional/test_framework/netutil.py b/test/functional/test_framework/netutil.py index 172b4a592e..21145f90e9 100644 --- a/test/functional/test_framework/netutil.py +++ b/test/functional/test_framework/netutil.py @@ -12,7 +12,6 @@ import struct import array import os -from binascii import unhexlify # STATE_ESTABLISHED = '01' # STATE_SYN_SENT = '02' @@ -44,7 +43,7 @@ def _remove_empty(array): def _convert_ip_port(array): host,port = array.split(':') # convert host from mangled-per-four-bytes form as used by kernel - host = unhexlify(host) + host = bytes.fromhex(host) host_out = '' for x in range(0, len(host) // 4): (val,) = struct.unpack('=I', host[x*4:(x+1)*4]) From ad840ec31aca0156272dde9f1bda523bbd444edb Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sat, 31 Jul 2021 21:23:16 +0200 Subject: [PATCH 09/21] Merge bitcoin#22593: remove `hex_str_to_bytes` helper Signed-off-by: Vijay --- test/functional/feature_asset_locks.py | 3 +-- test/functional/feature_dbcrash.py | 3 +-- test/functional/feature_llmq_chainlocks.py | 8 ++++---- test/functional/feature_llmq_is_cl_conflicts.py | 10 +++++----- test/functional/feature_llmq_signing.py | 4 ++-- test/functional/interface_rest.py | 3 +-- test/functional/rpc_addresses_deprecation.py | 5 ++--- test/functional/rpc_decodescript.py | 9 ++++----- test/functional/rpc_mnauth.py | 4 ++-- test/functional/test_framework/address.py | 8 ++++---- test/functional/test_framework/blocktools.py | 4 ++-- test/functional/test_framework/messages.py | 4 ++-- test/functional/test_framework/script_util.py | 5 ++--- test/functional/test_framework/test_framework.py | 3 +-- test/functional/test_framework/util.py | 7 +------ test/functional/test_framework/wallet.py | 3 +-- test/functional/test_framework/wallet_util.py | 7 +++---- 17 files changed, 38 insertions(+), 52 deletions(-) diff --git a/test/functional/feature_asset_locks.py b/test/functional/feature_asset_locks.py index 7c3593cf20..f83aa6e2e3 100755 --- a/test/functional/feature_asset_locks.py +++ b/test/functional/feature_asset_locks.py @@ -41,7 +41,6 @@ assert_equal, assert_greater_than, assert_greater_than_or_equal, - hex_str_to_bytes, ) from test_framework.wallet_util import bytes_to_wif @@ -170,7 +169,7 @@ def create_and_check_block(self, txes, expected_error = None): cbb = create_coinbase(height, dip4_activated=True, v20_activated=True) gbt = node_wallet.getblocktemplate() - cbb.vExtraPayload = hex_str_to_bytes(gbt["coinbase_payload"]) + cbb.vExtraPayload = bytes.fromhex(gbt["coinbase_payload"]) cbb.rehash() block = create_block(tip, cbb, block_time, version=4) # Add quorum commitments from block template diff --git a/test/functional/feature_dbcrash.py b/test/functional/feature_dbcrash.py index b91aa2eebf..b40152e3b6 100755 --- a/test/functional/feature_dbcrash.py +++ b/test/functional/feature_dbcrash.py @@ -41,7 +41,6 @@ from test_framework.util import ( assert_equal, create_confirmed_utxos, - hex_str_to_bytes, ) @@ -204,7 +203,7 @@ def generate_small_transactions(self, node, count, utxo_list): continue for _ in range(3): - tx.vout.append(CTxOut(output_amount, hex_str_to_bytes(utxo['scriptPubKey']))) + tx.vout.append(CTxOut(output_amount, bytes.fromhex(utxo['scriptPubKey']))) # Sign and send the transaction to get into the mempool tx_signed_hex = node.signrawtransactionwithwallet(tx.serialize().hex())['hex'] diff --git a/test/functional/feature_llmq_chainlocks.py b/test/functional/feature_llmq_chainlocks.py index 4e1618b582..c9698c5575 100755 --- a/test/functional/feature_llmq_chainlocks.py +++ b/test/functional/feature_llmq_chainlocks.py @@ -15,7 +15,7 @@ from test_framework.messages import CBlock, CCbTx from test_framework.test_framework import DashTestFramework -from test_framework.util import assert_equal, assert_raises_rpc_error, force_finish_mnsync, hex_str_to_bytes, softfork_active +from test_framework.util import assert_equal, assert_raises_rpc_error, force_finish_mnsync, softfork_active class LLMQChainLocksTest(DashTestFramework): @@ -309,7 +309,7 @@ def test_bestCLHeightDiff(self, mn_rr_active): tip0_hash = self.nodes[0].generate(1)[0] block_hex = self.nodes[0].getblock(tip0_hash, 0) mal_block = CBlock() - mal_block.deserialize(BytesIO(hex_str_to_bytes(block_hex))) + mal_block.deserialize(BytesIO(bytes.fromhex(block_hex))) cbtx = CCbTx() cbtx.deserialize(BytesIO(mal_block.vtx[0].vExtraPayload)) assert_equal(cbtx.bestCLHeightDiff, 0) @@ -324,7 +324,7 @@ def test_bestCLHeightDiff(self, mn_rr_active): assert_equal(self.nodes[1].getbestblockhash(), tip1_hash) # Update the sig too and it should pass now - cbtx.bestCLSignature = hex_str_to_bytes(self.nodes[1].getblock(tip1_hash, 2)["tx"][0]["cbTx"]["bestCLSignature"]) + cbtx.bestCLSignature = bytes.fromhex(self.nodes[1].getblock(tip1_hash, 2)["tx"][0]["cbTx"]["bestCLSignature"]) mal_block.vtx[0].vExtraPayload = cbtx.serialize() mal_block.vtx[0].rehash() mal_block.hashMerkleRoot = mal_block.calc_merkle_root() @@ -349,7 +349,7 @@ def test_bestCLHeightDiff(self, mn_rr_active): # Update the sig too and it should pass now when mn_rr is not active and fail otherwise old_blockhash = self.nodes[1].getblockhash(self.nodes[1].getblockcount() - 1) - cbtx.bestCLSignature = hex_str_to_bytes(self.nodes[1].getblock(old_blockhash, 2)["tx"][0]["cbTx"]["bestCLSignature"]) + cbtx.bestCLSignature = bytes.fromhex(self.nodes[1].getblock(old_blockhash, 2)["tx"][0]["cbTx"]["bestCLSignature"]) mal_block.vtx[0].vExtraPayload = cbtx.serialize() mal_block.vtx[0].rehash() mal_block.hashMerkleRoot = mal_block.calc_merkle_root() diff --git a/test/functional/feature_llmq_is_cl_conflicts.py b/test/functional/feature_llmq_is_cl_conflicts.py index 3af1b35041..4c329ab93a 100755 --- a/test/functional/feature_llmq_is_cl_conflicts.py +++ b/test/functional/feature_llmq_is_cl_conflicts.py @@ -16,7 +16,7 @@ from test_framework.messages import CInv, hash256, msg_clsig, msg_inv, ser_string, tx_from_hex, uint256_from_str from test_framework.p2p import P2PInterface from test_framework.test_framework import DashTestFramework -from test_framework.util import assert_equal, assert_raises_rpc_error, hex_str_to_bytes +from test_framework.util import assert_equal, assert_raises_rpc_error class TestP2PConn(P2PInterface): @@ -92,7 +92,7 @@ def test_chainlock_overrides_islock(self, test_block_conflict, mine_confllicting rawtx2_obj = tx_from_hex(rawtx2) rawtx1_txid = self.nodes[0].sendrawtransaction(rawtx1) - rawtx2_txid = hash256(hex_str_to_bytes(rawtx2))[::-1].hex() + rawtx2_txid = hash256(bytes.fromhex(rawtx2))[::-1].hex() # Create a chained TX on top of tx1 inputs = [] @@ -205,8 +205,8 @@ def test_chainlock_overrides_islock_overrides_nonchainlock(self): rawtx1 = self.create_raw_tx(self.nodes[0], self.nodes[0], 1, 1, 100)['hex'] rawtx2 = self.create_raw_tx(self.nodes[0], self.nodes[0], 1, 1, 100)['hex'] - rawtx1_txid = hash256(hex_str_to_bytes(rawtx1))[::-1].hex() - rawtx2_txid = hash256(hex_str_to_bytes(rawtx2))[::-1].hex() + rawtx1_txid = hash256(bytes.fromhex(rawtx1))[::-1].hex() + rawtx2_txid = hash256(bytes.fromhex(rawtx2))[::-1].hex() # Create an ISLOCK but don't broadcast it yet isdlock = self.create_isdlock(rawtx2) @@ -280,7 +280,7 @@ def create_chainlock(self, height, block): message_hash = block.hash recSig = self.get_recovered_sig(request_id, message_hash) - clsig = msg_clsig(height, block.sha256, hex_str_to_bytes(recSig['sig'])) + clsig = msg_clsig(height, block.sha256, bytes.fromhex(recSig['sig'])) return clsig if __name__ == '__main__': diff --git a/test/functional/feature_llmq_signing.py b/test/functional/feature_llmq_signing.py index cd59619566..b2547a9d1f 100755 --- a/test/functional/feature_llmq_signing.py +++ b/test/functional/feature_llmq_signing.py @@ -13,7 +13,7 @@ from test_framework.messages import CSigShare, msg_qsigshare, uint256_to_string from test_framework.p2p import P2PInterface from test_framework.test_framework import DashTestFramework -from test_framework.util import assert_equal, assert_raises_rpc_error, force_finish_mnsync, hex_str_to_bytes, wait_until_helper +from test_framework.util import assert_equal, assert_raises_rpc_error, force_finish_mnsync, wait_until_helper class LLMQSigningTest(DashTestFramework): @@ -89,7 +89,7 @@ def assert_sigs_nochange(hasrecsigs, isconflicting1, isconflicting2, timeout): sig_share.quorumMember = int(sig_share_rpc_1["quorumMember"]) sig_share.id = int(sig_share_rpc_1["id"], 16) sig_share.msgHash = int(sig_share_rpc_1["msgHash"], 16) - sig_share.sigShare = hex_str_to_bytes(sig_share_rpc_1["signature"]) + sig_share.sigShare = bytes.fromhex(sig_share_rpc_1["signature"]) for mn in self.mninfo: assert mn.node.getconnectioncount() == self.llmq_size # Get the current recovery member of the quorum diff --git a/test/functional/interface_rest.py b/test/functional/interface_rest.py index 1d8505fc72..faa92e6675 100755 --- a/test/functional/interface_rest.py +++ b/test/functional/interface_rest.py @@ -18,7 +18,6 @@ assert_equal, assert_greater_than, assert_greater_than_or_equal, - hex_str_to_bytes, ) from test_framework.messages import BLOCK_HEADER_SIZE @@ -156,7 +155,7 @@ def run_test(self): bin_request = b'\x01\x02' for txid, n in [spending, spent]: - bin_request += hex_str_to_bytes(txid) + bin_request += bytes.fromhex(txid) bin_request += pack("i", n) bin_response = self.test_rest_request("/getutxos", http_method='POST', req_type=ReqType.BIN, body=bin_request, ret_type=RetType.BYTES) diff --git a/test/functional/rpc_addresses_deprecation.py b/test/functional/rpc_addresses_deprecation.py index 5cdd109a47..dfeba3ab44 100755 --- a/test/functional/rpc_addresses_deprecation.py +++ b/test/functional/rpc_addresses_deprecation.py @@ -10,7 +10,6 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, - hex_str_to_bytes ) @@ -36,8 +35,8 @@ def test_addresses_deprecation(self): # This transaction is derived from test/util/data/txcreatemultisig1.json tx = CTransaction() - tx.deserialize(BytesIO(hex_str_to_bytes(signed))) - tx.vout[0].scriptPubKey = hex_str_to_bytes("522102a5613bd857b7048924264d1e70e08fb2a7e6527d32b7ab1bb993ac59964ff39721021ac43c7ff740014c3b33737ede99c967e4764553d1b2b83db77c83b8715fa72d2102df2089105c77f266fa11a9d33f05c735234075f2e8780824c6b709415f9fb48553ae") + tx.deserialize(BytesIO(bytes.fromhex(signed))) + tx.vout[0].scriptPubKey = bytes.fromhex("522102a5613bd857b7048924264d1e70e08fb2a7e6527d32b7ab1bb993ac59964ff39721021ac43c7ff740014c3b33737ede99c967e4764553d1b2b83db77c83b8715fa72d2102df2089105c77f266fa11a9d33f05c735234075f2e8780824c6b709415f9fb48553ae") tx_signed = node.signrawtransactionwithwallet(tx.serialize().hex())['hex'] txid = node.sendrawtransaction(hexstring=tx_signed, maxfeerate=0) diff --git a/test/functional/rpc_decodescript.py b/test/functional/rpc_decodescript.py index bf75825e92..438f246af5 100755 --- a/test/functional/rpc_decodescript.py +++ b/test/functional/rpc_decodescript.py @@ -10,7 +10,6 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, - hex_str_to_bytes, ) @@ -154,23 +153,23 @@ def decoderawtransaction_asm_sighashtype(self): signature_2_sighash_decoded = der_signature + '[NONE|ANYONECANPAY]' # 1) P2PK scriptSig - txSave.vin[0].scriptSig = hex_str_to_bytes(push_signature) + txSave.vin[0].scriptSig = bytes.fromhex(push_signature) rpc_result = self.nodes[0].decoderawtransaction(txSave.serialize().hex()) assert_equal(signature_sighash_decoded, rpc_result['vin'][0]['scriptSig']['asm']) # make sure that the sighash decodes come out correctly for a more complex / lesser used case. - txSave.vin[0].scriptSig = hex_str_to_bytes(push_signature_2) + txSave.vin[0].scriptSig = bytes.fromhex(push_signature_2) rpc_result = self.nodes[0].decoderawtransaction(txSave.serialize().hex()) assert_equal(signature_2_sighash_decoded, rpc_result['vin'][0]['scriptSig']['asm']) # 2) multisig scriptSig - txSave.vin[0].scriptSig = hex_str_to_bytes('00' + push_signature + push_signature_2) + txSave.vin[0].scriptSig = bytes.fromhex('00' + push_signature + push_signature_2) rpc_result = self.nodes[0].decoderawtransaction(txSave.serialize().hex()) assert_equal('0 ' + signature_sighash_decoded + ' ' + signature_2_sighash_decoded, rpc_result['vin'][0]['scriptSig']['asm']) # 3) test a scriptSig that contains more than push operations. # in fact, it contains an OP_RETURN with data specially crafted to cause improper decode if the code does not catch it. - txSave.vin[0].scriptSig = hex_str_to_bytes('6a143011020701010101010101020601010101010101') + txSave.vin[0].scriptSig = bytes.fromhex('6a143011020701010101010101020601010101010101') rpc_result = self.nodes[0].decoderawtransaction(txSave.serialize().hex()) assert_equal('OP_RETURN 3011020701010101010101020601010101010101', rpc_result['vin'][0]['scriptSig']['asm']) diff --git a/test/functional/rpc_mnauth.py b/test/functional/rpc_mnauth.py index 965a1efd87..09b4a7c2f9 100755 --- a/test/functional/rpc_mnauth.py +++ b/test/functional/rpc_mnauth.py @@ -6,7 +6,7 @@ from test_framework.messages import hash256 from test_framework.p2p import P2PInterface from test_framework.test_framework import DashTestFramework -from test_framework.util import assert_equal, assert_raises_rpc_error, hex_str_to_bytes +from test_framework.util import assert_equal, assert_raises_rpc_error ''' rpc_mnauth.py @@ -39,7 +39,7 @@ def run_test(self): assert "verified_proregtx_hash" in peerinfo assert "verified_pubkey_hash" in peerinfo assert_equal(peerinfo["verified_proregtx_hash"], protx_hash) - assert_equal(peerinfo["verified_pubkey_hash"], hash256(hex_str_to_bytes(public_key))[::-1].hex()) + assert_equal(peerinfo["verified_pubkey_hash"], hash256(bytes.fromhex(public_key))[::-1].hex()) # Test some error cases null_hash = "0000000000000000000000000000000000000000000000000000000000000000" assert_raises_rpc_error(-8, "proTxHash invalid", masternode.node.mnauth, diff --git a/test/functional/test_framework/address.py b/test/functional/test_framework/address.py index 1312cff489..e7a364799c 100644 --- a/test/functional/test_framework/address.py +++ b/test/functional/test_framework/address.py @@ -12,7 +12,7 @@ import unittest from .script import hash160, hash256, CScript -from .util import assert_equal, hex_str_to_bytes +from .util import assert_equal # Note unlike in bitcoin, this address isn't bech32 since we don't (at this time) support bech32. ADDRESS_BCRT1_UNSPENDABLE = 'yVg3NBUHNEhgDceqwVUjsZHreC5PBHnUo9' @@ -27,7 +27,7 @@ def byte_to_base58(b, version): result = '' str = b.hex() str = chr(version).encode('latin-1').hex() + str - checksum = hash256(hex_str_to_bytes(str)).hex() + checksum = hash256(bytes.fromhex(str)).hex() str += checksum[:8] value = int('0x' + str, 0) while value > 0: @@ -89,14 +89,14 @@ def script_to_p2sh(script, main=False): def check_key(key): if (type(key) is str): - key = hex_str_to_bytes(key) # Assuming this is hex string + key = bytes.fromhex(key) # Assuming this is hex string if (type(key) is bytes and (len(key) == 33 or len(key) == 65)): return key assert False def check_script(script): if (type(script) is str): - script = hex_str_to_bytes(script) # Assuming this is hex string + script = bytes.fromhex(script) # Assuming this is hex string if (type(script) is bytes or type(script) is CScript): return script assert False diff --git a/test/functional/test_framework/blocktools.py b/test/functional/test_framework/blocktools.py index e4b407744c..7746077c92 100644 --- a/test/functional/test_framework/blocktools.py +++ b/test/functional/test_framework/blocktools.py @@ -23,7 +23,7 @@ uint256_to_string, ) from .script import CScript, CScriptNum, CScriptOp, OP_TRUE, OP_CHECKSIG -from .util import assert_equal, hex_str_to_bytes +from .util import assert_equal from io import BytesIO MAX_BLOCK_SIGOPS = 20000 @@ -207,7 +207,7 @@ def create_transaction(node, txid, to_address, *, amount): """ raw_tx = create_raw_transaction(node, txid, to_address, amount=amount) tx = CTransaction() - tx.deserialize(BytesIO(hex_str_to_bytes(raw_tx))) + tx.deserialize(BytesIO(bytes.fromhex(raw_tx))) return tx def create_raw_transaction(node, txid, to_address, *, amount): diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index b98616df86..c91c56d36c 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -27,7 +27,7 @@ import time from test_framework.crypto.siphash import siphash256 -from test_framework.util import hex_str_to_bytes, assert_equal +from test_framework.util import assert_equal import dash_hash @@ -214,7 +214,7 @@ def from_hex(obj, hex_string): Note that there is no complementary helper like e.g. `to_hex` for the inverse operation. To serialize a message object to a hex string, simply use obj.serialize().hex()""" - obj.deserialize(BytesIO(hex_str_to_bytes(hex_string))) + obj.deserialize(BytesIO(bytes.fromhex(hex_string))) return obj diff --git a/test/functional/test_framework/script_util.py b/test/functional/test_framework/script_util.py index 67bd5bb1ce..40381c6fc1 100755 --- a/test/functional/test_framework/script_util.py +++ b/test/functional/test_framework/script_util.py @@ -4,7 +4,6 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Useful Script constants and utils.""" from test_framework.script import CScript, hash160, OP_DUP, OP_HASH160, OP_CHECKSIG, OP_EQUAL, OP_EQUALVERIFY -from test_framework.util import hex_str_to_bytes # To prevent a "tx-size-small" policy rule error, a transaction has to have a # size of at least 83 bytes (MIN_STANDARD_TX_SIZE in @@ -44,14 +43,14 @@ def script_to_p2sh_script(script, main = False): def check_key(key): if isinstance(key, str): - key = hex_str_to_bytes(key) # Assuming this is hex string + key = bytes.fromhex(key) # Assuming this is hex string if isinstance(key, bytes) and (len(key) == 33 or len(key) == 65): return key assert False def check_script(script): if isinstance(script, str): - script = hex_str_to_bytes(script) # Assuming this is hex string + script = bytes.fromhex(script) # Assuming this is hex string if isinstance(script, bytes) or isinstance(script, CScript): return script assert False diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index e3abe2fd2e..2e0c34d32f 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -48,7 +48,6 @@ force_finish_mnsync, get_bip9_details, get_datadir_path, - hex_str_to_bytes, initialize_datadir, p2p_port, set_node_times, @@ -1596,7 +1595,7 @@ def create_isdlock(self, hextx): block_count = self.mninfo[0].node.getblockcount() cycle_hash = int(self.mninfo[0].node.getblockhash(block_count - (block_count % 24)), 16) - isdlock = msg_isdlock(1, inputs, tx.sha256, cycle_hash, hex_str_to_bytes(rec_sig['sig'])) + isdlock = msg_isdlock(1, inputs, tx.sha256, cycle_hash, bytes.fromhex(rec_sig['sig'])) return isdlock diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index ec3c6dcde1..95d52a7f2b 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -6,7 +6,6 @@ """Helpful routines for regression testing.""" from base64 import b64encode -from binascii import unhexlify from decimal import Decimal, ROUND_DOWN from subprocess import CalledProcessError import hashlib @@ -215,10 +214,6 @@ def count_bytes(hex_string): return len(bytearray.fromhex(hex_string)) -def hex_str_to_bytes(hex_str): - return unhexlify(hex_str.encode('ascii')) - - def str_to_b64str(string): return b64encode(string.encode('utf-8')).decode('ascii') @@ -558,7 +553,7 @@ def gen_return_txouts(): from .messages import CTxOut txout = CTxOut() txout.nValue = 0 - txout.scriptPubKey = hex_str_to_bytes(script_pubkey) + txout.scriptPubKey = bytes.fromhex(script_pubkey) for _ in range(128): txouts.append(txout) return txouts diff --git a/test/functional/test_framework/wallet.py b/test/functional/test_framework/wallet.py index 829e05ebe4..5c789e4b38 100644 --- a/test/functional/test_framework/wallet.py +++ b/test/functional/test_framework/wallet.py @@ -26,7 +26,6 @@ ) from test_framework.util import ( assert_equal, - hex_str_to_bytes, satoshi_round, ) @@ -72,7 +71,7 @@ def __init__(self, test_node, *, mode=MiniWalletMode.ADDRESS_OP_TRUE): self._scriptPubKey = bytes(CScript([pub_key.get_bytes(), OP_CHECKSIG])) elif mode == MiniWalletMode.ADDRESS_OP_TRUE: self._address = ADDRESS_BCRT1_P2SH_OP_TRUE - self._scriptPubKey = hex_str_to_bytes(self._test_node.validateaddress(self._address)['scriptPubKey']) + self._scriptPubKey = bytes.fromhex(self._test_node.validateaddress(self._address)['scriptPubKey']) def scan_blocks(self, *, start=1, num): """Scan the blocks for self._address outputs and add them to self._utxos""" diff --git a/test/functional/test_framework/wallet_util.py b/test/functional/test_framework/wallet_util.py index c3ad36bb7e..89cb3a2eed 100755 --- a/test/functional/test_framework/wallet_util.py +++ b/test/functional/test_framework/wallet_util.py @@ -23,7 +23,6 @@ OP_HASH160, hash160, ) -from test_framework.util import hex_str_to_bytes Key = namedtuple('Key', ['privkey', 'pubkey', @@ -42,7 +41,7 @@ def get_key(node): Returns a named tuple of privkey, pubkey and all address and scripts.""" addr = node.getnewaddress() pubkey = node.getaddressinfo(addr)['pubkey'] - pkh = hash160(hex_str_to_bytes(pubkey)) + pkh = hash160(bytes.fromhex(pubkey)) return Key(privkey=node.dumpprivkey(addr), pubkey=pubkey, p2pkh_script=CScript([OP_DUP, OP_HASH160, pkh, OP_EQUALVERIFY, OP_CHECKSIG]).hex(), @@ -56,7 +55,7 @@ def get_generate_key(): eckey.generate() privkey = bytes_to_wif(eckey.get_bytes()) pubkey = eckey.get_pubkey().get_bytes().hex() - pkh = hash160(hex_str_to_bytes(pubkey)) + pkh = hash160(bytes.fromhex(pubkey)) return Key(privkey=privkey, pubkey=pubkey, p2pkh_script=CScript([OP_DUP, OP_HASH160, pkh, OP_EQUALVERIFY, OP_CHECKSIG]).hex(), @@ -72,7 +71,7 @@ def get_multisig(node): addr = node.getaddressinfo(node.getnewaddress()) addrs.append(addr['address']) pubkeys.append(addr['pubkey']) - script_code = CScript([OP_2] + [hex_str_to_bytes(pubkey) for pubkey in pubkeys] + [OP_3, OP_CHECKMULTISIG]) + script_code = CScript([OP_2] + [bytes.fromhex(pubkey) for pubkey in pubkeys] + [OP_3, OP_CHECKMULTISIG]) return Multisig(privkeys=[node.dumpprivkey(addr) for addr in addrs], pubkeys=pubkeys, p2sh_script=CScript([OP_HASH160, hash160(script_code), OP_EQUAL]).hex(), From d5e32e1b7965f70acc9a708f5d8d0158cd1b2558 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Thu, 30 May 2024 17:24:12 +0700 Subject: [PATCH 10/21] fix: disable gsl iostream feature for faster compile time We do not use this feature and implementation is trivial: just call `get()` --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 975936501c..26a7f44bb1 100644 --- a/configure.ac +++ b/configure.ac @@ -649,7 +649,7 @@ CXXFLAGS="$TEMP_CXXFLAGS" fi -CPPFLAGS="$CPPFLAGS -DHAVE_BUILD_INFO" +CPPFLAGS="$CPPFLAGS -DHAVE_BUILD_INFO -DGSL_NO_IOSTREAMS" AC_ARG_WITH([utils], [AS_HELP_STRING([--with-utils], From dc59cbc3155a0d4e0091d58b373c3e9dae740760 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Thu, 30 May 2024 13:39:34 +0700 Subject: [PATCH 11/21] fix: optimize includes in gsl/pointer.h to speed up compile time it removes: - algorithm: `forward` is a part of utility not algorithm - system_error: `hash` is already declared in memory --- src/gsl/pointers.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/gsl/pointers.h b/src/gsl/pointers.h index bf444ef5f2..93a194999e 100644 --- a/src/gsl/pointers.h +++ b/src/gsl/pointers.h @@ -20,12 +20,10 @@ #include // for Ensures, Expects #include -#include // for forward #include // for ptrdiff_t, nullptr_t, size_t -#include // for shared_ptr, unique_ptr -#include // for hash +#include // for shared_ptr, unique_ptr, hash #include // for enable_if_t, is_convertible, is_assignable -#include // for declval +#include // for declval, forward #if !defined(GSL_NO_IOSTREAMS) #include // for ostream From c602ca15e1f904bbd337e159b2fc38bcc4b6bf6e Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 26 Aug 2024 15:55:37 +0000 Subject: [PATCH 12/21] coinjoin: protect `m_wallet_manager_map` with `cs_wallet_manager_map` Avoid TSan-reported data race ``` WARNING: ThreadSanitizer: data race (pid=374820) Read of size 8 at 0x7b140002ce10 by thread T12: #0 _M_ptr /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/unique_ptr.h:154:42 (dashd+0xb58e08) (BuildId: c3fdce9f7e778985a4fb0968ff4506d9ad24d408) #1 get /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/unique_ptr.h:361:21 (dashd+0xb58e08) #2 operator-> /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/unique_ptr.h:355:9 (dashd+0xb58e08) #3 CoinJoinWalletManager::DoMaintenance() /src/dash/src/coinjoin/client.cpp:1907:9 (dashd+0xb58e08) [...] Previous write of size 8 at 0x7b140002ce10 by thread T17 (mutexes: write M0): #0 operator new(unsigned long) (dashd+0x162657) (BuildId: c3fdce9f7e778985a4fb0968ff4506d9ad24d408) #1 allocate /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/ext/new_allocator.h:114:27 (dashd+0xb772b4) (BuildId: c3fdce9f7e778985a4fb0968ff4506d9ad24d408) #2 allocate /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/alloc_traits.h:443:20 (dashd+0xb772b4) #3 _M_get_node /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/stl_tree.h:580:16 (dashd+0xb772b4) [...] #8 CoinJoinWalletManager::Add(CWallet&) /src/dash/src/coinjoin/client.cpp:1898:26 (dashd+0xb58c73) (BuildId: c3fdce9f7e778985a4fb0968ff4506d9ad24d408) [...] SUMMARY: ThreadSanitizer: data race [...] ``` --- src/coinjoin/client.cpp | 67 +++++++++++++++++------------- src/coinjoin/client.h | 30 ++++++++++--- src/dsnotificationinterface.cpp | 5 +-- src/masternode/utils.cpp | 6 +-- src/net_processing.cpp | 6 +-- src/wallet/test/coinjoin_tests.cpp | 4 +- 6 files changed, 74 insertions(+), 44 deletions(-) diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp index 1675a6408e..096ce8b1d1 100644 --- a/src/coinjoin/client.cpp +++ b/src/coinjoin/client.cpp @@ -96,11 +96,9 @@ PeerMsgRet CCoinJoinClientQueueManager::ProcessDSQueue(const CNode& peer, CDataS } // if the queue is ready, submit if we can - if (dsq.fReady && ranges::any_of(m_walletman.raw(), - [this, &dmn](const auto &pair) { - return pair.second->TrySubmitDenominate(dmn->pdmnState->addr, - this->connman); - })) { + if (dsq.fReady && m_walletman.ForAnyCJClientMan([this, &dmn](std::unique_ptr& clientman) { + return clientman->TrySubmitDenominate(dmn->pdmnState->addr, this->connman); + })) { LogPrint(BCLog::COINJOIN, "DSQUEUE -- CoinJoin queue (%s) is ready on masternode %s\n", dsq.ToString(), dmn->pdmnState->addr.ToString()); return {}; @@ -121,8 +119,9 @@ PeerMsgRet CCoinJoinClientQueueManager::ProcessDSQueue(const CNode& peer, CDataS LogPrint(BCLog::COINJOIN, "DSQUEUE -- new CoinJoin queue (%s) from masternode %s\n", dsq.ToString(), dmn->pdmnState->addr.ToString()); - ranges::any_of(m_walletman.raw(), - [&dsq](const auto &pair) { return pair.second->MarkAlreadyJoinedQueueAsTried(dsq); }); + m_walletman.ForAnyCJClientMan([&dsq](const std::unique_ptr& clientman) { + return clientman->MarkAlreadyJoinedQueueAsTried(dsq); + }); WITH_LOCK(cs_vecqueue, vecCoinJoinQueue.push_back(dsq)); } @@ -155,11 +154,14 @@ void CCoinJoinClientManager::ProcessMessage(CNode& peer, CChainState& active_cha } } -CCoinJoinClientSession::CCoinJoinClientSession(CWallet& wallet, CoinJoinWalletManager& walletman, CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman, - const CMasternodeSync& mn_sync, const std::unique_ptr& queueman, bool is_masternode) : +CCoinJoinClientSession::CCoinJoinClientSession(CWallet& wallet, CoinJoinWalletManager& walletman, + CCoinJoinClientManager& clientman, CDeterministicMNManager& dmnman, + CMasternodeMetaMan& mn_metaman, const CMasternodeSync& mn_sync, + const std::unique_ptr& queueman, + bool is_masternode) : m_wallet(wallet), m_walletman(walletman), - m_manager(*Assert(walletman.Get(wallet.GetName()))), + m_clientman(clientman), m_dmnman(dmnman), m_mn_metaman(mn_metaman), m_mn_sync(mn_sync), @@ -684,7 +686,7 @@ void CCoinJoinClientSession::CompletedTransaction(PoolMessage nMessageID) if (m_is_masternode) return; if (nMessageID == MSG_SUCCESS) { - m_manager.UpdatedSuccessBlock(); + m_clientman.UpdatedSuccessBlock(); keyHolderStorage.KeepAll(); WalletCJLogPrint(m_wallet, "CompletedTransaction -- success\n"); } else { @@ -995,7 +997,8 @@ bool CCoinJoinClientManager::DoAutomaticDenominating(CChainState& active_chainst AssertLockNotHeld(cs_deqsessions); LOCK(cs_deqsessions); if (int(deqSessions.size()) < CCoinJoinClientOptions::GetSessions()) { - deqSessions.emplace_back(m_wallet, m_walletman, m_dmnman, m_mn_metaman, m_mn_sync, m_queueman, m_is_masternode); + deqSessions.emplace_back(m_wallet, m_walletman, *this, m_dmnman, m_mn_metaman, m_mn_sync, m_queueman, + m_is_masternode); } for (auto& session : deqSessions) { if (!CheckAutomaticBackup()) return false; @@ -1100,7 +1103,7 @@ bool CCoinJoinClientSession::JoinExistingQueue(CAmount nBalanceNeedsAnonymized, continue; } - m_manager.AddUsedMasternode(dsq.masternodeOutpoint); + m_clientman.AddUsedMasternode(dsq.masternodeOutpoint); if (connman.IsMasternodeOrDisconnectRequested(dmn->pdmnState->addr)) { WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::JoinExistingQueue -- skipping masternode connection, addr=%s\n", dmn->pdmnState->addr.ToString()); @@ -1145,14 +1148,14 @@ bool CCoinJoinClientSession::StartNewQueue(CAmount nBalanceNeedsAnonymized, CCon // otherwise, try one randomly while (nTries < 10) { - auto dmn = m_manager.GetRandomNotUsedMasternode(); + auto dmn = m_clientman.GetRandomNotUsedMasternode(); if (!dmn) { strAutoDenomResult = _("Can't find random Masternode."); WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::StartNewQueue -- %s\n", strAutoDenomResult.original); return false; } - m_manager.AddUsedMasternode(dmn->collateralOutpoint); + m_clientman.AddUsedMasternode(dmn->collateralOutpoint); // skip next mn payments winners if (dmn->pdmnState->nLastPaidHeight + nWeightedMnCount < mnList.GetHeight() + WinnersToSkip()) { @@ -1526,7 +1529,7 @@ bool CCoinJoinClientSession::MakeCollateralAmounts(const CompactTallyItem& tally return false; } - m_manager.UpdatedSuccessBlock(); + m_clientman.UpdatedSuccessBlock(); WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::%s -- txid: %s\n", __func__, strResult.original); @@ -1803,7 +1806,7 @@ bool CCoinJoinClientSession::CreateDenominated(CAmount nBalanceToDenominate, con } // use the same nCachedLastSuccessBlock as for DS mixing to prevent race - m_manager.UpdatedSuccessBlock(); + m_clientman.UpdatedSuccessBlock(); WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::%s -- txid: %s\n", __func__, strResult.original); @@ -1894,35 +1897,43 @@ void CCoinJoinClientManager::GetJsonInfo(UniValue& obj) const obj.pushKV("sessions", arrSessions); } -void CoinJoinWalletManager::Add(CWallet& wallet) { - m_wallet_manager_map.try_emplace( - wallet.GetName(), - std::make_unique(wallet, *this, m_dmnman, m_mn_metaman, m_mn_sync, m_queueman, m_is_masternode) - ); +void CoinJoinWalletManager::Add(CWallet& wallet) +{ + { + LOCK(cs_wallet_manager_map); + m_wallet_manager_map.try_emplace(wallet.GetName(), + std::make_unique(wallet, *this, m_dmnman, m_mn_metaman, + m_mn_sync, m_queueman, m_is_masternode)); + } g_wallet_init_interface.InitCoinJoinSettings(*this); } -void CoinJoinWalletManager::DoMaintenance() { - for (auto& [wallet_str, walletman] : m_wallet_manager_map) { - walletman->DoMaintenance(m_chainstate, m_connman, m_mempool); +void CoinJoinWalletManager::DoMaintenance() +{ + LOCK(cs_wallet_manager_map); + for (auto& [_, clientman] : m_wallet_manager_map) { + clientman->DoMaintenance(m_chainstate, m_connman, m_mempool); } } void CoinJoinWalletManager::Remove(const std::string& name) { - m_wallet_manager_map.erase(name); + { + LOCK(cs_wallet_manager_map); + m_wallet_manager_map.erase(name); + } g_wallet_init_interface.InitCoinJoinSettings(*this); } void CoinJoinWalletManager::Flush(const std::string& name) { - auto clientman = Get(name); - assert(clientman != nullptr); + auto clientman = Assert(Get(name)); clientman->ResetPool(); clientman->StopMixing(); } CCoinJoinClientManager* CoinJoinWalletManager::Get(const std::string& name) const { + LOCK(cs_wallet_manager_map); auto it = m_wallet_manager_map.find(name); return (it != m_wallet_manager_map.end()) ? it->second.get() : nullptr; } diff --git a/src/coinjoin/client.h b/src/coinjoin/client.h index ba77e565cf..326f352c6a 100644 --- a/src/coinjoin/client.h +++ b/src/coinjoin/client.h @@ -9,6 +9,7 @@ #include #include +#include #include #include @@ -80,6 +81,7 @@ class CoinJoinWalletManager { {} ~CoinJoinWalletManager() { + LOCK(cs_wallet_manager_map); for (auto& [wallet_name, cj_man] : m_wallet_manager_map) { cj_man.reset(); } @@ -93,7 +95,21 @@ class CoinJoinWalletManager { CCoinJoinClientManager* Get(const std::string& name) const; - const wallet_name_cjman_map& raw() const { return m_wallet_manager_map; } + template + void ForEachCJClientMan(Callable&& func) + { + LOCK(cs_wallet_manager_map); + for (auto&& [_, clientman] : m_wallet_manager_map) { + func(clientman); + } + }; + + template + bool ForAnyCJClientMan(Callable&& func) + { + LOCK(cs_wallet_manager_map); + return ranges::any_of(m_wallet_manager_map, [&](auto& pair) { return func(pair.second); }); + }; private: CChainState& m_chainstate; @@ -105,7 +121,9 @@ class CoinJoinWalletManager { const std::unique_ptr& m_queueman; const bool m_is_masternode; - wallet_name_cjman_map m_wallet_manager_map; + + mutable Mutex cs_wallet_manager_map; + wallet_name_cjman_map m_wallet_manager_map GUARDED_BY(cs_wallet_manager_map); }; class CCoinJoinClientSession : public CCoinJoinBaseSession @@ -113,7 +131,7 @@ class CCoinJoinClientSession : public CCoinJoinBaseSession private: CWallet& m_wallet; CoinJoinWalletManager& m_walletman; - CCoinJoinClientManager& m_manager; + CCoinJoinClientManager& m_clientman; CDeterministicMNManager& m_dmnman; CMasternodeMetaMan& m_mn_metaman; const CMasternodeSync& m_mn_sync; @@ -168,8 +186,10 @@ class CCoinJoinClientSession : public CCoinJoinBaseSession void SetNull() override EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin); public: - explicit CCoinJoinClientSession(CWallet& wallet, CoinJoinWalletManager& walletman, CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman, - const CMasternodeSync& mn_sync, const std::unique_ptr& queueman, bool is_masternode); + explicit CCoinJoinClientSession(CWallet& wallet, CoinJoinWalletManager& walletman, + CCoinJoinClientManager& clientman, CDeterministicMNManager& dmnman, + CMasternodeMetaMan& mn_metaman, const CMasternodeSync& mn_sync, + const std::unique_ptr& queueman, bool is_masternode); void ProcessMessage(CNode& peer, CChainState& active_chainstate, CConnman& connman, const CTxMemPool& mempool, std::string_view msg_type, CDataStream& vRecv); diff --git a/src/dsnotificationinterface.cpp b/src/dsnotificationinterface.cpp index 3a30fec2c0..f948aa0148 100644 --- a/src/dsnotificationinterface.cpp +++ b/src/dsnotificationinterface.cpp @@ -86,9 +86,8 @@ void CDSNotificationInterface::UpdatedBlockTip(const CBlockIndex *pindexNew, con m_cj_ctx->dstxman->UpdatedBlockTip(pindexNew, *m_llmq_ctx->clhandler, m_mn_sync); #ifdef ENABLE_WALLET - for (auto& pair : m_cj_ctx->walletman->raw()) { - pair.second->UpdatedBlockTip(pindexNew); - } + m_cj_ctx->walletman->ForEachCJClientMan( + [&pindexNew](std::unique_ptr& clientman) { clientman->UpdatedBlockTip(pindexNew); }); #endif // ENABLE_WALLET m_llmq_ctx->isman->UpdatedBlockTip(pindexNew); diff --git a/src/masternode/utils.cpp b/src/masternode/utils.cpp index 90311288f9..6bf48f2b5c 100644 --- a/src/masternode/utils.cpp +++ b/src/masternode/utils.cpp @@ -23,9 +23,9 @@ void CMasternodeUtils::DoMaintenance(CConnman& connman, CDeterministicMNManager& std::vector vecDmns; // will be empty when no wallet #ifdef ENABLE_WALLET - for (auto& pair : cj_ctx.walletman->raw()) { - pair.second->GetMixingMasternodesInfo(vecDmns); - } + cj_ctx.walletman->ForEachCJClientMan([&vecDmns](const std::unique_ptr& clientman) { + clientman->GetMixingMasternodesInfo(vecDmns); + }); #endif // ENABLE_WALLET // Don't disconnect masternode connections when we have less then the desired amount of outbound nodes diff --git a/src/net_processing.cpp b/src/net_processing.cpp index c5a76acf10..db6d9c70ff 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -5095,9 +5095,9 @@ void PeerManagerImpl::ProcessMessage( //probably one the extensions #ifdef ENABLE_WALLET ProcessPeerMsgRet(m_cj_ctx->queueman->ProcessMessage(pfrom, msg_type, vRecv), pfrom); - for (auto& pair : m_cj_ctx->walletman->raw()) { - pair.second->ProcessMessage(pfrom, m_chainman.ActiveChainstate(), m_connman, m_mempool, msg_type, vRecv); - } + m_cj_ctx->walletman->ForEachCJClientMan([this, &pfrom, &msg_type, &vRecv](std::unique_ptr& clientman) { + clientman->ProcessMessage(pfrom, m_chainman.ActiveChainstate(), m_connman, m_mempool, msg_type, vRecv); + }); #endif // ENABLE_WALLET ProcessPeerMsgRet(m_cj_ctx->server->ProcessMessage(pfrom, msg_type, vRecv), pfrom); ProcessPeerMsgRet(m_sporkman.ProcessMessage(pfrom, m_connman, *this, msg_type, vRecv), pfrom); diff --git a/src/wallet/test/coinjoin_tests.cpp b/src/wallet/test/coinjoin_tests.cpp index d6d3b174e3..45afe2bba5 100644 --- a/src/wallet/test/coinjoin_tests.cpp +++ b/src/wallet/test/coinjoin_tests.cpp @@ -208,8 +208,8 @@ class CTransactionBuilderTestSetup : public TestChain100Setup BOOST_FIXTURE_TEST_CASE(coinjoin_manager_start_stop_tests, CTransactionBuilderTestSetup) { - BOOST_CHECK_EQUAL(m_node.cj_ctx->walletman->raw().size(), 1); - auto& cj_man = m_node.cj_ctx->walletman->raw().begin()->second; + CCoinJoinClientManager* cj_man = m_node.cj_ctx->walletman->Get(""); + BOOST_ASSERT(cj_man != nullptr); BOOST_CHECK_EQUAL(cj_man->IsMixing(), false); BOOST_CHECK_EQUAL(cj_man->StartMixing(), true); BOOST_CHECK_EQUAL(cj_man->IsMixing(), true); From 87d775d27c49a0fd1ddc151177d63ebd50af05dd Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sun, 11 Aug 2024 05:12:32 +0000 Subject: [PATCH 13/21] partial bitcoin#22868: Call load handlers without cs_wallet locked excludes changes in `wallet/context.h` due to `::vpwallets` (and thus, `cs_wallets`) not being deglobalized yet --- src/wallet/test/wallet_tests.cpp | 6 +----- src/wallet/wallet.cpp | 5 ++--- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 9e6089e617..9e3e9d2a55 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -1298,18 +1298,14 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup) // deadlock during the sync and simulates a new block notification happening // as soon as possible. addtx_count = 0; - auto handler = HandleLoadWallet([&](std::unique_ptr wallet) EXCLUSIVE_LOCKS_REQUIRED(wallet->wallet()->cs_wallet, cs_wallets) { + auto handler = HandleLoadWallet([&](std::unique_ptr wallet) { BOOST_CHECK(rescan_completed); m_coinbase_txns.push_back(CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())).vtx[0]); block_tx = TestSimpleSpend(*m_coinbase_txns[2], 0, coinbaseKey, GetScriptForRawPubKey(key.GetPubKey())); m_coinbase_txns.push_back(CreateAndProcessBlock({block_tx}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())).vtx[0]); mempool_tx = TestSimpleSpend(*m_coinbase_txns[3], 0, coinbaseKey, GetScriptForRawPubKey(key.GetPubKey())); BOOST_CHECK(m_node.chain->broadcastTransaction(MakeTransactionRef(mempool_tx), DEFAULT_TRANSACTION_MAXFEE, false, error)); - LEAVE_CRITICAL_SECTION(cs_wallets); - LEAVE_CRITICAL_SECTION(wallet->wallet()->cs_wallet); SyncWithValidationInterfaceQueue(); - ENTER_CRITICAL_SECTION(wallet->wallet()->cs_wallet); - ENTER_CRITICAL_SECTION(cs_wallets); }); wallet = TestLoadWallet(m_node.chain.get(), m_node.coinjoin_loader.get()); BOOST_CHECK_EQUAL(addtx_count, 4); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index f648fc2ced..ddefbbb911 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4949,8 +4949,6 @@ std::shared_ptr CWallet::Create(interfaces::Chain* chain, interfaces::C // Try to top up keypool. No-op if the wallet is locked. walletInstance->TopUpKeyPool(); - LOCK(walletInstance->cs_wallet); - if (chain && !AttachChain(walletInstance, *chain, error, warnings)) { return nullptr; } @@ -4966,9 +4964,10 @@ std::shared_ptr CWallet::Create(interfaces::Chain* chain, interfaces::C } } - walletInstance->SetBroadcastTransactions(gArgs.GetBoolArg("-walletbroadcast", DEFAULT_WALLETBROADCAST)); { + LOCK(walletInstance->cs_wallet); + walletInstance->SetBroadcastTransactions(gArgs.GetBoolArg("-walletbroadcast", DEFAULT_WALLETBROADCAST)); walletInstance->WalletLogPrintf("setExternalKeyPool.size() = %u\n", walletInstance->KeypoolCountExternalKeys()); walletInstance->WalletLogPrintf("GetKeyPoolSize() = %u\n", walletInstance->GetKeyPoolSize()); walletInstance->WalletLogPrintf("mapWallet.size() = %u\n", walletInstance->mapWallet.size()); From 19735323727e50936ae69ab3bfe05da7c864d331 Mon Sep 17 00:00:00 2001 From: Samuel Dobson Date: Mon, 9 Aug 2021 14:21:56 +1200 Subject: [PATCH 14/21] Merge bitcoin/bitcoin#22337: wallet: Use bilingual_str for errors 92993aa5cf37995e65e68dfd6f129ecaf418e01c Change SignTransaction's input_errors to use bilingual_str (Andrew Chow) 171366e89b828a557f8262d9dc14ff7a03f813f7 Use bilingual_str for address fetching functions (Andrew Chow) 9571c69b51115454c6a699be9492024f7b46c2b4 Add bilingual_str::clear() (Andrew Chow) Pull request description: In a couple of places in the wallet, errors are `std::string`. In order for these errors to be translated, change them to use `bilingual_str`. ACKs for top commit: hebasto: re-ACK 92993aa5cf37995e65e68dfd6f129ecaf418e01c, only rebased since my [previous](https://github.com/bitcoin/bitcoin/pull/22337#pullrequestreview-694542729) review, verified with klementtan: Code review ACK 92993aa5cf37995e65e68dfd6f129ecaf418e01c meshcollider: Code review ACK 92993aa5cf37995e65e68dfd6f129ecaf418e01c Tree-SHA512: 5400e419dd87db8c49b67ed0964de2d44b58010a566ca246f2f0760ed9ef6a9b6f6df7a6adcb211b315b74c727bfe8c7d07eb5690b5922fa5828ceef4c83461f Signed-off-by: Vijay --- src/coinjoin/client.cpp | 6 +++--- src/interfaces/chain.h | 2 +- src/node/interfaces.cpp | 2 +- src/node/transaction.cpp | 7 ++++--- src/node/transaction.h | 4 ++-- src/qt/psbtoperationsdialog.cpp | 2 +- src/rpc/rawtransaction.cpp | 5 +++-- src/rpc/rawtransaction_util.cpp | 9 +++++---- src/rpc/rawtransaction_util.h | 3 ++- src/script/sign.cpp | 11 ++++++----- src/script/sign.h | 3 ++- src/test/fuzz/script_sign.cpp | 3 ++- src/test/util/setup_common.cpp | 2 +- src/test/util/wallet.cpp | 3 ++- src/util/translation.h | 6 ++++++ src/wallet/interfaces.cpp | 3 ++- src/wallet/rpcwallet.cpp | 10 +++++----- src/wallet/scriptpubkeyman.cpp | 18 +++++++++--------- src/wallet/scriptpubkeyman.h | 12 ++++++------ src/wallet/test/coinselector_tests.cpp | 3 ++- src/wallet/test/wallet_tests.cpp | 8 ++++---- src/wallet/wallet.cpp | 24 ++++++++++++------------ src/wallet/wallet.h | 8 ++++---- 23 files changed, 85 insertions(+), 69 deletions(-) diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp index 1675a6408e..aaf2817a8d 100644 --- a/src/coinjoin/client.cpp +++ b/src/coinjoin/client.cpp @@ -638,14 +638,14 @@ bool CCoinJoinClientSession::SignFinalTransaction(CNode& peer, CChainState& acti // fill values for found outpoints m_wallet.chain().findCoins(coins); - std::map signing_errors; + std::map signing_errors; m_wallet.SignTransaction(finalMutableTransaction, coins, SIGHASH_ALL | SIGHASH_ANYONECANPAY, signing_errors); for (const auto& [input_index, error_string] : signing_errors) { // NOTE: this is a partial signing so it's expected for SignTransaction to return // "Input not found or already spent" errors for inputs that aren't ours - if (error_string != "Input not found or already spent") { - WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::%s -- signing input %d failed: %s!\n", __func__, input_index, error_string); + if (error_string.original != "Input not found or already spent") { + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::%s -- signing input %d failed: %s!\n", __func__, input_index, error_string.original); UnlockCoins(); keyHolderStorage.ReturnAll(); SetNull(); diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index dcc399a29d..d84688dcc0 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -198,7 +198,7 @@ class Chain virtual bool broadcastTransaction(const CTransactionRef& tx, const CAmount& max_tx_fee, bool relay, - std::string& err_string) = 0; + bilingual_str& err_string) = 0; //! Calculate mempool ancestor and descendant counts for the given transaction. virtual void getTransactionAncestry(const uint256& txid, size_t& ancestors, size_t& descendants) = 0; diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index eba23b0195..d453ff8975 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -879,7 +879,7 @@ class ChainImpl : public Chain auto it = m_node.mempool->GetIter(txid); return it && (*it)->GetCountWithDescendants() > 1; } - bool broadcastTransaction(const CTransactionRef& tx, const CAmount& max_tx_fee, bool relay, std::string& err_string) override + bool broadcastTransaction(const CTransactionRef& tx, const CAmount& max_tx_fee, bool relay, bilingual_str& err_string) override { const TransactionError err = BroadcastTransaction(m_node, tx, err_string, max_tx_fee, relay, /*wait_callback*/ false); // Chain clients only care about failures to accept the tx to the mempool. Disregard non-mempool related failures. diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp index 76264ffb1c..9c039c1a6e 100644 --- a/src/node/transaction.cpp +++ b/src/node/transaction.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include @@ -28,7 +29,7 @@ static TransactionError HandleATMPError(const TxValidationState& state, std::str } } -TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef tx, std::string& err_string, const CAmount& max_tx_fee, bool relay, bool wait_callback, bool bypass_limits) +TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef tx, bilingual_str& err_string, const CAmount& max_tx_fee, bool relay, bool wait_callback, bool bypass_limits) { // BroadcastTransaction can be called by either sendrawtransaction RPC or wallet RPCs. // node.peerman is assigned both before chain clients and before RPC server is accepting calls, @@ -59,7 +60,7 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t const MempoolAcceptResult result = AcceptToMemoryPool(node.chainman->ActiveChainstate(), *node.mempool, tx, bypass_limits, true /* test_accept */); if (result.m_result_type != MempoolAcceptResult::ResultType::VALID) { - return HandleATMPError(result.m_state, err_string); + return HandleATMPError(result.m_state, err_string.original); } else if (result.m_base_fees.value() > max_tx_fee) { return TransactionError::MAX_FEE_EXCEEDED; } @@ -68,7 +69,7 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t const MempoolAcceptResult result = AcceptToMemoryPool(node.chainman->ActiveChainstate(), *node.mempool, tx, bypass_limits, false /* test_accept */); if (result.m_result_type != MempoolAcceptResult::ResultType::VALID) { - return HandleATMPError(result.m_state, err_string); + return HandleATMPError(result.m_state, err_string.original); } // Transaction was accepted to the mempool. diff --git a/src/node/transaction.h b/src/node/transaction.h index 3867401586..511a252817 100644 --- a/src/node/transaction.h +++ b/src/node/transaction.h @@ -35,12 +35,12 @@ static const CFeeRate DEFAULT_MAX_RAW_TX_FEE_RATE{COIN / 10}; * * @param[in] node reference to node context * @param[in] tx the transaction to broadcast - * @param[out] err_string reference to std::string to fill with error string if available + * @param[out] err_string reference to bilingual_str to fill with error string if available * @param[in] relay flag if both mempool insertion and p2p relay are requested * @param[in] wait_callback wait until callbacks have been processed to avoid stale result due to a sequentially RPC. * return error */ -[[nodiscard]] TransactionError BroadcastTransaction(NodeContext& node, CTransactionRef tx, std::string& err_string, const CAmount& highfee, bool relay, bool wait_callback, bool bypass_limits = false); +[[nodiscard]] TransactionError BroadcastTransaction(NodeContext& node, CTransactionRef tx, bilingual_str& err_string, const CAmount& highfee, bool relay, bool wait_callback, bool bypass_limits = false); /** * Return transaction with a given hash. diff --git a/src/qt/psbtoperationsdialog.cpp b/src/qt/psbtoperationsdialog.cpp index bb642d1f1b..609f442bf2 100644 --- a/src/qt/psbtoperationsdialog.cpp +++ b/src/qt/psbtoperationsdialog.cpp @@ -107,7 +107,7 @@ void PSBTOperationsDialog::broadcastTransaction() } CTransactionRef tx = MakeTransactionRef(mtx); - std::string err_string; + bilingual_str err_string; TransactionError error = BroadcastTransaction( *m_client_model->node().context(), tx, err_string, DEFAULT_MAX_RAW_TX_FEE_RATE.GetFeePerK(), /* relay */ true, /* await_callback */ false); diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index e5ffa1f317..2fbd434ee4 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -41,6 +41,7 @@ #include #include #include +#include #include #include #include @@ -1165,12 +1166,12 @@ RPCHelpMan sendrawtransaction() bool bypass_limits = false; if (!request.params[3].isNull()) bypass_limits = request.params[3].get_bool(); - std::string err_string; + bilingual_str err_string; AssertLockNotHeld(cs_main); NodeContext& node = EnsureAnyNodeContext(request.context); const TransactionError err = BroadcastTransaction(node, tx, err_string, max_raw_tx_fee, /* relay */ true, /* wait_callback */ true, bypass_limits); if (TransactionError::OK != err) { - throw JSONRPCTransactionError(err, err_string); + throw JSONRPCTransactionError(err, err_string.original); } return tx->GetHash().GetHex(); diff --git a/src/rpc/rawtransaction_util.cpp b/src/rpc/rawtransaction_util.cpp index 11df0c274e..6453c14622 100644 --- a/src/rpc/rawtransaction_util.cpp +++ b/src/rpc/rawtransaction_util.cpp @@ -18,6 +18,7 @@ #include #include #include +#include CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniValue& outputs_in, const UniValue& locktime) { @@ -227,22 +228,22 @@ void SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, int nHashType = ParseSighashString(hashType); // Script verification errors - std::map input_errors; + std::map input_errors; bool complete = SignTransaction(mtx, keystore, coins, nHashType, input_errors); SignTransactionResultToJSON(mtx, complete, coins, input_errors, result); } -void SignTransactionResultToJSON(CMutableTransaction& mtx, bool complete, const std::map& coins, const std::map& input_errors, UniValue& result) +void SignTransactionResultToJSON(CMutableTransaction& mtx, bool complete, const std::map& coins, const std::map& input_errors, UniValue& result) { // Make errors UniValue UniValue vErrors(UniValue::VARR); for (const auto& err_pair : input_errors) { - if (err_pair.second == "Missing amount") { + if (err_pair.second.original == "Missing amount") { // This particular error needs to be an exception for some reason throw JSONRPCError(RPC_TYPE_ERROR, strprintf("Missing amount for %s", coins.at(mtx.vin.at(err_pair.first).prevout).out.ToString())); } - TxInErrorToJSON(mtx.vin.at(err_pair.first), vErrors, err_pair.second); + TxInErrorToJSON(mtx.vin.at(err_pair.first), vErrors, err_pair.second.original); } result.pushKV("hex", EncodeHexTx(CTransaction(mtx))); diff --git a/src/rpc/rawtransaction_util.h b/src/rpc/rawtransaction_util.h index c8865588bc..6ee04e77e1 100644 --- a/src/rpc/rawtransaction_util.h +++ b/src/rpc/rawtransaction_util.h @@ -8,6 +8,7 @@ #include #include +struct bilingual_str; class FillableSigningProvider; class UniValue; struct CMutableTransaction; @@ -25,7 +26,7 @@ class SigningProvider; * @param result JSON object where signed transaction results accumulate */ void SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, const std::map& coins, const UniValue& hashType, UniValue& result); -void SignTransactionResultToJSON(CMutableTransaction& mtx, bool complete, const std::map& coins, const std::map& input_errors, UniValue& result); +void SignTransactionResultToJSON(CMutableTransaction& mtx, bool complete, const std::map& coins, const std::map& input_errors, UniValue& result); /** * Parse a prevtxs UniValue array and get the map of coins from it diff --git a/src/script/sign.cpp b/src/script/sign.cpp index bb120d784c..c9491f65f5 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -11,6 +11,7 @@ #include