p2p: track AddrMan totals by network and table, improve precision of adding fixed seeds #26847

mzumsande · 2023-01-08T21:45:27Z

AddrMan currently doesn't track the number of its entries by network, it only knows the total number of addresses. This PR makes AddrMan keep track of these numbers, which would be helpful for multiple things:

Allow to specifically add fixed seeds to AddrMan of networks where we don't have any addresses yet - even if AddrMan as a whole is not empty (partly fixing Finding peers to connect to after -onlynet changes may be problematic #26035). This is in particular helpful if the user abruptly changes -onlynet settings (such that addrs that used to be reachable are no longer and vice versa), in which case they currently could get stuck and not find any outbound peers. The second commit of this PR implements this.
(Future work): Add logic for automatic connection management with respect to networks - such as making attempts to have at least one connection to each reachable network as suggested here. This would involve requesting an address from a particular network from AddrMan, and expanding its corresponding function AddrMan::Select() to do this requires internal knowledge of the current number of addresses for each network and table to avoid getting stuck in endless loops.
(Future work): Perhaps display the totals to users. At least I would find this helpful to debug, the existing option (./bitcoin-cli -addrinfo) is rather indirect by doing the aggregation itself in each call, doesn't distinguish between new and tried, and being based on AddrMan::GetAddr() it's also subject to a quality filter which we probably don't want in this spot.

DrahtBot · 2023-01-08T21:45:29Z

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	naumenkogs, stratospher, vasild, achow101
Concept ACK	amitiuttarwar, brunoerg, sipa

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#26261 (p2p: cleanup LookupIntern, Lookup and LookupHost by brunoerg)
#26114 (net: Make AddrFetch connections to fixed seeds by mzumsande)
#25284 (consensus: Remove dependency on ADDRV2_FORMAT by MarcoFalke)

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.

amitiuttarwar · 2023-01-08T21:56:32Z

approach ACK. relatively simple patch to ameliorate the issue of address discovery when switching networks

src/addrman.cpp

naumenkogs · 2023-01-09T08:15:02Z

src/addrman.cpp

Given this dependency, should we drop nNew and nTried altogether?

I think that's not so straightforward - nNew and nTried are part of the serialization after all. Also, I don't like that then we'd need to build the sum over all networks every time we needed nNew or nTried (as the check code does).

Also, I don't like that then we'd need to build the sum over all networks every time we needed nNew or nTried (as the check code does).

Well, I don't think something being inside the Check is a good justification — it's the other way around, Check is needed to make sure those values are sane. Furthermore, the sum over all networks is 5 addition operations, right?

However, I agree that dropping these values might be not trivial due to serialization. Not sure I'll find time to prototype that, but maybe someone will :) Otherwise feel free to close this suggestion.

Also, see this :)

I've changed the check such that it no longer compares with nNew and nTried but does a recount, see below for more context (of course the relation is still there...).

src/net.h

brunoerg

Concept ACK

src/addrman.cpp

sipa · 2023-01-09T22:05:06Z

src/addrman.cpp

It'd be good if the sanity check here actually recomputed the totals per-network (by iterating over the actual addrman entries).

I changed this to actually recompute the totals - while doing so I noticed that it is possible that it's possible that there are "empty" entries (0 new and 0 tried) in m_map_counts if an addr of some network was first added and later removed again from. That's not a problem in my opinion, but it means that the size of the map is not necessarily reliable. The changed check should work with this, potentially also adding an entry empty to local_counts.

src/addrman_impl.h

sipa · 2023-01-09T22:07:32Z

Concept/approach ACK

src/addrman.h

naumenkogs · 2023-01-12T08:20:15Z

src/addrman.cpp

+        return -20;
+    }
+    for (const auto& [net, count] : m_map_counts) {
+        if (local_counts[net].first != count.first || local_counts[net].second != count.second) {


This is probably too crazy, but I was thinking what if these values overflow (from ++) and are not equal anymore due to UB? I'm actually not sure what's our attitude towards such risks across the code.

Yes, It definitely shouldn't be possible and would mean that there is a serious bug in addrman, but I think that within the Check() code, we definitely want to abort. After all, Check() doesn't normally run in production (DEFAULT_ADDRMAN_CONSISTENCY_CHECKS == 0) and needs to be activated by the user.

vasild

Concept ACK

src/addrman.cpp

src/test/addrman_tests.cpp

src/net.cpp

vasild · 2023-01-13T15:32:59Z

src/net.cpp

+std::vector<Network> CConnman::GetRelevantFixedSeedNetworks() const
+{
+    std::vector<Network> networks{};
+    for (int n = 0; n < NET_MAX; n++) {
+        enum Network net = (enum Network)n;
+        if (net == NET_UNROUTABLE || net == NET_INTERNAL) continue;
+        if (IsReachable(net) && addrman.size(net, std::nullopt) == 0) {
+            networks.push_back(net);
+        }
+    }
+    return networks;
+}


This does not need to be a method of CConnman because it does not access any of its members (other than addrman). Would it be more appropriate to have it as AddrMan method? Then the public interface of AddrMan need not be changed by this PR. Edit: I mean no need to add this AddrMan method size(std::optional<Network> net, std::optional<bool> in_new).

I'm not sure if this belongs into AddrMan because of the IsReachable - I think the information which networks are reachable belongs into net, and possibly at some later point into CConnman (I know vfLimited is currently a global, but this might change). I don't think it makes sense that AddrMan should know or deal with this information, so I think I prefer leaving this out of AddrMan and reducing the change of the public interface by combining the size functions as suggested by Amiti below.

What about a standalone function?

I'd be ok with that, but then we'd have to pass it addrman, which is a member of CConnman, so I'm not sure why that is better.

amitiuttarwar · 2023-01-14T02:46:52Z

code looks good. I went through all the addrman touch points to the new & tried tables to double check nothing was missed. tried my best to think through edge cases of what could go wrong and don't have anything :)

left a bunch of review comments that are all style related, no blockers.

another style related suggestion: what do you think about consolidating the two size() functions? adding default params to the newly introduced function means you can remove the old one & none* of the call sites need to change. I tried it out here.
[*] none of the bitcoind callers. one test call site failed because of the shift from vRandom.size to nNew + nTried. the test is checking a corrupted addrman, and I don't think the size is testing anything meaningful there, so I removed that check.

src/addrman.cpp

src/addrman.h

amitiuttarwar · 2023-01-14T01:08:33Z

src/addrman.cpp

+    if (!net.has_value()) {
+        if (!in_new.has_value()) {
+            return nNew + nTried;
+        } else {
+            return *in_new ? nNew : nTried;
+        }
+    }
+    if (auto it = m_map_counts.find(*net); it != m_map_counts.end()) {
+        auto entry = it->second;
+        if (!in_new.has_value()) {
+            return entry.first + entry.second;
+        } else {
+            return *in_new ? entry.first : entry.second;
+        }
+    }


style suggestion to consider. mainly, reversing the logic blocks in the if statements to reduce the ! indirections of nested logic.

code

Suggested change

if (!net.has_value()) {

if (!in_new.has_value()) {

return nNew + nTried;

} else {

return *in_new ? nNew : nTried;

}

}

if (auto it = m_map_counts.find(*net); it != m_map_counts.end()) {

auto entry = it->second;

if (!in_new.has_value()) {

return entry.first + entry.second;

} else {

return *in_new ? entry.first : entry.second;

}

}

if (!net) {

if (in_new.has_value()) {

return *in_new ? nNew : nTried;

} else {

return nNew + nTried;

}

}

if (auto it = m_map_counts.find(*net); it != m_map_counts.end()) {

auto entry = it->second;

if (in_new.has_value()) {

return *in_new ? entry.first : entry.second;

} else {

return entry.first + entry.second;

}

}

also just noting that you use nNew + nTried to represent size of unique addresses in addrman. this breaks from the convention of using vRandom.size(), but makes sense to me.

I took your suggestion wrt to the order.

Yes, these need to be identical, but I actually changed it to use the old vRandom.size() used in size() so that there isn't a change from the status quo when the two size() functions are merged as you suggest below.

src/addrman_impl.h

src/net.h

vasild · 2023-01-14T13:56:14Z

... of what could go wrong and don't have anything ...

Oh, well.

When nothing can possibly go wrong, it will.

:)

I guess nNew and m_map_count[].first could go out of sync in the future. It is good that now they are always updated next to each other (or within 1-2 lines). Further enforcing of this would be to add a new method that modifies both and only change them through this method. E.g.

void IncNew(ssize_t delta, Network net)
{
    nNew += delta;
    m_map_count[net].first += delta;
}

Or ditch nNew and sum m_map_count[].first whenever we would read nNew, via a new method like size_t CountNew() const? Performance considerations?

amitiuttarwar · 2023-01-14T20:50:29Z

ah, just to clarify, I meant I couldn't identify anything that could go wrong in the current version of the code. ofc things can always go awry in the future, which is why code style is useful to help reduce the chances of that happening :)

I guess nNew and m_map_count[].first could go out of sync in the future. It is good that now they are always updated next to each other (or within 1-2 lines). Further enforcing of this would be to add a new method that modifies both and only change them through this method.

agreed that the counts of nNew (and also nTried) could go out of sync with the totals in m_map_count. it can also be problematic if these values are in sync with each other, but out of sync with the actual values stored on the new / tried tables & other addrman structures.

the code added to CheckAddrman are helpful because they offer a programatic testing against this inadvertently changing in the future. I don't think having specific "accessor" methods makes the most sense because we already have functions that offer logical coupling of creating & removing addrs on addrman*. also, it would require 4 additional functions (increment & decrement / new & tried).

[*] for eg. other than unserialization, all additions to the new table go through Addrman::Add, which calls AddSingle which uses Create as a helper to construct the AddrInfo object & start persisting to internal data structs. my comment here suggested moving incrementing both counts inside the Create function.

Or ditch nNew and sum m_map_count[].first whenever we would read nNew, via a new method like size_t CountNew() const? Performance considerations?

this one I'm more open to. martin pointed out some difficulties in that approach here, and I personally think its nbd either way.

mzumsande · 2023-01-16T23:00:51Z

Thanks a lot for the reviews! I think I have addressed all comments and implemented most of the suggestions - let me know if I missed something!

another style related suggestion: what do you think about consolidating the two size() functions? adding default params to the newly introduced function means you can remove the old one & none* of the call sites need to change. I tried it out here.
[*] none of the bitcoind callers. one test call site failed because of the shift from vRandom.size to nNew + nTried. the test is checking a corrupted addrman, and I don't think the size is testing anything meaningful there, so I removed that check.

I did that, and combined it with @vasild's suggestion to use upper case notation Size() as per the developer notes. This involved a lot of renaming of existing call sites, so I did it in an extra commit at the end. Since I switched back to use vRandom.size to keep existing behavior, the corrupted AddrMan test doesn't need to be changed anymore.

Or ditch nNew and sum m_map_count[].first whenever we would read nNew, via a new method like size_t CountNew() const? Performance considerations?

this one I'm more open to. martin pointed out some difficulties in that approach here, and I personally think its nbd either way.

Yes, since it's part of the serialization we can't completely do without it. Of course, we could decide to just use nNew and nTried locally in Unserialize(), and rebuild it from the sum from m_network_counts everywhere else. I would suspect that there is no major difference performance-wise because the maximum number of networks is very small - I could try to benchmark it.

src/addrman.cpp

naumenkogs · 2023-01-17T09:34:06Z

src/addrman.cpp

local_count[net] could be called on the non-existent key, but that's fine and will return 0, right?

perhaps a test for this case would be great...

local_count[net] could be called on the non-existent key, but that's fine and will return 0, right?

yes, that's correct - it will create a entry in local_counts with 0, and then return 0. I think that's actually already happening in some unit test I think - but I will try to add a more explicit test soon!

Oh, I should have tried to break it and see whether there is a such test. Explicit is better of course.

For this to happen, we need to have an address in AddrMan for a network at one point in time, which later gets removed, so that we have an entry in m_network_counts that has only zeroes, but no entry in local_counts.
This happens in the addrman_tests/remove_invalid unit test which tampers with the serialized stream, so that an address gets added and then removed during Deserialization - if you break CheckAddrman() by adding an assert at this spot, this subtest will fail.

I think it could also happen theoretically if the last remaining address from network A is being kicked out by an address from network B due to a collision in the new tables. But this doesn't seem trivial to write a unit test for, since addresses from different networks are typically from different groups, so I'm not sure if it's easily possible to write an explicit test for this.

… addresses Previously, we'd only load fixed seeds if we'd not know any addresses at all. This change makes it possible to change -onlynet abruptly, e.g. from -onlynet=onion to -onlynet=i2p and still find peers.

Create() is only called in one spot, so this doesn't change behavior. Co-authored-by: Amiti Uttarwar <[email protected]>

The functionality of the old size() is covered by the new Size() when no arguments are specified, so this does not change behavior. Co-authored-by: Martin Zumsande <[email protected]>

naumenkogs · 2023-01-27T07:23:43Z

utACK 80f39c9

stratospher · 2023-01-27T22:00:20Z

ACK 80f39c9

vasild

ACK 80f39c9

vasild · 2023-01-30T13:44:27Z

src/addrman.h

+    * @param[in] in_new           Select addresses only from one table (true = new, false = tried, nullopt = both)
+    * @return                     Number of unique addresses that match specified options.
+    */
+    size_t Size(std::optional<Network> net = {}, std::optional<bool> in_new = {}) const;


nit, if you re-touch: usually std::nullopt is used instead of {} and I think it is more readable, e.g.:

bitcoin/src/streams.h

Line 236 in 79e18eb

bool Rewind(std::optional<size_type> n = std::nullopt)

will do if I need to re-touch for another reason.

done in #27015

achow101 · 2023-01-31T20:55:28Z

ACK 80f39c9

…improve…

mzumsande · 2023-02-01T04:48:23Z

I just noticed that this can lead to intermittent failures of the load_addrman_corrupted unit test, while checking the size in

bitcoin/src/test/addrman_tests.cpp

Line 951 in ba3d327

BOOST_CHECK(addrman1.Size() == 1);

the internal checks fail (that weren't executed for Size() before this PR, and now are executed in some runs). I think it might be best to just remove this line from the test, since this is a situation where we purposefully load a corrupted peers.dat - seems a good thing that the Addrman Check fails then... I will open a PR tomorrow.

dc70c1e addrman: Use std::nullopt instead of {} (Martin Zumsande) 59cc66a test: Remove AddrMan unit test that fails consistency checks (Martin Zumsande) Pull request description: Two fixups for #26847: * Now that `AddrMan::Size()` performs internal consistency tests (it didn't before), we can't call it in the `load_addrman_corrupted` unit tests, where we deal with an artificially corrupted `AddrMan`. This would fail the test when using `-checkaddrman=1` (leading to spurious CI fails). Therefore remove the tests assertion, which is not particularly helpful anyway (in production we abort init when peers.dat is corrupted instead of querying AddrMan in its corrupted state). (See bitcoin/bitcoin#26847 (comment)) * Use `std::nullopt` instead of `{}` for default args (suggested in bitcoin/bitcoin#26847 (comment)) ACKs for top commit: MarcoFalke: lgtm ACK dc70c1e Tree-SHA512: dd8a988e23d71a66d3dd30560bb653c9ad17db6915abfa5f722818b0ab18921051ec9223bfbc75d967df8bcd204dfe473d680bf68e8a8e4e4998fbb91dc973c5

dc70c1e addrman: Use std::nullopt instead of {} (Martin Zumsande) 59cc66a test: Remove AddrMan unit test that fails consistency checks (Martin Zumsande) Pull request description: Two fixups for bitcoin#26847: * Now that `AddrMan::Size()` performs internal consistency tests (it didn't before), we can't call it in the `load_addrman_corrupted` unit tests, where we deal with an artificially corrupted `AddrMan`. This would fail the test when using `-checkaddrman=1` (leading to spurious CI fails). Therefore remove the tests assertion, which is not particularly helpful anyway (in production we abort init when peers.dat is corrupted instead of querying AddrMan in its corrupted state). (See bitcoin#26847 (comment)) * Use `std::nullopt` instead of `{}` for default args (suggested in bitcoin#26847 (comment)) ACKs for top commit: MarcoFalke: lgtm ACK dc70c1e Tree-SHA512: dd8a988e23d71a66d3dd30560bb653c9ad17db6915abfa5f722818b0ab18921051ec9223bfbc75d967df8bcd204dfe473d680bf68e8a8e4e4998fbb91dc973c5

…ve precision of adding fixed seeds

, bitcoin#25619, bitcoin#27214, bitcoin#26261, bitcoin#27745, bitcoin#27529, bitcoin#28341, partial bitcoin#25331 (addrman backports: part 4) e544d3c fmt: apply `clang-format-diff.py` suggestions, satisfy linter (Kittywhiskers Van Gogh) 7da74ff merge bitcoin#28341: Use HashWriter over legacy CHashWriter (Kittywhiskers Van Gogh) c798b49 merge bitcoin#27529: fix `feature_addrman.py` on big-endian systems (Kittywhiskers Van Gogh) 7d149c9 merge bitcoin#27745: select addresses by network follow-up (Kittywhiskers Van Gogh) 1d82994 merge bitcoin#26261: cleanup `LookupIntern`, `Lookup` and `LookupHost` (Kittywhiskers Van Gogh) 231ff82 merge bitcoin#27214: Enable selecting addresses by network (Kittywhiskers Van Gogh) e825595 merge bitcoin#25619: avoid overriding non-virtual ToString() in CService and use better naming (Kittywhiskers Van Gogh) 2e9b48a merge bitcoin#26847: track AddrMan totals by network and table, improve precision of adding fixed seeds (Kittywhiskers Van Gogh) 79a550e merge bitcoin#26040: comment "add only reachable addresses to addrman" (Kittywhiskers Van Gogh) 1adb635 merge bitcoin#25678: skip querying dns seeds if -onlynet disables IPv4 and IPv6 (Kittywhiskers Van Gogh) 2d99be0 partial bitcoin#25331: Add HashWriter without ser-type and ser-version (Kittywhiskers Van Gogh) Pull request description: ## Additional Information * Dependent on #6243 * Dependent on #5167 ## Breaking Changes None observed. ## Checklist: - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)** - [x] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have made corresponding changes to the documentation **(note: N/A)** - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK e544d3c Tree-SHA512: 63f014142c39c47bda3ac85dc6afeee8f2bfec71f033631bca16d41bb0785f4b090b3c860ddc3b3cf6c4a23558d3d102144fc83b065130c3f9ab91d0de8e4457

…t test) 925870d merge bitcoin#27015: bitcoin#26847 fixups (AddrMan totals) (Kittywhiskers Van Gogh) Pull request description: ## Additional Information As part of [dash#6254](#6254), [bitcoin#26847](bitcoin#26847) was backported and since then, it was observed that unit tests were flakier than expected ([build](https://gitlab.com/dashpay/dash/-/jobs/7811041841), [build](https://gitlab.com/dashpay/dash/-/jobs/7802460298)). The flakiness was caused by behavior introduced by the aforementioned backport, this was resolved upstream with [bitcoin#27015](bitcoin#27015), which this pull request contains. ## Breaking Changes None observed. ## Checklist: - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)** - [x] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have made corresponding changes to the documentation **(note: N/A)** - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK 925870d knst: ACK 925870d Tree-SHA512: 20fc8fb1b162803a71ec4087685460f52ed56c3c86d46ecac4cc0ef59c95b4b6206f0c53bef256242a4a5babb76e3564cfba56a84cbe844e187035de2308b818

DrahtBot added the P2P label Jan 8, 2023

DrahtBot mentioned this pull request Jan 9, 2023

net: Make AddrFetch connections to fixed seeds #26114

Closed

naumenkogs reviewed Jan 9, 2023

View reviewed changes

src/addrman.cpp Outdated Show resolved Hide resolved

naumenkogs reviewed Jan 9, 2023

View reviewed changes

src/addrman.cpp Outdated Show resolved Hide resolved

naumenkogs reviewed Jan 9, 2023

View reviewed changes

src/net.h Outdated Show resolved Hide resolved

brunoerg reviewed Jan 9, 2023

View reviewed changes

mzumsande force-pushed the 202212_addrman_tracknets branch from 85dc0c9 to 18acfdf Compare January 9, 2023 19:47

sipa reviewed Jan 9, 2023

View reviewed changes

mzumsande force-pushed the 202212_addrman_tracknets branch from 18acfdf to c75ca24 Compare January 11, 2023 22:11

naumenkogs reviewed Jan 12, 2023

View reviewed changes

src/addrman.h Outdated Show resolved Hide resolved

naumenkogs reviewed Jan 12, 2023

View reviewed changes

vasild reviewed Jan 13, 2023

View reviewed changes

amitiuttarwar reviewed Jan 14, 2023

View reviewed changes

mzumsande force-pushed the 202212_addrman_tracknets branch 2 times, most recently from 84dfadc to f93ff50 Compare January 16, 2023 22:53

mzumsande force-pushed the 202212_addrman_tracknets branch from f93ff50 to 1484e19 Compare January 16, 2023 23:04

This was referenced Jan 17, 2023

p2p: cleanup LookupIntern, Lookup and LookupHost #26261

Merged

net: Use serialization parameters for CAddress serialization #25284

Merged

BIP324: Enable v2 P2P encrypted transport #24545

Closed

naumenkogs reviewed Jan 17, 2023

View reviewed changes

src/addrman.cpp Outdated Show resolved Hide resolved

naumenkogs reviewed Jan 17, 2023

View reviewed changes

amitiuttarwar mentioned this pull request Jan 17, 2023

Expose addrman size info via RPC #26907

Closed

mzumsande and others added 3 commits January 26, 2023 18:11

net: Load fixed seeds from reachable networks for which we don't have…

c77c877

… addresses Previously, we'd only load fixed seeds if we'd not know any addresses at all. This change makes it possible to change -onlynet abruptly, e.g. from -onlynet=onion to -onlynet=i2p and still find peers.

addrman, refactor: move count increment into Create()

4885d6f

Create() is only called in one spot, so this doesn't change behavior. Co-authored-by: Amiti Uttarwar <[email protected]>

addrman, refactor: combine two size functions

80f39c9

The functionality of the old size() is covered by the new Size() when no arguments are specified, so this does not change behavior. Co-authored-by: Martin Zumsande <[email protected]>

mzumsande force-pushed the 202212_addrman_tracknets branch from 1484e19 to 80f39c9 Compare January 26, 2023 23:11

fanquake requested review from ajtowns and amitiuttarwar and removed request for amitiuttarwar January 27, 2023 10:00

stratospher mentioned this pull request Jan 29, 2023

cli: rework -addrinfo cli to use addresses which aren’t filtered for quality/recency #26988

Open

vasild approved these changes Jan 30, 2023

View reviewed changes

achow101 merged commit ba3d327 into bitcoin:master Jan 31, 2023

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Feb 1, 2023

Merge bitcoin#26847: p2p: track AddrMan totals by network and table, …

6df4c23

…improve…

mzumsande mentioned this pull request Feb 1, 2023

p2p: 26847 fixups (AddrMan totals) #27015

Merged

rajarshimaitra mentioned this pull request Feb 13, 2023

p2p: track AddrMan totals by network and table, improve precision of adding fixed seeds #26847 Bitshala/BitcoinCore-PR-Review-Club#11

Closed

glozow mentioned this pull request Feb 24, 2023

Socratic Seminar #22 Topics londonbitdevs/londonbitdevs-website#9

Closed

mzumsande deleted the 202212_addrman_tracknets branch October 13, 2023 15:17

vasild mentioned this pull request Aug 12, 2024

Finding peers to connect to after -onlynet changes may be problematic #26035

Closed

kwvg added a commit to kwvg/dash that referenced this pull request Sep 11, 2024

merge bitcoin#26847: track AddrMan totals by network and table, impro…

2e9b48a

…ve precision of adding fixed seeds

kwvg added a commit to kwvg/dash that referenced this pull request Sep 13, 2024

merge bitcoin#27015: bitcoin#26847 fixups (AddrMan totals)

925870d

kwvg mentioned this pull request Sep 13, 2024

backport: merge bitcoin#27015 (resolve flaky addrman unit test) dashpay/dash#6273

Merged

5 tasks

bitcoin locked and limited conversation to collaborators Oct 12, 2024

p2p: track AddrMan totals by network and table, improve precision of adding fixed seeds #26847

p2p: track AddrMan totals by network and table, improve precision of adding fixed seeds #26847

Uh oh!

Conversation

mzumsande commented Jan 8, 2023

Uh oh!

DrahtBot commented Jan 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews

Conflicts

Uh oh!

amitiuttarwar commented Jan 8, 2023

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mzumsande Jan 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

brunoerg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sipa commented Jan 9, 2023

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vasild left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vasild Jan 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amitiuttarwar commented Jan 14, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mzumsande Jan 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DrahtBot commented Jan 8, 2023 •

edited

Loading

mzumsande Jan 9, 2023 •

edited

Loading

vasild Jan 13, 2023 •

edited

Loading

mzumsande Jan 16, 2023 •

edited

Loading

mzumsande Jan 26, 2023 •

edited

Loading

mzumsande commented Feb 1, 2023 •

edited

Loading