<bitcoin-git>
[bitcoin] glozow merged pull request #26382: [24.x] p2p: Handle IsContinuationOfLowWorkHeadersSync return value correctly when new headers sync is started (24.x...2022-10-backport-26355) https://github.com/bitcoin/bitcoin/pull/26382
<bitcoin-git>
bitcoin/master eb679a7 w0xlt: rpc: make `address` field optional
<bitcoin-git>
bitcoin/master 551c8e9 Andrew Chow: Merge bitcoin/bitcoin#26349: rpc: make `address` field optional `list{tran...
Talkless has joined #bitcoin-core-dev
gnaf has quit [Quit: Konversation terminated!]
nanotube has quit [Ping timeout: 246 seconds]
NorrinRadd has joined #bitcoin-core-dev
<luke-jr>
FWIW, I'm finding some of the arguments against full RBF to be semi-convincing - as reasons to leave the default off, but not to remove the option
<luke-jr>
(note, not fully convincing :p)
<_aj_>
luke-jr: hey, why does knots have a service bit for full rbf but no preferential peering?
<instagibbs>
potentially preferentially non-peering to fullrbf? :)
jonatack has joined #bitcoin-core-dev
<_aj_>
luke-jr: (or is it just "patches welcome" ?)
<luke-jr>
_aj_: no particular reason; didn't seem important enough?
<_aj_>
luke-jr: what's the point of the service bit without preferential peering?
<luke-jr>
_aj_: and the code paths are such that I'd prefer to have anything in that area reviewed :x
<luke-jr>
_aj_: so other nodes can preferentially peer
<_aj_>
luke-jr: ah that's not quite "patches welcome" but near enough
<luke-jr>
eg, petertodd used to maintain one
<lightlike>
having the service bit also allows manual preferential peering via "addnode"
<luke-jr>
lightlike: you could do that without a service bit tho?
<lightlike>
but how would you know peers supporting full-rbf if they don't advertise that in their service bits?
Zenton has quit [Ping timeout: 246 seconds]
<sipa>
lightlike: I'm not sure you should be using rumoured IP addresses as targets manual connection in the first place... it works of course, but manual connections are more intended for nodes for which you have out-of-band reasons to know.
<bitcoin-git>
[bitcoin] achow101 closed pull request #26349: rpc: make `address` field optional `list{transactions, sinceblock}` response (master...issue_26338) https://github.com/bitcoin/bitcoin/pull/26349
<_aj_>
sipa: the only thing is they don't get discouraged/disconnected for misbehaviour (weak form of noban) isn't it?
<sipa>
Well and they get separate connection slots.
<sipa>
With separate rules for deciding how they're used.
<_aj_>
sipa: preferential peering would probably get extra connection slots too (and obviously have separate rules) though?
NorrinRadd has quit [Quit: NorrinRadd]
<_aj_>
sipa: (which maybe just reduces it to "i'm not sure you should do preferential peering, whether manual or automatic", i guess)
<sipa>
Wait maybe we're talking about different things.
<sipa>
I was just talking about the distinction between manual and automatic connections.
<lightlike>
btw jonatack did a twitter poll about this recently - almost 50% of the respondents that used manual connections at all used them to connect to unknown peers: https://twitter.com/jonatack/status/1524377548062351360
<jonatack>
Yes IIRC addnode connections have 8 separate slots in addition to the 8 regular + 2 block relay + temporary ones
<luke-jr>
lightlike: so anchors?
<jonatack>
lightlike: yes, I've attempted to communicate to the larger community that addnode conns are intended for trusted peers as an anti-eclipse protection
sudoforge has joined #bitcoin-core-dev
yanmaani2 has quit [Remote host closed the connection]
<_aj_>
sipa: right, trying to understand what exactly the (unexpected/undesirable) consequences are though. is using an extra connection slot problematic? i could see it being bad if everyone did it, for sure...
sipsorcery has joined #bitcoin-core-dev
<sipa>
_aj_: My point is more that it's not all that useful; if all you're doing is picking rumoured peers to connect to, why not let the automatic connection mechanism do its job?
yanmaani2 has joined #bitcoin-core-dev
<_aj_>
sipa: you're filtering the rumoured peers by an uncommon service bit
<sipa>
Doing the job the software should be doing for you :p
<sipa>
But ok, fair, that's a benefit.
<_aj_>
sipa: sure, but luke just said he'd want more review of a patch to fix that than he expects to get :)
<sipa>
Fair enough.
<jonatack>
lightlike: that twitter poll only had 160 respondants; it may be interesting to do one from an account with more reach like the bitcoin core org one
<lightlike>
sipa: you can pick networks that way. automatic peering gives you no guarantees to have outbound connections to each network you like, it just picks random addrs from addrman
<luke-jr>
jonatack (& anyone else): happy to retweet polls like this if you want to ping me with them ;)
<luke-jr>
at least in my experience, my polls tend to get a lot of responses
NorrinRadd has joined #bitcoin-core-dev
<jonatack>
luke-jr: sgtm. laanwj was also happy to retweet. we can do some.
<jonatack>
lightlike: yes. i have manual conns with tor, i2p and cjdns for instance.
<luke-jr>
I have 3 addnodes, 1 probably dead (Eligius), and 2 I don't remember XD
Talkless has quit [Quit: Konversation terminated!]
<laanwj>
welcome to the weekly general bitcoin-core-dev meeting
<jarolrod>
Hi
<ajonas>
hi
<sipa>
hi
<fjahr>
hi
<Murch>
hi
<pablomartin_>
hello
<laanwj>
no meeting topics have been proposed this week, but we still have some from last week from achow101: Adding (more) maintainers as GitHub org owners, Moratorium on refactors that are not part of larger projects that bring tangible features and fixes
<gleb071>
laanwj: i tested editing very moderately to not cause some org-wide notifications so it's possible you're right. i'm not interested enough to look further into that, but i can if you wish
<achow101>
speaking of permissions, of the current owners of the bitcoin and bitcoin-core orgs, only 1 is a current maintainer, the rest former
<achow101>
so I think it would make sense to have some of the current maintainers be added as owners as well
<laanwj>
gleb071: i don't particularly care either, i'm sure everyone in the org is responsible enough not to do stupid things with the project board
<achow101>
aiui, the main thing org owners do is administrative, and clean up spammers
<jarolrod>
if a maintainer is an owner, and drops commit access, seems sane to drop org ownership as well
<luke-jr>
but we should have more than 1
bomb-on has joined #bitcoin-core-dev
<luke-jr>
eg, in case someone gets hit by a bus
sipsorcery has quit [Ping timeout: 240 seconds]
<achow101>
luke-jr: yes, that's mainly why I bring this up
<vasild>
is that list public?
<luke-jr>
achow101: but they can potentially do a lot more than spammer-triage
<jarolrod>
org moderators can clean up spammers as well btw
<achow101>
vasild: maybe? for contributors already part of the org, you can see who the owners are, not sure how public though
<vasild>
ok
<laanwj>
no, this isn't public, and maybe it's important to be somewhat discrete about who exactly has owner permissions, for bus factors etc
<vasild>
then this is not suitable for public irc meeting?
<vasild>
s/not/semi/
<luke-jr>
FWIW, lack of bitcoin org ownership prevented me from adding Kalle to the bips repo a while back, but I'm not sure I want that kind of access (in terms of becoming a possible target)
<laanwj>
i'd agree adding one more maintainer would make sense though
<sipa>
agree as well
<laanwj>
not adding a maintainer but to the owners i mean
<luke-jr>
laanwj: in terms of privacy, maybe it's best if there is no correlation with maintainers then?
sipsorcery has joined #bitcoin-core-dev
<jonatack>
gleb071: i don't have any additional access priveleges and it looks like i could edit the project board too
<achow101>
I think it would make sense for maintainersto be owners as owners can (force) push to protected branches
<achow101>
and so any owner would be a committer, even if their key is not in trusted-keys
<laanwj>
luke-jr: i don't understand, you mean no one of the owners was available or wanted to add Kalle for you?
<luke-jr>
laanwj: no, I mean I had to ask an owner to do it
<laanwj>
oh, sure
<luke-jr>
which is fine IMO
<laanwj>
yeah... it doesn't seem like something that often happens, anyhow
<achow101>
I think knowledge of who the owners are is semi public already, as we all know who to ping to cleanup spam
<luke-jr>
[19:19:52] <jarolrod> org moderators can clean up spammers as well btw
<luke-jr>
so ownership of the org seems like a basically useless thing, but something we must maintain to avoid problems XD
<jonatack>
achow101: yes, istm anyone following the repo closely can see who is cleaning up
<jarolrod>
^^ we don't have any moderators currently though
<glozow>
achow101: to clarify you’re suggesting everyone in trusted-keys should also be org owners?
<luke-jr>
jarolrod: I assume that's as easy to resolve as adding owners
<jarolrod>
^^ that would be too many owners
<jarolrod>
i think he means of the owners, they should be a subset of current maintainers
<luke-jr>
^
<achow101>
luke-jr: I think org owners are also the only ones that can add new members? and also migrate repos in/out of the org (e.g. we might want to move libmultiprocess into bitcoin-core)
<luke-jr>
achow101: ah
<glozow>
oh i see
<jonatack>
i'd suggest achow101, providing you are willing
<jarolrod>
i'd suggest a specific maintainer who has a robot :D
<achow101>
I am willing
<vasild>
+1
<luke-jr>
+1 for achow101
<ariard>
ACK achow101 for org ownership
<gleb071>
ACK
<josie[m]>
ACK achow101 for org ownership
<b10c>
ach ackow101
<fjahr>
ACK
<josie[m]>
very game of thrones vibe: "I am willing"
<luke-jr>
ach? O.o
<pablomartin_>
ack
<jonatack>
b10c: nice inversion
<jarolrod>
👍
<laanwj>
ok... seems we have agreement on that, at least inside the meeting
<glozow>
ack
<laanwj>
#topic Moratorium on refactors that are not part of larger projects that bring tangible features and fixes (achow101)
<Murch>
+1
<core-meetingbot>
topic: Moratorium on refactors that are not part of larger projects that bring tangible features and fixes (achow101)
<sipa>
Like with the question of adding maintainers, I think this is primarily a decision to be made by current maintainers/owners, as they have the best view on where help and of what kind is needed. Personally, ACK achow101.
<laanwj>
yeah... but it's good to have broad agreement i guess
<achow101>
We briefly discussed at coredev to reduce the number of refactors opened as they can have quite an effect on rebasing and review
<achow101>
so perhaps we should stop opening refactoring prs that are not well motivated in the context of doing something larger that requires refactoring
<vasild>
Such a moratorium seems like a blanket NACK on a vaguely defined set of changes. Who is deciding whether something is tangible?
<achow101>
(this is both an effort to make it easier/faster to get things merged, and reduce the number of prs open)
<sipa>
I agree it's pretty vague, and hard to decide anything about in general.
<sipa>
But maybe you do have something more specific in mind (a few examples?)
<laanwj>
it seems somewhat vague... maybe encourage people to nack specific refactors they disagree with?
<luke-jr>
seems more like a vague suggestion
<instagibbs>
or encourage people to state up front goals in refactoring PRs
<luke-jr>
NACK is too strong - more like just silence on the PRs :P
amovfx has quit [Ping timeout: 252 seconds]
<achow101>
I suppose this is more of a "state your motivations" kind of suggestion, and perhaps think before refactoring
<laanwj>
no i mean really nack, as in "yes it's a minor improvement but it's not worth it to change this many files" and things like that
<josie[m]>
instagibbs: +1 , i think making a compelling argument for why you are doing something, and how it impacts further work is always a good idea
<_aj_>
"-0 -- what is this building towards?" or something
<vasild>
I think the current scheme works well: 1. if somebody bothered to write the code and 2. enough people think it is important enough and bother to review it and 3. some maintainer decides to merge it => then it is ok to have it in master
<sipa>
I think it's generally helpful if there were more concept acks/nacks, even ones motivated as "unclear motivation".
<brunoerg>
vasild: +1
<sipa>
(certainly something I'm guilty off to, I prefer to stay silent on things I don't strongly dislike...)
<gleb071>
The only two clear things we can do here is to say is to hear. I do share the opinion sometimes the refactorings are not justified given our review capacity. I will do my part in not producing such refactors.
<achow101>
one example of such a refactor was a getInt thing in unvalue(?) from a few months ago that caused rebasing for no clear reason
<jonatack>
it's hard to see this being more than a general encouragement, given the ad hoc nature of how we (prefer to work), but in general if the why doesn't make sense then the what probably doesn't matter, so that is always a good reason to well motivate PR descriptions
<gleb071>
"is to say and to hear" i meant...
<luke-jr>
achow101: I did NACK that, and got ignored
<lightlike>
what is the intention behind "moratorium"? Disallow them periodically in a certain time span for each release cycle, or disallow them indefinitely "for now"?
<josie[m]>
luke-jr: i kinda wished more people would NACK rather than be silent, so long as they take the time to explain why. it can be kinda frustrating with just silence because you dont know if people disagree or are just too busy
<luke-jr>
josie[m]: I'd put refactoring moratorium as "just too busy" more than "disagree" IMO
<achow101>
lightlike: indefinitely "for now"
<gleb071>
We can think of inventing a new means of communication. Say, if you lack reviewes, you explicitly tag relevant devs. Then they feel better giving low-prio-nack.
<achow101>
mainly this topic was to seed the ideas of either stop opening refactors, or be more agressive aboue nacking things
<vasild>
The section "### Refactoring" in CONTRIBUTING.md is relevant to this and IMO is very well written
<ajonas>
My concern is that a lot of the refactors that you may have in mind are authored by people not at this meeting. I think the best way to enforce would probably be to leave comments pushing for better motivation or NACKing (which requires work) and wait for the next kill, shill, merge session. Over time, that gets absorbed though it will also turn many newcomers away.
<jonatack>
lightlike: there have been proposals in the past to concentrate refactorings at certain parts of the release cycles, but that didn't go further for the same reason as here (probably): an ad hoc approach seems more conducive to this open source project than top-down project management
<laanwj>
newcomers really shouldn't open refactors :)
<_aj_>
ajonas: (kill/shill/fulfill?)
<laanwj>
that takes understanding of the code and purpose... we have that documented in CONTRIBUTING.md even
<josie[m]>
aj: nice
amovfx has joined #bitcoin-core-dev
<ajonas>
laanwj: no argument from me.
<josie[m]>
ajonas: i think this is a good point, and in any case, being more communicative on PRs can't hurt. im not sure about turning newcomers away tho. at least from my experience, the silence can be more discouraging then someone giving a (constructive) NACK
<laanwj>
we used to have a lot of out of the blue refactores, which prompted that to be written, it's not a new issue really but also a complicated one
<_aj_>
maybe point newcomers more at writing additional test coverage for existing code?
<luke-jr>
I noticed earlier a dumb refactor simply closed with no comment. I imagine that's pretty discouraging <.<
<laanwj>
josie[m]: i definietly agree to that (from my own experience of outside contributor to projects)
<laanwj>
"no i don't want this" is better than just silence for months or it lingering for years even
<jonatack>
maybe a canned response can be given to newcomer refactors. that said, from what i've seen when a newcomer proposes an intelligent|useful cleanup or refactoring, it not only tends to be well-
<jonatack>
received but also perhaps gets attention and merge more quickly
<jonatack>
which could encourage more refactoring perhaps
amovfx has quit [Ping timeout: 246 seconds]
<fjahr>
adding tests is good but also fixing actual bugs, there are enough open issues. But for a newcomers that is probably a lot of work and many seem to look for instant gratification.
<laanwj>
as a contributor you're generally aware not everything will be merged and there can be disagreements
<jonatack>
as refactoring outwardly may seem easier to review as well and divert review attention (as mentioned a bit earlier above in the meeting)
<josie[m]>
laanwj: +1
<laanwj>
fjahr: hah yes... a long time ago there used to be this awful system that paid out per commit... this encouraged a lot of people to try to get *something* into bitcoin core, the most trivial and cosmetic changes
<sipa>
that was terrible, i remember
<josie[m]>
jonatack: i think the fact that refactors appear to be easy can also make them very dangerous, as in they might not get as thorough a review there is a non-zero chance they introduce subtle bugs
<achow101>
jonatack: even "useful cleanups" are detrimental as they can cause conflicts with other things and delay other PRs from being merged
<luke-jr>
I think we did lose a developer over that being shutdown tho
<_aj_>
that one seemed to be traced the parallel script checking threads hanging on shutdown, i think because they were waiting for SyncWithValidationInterfaceQueue() to finish, but it was hanging on the schedule to finish a promise, and the scheduler thread was already stopped
<jonatack>
_aj_: have been seeing hangs on shutdown too
<_aj_>
luke-jr: so i fixed that locally by just having SyncWithValidationInterfaceQueue exit early if shutdown's requested; but had to make the promise be a shared_ptr, so that when the scheduler got cleared, it hadn't already been freed
___nick___ has quit [Ping timeout: 272 seconds]
<luke-jr>
_aj_: you found a different one then?
<_aj_>
luke-jr: LimitValidationInterfacequeue calls SyncWithValidationInterfaceQueue, so seems similar?
<vasild>
I found the code surrounding is gives me a headache
<vasild>
s/is/it/
* vasild
zZzZ
<jeremyrubin>
jonatack: i think i proposed having refactor windows a while back, but it was sort of a stopgap of retaining contributors whose utility function is driven by refactors
<jonatack>
jeremyrubin: yes, i had that proposal in mind
<luke-jr>
I guess that's a risk of refactor moratorium: we might lose people who only do refactoring :x
<luke-jr>
ideally they'd review other stuff in the meantime, but they might not want to
<jeremyrubin>
so one of the worst things for refactors is fixing circular deps because it definitely makes rebasing crappy (unless someone has some expert mode git-fu)
<jeremyrubin>
but it is something that is a part of "a project" to fix bad design/improve compile times or something
<_aj_>
improving compile times is a larger project, and something you can benchmark to demonstrate the benefit...
<luke-jr>
I'm not sure a project which has refactoring as its sole purpose, qualifies :P
<jeremyrubin>
_aj_: sort of -- for circular dependencies you could have module A depending on module B which depends on A and module C which depends on A which depends on B. Cleaning up A's dependency on B would not make any improvement until the work on removing B and A from C (if i've made the example well), but would be progress towards that goal if C is
<jeremyrubin>
trickier to refactor than A/B.
amovfx has joined #bitcoin-core-dev
<jeremyrubin>
i also don't know if there is any compile time overhead for circular dependencies, not familiar enough with exactly how it works to know if there is a penalty (seems if LTO is enabled, yes, if not, headers solve for that)
<sipa>
You do need to distinguish the "module/semantic" circular dependencies and "code inclusion" circular dependencies.
brunoerg has quit []
sudoforge has quit [Ping timeout: 240 seconds]
<BlueMatt[m]>
is there any reason to think bitcoin core rpc would not return stale forked-off blocks if requested by hash?
<_aj_>
BlueMatt[m]: pruning?
<BlueMatt[m]>
yea, i asked about pruning, i dont think that was the issue here. I was just double-checking that no one introduced any kind of "old stale" check before returning block data in rpc
<BlueMatt[m]>
bug may well be elsewhere, just checking off boxes
<sipa>
Is this in `getblock` ?
<sipa>
I don't see any logic in the getblock RPC that would reject requests for non-mainchain blocks.
<sipa>
It only affects the "confirmation" field.
<_aj_>
BlueMatt[m]: getting old blocks that are listed in chaintips via getblock is working fine for me fwiw
<laanwj>
i don't think there's such a check for RPC, it would be really strange, unless it's somehow inherited from P2P where there's checks like that against fingerprinting
<instagibbs>
wasnt there a PR to make it stop sending these after a month
<instagibbs>
anti-fingerprinting
<instagibbs>
or is this just p2p
<instagibbs>
sorry, ignore
<sipa>
Just P2P, I believe.
<laanwj>
it would be kind of bad if that was for RPC
<instagibbs>
agreed!
<BlueMatt[m]>
Thanks y’all. Yea, would be bad.
<BlueMatt[m]>
Oh, there’s nothing like that in REST either, right v
<BlueMatt[m]>
?
<laanwj>
rest and rpc tend to follow exactly the same path
<sipa>
Just checked rest, can't see logic there either.
<sipa>
Except for JSON block output, where again it's used for confirmations.
<jonatack>
instagibbs: i suppose "ephemeral anchors" is vocabulary originating from anchor outputs in lightning, but the term is similar to our existing p2p anchors (last known outbound block-relay-only peers that we try to re-connect to on startup)
<jonatack>
naming collisions ftw i guess
<instagibbs>
yeah we can bikeshed for the next year or two, all for it
<instagibbs>
first, make sure it's not broken
<instagibbs>
(it's broken)
<_aj_>
"ephemeral cpfp hook" ? surely it won't take a year or two...
<jonatack>
_aj_ is good with words; nominated for terms naming
<instagibbs>
three words now, great.
<jonatack>
ecpfphook
<instagibbs>
phook it is
<jonatack>
yess
<_aj_>
get phooked?
<jonatack>
catchy > literal
<jonatack>
like band names
amovfx has quit [Ping timeout: 240 seconds]
bitdex has joined #bitcoin-core-dev
dviola has quit [Ping timeout: 240 seconds]
yanmaani2 has quit [Remote host closed the connection]
yanmaani2 has joined #bitcoin-core-dev
<bitcoin-git>
[bitcoin] sipa closed pull request #26076: Switch hardened derivation marker to h in descriptors (master...2022/09/descriptors_h) https://github.com/bitcoin/bitcoin/pull/26076
<bitcoin-git>
[bitcoin] sipa reopened pull request #26076: Switch hardened derivation marker to h in descriptors (master...2022/09/descriptors_h) https://github.com/bitcoin/bitcoin/pull/26076