14:00:25 <achow101> #startmeeting 
14:00:29 <darosior> hi
14:00:34 <dergoegge> hi
14:00:37 <Murch[m]> hi
14:00:39 <pinheadmz> hi
14:00:41 <achow101> #bitcoin -core-dev Meeting: achow101 _aj_ amiti ariard aureleoules b10c BlueMatt brunoerg cfields darosior dergoegge dongcarl fanquake fjahr furszy gleb glozow hebasto instagibbs jamesob jarolrod jonatack josibake kallewoof kanzure kouloumos kvaciral laanwj LarryRuane lightlike luke-jr MacroFake Murch phantomcircuit pinheadmz promag provoostenator ryanofsky sdaftuar S3RK stickies-v sipa theStack TheCharlatan vasild
14:00:50 <hebasto> hi
14:00:51 <glozow> hi
14:00:52 <TheCharlatan> hi
14:00:55 <furszy> hi
14:01:02 <vasild> hi
14:01:09 <achow101> There are 2 pre proposed meeting topics this week, and last minute ones to add
14:01:37 <sdaftuar> hi
14:01:39 <theStack> hi
14:01:55 <luke-jr_> hi
14:02:06 <kevkevin> hi
14:02:07 <lightlike> Hi
14:02:23 <achow101> #topic package relay updates (glozow)
14:02:31 <stickies-v> hi
14:02:34 <glozow> #28970 is my priority. Also, #29242 was merged. a followup is open (#29724) and I think the next PR is ready for review (#28984).
14:02:35 <LarryRuane> Hi
14:02:37 <gribble> https://github.com/bitcoin/bitcoin/issues/28970 | p2p: opportunistically accept 1-parent-1-child packages by glozow · Pull Request #28970 · bitcoin/bitcoin · GitHub
14:02:40 <gribble> https://github.com/bitcoin/bitcoin/issues/29242 | Mempool util: Add RBF diagram checks for single chunks against clusters of size 2 by instagibbs · Pull Request #29242 · bitcoin/bitcoin · GitHub
14:02:42 <gribble> https://github.com/bitcoin/bitcoin/issues/29724 | 29242 Diagram check followups by instagibbs · Pull Request #29724 · bitcoin/bitcoin · GitHub
14:02:43 <gribble> https://github.com/bitcoin/bitcoin/issues/28984 | Cluster size 2 package rbf by instagibbs · Pull Request #28984 · bitcoin/bitcoin · GitHub
14:02:54 <glozow> Thats it from me
14:03:01 <b10c> hi
14:03:25 <achow101> #topic cluster mempool updates (sdaftuar)
14:03:30 <sdaftuar> hi--
14:03:35 <sdaftuar> Hi -- I found a memory accounting bug in the draft PR this week, which I think I've figured out and have a fix for, but haven't yet pushed. I'm verifying my fix now on my historical data and will push once I'm done.
14:03:41 <sdaftuar> Also, the draft PR is now badly in need of a rebase, which I probably won't get to in the next week -- I'm prioritizing trying to do research on historical data with the branch that I have.
14:03:51 <sdaftuar> My hope is to put together some kind of summary of the changes to transaction acceptance that would have occurred with the new mempool, based on last year's transaction data. (This includes redoing the preliminary performance benchmarks that I'd previously done which need to be re-done with the above-mentioned bugfix in place.)
14:04:03 <sdaftuar> So I'm planning to tackle the rebase and re-working of the draft PR after that research is complete.
14:04:10 <sdaftuar> Excitingly (for me), today I discovered over 1000 examples of RBFs that took place in January 2023 which were accepted by the old mempool logic but would have been rejected by the new feerate diagram check, which I'm in the process of exploring further.
14:04:16 <sdaftuar> If anyone has any questions that I can answer, please let me know!
14:04:22 <sdaftuar> (that's it for me)
14:04:57 <glozow> cool! (the RBFs)
14:05:25 <achow101> Is there anything that can be reviewed right now?
14:06:20 <kanzure> hi
14:07:14 <sdaftuar> achow101: well, i'm going to start asking people for help with the wallet and mini_miner changes soon. probably not a lot of useful review to be done now on the implementation itself, but i think reviewing the test changes could be helpful--
14:07:31 <sdaftuar> really i think it would be helpful if anyone is interested in writing better/more comprehensive tests which i could take in the PR
14:07:38 <sdaftuar> but that can also come later, of course
14:08:56 <achow101> #topic legacy wallet removal updates (achow101)
14:09:05 <vasild> "last year's transaction data" - do you take that from the blockchain or did you run some recorder that saves each tx that makes it to your mempool. E.g. if a transaction was accepted in the mempool but never mined, would it be in that historical data you tested with?
14:09:55 <sdaftuar> vasild: i have a data logging system that records every transaction and block that my node sees, and a patch set that allows me to play that data back through our validation logic
14:10:37 <vasild> cool :)
14:10:38 <achow101> Hasn't been much activity in the last week so the thing to review is still #26606. Also been reviewing #28574, hoping to get that in soon
14:10:40 <gribble> https://github.com/bitcoin/bitcoin/issues/26606 | wallet: Implement independent BDB parser by achow101 · Pull Request #26606 · bitcoin/bitcoin · GitHub
14:10:41 <gribble> https://github.com/bitcoin/bitcoin/issues/28574 | wallet: optimize migration process, batch db transactions by furszy · Pull Request #28574 · bitcoin/bitcoin · GitHub
14:11:16 <achow101> #topic Ad-hoc high priority for review
14:11:24 <achow101> Anything to add or remove from https://github.com/orgs/bitcoin/projects/1/views/4
14:13:23 <achow101> #topic enable misc-no-recursion: #29690 (proposing on behalf of @dergoegge) (stickies-v)
14:13:25 <gribble> https://github.com/bitcoin/bitcoin/issues/29690 | clang-tidy: Enable misc-no-recursion by dergoegge · Pull Request #29690 · bitcoin/bitcoin · GitHub
14:13:37 <dergoegge> i have a pr open to discourage the use of recursion (and to make it obvious when we introduce it) using the "misc-no-recursion" clang-tidy plugin #29690
14:13:37 <gribble> https://github.com/bitcoin/bitcoin/issues/29690 | clang-tidy: Enable misc-no-recursion by dergoegge · Pull Request #29690 · bitcoin/bitcoin · GitHub
14:13:57 <dergoegge> if anyone has an issue with that please comment on the PR :)
14:14:18 <vasild> Is recursion frowned upon? :-O
14:15:00 <luke-jr> there's some arguments made, but I think the point here is to make it explicit
14:15:35 <luke-jr> contrasted with (eg) calling the same function with a different signature
14:15:54 <dergoegge> there is nuance to that question, i'd frown upon it in our repo but it's mostly about making it explicit
14:16:56 <achow101> The suggestion is that if a function is recursive, just mark it as such, rather than avoiding it entirely?
14:17:29 <achow101> iirc it's used quite a bit in wallet related things (descriptors, signing, etc.)
14:17:32 <lightlike> Is this an actual problem? Are there too many (or any) PRs that attempt to introduce recursion?
14:18:00 <luke-jr> lightlike: introducing recursion shouldn't be forbidden.. that's a different thing from making it explicit
14:18:02 <fjahr> I think it’s not going to be doing much for us because it’s not like inexperienced developers include recursion in their PRs by accident very often. But since this also won’t affect ~99% all PRs anyway, I won’t stand in the way if enough others see value in this.
14:18:30 <sipa> there have been some cases of recursive functions in the codebase that could be triggered to cause stack overfloe
14:18:49 <sipa> (including ones introduced by me)
14:19:01 <luke-jr> I guess the real question is, would this have helped catch those? ;)
14:19:18 <sipa> i doubt it
14:20:13 <sipa> but it's hard to say, having the explicit marker in the codebase permanently may cause some people to occasionslly just look over all of them
14:20:26 <achow101> I suppose if you have opinions, go comment on the PR?
14:20:50 <luke-jr> is it the function that gets marked, or the call?
14:20:51 <sipa> ok
14:20:54 <luke-jr> achow101: k
14:20:55 <dergoegge> 👍
14:21:00 <vasild> In my experience some problems are recursive in nature and the recursive solution is neat an tidy and easy to follow and review and thus unlikely to have bugs. Iterative solutions for those on the other hand could be messy and easier to get wrong. I don't think a blanket ban on recursion makes sense in any project. Explicitly flagging a function "this is recursive" to avoid accidental recursion
14:21:00 <achow101> #topic writeups and disclosure of historical bugs reported but were never published (darosior)
14:21:05 <vasild> is another thing. +1 on that.
14:21:06 <sipa> luke-jr: in recursive functions, those two are the same :)
14:21:22 <darosior> So last time a group of us met in person, Niklas and i volunteered to make a few writeups about historical vulnerabilities which were reported to the project. So we did that. Thanks to fanquake for sharing with us the old reports.
14:21:22 <luke-jr> sipa: not really, the call site may be a page or two away
14:21:31 <darosior> We only have two though, looks like there were not that many in the end.
14:21:32 <sipa> luke-jr: the function gets marked
14:21:39 <darosior> We think we could write a blog post at bitcoincore.org about them, and maybe announce them on the ML. Does anyone has any feedback / opinion / objection to us doing this? (remember we are talking about very old vulnerabilities which were just never disclosed)
14:21:57 <achow101> darosior: I think putting them on bitcoincore.org is fine
14:21:59 <sdaftuar> seems like a good idea to me, thanks for doing this!
14:22:14 <instagibbs> +1
14:22:20 <luke-jr> darosior: there's definitely more than 2?
14:22:40 <darosior> luke-jr: we can only put so much pressure on fanquake to spit historical reports
14:23:05 <b10c> +1 on publishing
14:23:14 <darosior> cool, also maybe as a teaser: we want to animate a discussion at the next about how to handle disclosures moving forward
14:23:20 <achow101> we can talk offline if there are any more to write
14:23:28 <darosior> s/at the next/at the next in-person meeting/
14:24:03 <sipa> yeah i think i can help with more old ones too
14:24:11 <achow101> yes, I think bringing up this discussion at the next coredev would be good
14:24:19 <darosior> sipa: cool, will reach out
14:24:35 <luke-jr> k
14:24:44 <vasild> What's the point in publishing those? To get people running vulnerable versions to upgrade?
14:25:22 <dergoegge> vasild: yes and to give credit to the people that find the bugs
14:25:27 <darosior> I don't think anybody runs the affected versions anymore
14:25:31 <achow101> vasild: transparency, and it's also generally useful to have old vulns published so people can talk about them
14:25:38 <sdaftuar> vasild: i think it can also be instructive to contributors to learn from the past
14:25:41 <sipa> and also for ourselves, i think it's good to have a culture of keeping track of tjedr
14:25:45 <sipa> *these
14:26:13 <darosior> and there is also maybe a point to communicate to the larger Bitcoin community that this kind of thing does happen
14:26:13 <vasild> Good! I did not think about those
14:26:15 <kanzure> is there any existing disclosure content on bitcoincore.org? i don't want to see a "policy"
14:26:23 <luke-jr> darosior: there's at least a few trying to encourage downgrading to 0.12 :/
14:26:27 <achow101> kanzure: pretty sure there's a few older things
14:26:31 <sdaftuar> kanzure: isnt' the inflation bug on there?
14:26:39 <harding> kanzure: year, duplicate inputs bug is there
14:26:41 <darosior> sdaftuar: yes it it
14:26:54 <dergoegge> kanzure: e.g. https://bitcoincore.org/en/2019/11/08/CVE-2017-18350/
14:26:58 <achow101> they're buried deep in the blog section
14:27:12 <luke-jr> not-time-sensitive stuff has historically been published elsewhere tho
14:27:29 <luke-jr> maybe we can add a page of links to the menus somewhere so they're easy to find
14:27:35 <kanzure> there needs to be room to maneuver with respect to responsible disclosure and the unique nature of each incident. historical should be fine to document, but i wouldn't want it to be misinterpreted as an expectation on any specific timeline.
14:27:45 <achow101> luke-jr: it does make sense to have one place with/linking to all of them though
14:28:07 <achow101> kanzure: I don't think we're going to be setting a policy with hard deadlines
14:28:17 <darosior> kanzure: yes this topic was only about historical reports. Disclosure of more recent reports will be discussed at the next in-person meeting.
14:28:26 <sipa> kanzure: agreed, but i also think the number of bugs that require unusual treatment i a tiny fraction only
14:28:44 <kanzure> sipa: glad to hear that.
14:28:55 <darosior> Alright i think that's all we had to share, dergoegge anything to add?
14:29:04 <sipa> i assume darosior and dergoegge are talking about the few very serious ones we've had, and for those, we need ad-hoc proceddes
14:29:51 <dergoegge> darosior: no :)
14:29:54 <fjahr> some old fork based altcoins could be affected still right? Not saying that means we shouldn't do it, just a thought. Maybe you could grep for the affected lines in github?
14:30:41 <darosior> fjahr: it's a decade old, i don't think anything remotely sane still running would be on these versions anymore
14:30:55 <achow101> darosior: there are plenty of insane people :p
14:30:56 <b10c> fjahr: if they haven't cherry-picked commits in 10 years, they won't start now?
14:31:00 <kanzure> fjahr: yes this has been discussed on the mailing list in the past as an issue with responsible disclosure (do i personally owe any responsibility to disclose to altcoins that forked and didn't provide developer resources back? what about their users...?) etc
14:31:13 <luke-jr> b10c: maybe they have, just didn't know about the relevant ones?
14:31:41 <luke-jr> kanzure: even if they had provided resources back, they wouldn't necessarily know
14:31:56 <fjahr> b10c: yeah, still a heads up a few days before publishing would be nice, but no strong feelings either way...
14:32:03 <kanzure> (and there are also security issues with disclosing to altcoins: many of these projects are acting as adversarial entities! why would you disclose weapons or vulnerabilities or whatever?)
14:32:18 <luke-jr> there's only a small few legit altcoins tho, and I don't see why we should care about the scams
14:32:27 <achow101> as usual, I think it needs to be evaluated on a case by case basis
14:32:31 <luke-jr> the legit ones are likely already rebased anyway
14:32:37 <darosior> fjahr: for recent disclosure i agree out of courtesy we could be reaching to a few of the main altcoins a few days before publishing. For historical ones, meh
14:32:42 <kanzure> sure, sure. this is an old topic, and not much has changed.
14:33:06 <achow101> anything else to discuss?
14:33:08 <dergoegge> i think i'd be irresponsible not to give them a heads up but only after we've decided it is safe to publish anyway
14:35:38 <achow101> #endmeeting