Changes addrman to use the test-before-evict discipline in which an
address is to be evicted from the tried table is first tested and if
it is still online it is not evicted.
Adds tests to provide test coverage for this change.
This change was suggested as Countermeasure 3 in
Eclipse Attacks on Bitcoin’s Peer-to-Peer Network, Ethan Heilman,
Alison Kendler, Aviv Zohar, Sharon Goldberg. ePrint Archive Report
2015/263. March 2015.
6eddd43 Fix warnings when building with DEBUG_ADDRMAN (practicalswift)
Pull request description:
Fix warnings when building with `DEBUG_ADDRMAN`.
Warnings prior to this commit:
```
addrman.cpp:390:24: warning: comparison of integers of different signs: 'size_type' (aka 'unsigned long') and 'int' [-Wsign-compare]
if (vRandom.size() != nTried + nNew)
~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~
addrman.cpp:411:52: warning: comparison of integers of different signs: 'int' and 'size_type' (aka 'unsigned long') [-Wsign-compare]
if (info.nRandomPos < 0 || info.nRandomPos >= vRandom.size() || vRandom[info.nRandomPos] != n)
~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~
addrman.cpp:419:25: warning: comparison of integers of different signs: 'size_type' (aka 'unsigned long') and 'int' [-Wsign-compare]
if (setTried.size() != nTried)
~~~~~~~~~~~~~~~ ^ ~~~~~~
addrman.cpp:421:23: warning: comparison of integers of different signs: 'size_type' (aka 'unsigned long') and 'int' [-Wsign-compare]
if (mapNew.size() != nNew)
~~~~~~~~~~~~~ ^ ~~~~
4 warnings generated.
```
Tree-SHA512: 0316faecfe95066d2c9a0b6b3960086e43824f21a67086a895ea45fbce1327f8d6df5945fe923c2dbe4efce430bc1384d515d317c3930d97d24965e507cf734d
680bc2cbb Use range-based for loops (C++11) when looping over map elements (practicalswift)
Pull request description:
Before this commit:
```c++
for (std::map<T1, T2>::iterator x = y.begin(); x != y.end(); ++x) {
T1 z = (*x).first;
…
}
```
After this commit:
```c++
for (auto& x : y) {
T1 z = x.first;
…
}
```
Tree-SHA512: 954b136b7f5e6df09f39248a6b530fd9baa9ab59d7c2c7eb369fd4afbb591b7a52c92ee25f87f1745f47b41d6828b7abfd395b43daf84a55b4e6a3d45015e3a0
Warnings prior to this commit:
```
addrman.cpp:390:24: warning: comparison of integers of different signs: 'size_type' (aka 'unsigned long') and 'int' [-Wsign-compare]
if (vRandom.size() != nTried + nNew)
~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~
addrman.cpp:411:52: warning: comparison of integers of different signs: 'int' and 'size_type' (aka 'unsigned long') [-Wsign-compare]
if (info.nRandomPos < 0 || info.nRandomPos >= vRandom.size() || vRandom[info.nRandomPos] != n)
~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~
addrman.cpp:419:25: warning: comparison of integers of different signs: 'size_type' (aka 'unsigned long') and 'int' [-Wsign-compare]
if (setTried.size() != nTried)
~~~~~~~~~~~~~~~ ^ ~~~~~~
addrman.cpp:421:23: warning: comparison of integers of different signs: 'size_type' (aka 'unsigned long') and 'int' [-Wsign-compare]
if (mapNew.size() != nNew)
~~~~~~~~~~~~~ ^ ~~~~
4 warnings generated.
```
This changes the logging categories to boolean flags instead of strings.
This simplifies the acceptance testing by avoiding accessing a scoped
static thread local pointer to a thread local set of strings. It
eliminates the only use of boost::thread_specific_ptr outside of
lockorder debugging.
This change allows log entries to be directed to multiple categories
and makes it easy to change the logging flags at runtime (e.g. via
an RPC, though that isn't done by this commit.)
It also eliminates the fDebug global.
Configuration of unknown logging categories now produces a warning.
There are only a few uses of `insecure_random` outside the tests.
This PR replaces uses of insecure_random (and its accompanying global
state) in the core code with an FastRandomContext that is automatically
seeded on creation.
This is meant to be used for inner loops. The FastRandomContext
can be in the outer scope, or the class itself, then rand32() is used
inside the loop. Useful e.g. for pushing addresses in CNode or the fee
rounding, or randomization for coin selection.
As a context is created per purpose, thus it gets rid of
cross-thread unprotected shared usage of a single set of globals, this
should also get rid of the potential race conditions.
- I'd say TxMempool::check is not called enough to warrant using a special
fast random context, this is switched to GetRand() (open for
discussion...)
- The use of `insecure_rand` in ConnectThroughProxy has been replaced by
an atomic integer counter. The only goal here is to have a different
credentials pair for each connection to go on a different Tor circuit,
it does not need to be random nor unpredictable.
- To avoid having a FastRandomContext on every CNode, the context is
passed into PushAddress as appropriate.
There remains an insecure_random for test usage in `test_random.h`.
If a node is offline failed outbound connection attempts will crank up
the addrman counter and effectively blow away our state.
This change reduces the problem by only counting attempts made while
the node believes it has outbound connections to at least two
netgroups.
Connect and addnode connections are also not counted, as there is no
reason to unequally penalize them for their more frequent
connections -- though there should be no real effect from this
unless their addnode configureation is later removed.
Wasteful repeated connection attempts while only a few connections are
up are avoided via nLastTry.
This is still somewhat incomplete protection because our outbound
peers could be down but not timed out or might all be on 'local'
networks (although the requirement for multiple netgroups helps).
Adds several unittests for CAddrMan and CAddrInfo.
Increases the accuracy of addrman tests.
Removes non-determinism in tests by overriding the random number generator.
Extracts testing code from addrman class to test class.
On hosts that had spent some time with a failed internet connection their
nAttempts penalty was going through the roof (e.g. thousands for all peers)
and as a result the connect search was pegging the CPU and failing to get
more than a 4 connections after days of running (because it was taking so
long per try).
This change was suggested as Countermeasure 2 in
Eclipse Attacks on Bitcoin’s Peer-to-Peer Network, Ethan Heilman,
Alison Kendler, Aviv Zohar, Sharon Goldberg. ePrint Archive Report
2015/263. March 2015.
This change was suggested as Countermeasure 2 in
Eclipse Attacks on Bitcoin’s Peer-to-Peer Network, Ethan Heilman,
Alison Kendler, Aviv Zohar, Sharon Goldberg. ePrint Archive Report
2015/263. March 2015.
Give each address a single fixed location in the new and tried tables,
which become simple fixed-size arrays instead of sets and vectors.
This prevents attackers from having an advantages by inserting an
address multiple times.
This change was suggested as Countermeasure 1 in
Eclipse Attacks on Bitcoin’s Peer-to-Peer Network, Ethan Heilman,
Alison Kendler, Aviv Zohar, Sharon Goldberg. ePrint Archive Report
2015/263. March 2015.
It is also more efficient.
After the tinyformat switch sprintf() family functions support passing
actual std::string objects.
Remove unnecessary c_str calls (236 of them) in logging and formatting.
The function Get64(.) has a bug in case the width is not divisible by 64.
Since it is only ever used as Get64(0) this simply changes it to this
special case. Additionally, an assert is added, and a cast to prevent
a compiler error.
Use misc methods of avoiding unnecesary header includes.
Replace int typedefs with int##_t from stdint.h.
Replace PRI64[xdu] with PRI[xdu]64 from inttypes.h.
Normalize QT_VERSION ifs where possible.
Resolve some indirect dependencies as direct ones.
Remove extern declarations from .cpp files.
