aboutsummaryrefslogtreecommitdiff
path: root/src/addrman.h
AgeCommit message (Collapse)Author
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-09[addrman] Remove AddrMan friendsJohn Newbery
AddrMan's friends both inherit from AddrMan, so just make the private member protected and remove the friends.
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] Add doxygen comment to AddrMan::Add()John Newbery
Does not document the return value since we're going to fix the semantics in a future commit.
2021-09-28[style] Run changed files through clang formatter.Amiti Uttarwar
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[move-only] Move constants to test-only headerAmiti Uttarwar
Review hint: git diff --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
2021-09-28[move-only] Move CAddrInfo to test-only header fileAmiti Uttarwar
Now that no bitcoind callers require knowledge of the CAddrInfo object, it can be moved into the test-only header file. Review hint: use git diff --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
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-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-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-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-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-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-19[addrman] Remove all public uses of CAddrMan.Clear() from the testsJohn Newbery
Just use unique_ptr<CAddrMan>s and reset the pointer if a frest addrman is required. Also make CAddrMan::Clear() private to ensure that no call sites are missed.
2021-08-15[addrman] Merge the two Add() functionsAmiti Uttarwar
Merge the two definitions of this overloaded function to reduce code duplication.
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-05[tests] Make deterministic addrman use nKey = 1John Newbery
addrman_tests fail when consistency checks are enabled, since the tests set the deterministic test addrman's nKey value to zero, which is an invalid value. Change this so that deterministic addrman's nKey value is set to 1. This requires updating a few tests that are using magic values derived from nKey being set to 0.
2021-08-05[addrman] Add deterministic argument to CAddrMan ctorJohn Newbery
Removes the need for tests to update nKey and insecure_rand after constructing a CAddrMan.
2021-08-05Add missing const to CAddrMan::Check_()MarcoFalke
Also: Always compile the function signature to avoid similar issues in the future.
2021-08-05Merge bitcoin/bitcoin#21129: fuzz: check that ser+unser produces the same ↵MarcoFalke
AddrMan 87651795d8622d354f8e3c481eb868d9433b841c fuzz: check that ser+unser produces the same AddrMan (Vasil Dimov) 6408b24517f3418e2a408071b4c2ce26571f3167 fuzz: move init code to the CAddrManDeterministic constructor (Vasil Dimov) Pull request description: Add a fuzz test that fills addrman with a pile of randomly generated addresses, serializes it to a stream, unserializes the stream to another addrman object and compares the two. Some discussion of this already happened at https://github.com/jnewbery/bitcoin/pull/18. ACKs for top commit: practicalswift: cr ACK 87651795d8622d354f8e3c481eb868d9433b841c jonatack: ACK 87651795d8622d354f8e3c481eb868d9433b841c rebased to current master, reviewed, fuzz build, ran `FUZZ=addrman_serdeser src/test/fuzz/fuzz` Tree-SHA512: 7eda79279f14f2649840bf752e575d7b02cbaad541f74f7254855ebd4a32da988f042d78aa9228983350283bb74dd0c71f51f04c0846889c3ba2f19f01a0c303
2021-08-04fuzz: check that ser+unser produces the same AddrManVasil Dimov
2021-08-04Merge bitcoin/bitcoin#22576: doc: Update high-level addrman descriptionfanquake
036d7eadf5dd0e06e0734a5d78dbe28f4bfaa07f doc: Correct description of CAddrMan::Create() (Amiti Uttarwar) 318176aff1ded36d1fbc5977f288ac3bac1d8712 doc: Update high-level addrman description (Martin Zumsande) Pull request description: The high-level description of `addrman` has outdated information with respect to the eviction behavior, both for the New and Tried tables (at least since #5941) - this has confused me in the past. This PR corrects this and also adds basic info about the bucket size and position. ACKs for top commit: amitiuttarwar: reACK 036d7eadf5dd0e06e0734a5d78dbe28f4bfaa07f jnewbery: ACK 036d7eadf5dd0e06e0734a5d78dbe28f4bfaa07f Tree-SHA512: 3f0635d765f5e580a1fae31187742a833cef66ef2286d40eeb28f2253521260038e16e5f1a65741464a2ddfdbeb5c0f1bc38bf73841e600639033d59c3c534e4
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-08-02doc: Correct description of CAddrMan::Create()Amiti Uttarwar
Co-authored-by: Martin Zumsande <mzumsande@gmail.com>
2021-08-02doc: Update high-level addrman descriptionMartin Zumsande
2021-07-23Add missing GUARDED_BY to CAddrMan::insecure_randMarcoFalke
2021-07-21refactor: Mark CAddrMan::GetAddr constMarcoFalke
2021-07-21refactor: Mark CAddrMan::Select constMarcoFalke
2021-07-20[addrman] Remove unused test_before_evict argument from Good()John Newbery
This has never been used in the public interface method since it was introduced in #9037.
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.
generated by cgit v1.2.3 (git 2.26.2) at 2024-07-05 17:55:37 +0000