14:00:25 <achow101> #startmeeting 14:00:25 <core-meetingbot> Meeting started Thu Oct 19 14:00:25 2023 UTC. The chair is achow101. Information about MeetBot at https://bitcoin.jonasschnelli.ch/ircmeetings. 14:00:25 <core-meetingbot> Available commands: action commands idea info link nick 14:00:30 <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:41 <vasild> hi 14:00:43 <stickies-v> hi 14:00:45 <pinheadmz> hi 14:00:47 <glozow> hi 14:00:47 <instagibbs> hi 14:00:51 <josie> hi 14:00:55 <sdaftuar> hi 14:00:56 <theStack> hi 14:00:58 <dergoegge> hi 14:00:58 <furszy> hi 14:01:04 <hebasto> hi 14:01:04 <_aj_> hi 14:01:05 <achow101> There is 1 pre-proposed meeting topic this week. any last minute ones to add to the list? 14:01:07 <gleb> Hi. Poor internet here 14:01:18 <fjahr> hi 14:01:26 <darosior> Good day 14:01:51 <achow101> #topic priority projects 14:01:51 <core-meetingbot> topic: priority projects 14:02:19 <TheCharlatan> hi 14:02:27 <abubakarsadiq> hi 14:03:20 <achow101> The final count for the voting is Package relay - 19, Silent payments - 11, Multiprocess - 9, Legacy wallet removal - 9, cmake - 8, erlay - 7 stratum v2 - 4 14:03:52 <kanzure> hi 14:04:25 <achow101> We decided at CoreDev to go with the top 3 projects instead of 4, so that would be package relay, silent payments, and one of multiprocess or legacy wallet removal. 14:04:41 <pinheadmz> ooh a runoff 14:05:13 <achow101> i would be happy to have multiprocess as the priority project since legacy wallet removal has a plan to move forward this release anyways 14:05:40 <_aj_> i count 9 for cmake? 14:05:54 <instagibbs> cmake is happening when it's happening regardless iiuc 14:06:01 <_aj_> true :) 14:06:03 <instagibbs> Two Weeks(TM) whenever ready 14:06:04 <fanquake> Sometime after c++20 14:06:19 <TheCharlatan> ^^ 14:06:26 <_aj_> fanquake: woah</keanu> 14:06:27 <achow101> _aj_: i've not included w0xlt as they aren't in the org, and haven't seemed to be active recently? 14:06:42 <_aj_> achow101: ok 14:09:07 <achow101> any other thoughts on multiprocess vs. legacy wallet removal? 14:10:17 <vasild> which one of mine did you count? 14:10:34 <achow101> vasild: the signaling one 14:11:05 <instagibbs> legacy wallet removal is your baby, if you think it's fine not being priority it's probably fine? 14:11:33 <fjahr> achow101: if you are fine with multiprocessing taking the 3rd spot sounds good to me, I think multiprocess certainly needs more attention to make progress 14:11:39 <_aj_> removing code seems easier to rebase than adding code, maybe? 14:12:34 <b10c> hi 14:12:40 <sipa> hi 14:12:42 <achow101> _aj_: you'd think so. but I started rebasing my 2 year old removal branch and it's not a fun time 14:14:46 <fjahr> _aj_ I think it's the same, it just depends on how intermingled the change is with the rest of the code. 14:14:52 <achow101> is there a tracking issue for multiprocess? 14:15:11 <glozow> is ryan here? 14:15:16 <instagibbs> ryanofsky ping 14:15:24 <sipa> just pinged him IRL 14:15:33 <ryanofsky> no, can easily create one 14:15:37 <instagibbs> In RimworLd 14:15:38 <furszy> I think that people working on the wallet will continue reviewing PRs (or at least I will) vs multiprocess that needs some momentum 14:15:43 <fjahr> There are #10102 and https://github.com/bitcoin/bitcoin/projects/10 14:15:46 <gribble> https://github.com/bitcoin/bitcoin/issues/10102 | Multiprocess bitcoin by ryanofsky ÷ Pull Request #10102 ÷ bitcoin/bitcoin ÷ GitHub 14:16:10 <achow101> https://github.com/orgs/bitcoin/projects/1/views/2 is updated 14:16:11 <_aj_> projects/10 is closed/read-only though 14:16:22 <stickies-v> there's also https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Process-Separation 14:16:43 <ryanofsky> right now all the code is just in 10102, there are many commits that could be split up 14:19:11 <achow101> #topic Ad-hoc high priority for review 14:19:11 <core-meetingbot> topic: Ad-hoc high priority for review 14:19:18 <achow101> Anything to add or remove from https://github.com/orgs/bitcoin/projects/1/views/4 14:19:31 <pinheadmz> I wanna shill #27375 again 14:19:34 <gribble> https://github.com/bitcoin/bitcoin/issues/27375 | net: support unix domain sockets for -proxy and -onion by pinheadmz ÷ Pull Request #27375 ÷ bitcoin/bitcoin ÷ GitHub 14:19:44 <pinheadmz> has 2 ACKs, follwoups will offer unix sockets for rpc and zmq as well 14:19:49 <pinheadmz> imagine no more http://localhost! 14:21:12 <achow101> cool, will look when we start merging for 27.0 14:22:16 <achow101> #topic 26.0 milestone 14:22:17 <core-meetingbot> topic: 26.0 milestone 14:22:36 <achow101> branching is scheduled for sunday. there's still a bunch of things in the milestone 14:22:40 <achow101> https://github.com/bitcoin/bitcoin/milestone/60 14:23:00 <achow101> please review them 14:23:43 <achow101> #topic Fix for hash_serialized2 calculation and implications (for context see #28675 and #28685) (fjahr) 14:23:43 <core-meetingbot> topic: Fix for hash_serialized2 calculation and implications (for context see #28675 and #28685) (fjahr) 14:23:44 <gribble> https://github.com/bitcoin/bitcoin/issues/28675 | Assumeutxo: Altered txoutset dump is still valid ÷ Issue #28675 ÷ bitcoin/bitcoin ÷ GitHub 14:23:45 <gribble> https://github.com/bitcoin/bitcoin/issues/28685 | coinstats, assumeutxo: fix hash_serialized2 calculation by fjahr ÷ Pull Request #28685 ÷ bitcoin/bitcoin ÷ GitHub 14:23:46 <gribble> https://github.com/bitcoin/bitcoin/issues/28675 | Assumeutxo: Altered txoutset dump is still valid ÷ Issue #28675 ÷ bitcoin/bitcoin ÷ GitHub 14:23:47 <gribble> https://github.com/bitcoin/bitcoin/issues/28685 | coinstats, assumeutxo: fix hash_serialized2 calculation by fjahr ÷ Pull Request #28685 ÷ bitcoin/bitcoin ÷ GitHub 14:23:56 <fjahr> tldr; the hash_serialized_2 calculation had a bug as described here by theStack: https://github.com/bitcoin/bitcoin/issues/28675#issuecomment-1770389468 The fix is in #28685 and it will be renamed to hash_serialized_3. 14:23:57 <gribble> https://github.com/bitcoin/bitcoin/issues/28685 | coinstats, assumeutxo: fix hash_serialized2 calculation by fjahr ÷ Pull Request #28685 ÷ bitcoin/bitcoin ÷ GitHub 14:24:05 <fjahr> This has implications particularly for assumeutxo, the hashes in the chainparams will need to change. But we are also discussing a bunch of additional changes to improve further while we change the resulting hash already. 14:24:37 <fjahr> Particularly dergoegge found another issue with his fuzzer on negative values and proposed to get rid of VARINT completely. I would really like to hear if people would like do that or if we should keep the change more minimal. I have started preparing the change with the VARINTs removed but it would be good to know which one we want now so we only have to change the chainparams once since that causes some significant review 14:24:37 <fjahr> effort. 14:25:01 <fjahr> To be precise I think we have the choice between getting rid of all VARINTs in kernel/coinstats, getting rid of it just where dergoegge found the issue and leaving it and checking for negative value in deserialization (I guess we will add this check either way). 14:25:37 <fjahr> So generally would be good to have a few more eyes on this but particularly looking for feedback on the VARINT question. 14:26:01 <sipa> For performance reasons, my guess would be that removing all VARINTs from UTXO hashes is better. 14:26:03 <theStack> as commented in the PR, I think removing VARINTs in `ApplyHash` would make sense, but is only an option if that doesn't come with noticable loss in performance 14:26:26 <sipa> I suspect that VARINT coding costs per-byte more than SHA256 per-byte. 14:26:37 <sipa> But I haven't benchmarked it. 14:26:41 <dergoegge> something that also seems weird to me is that the serialization format is different for hashing with muhash 14:27:07 <theStack> sipa: oh, interesting, i would have expected it's the other way round. but i don't know too much about sha256 internals TBH 14:27:24 <fjahr> I think at the time we already new the muhash one made more sense, just kept the hash_serialized one around for consistency 14:27:33 <fjahr> *knew 14:27:58 <sipa> SHA256 (without hardware acceleration) is maybe a dozen CPU cycles per byte; varint coding involves lots of branches that can be mispredicted... less work overall, but probably a lot lower ILP 14:29:01 <sipa> Does the fact that it may impact chainparams really matter? I can imagine it's likely we want to revisit the actual hashing scheme for assumeutxo before mainnet deployment anyway, depending on how P2P serving would happen. 14:29:22 <achow101> was hash_serialized_2 always incorrect? 14:29:36 <fjahr> That would be another option, to apply the MuHash serialization so we are consistent 14:29:43 <fjahr> achow101: since 2018 14:29:47 <theStack> achow101: i think it's incorrect since v0.17 (see linked commit in the issue) 14:30:46 <theStack> at least that's the earliest tag that is shown by `git tag --contains 34ca75032012562d226b06ef9e86a2948d3a8d16` 14:30:59 <fjahr> sipa: at least we need to fix the testnet and signet params before the release 14:31:45 <sipa> fjahr: that doesn't sound like a big deal 14:31:48 <fjahr> not sure when we will even get into p2p distribution, if that's a focus for jamesob for 27 14:33:22 <fjahr> sipa: I just would like it if we can come to an agreement now then we don't need another follow-up to 28685 14:34:20 <achow101> if we change the serialization, will the coinstatsindex need to be reindexed? 14:34:36 <fjahr> no, the hash_serialized is not part of it 14:35:07 <sipa> i'm happy with either just using the muhash serialization for hash_serialized_3, or keeping VARINT 14:35:14 <sipa> but a benchmark would be useful 14:35:45 <achow101> why are the serialziations different? 14:36:19 <sipa> i suppose because they were designed at different times 14:37:15 <fjahr> what I wrote above, I think pieter came up with the one for muhash but we kept the old one for consistency of hash_serialized 14:37:36 <sipa> well i think i also came up with the one for hash_serialized 14:37:57 <fjahr> :) 14:38:11 <sipa> the muhash one is simpler, the hash_serialized one is more compact 14:38:17 <achow101> it'd be nice if they were consistent 14:38:47 <achow101> but it seems like people don't particularly care? 14:39:04 <sipa> for muhash i think performance matters less also, because the serialization cost is probably dwarfed by the muhash math overhead 14:39:30 <sipa> can someone benchmark if it matters at all, and if the more compact one is not significantly better, pick the muhash serialization for hash_serialized_3 ? 14:39:52 <fjahr> I can run the benchmarks and post them in the PR 14:40:30 <sipa> great 14:40:32 <achow101> sounds like a plan 14:41:01 <achow101> any other topics to discuss? 14:42:33 <MacroFake> Is the content hash needed, if the full file hash will be checked in the future? 14:42:55 <MacroFake> I guess to protect against the file changing while reading it? 14:43:55 <sipa> i could imagine introducing some kind of merkle-structured content hash in the future, that's used as a full file hash too 14:44:12 <sipa> but it sounds like there is a lot of design space for that 14:45:57 <bitcoin-git> [bitcoin] fanquake pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/9e616baec0f2...6e721c923c87 14:45:57 <bitcoin-git> bitcoin/master 2338715 fanquake: doc: add historical release notes for 25.1 14:45:58 <bitcoin-git> bitcoin/master 6e721c9 fanquake: Merge bitcoin/bitcoin#28667: doc: add historical release notes for 25.1 14:46:03 <bitcoin-git> [bitcoin] fanquake merged pull request #28667: doc: add historical release notes for 25.1 (master...add_25_1_rel_notes) https://github.com/bitcoin/bitcoin/pull/28667 14:46:04 <MacroFake> Are you saying with your proposal the full file hash would be equal to the content hash? 14:46:53 <MacroFake> (With full file hash I mean the dumb hash of the byte file, without parsing the content or looking at it) 14:47:01 <fjahr> I think flat file hash might be limiting as a hard requirement when we think about p2p distribution, not sure if it makes sense as temporary belt and suspenders 14:47:26 <sipa> no; i'm saying you'd verify the full file by computing its contents hash... which is designed in such a way that it's easy to validate the serialized file (and chunks of it) against 14:47:42 <sipa> and not having a dumb hash of the file at all 14:49:00 <sipa> i guess there could also be a dumb hash of the whole file as a final last-resort check, but for P2P distribution you really need a way to check for incorrect chunks very early anyway 14:51:10 <sipa> i guess it could be a tree-structured hash over the serialized file (and not over individual utxo entries in it)? 14:52:12 <achow101> seems like something that requires more thought than we can do for a fix for this release 14:52:13 <sipa> but this doesn't need to be part of this meeting, i think 14:52:30 <achow101> #endmeeting