highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-11-12 15:09:59 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#21644: p2p, bugfix: use NetPermissions::HasFlag() in CConnman::Bind()

Browse Source 
36fb036d25 p2p: allow NetPermissions::ClearFlag() only with PF_ISIMPLICIT (Jon Atack)
4e0d5788ba test: add net permissions noban/download unit test coverage (Jon Atack)
dde69f20a0 p2p, bugfix: use NetPermissions::HasFlag() in CConnman::Bind() (Jon Atack)

Pull request description:

  This is a bugfix follow-up to #16248 and #19191 that was noticed in #21506. Both v0.21 and master are affected.

  Since #19191, noban is a multi-flag that implies download, so the conditional in `CConnman::Bind()` using a bitwise AND on noban will return the same result for both the noban status and the download status. This means that download peers are incorrectly not being added to local addresses because they are mistakenly seen as noban peers.

  The second commit adds unit test coverage to illustrate and test the noban/download relationship and the `NetPermissions` operations involving them.

  The final commit adds documentation and disallows calling `NetPermissions::ClearFlag()` with any second param other than `NetPermissionFlags` "implicit" -- per current usage in the codebase -- because `ClearFlag()` should not be called with any second param that is a subflag of a multiflag, e.g. "relay" or "download," as that would leave the result in an invalid state corresponding to none of the existing NetPermissionFlags. Thanks to Vasil Dimov for noticing this.

ACKs for top commit:
  theStack:
    re-ACK 36fb036d25 ☕
  vasild:
    ACK 36fb036d25
  hebasto:
    ACK 36fb036d25, I have reviewed the code and it looks OK, I agree it can be merged.
  kallewoof:
    Code review ACK 36fb036d25

Tree-SHA512: 5fbc7ddbf31d06b35bf238f4d77ef311e6b6ef2e1bb9893f32f889c1a0f65774a3710dcb21d94317fe6166df9334a9f2d42630809e7fe8cbd797dd6f6fc49491
This commit is contained in:
W. J. van der Laan
2021-05-11 12:51:41 +02:00
parent 94f83534e4 36fb036d25
commit e175a20769
4 changed files with 36 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
src/net.cpp
View File

@@ -2399,8 +2399,9 @@ NodeId CConnman::GetNewNodeId()
bool CConnman::Bind(const CService &addr, unsigned int flags, NetPermissionFlags permissions) {
if (!(flags & BF_EXPLICIT) && !IsReachable(addr))
if (!(flags & BF_EXPLICIT) && !IsReachable(addr)) {
return false;
}
bilingual_str strError;
if (!BindListenPort(addr, strError, permissions)) {
if ((flags & BF_REPORT_ERROR) && clientInterface) {
@@ -2409,7 +2410,7 @@ bool CConnman::Bind(const CService &addr, unsigned int flags, NetPermissionFlags
return false;
}
if (addr.IsRoutable() && fDiscover && !(flags & BF_DONT_ADVERTISE) && !(permissions & PF_NOBAN)) {
if (addr.IsRoutable() && fDiscover && !(flags & BF_DONT_ADVERTISE) && !NetPermissions::HasFlag(permissions, NetPermissionFlags::PF_NOBAN)) {
AddLocal(addr, LOCAL_BIND);
}