Do not send dash-specific requests to masternodes before we are fully connected #1882
Conversation
UdjinM6
changed the title
… connected Open all masternode connections via CConnman::ThreadOpenMasternodeConnections only. Queue requests and process them after some timeout.
src/masternodeman.cpp
Outdated
| @@ -740,6 +740,48 @@ std::pair<CService, std::set<uint256> > CMasternodeMan::PopScheduledMnbRequestCo | |||
| return std::make_pair(pairFront.first, setResult); | |||
| } | |||
|
|
|||
| void CMasternodeMan::ProcessQueuedMnbRequests(CConnman& connman) | |||
| { | |||
| std::pair<CService, std::set<uint256> > p = mnodeman.PopScheduledMnbRequestConnection(); | |||
There was a problem hiding this comment.
Use of mnodeman is unneeded here as we are already in CMasternodeMan
src/privatesend-client.cpp
Outdated
| } | ||
|
|
||
| connman.AddPendingMasternode(infoMn.addr); | ||
| SetState(POOL_STATE_QUEUE); |
There was a problem hiding this comment.
Not sure if this is still correct. Before this PR, the connect to the MN was synchronous so at at least it was correct in the sense of successful connections (but we still didn't know if the MN accepted/processed the messages). Now we don't even know if the connection was successful. Maybe we have to introduce a CONNECTING state?
There was a problem hiding this comment.
I checked the timeout handling code as well and currently we'd have to wait 30 seconds until we run into a timeout if the connection attempt fails. Maybe we should also have a different timeout for the connection attempt?
src/privatesend-client.cpp
Outdated
| dsaQueue.push_back(std::make_pair(GetTime(), std::make_pair(infoMn.addr, CDarksendAccept(nSessionDenom, txMyCollateral)))); | ||
| } | ||
|
|
||
| SetState(POOL_STATE_QUEUE); |
src/privatesend-client.cpp
Outdated
| connman.AddPendingMasternode(infoMn.addr); | ||
| SetState(POOL_STATE_QUEUE); | ||
| nTimeLastSuccessfulStep = GetTimeMillis(); | ||
| LogPrintf("CPrivateSendClient::JoinExistingQueue -- connecting (from queue): nSessionDenom: %d (%s), addr=%s\n", |
There was a problem hiding this comment.
Can you reword the "connecting (from queue)" part so that it matches the log from StartNewQueue?
src/privatesend-client.cpp
Outdated
| return true; | ||
| } | ||
| strAutoDenomResult = _("Failed to join existing mixing queues"); |
There was a problem hiding this comment.
Would something like "Failed to find mixing queue to join" better describe what went wrong? (we didn't even try to join a queue as I understand it)
src/masternodeman.cpp
Outdated
| } | ||
|
|
||
| if (!mnbQueue.empty()) { | ||
| // wait at least 5 seconds since we tried to open corresponding connection |
There was a problem hiding this comment.
Hmm sounds like it go wrong or at least take more time then actually needed in some cases.
What about the other way around? You always loop through all fully connected nodes and for each node that is in mnbQueue you send the messages and remove the entry from mnbQueue.
After that loop, you go through mnbQueue and remove all entries that didn't get processed for some time already (which means connection failed).
This way the good nodes are processed ASAP, while the bad nodes fall out of the queue (which is not really a queue anymore) after some time.
Same applies to ProcessQueuedMnvRequests and ProcessQueuedDsa
|
Pushed few fixes. Not fixing |
… connected (dashpay#1882) * Do not send dash-specific requests to masternodes before we are fully connected Open all masternode connections via CConnman::ThreadOpenMasternodeConnections only. Queue requests and process them after some timeout. * drop excessive `mnodeman.` * switch from queues to maps of pending requests * adjust few strings, add TODO for POOL_STATE_CONNECTING * fix * there can be only one pending dsa request per ps client
Open all masternode connections via
CConnman::ThreadOpenMasternodeConnectionsonly. Queue requests and process them after some timeout.