Skip to content

Commit ea1e5c2

Browse files
laanwjlaanwj
committed
Merge bitcoin/bitcoin#22025: refactor: Group and re-order CAddrMan members by access type
8caf60d move-only: Group and re-order CAddrMan members by access type (Hennadii Stepanov) 5cd7f8a refactor: Do not expose CAddrMan members as protected without need (Hennadii Stepanov) Pull request description: This PR is split from #19238 as all of its commits are trivial to review. The last commit is easy to review with `git diff --color-moved=dimmed-zebra`. Addressed the following comments from #19238: - bitcoin/bitcoin@130b82f#r550865131 > Can you consolidate all the private members and protected members to be next to each other? Multiple private and protected access specifiers make this harder to read than is necessary. - bitcoin/bitcoin@130b82f#r557271783 > Yeah, class declaration is easier to read if there is just one instance of `public:`, `protected:` and `private:` (in that order). ACKs for top commit: jnewbery: ACK 8caf60d laanwj: Code review ACK 8caf60d jarolrod: ACK 8caf60d vasild: ACK 8caf60d Tree-SHA512: e6127fc658da7876e36f22e2fae162dc19502ed7f8e931fdebc827dabd627e5346c6fbe6f6d0cd27fd3e5c96690ff35022ff6b48f2747b748ebd66a45c851c2b
2 parents e20745c + 8caf60d commit ea1e5c2

File tree

1 file changed

+132
-134
lines changed

1 file changed

+132
-134
lines changed

‎src/addrman.h

Lines changed: 132 additions & 134 deletions
Original file line numberDiff line numberDiff line change
@@ -172,139 +172,6 @@ static const int64_t ADDRMAN_TEST_WINDOW = 40*60; // 40 minutes
172172
*/
173173
class CAddrMan
174174
{
175-
friend class CAddrManTest;
176-
protected:
177-
//! critical section to protect the inner data structures
178-
mutable RecursiveMutex cs;
179-
180-
private:
181-
//! Serialization versions.
182-
enum Format : uint8_t {
183-
V0_HISTORICAL = 0, //!< historic format, before commit e6b343d88
184-
V1_DETERMINISTIC = 1, //!< for pre-asmap files
185-
V2_ASMAP = 2, //!< for files including asmap version
186-
V3_BIP155 = 3, //!< same as V2_ASMAP plus addresses are in BIP155 format
187-
};
188-
189-
//! The maximum format this software knows it can unserialize. Also, we always serialize
190-
//! in this format.
191-
//! The format (first byte in the serialized stream) can be higher than this and
192-
//! still this software may be able to unserialize the file - if the second byte
193-
//! (see `lowest_compatible` in `Unserialize()`) is less or equal to this.
194-
static constexpr Format FILE_FORMAT = Format::V3_BIP155;
195-
196-
//! The initial value of a field that is incremented every time an incompatible format
197-
//! change is made (such that old software versions would not be able to parse and
198-
//! understand the new file format). This is 32 because we overtook the "key size"
199-
//! field which was 32 historically.
200-
//! @note Don't increment this. Increment `lowest_compatible` in `Serialize()` instead.
201-
static constexpr uint8_t INCOMPATIBILITY_BASE = 32;
202-
203-
//! last used nId
204-
int nIdCount GUARDED_BY(cs);
205-
206-
//! table with information about all nIds
207-
std::map<int, CAddrInfo> mapInfo GUARDED_BY(cs);
208-
209-
//! find an nId based on its network address
210-
std::map<CNetAddr, int> mapAddr GUARDED_BY(cs);
211-
212-
//! randomly-ordered vector of all nIds
213-
std::vector<int> vRandom GUARDED_BY(cs);
214-
215-
// number of "tried" entries
216-
int nTried GUARDED_BY(cs);
217-
218-
//! list of "tried" buckets
219-
int vvTried[ADDRMAN_TRIED_BUCKET_COUNT][ADDRMAN_BUCKET_SIZE] GUARDED_BY(cs);
220-
221-
//! number of (unique) "new" entries
222-
int nNew GUARDED_BY(cs);
223-
224-
//! list of "new" buckets
225-
int vvNew[ADDRMAN_NEW_BUCKET_COUNT][ADDRMAN_BUCKET_SIZE] GUARDED_BY(cs);
226-
227-
//! last time Good was called (memory only)
228-
int64_t nLastGood GUARDED_BY(cs);
229-
230-
//! Holds addrs inserted into tried table that collide with existing entries. Test-before-evict discipline used to resolve these collisions.
231-
std::set<int> m_tried_collisions;
232-
233-
protected:
234-
//! secret key to randomize bucket select with
235-
uint256 nKey;
236-
237-
//! Source of random numbers for randomization in inner loops
238-
FastRandomContext insecure_rand;
239-
240-
//! Find an entry.
241-
CAddrInfo* Find(const CNetAddr& addr, int *pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs);
242-
243-
//! find an entry, creating it if necessary.
244-
//! nTime and nServices of the found node are updated, if necessary.
245-
CAddrInfo* Create(const CAddress &addr, const CNetAddr &addrSource, int *pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs);
246-
247-
//! Swap two elements in vRandom.
248-
void SwapRandom(unsigned int nRandomPos1, unsigned int nRandomPos2) EXCLUSIVE_LOCKS_REQUIRED(cs);
249-
250-
//! Move an entry from the "new" table(s) to the "tried" table
251-
void MakeTried(CAddrInfo& info, int nId) EXCLUSIVE_LOCKS_REQUIRED(cs);
252-
253-
//! Delete an entry. It must not be in tried, and have refcount 0.
254-
void Delete(int nId) EXCLUSIVE_LOCKS_REQUIRED(cs);
255-
256-
//! Clear a position in a "new" table. This is the only place where entries are actually deleted.
257-
void ClearNew(int nUBucket, int nUBucketPos) EXCLUSIVE_LOCKS_REQUIRED(cs);
258-
259-
//! Mark an entry "good", possibly moving it from "new" to "tried".
260-
void Good_(const CService &addr, bool test_before_evict, int64_t time) EXCLUSIVE_LOCKS_REQUIRED(cs);
261-
262-
//! Add an entry to the "new" table.
263-
bool Add_(const CAddress &addr, const CNetAddr& source, int64_t nTimePenalty) EXCLUSIVE_LOCKS_REQUIRED(cs);
264-
265-
//! Mark an entry as attempted to connect.
266-
void Attempt_(const CService &addr, bool fCountFailure, int64_t nTime) EXCLUSIVE_LOCKS_REQUIRED(cs);
267-
268-
//! Select an address to connect to, if newOnly is set to true, only the new table is selected from.
269-
CAddrInfo Select_(bool newOnly) EXCLUSIVE_LOCKS_REQUIRED(cs);
270-
271-
//! See if any to-be-evicted tried table entries have been tested and if so resolve the collisions.
272-
void ResolveCollisions_() EXCLUSIVE_LOCKS_REQUIRED(cs);
273-
274-
//! Return a random to-be-evicted tried table address.
275-
CAddrInfo SelectTriedCollision_() EXCLUSIVE_LOCKS_REQUIRED(cs);
276-
277-
#ifdef DEBUG_ADDRMAN
278-
//! Perform consistency check. Returns an error code or zero.
279-
int Check_() EXCLUSIVE_LOCKS_REQUIRED(cs);
280-
#endif
281-
282-
/**
283-
* Return all or many randomly selected addresses, optionally by network.
284-
*
285-
* @param[out] vAddr Vector of randomly selected addresses from vRandom.
286-
* @param[in] max_addresses Maximum number of addresses to return (0 = all).
287-
* @param[in] max_pct Maximum percentage of addresses to return (0 = all).
288-
* @param[in] network Select only addresses of this network (nullopt = all).
289-
*/
290-
void GetAddr_(std::vector<CAddress>& vAddr, size_t max_addresses, size_t max_pct, std::optional<Network> network) EXCLUSIVE_LOCKS_REQUIRED(cs);
291-
292-
/** We have successfully connected to this peer. Calling this function
293-
* updates the CAddress's nTime, which is used in our IsTerrible()
294-
* decisions and gossiped to peers. Callers should be careful that updating
295-
* this information doesn't leak topology information to network spies.
296-
*
297-
* net_processing calls this function when it *disconnects* from a peer to
298-
* not leak information about currently connected peers.
299-
*
300-
* @param[in] addr The address of the peer we were connected to
301-
* @param[in] nTime The time that we were last connected to this peer
302-
*/
303-
void Connected_(const CService& addr, int64_t nTime) EXCLUSIVE_LOCKS_REQUIRED(cs);
304-
305-
//! Update an entry's service bits.
306-
void SetServices_(const CService &addr, ServiceFlags nServices) EXCLUSIVE_LOCKS_REQUIRED(cs);
307-
308175
public:
309176
// Compressed IP->ASN mapping, loaded from a file when a node starts.
310177
// Should be always empty if no file was provided.
@@ -325,7 +192,6 @@ friend class CAddrManTest;
325192
// Read asmap from provided binary file
326193
static std::vector<bool> DecodeAsmap(fs::path path);
327194

328-
329195
/**
330196
* Serialized format.
331197
* * format version byte (@see `Format`)
@@ -759,6 +625,138 @@ friend class CAddrManTest;
759625
Check();
760626
}
761627

628+
protected:
629+
//! secret key to randomize bucket select with
630+
uint256 nKey;
631+
632+
//! Source of random numbers for randomization in inner loops
633+
FastRandomContext insecure_rand;
634+
635+
private:
636+
//! critical section to protect the inner data structures
637+
mutable RecursiveMutex cs;
638+
639+
//! Serialization versions.
640+
enum Format : uint8_t {
641+
V0_HISTORICAL = 0, //!< historic format, before commit e6b343d88
642+
V1_DETERMINISTIC = 1, //!< for pre-asmap files
643+
V2_ASMAP = 2, //!< for files including asmap version
644+
V3_BIP155 = 3, //!< same as V2_ASMAP plus addresses are in BIP155 format
645+
};
646+
647+
//! The maximum format this software knows it can unserialize. Also, we always serialize
648+
//! in this format.
649+
//! The format (first byte in the serialized stream) can be higher than this and
650+
//! still this software may be able to unserialize the file - if the second byte
651+
//! (see `lowest_compatible` in `Unserialize()`) is less or equal to this.
652+
static constexpr Format FILE_FORMAT = Format::V3_BIP155;
653+
654+
//! The initial value of a field that is incremented every time an incompatible format
655+
//! change is made (such that old software versions would not be able to parse and
656+
//! understand the new file format). This is 32 because we overtook the "key size"
657+
//! field which was 32 historically.
658+
//! @note Don't increment this. Increment `lowest_compatible` in `Serialize()` instead.
659+
static constexpr uint8_t INCOMPATIBILITY_BASE = 32;
660+
661+
//! last used nId
662+
int nIdCount GUARDED_BY(cs);
663+
664+
//! table with information about all nIds
665+
std::map<int, CAddrInfo> mapInfo GUARDED_BY(cs);
666+
667+
//! find an nId based on its network address
668+
std::map<CNetAddr, int> mapAddr GUARDED_BY(cs);
669+
670+
//! randomly-ordered vector of all nIds
671+
std::vector<int> vRandom GUARDED_BY(cs);
672+
673+
// number of "tried" entries
674+
int nTried GUARDED_BY(cs);
675+
676+
//! list of "tried" buckets
677+
int vvTried[ADDRMAN_TRIED_BUCKET_COUNT][ADDRMAN_BUCKET_SIZE] GUARDED_BY(cs);
678+
679+
//! number of (unique) "new" entries
680+
int nNew GUARDED_BY(cs);
681+
682+
//! list of "new" buckets
683+
int vvNew[ADDRMAN_NEW_BUCKET_COUNT][ADDRMAN_BUCKET_SIZE] GUARDED_BY(cs);
684+
685+
//! last time Good was called (memory only)
686+
int64_t nLastGood GUARDED_BY(cs);
687+
688+
//! Holds addrs inserted into tried table that collide with existing entries. Test-before-evict discipline used to resolve these collisions.
689+
std::set<int> m_tried_collisions;
690+
691+
//! Find an entry.
692+
CAddrInfo* Find(const CNetAddr& addr, int *pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs);
693+
694+
//! find an entry, creating it if necessary.
695+
//! nTime and nServices of the found node are updated, if necessary.
696+
CAddrInfo* Create(const CAddress &addr, const CNetAddr &addrSource, int *pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs);
697+
698+
//! Swap two elements in vRandom.
699+
void SwapRandom(unsigned int nRandomPos1, unsigned int nRandomPos2) EXCLUSIVE_LOCKS_REQUIRED(cs);
700+
701+
//! Move an entry from the "new" table(s) to the "tried" table
702+
void MakeTried(CAddrInfo& info, int nId) EXCLUSIVE_LOCKS_REQUIRED(cs);
703+
704+
//! Delete an entry. It must not be in tried, and have refcount 0.
705+
void Delete(int nId) EXCLUSIVE_LOCKS_REQUIRED(cs);
706+
707+
//! Clear a position in a "new" table. This is the only place where entries are actually deleted.
708+
void ClearNew(int nUBucket, int nUBucketPos) EXCLUSIVE_LOCKS_REQUIRED(cs);
709+
710+
//! Mark an entry "good", possibly moving it from "new" to "tried".
711+
void Good_(const CService &addr, bool test_before_evict, int64_t time) EXCLUSIVE_LOCKS_REQUIRED(cs);
712+
713+
//! Add an entry to the "new" table.
714+
bool Add_(const CAddress &addr, const CNetAddr& source, int64_t nTimePenalty) EXCLUSIVE_LOCKS_REQUIRED(cs);
715+
716+
//! Mark an entry as attempted to connect.
717+
void Attempt_(const CService &addr, bool fCountFailure, int64_t nTime) EXCLUSIVE_LOCKS_REQUIRED(cs);
718+
719+
//! Select an address to connect to, if newOnly is set to true, only the new table is selected from.
720+
CAddrInfo Select_(bool newOnly) EXCLUSIVE_LOCKS_REQUIRED(cs);
721+
722+
//! See if any to-be-evicted tried table entries have been tested and if so resolve the collisions.
723+
void ResolveCollisions_() EXCLUSIVE_LOCKS_REQUIRED(cs);
724+
725+
//! Return a random to-be-evicted tried table address.
726+
CAddrInfo SelectTriedCollision_() EXCLUSIVE_LOCKS_REQUIRED(cs);
727+
728+
#ifdef DEBUG_ADDRMAN
729+
//! Perform consistency check. Returns an error code or zero.
730+
int Check_() EXCLUSIVE_LOCKS_REQUIRED(cs);
731+
#endif
732+
733+
/**
734+
* Return all or many randomly selected addresses, optionally by network.
735+
*
736+
* @param[out] vAddr Vector of randomly selected addresses from vRandom.
737+
* @param[in] max_addresses Maximum number of addresses to return (0 = all).
738+
* @param[in] max_pct Maximum percentage of addresses to return (0 = all).
739+
* @param[in] network Select only addresses of this network (nullopt = all).
740+
*/
741+
void GetAddr_(std::vector<CAddress>& vAddr, size_t max_addresses, size_t max_pct, std::optional<Network> network) EXCLUSIVE_LOCKS_REQUIRED(cs);
742+
743+
/** We have successfully connected to this peer. Calling this function
744+
* updates the CAddress's nTime, which is used in our IsTerrible()
745+
* decisions and gossiped to peers. Callers should be careful that updating
746+
* this information doesn't leak topology information to network spies.
747+
*
748+
* net_processing calls this function when it *disconnects* from a peer to
749+
* not leak information about currently connected peers.
750+
*
751+
* @param[in] addr The address of the peer we were connected to
752+
* @param[in] nTime The time that we were last connected to this peer
753+
*/
754+
void Connected_(const CService& addr, int64_t nTime) EXCLUSIVE_LOCKS_REQUIRED(cs);
755+
756+
//! Update an entry's service bits.
757+
void SetServices_(const CService &addr, ServiceFlags nServices) EXCLUSIVE_LOCKS_REQUIRED(cs);
758+
759+
friend class CAddrManTest;
762760
};
763761

764762
#endif // BITCOIN_ADDRMAN_H

0 commit comments

Comments
 (0)