< midnight> luke-jr: Strongly recommend not visiting that site without VPN or Tor.
< luke-jr> midnight: eh?
< midnight> ... that site. Unless I'm mistaking who that is, strongly recommend you don't visit it withut a VPN or Tor.
< luke-jr> no JS on it.. and content looks reasonable O.o
< midnight> Yep, I know.
< sipa> aj: you can resolve, i think :)
< aj> sipa: yep, just checking you didn't leave it open for some reason i missed
< bitcoin-git> [bitcoin] fanquake pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/655937ebcbf6...5db44c740ebd
< bitcoin-git> bitcoin/master b3972bc Hennadii Stepanov: doc: Mention signet in -help output
< bitcoin-git> bitcoin/master 5db44c7 fanquake: Merge #20014: doc: Mention signet in -help output
< bitcoin-git> [bitcoin] fanquake merged pull request #20014: doc: Mention signet in -help output (master...200925-signet) https://github.com/bitcoin/bitcoin/pull/20014
< bitcoin-git> [bitcoin] fanquake pushed 3 commits to master: https://github.com/bitcoin/bitcoin/compare/5db44c740ebd...7ea649946daa
< bitcoin-git> bitcoin/master fae243f MarcoFalke: test: Remove confusing cast to same type (int to int)
< bitcoin-git> bitcoin/master faa94cb MarcoFalke: test: Check that invalid peer traffic is accounted for
< bitcoin-git> bitcoin/master 7ea6499 fanquake: Merge #20028: test: Check that invalid peer traffic is accounted for
< bitcoin-git> [bitcoin] fanquake merged pull request #20028: test: Check that invalid peer traffic is accounted for (master...2009-testInvalidTraffic) https://github.com/bitcoin/bitcoin/pull/20028
< bitcoin-git> [bitcoin] fanquake closed pull request #14729: correct -onion default to -proxy behavior (master...qubenix-proxyfix) https://github.com/bitcoin/bitcoin/pull/14729
< bitcoin-git> [bitcoin] fanquake pushed 5 commits to master: https://github.com/bitcoin/bitcoin/compare/7ea649946daa...e36aa351a31c
< bitcoin-git> bitcoin/master efc9b85 Sjors Provoost: Mark send RPC experimental
< bitcoin-git> bitcoin/master 0fc1c68 Sjors Provoost: [rpc] send: fix parsing replaceable option
< bitcoin-git> bitcoin/master d813d26 Sjors Provoost: [rpc] send: various touch-ups
< bitcoin-git> [bitcoin] fanquake merged pull request #19969: Send RPC bug fix and touch-ups (master...2020/09/send_touchups) https://github.com/bitcoin/bitcoin/pull/19969
< bitcoin-git> [bitcoin] fanquake pushed 7 commits to master: https://github.com/bitcoin/bitcoin/compare/e36aa351a31c...6af9b31bfc6d
< bitcoin-git> bitcoin/master 2716647 Troy Giorshev: Give V1TransportDeserializer an m_node_id member
< bitcoin-git> bitcoin/master 890b1d7 Troy Giorshev: Move checksum check from net_processing to net
< bitcoin-git> bitcoin/master 1ca20c1 Troy Giorshev: Add doxygen comment for ReceiveMsgBytes
< bitcoin-git> [bitcoin] fanquake merged pull request #19107: p2p: Move all header verification into the network layer, extend logging (master...p2p-refactor-header) https://github.com/bitcoin/bitcoin/pull/19107
< bitcoin-git> [bitcoin] fanquake pushed 6 commits to master: https://github.com/bitcoin/bitcoin/compare/6af9b31bfc6d...ec9b4492eba5
< bitcoin-git> bitcoin/master dba8196 Antoine Poinsot: policy/fees: correct decay explanation comments
< bitcoin-git> bitcoin/master 5b8cb35 Antoine Poinsot: policy/fee: remove requireGreater parameter in EstimateMedianVal()
< bitcoin-git> bitcoin/master 569d92a Antoine Poinsot: policy/fees: small readability improvements
< bitcoin-git> [bitcoin] fanquake merged pull request #19630: Cleanup fee estimation code (master...20/07/refactor_feeest_code) https://github.com/bitcoin/bitcoin/pull/19630
< bitcoin-git> [bitcoin] vasild closed pull request #19031: Implement ADDRv2 support (part of BIP155) (master...addrv2) https://github.com/bitcoin/bitcoin/pull/19031
< bitcoin-git> [bitcoin] jnewbery closed pull request #17566: Switch to weight units for all feerates computation (master...feerate_in_weight) https://github.com/bitcoin/bitcoin/pull/17566
< bitcoin-git> [bitcoin] darosior closed pull request #16890: rpc: Don't allow to 'estimatesmartfee' in blocksonly mode (master...estimatesmartfee_blockonly) https://github.com/bitcoin/bitcoin/pull/16890
< bitcoin-git> [bitcoin] vasild opened pull request #20033: style: minor improvements as a followup to #19845 (master...nits_followup_to_19845) https://github.com/bitcoin/bitcoin/pull/20033
< bitcoin-git> [bitcoin] jnewbery closed pull request #15206: Immediately disconnect on invalid net message checksum (master...2019/01/netmsg_2) https://github.com/bitcoin/bitcoin/pull/15206
< bitcoin-git> [bitcoin] jnewbery closed pull request #15197: Refactor and slightly stricter p2p message processing (master...2019/01/netmsg_1) https://github.com/bitcoin/bitcoin/pull/15197
< bitcoin-git> [bitcoin] ryanofsky opened pull request #20034: test: Get rid of default wallet hacks (master...pr/defw) https://github.com/bitcoin/bitcoin/pull/20034
< vasild> hebasto: I am looking at https://github.com/bitcoin/bitcoin/pull/19991#discussion_r495073463 -- that wouldn't allow to configure the two things independently.
< vasild> I think we should allow for "auto create tor service: yes/no" and independently of that "do extra bind and consider incoming connections to that as incoming tor: bind_addr:bind_port"
< vasild> so that it is possible to configure all 4 possibilities: auto create+extra bind, auto create+no extra bind, no auto create+extra bind, no auto create+no extra bind
< bitcoin-git> [bitcoin] MarcoFalke opened pull request #20035: signet: Fix uninitialized read in validation (master...2009-signetUninitRead) https://github.com/bitcoin/bitcoin/pull/20035
< bitcoin-git> [bitcoin] fanquake pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/ec9b4492eba5...d82b2c6e65ff
< bitcoin-git> bitcoin/master 62dba96 nthumann: log: print unexpected version warning in validation log category
< bitcoin-git> bitcoin/master d82b2c6 fanquake: Merge #19898: log: print unexpected version warning in validation log cate...
< bitcoin-git> [bitcoin] fanquake merged pull request #19898: log: print unexpected version warning in validation log category (master...log-fix-unexpected-version) https://github.com/bitcoin/bitcoin/pull/19898
< bitcoin-git> [bitcoin] MarcoFalke opened pull request #20036: signet: Add assumed values for default signet (master...2009-signetAssumed) https://github.com/bitcoin/bitcoin/pull/20036
< Bullit> What happened last thing it said at 33% block load ¨i´ll warn you again¨ then Bitcoin Core QT Shutdown during start up full chain is running again on laptop, please don´t make me a node see my energydrink company
< bitcoin-git> [bitcoin] laanwj pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/d82b2c6e65ff...8aa617896139
< bitcoin-git> bitcoin/master 9b4fa0a practicalswift: net: Print error message if -proxy is specified without arguments (instead...
< bitcoin-git> bitcoin/master 8aa6178 Wladimir J. van der Laan: Merge #20003: net: Exit with error message if -proxy is specified without ...
< bitcoin-git> [bitcoin] laanwj merged pull request #20003: net: Exit with error message if -proxy is specified without arguments (instead of continuing without proxy server) (master...error-on-empty-proxy) https://github.com/bitcoin/bitcoin/pull/20003
< bitcoin-git> [bitcoin] laanwj pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/8aa617896139...9e6f56f6ea0f
< bitcoin-git> bitcoin/master f22d6a1 practicalswift: log: Remove static log message "Initializing chainstate Chainstate [ibd] @...
< bitcoin-git> bitcoin/master 9e6f56f Wladimir J. van der Laan: Merge #19984: log: Remove static log message "Initializing chainstate Chai...
< bitcoin-git> [bitcoin] laanwj merged pull request #19984: log: Remove static log message "Initializing chainstate Chainstate [ibd] @ height -1 (null)" (master...logging-should-not-be-static-department) https://github.com/bitcoin/bitcoin/pull/19984
< wumpus> the signet default network onion seed seems to be offline (ntv3mtqw5wt63red.onion:38333)
< hebasto> vasild: agree
< wumpus> kallewoof ^^
< kallewoof> wumpus: huh. ping provoostenator as he's running it
< bitcoin-git> [bitcoin] laanwj pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/9e6f56f6ea0f...de4b7f25acef
< bitcoin-git> bitcoin/master 8a4dcda gr0kchain: doc: Added default signet config for linearize script
< bitcoin-git> bitcoin/master de4b7f2 Wladimir J. van der Laan: Merge #20015: doc: Added default signet config for linearize script
< bitcoin-git> [bitcoin] laanwj merged pull request #20015: doc: Added default signet config for linearize script (master...master) https://github.com/bitcoin/bitcoin/pull/20015
< wumpus> kallewoof: ohh whoops I didn't know that
< kallewoof> wumpus: i will set up an onion seed on my side as well to make it a bit more stable
< wumpus> having at least two would be nice, I noticed because I tried to use signet on a onlynet=onion node :)
< kallewoof> wumpus: yeah, didn't really cover that very well. :P
< roconnor> luke-jr: I found 10 lines of miscompiled code on my desktop system. You can read my report @ http://r6.ca/blog/20200929T023701Z.html
< luke-jr> roconnor: you don't use Twitter, do you? (if you do, I'll retweet it)
< roconnor> I don't use twitter.
< roconnor> you can share it however you like.
< jonatack> very helpful report roconnor, thank you
< luke-jr> indeed, thanks
< luke-jr> annoyingly, icu is commonly static linked I think
< luke-jr> possibly into Bitcoin-Qt too, though I can't see any way one would exploit this
< wumpus> going to test with both #19954 and #19991, surprisingly it's a clean merge
< gribble> https://github.com/bitcoin/bitcoin/issues/19954 | tor: complete the TORv3 implementation by vasild · Pull Request #19954 · bitcoin/bitcoin · GitHub
< gribble> https://github.com/bitcoin/bitcoin/issues/19991 | net: Use alternative port for incoming Tor connections by hebasto · Pull Request #19991 · bitcoin/bitcoin · GitHub
< bitcoin-git> [bitcoin] prayank23 opened pull request #20039: test: Convert amounts from float to decimal (master...floatdecimal) https://github.com/bitcoin/bitcoin/pull/20039
< bitcoin-git> [bitcoin] achow101 opened pull request #20040: wallet: Refactor OutputGroups to handle fees and spending eligibility on insert (master...inner-groupoutputs) https://github.com/bitcoin/bitcoin/pull/20040
< roconnor> luke-jr: yeah, the icu thing seems fairly benign. There are no unicode versions below, so I don't know how you would coax a problem out of the versionFilter.
< roconnor> luke-jr: I think I saw the icu code crop up about 3 times.
< dburkett> I understand (somewhat) the hesitation to refactor the code to add a little modularity, but there seems to be quite a bit of low-hanging fruit that routinely gets passed up.
< dburkett> Here's a quick example: https://github.com/bitcoin/bitcoin/blob/d82b2c6e65ff1ade508e2e8abeb07d0ff37e09c4/src/validation.cpp#L1128-L1238 doesn't seem to be related to validation at all, and looks like an obvious candidate to move to some sort of BlockStore class or codefile.
< dburkett> Do we avoid this kind of refactoring due simply to a lack of resources? Or is there some other reasoning that I'm missing?
< sipa> dburkett: these are functions that are already only used from within validation.cpp, so i don't think there is particularly much to gain in modularity from moving them elsewhere
< sipa> not saying it wouldn't be cleaner, but refactorings cost review time (and can be frustrating when people aren't excited enough to spend much time on it) - especially for things close to validation
< luke-jr> dburkett: something like https://github.com/bitcoin/bitcoin/pull/771 ?
< luke-jr> and in addition to sipa's points, it also adds rebase time to other PRs
< dburkett> It feels like we would have benefits in not only readability, but also testability if we componentize things like that.
< luke-jr> dburkett: I'm not saying there aren't benefits
< sipa> dburkett: yeah, no objection there - but everything has costs too
< luke-jr> if there was one perfect person just refactoring trusted to not break anything, it'd be easy to just get it done
< dburkett> luke-jr: yes, something much like that
< luke-jr> but even without malice, people can make mistakes, so everything needs review
< sipa> luke-jr: cblockstore was arguably much more of what is now validationinterface, just not including the actual storage of blocks :)
< luke-jr> sipa: I couldn't think of a better match :P
< sipa> but sure, as evidence that people have tried this before, in various levels of ambitiousness
< dburkett> sipa: I completely understand everything has costs. I guess my question is, if I did small, careful, and targeted refactoring like this, do you think it's something that would provide enough benefit to others to get the necessary review?
< sipa> dburkett: it would be very low on my personal priority list
< sipa> i can't speak about others'
< sipa> but making it small and targetted is certainly easier
< sipa> i don't mean to discourage you, though!
< dburkett> Gotcha. Thanks for the honesty. Bitcoin development has often scared myself and many others away in the past due to the lack of readability and organization of the code.
< sipa> dburkett: it depends on the kind of refactor, but many "innocent" refactors in the past have made things much harder to find for me
< sipa> this is especially true for things that started with a nice ambitious goal, but only ever got partially completed, leaving things in a less consistent state than they were before
< dburkett> But having smaller components to reason about and work on in isolation helps improve that. I may take a stab at some of the smaller refactoring to see how it goes
< jonatack> dburkett: speaking personally, when deciding between (a) proposing changes, and (b) reviewing important open proposals that have been sitting for months or years due to lack of review, the truth is that few to no cases of (a) add as much value as doing (b)
< luke-jr> maybe have the end goal completed, but PR part at a time?
< jonatack> so it's all relative...question of prorities and opportunity cost
< luke-jr> jonatack: good point
< dburkett> luke-jr: seems like a solid approach
< luke-jr> dburkett: in addition to resolving bottlenecks, review also helps you learn the code ;)
< dburkett> jonatack: I completely understand, though I think a lot of the more experienced devs like yourself downplay the difficulty of learning such a large and complex codebase, which could contribute to why we struggle to get reviewers. Every change to a 3000 line file just seems so intimidating to those not intimately familiar with it lol
< jonatack> dburkett: i consider myself a new dev here :)
< luke-jr> XD
< sipa> dburkett: i'm no reference in this context, but i tend to find a project with hundreds of small files far more intimidating than one big one
< sipa> i can't explain why
< dburkett> btw, I do have a reasonable grasp on the code, only because I invested the significant time to learn it. I agree review helps learn it as well. I'm merely sharing my experiences from my own time learning the code.
< sipa> i guess it all has to do with structure; you could have a clusterfuck of small files, or a clusterfuck of a large file, and either can have very unclear responsibilities/structure
< sipa> but needing to look through many files, not knowing what they may do, just feels scarier to me than one big thing to read through :)
< dburkett> sipa: I will speculate that it's because you're an old-time Linux hacker (not calling you old), rather than a modern corporate dev. There's certainly advantages to each approach, and both can be taken too far.
< luke-jr> https://cmpwn.com/@sir/104949263468735420 free Talos II being offered for cost of shipping, looks like to whomever can propose the best use they'd put it to
< sipa> dburkett: i never contributed to open source before bitcoin
< luke-jr> :o
< dburkett> Then I don't know :)
< sipa> at least not in any meaningful capacity
< sipa> dburkett: and at university everything was java, with a huge "many tiny classes, each in one file!" mantra - which i think is horrible
< luke-jr> sipa: sometimes I think the file list and/or opening more files is more annoying than just searchign in an already open/visible file
< sipa> luke-jr: yes!
< dburkett> But the bouncing around in the same file makes it harder to keep tabs open with specific functions you're investigating. I totally get the sentiment though, and agree it can be taken too far
< sipa> "tabs" ?
< luke-jr> dburkett: Kate at least has bookmarks
< luke-jr> which is another case many files can be a nightmare: which file had the bookmark I want? :P
< dburkett> In a modern IDE- remember this is 2020 :)
< sipa> hmm, haven't used IDEs in years
< luke-jr> how modern is an IDE without bookmarks? :P
< sipa> i did for java, and used eclipse-cdt for a C project a decade ago or so
< sipa> but i find i'm much more efficient working with 2-3 terminal windows with text editors
< dburkett> IDEs have bookmarks, but they're less natural than opening separate files and navigating between them. I believe my questions have been answered by this dialog though. It's a difference purely in style between the vim hackers and modern IDE devs. It's a gap that won't be easy to bridge.
< sipa> dburkett: to be clear, none of this has any bearing on wanting to separate out logically separate parts of the code to be physically separate as well (whether that means a separate file, another directory, ...)
< sipa> but i do object to a blanket "smaller files better" view ;)
< dburkett> Whether smaller files are "better" or not, I think it's reasonable to argue that small files are generally safer, and result in easier to maintain APIs. Though definitely any mantra can be taken too far, as the java folks have shown :)
< sipa> yeah
< sipa> and personal preferences will differ
< dburkett> Anyhow, thanks for the discussion. It's valuable to hear about everyone's personal opinions and reasoning. It'll help guide my small, targeted attempts at refactoring.
< sipa> dburkett: i think smaller well-defined interfaces are generally better - and if a single file is a single interface, perhaps
< sipa> but in C++ it's not hard to fall into an anti-pattern where a single interface is defined by multiple files
< dburkett> Yes, perhaps the difference in our thinking is only in our opinions about how many interfaces belong in a single file. I'll buy that physical structure is very much a personal preference, while logical structure has more universal rights and wrongs.
< sipa> i also think (and this may be controversial, so certainly not going to push it onto anything), that you can have a single file that contains a series of well-documented interfaces (with comments saying "first define the low-level data structures used:" "here is some of the computational stuff" "here is the main interface implementation"), if those preliminary things are only used by what follows in
< dburkett> sipa: I've not come across many examples where a single interface spans multiple files, but I don't doubt that there are many examples out there. I agree that is not great at all.
< sipa> the same file
< sipa> it prevents littering the directory structure with extra files that are private to one concept anyway
< dburkett> So you're talking about file-scope data structures and interfaces in a cpp file that you're not exposing in a header. I definitely understand that pattern.
< sipa> yeah
< dburkett> When it becomes too burdensome, a directory or separate lib or something becomes more important. But if not taken too far, that's a pattern I often follow myself.