Skip to content

Commit e96bfe5

Browse files
committed
dash(embedded): fix CR-1 cold-start dial wedge + CR-2 dormant daemon-disjointness
Review of the CoinPeerManager port (#794) required two fixes, both in the new --coin-p2p-discover opt-in path. CR-1 — daemonless cold-start dial wedge: CoinClient::connect() early-returned on an empty target list without arming the reconnect loop, and update_dial_targets() ignored empty lists. So --coin-p2p-discover WITHOUT --coin-p2p-connect on a first run with DNS unavailable (empty initial dial) armed nothing and wedged until restart. Fix: connect() arms the 30s reconnect loop unconditionally (empty-plan guard on the tick so current()/advance() never throw), and update_dial_targets() kicks an immediate dial on the empty->non-empty transition — so the arm connects as soon as fixed seeds (t+60s) / HTTP seeds (t+90s) arrive. Extracted arm_reconnect_timer() helper. Regression pin added (empty_connect_arms_without_wedging). CR-2 — overlap filter + coind -20 penalty were dormant as wired: main_dash never called set_getpeerinfo_fn, so m_coind_peers stayed empty and no peer was ever Source::coind — the active daemon-disjointness never engaged (only passive independence). Fix: added NodeRPC::getpeerinfo() to the DASH RPC (parses each entry's addr, IPv6-bracket aware) and wired it in the discover path when a dashd RPC is armed, so the -20 penalty + overlap filter engage in the connect+discover / oracle-shadow deployment. Absent RPC (fully daemonless) = seeds-only passive independence. Mirrors main_ltc.cpp getpeerinfo wiring. Nits: pinned target no longer duplicated in the dial list (pinned keys passed as the get_peers_to_connect exclusion set); dead is_daemon_overlap_peer() removed; declaration/destruction order corrected so the manager outlives the client and refresh timer that borrow it. Build clean; full test_dash_p2p_node target 23/23 (was 22 + CR-1 pin); --selftest unchanged.
1 parent ef3a70e commit e96bfe5

6 files changed

Lines changed: 144 additions & 32 deletions

File tree

src/c2pool/main_dash.cpp

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -731,12 +731,15 @@ int run_node(bool testnet, const std::string& rpc_endpoint,
731731
// layers, never conflated. The client rides the SAME ioc as the
732732
// sharechain node and stratum; declared after config/coin_state (both of
733733
// which it borrows), so it is destroyed before them at scope exit.
734-
std::unique_ptr<dash::coin::p2p::CoinClient<dash::Config>> coin_p2p;
735734
// DASH-ISOLATED scored/diverse peer discovery (--coin-p2p-discover). Owns
736735
// its own peer table + seed bootstrap + scoring; feeds the single embedded
737-
// connection an INDEPENDENT (non-dashd) diverse peer set. Declared BEFORE
738-
// coin_p2p so it outlives the client that borrows it (reverse-destruction).
736+
// connection an INDEPENDENT (non-dashd) diverse peer set. Declared FIRST of
737+
// the three so it is destroyed LAST — the CoinClient's callbacks and the
738+
// refresh timer both capture it by raw pointer, so it must outlive both.
739739
std::unique_ptr<dash::coin::DashCoinPeerManager> coin_peer_mgr;
740+
std::unique_ptr<dash::coin::p2p::CoinClient<dash::Config>> coin_p2p;
741+
// Refresh timer declared LAST -> destroyed FIRST: it stops (and its lambda
742+
// stops capturing coin_p2p / coin_peer_mgr) before either is torn down.
740743
std::unique_ptr<core::Timer> coin_dial_refresh_timer;
741744
if (!coin_p2p_targets.empty() || coin_p2p_discover) {
742745
config.coin()->m_testnet = testnet;
@@ -770,6 +773,25 @@ int run_node(bool testnet, const std::string& rpc_endpoint,
770773
}
771774
coin_peer_mgr->set_dns_seeds(dash::coin::dash_dns_seeds(testnet));
772775
coin_peer_mgr->set_fixed_seeds(dash::coin::dash_fixed_seeds(testnet));
776+
777+
// ── ACTIVE daemon-disjointness (connect+discover / oracle-shadow) ──
778+
// When a local dashd RPC is armed, feed its getpeerinfo so the
779+
// manager tracks the daemon's OWN peers (m_coind_peers). That is
780+
// what makes the coind-source -20 penalty AND the daemon-peer
781+
// overlap filter engage — without it those ported mechanisms stay
782+
// dormant (m_coind_peers empty => no peer ever Source::coind) and
783+
// only passive independence holds. Mirrors main_ltc.cpp:5484.
784+
// Absent RPC (fully daemonless) => seeds-only, passive independence.
785+
if (rpc) {
786+
coin_peer_mgr->set_getpeerinfo_fn(
787+
[rp = rpc.get()]() -> std::vector<NetService> {
788+
try { return rp->getpeerinfo(); }
789+
catch (...) { return {}; }
790+
});
791+
std::cout << "[run] daemon-disjointness ACTIVE: dashd getpeerinfo "
792+
"wired -> coind -20 penalty + overlap filter engage\n";
793+
}
794+
773795
coin_peer_mgr->start();
774796

775797
// addr-crawl discoveries feed back into the manager (source-scored
@@ -787,11 +809,18 @@ int run_node(bool testnet, const std::string& rpc_endpoint,
787809
mgr->notify_disconnected(s.to_string());
788810
});
789811

812+
// Pinned keys: passed to get_peers_to_connect() as the "already
813+
// handled" set so the protected pinned node (score 999999, always
814+
// ranked first) is NOT also appended by the scorer — the pinned
815+
// entries are prepended explicitly, so this dedups them out.
816+
std::set<std::string> pinned_keys;
817+
for (auto& t : coin_p2p_targets) pinned_keys.insert(t.to_string());
818+
790819
// Initial dial plan: pinned local dashd first (preferred), then the
791-
// top scored+diverse discovered peers.
820+
// top scored+diverse discovered peers (pinned excluded from that set).
792821
std::vector<NetService> dial;
793822
for (auto& t : coin_p2p_targets) dial.push_back(t);
794-
for (auto& ep : coin_peer_mgr->get_peers_to_connect({}))
823+
for (auto& ep : coin_peer_mgr->get_peers_to_connect(pinned_keys))
795824
dial.push_back(ep.to_net_service());
796825
coin_p2p->connect(dial);
797826

@@ -801,9 +830,10 @@ int run_node(bool testnet, const std::string& rpc_endpoint,
801830
coin_dial_refresh_timer = std::make_unique<core::Timer>(&ioc, /*repeat=*/true);
802831
coin_dial_refresh_timer->start(60, [cp = coin_p2p.get(),
803832
mgr = coin_peer_mgr.get(),
804-
pinned = coin_p2p_targets]() {
805-
std::vector<NetService> refreshed = pinned; // pinned always preferred
806-
for (auto& ep : mgr->get_peers_to_connect({}))
833+
pinned = coin_p2p_targets,
834+
pinned_keys]() {
835+
std::vector<NetService> refreshed = pinned; // pinned always preferred, first
836+
for (auto& ep : mgr->get_peers_to_connect(pinned_keys))
807837
refreshed.push_back(ep.to_net_service());
808838
cp->update_dial_targets(std::move(refreshed));
809839
});

src/impl/dash/coin/coin_peer_manager.hpp

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -467,15 +467,6 @@ class DashCoinPeerManager
467467
return static_cast<int>(m_peers.size()) < m_config.max_peers;
468468
}
469469

470-
/// Whether a given peer key is the daemon's own peer (overlap filter).
471-
/// Exposed so the dialer can prefer independent (non-overlapping) peers —
472-
/// the crux of "different peers → independent mempool/relay view".
473-
bool is_daemon_overlap_peer(const std::string& key) const
474-
{
475-
std::lock_guard<std::mutex> lock(m_mutex);
476-
return m_coind_peers.count(key) > 0;
477-
}
478-
479470
/// Remove peers that are exhausted (max attempts reached, not protected).
480471
void prune_dead_peers()
481472
{

src/impl/dash/coin/p2p_client.hpp

Lines changed: 47 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -235,25 +235,27 @@ class CoinClient : public core::ICommunicator, public core::INetwork, public cor
235235

236236
/// Dial the given targets with automatic reconnection (30s interval,
237237
/// round-robin over the target list on each retry).
238+
///
239+
/// An EMPTY target list is legal in the --coin-p2p-discover cold-start case
240+
/// (fresh peer-db + DNS unavailable): the reconnect loop is armed anyway and
241+
/// simply idles (the empty-plan guard below), so when update_dial_targets()
242+
/// later delivers seed-discovered peers (fixed seeds at t+60s / HTTP at
243+
/// t+90s) the dial starts — no restart needed. Without this, an initially
244+
/// empty discover peer set would wedge permanently.
238245
void connect(std::vector<NetService> targets)
239246
{
240-
if (targets.empty()) return;
241247
m_dial_plan.set_targets(std::move(targets));
242248
m_reconnect_enabled = true;
243-
LOG_INFO << "[" << m_chain_label << "] dialing "
244-
<< m_dial_plan.current().to_string()
245-
<< " (" << m_dial_plan.size() << " target[s] in plan)";
246-
core::Factory<core::Client>::connect(m_dial_plan.current());
247-
248-
m_reconnect_timer = std::make_unique<core::Timer>(m_context, /*repeat=*/true);
249-
m_reconnect_timer->start(RECONNECT_INTERVAL_SEC, [this]() {
250-
if (!m_peer && m_reconnect_enabled) {
251-
const auto& target = m_dial_plan.advance();
252-
LOG_INFO << "[" << m_chain_label << "] reconnecting to "
253-
<< target.to_string() << "...";
254-
core::Factory<core::Client>::connect(target);
255-
}
256-
});
249+
if (!m_dial_plan.empty()) {
250+
LOG_INFO << "[" << m_chain_label << "] dialing "
251+
<< m_dial_plan.current().to_string()
252+
<< " (" << m_dial_plan.size() << " target[s] in plan)";
253+
core::Factory<core::Client>::connect(m_dial_plan.current());
254+
} else {
255+
LOG_INFO << "[" << m_chain_label << "] no initial dial targets; "
256+
"reconnect loop armed, awaiting seed discovery";
257+
}
258+
arm_reconnect_timer();
257259
}
258260

259261
/// Refresh the reconnect dial plan in place WITHOUT tearing the current
@@ -263,12 +265,24 @@ class CoinClient : public core::ICommunicator, public core::INetwork, public cor
263265
/// reconnect the single embedded connection rotates onto an INDEPENDENT
264266
/// peer — the mechanism that graduates the embedded arm to a network-
265267
/// standalone witness. Empty target lists are ignored (never wedge redial).
268+
///
269+
/// Cold-start kick: if the plan was EMPTY (the discover daemonless case) and
270+
/// we are currently disconnected with the reconnect loop armed, dial the
271+
/// first new target immediately rather than waiting up to 30s for the next
272+
/// reconnect tick — so the arm connects as soon as seeds arrive.
266273
void update_dial_targets(std::vector<NetService> targets)
267274
{
268275
if (targets.empty()) return;
276+
bool was_empty = m_dial_plan.empty();
269277
m_dial_plan.set_targets(std::move(targets));
270278
LOG_DEBUG_COIND << "[" << m_chain_label << "] dial plan refreshed ("
271279
<< m_dial_plan.size() << " scored target[s])";
280+
if (was_empty && !m_peer && m_reconnect_enabled) {
281+
LOG_INFO << "[" << m_chain_label << "] cold-start dial "
282+
<< m_dial_plan.current().to_string()
283+
<< " (first seed-discovered target)";
284+
core::Factory<core::Client>::connect(m_dial_plan.current());
285+
}
272286
}
273287

274288
// INetwork
@@ -460,6 +474,24 @@ class CoinClient : public core::ICommunicator, public core::INetwork, public cor
460474
}
461475

462476
private:
477+
/// Arm the 30s round-robin reconnect loop. The empty-plan guard makes it
478+
/// safe to arm before any target exists (--coin-p2p-discover cold start):
479+
/// the tick no-ops until update_dial_targets() supplies seed-discovered
480+
/// peers, then rotates over them. current()/advance() on an empty plan
481+
/// would throw, hence the guard.
482+
void arm_reconnect_timer()
483+
{
484+
m_reconnect_timer = std::make_unique<core::Timer>(m_context, /*repeat=*/true);
485+
m_reconnect_timer->start(RECONNECT_INTERVAL_SEC, [this]() {
486+
if (!m_peer && m_reconnect_enabled && !m_dial_plan.empty()) {
487+
const auto& target = m_dial_plan.advance();
488+
LOG_INFO << "[" << m_chain_label << "] reconnecting to "
489+
<< target.to_string() << "...";
490+
core::Factory<core::Client>::connect(target);
491+
}
492+
});
493+
}
494+
463495
void ensure_timeout_timer()
464496
{
465497
if (!m_timeout_timer)

src/impl/dash/coin/rpc.cpp

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -528,6 +528,40 @@ std::string NodeRPC::getbestblockhash()
528528
return {};
529529
}
530530

531+
// getpeerinfo -> the dashd's own connected-peer addresses. Each entry's "addr"
532+
// is "host:port" (IPv6 as "[::1]:9999"); parsed to NetService. Empty on a
533+
// non-array/absent result. Validation (routable/port) is the peer manager's.
534+
std::vector<NetService> NodeRPC::getpeerinfo()
535+
{
536+
std::vector<NetService> peers;
537+
auto result = CallAPIMethod("getpeerinfo");
538+
if (!result.is_array())
539+
return peers;
540+
for (auto& entry : result)
541+
{
542+
if (!entry.contains("addr") || !entry["addr"].is_string())
543+
continue;
544+
std::string addr = entry["addr"].get<std::string>();
545+
auto colon = addr.rfind(':');
546+
if (colon == std::string::npos)
547+
continue;
548+
std::string host = addr.substr(0, colon);
549+
uint16_t port = 0;
550+
try {
551+
port = static_cast<uint16_t>(std::stoul(addr.substr(colon + 1)));
552+
} catch (...) {
553+
continue;
554+
}
555+
// Strip IPv6 brackets: "[2001:db8::1]" -> "2001:db8::1".
556+
if (host.size() >= 2 && host.front() == '[' && host.back() == ']')
557+
host = host.substr(1, host.size() - 2);
558+
if (host.empty() || port == 0)
559+
continue;
560+
peers.emplace_back(host, port);
561+
}
562+
return peers;
563+
}
564+
531565
// verbose: true -- json result, false -- hex-encode result;
532566
nlohmann::json NodeRPC::getblockheader(uint256 header, bool verbose)
533567
{

src/impl/dash/coin/rpc.hpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,14 @@ class NodeRPC : public jsonrpccxx::IClientConnector
165165
// template refresh without waiting on the 30 s staleness TTL. Empty string
166166
// on a null/absent result.
167167
std::string getbestblockhash();
168+
// getpeerinfo -> the dashd's OWN connected-peer addresses (the "addr" field
169+
// of each entry), parsed to NetService. Feeds the embedded
170+
// DashCoinPeerManager's daemon-peer overlap filter + coind-source -20 score
171+
// penalty so the embedded arm actively avoids mirroring the local dashd's
172+
// peers (network-view disjointness). Returns empty on a null/absent result;
173+
// transport errors propagate (the caller swallows them). No dashd config
174+
// change required (getpeerinfo is a default RPC).
175+
std::vector<NetService> getpeerinfo();
168176
// verbose: true -- json result, false -- hex-encode result;
169177
nlohmann::json getblockheader(uint256 header, bool verbose = true);
170178
// verbosity: 0 for hex-encoded data, 1 for a json object, and 2 for json object with transaction data

test/test_dash_coin_p2p_client.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,23 @@ struct ClientRig
173173
}
174174
};
175175

176+
// ── (b') Cold-start empty dial plan (daemonless --coin-p2p-discover) ────────
177+
//
178+
// Regression pin for the discover cold-start wedge: connect() with an EMPTY
179+
// target list (fresh peer-db + DNS unavailable) must NOT early-return into a
180+
// dead state — it arms the reconnect loop and idles, leaving the client
181+
// safely disconnected (no throw, no session) until seed-discovered peers land
182+
// via update_dial_targets(). An empty update is a safe no-op.
183+
TEST(DashCoinP2PClient, empty_connect_arms_without_wedging)
184+
{
185+
ClientRig rig;
186+
EXPECT_NO_THROW(rig.client.connect({})); // empty initial dial plan
187+
EXPECT_FALSE(rig.client.is_connected());
188+
EXPECT_FALSE(rig.client.is_handshake_complete());
189+
EXPECT_NO_THROW(rig.client.update_dial_targets({})); // empty refresh: no-op
190+
EXPECT_FALSE(rig.client.is_connected());
191+
}
192+
176193
TEST(DashCoinP2PClient, client_completes_handshake_and_captures_peer_metadata)
177194
{
178195
ClientRig rig;

0 commit comments

Comments
 (0)