Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: broadcast dsq messages using the inventory system #6148

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions doc/release-notes-6148.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
P2P and Network Changes
-----------------------

The DSQ message, starting in protocol version 70234, is broadcast using the inventory system, and not simply
relaying to all connected peers. This should reduce the bandwidth needs for all nodes, however, this affect will
be most noticeable on highly connected masternodes. (#6148)
10 changes: 8 additions & 2 deletions src/coinjoin/client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <masternode/meta.h>
#include <masternode/sync.h>
#include <net.h>
#include <net_processing.h>
#include <netmessagemaker.h>
#include <shutdown.h>
#include <util/check.h>
Expand All @@ -22,9 +23,9 @@
#include <util/translation.h>
#include <validation.h>
#include <version.h>
#include <walletinitinterface.h>
#include <wallet/coincontrol.h>
#include <wallet/fees.h>
#include <walletinitinterface.h>

#include <memory>
#include <univalue.h>
Expand All @@ -47,6 +48,11 @@ PeerMsgRet CCoinJoinClientQueueManager::ProcessDSQueue(const CNode& peer, CDataS
CCoinJoinQueue dsq;
vRecv >> dsq;

{
LOCK(cs_main);
Assert(peerman)->EraseObjectRequest(peer.GetId(), CInv(MSG_DSQ, dsq.GetHash()));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if by any reason CCoinJoinClientQueueManager will be initialized before peerman?

Should we just change assert here to if?

Suggested change
Assert(peerman)->EraseObjectRequest(peer.GetId(), CInv(MSG_DSQ, dsq.GetHash()));
if (peerman) peerman->EraseObjectRequest(peer.GetId(), CInv(MSG_DSQ, dsq.GetHash()));

}

if (dsq.masternodeOutpoint.IsNull() && dsq.m_protxHash.IsNull()) {
return tl::unexpected{100};
}
Expand Down Expand Up @@ -126,7 +132,7 @@ PeerMsgRet CCoinJoinClientQueueManager::ProcessDSQueue(const CNode& peer, CDataS
WITH_LOCK(cs_vecqueue, vecCoinJoinQueue.push_back(dsq));
}
} // cs_ProcessDSQueue
dsq.Relay(connman);
dsq.Relay(connman, *peerman);
return {};
}

Expand Down
15 changes: 12 additions & 3 deletions src/coinjoin/client.h
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ class CCoinJoinClientQueueManager : public CCoinJoinBaseManager
{
private:
CConnman& connman;
std::unique_ptr<PeerManager>& peerman;
CoinJoinWalletManager& m_walletman;
CDeterministicMNManager& m_dmnman;
CMasternodeMetaMan& m_mn_metaman;
Expand All @@ -229,9 +230,17 @@ class CCoinJoinClientQueueManager : public CCoinJoinBaseManager
const bool m_is_masternode;

public:
explicit CCoinJoinClientQueueManager(CConnman& _connman, CoinJoinWalletManager& walletman, CDeterministicMNManager& dmnman,
CMasternodeMetaMan& mn_metaman, const CMasternodeSync& mn_sync, bool is_masternode) :
connman(_connman), m_walletman(walletman), m_dmnman(dmnman), m_mn_metaman(mn_metaman), m_mn_sync(mn_sync), m_is_masternode{is_masternode} {};
explicit CCoinJoinClientQueueManager(CConnman& _connman, std::unique_ptr<PeerManager>& _peerman,
CoinJoinWalletManager& walletman, CDeterministicMNManager& dmnman,
CMasternodeMetaMan& mn_metaman, const CMasternodeSync& mn_sync,
bool is_masternode) :
connman(_connman),
peerman(_peerman),
m_walletman(walletman),
m_dmnman(dmnman),
m_mn_metaman(mn_metaman),
m_mn_sync(mn_sync),
m_is_masternode{is_masternode} {};

PeerMsgRet ProcessMessage(const CNode& peer, std::string_view msg_type, CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs_vecqueue);
PeerMsgRet ProcessDSQueue(const CNode& peer, CDataStream& vRecv);
Expand Down
8 changes: 6 additions & 2 deletions src/coinjoin/coinjoin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include <masternode/node.h>
#include <masternode/sync.h>
#include <messagesigner.h>
#include <net_processing.h>
#include <netmessagemaker.h>
#include <txmempool.h>
#include <util/moneystr.h>
Expand Down Expand Up @@ -46,6 +47,7 @@ uint256 CCoinJoinQueue::GetSignatureHash() const
{
return SerializeHash(*this, SER_GETHASH, PROTOCOL_VERSION);
}
uint256 CCoinJoinQueue::GetHash() const { return SerializeHash(*this, SER_NETWORK, PROTOCOL_VERSION); }

bool CCoinJoinQueue::Sign(const CActiveMasternodeManager& mn_activeman)
{
Expand All @@ -69,11 +71,13 @@ bool CCoinJoinQueue::CheckSignature(const CBLSPublicKey& blsPubKey) const
return true;
}

bool CCoinJoinQueue::Relay(CConnman& connman)
bool CCoinJoinQueue::Relay(CConnman& connman, PeerManager& peerman)
{
CInv inv(MSG_DSQ, GetHash());
peerman.RelayInv(inv, DSQ_INV_VERSION);
connman.ForEachNode([&connman, this](CNode* pnode) {
CNetMsgMaker msgMaker(pnode->GetCommonVersion());
if (pnode->fSendDSQueue) {
if (pnode->fSendDSQueue && pnode->nVersion < DSQ_INV_VERSION) {
connman.PushMessage(pnode, msgMaker.Make(NetMsgType::DSQUEUE, (*this)));
}
});
Expand Down
16 changes: 15 additions & 1 deletion src/coinjoin/coinjoin.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class CBlockIndex;
class CMasternodeSync;
class CTxMemPool;
class TxValidationState;
class PeerManager;

namespace llmq {
class CChainLocksHandler;
Expand Down Expand Up @@ -206,6 +207,7 @@ class CCoinJoinQueue
}
}

[[nodiscard]] uint256 GetHash() const;
[[nodiscard]] uint256 GetSignatureHash() const;
/** Sign this mixing transaction
* return true if all conditions are met:
Expand All @@ -218,7 +220,7 @@ class CCoinJoinQueue
/// Check if we have a valid Masternode address
[[nodiscard]] bool CheckSignature(const CBLSPublicKey& blsPubKey) const;

bool Relay(CConnman& connman);
bool Relay(CConnman& connman, PeerManager& peerman);

/// Check if a queue is too old or too far into the future
[[nodiscard]] bool IsTimeOutOfBounds(int64_t current_time = GetAdjustedTime()) const;
Expand Down Expand Up @@ -340,6 +342,18 @@ class CCoinJoinBaseManager

int GetQueueSize() const EXCLUSIVE_LOCKS_REQUIRED(!cs_vecqueue) { LOCK(cs_vecqueue); return vecCoinJoinQueue.size(); }
bool GetQueueItemAndTry(CCoinJoinQueue& dsqRet) EXCLUSIVE_LOCKS_REQUIRED(!cs_vecqueue);

bool HasQueue(const uint256& queueHash) EXCLUSIVE_LOCKS_REQUIRED(!cs_vecqueue)
{
LOCK(cs_vecqueue);
return std::any_of(vecCoinJoinQueue.begin(), vecCoinJoinQueue.end(),
[&queueHash](auto q) { return q.GetHash() == queueHash; });
}
std::optional<CCoinJoinQueue> GetQueueFromHash(const uint256& queueHash) EXCLUSIVE_LOCKS_REQUIRED(!cs_vecqueue)
{
LOCK(cs_vecqueue);
return ranges::find_if_opt(vecCoinJoinQueue, [&queueHash](const auto& q) { return q.GetHash() == queueHash; });
}
};

// Various helpers and dstx manager implementation
Expand Down
18 changes: 12 additions & 6 deletions src/coinjoin/context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,21 @@
#endif // ENABLE_WALLET
#include <coinjoin/server.h>

CJContext::CJContext(CChainState& chainstate, CConnman& connman, CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman,
CTxMemPool& mempool, const CActiveMasternodeManager* const mn_activeman, const CMasternodeSync& mn_sync,
const std::unique_ptr<PeerManager>& peerman, bool relay_txes) :
CJContext::CJContext(CChainState& chainstate, CConnman& connman, CDeterministicMNManager& dmnman,
CMasternodeMetaMan& mn_metaman, CTxMemPool& mempool,
const CActiveMasternodeManager* const mn_activeman, const CMasternodeSync& mn_sync,
std::unique_ptr<PeerManager>& peerman, bool relay_txes) :
dstxman{std::make_unique<CDSTXManager>()},
#ifdef ENABLE_WALLET
walletman{std::make_unique<CoinJoinWalletManager>(chainstate, connman, dmnman, mn_metaman, mempool, mn_sync, queueman, /* is_masternode = */ mn_activeman != nullptr)},
queueman {relay_txes ? std::make_unique<CCoinJoinClientQueueManager>(connman, *walletman, dmnman, mn_metaman, mn_sync, /* is_masternode = */ mn_activeman != nullptr) : nullptr},
walletman{std::make_unique<CoinJoinWalletManager>(chainstate, connman, dmnman, mn_metaman, mempool, mn_sync,
queueman, /* is_masternode = */ mn_activeman != nullptr)},
queueman{relay_txes
? std::make_unique<CCoinJoinClientQueueManager>(connman, peerman, *walletman, dmnman, mn_metaman,
mn_sync, /* is_masternode = */ mn_activeman != nullptr)
: nullptr},
#endif // ENABLE_WALLET
server{std::make_unique<CCoinJoinServer>(chainstate, connman, dmnman, *dstxman, mn_metaman, mempool, mn_activeman, mn_sync, peerman)}
server{std::make_unique<CCoinJoinServer>(chainstate, connman, dmnman, *dstxman, mn_metaman, mempool, mn_activeman,
mn_sync, peerman)}
{}

CJContext::~CJContext() {}
6 changes: 3 additions & 3 deletions src/coinjoin/context.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ class CoinJoinWalletManager;
struct CJContext {
CJContext() = delete;
CJContext(const CJContext&) = delete;
CJContext(CChainState& chainstate, CConnman& connman, CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman,
CTxMemPool& mempool, const CActiveMasternodeManager* const mn_activeman, const CMasternodeSync& mn_sync,
const std::unique_ptr<PeerManager>& peerman, bool relay_txes);
CJContext(CChainState& chainstate, CConnman& connman, CDeterministicMNManager& dmnman,
CMasternodeMetaMan& mn_metaman, CTxMemPool& mempool, const CActiveMasternodeManager* const mn_activeman,
const CMasternodeSync& mn_sync, std::unique_ptr<PeerManager>& peerman, bool relay_txes);
~CJContext();

const std::unique_ptr<CDSTXManager> dstxman;
Expand Down
11 changes: 8 additions & 3 deletions src/coinjoin/server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,11 @@ PeerMsgRet CCoinJoinServer::ProcessDSQUEUE(const CNode& peer, CDataStream& vRecv
CCoinJoinQueue dsq;
vRecv >> dsq;

{
LOCK(cs_main);
Assert(m_peerman)->EraseObjectRequest(peer.GetId(), CInv(MSG_DSQ, dsq.GetHash()));
}

if (dsq.masternodeOutpoint.IsNull() && dsq.m_protxHash.IsNull()) {
return tl::unexpected{100};
}
Expand Down Expand Up @@ -178,7 +183,7 @@ PeerMsgRet CCoinJoinServer::ProcessDSQUEUE(const CNode& peer, CDataStream& vRecv
TRY_LOCK(cs_vecqueue, lockRecv);
if (!lockRecv) return {};
vecCoinJoinQueue.push_back(dsq);
dsq.Relay(connman);
dsq.Relay(connman, *m_peerman);
}
return {};
}
Expand Down Expand Up @@ -511,7 +516,7 @@ void CCoinJoinServer::CheckForCompleteQueue()
LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CheckForCompleteQueue -- queue is ready, signing and relaying (%s) " /* Continued */
"with %d participants\n", dsq.ToString(), vecSessionCollaterals.size());
dsq.Sign(*m_mn_activeman);
dsq.Relay(connman);
dsq.Relay(connman, *m_peerman);
}
}

Expand Down Expand Up @@ -724,7 +729,7 @@ bool CCoinJoinServer::CreateNewSession(const CCoinJoinAccept& dsa, PoolMessage&
GetAdjustedTime(), false);
LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CreateNewSession -- signing and relaying new queue: %s\n", dsq.ToString());
dsq.Sign(*m_mn_activeman);
dsq.Relay(connman);
dsq.Relay(connman, *m_peerman);
LOCK(cs_vecqueue);
vecCoinJoinQueue.push_back(dsq);
}
Expand Down
9 changes: 5 additions & 4 deletions src/coinjoin/server.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public CCoinJoinBaseManager
CTxMemPool& mempool;
const CActiveMasternodeManager* const m_mn_activeman;
const CMasternodeSync& m_mn_sync;
const std::unique_ptr<PeerManager>& m_peerman;
std::unique_ptr<PeerManager>& m_peerman;

// Mixing uses collateral transactions to trust parties entering the pool
// to behave honestly. If they don't it takes their money.
Expand Down Expand Up @@ -90,9 +90,10 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public CCoinJoinBaseManager
void SetNull() override EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin);

public:
explicit CCoinJoinServer(CChainState& chainstate, CConnman& _connman, CDeterministicMNManager& dmnman, CDSTXManager& dstxman,
CMasternodeMetaMan& mn_metaman, CTxMemPool& mempool, const CActiveMasternodeManager* const mn_activeman,
const CMasternodeSync& mn_sync, const std::unique_ptr<PeerManager>& peerman) :
explicit CCoinJoinServer(CChainState& chainstate, CConnman& _connman, CDeterministicMNManager& dmnman,
CDSTXManager& dstxman, CMasternodeMetaMan& mn_metaman, CTxMemPool& mempool,
const CActiveMasternodeManager* const mn_activeman, const CMasternodeSync& mn_sync,
std::unique_ptr<PeerManager>& peerman) :
m_chainstate(chainstate),
connman(_connman),
m_dmnman(dmnman),
Expand Down
19 changes: 19 additions & 0 deletions src/net_processing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2127,8 +2127,15 @@ bool PeerManagerImpl::AlreadyHave(const CInv& inv)
return m_llmq_ctx->clhandler->AlreadyHave(inv);
case MSG_ISDLOCK:
return m_llmq_ctx->isman->AlreadyHave(inv);
case MSG_DSQ:
#ifdef ENABLE_WALLET
return m_cj_ctx->server->HasQueue(inv.hash) || m_cj_ctx->queueman->HasQueue(inv.hash);
#else
return m_cj_ctx->server->HasQueue(inv.hash);
UdjinM6 marked this conversation as resolved.
Show resolved Hide resolved
#endif
}


// Don't know what it is, just say we already got one
return true;
}
Expand Down Expand Up @@ -2635,6 +2642,18 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic
push = true;
}
}
if (!push && inv.type == MSG_DSQ) {
auto opt_dsq = m_cj_ctx->server->GetQueueFromHash(inv.hash);
#ifdef ENABLE_WALLET
if (!opt_dsq.has_value()) {
opt_dsq = m_cj_ctx->queueman->GetQueueFromHash(inv.hash);
}
#endif
if (opt_dsq.has_value()) {
m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::DSQUEUE, *opt_dsq));
push = true;
}
}

if (!push) {
vNotFound.push_back(inv);
Expand Down
1 change: 1 addition & 0 deletions src/protocol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ const char* CInv::GetCommandInternal() const
case MSG_QUORUM_RECOVERED_SIG: return NetMsgType::QSIGREC;
case MSG_CLSIG: return NetMsgType::CLSIG;
case MSG_ISDLOCK: return NetMsgType::ISDLOCK;
case MSG_DSQ: return NetMsgType::DSQUEUE;
default:
return nullptr;
}
Expand Down
1 change: 1 addition & 0 deletions src/protocol.h
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,7 @@ enum GetDataMsg : uint32_t {
MSG_CLSIG = 29,
/* MSG_ISLOCK = 30, */ // Non-deterministic InstantSend and not used anymore
MSG_ISDLOCK = 31,
MSG_DSQ = 32,
UdjinM6 marked this conversation as resolved.
Show resolved Hide resolved
};

/** inv message data */
Expand Down
5 changes: 4 additions & 1 deletion src/version.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
*/


static const int PROTOCOL_VERSION = 70233;
static const int PROTOCOL_VERSION = 70234;

//! initial proto version, to be increased after version/verack negotiation
static const int INIT_PROTO_VERSION = 209;
Expand Down Expand Up @@ -55,6 +55,9 @@ static const int MNLISTDIFF_CHAINLOCKS_PROTO_VERSION = 70230;
//! Legacy ISLOCK messages and a corresponding INV were dropped in this version
static const int NO_LEGACY_ISLOCK_PROTO_VERSION = 70231;

//! Inventory type for DSQ messages added
static const int DSQ_INV_VERSION = 70234;

// Make sure that none of the values above collide with `ADDRV2_FORMAT`.

#endif // BITCOIN_VERSION_H
4 changes: 2 additions & 2 deletions test/lint/lint-circular-dependencies.sh
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,11 @@ EXPECTED_CIRCULAR_DEPENDENCIES=(
"evo/deterministicmns -> validationinterface -> evo/deterministicmns"
"logging -> util/system -> sync -> logging/timer -> logging"

"coinjoin/client -> net_processing -> coinjoin/client"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you avoid adding new circulat dependencies?

These 2 disappeared just because they have shorter path between coinjoin/client.... coinjoin/client

"coinjoin/client -> coinjoin/util -> wallet/wallet -> psbt -> node/transaction -> node/context -> coinjoin/context -> coinjoin/client"
"coinjoin/client -> coinjoin/coinjoin -> llmq/chainlocks -> net_processing -> coinjoin/client"

"coinjoin/client -> net_processing -> coinjoin/context -> coinjoin/client"
"coinjoin/context -> coinjoin/server -> net_processing -> coinjoin/context"
"coinjoin/server -> net_processing -> coinjoin/server"
"llmq/context -> llmq/ehf_signals -> net_processing -> llmq/context"
"coinjoin/client -> coinjoin/util -> wallet/wallet -> psbt -> node/transaction -> node/context -> coinjoin/context -> coinjoin/client"
"llmq/blockprocessor -> net_processing -> llmq/blockprocessor"
"llmq/chainlocks -> net_processing -> llmq/chainlocks"
"llmq/dkgsession -> net_processing -> llmq/quorums -> llmq/dkgsession"
Expand All @@ -100,7 +101,6 @@ EXPECTED_CIRCULAR_DEPENDENCIES=(
"llmq/blockprocessor -> net_processing -> llmq/context -> llmq/blockprocessor"
"llmq/blockprocessor -> net_processing -> llmq/quorums -> llmq/blockprocessor"
"llmq/chainlocks -> net_processing -> llmq/context -> llmq/chainlocks"
"coinjoin/client -> coinjoin/coinjoin -> llmq/chainlocks -> net_processing -> coinjoin/client"
"rpc/blockchain -> rpc/server -> rpc/blockchain"
)

Expand Down
Loading