< bitcoin-git> [bitcoin] svannord opened pull request #19196: Create Starting Out (master...patch-1) https://github.com/bitcoin/bitcoin/pull/19196
< bitcoin-git> [bitcoin] fanquake closed pull request #19196: Create Starting Out (master...patch-1) https://github.com/bitcoin/bitcoin/pull/19196
< phantomcircuit> sipa, for the asmap stuff is there a simple "is this ip in this asn?" function?
< sipa> phantomcircuit: my asmap tool PR has a lookup function
< sipa> i also have python code that can do the same
< sipa> #18573
< gribble> https://github.com/bitcoin/bitcoin/issues/18573 | [RFC] bitcoin-asmap utility by sipa · Pull Request #18573 · bitcoin/bitcoin · GitHub
< sipa> also testmap.py in https://github.com/sipa/asmap
< sipa> phantomcircuit: or you mean inside bitcoin core codebase? GetMappedAs in CNetAddr
< bitcoin-git> [bitcoin] fanquake opened pull request #19197: init: use std::thread for ThreadImport() (master...thread_import_no_boost) https://github.com/bitcoin/bitcoin/pull/19197
< phantomcircuit> sipa, i mean for banning by AS
< phantomcircuit> sipa, is there a reason the asmap isn't a global? seems like something that should be a global
< sipa> phantomcircuit: i think connman or addrman is a reasonable place for.it
< sipa> i forget where it's held
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/b1b173994406...1b90a7b61a37
< bitcoin-git> bitcoin/master 501e6ab Calvin Kim: doc: Add documentation for 'checklevel' argument in 'verifychain' RPC call
< bitcoin-git> bitcoin/master 1b90a7b MarcoFalke: Merge #19005: doc: Add documentation for 'checklevel' argument in 'verifyc...
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #19005: doc: Add documentation for 'checklevel' argument in 'verifychain' RPC… (master...add-documentation-for-verifychain) https://github.com/bitcoin/bitcoin/pull/19005
< bitcoin-git> [bitcoin] MarcoFalke opened pull request #19198: test: Check that peers with forcerelay permission are not asked to feefilter (master...2006-testForcerelayFeefilter) https://github.com/bitcoin/bitcoin/pull/19198
< bitcoin-git> [bitcoin] jonatack opened pull request #19200: rpc: remove deprecated getaddressinfo fields (master...remove-deprecated-getaddressinfo-fields) https://github.com/bitcoin/bitcoin/pull/19200
< bitcoin-git> [bitcoin] MarcoFalke opened pull request #19201: ci: Switch to bitcoincore.org download (master...2006-ci404) https://github.com/bitcoin/bitcoin/pull/19201
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/1b90a7b61a37...43695b0cf851
< bitcoin-git> bitcoin/master fa4cd1f MarcoFalke: ci: Switch to bitcoincore.org download
< bitcoin-git> bitcoin/master 43695b0 MarcoFalke: Merge #19201: ci: Switch to bitcoincore.org download
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #19201: ci: Switch to bitcoincore.org download (master...2006-ci404) https://github.com/bitcoin/bitcoin/pull/19201
< bitcoin-git> [bitcoin] jonatack opened pull request #19202: log: remove deprecated `db` log category (master...remove-deprecated-db-log-category) https://github.com/bitcoin/bitcoin/pull/19202
< bitcoin-git> [bitcoin] MarcoFalke pushed 3 commits to master: https://github.com/bitcoin/bitcoin/compare/43695b0cf851...b3091b2be7d1
< bitcoin-git> bitcoin/master 4c0c893 Jon Atack: log: remove deprecated `db` log category
< bitcoin-git> bitcoin/master c514a4f Jon Atack: doc: release note for `db` log category removal
< bitcoin-git> bitcoin/master b3091b2 MarcoFalke: Merge #19202: log: remove deprecated `db` log category
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #19202: log: remove deprecated `db` log category (master...remove-deprecated-db-log-category) https://github.com/bitcoin/bitcoin/pull/19202
< phantomcircuit> sipa, can i get all of the subnets in an asn from the asmap?
< sipa> not without decoding the entire thing
< phantomcircuit> it looks like no
< sipa> but that's still pretty fast
< phantomcircuit> that makes adding asn based bans much harder but yeah definitely still doable
< sipa> ah i see
< sipa> you want to translate banned ASes to subnets
< phantomcircuit> would it be reasonable to do that when adding asn bans? cause that changes it to like two lines of code from many more
< luke-jr> maybe the banlist should support ASNs directly?
< sipa> that would certainly be cleaner
< sipa> but i see how that would be harder to implement
< sipa> i think if banlists on disk supported ASes natively, but at runtime they're converted to subnets, maybe it's ok?
< phantomcircuit> i can definitely support asn's directly instead, it's not something where one approach is obviously better to me though
< sipa> so that they correctly persist across restarts if asmap gets updated
< phantomcircuit> adding support naively will break backwards compat with banlist.dat though
< luke-jr> sipa: nah imo
< luke-jr> sipa: setban should support adding/removing AS bans, without affecting overlapping subnet bans, IMO
< sipa> luke-jr: that's reasonable
< luke-jr> plus we might want to update ASmaps at runtime someday
< sipa> phantomcircuit: i think it's expected that banlist.dat compatibility breaks when you ban an AS - there is no way for an old version to behave correctly
< sipa> you wouldn't want to run with a partial set of bans
< phantomcircuit> sipa, no i mean doing this naively would mean clearing all your old bans
< sipa> why?
< luke-jr> hmm
< luke-jr> sipa: do we have a way to sanely handle an incompatible banlist in current/old versions?
< sipa> no clue
< luke-jr> losing the banlist entirely would be strictly worse than just losing AS bans
< sipa> i have never looked at its serialization
< sipa> agree
< sipa> well
< phantomcircuit> sipa, banlist.dat is basically just a serialized banmap_t which is a map of subnets
< sipa> i think that if you add an AS ban, you should not lose existing subnet bans
< sipa> but if you add an AS ban, and then downgrade to a version that does not support AS bans, ideally it just fails to start
< sipa> phantomcircuit: banlist serialized entries have a version number
< sipa> so there is no problem
< sipa> just increment the version number of AS-based bans; but keep supporting the old subnet-only one in the deserializer
< sipa> unfortunately the version number is not enforced
< sipa> so old versions with read corrupted data
< luke-jr> looks like if we change the root type (currently map<CSubNet, CBanEntry>) old versions should refuse to load it
< phantomcircuit> that means changing a bunch of the logic for serialize/deserialize
< luke-jr> and refuse to start
< phantomcircuit> or i could just map asn's to subnet and it's potentially much less code and compatible...
< luke-jr> sipa: there's a checksum to stop corrupted data
< luke-jr> phantomcircuit: could make a new type that serialises the same for subnets?
< luke-jr> and just serialise ::0/0 followed by AS info for anAS ban
< luke-jr> the checksum should stop it from loading that as ::0/0 in old versions
< luke-jr> (possibly stored as ::ASN/0 in memory to avoid wasting extra RAM
< sipa> phantomcircuit: i'll have a look at making the ser/deser code more upgradable
< phantomcircuit> sipa, the addrdb and bandb share the ser/deser logic to do this they probably just need to be split
< sipa> phantomcircuit: perhaps
< sipa> phantomcircuit: std::map is serialized with a size prefix
< sipa> so we could change the serialization to stary with a giant number
< sipa> to indicate a new version
< sipa> so that it will fail to load on old versions
< sipa> *start
< phantomcircuit> just start with a dummy db that has a bad checksum
< sipa> that's also possible
< phantomcircuit> that seems like it would be more obvious
< phantomcircuit> also changing banmap_t is going to effect a whole bunch of logic
< sipa> i know
< sipa> but that code is due for a cleanup anyway, o think
< sipa> *i
< luke-jr> …
< luke-jr> these all sound much more complex than what I just suggested :P
< sipa> which is?
< luke-jr> [17:29:56] <luke-jr> and just serialise ::0/0 followed by AS info for anAS ban
< sipa> that won't work
< luke-jr> why not?
< sipa> it will result in old software reading corrupted datae
< luke-jr> which will fail the checksum
< sipa> ah, indeed!
< sipa> so you'd need a new data type instead of CSubNet to use as key in the map
< sipa> which can represent either a CSubNet or an ASN
< luke-jr> yes
< sipa> and csubnet's remain serialized as-is
< luke-jr> exactly
< sipa> and ASN are serialized as some sentinel value that's unlikely to be already present in banmans (::0/0 seems right as luke-jr suggests) + version number + ASN
< luke-jr> I mean, we could use the CSubNet part for ASN data too, so long as we don't end up with the same length
< sipa> it needs to be recognizable as an AS entry to the deserializer
< sipa> CSubNets do not have an explicit length in their serialization afaik
< luke-jr> ah
< phantomcircuit> uh whuch checksum would that fail?
< phantomcircuit> i dont see any reason that would fail the checksum in DeserializeDB
< sipa> phantomcircuit: because the data deserialized wouldn't match
< sipa> so it would read a checksum in the wrong place
< phantomcircuit> i mean we can just append the asn stuff then and the checksum will be wrong that way to
< sipa> yes, but how would a deserializer notice that there is more stuff to follow?
< sipa> it would need to distinguish ban entries from the checksum...
< phantomcircuit> start/data/data/hash and try to read data as both a hash and a map of asns
< sipa> that's pretty ugly i think, but yes it will work
< sipa> i think it's a bigger change
< phantomcircuit> sipa, could equally just have v1/v2 deserializers
< sipa> phantomcircuit: and try both?
< phantomcircuit> yeah
< sipa> well, see it this way: i think you're going to need a data type that represents "subnet or ASN" anyway, to put in the banmap map as key
< sipa> if you give that data type a serialization that's identical to a subnet's for subsets, and to some sentinal + ASN for asns, you have no other serialization issues
< sipa> *sentinel
< phantomcircuit> yeah that's a good point
< phantomcircuit> why is it `using banman_t = std::map...` and not a typedef ?
< sipa> using is the newer syntax
< sipa> it does the same as a typedef, but is more flexible
< phantomcircuit> ok so they can still be templated where typedef cannot be
< phantomcircuit> interesting
< phantomcircuit> sipa, oh CSubNet has a bool is valid flag
< sipa> oh, and it's serialized!
< sipa> so all you need to do is serialize ASN entries as something that gets read as valid=false by old nodes, and has a different length
< luke-jr> nice
< phantomcircuit> CSubNetOrAsn ?
< phantomcircuit> CNetworkBlock?
< sipa> BanMatch ?
< phantomcircuit> im bad at naming things
< sipa> (no more C prefix for classes)
< phantomcircuit> banmatch is good
< phantomcircuit> asn's are being represented just as a 32bit number right
< sipa> yeah
< bitcoin-git> [bitcoin] practicalswift opened pull request #19203: tests/net: Add regression fuzz harness for CVE-2017-18350. Add FuzzedSocket. Add thin SOCKET wrapper. (master...fuzzers-Socks5) https://github.com/bitcoin/bitcoin/pull/19203
< bitcoin-git> [bitcoin] MarcoFalke opened pull request #19204: p2p: Reduce inv traffic during IBD (master...2006-netInvWaste) https://github.com/bitcoin/bitcoin/pull/19204