Changeset View
Standalone View
src/avalanche.cpp
// Copyright (c) 2018 The Bitcoin developers | // Copyright (c) 2018 The Bitcoin developers | ||||
// Distributed under the MIT software license, see the accompanying | // Distributed under the MIT software license, see the accompanying | ||||
// file COPYING or http://www.opensource.org/licenses/mit-license.php. | // file COPYING or http://www.opensource.org/licenses/mit-license.php. | ||||
#include "avalanche.h" | #include "avalanche.h" | ||||
#include "chain.h" | #include "chain.h" | ||||
#include "config/bitcoin-config.h" | |||||
#include "netmessagemaker.h" | #include "netmessagemaker.h" | ||||
#include "scheduler.h" | #include "scheduler.h" | ||||
#include "validation.h" | #include "validation.h" | ||||
#include <boost/range/adaptor/reversed.hpp> | #include <boost/range/adaptor/reversed.hpp> | ||||
#include <tuple> | #include <tuple> | ||||
static uint32_t countBits(uint32_t v) { | |||||
jasonbcox: Needs test(s). Something like a map of 1-byte hex values and their known countBits() results… | |||||
deadalnixAuthorUnsubmitted Done Inline ActionsI would rather keep that function as an internal implementation detail than expose it from the API, unless it is required. The behavior is tested via registerVote's test, and while that is not ideal, adding a unittest specifically for it wouldn't cut it either because it would check for the compiler intrinsic on pretty much all plateforms, which we assume is correct (and if it's not, there isn't much we can do about it). deadalnix: I would rather keep that function as an internal implementation detail than expose it from the… | |||||
jasonbcoxUnsubmitted Not Done Inline ActionsAs long as registerVote has sufficient testing, then I suppose we're good. Looking at the existing tests, I think the coverage will be enough. I'll continue to think about any new test cases. jasonbcox: As long as registerVote has sufficient testing, then I suppose we're good. Looking at the… | |||||
MengerianUnsubmitted Not Done Inline ActionsCould you keep the comment in? Return the number of bits set, as an integer value. Mengerian: Could you keep the comment in?
Maybe something like:
Return the number of bits set, as an… | |||||
#if HAVE_DECL___BUILTIN_POPCOUNT | |||||
return __builtin_popcount(v); | |||||
#else | |||||
v = v - ((v >> 1) & 0x55555555); | |||||
v = (v & 0x33333333) + ((v >> 2) & 0x33333333); | |||||
return ((v + (v >> 4) & 0xF0F0F0F) * 0x1010101) >> 24; | |||||
FabienUnsubmitted Not Done Inline Actionsavalanche.cpp:23:16: warning: suggest parentheses around ‘+’ in operand of ‘&’ [-Wparentheses] return (((v + (v >> 4)) & 0xF0F0F0F) * 0x1010101) >> 24; Fabien: avalanche.cpp:23:16: warning: suggest parentheses around ‘+’ in operand of ‘&’ [-Wparentheses]… | |||||
#endif | |||||
} | |||||
bool VoteRecord::registerVote(bool vote) { | |||||
jasonbcoxUnsubmitted Not Done Inline ActionsMoving this should be in a separate diff. Seems totally unrelated to the popcount change. jasonbcox: Moving this should be in a separate diff. Seems totally unrelated to the popcount change. | |||||
deadalnixAuthorUnsubmitted Done Inline ActionsIn fact it is. The goal here was to reduce dependencies. The popcount code is used only here and therefore only accessible here. If popcount becomes something that is generally useful, then we'll move it into some header, but in the meantime, I'd rather have it here and static. deadalnix: In fact it is. The goal here was to reduce dependencies. The popcount code is used only here… | |||||
votes = (votes << 1) | vote; | |||||
auto bits = countBits(votes & 0xff); | |||||
bool yes = bits > 6; | |||||
bool no = bits < 2; | |||||
if (!yes && !no) { | |||||
// The vote is inconclusive. | |||||
return false; | |||||
} | |||||
if (isAccepted() == yes) { | |||||
// If the vote is in agreement with our internal status, increase | |||||
// confidence. | |||||
confidence += 2; | |||||
return getConfidence() == AVALANCHE_FINALIZATION_SCORE; | |||||
} | |||||
// The vote did not agree with our internal state, in that case, reset | |||||
// confidence. | |||||
confidence = yes; | |||||
return true; | |||||
} | |||||
static bool IsWorthPolling(const CBlockIndex *pindex) { | static bool IsWorthPolling(const CBlockIndex *pindex) { | ||||
AssertLockHeld(cs_main); | AssertLockHeld(cs_main); | ||||
if (pindex->nStatus.isInvalid()) { | if (pindex->nStatus.isInvalid()) { | ||||
// No point polling invalid blocks. | // No point polling invalid blocks. | ||||
return false; | return false; | ||||
} | } | ||||
▲ Show 20 Lines • Show All 317 Lines • Show Last 20 Lines |
Needs test(s). Something like a map of 1-byte hex values and their known countBits() results, plus concatenating these hex values together of various lengths, should do the job.