<bytes1440000>
luke-jr: let me rephrase: its not just consensus related code, bitcoin core can have bugs and vulnerabilities in different thing. Example: CVE-2018–20587
bytes1440000 has quit [Quit: Client closed]
Kaizen_Kintsugi_ has quit [Remote host closed the connection]
Kaizen_Kintsugi_ has joined #bitcoin-core-dev
Kaizen_Kintsugi_ has quit [Ping timeout: 252 seconds]
Kaizen_Kintsugi_ has joined #bitcoin-core-dev
Kaizen_Kintsugi_ has quit [Ping timeout: 252 seconds]
Kaizen_Kintsugi_ has joined #bitcoin-core-dev
hashfuncd0e has quit [Ping timeout: 248 seconds]
<luke-jr>
obviously
<luke-jr>
and when found, these get fixed
cold has quit [Quit: Quitting...]
cold has joined #bitcoin-core-dev
Kaizen_Kintsugi_ has quit [Ping timeout: 252 seconds]
Kaizen_Kintsugi_ has joined #bitcoin-core-dev
Kaizen_Kintsugi_ has quit [Ping timeout: 248 seconds]
sudoforge has quit [Ping timeout: 240 seconds]
Kaizen_Kintsugi_ has joined #bitcoin-core-dev
brunoerg has quit [Ping timeout: 252 seconds]
Evel-Knievel has joined #bitcoin-core-dev
cmirror has quit [Remote host closed the connection]
brunoerg has joined #bitcoin-core-dev
cmirror has joined #bitcoin-core-dev
Kaizen_Kintsugi_ has quit [Ping timeout: 260 seconds]
z9z0b3t1c has joined #bitcoin-core-dev
z9z0b3t__ has quit [Ping timeout: 246 seconds]
brunoerg has quit [Ping timeout: 248 seconds]
brunoerg has joined #bitcoin-core-dev
brunoerg has quit [Remote host closed the connection]
brunoerg has joined #bitcoin-core-dev
<TallTim>
luke-jr, as an outside observer I'm slightly amused that you're being asked to essentially predict bugs. :)
mikehu44 has joined #bitcoin-core-dev
brunoerg has quit [Ping timeout: 240 seconds]
Kaizen_Kintsugi_ has joined #bitcoin-core-dev
Kaizen_Kintsugi_ has quit [Ping timeout: 240 seconds]
<luke-jr>
>_<
brunoerg has joined #bitcoin-core-dev
weez22 has quit [Quit: Connection closed for inactivity]
bfsfhkacjzgcytf9 has joined #bitcoin-core-dev
Kaizen_Kintsugi_ has joined #bitcoin-core-dev
bfsfhkacjzgcytf9 has quit [Ping timeout: 240 seconds]
upekkha has quit [*.net *.split]
yakshaver has quit [*.net *.split]
arik has quit [*.net *.split]
Nebraskka has quit [*.net *.split]
sebx2a has quit [*.net *.split]
upekkha has joined #bitcoin-core-dev
yakshaver has joined #bitcoin-core-dev
sebx2a has joined #bitcoin-core-dev
arik has joined #bitcoin-core-dev
Nebraskka has joined #bitcoin-core-dev
MatrixBot1 has quit [*.net *.split]
ls55 has quit [*.net *.split]
RubenSomsen has quit [*.net *.split]
lightlike has quit [*.net *.split]
cjd has quit [*.net *.split]
blkncd has quit [*.net *.split]
blkncd has joined #bitcoin-core-dev
ls55 has joined #bitcoin-core-dev
MatrixBot1 has joined #bitcoin-core-dev
cjd has joined #bitcoin-core-dev
RubenSomsen has joined #bitcoin-core-dev
lightlike has joined #bitcoin-core-dev
brunoerg has quit [Ping timeout: 260 seconds]
brunoerg has joined #bitcoin-core-dev
jonatack has joined #bitcoin-core-dev
brunoerg has quit [Ping timeout: 260 seconds]
brunoerg has joined #bitcoin-core-dev
Kaizen_Kintsugi_ has quit [Ping timeout: 248 seconds]
<bitcoin-git>
bitcoin/master cba909e John Newbery: [net] Stop testing version 1 compact blocks.
<bitcoin-git>
bitcoin/master 16730b6 John Newbery: [net processing] Only advertise support for version 2 compact blocks
<bitcoin-git>
bitcoin/master 42882fc John Newbery: [net processing] Only accept `sendcmpct` with version=2
<bitcoin-git>
[bitcoin] fanquake merged pull request #20799: net processing: Only support version 2 compact blocks (master...2020-12-remove-cmpctblock-v1) https://github.com/bitcoin/bitcoin/pull/20799
Kaizen_Kintsugi_ has quit [Ping timeout: 272 seconds]
salvatoshi has quit [Ping timeout: 240 seconds]
cryptapus has quit [Remote host closed the connection]
aleggg has quit [Ping timeout: 252 seconds]
aleggg has joined #bitcoin-core-dev
Kaizen_Kintsugi_ has joined #bitcoin-core-dev
evanlinjin has joined #bitcoin-core-dev
goatpig has quit [Remote host closed the connection]
cryptapus has joined #bitcoin-core-dev
bomb-on has joined #bitcoin-core-dev
evanlinjin has quit [Ping timeout: 240 seconds]
Talkless has joined #bitcoin-core-dev
jonatack has quit [Ping timeout: 260 seconds]
Kaizen_Kintsugi_ has quit [Ping timeout: 260 seconds]
bairen has quit [Remote host closed the connection]
Kaizen_Kintsugi_ has joined #bitcoin-core-dev
<luke-jr>
hebasto: do we want issues for master-only bugs now?
<sipa>
Why not?
<luke-jr>
idk, seems like it would hopefully be too short-lived
<sipa>
If it's longer lived than you mentioning it on IRC, it seems preferable to have an issue open for it so it's not forgotten.
<luke-jr>
maybe I should just debug it :x
<sipa>
Like, if you or something else were to open a PR immediately to fix it, I'd say an issue isn't needed (but it still wouldn't hurt).
<sipa>
*someone else
NorrinRadd has joined #bitcoin-core-dev
NorrinRadd has quit [Ping timeout: 272 seconds]
<luke-jr>
ugh, it's in spaghetti code
* luke-jr
just opens an issue instead x.x
z9z0b3t1_ has joined #bitcoin-core-dev
z9z0b3t1c has quit [Ping timeout: 256 seconds]
<bitcoin-git>
[bitcoin] jnewbery opened pull request #25147: Net processing: follow ups to #20799 (removing support for v1 compact blocks) (master...20205_20799_follow_ups) https://github.com/bitcoin/bitcoin/pull/25147
sipsorcery has joined #bitcoin-core-dev
szkl has quit [Quit: Connection closed for inactivity]
<bitcoin-git>
bitcoin/master ba10b90 Luke Dashjr: Wallet: Ensure m_attaching_chain is set before registering for signals
<bitcoin-git>
bitcoin/master 98f4db3 Andrew Chow: Merge bitcoin/bitcoin#25088: Wallet: Ensure m_attaching_chain is set befor...
<bitcoin-git>
[bitcoin] achow101 merged pull request #25088: Wallet: Ensure m_attaching_chain is set before registering for signals (master...fix_wallet_race_attachingbb) https://github.com/bitcoin/bitcoin/pull/25088
<bitcoin-git>
bitcoin/master 2a22f03 avirgovi: parsing external signer master fingerprint string as bytes instead of cari...
<bitcoin-git>
bitcoin/master 91a42d6 Andrew Chow: Merge bitcoin/bitcoin#25019: parse external signer master fp as bytes in E...
<bitcoin-git>
[bitcoin] achow101 merged pull request #25019: parse external signer master fp as bytes in ExternalSigner::SignTransaction (master...lower_master_fp_of_ext_signer_in_SignTransaction) https://github.com/bitcoin/bitcoin/pull/25019
___nick___ has quit [Ping timeout: 260 seconds]
Nekorand has quit [Quit: Leaving]
<bitcoin-git>
[bitcoin] hebasto opened pull request #25149: refactor: Add thread safety annotation to `BanMan::SweepBanned()` (master...220516-bantsa) https://github.com/bitcoin/bitcoin/pull/25149
szkl has joined #bitcoin-core-dev
hashfunc16f7 has joined #bitcoin-core-dev
djb27_ has quit [Ping timeout: 248 seconds]
z9z0b3t1c has joined #bitcoin-core-dev
z9z0b3t1_ has quit [Ping timeout: 256 seconds]
hashfunc16f7 has quit [Ping timeout: 240 seconds]
Kaizen_Kintsugi_ has quit [Remote host closed the connection]
Kaizen_Kintsugi_ has joined #bitcoin-core-dev
Kaizen_Kintsugi_ has quit [Remote host closed the connection]
Kaizen_Kintsugi_ has joined #bitcoin-core-dev
Kaizen_Kintsugi_ has quit [Remote host closed the connection]
Kaizen_Kintsugi_ has joined #bitcoin-core-dev
<david-bakin>
So, I understand you squash commits before submitting a PR. But when responding to code review suggestions/requests - do you squash and force-push those fix commits? Because doesn't that mess up the PR comment/history?
Kaizen_Kintsugi_ has quit [Ping timeout: 248 seconds]
<sipa>
There is a general philosophical divide on this among projects, but Bitcoin Core follows a very strong squash-before-merging policy.
<sipa>
Your PR must be in a state where the individual commits make logical sense, without fixups.
<david-bakin>
ok, squash-before-merge is fine for sure, but I thought I just read here the other day I should squash before submitting the PR? or did I read that wrong? the contributor note does talk about separating format/refactor commits from "featureish" commits ...
<sipa>
Before submitting, definitely.
<sipa>
No reason to bother the reviewers with the internal history your PR went through privately.
<david-bakin>
ok ... so ... a) before submitting and c) at the merge-to-master but NOT necessairly b) during cr itself where it will mess up the PR's comment history?
sipsorcery has quit [Ping timeout: 260 seconds]
<sipa>
Up to you. It may make sense to squash during a PR's review process, or not.
<david-bakin>
hmm. ok ... I'll do what seems best & see what happens ...
<sipa>
The goal is making things easy for reviewers, including after the fact ones that want to understand what a PR did after it got merged.
<david-bakin>
ok
<sipa>
If you've had a lot of in depth code review, you may not want to squash immediately, because reviewers may dislike having to go through it again.
<sipa>
But also, a lot of time reviewers spend is on understanding your code changes and concept, rather than actual line-by-line review, and most of that isn't invalidated by squashing.
<david-bakin>
ok. here's a question I have about the bitcoin CR experience vs that I've experienced at companies: Is the line-by-line review not valued in general or is it just the unfortunate lack of reviewers + reviewer's time? IOW, I don't have the experience (yet) to do the "big picture" concept acks/nacks - but I _do_ know how to do close-reading of code for correctness. Is it worth doing?
<sipa>
I'm not sure what you mean. Both are important.
Kaizen_Kintsugi_ has joined #bitcoin-core-dev
<sipa>
But people only pay attention to PRs they are interested in obviously, as there is no one to tell them what to work on. That sometimes means fighting for reviewer attention (regardless of what type of review).
<david-bakin>
ok
jarthur_ has joined #bitcoin-core-dev
jarthur has quit [Ping timeout: 260 seconds]
Kaizen_Kintsugi_ has quit [Remote host closed the connection]
Kaizen_Kintsugi_ has joined #bitcoin-core-dev
Kaizen_Kintsugi_ has quit [Ping timeout: 260 seconds]
Kaizen_Kintsugi_ has joined #bitcoin-core-dev
<luke-jr>
david-bakin: I'd say the actual code review is *more* helpful than the high-level concept ack, since the latter is easy and the former tends to be the bottleneck
<david-bakin>
luke-jr that's what I would have thought (again, based on my long experience as a software engineer) but ... I wasn't actually sure it was valued here ... especially by people (like me) who don't have the knowledge to give a concept (n)ack ... I'm thinking of doing CRs with _just_ the close reading and _not_ the concept (n)ack it that'd be appropriate
<sipa>
luke-jr: it may depend on what PR you're talking about. I think it happens that some bigger, more conceptually invasive don't get reviewed (enough) precisely because it's unclear to reviewers to what extent it's likely the project will go that direction.
<sipa>
There is a spectrum of course between very high level conceptual direction, and line-by-line nitty naming style comments.
<sipa>
But personally, I find that even when I've done line-by-line review of a change, I don't actually mind it being squashed, because most of the work was trying to understand how and why the code changes are what they are. Going over the line-by-line changes again a second time tends to be a lot faster (and even if it's a lot, diffing the diffs can help).
brunoerg has quit [Remote host closed the connection]
<david-bakin>
sipa yes I'm not much fussed about naming myself or other small things - my focus is generally on coding for reliability and testability and readability. my question about squashing was becauase I'm not actually that used to how github handles comments on "squashed" code - i'm more used to dedicated code review tools that specifically know about diffs between different commits on
<david-bakin>
the way to the final pr 'cause that's their focus
bitdex has joined #bitcoin-core-dev
<sipa>
With a few very big changes in the past (my own PRs) I've maintained two parallel versions of the PR, one which was mostly append-only (adding fixup commits), and one with the merged-together clean-history version, keeping them in sync w.r.t. their final state.
brunoerg has joined #bitcoin-core-dev
<sipa>
So that you could both see the result, and the history how it got there. But that's definitely overkill for most changes.
<david-bakin>
i'm not there yet! little bitty changes for me at this point! but we'll see in a few years ...
<sipa>
I like your enthousiasm.
<david-bakin>
!!
<gribble>
Error: "!" is not a valid command.
<david-bakin>
yes!
<david-bakin>
( I tried to answer "!!" but some bot said that wasn't a valid command - talk about squishing someone's enthusiasm - nuts to that!)