14:00:40 <achow101> #startmeeting 14:00:54 <josie> hi 14:00:57 <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:01:02 <gleb> hi 14:01:03 <instagibbs> hi 14:01:04 <hebasto> hi 14:01:10 <glozow> hi 14:01:19 <lightlike> Hi 14:01:20 <fjahr> hi 14:01:20 <sipa> hi 14:01:42 <achow101> There are no pre-proposed meeting topics this week. Any last minute ones to add to the list? 14:01:59 <stickies-v> hi 14:02:02 <_aj_> chat about log api? 14:02:31 <achow101> #topic package relay updates (glozow) 14:02:59 <abubakarsadiq> hi 14:03:01 <glozow> The PRs that are up for review are #28364 and #28251. 14:03:01 <glozow> #28364 has some acks though there is some concern about conflict with the severity level logging project. Maybe good to have log api chat? 14:03:03 <gribble> https://github.com/bitcoin/bitcoin/issues/28364 | log: log wtxids when possible, add TXPACKAGES category by glozow ÷ Pull Request #28364 ÷ bitcoin/bitcoin ÷ GitHub 14:03:06 <gribble> https://github.com/bitcoin/bitcoin/issues/28251 | validation: fix coins disappearing mid-package evaluation by glozow ÷ Pull Request #28251 ÷ bitcoin/bitcoin ÷ GitHub 14:03:06 <darosior> hi 14:03:07 <gribble> https://github.com/bitcoin/bitcoin/issues/28364 | log: log wtxids when possible, add TXPACKAGES category by glozow ÷ Pull Request #28364 ÷ bitcoin/bitcoin ÷ GitHub 14:03:14 <glozow> #28345 is likely also of interest for those reviewing package relay 14:03:17 <gribble> https://github.com/bitcoin/bitcoin/issues/28345 | Bugfix: Package relay / bytespersigop checks by luke-jr ÷ Pull Request #28345 ÷ bitcoin/bitcoin ÷ GitHub 14:03:21 <TheCharlatan> hi 14:04:23 <luke-jr> I feel like virtual size stuff needs simplification somehow, but it's annoying due to the project modularity boundaries and need for the prev input data 14:04:34 <instagibbs> remove virtual stuff :P 14:04:39 <instagibbs> logs off 14:05:05 <instagibbs> practically speaking, whats the most common pattern that hits it 14:05:11 <instagibbs> I would have suspects bare multisig? 14:05:41 <sipa> nothing sane ever hits the increment-vsize-due-to-high-sigops behavior 14:05:47 <sipa> iirc 14:06:15 <glozow> #27591 is perhaps relevant for this discussion? it happened in the wild and a user got super confused that "vsize" was different in the mempool RPC 14:06:17 <gribble> https://github.com/bitcoin/bitcoin/issues/27591 | rpc: distinguish between vsize and sigop-adjusted mempool vsize by glozow ÷ Pull Request #27591 ÷ bitcoin/bitcoin ÷ GitHub 14:07:04 <luke-jr> sipa: right, we probably only _need_ it for external transactions (p2p, receiving in wallet) 14:07:36 <Murch> Hi 14:07:40 <instagibbs> presumably to reduce cpu DoS, without outright denying the txns? 14:07:50 <sipa> does anything at all ever care about the non-adjusted vsize? 14:08:10 <luke-jr> I think non-adjusted is only relevant because BIP141 specified it 14:08:13 <luke-jr> but not in practice 14:08:16 <sipa> right 14:08:19 <glozow> I don't think so. We only use non-adjusted when we don't have the prev inputs 14:08:25 <luke-jr> otoh it's also much simpler to calculate 14:08:45 <luke-jr> eg, it might not even be possible to get adjusted vsize for historical txs not in our wallet 14:08:50 <sipa> my suggestion would be to document it; if people care about the consensus-relevant part, there is always weight too, which is more accurate and is unadjusted 14:09:36 <glozow> ð 14:09:38 <luke-jr> if not for the complexity of the adjusted-vsize caclulations, I would almost prefer to just say vsize is always adjusted and weight is consensus 14:09:39 <sipa> yeah the dependency on input data makes it annoying 14:09:41 <_aj_> could change our rpc fields to say "ajdvsize" when we've taken sigops into account? 14:09:47 <_aj_> ajdvsize 14:09:49 <_aj_> jesus 14:09:52 <_aj_> adjvsize 14:10:02 <sipa> ETOOMANYVOWELS 14:10:05 <instagibbs> "aj" hmmm 14:10:10 <luke-jr> lol 14:10:13 <_aj_> yeah, pick what my fingers like to type 14:10:32 <josie> _aj_vsize 14:10:47 <luke-jr> well, that's part of why I think vsize vs weight is a better approach, it has a clear distinction 14:10:51 <_aj_> no, pls don't make me be blamed for legacy sigop limits forever 14:11:02 <glozow> Would welcome opinions on #27591, the goal there is to clean up the user-facing docs 14:11:04 <gribble> https://github.com/bitcoin/bitcoin/issues/27591 | rpc: distinguish between vsize and sigop-adjusted mempool vsize by glozow ÷ Pull Request #27591 ÷ bitcoin/bitcoin ÷ GitHub 14:12:25 <achow101> #topic BIP 324 updates (sipa) 14:12:30 <sipa> hi 14:12:53 <sipa> making good progress and getting review on #28196, which now also has unit tests 14:12:56 <gribble> https://github.com/bitcoin/bitcoin/issues/28196 | BIP324 connection support by sipa ÷ Pull Request #28196 ÷ bitcoin/bitcoin ÷ GitHub 14:13:49 <sipa> all the observable "now actually make use of v2" is in #28331 (which i'm running on some of my nodes) 14:13:51 <gribble> https://github.com/bitcoin/bitcoin/issues/28331 | BIP324 integration by sipa ÷ Pull Request #28331 ÷ bitcoin/bitcoin ÷ GitHub 14:14:41 <achow101> #topic libbitcoinkernel updates (TheCharlatan) 14:15:03 <TheCharlatan> I've been working towards removing the last few unwanted headers that are currently required to use the kernel library. 14:15:09 <TheCharlatan> As far as I know these are the boost multi index headers, the clientversion header and the bitcoin-config header. 14:15:14 <TheCharlatan> cfields opened a RFC pr for the clientversion https://github.com/bitcoin/bitcoin/pull/28327 14:15:20 <TheCharlatan> Likewise, I opened a RFC pr for multi index https://github.com/bitcoin/bitcoin/pull/28335 14:15:26 <TheCharlatan> The goal with these is to receive some feedback on the approaches and concept. 14:16:09 <TheCharlatan> that's it :) 14:16:40 <cfields> No need to look at 28327, I believe it'll be obsoleted by #25284 which IS interesting and worth reviewing :) 14:16:43 <gribble> https://github.com/bitcoin/bitcoin/issues/25284 | net: Use serialization parameters for CAddress serialization by MarcoFalke ÷ Pull Request #25284 ÷ bitcoin/bitcoin ÷ GitHub 14:17:05 <sipa> (is on my list, should be easy as i think it's based on my code...) 14:17:45 <achow101> #topic assumeutxo updates (jamesob) 14:18:52 <fjahr> if he is not here, some review progress on #27596, not much else I think 14:18:55 <gribble> https://github.com/bitcoin/bitcoin/issues/27596 | assumeutxo (2) by jamesob ÷ Pull Request #27596 ÷ bitcoin/bitcoin ÷ GitHub 14:19:06 <achow101> #topic Ad-hoc high priority for review 14:19:51 <josie> would love to see https://github.com/bitcoin/bitcoin/pull/28246 get merged 14:19:57 <achow101> Anything to add or remove from https://github.com/orgs/bitcoin/projects/1/views/4 14:20:46 <josie> in general, I think it makes a lot of sense on its own, and I'm also building off of it in #28122 14:20:49 <gribble> https://github.com/bitcoin/bitcoin/issues/28122 | Silent Payments: Implement BIP352 by josibake ÷ Pull Request #28122 ÷ bitcoin/bitcoin ÷ GitHub 14:21:49 <achow101> josie: added it to the board 14:22:10 <Murch> IâÂÂd love to get more review on the Ancestor Aware Funding PR, #26152 14:22:13 <gribble> https://github.com/bitcoin/bitcoin/issues/26152 | Bump unconfirmed ancestor transactions to target feerate by murchandamus ÷ Pull Request #26152 ÷ bitcoin/bitcoin ÷ GitHub 14:22:33 <Murch> I think itâÂÂs pretty close, but currently has only 8 concept ACKs and one stale ACK 14:23:09 <achow101> Murch: will look at it, eventually 14:23:40 <Murch> Oh, 2 ACKs now :) 14:23:52 <achow101> #topic log api (_aj_) 14:24:50 <_aj_> hi, jon atack's been working on severity based logging for a while - #25203 - and been advocating for switching to its api in various prs eg https://github.com/bitcoin/bitcoin/pull/28364#discussion_r1310579410 14:24:53 <gribble> https://github.com/bitcoin/bitcoin/issues/25203 | Severity-based logging -- parent PR by jonatack ÷ Pull Request #25203 ÷ bitcoin/bitcoin ÷ GitHub 14:25:58 <_aj_> i think it looks really clunky, `LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "sendtxrcncl received ...` so a while ago suggested simplifying it, and more recently did a concrete PR #28318 based on that (giving LogDebug(BCLog::NET, "sendtxrc...") instead) 14:26:00 <gribble> https://github.com/bitcoin/bitcoin/issues/28318 | logging: Simplify API for level based logging by ajtowns ÷ Pull Request #28318 ÷ bitcoin/bitcoin ÷ GitHub 14:26:39 <_aj_> just wondered if people had thoughts, or if everyone's already agreed on LogPrintLevel(NET, DEBUG, "...") everywhere or ... 14:27:49 <sipa> i don't have a particularly strong opinion, but avoiding clutter sounds nice 14:27:55 <achow101> less typing is more better? 14:28:00 <fanquake> It's not completely clear to me what the project is (I don't think it's been active for a while, until 20576 was reopened and description rewritten in the last hour?), what the benefits are, or what needs to be done (guess slightly clearer now with new PR description). The RFC/motivation in #20576 doesn't have much discussion at all. 14:28:01 <TheCharlatan> _aj_ I prefer LogDebug style 14:28:02 <gribble> https://github.com/bitcoin/bitcoin/issues/20576 | RFC on logging improvements ÷ Issue #20576 ÷ bitcoin/bitcoin ÷ GitHub 14:28:15 <fjahr> I does look clunky but I guess having a shorter alias for LogPrintLevel would also work for me, so no strong opinion... 14:28:33 <stickies-v> agreed that having clunky logging interface for the most common situations is not great, your approach in 28318 looks much cleaner indeed 14:28:59 <fanquake> Given it's not a priority project, I don't think it should be a blocker for PRs like 28364, especially when for contributors it's not necessarily clear what should be done 14:29:08 <Murch> Is jon_atack here perhaps? 14:29:26 <_aj_> assume not, but we could continue the conversation on the PRs? 14:30:07 <sipa> i think so 14:30:40 <achow101> Any other topics to discuss? 14:30:49 <fjahr> some probably read the previous conversation here, pinheadmz made a new github bot that currently lives in #bitcoin-core-github and now emulates the behavior of the matrix bridge/gh-bot. If people like it could be pointed on this channel as well. But I guess itâÂÂs also fine to have a separate channel, at least that also works for me. Just wanted to put it out there as an option. 14:31:03 <_aj_> fjahr: (yeah, i just made LogDebug an macro replacing with LogPrintLevel in my pr) 14:31:09 <fanquake> I assume a lot of the necessary changes could also be done with scripted diffs at some point? Rather than making each induvidual PR make changes etc. 14:31:42 <glozow> so should I just leave #28364 as is? I do personally prefer the less verbose api... 14:31:44 <gribble> https://github.com/bitcoin/bitcoin/issues/28364 | log: log wtxids when possible, add TXPACKAGES category by glozow ÷ Pull Request #28364 ÷ bitcoin/bitcoin ÷ GitHub 14:31:47 <josie> +1 that log severity project should not be a blocker for package relay PRs 14:32:18 <fanquake> I think 28364 could be merged as-is, up to you though 14:32:27 <josie> glozow: +1 for leave as is, esp since it seems like discussion on the log stuff will be ongoing 14:32:32 <instagibbs> priority project rule #1 pretty much 14:32:34 <gribble> https://github.com/bitcoin/bitcoin/issues/1 | JSON-RPC support for mobile devices ("ultra-lightweight" clients) ÷ Issue #1 ÷ bitcoin/bitcoin ÷ GitHub 14:32:48 <instagibbs> if/when direction on logging is set, things can be batch- changed 14:33:06 <lightlike> fanquake: I agree. I think if we change, we should change all logs of a given category together, and have some high-level plan of what kind of things goes into which severity level. 14:33:36 <glozow> ok awesome. maybe when we're all together we can have a deeper logging discussion. i will leave it as is and after it's merged, I'll open a PR with the txorphanage changes + tests. 14:34:49 <Murch> If people feel that it would be a worthwhile change, we could perhaps make the replacement of the logging plumbing a focus in the next release cycle? 14:34:55 <fanquake> lightlike: yea, the "what belongs in which catergory/severity level" isn't quite clear to me yet, and compunded with the unconditional logging and other things 14:35:06 <Murch> seems like a big enough thing to have a bunch of people coordinate on it 14:35:13 <instagibbs> I heard there's a meeting soon(TM), seems like a good topic 14:35:19 <_aj_> c++20 and cmake could make for a lot of plumbing changes next cycle 14:36:43 <fanquake> Better oil up your wrench 14:36:48 <MacroFake> sed -i 's|Span|std::span|g' $( git grep -l Span ) 14:37:01 <_aj_> using Span = std::span :) 14:37:04 <Murch> Modules are only expected with c++23, right? :( 14:37:05 <achow101> #endmeeting