aboutsummaryrefslogtreecommitdiff
path: root/src/addrman.cpp
AgeCommit message (Collapse)Author
2021-09-28[style] Run changed files through clang formatter.Amiti Uttarwar
2021-09-28scripted-diff: Rename CAddrInfo to AddrInfoAmiti Uttarwar
-BEGIN VERIFY SCRIPT- git grep -l CAddrInfo src/ | xargs sed -i 's/CAddrInfo/AddrInfo/g' -END VERIFY SCRIPT-
2021-09-28scripted-diff: Rename CAddrMan to AddrManAmiti Uttarwar
-BEGIN VERIFY SCRIPT- git grep -l CAddrMan src/ test/ | xargs sed -i 's/CAddrMan/AddrMan/g' -END VERIFY SCRIPT-
2021-09-28[includes] Fix up included filesAmiti Uttarwar
2021-09-28[doc] Update commentsAmiti Uttarwar
Maintain comments on the external interfaces rather than on the internal functions that implement them.
2021-09-28[refactor] Update GetAddr_() function signatureAmiti Uttarwar
Update so the internal function signature matches the external one, as is the case for the other addrman functions.
2021-09-28[net, addrman] Remove external dependencies on CAddrInfo objectsAmiti Uttarwar
CAddrInfo objects are an implementation detail of how AddrMan manages and adds metadata to different records. Encapsulate this logic by updating Select & SelectTriedCollision to return the additional info that the callers need.
2021-09-28[addrman] Introduce CAddrMan::Impl to encapsulate addrman implementation.Amiti Uttarwar
Introduce the pimpl pattern for CAddrMan to separate the implementation details from the externally used object representation. This reduces compile-time dependencies and conceptually clarifies AddrMan's interface from the implementation specifics. Since the unit & fuzz tests currently rely on accessing CAddrMan internals, this commit introduces addrman_impl.h, which is exclusively imported by addrman.cpp and test files. Review hint: git diff --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
2021-09-28[move-only] Match ordering of CAddrMan declarations and definitionsAmiti Uttarwar
Also move `Check` and `ForceCheckAddrman` to be after the `FunctionName_` functions. Review hint: use git diff --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
2021-09-28[move-only] Move CAddrMan function definitions to cppAmiti Uttarwar
In preparation for introducing the pimpl pattern to addrman, move all function bodies out of the header file. Review hint: use git diff --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
2021-09-21addrman: Replace assert with throw on corrupt dataMarcoFalke
Assert should only be used for program internal logic errors, not to sanitize external user input.
2021-09-21Refactor: Turn the internal addrman check helper into a forced checkMarcoFalke
2021-09-21move-only: Move CAddrMan::Check to cpp fileMarcoFalke
This speeds up compilation of the whole program because the included header file is smaller. Can be reviewed with --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
2021-09-16addrman: Improve performance of GoodMartin Zumsande
This is done by removing an unnecessary loop in Good_() and looping through the new tables in MakeTried() more efficiently, choosing a starting value that allow us to stop early in typical cases. Co-authored-by: John Newbery <john@johnnewbery.com>
2021-09-10Merge bitcoin/bitcoin#22911: [net] Minor cleanups to asmapfanquake
853c4edb70f897a6a7165abaea4a303d7d448721 [net] Remove asmap argument from CNode::CopyStats() (John Newbery) 9fd5618610e91e3949536c5122cf31eb58c9aa6b [asmap] Make DecodeAsmap() a utility function (John Newbery) bfdf4ef334a16ef6108a658bf4f8514754128c18 [asmap] Remove SanityCheckASMap() from netaddress (John Newbery) 07a9eccb60485e71494664cc2b1964ae06a3dcf0 [net] Remove CConnman::Options.m_asmap (John Newbery) Pull request description: These small cleanups to the asmap code are the first 4 commits from #22910. They're minor improvements that are independently useful whether or not 22910 is merged. ACKs for top commit: naumenkogs: ACK 853c4edb70f897a6a7165abaea4a303d7d448721 theStack: Concept and code-review ACK 853c4edb70f897a6a7165abaea4a303d7d448721 🗺️ fanquake: ACK 853c4edb70f897a6a7165abaea4a303d7d448721 Tree-SHA512: 64783743182592ac165df6ff8d18870b63861e9204ed722c207fca6938687aac43232a5ac4d8228cf8b92130ab0349de1b410a2467bb5a9d60dd9a7221b3b85b
2021-09-09Merge bitcoin/bitcoin#22915: Remove confusing CAddrDBMarcoFalke
fade9a1a4db71241ccad03fdacfb626453952963 Remove confusing CAddrDB (MarcoFalke) fa7f77b7d1709bf35808fced0d67b6e97b784d63 Fix addrdb includes (MarcoFalke) fa3f5d0dae2381038439b91ef2af85ec277c8294 Move addrman includes from .h to .cpp (MarcoFalke) Pull request description: Split out from #22762 to avoid having to carry it around in (an)other rebase(s) ACKs for top commit: ryanofsky: Code review ACK fade9a1a4db71241ccad03fdacfb626453952963 lsilva01: Code Review ACK https://github.com/bitcoin/bitcoin/pull/22915/commits/fade9a1a4db71241ccad03fdacfb626453952963 Tree-SHA512: 7615fb0b6235d0c1e6f8cd6263dd18c4d95890567c2b797fe6fce6cb12cc85ce6eacbe07dbb6d81b05d179ef03b42edfd61c940e35a1044ce6d363b54c2dae5c
2021-09-08Merge bitcoin/bitcoin#22879: addrman: Fix format string in deserialize errorfanquake
fab0b55cf060c2b14fae5cee13f0a2dcaebde892 addrman: Fix format string in deserialize error (MarcoFalke) facce4ca44bc206b7656e297a7fa5dfb83a01012 test: Remove useless overwrite (MarcoFalke) Pull request description: The format string is evaluated differently on modern compilers (clang 10 and later, as well as gcc 10 and later). Work around the behaviour change in compilers by pinning the underlying type of the format arguments. Can be tested by observing a failing test when running against master compiled with clang 10 or gcc 10 (or later). ACKs for top commit: jonatack: ACK fab0b55cf060c2b14fae5cee13f0a2dcaebde892 verified the test fails on master as expected only at line 61 (assertion fixed by the code change); the last two test additions pass as expected mzumsande: ACK fab0b55cf060c2b14fae5cee13f0a2dcaebde892 Tree-SHA512: 07462901435107f3bc79098fd7d06446bfe8fe065fffdd35adfcba8f1dd3c499575006557afe7bc74b79d690c5ef7b58e3e031e908161be5529cf237e3b30609
2021-09-07[asmap] Make DecodeAsmap() a utility functionJohn Newbery
DecopeAsmap is a pure utility function and doesn't have any dependencies on addrman, so move it to util/asmap. Reviewer hint: use: `git diff --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space`
2021-09-07[asmap] Remove SanityCheckASMap() from netaddressJohn Newbery
SanityCheckASMap(asmap, bits) simply calls through to SanityCheckASMap(asmap) in util/asmap. Update all callers to simply call that function.
2021-09-07Move addrman includes from .h to .cppMarcoFalke
This is a follow-up to the code move in commit a820e79512b67b1bfda20bdc32b47086d2b0910d
2021-09-06Merge bitcoin/bitcoin#22791: init: Fix asmap/addrman initialization order bugMarcoFalke
724c4975622bc22cedc3f3814dfc8e66cf8371f7 [fuzz] Add ConsumeAsmap() function (John Newbery) 5840476714ffebb2599999c85a23b52ebcff6090 [addrman] Make m_asmap private (John Newbery) f9002cb5dbd573cd9ca200de21319fa296e26055 [net] Rename the copyStats arg from m_asmap to asmap (John Newbery) f572f2b2048994b3b50f4cfd5de19e40b1acfb22 [addrman] Set m_asmap in CAddrMan initializer list (John Newbery) 593247872decd6d483a76e96d79433247226ad14 [net] Remove CConnMan::SetAsmap() (John Newbery) 50fd77045e2f858a53486b5e02e1798c92ab946c [init] Read/decode asmap before constructing addrman (John Newbery) Pull request description: Commit 181a1207 introduced an initialization order bug: CAddrMan's m_asmap must be set before deserializing peers.dat. The first commit restores the correct initialization order. The remaining commits make `CAddrMan::m_asmap` usage safer: - don't reach into `CAddrMan`'s internal data from `CConnMan` - set `m_asmap` in the initializer list and make it const - make `m_asmap` private, and access it (as a reference to const) from a getter. This ensures that peers.dat deserialization must happen after setting m_asmap, since m_asmap is set during CAddrMan construction. ACKs for top commit: mzumsande: Tested ACK 724c4975622bc22cedc3f3814dfc8e66cf8371f7 amitiuttarwar: code review but utACK 724c497562 naumenkogs: utACK 724c4975622bc22cedc3f3814dfc8e66cf8371f7 vasild: ACK 724c4975622bc22cedc3f3814dfc8e66cf8371f7 MarcoFalke: review ACK 724c4975622bc22cedc3f3814dfc8e66cf8371f7 👫 Tree-SHA512: 684a4cf9e3d4496c9997fb2bc4ec874809987055c157ec3fad1d2143b8223df52b5a0af787d028930b27388c8efeba0aeb2446cb35c337a5552ae76112ade726
2021-09-05addrman: Fix format string in deserialize errorMarcoFalke
Also add a regression test.
2021-08-27[addrman] Make m_asmap privateJohn Newbery
Add a GetAsmap() getter function that returns a reference to const.
2021-08-27[addrman] Set m_asmap in CAddrMan initializer listJohn Newbery
This allows us to make it const.
2021-08-26[refactor] [addrman] Update constant commentsJohn Newbery
Co-authored-by: Amiti Uttarwar <amiti@uttarwar.org>
2021-08-26[move-only] Extract constants from addrman .h to .cppAmiti Uttarwar
Reviewer hint: use `git diff --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space` Co-authored-by: John Newbery <john@johnnewbery.com>
2021-08-26[addrman] Change addrman #define constants to be constexprsAmiti Uttarwar
Co-authored-by: John Newbery <john@johnnewbery.com>
2021-08-26[addrman] Move CAddrMan::Unserialize to cpp fileJohn Newbery
Reviewer hint: use `git diff --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space` Co-authored-by: Amiti Uttarwar <amiti@uttarwar.org>
2021-08-26[addrman] Move CAddrMan::Serialize to cpp fileJohn Newbery
Reviewer hint: use `git diff --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space` Co-authored-by: Amiti Uttarwar <amiti@uttarwar.org>
2021-08-19[addrman] Clean up ctorJohn Newbery
Use default initialization and initializer lists, and use range-based for loops for resetting the buckets.
2021-08-19[addrman] inline Clear() into CAddrMan ctorJohn Newbery
Clear() is now only called from the ctor, so just inline the code into that function. The LOCK(cs) can be removed, since there can be no data races in the ctor. Also move the function definition out of the header and into the cpp file.
2021-08-13p2p: log addrman consistency checksJon Atack
2021-08-12[addrman] Make addrman consistency checks a runtime optionJohn Newbery
Currently addrman consistency checks are a compile time option, and are not enabled in our CI. It's unlikely anyone is running these consistency checks. Make them a runtime option instead, where users can enable addrman consistency checks every n operations (similar to mempool tests). Update the addrman unit tests to do internal consistency checks every 100 operations (checking on every operations causes the test runtime to increase by several seconds). Also assert on a failed addrman consistency check to terminate program execution.
2021-08-05Add missing const to CAddrMan::Check_()MarcoFalke
Also: Always compile the function signature to avoid similar issues in the future.
2021-08-03Merge bitcoin/bitcoin#22496: addrman: Remove addrman hotfixesfanquake
65332b1178c75e1f83415bad24918996a1524866 [addrman] Remove RemoveInvalid() (John Newbery) Pull request description: PRs #22179 and #22112 (EDIT: later reverted in #22497) added hotfix code to addrman to remove invalid addresses and mutate the ports of I2P entries after entering into addrman. Those hotfixes included at least two addrman data corruption bugs: - #22467 (Assertion `nRndPos1 < vRandom.size() && nRndPos2 < vRandom.size()' failed) - #22470 (Changing I2P ports in addrman may wronly skip some entries from "new" buckets) Hotfixing addrman is inherently dangerous. There are many members that have implicit assumptions on each others' state, and mutating those directly can lead to violating addrman's internal invariants. Instead of trying to hotfix addrman, just don't insert any invalid addresses. For now, those are addresses which fail `CNetAddr::IsValid()`. ACKs for top commit: sipa: utACK 65332b1178c75e1f83415bad24918996a1524866. I tried to reason through scenarios that could introduce inconsistencies with this code, but can't find any. fanquake: ACK 65332b1178c75e1f83415bad24918996a1524866 - Skipping the addition of invalid addresses (this code was initially added for Tor addrs) rather than adding all the invalids then removing them all when finishing unserializing seems like an improvement. Especially if it can be achieved with less code. Tree-SHA512: 023113764cb475572f15da7bf9824b62b79e10a7e359af2eee59017df354348d2aeed88de0fd4ad7a9f89a0dad10827f99d70af6f1cb20abb0eca2714689c8d7
2021-07-23Fix incorrect whitespace in addrmanMarcoFalke
Leaving it as-is would be annoying because some editor fix-up the spacing when opening a file or editing it.
2021-07-21refactor: Mark CAddrMan::GetAddr constMarcoFalke
2021-07-21refactor: Mark CAddrMan::Select constMarcoFalke
2021-07-20[addrman] Remove RemoveInvalid()John Newbery
Instead of deserializing addresses, placing them in the buckets, and then removing them if they're invalid, check first and don't place in the buckets if they're invalid.
2021-07-19scripted-diff: remove ResetI2PPorts() (revert e0a2b390c14)Vasil Dimov
`CAddrMan::ResetI2PPorts()` was temporary. Remove it: * it has partially achieved its goal: probably ran on about half of the I2P nodes * it is hackish, deemed risky and two bugs where found in it https://github.com/bitcoin/bitcoin/issues/22467 https://github.com/bitcoin/bitcoin/issues/22470 -BEGIN VERIFY SCRIPT- git show e0a2b390c144e123e2fc8a289fdff36815476964 |git apply -R -END VERIFY SCRIPT- Fixes https://github.com/bitcoin/bitcoin/issues/22467 Fixes https://github.com/bitcoin/bitcoin/issues/22470
2021-07-09addrman: reset I2P ports to 0 when loading from diskVasil Dimov
This is a temporary change to convert I2P addresses that have propagated with port 8333 to ones with port 0. It would cause a problem some day if indeed some bitcoin software is listening on port 8333 only and rejects connections to port 0 and we are still using SAM 3.1 which only supports port 0. In this case we would replace 8333 with 0 and try to connect to such nodes. This commit should be included in 22.0 and be reverted before 23.0 is released.
2021-07-08Merge bitcoin/bitcoin#22179: Torv2 removal followupsW. J. van der Laan
00b875ba9414463d0041da6924fd9b54d6a06dee addrman: remove invalid addresses when unserializing (Vasil Dimov) bdb62096f0109b2ec76849d33d6cf7187dea299f fuzz: reduce possible networks check (Vasil Dimov) a164cd3ba694ffeba03b2887a411b7f82f6c087e net: simplify CNetAddr::IsRoutable() (Vasil Dimov) Pull request description: * Simplify some code, now that we know `CNetAddr::IsRFC4193()` and `CNetAddr::IsTor()` cannot be `true` at the same time. * Drop Tor v2 addresses when loading addrman from `peers.dat` - they would have been loaded as dummy-all-zeros IPv6 addresses and linger in addrman, wasting space. ACKs for top commit: sipa: ACK 00b875ba9414463d0041da6924fd9b54d6a06dee. Reviewed the code, and tested with -DDEBUG_ADDRMAN (unit tests + mainnet run with peers.dat that contained v2 onions). laanwj: Code review and lightly tested ACK 00b875ba9414463d0041da6924fd9b54d6a06dee jonatack: ACK 00b875ba9414463d0041da6924fd9b54d6a06dee reviewed, debug-built with -DEBUG_ADDRMAN rebased to current master, restarted node on mainnet/signet/testnet and verified that on each chain -addrinfo shows no change in address counts (as expected). Added some sanity check asserts, rebuilt/re-ran test. Checked that the new test fails on master with "test/addrman_tests.cpp(824): error: in "addrman_tests/remove_invalid": check addrman.size() == 2 has failed [4 != 2]" jarolrod: ACK 00b875ba9414463d0041da6924fd9b54d6a06dee Tree-SHA512: 6ed8e6745134b1b94fffaba28482de909ea39483b46b7f57bda61cdbae7a51251d15cb674de3631772fbeabe153d77a19269f96e62a89102a2d5c01e48f0ba06
2021-06-14Add AssertLockHeld to CAddrMan private functionsHennadii Stepanov
2021-06-14refactor: Avoid recursive locking in CAddrMan::sizeHennadii Stepanov
2021-06-12Merge bitcoin/bitcoin#18722: addrman: improve performance by using more ↵fanquake
suitable containers a92485b2c250fd18f55d22aa32722bf52ab32bfe addrman: use unordered_map instead of map (Vasil Dimov) Pull request description: `CAddrMan` uses `std::map` internally even though it does not require that the map's elements are sorted. `std::map`'s access time is `O(log(map size))`. `std::unordered_map` is more suitable as it has a `O(1)` access time. This patch lowers the execution times of `CAddrMan`'s methods as follows (as per `src/bench/addrman.cpp`): ``` AddrMan::Add(): -3.5% AddrMan::GetAddr(): -76% AddrMan::Good(): -0.38% AddrMan::Select(): -45% ``` ACKs for top commit: jonatack: ACK a92485b2c250fd18f55d22aa32722bf52ab32bfe achow101: ACK a92485b2c250fd18f55d22aa32722bf52ab32bfe hebasto: re-ACK a92485b2c250fd18f55d22aa32722bf52ab32bfe, only suggested changes and rebased since my [previous](https://github.com/bitcoin/bitcoin/pull/18722#pullrequestreview-666663681) review. Tree-SHA512: d82959a00e6bd68a6c4c5a265dd08849e6602ac3231293b7a3a3b7bf82ab1d3ba77f8ca682919c15c5d601b13e468b8836fcf19595248116635f7a50d02ed603
2021-06-07addrman: remove invalid addresses when unserializingVasil Dimov
The Tor v2 addresses, left over from when Tor v2 was supported will be unserialized as a dummy, invalid `::` (all zeros) IPv6 address. Remove them so that they do not take up space in addrman.
2021-05-31refactor: Switch serialize to uint8_t (1/n)MarcoFalke
2021-05-28addrman: use unordered_map instead of mapVasil Dimov
`CAddrMan` uses `std::map` internally even though it does not require that the map's elements are sorted. `std::map`'s access time is `O(log(map size))`. `std::unordered_map` is more suitable as it has a `O(1)` access time. This patch lowers the execution times of `CAddrMan`'s methods as follows (as per `src/bench/addrman.cpp`): ``` AddrMan::Add(): -3.5% AddrMan::GetAddr(): -76% AddrMan::Good(): -0.38% AddrMan::Select(): -45% ```
2021-05-19p2p: pull time call out of loop in CAddrMan::GetAddr_()João Barbosa
2021-05-19p2p: enable CAddrMan::GetAddr_() by network, add doxygenJon Atack
generated by cgit v1.2.3 (git 2.26.2) at 2024-09-01 11:16:29 +0000