< 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
< 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/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/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] 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 1.0.0.0, 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>
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>
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.