This PR is a follow-up to #23801. This PR aims to make the following changes to all the time variables in net_processing.cpp wherever possible.
std::chrono.chorno::literals wherever possible.auto keywords wherever possible.var{val} convention of initialization.This PR also minimizes the number of times, serialization of time count_seconds(..) occurs.
65 static constexpr auto STALE_CHECK_INTERVAL{10min};
66 /** How frequently to check for extra outbound peers and disconnect */
67 static constexpr auto EXTRA_PEER_CHECK_INTERVAL{45s};
68 /** Minimum time an outbound-peer-eviction candidate must be connected for, in order to evict */
69-static constexpr std::chrono::seconds MINIMUM_CONNECT_TIME{30};
70+static constexpr auto MINIMUM_CONNECT_TIME{30s};
4347@@ -4348,7 +4348,7 @@ void PeerManagerImpl::CheckForStaleTipAndEvictPeers()
4348 // Check whether our tip is stale, and if so, allow using an extra
4349 // outbound peer
4350 if (!fImporting && !fReindex && m_connman.GetNetworkActive() && m_connman.GetUseAddrmanOutgoing() && TipMayBeStale()) {
4351- LogPrintf("Potential stale tip detected, will try using extra outbound peer (last tip update: %d seconds ago)\n", count_seconds(now) - count_seconds(m_last_tip_update));
4352+ LogPrintf("Potential stale tip detected, will try using extra outbound peer (last tip update: %d seconds ago)\n", count_seconds(now - m_last_tip_update.load()));
nit: (Feel free to ignore). Maybe add a line break while touching this?
0 LogPrintf("Potential stale tip detected, will try using extra outbound peer (last tip update: %d seconds ago)\n",
1count_seconds(now - m_last_tip_update.load()));
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
96 /** How long to delay requesting transactions from overloaded peers (see MAX_PEER_TX_REQUEST_IN_FLIGHT). */
97-static constexpr auto OVERLOADED_PEER_TX_DELAY = std::chrono::seconds{2};
98+static constexpr auto OVERLOADED_PEER_TX_DELAY{2s};
99 /** How long to wait (in microseconds) before downloading a transaction from an additional peer */
100-static constexpr std::chrono::microseconds GETDATA_TX_INTERVAL{std::chrono::seconds{60}};
101+static constexpr auto GETDATA_TX_INTERVAL{60us};
Sorry. I made an error interpreting this line of code earlier.
In the updated PR (commit 16d0b619d72971b23eabcb0a5849370720f93909), I have changed the variable to std::chrono::seconds.
std::chrono::microseconds GETDATA_TX_INTERVAL{std::chrono::seconds{60}}; -> auto GETDATA_TX_INTERVAL{60s};
As I could not see any apparent reason to keep it in microseconds.
Updated from 17af89fdaf0f95a21f36d54421d3bef745af13be to 16d0b619d72971b23eabcb0a5849370720f93909 (pr23832.01 -> pr23832.02)
Addressed @MarcoFalke comments
Changes:
LogPrintf line.
crACK 16d0b619d72971b23eabcb0a5849370720f93909
std::chrono::* is used when converting from int to std::chronochorno::literals is used948@@ -949,10 +949,10 @@ bool PeerManagerImpl::TipMayBeStale()
949 {
950 AssertLockHeld(cs_main);
951 const Consensus::Params& consensusParams = m_chainparams.GetConsensus();
952- if (count_seconds(m_last_tip_update) == 0) {
953+ if (m_last_tip_update.load() == 0s) {
Wrong commit? This has nothing to do with switching types?
Edit: Ok, I see it doesn’t switch the type of m_last_tip_update, but of the literal 0. Feel free to ignore.
56@@ -57,8 +57,8 @@ static constexpr auto HEADERS_DOWNLOAD_TIMEOUT_PER_HEADER = 1ms;
57 * behind headers chain.
58 */
59 static constexpr int32_t MAX_OUTBOUND_PEERS_TO_PROTECT_FROM_DISCONNECT = 4;
60-/** Timeout for (unprotected) outbound peers to sync to our chainwork, in seconds */
61-static constexpr int64_t CHAIN_SYNC_TIMEOUT = 20 * 60; // 20 minutes
62+/** Timeout for (unprotected) outbound peers to sync to our chainwork */
CHAIN_SYNC_TIMEOUT is used in docs?
Just to confirm. Are you talking about this line?
This line could use a fixing.
However, doing so will revoke the current 3 ACKs on the current top commit of this PR. So would you suggest I should update this PR for this change?
If it is fixed, it should be fixed here to avoid another follow-up pull. If not, it might be best to leave it as-is.
A re-ACK is less overhead and effort than a new pull + fresh ACK.
ACK 16d0b619d72971b23eabcb0a5849370720f9390 🔠
Signature:
0-----BEGIN PGP SIGNED MESSAGE-----
1Hash: SHA512
2
3ACK 16d0b619d72971b23eabcb0a5849370720f9390 🔠
4-----BEGIN PGP SIGNATURE-----
5
6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
7pUgWGgv/XEpKkyorblBTjNK5oX47xGt3LfLxMXjnRW59LYfE9yo1W+TkYiEYALGN
8ltJw9WQCyMpsJYLHrjL9ll6Z6w+ZE4F6MAZybTM2tOMzToDJgLT0+L6eQsb6HDgn
9bZhp+HJXtWDrL+kD7RxpNbeeSDVRGDLTUElzuBQRacShUM+aXX2vrf6/tQyih1F+
10H7E4EKH9f5ntWlxnT0l2h5ildkX6txh31fU3H9fwrKvRZ5bNZEogGF07fOaZFreE
11y06UR4wxelPw4eRKdavhe79GlV6C+uFrf7SNLDzg7CziQTKJbU41m+WZ8f06gtQK
125EkT11i7tKaEiMbUeGIFDFOYsu9xCeTRr6oCM1nomb4iNREKAIClp24XtiN8hQye
13kN7QPWoPot5LOk6geT5UN0mti6j5O//fW8kT+7Z1D7aFI7fSBwVqHNjK+YAnXrrk
14O3TSZVyXu/+ljzvYWLZUPpqQaa3durNCjOFtV+VyysP7ApasVXK6W/FXx9Zf2uiw
1549855sGB
16=q2XZ
17-----END PGP SIGNATURE-----
This commit does following changes to time variables in net_processing.h:
- Used {} initialization.
- Uses universal initializer auto.
- Uses chrono::literals
The reason for these changes is to make code simpler, and easier to
understand and rationalize.Updated from 16d0b619d72971b23eabcb0a5849370720f93909 to fe86eb50c986f7b5ccce63f984d8a51cd9ee9e2c (pr23832.02 -> pr23832.03, diff) Addressed @MarcoFalke suggestion.
Should be trivial to reACK.
re-ACK fe86eb50c986f7b5ccce63f984d8a51cd9ee9e2c 🏕
only minor doc change
Signature:
0-----BEGIN PGP SIGNED MESSAGE-----
1Hash: SHA512
2
3re-ACK fe86eb50c986f7b5ccce63f984d8a51cd9ee9e2c 🏕
4
5only minor doc change
6-----BEGIN PGP SIGNATURE-----
7
8iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
9pUhSQAv/aT3khOx+9Ns7XrLCiJPyXT33dwP69vqblTWRrSrga/mRkuvsGrAG89xG
10z3zhR8Mc5KUA2+OOshFwM4twxSzh/8ugcWAiIQ8P0zkmF+oL+wV1K0l3TLf8W8Vt
11gPhIgKt+7t+lNZYvvMCD3bTImrG0/0+/TpuV6NaP+hNKGEmswOaXoqle/kez2ZXU
12ObsjBJvlSk3EeBZRDQxh3ZuO99tkhHFM4OzHaAdhoE8zi0c6FCi+uNxSDPs/UH6Z
13rYIJnSAYxUZ/DnJUqLZTd59N/tUAZPjBbs/yso6V/tr7yt4tbVVaPaOSzbj/e95B
14pqg3xSssL6KYqUzeYudFJY0K2S9nvQM+si7STkqFZ3aSPf1GRcXCbkUEoqVENRMG
15yrgIaSC23WrRMas+CSk5gaqKHUTlpM/6WV8KnNaosQbSBhU7mi/lTf6SZNc5XomC
16mkek8R4TRft1Q7qivmOrVsSlBuGrrCRjMtc7ddwlJ2f2bz2kLgkwUzw1hsGcf+Lj
17nmp944H6
18=S8MN
19-----END PGP SIGNATURE-----