Fix bug around transaction requests

If a transaction is already in-flight when a peer announces a new tx to us, we
schedule a time in the future to reconsider whether to download. At that future
time, there was a bug that would prevent transactions from being rescheduled
for potential download again (ie if the transaction was still in-flight at the
time of reconsideration, such as from some other peer). Fix this.
This commit is contained in:
Suhas Daftuar 2019-04-22 14:13:30 -04:00
parent f635a3ba11
commit 308b76732f

View file

@ -700,6 +700,24 @@ void UpdateTxRequestTime(const uint256& txid, int64_t request_time) EXCLUSIVE_LO
} }
} }
int64_t CalculateTxGetDataTime(const uint256& txid, int64_t current_time, bool use_inbound_delay) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
int64_t process_time;
int64_t last_request_time = GetTxRequestTime(txid);
// First time requesting this tx
if (last_request_time == 0) {
process_time = current_time;
} else {
// Randomize the delay to avoid biasing some peers over others (such as due to
// fixed ordering of peer processing in ThreadMessageHandler)
process_time = last_request_time + GETDATA_TX_INTERVAL + GetRand(MAX_GETDATA_RANDOM_DELAY);
}
// We delay processing announcements from inbound peers
if (use_inbound_delay) process_time += INBOUND_PEER_TX_DELAY;
return process_time;
}
void RequestTx(CNodeState* state, const uint256& txid, int64_t nNow) EXCLUSIVE_LOCKS_REQUIRED(cs_main) void RequestTx(CNodeState* state, const uint256& txid, int64_t nNow) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{ {
@ -713,19 +731,9 @@ void RequestTx(CNodeState* state, const uint256& txid, int64_t nNow) EXCLUSIVE_L
} }
peer_download_state.m_tx_announced.insert(txid); peer_download_state.m_tx_announced.insert(txid);
int64_t process_time; // Calculate the time to try requesting this transaction. Use
int64_t last_request_time = GetTxRequestTime(txid); // fPreferredDownload as a proxy for outbound peers.
// First time requesting this tx int64_t process_time = CalculateTxGetDataTime(txid, nNow, !state->fPreferredDownload);
if (last_request_time == 0) {
process_time = nNow;
} else {
// Randomize the delay to avoid biasing some peers over others (such as due to
// fixed ordering of peer processing in ThreadMessageHandler)
process_time = last_request_time + GETDATA_TX_INTERVAL + GetRand(MAX_GETDATA_RANDOM_DELAY);
}
// We delay processing announcements from non-preferred (eg inbound) peers
if (!state->fPreferredDownload) process_time += INBOUND_PEER_TX_DELAY;
peer_download_state.m_tx_process_time.emplace(process_time, txid); peer_download_state.m_tx_process_time.emplace(process_time, txid);
} }
@ -3967,7 +3975,10 @@ bool PeerLogicValidation::SendMessages(CNode* pto)
auto& tx_process_time = state.m_tx_download.m_tx_process_time; auto& tx_process_time = state.m_tx_download.m_tx_process_time;
while (!tx_process_time.empty() && tx_process_time.begin()->first <= nNow && state.m_tx_download.m_tx_in_flight.size() < MAX_PEER_TX_IN_FLIGHT) { while (!tx_process_time.empty() && tx_process_time.begin()->first <= nNow && state.m_tx_download.m_tx_in_flight.size() < MAX_PEER_TX_IN_FLIGHT) {
const uint256& txid = tx_process_time.begin()->second; const uint256 txid = tx_process_time.begin()->second;
// Erase this entry from tx_process_time (it may be added back for
// processing at a later time, see below)
tx_process_time.erase(tx_process_time.begin());
CInv inv(MSG_TX | GetFetchFlags(pto), txid); CInv inv(MSG_TX | GetFetchFlags(pto), txid);
if (!AlreadyHave(inv)) { if (!AlreadyHave(inv)) {
// If this transaction was last requested more than 1 minute ago, // If this transaction was last requested more than 1 minute ago,
@ -3987,14 +3998,14 @@ bool PeerLogicValidation::SendMessages(CNode* pto)
// up processing to happen after the download times out // up processing to happen after the download times out
// (with a slight delay for inbound peers, to prefer // (with a slight delay for inbound peers, to prefer
// requests to outbound peers). // requests to outbound peers).
RequestTx(&state, txid, nNow); int64_t next_process_time = CalculateTxGetDataTime(txid, nNow, !state.fPreferredDownload);
tx_process_time.emplace(next_process_time, txid);
} }
} else { } else {
// We have already seen this transaction, no need to download. // We have already seen this transaction, no need to download.
state.m_tx_download.m_tx_announced.erase(inv.hash); state.m_tx_download.m_tx_announced.erase(inv.hash);
state.m_tx_download.m_tx_in_flight.erase(inv.hash); state.m_tx_download.m_tx_in_flight.erase(inv.hash);
} }
tx_process_time.erase(tx_process_time.begin());
} }