p2p: supplying and using asmap to improve IP bucketing in addrman #16702

pull naumenkogs wants to merge 4 commits into bitcoin:master from naumenkogs:asn_buckets changing 15 files +631 −93
  1. naumenkogs commented at 7:24 pm on August 23, 2019: member

    This PR attempts to solve the problem explained in #16599. A particular attack which encouraged us to work on this issue is explained here [Erebus Attack against Bitcoin Peer-to-Peer Network] (by @muoitranduc)

    Instead of relying on /16 prefix to diversify the connections every node creates, we would instead rely on the (ip -> ASN) mapping, if this mapping is provided.

    A .map file can be created by every user independently based on a router dump, or provided along with the Bitcoin release. Currently we use the python scripts written by @sipa to create a .map file, which is no larger than 2MB (awesome!).

    Here I suggest adding a field to peers.dat which would represent a hash of asmap file used while serializing addrman (or 0 for /16 prefix legacy approach). In this case, every time the file is updated (or grouping method changed), all buckets will be re-computed. I believe that alternative selective re-bucketing for only updated ranges would require substantial changes.

    TODO:

    • more unit tests
    • find a way to test the code without including >1 MB mapping file in the repo.
    • find a way to check that mapping file is not corrupted (checksum?)
    • comments and separate tests for asmap.cpp
    • make python code for .map generation public
    • figure out asmap distribution (?)

    ~Interesting corner case: I’m using std::hash to compute a fingerprint of asmap, and std::hash returns size_t. I guess if a user updates the OS to 64-bit, then the hash of asap will change? Does it even matter?~

  2. fanquake added the label P2P on Aug 23, 2019
  3. in src/addrman.h:344 in c8fdb2fe3e outdated 
    340+        s << ((unsigned char)36);
341         s << nKey;
342+        unsigned long long asmap_version = 0;
343+        if (m_asmap.size() != 0) {
344+            std::hash<std::vector<bool>> hasher;
345+            asmap_version = hasher(m_asmap);
    MarcoFalke commented at 7:40 pm on August 23, 2019:

    nit: You might want to make it the same length in bits an all architectures:

    0            const uint64_t asmap_version{hasher(m_asmap)};
1            s << asmap_version;
    sipa commented at 0:08 am on August 24, 2019:
    I don’t think using std::hash is the right choice here, as it has no well-defined semantics (even updating your c++ stdlib could change the function). You can use SerializeHash(m_asmap) instead.
  4. in src/test/addrman_tests.cpp:9 in c8fdb2fe3e outdated 
    4@@ -5,6 +5,8 @@
5 #include <test/setup_common.h>
6 #include <string>
7 #include <boost/test/unit_test.hpp>
8+#include <util/asmap.h>
9+#include <test/data/asmap.json.h>
    MarcoFalke commented at 7:42 pm on August 23, 2019:

    This doesn’t look like json. Also, this file is 21 MB, a bit too much for the git repo.

    You might be better off adding a makefile rule %.bin.h: %.bin (similar to %.json.h: %.json).

    Sjors commented at 7:56 pm on August 23, 2019:
    For a big file that needs processing, maybe add a contrib script that fetches it (checking the sha256 hash) and does its (determinstic) thing. And then just add the result to the repo.
    naumenkogs commented at 8:21 pm on August 23, 2019:
    Right, I’ll do both things.
  5. DrahtBot commented at 10:21 pm on August 23, 2019: member

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #17812 (config, test: asmap functional tests and feature refinements by jonatack)
    • #17809 (rpc: Auto-format RPCResult by MarcoFalke)
    • #17804 (doc: Misc RPC help fixes by MarcoFalke)
    • #17428 (p2p: Try to preserve outbound block-relay-only connections during restart by hebasto)
    • #17399 (validation: Templatize ValidationState instead of subclassing by jkczyz)
    • #17383 (Refactor: Move consts to their correct translation units by jnewbery)
    • #16748 ([WIP] Add support for addrv2 (BIP155) by dongcarl)
    • #16673 (Relog configuration args on debug.log rotation by LarryRuane)
    • #16115 (On bitcoind startup, write config args to debug.log by LarryRuane)
    • #10102 ([experimental] Multiprocess bitcoin by ryanofsky)

    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.

  6. in src/util/asmap.cpp:67 in c8fdb2fe3e outdated 
    62+        result++;
63+    }
64+    return result;
65+}
66+
67+int Interpret(std::vector<bool> asmap, unsigned long long num, int bits)
    sipa commented at 11:15 pm on August 23, 2019:
    Don’t pass the map by value; it’s copying the entire megabyte bitmap on every call.
  7. in src/util/asmap.cpp:8 in c8fdb2fe3e outdated 
    0@@ -0,0 +1,103 @@
1+// Copyright (c) 2019 The Bitcoin Core developers
2+// Distributed under the MIT software license, see the accompanying
3+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
5+#include <vector>
6+#include <assert.h>
7+
8+int DecodeBits(std::vector<bool> stream, int* bitpos, int minval, const int bit_sizes[], int bit_sizes_n)
    sipa commented at 11:16 pm on August 23, 2019:
    Don’t pass the entire map by value.
    sipa commented at 11:19 pm on August 23, 2019:
    A more C++ish way to do this would be to pass a std::vector<bool>::const_iterator& it (for the beginning of the value, updated to be after the value afterwards) and a const std::vector<bool>::const_iterator& end (for the end of the area).
  8. in src/util/asmap.h:5 in c8fdb2fe3e outdated 
    0@@ -0,0 +1,5 @@
1+// Copyright (c) 2019 The Bitcoin Core developers
2+// Distributed under the MIT software license, see the accompanying
3+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
5+int Interpret(std::vector<bool> asmap, unsigned long long num, int bits);
    sipa commented at 11:18 pm on August 23, 2019:
    An unsigned long long is generally not enough to represent an IPv6 address. Maybe you want to pass the IP address also as an std::vector<bool>, and have the caller convert the IP address to a bitmap?
  9. sipa commented at 0:15 am on August 24, 2019: member
    FWIW, the asmap generated from https://dev.maxmind.com/geoip/geoip2/geolite2/ is 988387 bytes in size.
  10. sipa commented at 0:22 am on August 24, 2019: member
    Encoding the map as bool[] array in the source code will add to the executable 4 bytes per bit in the map, that’s a bit excessive. You should probably encode it as a encode it as a uint8_t[] instead (with 8 bits per array element).
  11. wiz commented at 5:39 am on August 24, 2019: contributor
    @sipa Would you mind providing the python script used to generate the asmap? I think Bitcoiners like @TheBlueMatt and myself who maintain their own BGP full table view of the Internet would like to generate their own asmap instead of trusting maxmind, or at least verify it.
  12. practicalswift commented at 5:42 am on August 24, 2019: contributor
    Concept ACK
  13. sipa commented at 9:06 am on August 24, 2019: member
    @wiz The script is here: https://gist.github.com/sipa/b90070570597b950f29a6297772a7636 though we need more tooling to convert from common dump formats, sanity check, … I’ll publish some when they’re a bit cleaned up and usable.
  14. wiz commented at 11:27 am on August 24, 2019: contributor

    Thanks for the script, it helps a lot with my own implementation and I also wanted to point out a few potential attack vectors to circumvent the asmap protection:

    1. Private ASN ranges

    There are several ASN ranges which can be used as originating ASNs without registration or verification and could be used to circumvent the asmap protection. For example I could announce one /24 from 65000, one /24 from 65001, and so on, all behind my actual ASN, in order to launch an Erebus attack. I propose that we map any IP blocks originating from the following invalid ASNs to the next-valid ASN upstream of it:

    00                       # Reserved RFC7607                                     
123456                   # AS_TRANS RFC6793                                     
264496-64511             # Reserved for use in docs and code RFC5398            
364512-65534             # Reserved for Private Use RFC6996                     
465535                   # Reserved RFC7300                                     
565536-65551             # Reserved for use in docs and code RFC5398            
665552-131071            # Reserved                                             
74200000000-4294967294   # Reserved for Private Use RFC6996                     
84294967295              # Reserved RFC7300
    1. Multiple originating ASNs

    There are actually around 50 routing prefixes on the Internet that legitimately have multiple originating ASNs, so it’s not always a strict 1:1 mapping. For example, a valid use case could be an ISP that aggregates multiple customer /29 or /30 prefixes into a single /24 announcement (since /24 is the smallest globally routable prefix size that would be generally accepted) and my router indicates all of the multiple originating ASNs in curly braces like this:

    0141.145.0.0/19 {AS7160,43898} 
1144.21.0.0 {AS7160,AS43894}
2158.13.154.0/24 {AS367,AS1479,AS1504,AS1526,AS1541}
3160.34.0.0/20 {AS4192,AS7160}
4160.34.16.0/20 {AS7160,AS43898}

    Of course this could be used to circumvent the asmap protection as well, so for this case I also propose we instead identify the upstream aggregating ASN as the originating ASN. For my implementation I’m simply going to use a regex to strip out the curly braces aggregation to ignore it.

  15. practicalswift commented at 3:52 pm on August 24, 2019: contributor

    @wiz Very good points!

    1. Private ASN ranges

    Besides using private ASN ranges another obvious attack vector would be to make use of non-reserved but unused or unallocated AS-numbers as a faux downstream for a specific attacker controlled prefix:

    Assume that AS54321 is a non-private but unused or unallocated ASN.

    An attacker AS666 with upstream transit provider AS123 could fake having AS54321 as a downstream transit customer with the network 200.201.202.0/24.

    The global view would hence be:

    • 200.201.202.0/24: AS123 AS666 AS54321

    Traffic to 200.201.202.0/24 would end up in the hands of AS666 who get a free extra slot our ASN lottery :-) This could obviously be repeated with N additional prefixes to get N extra slots.

    Assuming that AS54321 is not in active use by any organisation no one will complain about this behaviour. (In contrast to “prefix hijacking” where the hijacked network will notice that their traffic is routed in a strange way and complain.)

    How can we guard against this attack?

    To guard against an attacker making use of non-IANA allocated ASNs as faux specific attacker controlled prefix:

    Instead of blacklisting known reserved AS numbers, we could be stricter and apply a whitelisting approach: allow only AS number ranges that have been been allocated to the five Regional Internet Registries (AFRINIC, APNIC, ARIN, LACNIC and RIPE) by IANA. That data is available in machine readable form.

    To guard against an attacker making use of IANA allocated but RIR unallocated ASNs as faux downstream for a specific attacker controlled prefix:

    I believe all five RIR:s provide machine readable lists of the individual ASNs they have assigned to organisations.

    (Note that this data comes with the country code for the organisation that has been assigned the AS-number. That could be handy if we some time in the future would like to implement logic for avoiding country or region based netsplits. If incorporated in the asmap data we could say not only what ASN a peer belongs to but also the region (based on which RIR that handed out the ASN) and also which country the organisation announcing the peers prefix belongs to. This would be a good approximation for the actual country/region of the peer.)

    To guard against an attacker making use of IANA allocated and RIR allocated ASNs as faux downstreams for a specific attacker controlled prefix:

    This is harder to guard against.

    Perhaps we could require that a prefix-to-ASN mapping has been stable over say N months before being included in the asmap. That would make the described attack harder to carry out without being noticed.

    Another approach would be to analyse a full routing table when creating the asmap and reason about the ASNs that are in the path to the announcing origin AS. Could be tricky to distinguish between tier-1:s and attackers :-\

  16. wiz commented at 5:47 pm on August 24, 2019: contributor
    Yeah, the AS map mitigation may be flawed. How about requiring everyone to also have a few Tor peers, presumably bypassing the network partition attack?
  17. TheBlueMatt commented at 6:56 pm on August 24, 2019: member
    Indeed. ASN mappings are not a foolproof solution, but they’re better than just using /16s (after all, there are lots of unused /16s you could announce if you wanted to). Ultimately some monitoring and building up filtering lists over time as we observe malicious behavior may improve things, but, indeed, ensuring redundant connectivity is the only ultimate solution. Once #15759 lands, I’d really like to propose a default of 2 additional blocksonly Tor connections if Tor support is enabled (see-also https://twitter.com/TheBlueMatt/status/1160620919775211520, in which someone suggested their ISP was censoring Bitcoin P2P traffic, and only after setting bitcoind to Tor-only did it manage to connect).
  18. TheBlueMatt commented at 7:06 pm on August 24, 2019: member
    One thing we can play with after we build an initial table is to look at the paths, instead of looking only at the last ASN in the path. eg if, from many vantage points on the internet, a given IP block always passes from AS 1 to AS 2, we could consider it as a part of AS 1 (given it appears to only have one provider - AS 1). In order to avoid Western bias we’d need to do it across geographic regions and from many vantage points (eg maybe contact a Tier 1 and get their full routing table view, not just the selected routes), but once we get the infrastructure in place, further filtering can be played with.
  19. in src/util/asmap.cpp:8 in 229da7eb00 outdated 
    0@@ -0,0 +1,103 @@
1+// Copyright (c) 2019 The Bitcoin Core developers
2+// Distributed under the MIT software license, see the accompanying
3+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
5+#include <vector>
6+#include <assert.h>
7+
8+int DecodeBits(const std::vector<bool> &stream, int* bitpos, int minval, const int bit_sizes[], int bit_sizes_n)
    sipa commented at 9:01 pm on August 26, 2019:

    This is still a very C-like way of passing arguments. I suggest:

    • Instead of a vector reference and pointer to int position, pass a start (reference to) const_iterator, and an end const_iterator.
    • Instead of a pointer to an array of bit sizes and a length, pass a const reference to a vector of bit sizes.
    sipa commented at 9:03 pm on August 26, 2019:
    You probably want to use uint32_t as return type here and the functions calling it, as int doesn’t have a well defined range.
    sipa commented at 9:04 pm on August 26, 2019:
    Most of the functions and global constants in this file can be in an anonymous namespace (which makes them not pollute the global namespace, and allows some optimizations).
  20. in src/util/asmap.cpp:35 in 229da7eb00 outdated 
    30+    }
31+    return -1;
32+}
33+
34+const int TYPE_BIT_SIZES[] = {0, 0, 1};
35+int DecodeType(const std::vector<bool> &stream, int* bitpos)
    sipa commented at 9:01 pm on August 26, 2019:
    Similarly for these you can pass a start and end iterator to the bits in the vector.
  21. in src/util/asmap.cpp:57 in 229da7eb00 outdated 
    52+const int JUMP_BIT_SIZES[] = {5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30};
53+int DecodeJump(const std::vector<bool> &stream, int* bitpos) {
54+    return DecodeBits(stream, bitpos, 17, JUMP_BIT_SIZES, 26);
55+}
56+
57+int BitLength(int target)
    sipa commented at 9:05 pm on August 26, 2019:
    You can use src/crypto/common.h’s CountBits for this (which uses the efficient __builtin_clz if it exists).
  22. fanquake referenced this in commit efe1ee0d8d on Aug 26, 2019
  23. naumenkogs force-pushed on Aug 27, 2019
  24. in src/util/asmap.h:4 in 0921912adb outdated 
    0@@ -0,0 +1,5 @@
1+// Copyright (c) 2019 The Bitcoin Core developers
2+// Distributed under the MIT software license, see the accompanying
3+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
    MarcoFalke commented at 6:43 pm on August 27, 2019: 
    0src/util/asmap.h seems to be missing the expected include guard:
1  #ifndef BITCOIN_UTIL_ASMAP_H
2  #define BITCOIN_UTIL_ASMAP_H
3  ...
4  #endif // BITCOIN_UTIL_ASMAP_H
    naumenkogs commented at 7:01 pm on August 27, 2019:
    Will do. What is it actually used for?
    MarcoFalke commented at 7:19 pm on August 27, 2019:
    I think compilers will spit out warnings if symbols are declared more than once. This will happen with this header, if it is (indirectly) included twice.
    sipa commented at 3:08 pm on August 28, 2019:
    See https://en.wikipedia.org/wiki/Include_guard
  25. in src/init.cpp:438 in d688a65773 outdated 
    434@@ -429,6 +435,7 @@ void SetupServerArgs()
435     gArgs.AddArg("-peertimeout=<n>", strprintf("Specify p2p connection timeout in seconds. This option determines the amount of time a peer may be inactive before the connection to it is dropped. (minimum: 1, default: %d)", DEFAULT_PEER_CONNECT_TIMEOUT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CONNECTION);
436     gArgs.AddArg("-torcontrol=<ip>:<port>", strprintf("Tor control port to use if onion listening enabled (default: %s)", DEFAULT_TOR_CONTROL), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
437     gArgs.AddArg("-torpassword=<pass>", "Tor control port password (default: empty)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
438+    gArgs.AddArg("-asmap=<file>", "Specify asn mapping, relative to the -datadir path (only useable from configuration file, not command line)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
    Sjors commented at 8:52 am on August 28, 2019:
    Why isn’t it usable from the command line?
    naumenkogs commented at 2:05 pm on August 28, 2019:
    no real reason, will allow command line
  26. in src/addrman.h:338 in d688a65773 outdated 
    336@@ -304,8 +337,13 @@ class CAddrMan
337 
338         unsigned char nVersion = 1;
    Sjors commented at 9:15 am on August 28, 2019:
    Bump nVersion if -asmap is set?
    naumenkogs commented at 3:42 pm on August 28, 2019:

    I hesitated to do this because I don’t have a good sense of when the version number should be used for in this context. In my implementation nKeySize tells you whether hash is present or not. Perhaps you’re right and nVersion is a better way to track whether asmap is present.

    Opinions from other reviewers?

    Sjors commented at 3:46 pm on August 28, 2019:
    I would say nVersion says something about backwards compatibility, but at first glance it doesn’t look like the current version rejects a higher nVersion. In any case I prefer directly checking the version over implicit checks that will confuse future devs.
    naumenkogs commented at 4:34 pm on August 28, 2019:
    If you feed addrman serialized by my current implementation to the pre-PR code, keySize != 32 will fail.
  27. Sjors commented at 9:17 am on August 28, 2019: member

    Concept ACK. It makes sense that this is opt-in for now, and I like that it can rebucket if the map changes, so it doesn’t have to be perfect.

    Can you get rid of the merge (and oops) commit? A rebase is much easier to review commit-by-commit.

    I’d like to try this out, but I need something to feed @sipa’s script with (and then pass that into -asmap=<file>). Is there an easy incantation to convert the geolite IPv4/IPv6 ASN tables? Or to produce it myself from a VPS?

    Do we need to reconsider the number of buckets and their size?

  28. naumenkogs force-pushed on Aug 28, 2019
  29. in src/netaddress.cpp:481 in 7450dab576 outdated 
    476+        nClass = NET_IPV6;
477+        std::vector<bool> ip_bits(128);
478+        for (int8_t byte_i = 0; byte_i < 16; ++byte_i) {
479+            uint8_t cur_byte = GetByte(byte_i);
480+            for (uint8_t bit_i = 0; bit_i < 8; ++bit_i) {
481+                ip_bits[byte_i * 8 + bit_i] = (cur_byte >> bit_i) & 1;
    sipa commented at 6:12 pm on August 28, 2019:
    I think you’re swapping bit order here. The high bits from the IP’s bytes should go first.
  30. naumenkogs force-pushed on Aug 30, 2019
  31. naumenkogs force-pushed on Sep 4, 2019
  32. DrahtBot added the label Needs rebase on Sep 7, 2019
  33. luke-jr commented at 9:13 pm on September 20, 2019: member

    Do we need to know the actual ASNs, or isn’t it enough to just differentiate between distinct ASNs without necessarily knowing their numbers?

    Re Tor: Tor is centralised, so does it actually protect from partitioning at all?

  34. naumenkogs commented at 12:59 pm on September 21, 2019: member

    @luke-jr there is no particular requirement in knowing the actual AS numbers, at least in a simplified . Do you see a way to optimize it by not using ASNs?

    Also, I suggest to not have Tor discussion here. It seems there are couple more relevant PRs now.

  35. luke-jr commented at 1:38 pm on September 21, 2019: member
    I haven’t looked at the actual implementation, but it seems obvious that if we don’t need to know the AS numbers, then not using them can make the asmap file even smaller (unless it already is omitting them).
  36. luke-jr commented at 1:39 pm on September 21, 2019: member
    (But if we tie the address database to known-AS asmap data, it may become more difficult to remove the specific AS numbers later…)
  37. laanwj added the label Feature on Sep 30, 2019
  38. laanwj commented at 10:11 am on November 1, 2019: member
    Concept ACK (adding 0.20 milestone optimistically)
  39. laanwj added this to the milestone 0.20.0 on Nov 1, 2019
  40. naumenkogs force-pushed on Nov 25, 2019
  41. naumenkogs commented at 10:02 pm on November 25, 2019: member
    Rebased and cleaned up commits. Should be ready for review. I would also suggest taking a look at the related issue, which has more high-level discussion on the consequences of this change.
  42. DrahtBot removed the label Needs rebase on Nov 25, 2019
  43. jonatack commented at 5:40 pm on November 28, 2019: member
    @naumenkogs Nice – good timing. I’d like to host this PR for the PR review club session on Wednesday, December 11th. Would you be available to join in/co-host/participate?
  44. jonatack commented at 9:29 am on November 29, 2019: member
    Review meeting on this PR is up: https://bitcoincore.reviews/16702.html… could @fanquake or someone add a review club label here?
  45. laanwj added the label Review club on Nov 29, 2019
  46. laanwj added this to the "Blockers" column in a project
  47. naumenkogs commented at 9:19 pm on December 5, 2019: member

    @Sjors

    Do we need to reconsider the number of buckets and their size?

    This is a sensitive topic: Ethan’s Eclipse paper suggests more buckets, Erebus suggests less buckets… So I would keep this discussion out of the PR.

  48. in src/addrman.h:290 in 4128c9c812 outdated 
    285+    // This is done in response to Erebus attack, but also to generally
286+    // diversify the connections every node creates,
287+    // especially useful when a large fraction of nodes
288+    // operate under a couple of cloud providers.
289+    //
290+    // If a new asmap was provided, the exsiting records
    jonatack commented at 10:10 am on December 11, 2019:
    nit: s/exsiting/existing/
  49. in src/addrman.h:294 in 4128c9c812 outdated 
    289+    //
290+    // If a new asmap was provided, the exsiting records
291+    // would be re-bucketed accordingly.
292+    std::vector<bool> m_asmap;
293+
294+    static std::vector<bool> DecodeAsmap(fs::path path)
    laanwj commented at 2:02 pm on December 11, 2019:
    would generally prefer for the implementation of this function to in the cpp instead of the header, is there a specific reason to put it here?
  50. laanwj commented at 2:06 pm on December 11, 2019: member

    meta: if you want to be credited correctly, please change your gitconfig user.name to something other than “User” and git commit --amend --reset-author your commits:

    0$ git show --pretty=full 4128c9c8124e4627cc45f6df2b7017a89296fc5a
1commit 4128c9c8124e4627cc45f6df2b7017a89296fc5a (pull/16702/head)
2Author:     User <…@…>
3Commit:     User <…@…>
  51. in src/Makefile.am:529 in 4128c9c812 outdated 
    525@@ -525,6 +526,7 @@ libbitcoin_util_a_SOURCES = \
526   util/time.cpp \
527   util/url.cpp \
528   util/validation.cpp \
529+  util/asmap.cpp \
    jonatack commented at 5:05 pm on December 11, 2019:
    Any reason this is placed last instead of in alphabetical order?
    naumenkogs commented at 6:12 pm on December 18, 2019:
    It was already messed up before me… But alright, will put my file in a proper place.
  52. in src/addrman.h:332 in 4128c9c812 outdated 
    348@@ -302,7 +349,7 @@ class CAddrMan
349     {
350         LOCK(cs);
351 
352-        unsigned char nVersion = 1;
353+        unsigned char nVersion = 2;
    jonatack commented at 5:43 pm on December 11, 2019:
    Could add code documentation here? Is this the same as line 320 above: version byte (currently 1)?
  53. in src/addrman.h:491 in 4128c9c812 outdated 
    493+
494+        for (int n = 0; n < nNew; n++) {
495+            CAddrInfo &info = mapInfo[n];
496+            int bucket = entryToBucket[n];
497+            int nUBucketPos = info.GetBucketPosition(nKey, true, bucket);
498+            // Bucketing has not changed, using existing bucket positions for the new table
    jonatack commented at 5:46 pm on December 11, 2019:
    I could be misunderstanding; should this comment be 2 lines lower if it refers to the first part of the conditional?
  54. in src/init.cpp:435 in 4128c9c812 outdated 
    431@@ -426,6 +432,7 @@ void SetupServerArgs()
432     gArgs.AddArg("-peertimeout=<n>", strprintf("Specify p2p connection timeout in seconds. This option determines the amount of time a peer may be inactive before the connection to it is dropped. (minimum: 1, default: %d)", DEFAULT_PEER_CONNECT_TIMEOUT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CONNECTION);
433     gArgs.AddArg("-torcontrol=<ip>:<port>", strprintf("Tor control port to use if onion listening enabled (default: %s)", DEFAULT_TOR_CONTROL), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
434     gArgs.AddArg("-torpassword=<pass>", "Tor control port password (default: empty)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
435+    gArgs.AddArg("-asmap=<file>", "Specify asn mapping, relative to the -datadir path", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
    jonatack commented at 5:52 pm on December 11, 2019:

    suggestion: add the default

    0ANY, OptionsCategory::CONNECTION);
1-    gArgs.AddArg("-asmap=<file>", "Specify asn mapping, relative to the -datadir path", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
2+    gArgs.AddArg("-asmap=<file>", strprintf("Specify asn mapping, relative to the -datadir path (default: %u)", DEFAULT_ASMAP_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
    jonatack commented at 12:34 pm on December 25, 2019:

    suggestion to aid in grepping the bitcoind help for “peers” or “connections”:

    0-    "Specify asn mapping..."
1+    "Specify asn mapping for connections to peers..."
  55. jonatack commented at 5:59 pm on December 11, 2019: member
    Concept ACK. Have been testing running bitcoind with the -asmap= option pointing to the location of demo.map in a git clone of https://github.com/sipa/asmap, and am starting to compare the peers chosen for outbound connections. Have only begun code review; a few minor thoughts below, feel free to ignore for now. I hope to see this make it in for 0.20 soon to have time for further testing and improvements.
  56. EthanHeilman commented at 6:52 pm on December 11, 2019: contributor

    As requested I’m expanding on my comments from PR review club.

    I can see the case for merging this but it does make addrman’s behavior more complex.

    Complexity costs to discovering and fixing bugs: Having outgoing connection logic depend on asmap makes reasoning about connection behavior harder.

    For instance consider a bug causing outgoing connections are being made to IP addresses in the same group. This would fairly easy to detect in the current logic, you would just look at the log files and see outgoing connections to the same /16. Under the new logic you would need to read and understand which IPs belonged to which ASNs and then use that logic to understand the logs. @jnewbery brought up that better logging in this PR could make reduce the impact on debugging. I agree and II would suggest adding the following log statement when determining the group of an IP: IP 1.2.3.4 from ASN 444 in group 56

    That being said, it may still be the case that if the bug exists in the group logic it may not show up in the log files.

  57. pinheadmz commented at 7:00 pm on December 11, 2019: member
    One thought I had during PR review club was that it might be interesting to see each peer’s ASN in rpc getpeerinfo where possible.
  58. in src/util/asmap.cpp:71 in 4128c9c812 outdated 
    66+{
67+    std::vector<bool>::const_iterator pos = asmap.begin();
68+    uint8_t bits = ip.size();
69+    uint8_t default_asn = 0;
70+    uint32_t opcode, jump, match, matchlen;
71+    while (1) {
    EthanHeilman commented at 7:05 pm on December 11, 2019:
    What is the argument that a corrupted asmap file can’t cause an infinite loop here? I could be reading this wrong but it seems like an op_code 1 that jumps to itself it will never exit.
    sipa commented at 7:12 pm on December 11, 2019:
    DecodeType increments pos itself. The jump is relative to the end of the jump instruction.
    EthanHeilman commented at 8:13 pm on December 11, 2019:
    That makes sense. I guess you’d need to exploit an integer overflow.
  59. in src/util/asmap.cpp:34 in 4128c9c812 outdated 
    29+                val += bit << (*bit_sizes_it - 1 - b);
30+            }
31+            return val;
32+        }
33+    }
34+    return -1;
    EthanHeilman commented at 7:16 pm on December 11, 2019:

    1. What happens when you return -1 and the type is uint32_t? Does it return (2^32)-1?

    2. It doesn’t seem like anything checks for -1 to catch this error.

    3. If the call stack was DecodeJump-->DecodeBits and DecodeBits returns -1 then the next operation would be pos += jump; where jump=-1. Does this then integer overflow pos?

    naumenkogs commented at 7:37 pm on December 18, 2019:
    I’m thinking this should probably be assert(0) — the file is corrupted @sipa ?
    naumenkogs commented at 7:49 pm on December 18, 2019:
    Your opinion in other util related issues would be also useful.
    EthanHeilman commented at 1:57 pm on December 27, 2019:
    Feel free to tag me in other util related issues, happy to give a look and add my two cents.
    laanwj commented at 12:12 pm on January 15, 2020:

    I do not think it is even possible to reach this. The

     0if (bit_sizes_it + 1 != bit_sizes.end()) {
 1    …
 2} else {
 3    bit = 0;
 4}
 5if (bit) {
 6    …
 7} else {
 8    …
 9    return val;
10}

    guarantees that the one-to-last bit size will always return a value. (so yes, replacing with assert(0) would make sense)

    EthanHeilman commented at 2:46 pm on January 15, 2020:

    assert(0) seems safer than -1.

    I agree that it is unlikely this branch will be hit. If bit_sizes has a length of zero it short circuits the for loop and would hit that line however so far all the calls to DecodeBits hardcode a parameter with values that have length > 0.

    laanwj commented at 3:03 pm on January 15, 2020:

    Or refactor the function, moving the second for loop outside of the first, so that the unreachable code is no longer there:

     0uint32_t DecodeBits(std::vector<bool>::const_iterator& bitpos, uint8_t minval, const std::vector<uint8_t> &bit_sizes)                                                        
 1{
 2    assert(bit_sizes.len() >= 1);
 3    uint32_t val = minval;
 4    bool bit;
 5    std::vector<uint8_t>::const_iterator bit_sizes_it = bit_sizes.begin();
 6    for (; (bit_sizes_it + 1) != bit_sizes.end(); ++bit_sizes_it) {
 7        bit = *bitpos;
 8        ++bitpos;
 9        if (bit) {
10            val += (1 << *bit_sizes_it);
11        } else {
12            break;
13        }
14    }
15    for (int b = 0; b < *bit_sizes_it; ++b) {
16        bit = *bitpos;
17        ++bitpos;
18        val += bit << (*bit_sizes_it - 1 - b);
19    }
20    return val;
21}
    jamesob commented at 4:38 pm on January 16, 2020:
    This code is a great candidate for unittesting - I’m inclined to say that you should use @laanwj’s code here but it’s somewhat difficult to verify that it will behave in exactly the same way sipa’s original code does. Happy to add some tests in a follow-up PR.
  60. in src/util/asmap.cpp:94 in 4128c9c812 outdated 
    89+                bits--;
90+            }
91+        } else if (opcode == 3) {
92+            default_asn = DecodeASN(pos);
93+        } else {
94+            assert(0);
    EthanHeilman commented at 7:18 pm on December 11, 2019:
    It might make sense to log how we ended up here since it seems like something real bad must’ve happened. Is there any way to recover or is this case so bad that should the Bitcoin node should immediately stop?
    jamesob commented at 9:57 pm on January 15, 2020:
    I’m sort of inclined to say we should log really loudly and then fall back to IP-based bucketing since crashing based on asmap failure seems pretty drastic - especially since this is called not just at startup but throughout runtime. If we crash, a problem with this code could then potentially cause widespread outage.
  61. in src/util/asmap.cpp:74 in 4128c9c812 outdated 
    69+    uint8_t default_asn = 0;
70+    uint32_t opcode, jump, match, matchlen;
71+    while (1) {
72+        assert(pos != asmap.end());
73+        opcode = DecodeType(pos);
74+        if (opcode == 0) {
    EthanHeilman commented at 7:19 pm on December 11, 2019:
    I would suggest using an enum or some constant to help document the intent of these values
    jamesob commented at 9:45 pm on January 15, 2020:
    Agree with @EthanHeilman. I know we’re trying to avoid nits but this seems like basic hygiene.
  62. in src/netaddress.cpp:490 in 4128c9c812 outdated 
    485+        }
486+
487+        uint32_t asn = Interpret(asmap, ip_bits);
488+        vchRet.push_back(nClass);
489+        for (int i = 0; i < 4; i++) {
490+            vchRet.push_back(asn >> (8 * i) & 0xFF);
    EthanHeilman commented at 7:27 pm on December 11, 2019:
    I’d add parens here to make the order of operations more explicit to a reader (asn >> (8 * i)) & 0xFF
  63. in src/init.cpp:1822 in 4128c9c812 outdated 
    1813@@ -1807,6 +1814,21 @@ bool AppInitMain(NodeContext& node)
1814         return false;
1815     }
1816 
1817+    // set asmap if supplied
1818+    if (gArgs.IsArgSet("-asmap")) {
1819+        const fs::path asmap_path = GetDataDir() / gArgs.GetArg("-asmap", DEFAULT_ASMAP_FILENAME);
1820+        std::vector<bool> asmap = CAddrMan::DecodeAsmap(asmap_path);
1821+        if (asmap.size() == 0) {
1822+            InitError(strprintf(_("Could not parse specified asmap: '%s'").translated, asmap_path));
    jonatack commented at 9:05 pm on December 11, 2019:

    Further to #16702 (review), would suggest here "Could not find or parse specified asmap: '%s'".

    Rationale: the most frequent cause is likely to be an incorrect path or filename, not an unparseable file, and this error message will be output in both cases.

  64. adamjonas commented at 9:13 pm on December 11, 2019: member
    4b6543941 isn’t compiling for me. I’m getting two errors in test/addrman_tests.cpp based on changes to GetTriedBucket and GetNewBucket.
  65. in src/init.cpp:1819 in 4128c9c812 outdated 
    1813@@ -1807,6 +1814,21 @@ bool AppInitMain(NodeContext& node)
1814         return false;
1815     }
1816 
1817+    // set asmap if supplied
1818+    if (gArgs.IsArgSet("-asmap")) {
1819+        const fs::path asmap_path = GetDataDir() / gArgs.GetArg("-asmap", DEFAULT_ASMAP_FILENAME);
    jonatack commented at 9:20 pm on December 11, 2019:

    This appears to be a bug. Passing -asmap with no file specified returns: Error: Could not parse specified asmap: '"/home/jon/.bitcoin"'

    It seems it should error by looking for the default file defined in line 104 above: Error: Could not parse specified asmap: '"/home/jon/.bitcoin/ip_asn.map"'

    jonatack commented at 11:10 pm on December 11, 2019:

    I think what you want here is:

    0-        const fs::path asmap_path = GetDataDir() / gArgs.GetArg("-asmap", DEFAULT_ASMAP_FILENAME);
1+        std::string asmap_file = gArgs.GetArg("-asmap", "");
2+        if (asmap_file.empty()) {
3+            asmap_file = DEFAULT_ASMAP_FILENAME;
4+        }
5+        const fs::path asmap_path = GetDataDir() / asmap_file;

    This provides the behavior of defaulting to DEFAULT_ASMAP_FILENAME if the user passes -asmap without a filename.

    naumenkogs commented at 11:14 pm on December 11, 2019:
    Weird. You’re handling a special case when somebody passes -asmap="". What if somebody passes -asmap=" " or asmap="/"? These are conditions, which are equally possible in users stupidity. If a user don’t want asmap, they ignore the flag. I don’t think protecting against a specific case of empty string is reasonable. Other opinions?
    jonatack commented at 1:20 pm on December 12, 2019:

    If I understand the intended interface in this PR:

    1. bitcoind: use /16 bucketing
    2. bitcoind -asmap: use default asmap file in the datadir
    3. bitcoind -asmap=<path-to-file>: use specifed asmap

    My suggestion handles case 2.

    I could be confused, but I don’t see in what case DEFAULT_ASMAP_FILENAME would be used otherwise. AFAICT, behind the if (gArgs.IsArgSet("-asmap")) conditional, DEFAULT_ASMAP_FILENAME in gArgs.GetArg("-asmap", DEFAULT_ASMAP_FILENAME) will never be called.

    fjahr commented at 10:04 pm on December 15, 2019:

    Edit: tested with different scenarios and @jonatack s code works as described. It loosely follows the same pattern as -prune which uses similar behavior of the three cases, so I would say it is consistent as well.

    With current code starting bitcoind -asmap (with no default file present):

    0  2019-12-15T21:40:36Z Opened asmap file (864 bytes) from disk.
1  2019-12-15T21:40:36Z
2
3  ************************
4  EXCEPTION: NSt3__18ios_base7failureE
5  CAutoFile::read: fread failed: unspecified iostream_category error bitcoin in AppInit()
6
7  ************************
8  EXCEPTION: NSt3__18ios_base7failureE
9  CAutoFile::read: fread failed: unspecified iostream_category error bitcoin in AppInit()

    With change:

    02019-12-15T21:59:13Z Failed to open asmap file from disk.
12019-12-15T21:59:13Z Error: Could not parse specified asmap: '"/path/to/testnet3/ip_asn.map"'
2Error: Could not parse specified asmap: '"/path/to/testnet3/ip_asn.map"'
    jonatack commented at 10:42 pm on December 15, 2019:
    @fjahr that version would ignore any user-specified file, e.g. case 3.
    fjahr commented at 10:43 pm on December 15, 2019:
    @jonatack yeah, I am just revising my answer as I am doing more testing, commented prematurely
    naumenkogs commented at 7:06 pm on December 18, 2019:
    I didn’t think about -asmap call without a parameter as “use existing”. But yeah, perhaps it makes sense. Especially since both of you share that intuition. Will apply your suggested change.
  66. in src/addrman.h:305 in 4128c9c812 outdated 
    300+            LogPrintf("Failed to open asmap file from disk.\n");
301+            return bits;
302+        }
303+        fseek(filestr, 0, SEEK_END);
304+        int length = ftell(filestr);
305+        LogPrintf("Opened asmap file (%d bytes) from disk.\n", length);
    jonatack commented at 11:17 pm on December 11, 2019:

    Suggest to provide feedback on which filename was opened:

    0-        LogPrintf("Opened asmap file (%d bytes) from disk.\n", length);
1+        LogPrintf("Opened asmap file %s (%d bytes) from disk.\n", path, length);

    Tested examples of output:

    0Opened asmap file "/home/jon/.bitcoin/ip_asn.map" (932999 bytes) from disk.
1Opened asmap file "/home/jon/.bitcoin/../projects/bitcoin/asmap/demo.map" (932999 bytes) from disk.
  67. fjahr commented at 11:58 pm on December 15, 2019: member

    Concept ACK

    Also built, ran automated tests, did some manual testing, light code review on 4b65439415a17bcb6f99bb1996c5ce01c70d08b6.

    I have two thoughts:

    • Should we include an implementation of the encoding code here as well, to make sure that anyone who wants to use it has a reference implementation that is reviewed properly? Otherwise, I think there should be at least some more documentation.
    • Should there be some detection on possibly malicious asmap files? I.e. would it be possible that the user ends up with a file that maps all IPs to one ASN and thus ends up with only one bucket, making it easier to eclipse attack? Should we warn about that? Maybe we should fall back to /16 below a certain number?
  68. naumenkogs force-pushed on Dec 18, 2019
  69. Add asmap utility which queries a mapping 
    The scripts for creating a compact IP->ASN mapping are here:
https://github.com/sipa/asmap

Co-authored-by: Pieter Wuille <pieter.wuille@gmail.com>
    8feb4e4b66
  70. naumenkogs force-pushed on Dec 18, 2019
  71. naumenkogs force-pushed on Dec 18, 2019
  72. naumenkogs force-pushed on Dec 24, 2019
  73. naumenkogs commented at 6:30 pm on December 24, 2019: member

    @pinheadmz added a critical_as field to a getpeerinfo RPC call response. I had to do some refactoring for this.

    I couldn’t find a good way to test it with functional or unit tests really, because it’s hard to avoid triggering GetNetClass() == NET_UNROUTABLE/NET_LOCAL. And triggering this prevents returning an asmap. I’d be glad if someone suggests a good way to test it. I tested this behavior by running a node.

  74. in src/init.cpp:1823 in ca7916a3c3 outdated 
    1818+    if (gArgs.IsArgSet("-asmap")) {
1819+        std::string asmap_file = gArgs.GetArg("-asmap", "");
1820+        if (asmap_file.empty()) {
1821+            asmap_file = DEFAULT_ASMAP_FILENAME;
1822+        }
1823+        const fs::path asmap_path = GetDataDir() / asmap_file;
    pinheadmz commented at 8:02 pm on December 24, 2019:

    This implies that the asmap file will always be in the default data directory. Should it allow for absolute paths as well? For example, I ended up with this error on my first naive run of this branch:

    0$ bitcoind -testnet -asmap=/Users/matthewzipkin/Desktop/work/asmap/demo.map
1...
2Error: Could not find or parse specified asmap:
3'"/Users/matthewzipkin/Library/Application Support/Bitcoin/testnet3/Users/matthewzipkin/Desktop/work/asmap/demo.map"'
    naumenkogs commented at 8:27 pm on December 24, 2019:
    At this point I’m not even sure what should be the right behaviour here. summoning @jonatack.
    jonatack commented at 11:11 am on December 25, 2019:

    @pinheadmz @naumenkogs Yes, the PR as authored specifies a relative path and I tested it as such:

    0gArgs.AddArg("-asmap=<file>", "Specify asn mapping, relative to the -datadir path", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);

    I agree allowing for absolute paths would be handy. Will perhaps propose a pair of commits to do this and add tests for expected behavior as per #16702 (review) for relative and absolute paths, that can be pulled in or proposed separately, per @naumenkogs’ preference, so he doesn’t have to bother with it.

    jonatack commented at 12:19 pm on December 30, 2019:
    done in #17812
    jamesob commented at 9:41 pm on January 14, 2020:
    Agree this should accept an absolute path. We should either pull @jonatack’s change in to this PR or follow up shortly after this merges.
  75. pinheadmz commented at 8:13 pm on December 24, 2019: member

    @naumenkogs very cool seeing the ASN number in the RPC call. Tested on testnet at ca7916a3c3024c7af2b3df1069ab1a4f9cee2ca6 with the demo.map file from https://github.com/sipa/asmap

    Was able to confirm the returned data against whois and https://bgpview.io/asn/14061#prefixes-v4:

    RPC output:

    0    "addr": "188.166.99.121:18333",
1    "critical_as": 14061,

    WHOIS:

    0inetnum:        188.166.0.0 - 188.166.127.255
1netname:        EU-DIGITALOCEAN-NL1
  76. practicalswift commented at 1:10 am on December 25, 2019: contributor
    What is a “critical AS”? :)
  77. naumenkogs commented at 1:14 am on December 25, 2019: member

    @practicalswift see “Rationale” here. Basically, it doesn’t have to be the actual AS of the node, but it better be the bottleneck-AS on BGP path to that node.

    Do you think it also needs further clarification in the Bitcoin Core source code? I couldn’t generalize it better. It’s like whatever asmap tells us — except it’s should be some heuristic used for diversification.

  78. naumenkogs force-pushed on Dec 25, 2019
  79. naumenkogs force-pushed on Dec 25, 2019
  80. naumenkogs commented at 1:40 am on December 25, 2019: member

    Okay I’m less happy with the last tiny logging commit 1619fe1 2 reasons:

    • Every time GetNewBucket(...) is called, we call GetCriticalAS() twice: in GetGroupand separately just for logging. It’s obviously suboptimal, but I can’t see a clean way to do it better. The method is also very cheap.
    • Right now GetTriedBucket() is always called only when we’re assigning a bucket to a node, so logging there makes sense. If at some point in future we will be calling this method just for info — logging wouldn’t make sense. It is possible to move logging to where the method is used, but it would require copy-pasting logging 3 times. Should we leave it for the future case I explained?

    So basically I don’t have a clean way to do these 2 things better, but I’m open to suggestions :)

  81. in src/rpc/net.cpp:85 in 1619fe188b outdated 
    81@@ -82,7 +82,7 @@ static UniValue getpeerinfo(const JSONRPCRequest& request)
82             "    \"id\": n,                   (numeric) Peer index\n"
83             "    \"addr\":\"host:port\",      (string) The IP address and port of the peer\n"
84             "    \"addrbind\":\"ip:port\",    (string) Bind address of the connection to the peer\n"
85-            "    \"addrlocal\":\"ip:port\",   (string) Local address as reported by the peer\n"
    jonatack commented at 11:35 am on December 25, 2019:
    Was this line inadvertently removed? addrlocal is still present in the output, line 152 obj.pushKV("addrlocal", stats.addrLocal))
    naumenkogs commented at 1:11 pm on December 25, 2019:
    this was an accident, thanks!
  82. jonatack commented at 12:37 pm on December 25, 2019: member

    Testing a node running 1619fe188b278b82e9e147ee2de69abc4642c510 rebased onto current master with sipa/asmap/demo.map on mainnet (via a vpn but not tor).

    0$ bitcoind -asmap=../projects/bitcoin/asmap/demo.map
1...
22019-12-25T11:45:08Z Opened asmap file "/home/jon/.bitcoin/../projects/bitcoin/asmap/demo.map" (932999 bytes) from disk.
32019-12-25T11:45:09Z Using asmap version 7ead927689f884bc4021618017cc1438d9328578779edd2adfbed044fb52928e for IP bucketing.

    • getpeerinfo is returning interesting results.

    • I have not yet seen one of the new "IP %s with critical AS%i belongs to new|tried bucket %i.\n" extra logging messages.

    • Would it be helpful to include the AS number in the debug log peer here?

    0src/net_processing.cpp:2087: LogPrintf("New outbound peer connected: version: %d, blocks=%d, peer=%d%s (%s)\n",
  83. naumenkogs force-pushed on Dec 25, 2019
  84. Integrate ASN bucketing in Addrman and add tests 
    Instead of using /16 netgroups to bucket nodes in Addrman for connection
diversification, ASN, which better represents an actor in terms
of network-layer infrastructure, is used.
For testing, asmap.raw is used. It represents a minimal
asmap needed for testing purposes.
    ec45646de9
  85. naumenkogs force-pushed on Dec 25, 2019
  86. naumenkogs commented at 2:35 pm on December 25, 2019: member

    @jonatack

    I have not yet seen one of the new “IP %s with critical AS%i belongs to new|tried bucket %i.\n” extra logging messages.

    This is because you’re not specifying -debug=net.

    Would it be helpful to include the AS number in the debug log peer here?

    Eh, I don’t know. I don’t think it’s particularly useful in this context. In some cases it might even not represent the part of the actual BGP path (consider a multi-homed node). AS info is also less useful without bucket info. But even with buckets, there are much more buckets than connections, so this to me seems more like a debug info. That’s why I’m leaving it in -debug=net

  87. in src/addrman.cpp:29 in cc0a921b3e outdated 
    31+    uint64_t hash1 = (CHashWriter(SER_GETHASH, 0) << nKey << GetGroup(asmap) << vchSourceGroupKey).GetCheapHash();
32     uint64_t hash2 = (CHashWriter(SER_GETHASH, 0) << nKey << vchSourceGroupKey << (hash1 % ADDRMAN_NEW_BUCKETS_PER_SOURCE_GROUP)).GetCheapHash();
33-    return hash2 % ADDRMAN_NEW_BUCKET_COUNT;
34+    int new_bucket = hash2 % ADDRMAN_NEW_BUCKET_COUNT;
35+    uint32_t critical_as = GetCriticalAS(asmap);
36+    LogPrint(BCLog::NET, "IP %s with critical AS%i belongs to new bucket %i.\n", ToStringIP(), critical_as, new_bucket);
    jonatack commented at 4:10 pm on December 25, 2019:

    Okay I’m less happy with the last tiny logging commit 1619fe1 2 reasons:

    Agreed, refactoring this post-merge as a follow-up may be worthwhile.

    This is because you’re not specifying -debug=net.

    Thanks! I overlooked the LogPrint(BCLog::NET).

    nit: perhaps add a space between “AS” and the value in GetTriedBucket and GetNewBucket

    02019-12-25T15:48:12Z IP 87.166.109.213 with critical AS3320 belongs to new bucket 238.
12019-12-25T15:48:12Z IP 84.44.193.228 with critical AS8422 belongs to new bucket 238.
22019-12-25T15:48:12Z IP 64.225.33.255 with critical AS200 belongs to new bucket 219.
32019-12-25T15:48:12Z IP 186.52.76.207 with critical AS6057 belongs to new bucket 301.
42019-12-25T15:48:12Z IP 186.4.77.132 with critical AS27964 belongs to new bucket 361.
52019-12-25T15:48:12Z IP 109.195.227.7 with critical AS57378 belongs to new bucket 808.
  88. practicalswift commented at 10:17 pm on December 25, 2019: contributor

    @naumenkogs

    Ah, is “critical AS” for a prefix simply the non-stub AS-number closest to the originating AS-number in an AS path? (BTW, I assume “critical AS” is a made up term? I haven’t encountered it before :))

    In a single-homed/stub then the “critical AS” is typically its upstream/transit provider of the originating AS, and in a multi-homed setup the “critical AS” is typically the originating AS itself?

    Consider the following:

    010.11.12.0/24: AS382 AS492 AS520
110.11.12.0/24: AS942 AS492 AS520
220.22.24.0/24: AS882 AS482 AS720
320.22.24.0/24: AS102 AS201 AS720

    In the first example AS492 is the non-stub AS-number closest to AS520 which is the AS-number originating 10.11.12.0/24.

    In the second example the network originating 20.22.24.0/24 (AS720) is a non-stub itself.

    Which gives us “critical AS-numbers” (or “non-stub originating AS-numbers”):

    010.11.12.0/24 AS492
120.22.24.0/24 AS720

    Is my understanding correct? :)

  89. naumenkogs commented at 7:17 pm on December 26, 2019: member

    @practicalswift the exact algorithm of deciding what is critical is out of this PR — it is in the asmap construction. For the PR, what matters is that it is some AS by which we want to diversify. And yeah, I made up the term.

    Yes, your examples are correct, according to my PR to the asmap lib.

    I’m not that sure about your definitions, because consider this case:

    033.22.24.0/24: AS882 AS111 AS482 AS720
133.22.24.0/24: AS102 AS111 AS201 AS720

    In my PR it would be AS720, although one might think that the answer is AS111. This is debatable I guess. We can discuss it in that repo.

  90. practicalswift commented at 11:38 pm on December 27, 2019: contributor

    @naumenkogs Thanks for clarifying!

    I don’t want to bikeshed but it is something about the “critical” in “critical AS” that confused me at least :)

    I guess what we want to say that it is an AS number that we group by, but it is not necessarily the originating AS number for the prefix in question as one would assume (if we called it only “AS”).

    Can we find a more neutral name? Perhaps “AS group”, “effective AS”, “assumed AS” or even “faux AS”? :)

  91. jonatack commented at 12:39 pm on December 30, 2019: member

    @naumenkogs as per #16702 (review), in #17812 I added feature functional tests and absolute asmap paths, separated asmap found/parsing checks, added a test which the separation enables, plus a handful of minor suggestions found while doing this. The changes are separate to aid picking and choosing; I don’t mind squashing down a bit if needed. Feel free to pull in commits, use nit changes, or leave it for after this PR is merged.

    I also reviewed the unit tests and bucketing in more depth and have questions which I’ll post here or ask you on IRC.

  92. muoitranduc commented at 8:47 am on January 9, 2020: none
    @naumenkogs Can you elaborate more on the idea of having a critial AS for a prefix? Isn’t that this critical AS only presents the most common suffix AS on the propagated AS paths observed by RIPE collectors only? In other words, the traffic from a node (which is not at the same location with any RIPE collectors) to the prefix may not traverse the critical AS. If you want a simple prefix-to-asn mapping, why do you need the AS paths?
  93. practicalswift commented at 11:04 am on January 9, 2020: contributor

    @naumenkogs What do you think of using the term “mapped AS” instead of “critical AS”? That would be consistent with the “AS map” terminology already in use and would not imply any specific method for the generation of the AS map file.

    So instead of introducing two separate terms “AS map” and “critical AS” we only introduce “AS map”/“mapped AS”. I think that would be less confusing for newcomers :)

  94. naumenkogs force-pushed on Jan 10, 2020
  95. naumenkogs force-pushed on Jan 10, 2020
  96. naumenkogs commented at 7:30 pm on January 10, 2020: member

    @practicalswift I liked the “mapped_as” so I used it instead. “Critical” is still used in 2 places (comments basically) as a synonym, to make understanding easier. Let me know if you have concerns about that. @muoitranduc Yes, you are correct. The first element of the most common suffix basically. This is for users which don’t want to make their own asmap (probably the majority, because we cannot force everyone make their own asmap). Just a good approximation, because we will aggregate paths from at least 25 RIPE collectors (and maybe other places as well). In a lot of cases, it will map to the last hop (actual owning ASN). At the high-level, I believe looking at the common suffix is also more robust to attacks like Erebus. It’s not worse, because it’s this mapping is a super-mapping over actual-asn-mapping (the former takes the latter into account AND does more). At a high level, it not only prevents you from connecting to 8/8 Amazon nodes, but also from 8/8 nodes having Amazon as a critical part of the route. If you want to discuss this design more from the research prospective — maybe we can exchange couple emails first, and then publish the summary here shortly after?

    Meta: This PR already has 100 comments so it’s a bit difficult to navigate through everything. Let’s try to be more precise and focus on the most important issues with this PR, because as you know fixing nits might be everlasting. Thank you @jonatack for addressing some of them in a separate PR.

  97. naumenkogs force-pushed on Jan 15, 2020
  98. laanwj commented at 1:02 pm on January 15, 2020: member
    I think reading the asmap happens too late in the initialization process, unless there’s a strong reason to do it like this that I’m missing. When reading it before loading the block index and doing initial verification, there would be much faster feedback on invalid -asmap command line arguments. It’s a bit silly to spin up the entire node just to it shut down again. Could be changed in a later PR (maybe #17812) anyhow.
  99. laanwj commented at 1:09 pm on January 15, 2020: member
    code review and light testing (both IPv4 and IPv6) ACK 50f655e4d50a3b8b39fc01a4aaab3d2aeb05e6be
  100. jamesob commented at 9:12 pm on January 15, 2020: member
    For what it’s worth I think reviewing @sipa’s asmap format is prerequisite to signing off on this PR (since the data this PR is dependent on is encoded using the former). I’ve reviewed that code (and added some commentary: https://github.com/sipa/asmap/pull/2), and will now be starting on this code.
  101. in src/test/addrman_tests.cpp:18 in ec45646de9 outdated 
    13 #include <random.h>
14 
15 class CAddrManTest : public CAddrMan
16 {
17+private:
18+    bool deterministic;
    jamesob commented at 6:48 pm on January 16, 2020:
    nit: convention would say m_deterministic = ..., then you don’t have to do awkward naming to avoid shadowing below
  102. in src/test/addrman_tests.cpp:644 in ec45646de9 outdated 
    641+    // Test: IP addresses in the different /16 prefix MAY NOT map to more than
642+    // 8 buckets.
643+    BOOST_CHECK(buckets.size() == 8);
644+}
645+
646+BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket)
    jamesob commented at 9:52 pm on January 16, 2020:

    A lot of commonalities between this test case and the last. I know this is nitty stuff but you could save a lot of code (and thus review effort) by consolidating it into a function that takes a function parameter that returns info.Get{Tried,New}Bucket(...). Again not a blocker, just would be easier review without having to read through something that’s kind of the same but not.

    And these tests are basically duplicates of the existing tests with a null asmap (caddrinfo.*_legacy), so it seems like there’s a lot of code duplication that could be removed here. Might make a good follow-up PR I guess.

  103. in src/test/addrman_tests.cpp:724 in ec45646de9 outdated 
    721+    // than 1 bucket.
722+    BOOST_CHECK(buckets.size() == 1);
723+
724+}
725+
726+BOOST_AUTO_TEST_CASE(addrman_serialization)
    jamesob commented at 10:01 pm on January 16, 2020:
    Thanks for testing this!
  104. in src/test/netbase_tests.cpp:300 in ec45646de9 outdated 
    308+    BOOST_CHECK(ResolveIP("64:FF9B::102:304").GetGroup(asmap) == std::vector<unsigned char>({(unsigned char)NET_IPV4, 1, 2})); // RFC6052
309+    BOOST_CHECK(ResolveIP("2002:102:304:9999:9999:9999:9999:9999").GetGroup(asmap) == std::vector<unsigned char>({(unsigned char)NET_IPV4, 1, 2})); // RFC3964
310+    BOOST_CHECK(ResolveIP("2001:0:9999:9999:9999:9999:FEFD:FCFB").GetGroup(asmap) == std::vector<unsigned char>({(unsigned char)NET_IPV4, 1, 2})); // RFC4380
311+    BOOST_CHECK(ResolveIP("FD87:D87E:EB43:edb1:8e4:3588:e546:35ca").GetGroup(asmap) == std::vector<unsigned char>({(unsigned char)NET_ONION, 239})); // Tor
312+    BOOST_CHECK(ResolveIP("2001:470:abcd:9999:9999:9999:9999:9999").GetGroup(asmap) == std::vector<unsigned char>({(unsigned char)NET_IPV6, 32, 1, 4, 112, 175})); //he.net
313+    BOOST_CHECK(ResolveIP("2001:2001:9999:9999:9999:9999:9999:9999").GetGroup(asmap) == std::vector<unsigned char>({(unsigned char)NET_IPV6, 32, 1, 32, 1})); //IPv6
    jamesob commented at 10:26 pm on January 16, 2020:
    If you wanted better (albeit incidental) coverage of the ASN parsing stuff, you could use asmap.raw to verify that IPs are mapped properly. Can be done in a follow-up.
  105. in src/net.cpp:496 in 3d639e7872 outdated 
    492@@ -493,12 +493,13 @@ void CNode::SetAddrLocal(const CService& addrLocalIn) {
493 
494 #undef X
495 #define X(name) stats.name = name
496-void CNode::copyStats(CNodeStats &stats)
497+void CNode::copyStats(CNodeStats &stats, std::vector<bool> m_asmap)
    jamesob commented at 10:27 pm on January 16, 2020:
    Should this be taking std::vector<bool>& instead? That’s potentially a pretty large vector to be copying.
  106. jamesob commented at 10:35 pm on January 16, 2020: member

    This is looking pretty close. It seems like a great change to me conceptually. FWIW, at some point the addrman doc preamble merits updating to mention bucketing-by-ASN when available. https://github.com/bitcoin/bitcoin/blob/master/src/addrman.h#L92-L116

    The only point that I think is worth addressing before I ACK is the asmap vector copy in CNode::copyStats.

    We could use more test coverage IMO but that can be done in followups.

  107. Return mapped AS in RPC call getpeerinfo 
    If ASN bucketing is used, return a corresponding AS
used in bucketing for a given peer.
    e4658aa8ea
  108. Add extra logging of asmap use and bucketing 3c1bc40205
  109. naumenkogs force-pushed on Jan 23, 2020
  110. naumenkogs commented at 7:27 pm on January 23, 2020: member
    I think no more pending critical comments, everything unaddressed will be easier to get through via a follow-up, so let’s get this merged? Looking for acks :)
  111. laanwj commented at 8:17 pm on January 23, 2020: member
    re-ACK 3c1bc40205a3fcab606e70b0e3c13d68b2860e34 only change is std::vector<bool> &m_asmap
  112. jamesob approved
  113. jamesob commented at 8:53 pm on January 23, 2020: member

    ACK 3c1bc40205a3fcab606e70b0e3c13d68b2860e34 (jamesob/ackr/16702.3.naumenkogs.p2p_supplying_and_using)

    Only change since last ACK is changing the m_asmap param from value to reference. Built locally.

    0Tested on Linux-4.15.0-47-generic-x86_64-with-Ubuntu-18.04-bionic
1
2Configured with ./configure LDFLAGS=-L/home/james/src/bitcoin/db4/lib/ CPPFLAGS=-I/home/james/src/bitcoin/db4/include/ CXXFLAGS=-std=c++11 -Wthread-safety-analysis -Werror -Wthread-safety-analysis -Werror --enable-wallet --enable-debug --with-daemon CXX=/usr/bin/clang++-4.0 CC=/usr/bin/clang-4.0 PYTHONPATH= --disable-shared --with-pic --enable-benchmark=no --with-bignum=no --enable-module-recovery --disable-jni --disable-shared --with-pic --enable-benchmark=no --with-bignum=no --enable-module-recovery --disable-jni --no-create --no-recursion
3
4Compiled with /usr/bin/ccache /usr/bin/clang++-4.0 -std=c++11 -mavx -mavx2 -std=c++11 -Wthread-safety-analysis -Werror -Wthread-safety-analysis -Werror -O0 -g3 -ftrapv  -Wstack-protector -fstack-protector-all -msse4 -msha -msse4.1 -msse4.2  i
5
6Compiler version: clang version 4.0.1-10 (tags/RELEASE_401/final)
  114. jonatack commented at 1:03 pm on January 27, 2020: member

    I think reading the asmap happens too late in the initialization process, unless there’s a strong reason to do it like this that I’m missing. When reading it before loading the block index and doing initial verification, there would be much faster feedback on invalid -asmap command line arguments. It’s a bit silly to spin up the entire node just to it shut down again. Could be changed in a later PR (maybe #17812) anyhow.

    I can confirm that moving the asmap init.cpp code from the end of Step 12: start node to the end of the Step 6: network initialization section speeds up the feature_asmap.py tests in #17812 from 60 seconds down to 5, while they continue to function as expected… by dramatically speeding up the 2 tests that use assert_start_raises_init_error. Updating #17812.

  115. in src/rpc/net.cpp:86 in 3c1bc40205 
    82@@ -83,6 +83,7 @@ static UniValue getpeerinfo(const JSONRPCRequest& request)
83             "    \"addr\":\"host:port\",      (string) The IP address and port of the peer\n"
84             "    \"addrbind\":\"ip:port\",    (string) Bind address of the connection to the peer\n"
85             "    \"addrlocal\":\"ip:port\",   (string) Local address as reported by the peer\n"
86+            "    \"mapped_as\":\"mapped_as\", (string) The AS in the BGP route to the peer used for diversifying peer selection\n"
    jonatack commented at 1:33 pm on January 27, 2020:
    The mapped_as value is currently returned as an integer.
    jonatack commented at 1:42 pm on January 27, 2020:
    Will address this in #17812.
    laanwj commented at 12:52 pm on January 29, 2020:
    Doesn’t returning it as an integer make sense, as it is an integer? So I think we should change the documentation.
    jonatack commented at 1:05 pm on January 29, 2020:

    Doesn’t returning it as an integer make sense, as it is an integer? So I think we should change the documentation.

    Thanks! Agreed.

  116. jonatack commented at 1:41 pm on January 27, 2020: member

    ACK 3c1bc40205a3fcab606e70b0e3c13d68b2860e34

    Built, ran tests, bitcoind, and getpeerinfo and observed the mapped_as fields. Idem rebased onto current master 3253b5d:

     0$ bitcoind -asmap=../projects/bitcoin/asmap/demo.map
 12020-01-27T13:20:06Z Feeding 36895 bytes of environment data into RNG
 22020-01-27T13:20:06Z Bitcoin Core version v0.19.99.0-3c1bc40205 (debug build)
 3.../...
 42020-01-27T13:20:43Z init message: Starting network threads...
 52020-01-27T13:20:43Z Opened asmap file "/home/jon/.bitcoin/../projects/bitcoin/asmap/demo.map" (932999 bytes) from disk.
 62020-01-27T13:20:43Z msghand thread start
 72020-01-27T13:20:43Z addcon thread start
 82020-01-27T13:20:43Z net thread start
 92020-01-27T13:20:43Z opencon thread start
102020-01-27T13:20:43Z dnsseed thread start
112020-01-27T13:20:44Z Using asmap version 7ead927689f884bc4021618017cc1438d9328578779edd2adfbed044fb52928e for IP bucketing.
122020-01-27T13:20:44Z init message: Done loading

    0$ bitcoind
12020-01-27T13:34:19Z Feeding 36908 bytes of environment data into RNG
22020-01-27T13:34:19Z Bitcoin Core version v0.19.99.0-3c1bc40205 (debug build)
3.../...
42020-01-27T13:36:13Z init message: Starting network threads...
52020-01-27T13:36:13Z Using /16 prefix for IP bucketing.
62020-01-27T13:36:13Z init message: Done loading

    0$ bitcoin-cli getpeerinfo
1[
2  {
3    "id": 0,
4    "addr": "88.99.150.13:8333",
5    "addrlocal": "213.152.162.154:37866",
6    "addrbind": "10.24.132.40:37866",
7    "mapped_as": 24940,
8    "services": "000000000000040d",

    Code re-review. Agree with improving test coverage and tests, docs, running the code earlier in init.cpp, passing absolute filenames, adding the default filename to the help, and various other little things that can be done in follow-ups.

  117. luke-jr commented at 3:45 am on January 29, 2020: member

    Is the plan to actually generate the asmap files from GeoIP’s GeoLite database?

    Has anyone reviewed their EULA which presumably controls asmaps derived from their database?

  118. laanwj commented at 12:50 pm on January 29, 2020: member
    I think how the particular asmap database is generated is an important discussion if and when it’s going to be shipped with bitcoin core itself. This PR just adds a feature to load and interpret one, independent of where it comes from.
  119. laanwj referenced this in commit 01fc5891fb on Jan 29, 2020
  120. laanwj merged this on Jan 29, 2020
  121. laanwj closed this on Jan 29, 2020
  122. laanwj removed this from the "Blockers" column in a project
  123. sipa commented at 8:11 pm on January 29, 2020: member
    @luke-jr https://github.com/sipa/asmap/pull/1 adds tooling to build asmap files from RIPE’s dumps.
  124. Empact commented at 10:25 pm on January 29, 2020: member
    For the sake of documentation, this apparently introduced an appveyor test failure #18020, fixed in #18022.
  125. sipa commented at 11:15 pm on January 29, 2020: member
    Sorry for the late review. I’ve opened #18023 with things that I think can be improved here.
  126. jonatack referenced this in commit b02e05ab04 on Jan 30, 2020
  127. jonatack referenced this in commit e44e8842b4 on Jan 30, 2020
  128. jonatack referenced this in commit 5c21546fa1 on Jan 31, 2020
  129. jonatack referenced this in commit ff746334ab on Jan 31, 2020
  130. sidhujag referenced this in commit aec7334f19 on Feb 1, 2020
  131. luke-jr referenced this in commit b42b932f64 on Feb 16, 2020
  132. luke-jr referenced this in commit 050811dc51 on Feb 16, 2020
  133. luke-jr referenced this in commit 958c29e58c on Feb 16, 2020
  134. luke-jr referenced this in commit c16c55f015 on Feb 16, 2020
  135. fanquake referenced this in commit d0601e67f1 on Mar 5, 2020
  136. laanwj referenced this in commit 244e88e6b5 on Mar 25, 2020
  137. DeckerSU referenced this in commit bfaf287a3a on Jun 7, 2020
  138. DeckerSU referenced this in commit 5550e12f28 on Jun 7, 2020
  139. Mixa84 referenced this in commit 31f22e8737 on Sep 9, 2020
  140. dimxy referenced this in commit 4d68c1accc on Sep 9, 2020
  141. jasonbcox referenced this in commit 4b12d4863d on Oct 14, 2020
  142. TheComputerGenie referenced this in commit 09f3abfb68 on Oct 30, 2020
  143. TheComputerGenie referenced this in commit df8e111a61 on Oct 30, 2020
  144. deadalnix referenced this in commit 269183ab93 on Oct 30, 2020
  145. jasonbcox referenced this in commit ff7f19b6fc on Oct 30, 2020
  146. jasonbcox referenced this in commit 6a8bc82a33 on Oct 30, 2020
  147. jasonbcox referenced this in commit e98b0bdb18 on Oct 30, 2020
  148. TheComputerGenie referenced this in commit 55bf201e52 on Oct 31, 2020
  149. in src/addrman.h:471 in 3c1bc40205 
    473+
474+        for (int n = 0; n < nNew; n++) {
475+            CAddrInfo &info = mapInfo[n];
476+            int bucket = entryToBucket[n];
477+            int nUBucketPos = info.GetBucketPosition(nKey, true, bucket);
478+            if (nVersion == 2 && nUBuckets == ADDRMAN_NEW_BUCKET_COUNT && vvNew[bucket][nUBucketPos] == -1 &&
    jnewbery commented at 11:13 am on November 1, 2020:

    Why is this nVersion == 2 check required? This means that anyone upgrading to v0.20 will have a lot of their new table data lost, even if they’re not using an asmap. Without this check, we’d still make sure not to place entries in the wrong new table because of the asmap version check.

    On code readability/style, it’d make a lot more sense to extract this check to above the loop and comment it:

    0// New table positions can only be retained if the file is the right format and
1// was constructed with the same asmap.
2const bool compatible_file = nUBuckets == ADDRMAN_NEW_BUCKET_COUNT && serialized_asmap_version == supplied_asmap_version
    jnewbery commented at 3:06 pm on November 1, 2020:
    More generally, what’s the plan for updating the asmap? Presumably it’s something that could change fairly frequently (i.e. at least as often as we release major versions of Bitcoin Core). If that’s the case and the asmap is changed every major version, are we happy with the idea that people’s new tables will lose data at every major upgrade?
    naumenkogs commented at 12:31 pm on November 25, 2020:

    This means that anyone upgrading to v0.20 will have a lot of their new table data lost, even if they’re not using an asmap.

    You are right, I think this was a mistake. I was probably thinking about downgrading nVersion (say a file from 0.22 with nVersion=3 now downgraded to 0.21), so the check should be nVersion <= 2. I agree with your comment suggestion, but why you separate those two other checks? It still makes sure that “file is in the right format”.

    naumenkogs commented at 12:37 pm on November 25, 2020:

    More generally, what’s the plan for updating the asmap?

    TBD

    If that’s the case and the asmap is changed every major version, are we happy with the idea that people’s new tables will lose data at every major upgrade?

    Depends on the answer to the previous question at least :) Note that what’s lost is just the position of records in the table, and presumably a few of records from buckets with high collisions? So I wouldn’t be very worried (in the context of my previous message as well). It’s not like we’re loosing many ADDR records here.

    jnewbery commented at 2:06 pm on November 25, 2020:

    what’s lost is just the position of records in the table, and presumably a few of records from buckets with high collisions

    A new address can appear in up to 8 different buckets if it’s been gossipped to us from different peers. If it gets rebucketed during file load, it’ll only be placed in 1 bucket (or 0, if there’s a collision). That seems undesirable.

    naumenkogs commented at 10:16 am on November 26, 2020:
    You are right.
  150. jasonbcox referenced this in commit 448418120b on Nov 1, 2020
  151. sidhujag referenced this in commit 61c24989ae on Nov 10, 2020
  152. deadalnix referenced this in commit afdf7c7a75 on Jan 12, 2021
  153. kittywhiskers referenced this in commit 32b476d10d on Mar 9, 2021
  154. kittywhiskers referenced this in commit 6d0b7e8ef2 on Mar 9, 2021
  155. kittywhiskers referenced this in commit c2970001ab on Mar 9, 2021
  156. kittywhiskers referenced this in commit aad9b5cfd0 on Mar 9, 2021
  157. kittywhiskers referenced this in commit db7f93b5ac on Mar 9, 2021
  158. kittywhiskers referenced this in commit e0dfcf752f on Mar 9, 2021
  159. kittywhiskers referenced this in commit 798c4ba70b on Mar 9, 2021
  160. kittywhiskers referenced this in commit 3c64f0d7d9 on Mar 9, 2021
  161. kittywhiskers referenced this in commit f0bc731652 on Mar 9, 2021
  162. kittywhiskers referenced this in commit 6e1a7318a7 on Mar 9, 2021
  163. kittywhiskers referenced this in commit 540657a57e on Mar 9, 2021
  164. kittywhiskers referenced this in commit cdf03ffc06 on Mar 9, 2021
  165. kittywhiskers referenced this in commit 624f474391 on Mar 9, 2021
  166. kittywhiskers referenced this in commit 77f64fa420 on Mar 9, 2021
  167. kittywhiskers referenced this in commit 9a78811161 on Mar 9, 2021
  168. kittywhiskers referenced this in commit b5eeeeff45 on Mar 9, 2021
  169. kittywhiskers referenced this in commit 2abf1b2f77 on Mar 9, 2021
  170. kittywhiskers referenced this in commit 36c368099a on Mar 9, 2021
  171. kittywhiskers referenced this in commit 9a0765e765 on Mar 9, 2021
  172. kittywhiskers referenced this in commit bd191f59d1 on Mar 9, 2021
  173. kittywhiskers referenced this in commit 1f2c5dc437 on Mar 12, 2021
  174. kittywhiskers referenced this in commit 65062e2c88 on Mar 12, 2021
  175. kittywhiskers referenced this in commit 07b2199907 on Mar 12, 2021
  176. kittywhiskers referenced this in commit 426dbbda51 on Mar 12, 2021
  177. kittywhiskers referenced this in commit 28724c6f91 on Mar 12, 2021
  178. kittywhiskers referenced this in commit d4dda9f9d2 on Mar 12, 2021
  179. kittywhiskers referenced this in commit b4558b769d on Mar 12, 2021
  180. kittywhiskers referenced this in commit dc93ffcebe on Mar 12, 2021
  181. kittywhiskers referenced this in commit a03b9b0d8c on Mar 12, 2021
  182. kittywhiskers referenced this in commit 8bc60430e0 on Mar 14, 2021
  183. kittywhiskers referenced this in commit 3d5ad54069 on Mar 14, 2021
  184. kittywhiskers referenced this in commit 78125dc73b on Mar 18, 2021
  185. kittywhiskers referenced this in commit 9670323cbc on Mar 18, 2021
  186. kittywhiskers referenced this in commit 02976ff105 on Mar 18, 2021
  187. kittywhiskers referenced this in commit 0238fe5249 on Mar 19, 2021
  188. kittywhiskers referenced this in commit 4229df7fbf on Mar 19, 2021
  189. kittywhiskers referenced this in commit 63602a5e1e on Apr 21, 2021
  190. kittywhiskers referenced this in commit d2d32870f3 on Apr 21, 2021
  191. kittywhiskers referenced this in commit 06b7b250a9 on Apr 21, 2021
  192. kittywhiskers referenced this in commit 6b78f60bfe on Apr 23, 2021
  193. kittywhiskers referenced this in commit 293994d96e on Apr 23, 2021
  194. kittywhiskers referenced this in commit bde64ae98f on Apr 23, 2021
  195. kittywhiskers referenced this in commit c2b2207cf1 on Apr 23, 2021
  196. kittywhiskers referenced this in commit fec6cb01e4 on May 11, 2021
  197. kittywhiskers referenced this in commit 52074fad19 on May 11, 2021
  198. kittywhiskers referenced this in commit e35c4c8b05 on May 11, 2021
  199. UdjinM6 referenced this in commit 5404b6b948 on May 15, 2021
  200. kittywhiskers referenced this in commit ee78761e89 on May 18, 2021
  201. kittywhiskers referenced this in commit 32a96607b9 on May 18, 2021
  202. UdjinM6 referenced this in commit 98541dc0cf on May 19, 2021
  203. kittywhiskers referenced this in commit 067aa67a9d on May 19, 2021
  204. kittywhiskers referenced this in commit 4b2b5f78d4 on May 19, 2021
  205. PastaPastaPasta referenced this in commit 2f555f87d1 on May 19, 2021
  206. kittywhiskers referenced this in commit 1e46ea93a3 on May 20, 2021
  207. random-zebra referenced this in commit 85f000ecbe on Jul 30, 2021
  208. random-zebra referenced this in commit b4751e10ce on Aug 11, 2021
  209. DrahtBot locked this on Feb 15, 2022


naumenkogs MarcoFalke sipa Sjors DrahtBot wiz practicalswift TheBlueMatt luke-jr laanwj jonatack EthanHeilman pinheadmz jamesob adamjonas fjahr muoitranduc Empact jnewbery

Labels
Feature P2P Review club

Milestone
0.20.0

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-07-03 13:13 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me