aboutsummaryrefslogtreecommitdiff
path: root/src/addrman.cpp
AgeCommit message (Collapse)Author
2022-07-30addrman: Use system time instead of adjusted network timeMarcoFalke
2022-07-26refactor: Use type-safe std::chrono for addrman timeMarcoFalke
2022-07-26scripted-diff: Rename addrman time symbolsMarcoFalke
-BEGIN VERIFY SCRIPT- ren() { sed -i "s:\<$1\>:$2:g" $(git grep -l "\<$1\>" ./src ./test); } ren nLastTry m_last_try ren nLastSuccess m_last_success ren nLastGood m_last_good ren nLastCountAttempt m_last_count_attempt ren nSinceLastTry since_last_try ren nTimePenalty time_penalty ren nUpdateInterval update_interval ren fCurrentlyOnline currently_online -END VERIFY SCRIPT-
2022-07-26refactor: Remove not needed std::maxMacroFake
2022-06-08Remove redundant nLastTry checkMarcoFalke
All other places calculate "now - nLastTry", which is safe and correct to do when nLastTry is 0. So do the same here.
2022-06-08Remove redundant nTime checksMarcoFalke
nTime is always initialized on deserialization or default-initialized with TIME_INIT, so special casing 0 does not make sense.
2022-05-25Get time less often in AddrManImpl::ResolveCollisions_()MarcoFalke
This makes the code less verbose. Also, future changes that change how to get the time are less verbose. Moreover, GetAdjustedTime() might arbitrarily change the value during the execution of this function. For example, the system time advances over a second boundary, or the network adjusts the time arbitrarily. Most of the time however the value will not change, so it seems better to always lock the value in this scope for clarity.
2022-04-20[netgroupman] Move asmap checksum calculation to NetGroupManagerJohn Newbery
2022-04-20[netgroupman] Move GetMappedAS() and GetGroup() logic to NetGroupManagerJohn Newbery
Reviewer hint: use: `git diff --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space`
2022-04-20[netgroupman] Add GetMappedAS() and GetGroup()John Newbery
These currently call through to the CNetAddr methods. The logic will be moved in a future commit.
2022-04-20[net] Move asmap into NetGroupManagerJohn Newbery
2022-03-25addrman: fix incorrect named argsfanquake
2022-02-25addrman: Log too low compat valueMarcoFalke
Also remove uint8_t{} casts from values that are already of the same type.
2022-02-25p2p: Avoid InitError when downgrading peers.datjunderw
fixes #24188 When downgrading, a peers.dat with a future version that has a minimum required version larger than the downgraded version would cause an InitError. This commit changes this behavior to overwrite the existing peers.dat with a new empty one, while creating a backup in peers.dat.bak.
2022-01-04Merge bitcoin/bitcoin#23826: test: Make AddrMan unit tests use public ↵fanquake
interface, extend coverage ea4c9fd4ab9aaa2e8f2c2e38a75c9f05d0bfc866 test: Cover eviction by timeout in addrman_evictionworks (Martin Zumsande) 4f1bb467b556ec93c9b8f758783fda4d050da491 test: Add test for multiplicity in addrman new tables (Martin Zumsande) e880bb7836dab2018049390884220177c6db9b92 test: Add test for updating addrman entries (Martin Zumsande) f02eee8c8784dfc8db80a21ab6508f7c99298255 test: introduce utility function to retrieve an addrman (Martin Zumsande) f0e5efb82493f7a14580335ce719d5be81c8713e test: Remove unused AddrManTest class (Martin Zumsande) b696d7870b29232057600df5ddd8351888253b95 test: Remove tests for internal helper functions (Martin Zumsande) 0538520091bf2982a029a0298835400f5afbdc15 test: use AddrMan instead of AddrManTest where possible (Martin Zumsande) 1c65d427bbf61bb558cf7e18f7aff99b19f68508 test: Inline SimConnFail function (Martin Zumsande) 5b7aac34f2363822c3a1cfafda8ffc9528905058 test: delete unused GetBucketAndEntry function (Amiti Uttarwar) 2ba1e74e59a325ca6cb140757067dd5e0c7c249b test: Update addrman_serialization unit test to use AddrMan's interface (Amiti Uttarwar) dad5f760211df314d650999e0a76edb0151b4fe1 addrman: Introduce a test-only function to lookup addresses (Amiti Uttarwar) Pull request description: This PR (joint work with Amiti Uttarwar) changes the addrman unit tests such that they only use the public `AddrMan` interface: This has the advantage that the tests are less implementation-dependent, i.e. it would be possible to rewrite the internal addrman implementation (as drafted [here](https://github.com/sipa/bitcoin/tree/202106_multiindex_addrman) for using a multiindex) without having to adjust the tests. This includes the following steps: * Adding a test-only function `FindAddressEntry()` to the public addrman interface which returns info about an address in addrman (e.g. bucket, position, whethe the address is in new or tried). Obviously we want to do this sparingly, but I think a single test-only function is ok (which could also be useful elsewhere, e.g. in fuzz tests). * Removal of the `AddrManTest` subclass which would reach into AddrMan's internals, using `AddrMan` instead * Removal of tests for internal helper functions that are not publicly exposed (these are still tested indirectly via the public functions calling them). * Additional tests for previously untested features such as multiplicity in the new tables, that can be tested with the help of `FindAddressEntry()`. All in all, this PR increases the unit test coverage of AddrMan by a bit. ACKs for top commit: jnewbery: ACK ea4c9fd4ab9aaa2e8f2c2e38a75c9f05d0bfc866 josibake: reACK https://github.com/bitcoin/bitcoin/pull/23826/commits/ea4c9fd4ab9aaa2e8f2c2e38a75c9f05d0bfc866 Tree-SHA512: c2d4ec8bdc62ffd6055ddcd37dea85ec08c76889e9e417e8d7c62a96cf68a8bcbe8c67bec3344d91fa7d3c499f6d9f810962da1dddd38e70966186b10b8ab447
2021-12-30scripted-diff: Bump copyright headersHennadii Stepanov
-BEGIN VERIFY SCRIPT- ./contrib/devtools/copyright_header.py update ./ -END VERIFY SCRIPT- Commits of previous years: * 2020: fa0074e2d82928016a43ca408717154a1c70a4db * 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
2021-12-28addrman: Introduce a test-only function to lookup addressesAmiti Uttarwar
Co-Authored-By: Martin Zumsande <mzumsande@gmail.com>
2021-12-14refactor: make AddrMan::Good return booljosibake
If AddrMan::Good is unable to add an entry to tried (for a number of reasons), return false. This makes it much easier and cleaner to directly test for tried collisions. It also allows anyone calling Good() to handle the case where adding an address to tried is unsuccessful. Update docs to doxygen style.
2021-11-10Merge bitcoin/bitcoin#22872: log: improve checkaddrman logging with duration ↵MarcoFalke
in milliseconds 22b44fc696dc1078c40d17e2d497c74c7b4ae750 p2p: improve checkaddrman logging with duration in milliseconds (Jon Atack) ec65bed00ee2e403e39b3c5977caf4abd31ccc87 log, timer: add LOG_TIME_MILLIS_WITH_CATEGORY_MSG_ONCE macro (Jon Atack) 325da75a5396f3161a6eade74b349105ed5722ab log, timer: allow not repeating log message on completion (Jon Atack) Pull request description: This patch: - updates the `logging/timer.h::Timer` class to allow not repeating the log message on completion - adds a `LOG_TIME_MILLIS_WITH_CATEGORY_MSG_ONCE` macro that prints the descriptive message when logging the start but not when logging the completion - updates the checkaddrman logging to log the duration, and renames the function like the `-checkaddrman` configuration option in order to prefix every log message with `CheckAddrman` instead of the longer, less pleasant, and different-from-checkaddrman `ForceCheckAddrman` (the Doxygen documentation on the function already makes clear that it is unaffected by `m_consistency_check_ratio`). before ``` 2021-09-21T18:42:50Z [opencon] Addrman checks started: new 64864, tried 1690, total 66554 2021-09-21T18:42:50Z [opencon] Addrman checks completed successfully ``` after ``` 2021-09-21T18:42:50Z [opencon] CheckAddrman: new 64864, tried 1690, total 66554 started 2021-09-21T18:42:50Z [opencon] CheckAddrman: completed (76.21ms) ``` To test, build and run bitcoind with `-debug=addrman -checkaddrman=<n>` for a value of `n` in the range of, say, 10 to 40. ACKs for top commit: laanwj: Code review ACK 22b44fc696dc1078c40d17e2d497c74c7b4ae750 Tree-SHA512: 658c0dfaaa9d07092e2418f2d05007c58cc35be6593f22b3c592ce793334a885dd92dacc46bdeddc9d37939cf11174660a094c07c0fa117fbb282953aa45a94d
2021-11-01Merge bitcoin/bitcoin#23380: addrman: Fix AddrMan::Add() return semantics ↵fanquake
and logging 61ec0539b26a902a41a2602187a71f9dba3c6935 [MOVEONLY] reorder functions in addrman_impl.h and addrman.cpp (John Newbery) 2095df7b7bfcb9ab0c5710a93112f7f341e753c9 [addrman] Add Add_() inner function, fix Add() return semantics (John Newbery) 2658eb6d68460272deefb3fcc653b03f6ec6e7cf [addrman] Rename Add_() to AddSingle() (John Newbery) e58598e833d5737900fe3c4369e26f2a08166892 [addrman] Add doxygen comment to AddrMan::Add() (John Newbery) Pull request description: Previously, Add() would return true if the function created a new AddressInfo object, even if that object could not be successfully entered into the new table and was deleted. That would happen if the new table position was already taken and the existing entry could not be removed. Instead, return true if the new AddressInfo object is successfully entered into the new table. This fixes a bug in the "Added %i addresses" log, which would not always accurately log how many addresses had been added. ACKs for top commit: naumenkogs: ACK 61ec0539b26a902a41a2602187a71f9dba3c6935 mzumsande: ACK 61ec0539b26a902a41a2602187a71f9dba3c6935 shaavan: ACK 61ec0539b26a902a41a2602187a71f9dba3c6935 Tree-SHA512: 276f1e8297d4b6d411d05d06ffc7c176f6290a784da039926ab6c471a8ed8e9159ab4f56c893b1285737ae292954930f0d28012d89dfb3f2f825d7df41016feb
2021-10-28[MOVEONLY] reorder functions in addrman_impl.h and addrman.cppJohn Newbery
Keep the internal {Function}_() functions grouped together. Review with `git diff --color-moved=dimmed-zebra`
2021-10-28[addrman] Add Add_() inner function, fix Add() return semanticsJohn Newbery
Previously, Add() would return true if the function created a new AddressInfo object, even if that object could not be successfully entered into the new table and was deleted. That would happen if the new table position was already taken and the existing entry could not be removed. Instead, return true if the new AddressInfo object is successfully entered into the new table. This fixes a bug in the "Added %i addresses" log, which would not always accurately log how many addresses had been added. p2p_addrv2_relay.py and p2p_addr_relay.py need to be updated since they were incorrectly asserting on the buggy log (assuming that addresses are added to addrman, when there could in fact be new table position collisions that prevent some of those address records from being added).
2021-10-28[addrman] Rename Add_() to AddSingle()John Newbery
2021-10-25Introduce new V4 format addrmanPieter Wuille
92617b7a758c0425330fba4b886296730567927c effectively changed the on-disk format in an incompatible way: old deserializers cannot deal with multiple entries for the same IP. Introduce a V4_MULTIPORT format, and increment the compatibility base, so that old versions correctly recognize it as an incompatible future version.
2021-10-25Merge bitcoin/bitcoin#23306: Make AddrMan support multiple ports per IPMarcoFalke
92617b7a758c0425330fba4b886296730567927c Make AddrMan support multiple ports per IP (Pieter Wuille) Pull request description: For a long part of Bitcoin's history, this codebase has aggressively avoided making automatic connections to anything but nodes running on port 8333. I'd like to propose changing that, and this is a first PR necessary for that. The folklore justification (eventually actually added as a comment to the codebase in #20668) is that this is to prevent the Bitcoin P2P network from being leveraged to perform a DoS attack on other services, if their IP/port would get rumoured. It appears, at least the current network scale - and probably significantly larger - that the impact is very low at best (see calculations by vasild in https://github.com/bitcoin/bitcoin/issues/5150#issuecomment-853888909 e.g.). Another possible justification would be a risk that treating different IP:port combinations separately would help perform Eclipse attacks (by an attacker rumouring their own IP with many ports). This concern is (a) no different than what is possible with IPv6 (where large ranges of IP addresses are very cheaply available), and (b) already hopefully sufficiently addressed by addrman's design (which limits access through based selected based on network groups). And this policy has downsides too; in particular, a fixed port is easy to detect, and a very obvious sign a Bitcoin node is running there. One obstacle in moving away from a default port that is the fact that addrman is currently restricted to a single entry per IP address. If ports are no longer expected to be generally always the default one, we need to deal with the case where conflicting information is relayed. It turns out there is a very natural solution to this: treat (IP,port) combination exactly as we're treating IPs now; this automatically means that the same IP may appear with multiple ports, simply because those would be distinct entries. Given that indexing into addrman's bucket _already_ uses the port number, the only change required is making all addrman lookup be (IP,port) (aka `CService`) based, rather than IP (aka `CNetAddr`) based. This PR doesn't include any change to the actual outbound connection preference logic, as perhaps that's something that we want to phase in more gradually. ACKs for top commit: jnewbery: Code review ACK 92617b7a758c0425330fba4b886296730567927c naumenkogs: ACK 92617b7a758c0425330fba4b886296730567927c ajtowns: ACK 92617b7a758c0425330fba4b886296730567927c vasild: ACK 92617b7a758c0425330fba4b886296730567927c Tree-SHA512: 9eef06ce97a8b54a3f05fb8acf6941f253a9a5e0be8ce383dd05c44bb567cea243b74ee5667178e7497f6df2db93adab97ac66edbc37c883fd8ec840ee69a33f
2021-10-22Make AddrMan support multiple ports per IPPieter Wuille
2021-10-22Merge bitcoin/bitcoin#23140: Make CAddrman::Select_ select buckets, not ↵W. J. van der Laan
positions, first 632aad9e6d8369750f4327a886ca5b3d3fed89bd Make CAddrman::Select_ select buckets, not positions, first (Pieter Wuille) Pull request description: The original CAddrMan behaviour (before #5941) was to pick a uniformly random non-empty bucket, and then pick a random element from that bucket. That commit, which introduced deterministic placement of entries in buckets, changed this to picking a uniformly random non-empty bucket position instead. I believe that was a mistake. Buckets are our best metric for spreading out requests across independently-controlled nodes. That does mean that if a bucket has fewer entries, its entries are supposed to be picked more frequently. This PR reverts to the original high-level behavior, but on top of the deterministic placement logic. ACKs for top commit: jnewbery: utACK 632aad9e6d8369750f4327a886ca5b3d3fed89bd naumenkogs: ACK 632aad9e6d8369750f4327a886ca5b3d3fed89bd mzumsande: ACK 632aad9e6d8369750f4327a886ca5b3d3fed89bd Tree-SHA512: 60768afba2b6f0abd0dddff04381cab5acf374df48fc0e883ee16dde7cf7fd33056a04b573cff24a1b4d8e2a645bf0f0b3689eec84da4ff330e7b59ef142eca1
2021-10-19log: improve addrman loggingMartin Zumsande
2021-10-05p2p: improve checkaddrman logging with duration in millisecondsJon Atack
and update the function name to CheckAddrman (drop "Force") for nicer log output as it is prefixed to each of these log messages: 2021-09-21T18:42:50Z [opencon] CheckAddrman: new 64864, tried 1690, total 66554 started 2021-09-21T18:42:50Z [opencon] CheckAddrman: completed (76.21ms) The existing Doxygen documentation on the function already makes clear that it is unaffected by m_consistency_check_ratio.
2021-10-05Make CAddrman::Select_ select buckets, not positions, firstPieter Wuille
The original CAddrMan behaviour (before commit e6b343d880f50d52390c5af8623afa15fcbc65a2) was to pick a uniformly random non-empty bucket, and then pick a random element from that bucket. That commit, which introduced deterministic placement of entries in buckets, changed this to picking a uniformly random non-empty bucket position instead. This commit reverts the original high-level behavior, but in the deterministic placement model.
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
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-07 03:10:41 +0000