The problem with #20852 is that if a host is banned that cannot be
serialized in addrv1 format (e.g. Torv3) it is serialized as a dummy
IPv6 (::) in banlist.dat. So, upon restart, the ban entry is
lost (is deserialized as an invalid subnet that matches nothing).
revert #20852 net: allow CSubNet of non-IP networks #20926
pull vasild wants to merge 2 commits into bitcoin:master from vasild:revert_20852 changing 4 files +19 −123-
vasild commented at 2:51 PM on January 13, 2021: member
-
07aefe77ea
revert 39b432 net: allow CSubNet of non-IP networks
The problem with #20852 is that if a host is banned that cannot be serialized in addrv1 format (e.g. Torv3) it is serialized as a dummy IPv6 (`::`) in `banlist.dat`. So, upon restart, the ban entry is lost (is deserialized as an invalid subnet that matches nothing). This reverts commit 39b43298d9c54f9c18bef36f3d5934f57aefd088 from https://github.com/bitcoin/bitcoin/pull/20852.
-
e61023506e
revert 94d335 net: allow CSubNet of non-IP networks
The problem with #20852 is that if a host is banned that cannot be serialized in addrv1 format (e.g. Torv3) it is serialized as a dummy IPv6 (`::`) in `banlist.dat`. So, upon restart, the ban entry is lost (is deserialized as an invalid subnet that matches nothing). This reverts commit 94d335da7f8232bc653c9b08b0a33b517b4c98ad from https://github.com/bitcoin/bitcoin/pull/20852.
- fanquake added the label P2P on Jan 13, 2021
-
MarcoFalke commented at 2:56 PM on January 13, 2021: member
Why? Reverting the pull won't fix any issue. I presume the ban will either silently or verbosely fail, which seems worse than current master.
-
vasild commented at 2:59 PM on January 13, 2021: member
#20852 also aimed to fix a problem with
CConnman::DisconnectNode()not disconnecting non-IP hosts - this fix is proper. Banning a Torv2 also works properly through restart (Torv2 addresses can be serialized in addrv1 format in 16 bytes).However banning a Torv3 host and saving it to
banlist.datrequires a file format change. -
MarcoFalke commented at 3:13 PM on January 13, 2021: member
However banning a Torv3 host and saving it to banlist.dat requires a file format change.
If you are worried that someone is running master and doesn't see the silent fail (reading the banlist file), you can turn it into a verbose fail. Though I'd rather prefer to spend the resources on fixing the file format, so that we have something to backport to 0.21.1 hopefully.
-
vasild commented at 3:16 PM on January 13, 2021: member
Alright, lets leave it as is.
http://www.erisian.com.au/bitcoin-core-dev/log-2021-01-13.html#l-190
- vasild closed this on Jan 13, 2021
- vasild deleted the branch on Jan 13, 2021
- DrahtBot locked this on Aug 16, 2022
Contributors
Labels