net: Add test-before-evict discipline to addrman #9037

pull EthanHeilman wants to merge 1 commits into bitcoin:master from EthanHeilman:test-before-evict changing 4 files +319 −19
  1. EthanHeilman commented at 9:53 pm on October 28, 2016: contributor

    This change implement countermeasures 3 (test-before-evict) suggested in our paper: “Eclipse Attacks on Bitcoin’s Peer-to-Peer Network”.

    Design:

    A collision occurs when an address, addr1, is being moved to the tried table from the new table, but maps to a position in the tried table which already contains an address (addr2). The current behavior is that addr1 would evict addr2 from the tried table.

    This change ensures that during a collision, addr1 is not inserted into tried but instead inserted into a buffer (setTriedCollisions). The to-be-evicted address, addr2, is then tested by a feeler connection. If addr2 is found to be online, we remove addr1 from the buffer and addr2 is not evicted, on the other hand if addr2 is found be offline it is replaced by addr1.

    An additional small advantage of this change is that, as no more than ten addresses can be in the test buffer at once, and addresses are only cleared one at a time from the test buffer (at 2 minute intervals), thus an attacker is forced to wait at least two minutes to insert a new address into tried after filling up the test buffer. This rate limits an attacker attempting to launch an eclipse attack.

    Risk mitigation:

    • To prevent this functionality from being used as a DoS vector, we limit the number of addresses which are to be tested to ten. If we have more than ten addresses to test, we drop new addresses being added to tried if they would evict an address. Since the feeler thread only creates one new connection every 2 minutes the additional network overhead is limited.
    • An address in tried gains immunity from tests for 4 hours after it has been tested or successfully connected to.

    Tests:

    This change includes additional addrman unittests which test this behavior.

    I ran an instance of this change with a much smaller tried table (2 buckets of 64 addresses) so that collisions were much more likely and observed evictions.

    02016-10-27 07:20:26 Swapping 208.12.64.252:8333 for 68.62.95.247:8333 in tried table
12016-10-27 07:20:26 Moving 208.12.64.252:8333 to tried

    I documented tests we ran against similar earlier versions of this change in #6355.

    Security Benefit

    This is was originally posted in PR #8282 see this comment for full details.

    To determine the security benefit of these larger numbers of IPs in the tried table I modeled the attack presented in Eclipse Attacks on Bitcoin’s Peer-to-Peer Network.

    attackergraph40000-10-1000short-line

    Default node: 595 attacker IPs for ~50% attack success. Default node + test-before-evict: 620 attacker IPs for ~50% attack success. Feeler node: 5540 attacker IPs for ~50% attack success. Feeler node + test-before-evict: 8600 attacker IPs for ~50% attack success.

    The node running feeler connections has 10 times as many online IP addresses in its tried table making an attack 10 times harder (i.e. requiring the an attacker require 10 times as many IP addresses in different /16s). Adding test-before-evict increases resistance of the node by an additional 3000 attacker IP addresses.

    Below I graph the attack over even greater attacker resources (i.e. more attacker controled IP addresses). Note that test-before-evict maintains some security far longer even against an attacker with 50,000 IPs. If this node had a larger tried table test-before-evict could greatly boost a nodes resistance to eclipse attacks.

    attacker graph long view

  2. EthanHeilman commented at 9:54 pm on October 28, 2016: contributor
    This is the test-before-evict feature originally proposed in #6355.
  3. EthanHeilman commented at 10:04 pm on October 28, 2016: contributor

    One issue which needs to be address in my code above is that I read from mapInfo via [ ]. This violates the very sensible coding style guide requirement that [ ] is never used for map reads. I could use an assert as is used in other code in addrman:

    0assert(mapInfo.count(nId) == 1);

    Instead I’d prefer to create a private method for accessing mapInfo elements by nId similar to CAddrMan::Find. As this is a refactor task which touches code outside of test-before-evict, I want to break it out into a separate commit to be included in this pull request.

  4. in src/addrman.cpp: in 18e642c9c5 outdated 
    569+
570+            // Has attempted to connect and failed in last X hours
571+            } else if (GetAdjustedTime() - infoOld.nLastTry < ADDRMAN_REPLACEMENT_HOURS*(60*60)) {
572+                LogPrint("addrman", "Swapping %s for %s in tried table\n", infoNew.ToString(), infoOld.ToString());
573+                // Replaces an existing address already in the tried table with the new address
574+                Good(infoNew, false);
    kazcw commented at 10:27 pm on October 28, 2016:
    Lock is held, could use Good_
  5. in src/addrman.cpp: in 18e642c9c5 outdated 
    573+                // Replaces an existing address already in the tried table with the new address
574+                Good(infoNew, false);
575+                eraseCollision = true;
576+            }
577+        } else { // Collision is not actually a collision anymore
578+            Good(infoNew, false);
    kazcw commented at 10:28 pm on October 28, 2016:
    Lock held as above
    EthanHeilman commented at 2:00 am on October 30, 2016:
    Good call!
  6. in src/addrman.cpp: in 18e642c9c5 outdated 
    539@@ -528,3 +540,75 @@ void CAddrMan::SetServices_(const CService& addr, ServiceFlags nServices)
540 int CAddrMan::RandomInt(int nMax){
541     return GetRandInt(nMax);
542 }
543+
544+void CAddrMan::ResolveCollisions_()
545+{
546+    for (std::set<int>::iterator it = setTriedCollisions.begin(); it != setTriedCollisions.end();) {
547+        int nIdnew = (int)*it;
    kazcw commented at 10:29 pm on October 28, 2016:
    Nit: is this cast necessary?
    EthanHeilman commented at 2:00 am on October 30, 2016:
    Nope, removed it.
  7. in src/addrman.cpp: in 18e642c9c5 outdated 
    591+{
592+    int nRnd = GetRandInt(setTriedCollisions.size());
593+
594+    int i = 0;
595+    // Selects a random element from setTriedCollisions.
596+    for (std::set<int>::iterator it = setTriedCollisions.begin(); it != setTriedCollisions.end(); it++) {
    kazcw commented at 11:15 pm on October 28, 2016:
    Nit: I think std::advance would be clearer than a loop for picking one element
  8. kazcw approved
  9. kazcw commented at 11:41 pm on October 28, 2016: contributor
    utACK the implementation as of 18e642c, though I haven’t read up on the details of the attack yet
  10. fanquake added the label P2P on Oct 29, 2016
  11. EthanHeilman commented at 7:38 pm on November 1, 2016: contributor
    I added some extra checks to ensure that if setTriedCollisions points to a key which doesn’t exist in mapInfo it just removes that bad key.
  12. EthanHeilman commented at 11:36 pm on November 21, 2016: contributor
    Is this good to merge? Any changes I should make?
  13. EthanHeilman commented at 0:38 am on December 9, 2016: contributor
    Fixed a merge conflict.
  14. EthanHeilman commented at 8:51 pm on December 18, 2016: contributor
    @sipa Any suggestions or improvements I can make to this?
  15. sipa commented at 3:45 pm on April 9, 2017: member
    utACK @EthanHeilman Sorry for the delay here. I think we should try to get this in for 0.15. Would you mind rebasing again?
  16. EthanHeilman commented at 5:05 pm on April 10, 2017: contributor
    @sipa Will rebase today or tomorrow.
  17. EthanHeilman commented at 1:43 pm on April 13, 2017: contributor
    @sipa rebased, also updated some of the log statement to follow the new format.
  18. EthanHeilman commented at 8:35 am on July 27, 2017: contributor
    Updated to obey new Bitcoin coding style guide
  19. EthanHeilman commented at 8:48 pm on August 10, 2017: contributor
    ping @theuni
  20. theuni commented at 9:51 pm on August 16, 2017: member
    @EthanHeilman Thanks for adapting the style. I’ll review thoroughly soon.
  21. in src/addrman.cpp:558 in 8604020bab outdated 
    553+
554+            if (!info_new.IsValid()) { // id_new may no longer map to a valid address
555+                erase_collision = true;
556+
557+            // The position in the tried bucket is not empty
558+            } else if (vvTried[tried_bucket][tried_bucket_pos] != -1) {
    kallewoof commented at 1:19 am on February 20, 2018:

    Nit: The above if case looks misleadingly like a single-no-curling-brace-block (I misread it as such) until you realize the thing below the comment (// The position...) is actually ending the block.

    Maybe move the comment inside and indented inside the else block?

    EthanHeilman commented at 2:28 am on February 26, 2018:
    It looked weird below the else if and in a brief search I couldn’t find any other examples of that style in code. Instead I moved it to right of the if else.
  22. in src/addrman.cpp:569 in 8604020bab outdated 
    564+                // Has successfully connected in last X hours
565+                if (GetAdjustedTime() - info_old.nLastSuccess < ADDRMAN_REPLACEMENT_HOURS*(60*60)) {
566+                    erase_collision = true;
567+
568+                // Has attempted to connect and failed in last X hours
569+                } else if (GetAdjustedTime() - info_old.nLastTry < ADDRMAN_REPLACEMENT_HOURS*(60*60)) {
    kallewoof commented at 1:20 am on February 20, 2018:
    Same here, in case you address the above indentation/readability concern.
  23. in src/addrman.cpp:542 in 8604020bab outdated 
    537+void CAddrMan::ResolveCollisions_()
538+{
539+    for (std::set<int>::iterator it = m_tried_collisions.begin(); it != m_tried_collisions.end();) {
540+        int id_new = *it;
541+
542+        bool erase_collision = false;
    kallewoof commented at 1:24 am on February 20, 2018:
    It seems like you can reduce the lines below by defaulting to true here, as several cases simply turn it on. Unless it is too complicated to swap.
    EthanHeilman commented at 2:23 am on February 26, 2018:
    I appreciate this comment, but I’m concerned it would be more confusing to read if I shortened it.
  24. in src/addrman.cpp:600 in 8604020bab outdated 
    595+
596+    // Selects a random element from m_tried_collisions
597+    std::advance(it, GetRandInt(m_tried_collisions.size()));
598+    int id_new = *it;
599+
600+    // If newId not found in mapInfo remove it from m_tried_collisions
    kallewoof commented at 1:26 am on February 20, 2018:
    Comment mentions old name (newId).
    EthanHeilman commented at 2:24 am on February 26, 2018:
    Good catch, thanks!
  25. in src/addrman.cpp:615 in 8604020bab outdated 
    610+    int tried_bucket_pos = newInfo.GetBucketPosition(nKey, false, tried_bucket);
611+
612+    int id_old = vvTried[tried_bucket][tried_bucket_pos];
613+    CAddrInfo& info_old = mapInfo[id_old];
614+
615+    return info_old;
    kallewoof commented at 1:27 am on February 20, 2018:
    Maybe just return mapInfo[id_old];?
    EthanHeilman commented at 2:28 am on February 26, 2018:
    done
  26. in src/test/addrman_tests.cpp:577 in 8604020bab outdated 
    572+    BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0");
573+
574+    // Add twenty two addresses.
575+    CNetAddr source = ResolveIP("252.2.2.2");
576+    for (unsigned int i = 1; i < 23; i++) {
577+        CService addr = ResolveService("250.1.1."+boost::to_string(i));
    kallewoof commented at 1:29 am on February 20, 2018:
    (Tests so maybe no biggie but) use std::to_string() not boost::to_string()
  27. in src/test/addrman_tests.cpp:588 in 8604020bab outdated 
    583+        BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0");
584+    }
585+
586+    // Ensure Good handles duplicates well.
587+    for (unsigned int i = 1; i < 23; i++) {
588+        CService addr = ResolveService("250.1.1."+boost::to_string(i));
    kallewoof commented at 1:29 am on February 20, 2018:
    std::to_string
    EthanHeilman commented at 10:16 pm on February 20, 2018:
    Let me fix these and all push later tonight
    EthanHeilman commented at 2:36 am on February 26, 2018:
    Fixed
  28. kallewoof commented at 1:30 am on February 20, 2018: member
    utACK 8604020bab3963599ebfd933b91096ae1f686d69
  29. in src/addrman.cpp:242 in 1fdbcc850a outdated 
    238+    int tried_bucket = info.GetTriedBucket(nKey);
239+    int tried_bucket_pos = info.GetBucketPosition(nKey, false, tried_bucket);
240+
241+    // Will moving this address into tried evict another entry?
242+    if (test_before_evict && (vvTried[tried_bucket][tried_bucket_pos] != -1)) {
243+        LogPrint(BCLog::ADDRMAN, "addrman", "Collision inserting element into tried table, moving %s to m_tried_collisions=%d \n", addr.ToString(), m_tried_collisions.size());
    kallewoof commented at 2:39 am on February 26, 2018:
    Nit: remove space in =%d \n
  30. in src/addrman.cpp:244 in 1fdbcc850a outdated 
    241+    // Will moving this address into tried evict another entry?
242+    if (test_before_evict && (vvTried[tried_bucket][tried_bucket_pos] != -1)) {
243+        LogPrint(BCLog::ADDRMAN, "addrman", "Collision inserting element into tried table, moving %s to m_tried_collisions=%d \n", addr.ToString(), m_tried_collisions.size());
244+
245+        if (m_tried_collisions.size() < ADDRMAN_SET_TRIED_COLLISION_SIZE)
246+            m_tried_collisions.insert(nId);
    kallewoof commented at 2:40 am on February 26, 2018:
    Always use {} brackets for multiline ifs (new convention as of a while ago).
  31. in src/addrman.cpp:556 in 1fdbcc850a outdated 
    551+            int tried_bucket = info_new.GetTriedBucket(nKey);
552+            int tried_bucket_pos = info_new.GetBucketPosition(nKey, false, tried_bucket);
553+
554+            if (!info_new.IsValid()) { // id_new may no longer map to a valid address
555+                erase_collision = true;
556+
    kallewoof commented at 2:41 am on February 26, 2018:
    Nit: IMO nix newline (I understand that you want to space out for readability but this isn’t usually done, and some argue that more compact gives a better overview).
    EthanHeilman commented at 4:51 pm on March 3, 2018:
    Keeping new lines between code and comments as it more clearly distinguishes which line the comment is associated with. Removing newlines in other circumstances
  32. in src/addrman.cpp:556 in 1fdbcc850a outdated 
    553+
554+            if (!info_new.IsValid()) { // id_new may no longer map to a valid address
555+                erase_collision = true;
556+            
557+            } else if (vvTried[tried_bucket][tried_bucket_pos] != -1) { // The position in the tried bucket is not empty
558+
    kallewoof commented at 2:42 am on February 26, 2018:
    Nit: see above (your call, of course)
  33. in src/addrman.cpp:566 in 1fdbcc850a outdated 
    561+                CAddrInfo& info_old = mapInfo[id_old];
562+
563+                // Has successfully connected in last X hours
564+                if (GetAdjustedTime() - info_old.nLastSuccess < ADDRMAN_REPLACEMENT_HOURS*(60*60)) {
565+                    erase_collision = true;
566+
    kallewoof commented at 2:42 am on February 26, 2018:
    -'-
    EthanHeilman commented at 4:54 pm on March 3, 2018:
    What does -’- mean?
    kallewoof commented at 9:54 pm on March 4, 2018:
    Sorry, it means “same as above”. Like “Ditto mark”.
  34. in src/addrman.cpp:590 in 1fdbcc850a outdated 
    585+}
586+
587+CAddrInfo CAddrMan::SelectTriedCollision_()
588+{
589+    if (m_tried_collisions.size() == 0)
590+        return CAddrInfo();
    kallewoof commented at 2:43 am on February 26, 2018:
    Use {} for multiline if
    EthanHeilman commented at 4:58 pm on March 3, 2018:
    I reread the style guide more carefully and now I see what you mean
  35. in src/addrman.h:557 in 1fdbcc850a outdated 
    557     {
558-        LOCK(cs);
559-        Check();
560-        Good_(addr, nTime);
561-        Check();
562+        {
    kallewoof commented at 2:46 am on February 26, 2018:
    Why the extra {}?
    EthanHeilman commented at 5:05 pm on March 3, 2018:

    Not sure, Bitcoin used to have a second set of braces there when this pull request was originally created. They have been removed in the latest version.

    https://github.com/bitcoin/bitcoin/blob/fb26bf0ea3822638b10a783f054c280fc053a2b5/src/addrman.h#L526

  36. in src/addrman.h:576 in 1fdbcc850a outdated 
    570@@ -554,6 +571,28 @@ class CAddrMan
571         Check();
572     }
573 
574+    //! See if any to-be-evicted tried table entries have been tested and if so resolve the collisions.
575+    void ResolveCollisions(){
576+         {
    kallewoof commented at 2:47 am on February 26, 2018:
    Same question here (why double {}?)
  37. in src/addrman.h:575 in 1fdbcc850a outdated 
    570@@ -554,6 +571,28 @@ class CAddrMan
571         Check();
572     }
573 
574+    //! See if any to-be-evicted tried table entries have been tested and if so resolve the collisions.
575+    void ResolveCollisions(){
    kallewoof commented at 2:47 am on February 26, 2018:
    Nit: space after () before {. I think functions actually put the { on the next line, which you did in other cases.
    EthanHeilman commented at 5:12 pm on March 3, 2018:
    I just double checked the style guide and you are correct current style is { on a new line for methods.
  38. in src/addrman.h:585 in 1fdbcc850a outdated 
    580+            Check();
581+        }
582+    }
583+
584+    //! Randomly select an address in tried that another address is attempting to evict.
585+    CAddrInfo SelectTriedCollision(){
    kallewoof commented at 2:48 am on February 26, 2018:
    Nit on (){ (add space or newline)
  39. in src/addrman.h:585 in 1fdbcc850a outdated 
    582+    }
583+
584+    //! Randomly select an address in tried that another address is attempting to evict.
585+    CAddrInfo SelectTriedCollision(){
586+        CAddrInfo ret;
587+        {
    kallewoof commented at 2:48 am on February 26, 2018:
    Same Q on {}. The lock will go out of scope before ret, but I don’t think it will matter here as the lock will go out of scope on return regardless.
    EthanHeilman commented at 5:17 pm on March 3, 2018:
    I’m using the pattern from select. Since they essentially do the same thing they should look the same. https://github.com/bitcoin/bitcoin/blob/master/src/addrman.h#L562
  40. in src/net.cpp:1829 in 1fdbcc850a outdated 
    1823@@ -1824,11 +1824,18 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
1824             }
1825         }
1826 
1827+        addrman.ResolveCollisions();
1828+
1829+
    kallewoof commented at 2:50 am on February 26, 2018:
    Remove extra newline.
    EthanHeilman commented at 5:18 pm on March 3, 2018:
    done
  41. in src/net.cpp:1837 in 1fdbcc850a outdated 
    1834-            CAddrInfo addr = addrman.Select(fFeeler);
1835+            CAddrInfo addr = addrman.SelectTriedCollision();
1836+
1837+            // SelectTriedCollision returns an invalid address if it is empty.
1838+            if (!fFeeler || !addr.IsValid())
1839+                addr = addrman.Select(fFeeler);
    kallewoof commented at 2:50 am on February 26, 2018:
    Wrap multiline if in {}.
  42. in src/test/addrman_tests.cpp:56 in 1fdbcc850a outdated 
    51@@ -52,6 +52,13 @@ class CAddrManTest : public CAddrMan
52     {
53         CAddrMan::Delete(nId);
54     }
55+
56+    void SimConnFail(CService& addr){
    kallewoof commented at 2:50 am on February 26, 2018:
    Space or newline before {
  43. kallewoof commented at 2:51 am on February 26, 2018: member

    utACK 1fdbcc850ac5eaa5d6d51556f19bf0ff522b9ef9

    Sorry, some more comments after re-reading.

  44. in src/addrman.cpp:247 in 76179949e0 outdated 
    242+        LogPrint(BCLog::ADDRMAN, "addrman", "Collision inserting element into tried table, moving %s to m_tried_collisions=%d\n", addr.ToString(), m_tried_collisions.size());
243+        if (m_tried_collisions.size() < ADDRMAN_SET_TRIED_COLLISION_SIZE) {
244+            m_tried_collisions.insert(nId);
245+        }
246+    } else {
247     LogPrint(BCLog::ADDRMAN, "Moving %s to tried\n", addr.ToString());
    kallewoof commented at 9:56 pm on March 4, 2018:
    Indent LogPrint line? If you get a lot of indentation-only changes you can suggest people add ?w=1 to the diff page to ignore whitespace only changes.
  45. in src/test/addrman_tests.cpp:57 in 76179949e0 outdated 
    51@@ -52,6 +52,14 @@ class CAddrManTest : public CAddrMan
52     {
53         CAddrMan::Delete(nId);
54     }
55+
56+    void SimConnFail(CService& addr)
    kallewoof commented at 9:59 pm on March 4, 2018:
    It’s not clear to me what this function’s name is an abbreviation of. “Simulate connection failure?” Is it for testing/debugging?
    EthanHeilman commented at 5:27 pm on March 5, 2018:
    Added comment
    EthanHeilman commented at 5:34 pm on March 5, 2018:
    But yes, it is for testing. The test before evict logic needs connections to fail to exercise the eviction code. This allows when we test a particular addresses to force it to fail.
  46. kallewoof commented at 10:00 pm on March 4, 2018: member
    A few more nits! Also: amend the commit message to add an empty newline after Add test-before-evict discipline to addrman, as it will screw with the auto-commit-log script for releases otherwise.
  47. fanquake added this to the "Blockers" column in a project
  48. kallewoof approved
  49. kallewoof commented at 6:46 pm on March 5, 2018: member
    utACK 8989b6f1aee08e860cda864b085e8d33749dcebe
  50. sdaftuar commented at 8:55 pm on March 5, 2018: member

    Code review ACK, though I haven’t reviewed the tests. (Also I am new to this code, but I tried to come up with all the ways I thought this could be broken, and failed to come up with anything.)

    EDIT: travis error is some sort of whitespace issue:

    0This diff appears to have added new lines with trailing whitespace.
1The following changes were suspected:
2diff --git a/src/addrman.cpp b/src/addrman.cpp
3@@ -523,0 +536,74 @@ int CAddrMan::RandomInt(int nMax){
4+
5^---- failure generated from contrib/devtools/lint-whitespace.sh
6The command "if [ "$CHECK_DOC" = 1 -a "$TRAVIS_EVENT_TYPE" = "pull_request" ]; then contrib/devtools/lint-all.sh; fi" failed and exited with 1 during .
7Your build has been stopped.
  51. EthanHeilman commented at 9:48 pm on March 5, 2018: contributor
    @sdaftuar I’ve been trying to find the whitespace error for a while now. I can’t seem to find it. The line number identified seems to just be a carriage return. Any help?
  52. in src/addrman.cpp:565 in 8989b6f1ae outdated 
    560+                // Has successfully connected in last X hours
561+                if (GetAdjustedTime() - info_old.nLastSuccess < ADDRMAN_REPLACEMENT_HOURS*(60*60)) {
562+                    erase_collision = true;
563+                } else if (GetAdjustedTime() - info_old.nLastTry < ADDRMAN_REPLACEMENT_HOURS*(60*60)) { // attempted to connect and failed in last X hours
564+                    LogPrint(BCLog::ADDRMAN, "addrman", "Swapping %s for %s in tried table\n", info_new.ToString(), info_old.ToString());
565+
    sipa commented at 9:52 pm on March 5, 2018:
    The trailing spaces are on this line.
  53. sipa commented at 3:20 pm on March 6, 2018: member
    @EthanHeilman It seems the unit tests fail now.
  54. Add test-before-evict discipline to addrman 
    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.
    e68172ed9f
  55. in src/test/addrman_tests.cpp:62 in 06464318cf outdated 
    57+    void SimConnFail(CService& addr)
58+    {
59+         int64_t time = 1;
60+         Good_(addr, true, time); // Set last good connection in the deep past.
61+         bool count_failure = false;
62+         Attempt(addr, count_failure);
    sdaftuar commented at 4:04 pm on March 6, 2018:

    Changing this to: Attempt(addr, count_failure, GetAdjustedTime() - 90); fixes the unit test failure.

    nit: the indentation is off on the line right below

  56. sdaftuar commented at 6:50 pm on March 6, 2018: member
    I’ve reviewed the tests and the last change; utACK e68172ed9fd0e35d4e848142e9b36ffcc41de254
  57. sipa commented at 8:07 pm on March 6, 2018: member
    utACK e68172ed9fd0e35d4e848142e9b36ffcc41de254
  58. kallewoof commented at 8:28 pm on March 6, 2018: member
    utACK e68172e
  59. in src/addrman.cpp:242 in e68172ed9f 
    238+    int tried_bucket = info.GetTriedBucket(nKey);
239+    int tried_bucket_pos = info.GetBucketPosition(nKey, false, tried_bucket);
240+
241+    // Will moving this address into tried evict another entry?
242+    if (test_before_evict && (vvTried[tried_bucket][tried_bucket_pos] != -1)) {
243+        LogPrint(BCLog::ADDRMAN, "addrman", "Collision inserting element into tried table, moving %s to m_tried_collisions=%d\n", addr.ToString(), m_tried_collisions.size());
    theuni commented at 8:32 pm on March 6, 2018:
    I’m assuming this was a rebase issue, but the “addrman” param is unnecessary here. It’s actually not clear to me what this would do.
    sipa commented at 8:33 pm on March 6, 2018:
    I think it will log “addrman” and nothing more.
    laanwj commented at 8:43 pm on March 6, 2018:
    Whoops, sorry @theuni. I think it will log “Error not enough conversion specifiers while formatting log message: addrman”
    laanwj commented at 9:02 pm on March 6, 2018:
    See #12622
  60. theuni commented at 8:33 pm on March 6, 2018: member
    utACK modulo the log comment.
  61. laanwj merged this on Mar 6, 2018
  62. laanwj closed this on Mar 6, 2018
  63. laanwj referenced this in commit a36834f10b on Mar 6, 2018
  64. laanwj referenced this in commit 3f5e116609 on Mar 6, 2018
  65. laanwj referenced this in commit b4db76c550 on Mar 6, 2018
  66. laanwj referenced this in commit e1d6e2af6d on Mar 6, 2018
  67. fanquake removed this from the "Blockers" column in a project
  68. sipsorcery referenced this in commit f6c6e65b35 on Mar 7, 2018
  69. sipsorcery referenced this in commit fbfd1f0b24 on Mar 7, 2018
  70. sickpig referenced this in commit 80bb2c96a5 on Apr 27, 2018
  71. sickpig referenced this in commit c284916d03 on Apr 27, 2018
  72. sickpig referenced this in commit 562fe5d417 on Apr 27, 2018
  73. HashUnlimited referenced this in commit 95e636c735 on Jul 31, 2018
  74. lionello referenced this in commit 67e4b519bb on Nov 7, 2018
  75. PastaPastaPasta referenced this in commit 84620651c8 on Jun 10, 2020
  76. PastaPastaPasta referenced this in commit 903d29cbe3 on Jun 10, 2020
  77. PastaPastaPasta referenced this in commit 8e50809b48 on Jun 13, 2020
  78. PastaPastaPasta referenced this in commit 231301b009 on Jun 13, 2020
  79. PastaPastaPasta referenced this in commit 79b2ce9b86 on Jun 13, 2020
  80. PastaPastaPasta referenced this in commit b13c43b89b on Jun 13, 2020
  81. PastaPastaPasta referenced this in commit 8b9012d292 on Jun 13, 2020
  82. PastaPastaPasta referenced this in commit a2a37735d9 on Jun 13, 2020
  83. PastaPastaPasta referenced this in commit f442923aee on Jun 17, 2020
  84. PastaPastaPasta referenced this in commit c94e51f0a5 on Jun 17, 2020
  85. random-zebra referenced this in commit 777638e7bc on Aug 27, 2020
  86. random-zebra referenced this in commit cbd9271afb on Sep 7, 2020
  87. gades referenced this in commit 7ad9414b05 on Jun 25, 2021
  88. gades referenced this in commit 2cdf49674c on Jul 1, 2021
  89. jnewbery referenced this in commit e6caaea0ea on Jul 20, 2021
  90. jnewbery referenced this in commit f036dfbb69 on Jul 20, 2021
  91. fanquake referenced this in commit 0fffd6c4fb on Jul 21, 2021
  92. josibake referenced this in commit a2ba966358 on Jul 21, 2021
  93. sidhujag referenced this in commit 7baf6a896b on Jul 23, 2021
  94. rebroad referenced this in commit b3a66956e9 on Jul 29, 2021
  95. MarcoFalke locked this on Sep 8, 2021


EthanHeilman kazcw sipa theuni kallewoof sdaftuar laanwj

Labels
P2P

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-19 03:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me