Skip to content

Commit

Permalink
Revert "More Dandelion Fixes"
Browse files Browse the repository at this point in the history 
This reverts commit 394bc1f.
  • Loading branch information
@JaredTate
JaredTate committed Dec 20, 2024
1 parent 394bc1f commit 4a5e1fe
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 72 deletions.
13 changes: 4 additions & 9 deletions src/dandelion.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,27 +59,22 @@ bool CConnman::localDandelionDestinationPushInventory(const CInv& inv)
}

bool CConnman::insertDandelionEmbargo(const uint256& hash, std::chrono::microseconds& embargo) {
LOCK(cs_vNodes);

// Add buffer to embargo time
embargo += std::chrono::seconds{5}; // Extra safety margin

LOCK(cs_vNodes); // Add lock for thread safety
auto pair = mDandelionEmbargo.insert(std::make_pair(hash, embargo));
return pair.second;
}

bool CConnman::isTxDandelionEmbargoed(const uint256& hash) const {
LOCK(cs_vNodes);
LOCK(cs_vNodes); // Add lock for thread safety
auto pair = mDandelionEmbargo.find(hash);
if (pair != mDandelionEmbargo.end()) {
// Check if embargo period has expired
auto current_time = GetTime<std::chrono::microseconds>();
// Add buffer to ensure proper embargo period
return current_time < (pair->second + std::chrono::seconds{2});
return current_time < pair->second;
}
return false;
}


bool CConnman::removeDandelionEmbargo(const uint256& hash)
{
bool removed = false;
Expand Down
4 changes: 2 additions & 2 deletions src/net.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ static const int64_t DEFAULT_PEER_CONNECT_TIMEOUT = 60;
/** Number of file descriptors required for message capture **/
static const int NUM_FDS_MESSAGE_CAPTURE = 1;
/** The default setting for dandelion transactions */
static const bool DEFAULT_DANDELION = true;
static const bool DEFAULT_DANDELION = false;

static const bool DEFAULT_FORCEDNSSEED = false;
static const bool DEFAULT_DNSSEED = true;
Expand All @@ -95,7 +95,7 @@ static const int DANDELION_MAX_DESTINATIONS = 2;
/** Expected time between Dandelion routing shuffles (in seconds). */
static constexpr auto DANDELION_SHUFFLE_INTERVAL = 10min;
/** The minimum amount of time a Dandelion transaction is embargoed (seconds) */
static constexpr auto DANDELION_EMBARGO_MINIMUM = 15s;
static constexpr auto DANDELION_EMBARGO_MINIMUM = 10s;
/** The average additional embargo time beyond the minimum amount (seconds) */
static constexpr auto DANDELION_EMBARGO_AVG_ADD = 20s;

Expand Down
85 changes: 34 additions & 51 deletions src/net_processing.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1663,38 +1663,28 @@ void PeerManagerImpl::_RelayTransaction(const uint256& txid, const uint256& wtxi
void PeerManagerImpl::RelayDandelionTransaction(const CTransaction& tx, CNode* pfrom)
{
FastRandomContext rng;
bool willFluff = !m_connman.isTxDandelionEmbargoed(tx.GetHash());

bool willFluff = rng.randrange(100) < DANDELION_FLUFF;
if (willFluff) {
LogPrint(BCLog::DANDELION, "Dandelion fluff: %s\n", tx.GetHash().ToString());
CTransactionRef ptx = m_stempool.get(tx.GetHash());
if (ptx) {
{
LOCK(cs_main);
const MempoolAcceptResult result = AcceptToMemoryPool(
m_chainman.ActiveChainstate(),
m_mempool,
ptx,
false /* bypass_limits */
);

if (result.m_result_type == MempoolAcceptResult::ResultType::VALID) {
RelayTransaction(tx.GetHash(), tx.GetWitnessHash());
m_connman.removeDandelionEmbargo(tx.GetHash());
}
AcceptToMemoryPool(m_chainman.ActiveChainstate(), m_mempool, ptx, false);
}
LogPrint(BCLog::MEMPOOL, "AcceptToMemoryPool: accepted %s (poolsz %u txn, %u kB)\n",
tx.GetHash().ToString(), m_mempool.size(), m_mempool.DynamicMemoryUsage() / 1000);
RelayTransaction(tx.GetHash(), tx.GetWitnessHash());
return;
}

// Continue stem phase
CInv inv(MSG_DANDELION_TX, tx.GetHash());
CNode* destination = m_connman.getDandelionDestination(pfrom);
if (destination) {
destination->PushOtherInventory(inv);
}
}

void PeerManagerImpl::CheckDandelionEmbargoes()
{
void PeerManagerImpl::CheckDandelionEmbargoes() {
auto current_time = GetTime<std::chrono::microseconds>();

LOCK(cs_main);
Expand All @@ -1707,12 +1697,9 @@ void PeerManagerImpl::CheckDandelionEmbargoes()
LogPrint(BCLog::DANDELION, "dandeliontx %s embargo expired\n", iter->first.ToString());
CTransactionRef ptx = m_stempool.get(iter->first);
if (ptx) {
const MempoolAcceptResult result = AcceptToMemoryPool(
m_chainman.ActiveChainstate(),
m_mempool,
ptx,
false /* bypass_limits */
);
// Move transaction from stempool to mempool when embargo expires
const MempoolAcceptResult result = AcceptToMemoryPool(m_chainman.ActiveChainstate(),
m_mempool, ptx, false);
if (result.m_result_type == MempoolAcceptResult::ResultType::VALID) {
RelayTransaction(iter->first, ptx->GetWitnessHash());
}
Expand Down Expand Up @@ -4084,37 +4071,33 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
return;
}

if (msg_type == NetMsgType::DANDELIONTX) {
if (msg_type == NetMsgType::DANDELIONTX)
{
TxValidationState state;
CTransactionRef ptx;
vRecv >> ptx;
const CTransaction& tx = *ptx;
CInv inv(MSG_DANDELION_TX, tx.GetHash());
LOCK(cs_main);
if (m_connman.isDandelionInbound(&pfrom)) {
// Create mutable transaction first
CMutableTransaction mtx;
vRecv >> mtx;

// Convert to immutable transaction
CTransactionRef tx = MakeTransactionRef(std::move(mtx));
const uint256& txhash = tx->GetHash();

// Enhanced embargo check
if (m_connman.isTxDandelionEmbargoed(txhash)) {
// Send notfound to maintain privacy
m_connman.PushMessage(&pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::NOTFOUND));
return;
}

// Only process if not in stempool
if (!m_stempool.exists(txhash)) {
const MempoolAcceptResult result = AcceptToMemoryPool(
m_chainman.ActiveChainstate(),
m_stempool,
tx,
false /* bypass_limits */,
false /* test_accept */
);

if (!m_stempool.exists(inv.hash)) {
MempoolAcceptResult result = AcceptToMemoryPool(m_chainman.ActiveChainstate(), m_stempool, ptx, false);
if (result.m_result_type == MempoolAcceptResult::ResultType::VALID) {
// Add to stempool and relay
RelayDandelionTransaction(*tx, &pfrom);
LogPrint(BCLog::MEMPOOL, "AcceptToStemPool: peer=%d: accepted %s (poolsz %u txn, %u kB)\n",
pfrom.GetId(), tx.GetHash().ToString(), m_stempool.size(), m_stempool.DynamicMemoryUsage() / 1000);
auto current_time = GetTime<std::chrono::milliseconds>();
std::chrono::microseconds nEmbargo = DANDELION_EMBARGO_MINIMUM + PoissonNextSend(current_time, DANDELION_EMBARGO_AVG_ADD);
m_connman.insertDandelionEmbargo(tx.GetHash(),nEmbargo);
auto embargo_timeout = std::chrono::duration_cast<std::chrono::seconds>(nEmbargo - current_time).count();
LogPrint(BCLog::DANDELION, "dandeliontx %s embargoed for %d seconds\n", tx.GetHash().ToString(), embargo_timeout);
}
if (state.IsInvalid()) {
LogPrint(BCLog::MEMPOOLREJ, "%s from peer=%d was not accepted: %s\n", tx.GetHash().ToString(),
pfrom.GetId(), state.ToString());
}
}
if (m_stempool.exists(inv.hash)) {
RelayDandelionTransaction(tx, &pfrom);
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/node/transaction.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,9 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t
if (relay) {
if (gArgs.GetBoolArg("-dandelion", DEFAULT_DANDELION)) {
auto current_time = GetTime<std::chrono::microseconds>();
// Increase minimum embargo duration
// Increase minimum embargo time to ensure proper stem phase
std::chrono::microseconds nEmbargo = current_time +
DANDELION_EMBARGO_MINIMUM +
std::chrono::seconds{30} + // Add fixed delay
PoissonNextSend(current_time, DANDELION_EMBARGO_AVG_ADD);

if (node.connman->insertDandelionEmbargo(txid, nEmbargo)) {
Expand All @@ -140,8 +139,9 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t
}
}
}
// Fall back only if Dandelion completely fails
// Fall back to regular transaction relay if Dandelion fails
node.peerman->RelayTransaction(txid, wtxid);
}

return TransactionError::OK;
}
}
7 changes: 1 addition & 6 deletions test/functional/p2p_dandelion.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,7 @@ def set_test_params(self):
self.num_nodes = 3
self.extra_args = []
for i in range(self.num_nodes):
# Enable debug logging and increase embargo time
self.extra_args.append([
"-dandelion=1",
"-debug=dandelion",
"-dandelionembargo=30" # Increase embargo time
])
self.extra_args.append(["-dandelion=1"]) # ,"-debug=dandelion","-printtoconsole=1"

def setup_network(self):
self.setup_nodes()
Expand Down

0 comments on commit 4a5e1fe

Please sign in to comment.