💬 maflcko commented on pull request "i2p: log connection was refused due to arbitrary port":
(https://github.com/bitcoin/bitcoin/pull/29393#discussion_r1661142027)
This is reverted in https://github.com/bitcoin/bitcoin/pull/30345/files (as of now)

💬 andrewtoth commented on pull request "optimization: Speed up Base58 encoding/decoding by 400%/200% via preliminary byte packing":
(https://github.com/bitcoin/bitcoin/pull/29473#issuecomment-2200316763)
@paplorinc https://github.com/bitcoin/bitcoin/pull/28945 was abandoned and I don't think @fjahr has taken it over. Perhaps that would be interesting to you.

💬 brunoerg commented on pull request "i2p: log connection was refused due to arbitrary port":
(https://github.com/bitcoin/bitcoin/pull/29393#discussion_r1661167148)
left a comment there

📝 fjahr opened a pull request: "sync: improve CCoinsViewCache ReallocateCache - 2nd try"
(https://github.com/bitcoin/bitcoin/pull/30370)
Rebased and reopened #28945

Original description:

> TL;DR: this change improves sync time on my PC by ~6%.
>
> While syncing when the CCoinsViewCache gets full it is flushed to disk and then memory is freed. Depending on available cache size, this happens several times. This PR removes the unnecessary reallocations for temporary CCoinsViewCache and reserves the cacheCoins's bucket array to its previous size.
>
> I benchmarked the change on an AMD Ryzen 9 7950X with this command:
>
...

💬 fjahr commented on pull request "optimization: Speed up Base58 encoding/decoding by 400%/200% via preliminary byte packing":
(https://github.com/bitcoin/bitcoin/pull/29473#issuecomment-2200359855)
I have reopened https://github.com/bitcoin/bitcoin/pull/28945 now

💬 andrewtoth commented on pull request "optimization: Speed up Base58 encoding/decoding by 400%/200% via preliminary byte packing":
(https://github.com/bitcoin/bitcoin/pull/29473#issuecomment-2200364198)
Ahh sorry thanks.

🤔 glozow reviewed a pull request: "kernel: De-globalize validation caches"
(https://github.com/bitcoin/bitcoin/pull/30141#pullrequestreview-2151586199)
concept + light code review ACK 064a401e10f2b2da8f31f682929431d06c671d93

💬 glozow commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1661166403)
maybe a comment to document that we intend to make a copy of the pre-initialized hasher?

💬 glozow commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1661174570)
why `GetRandHash` instead of `ConsumeUint256`?

💬 glozow commented on pull request "kernel: remove mempool_persist":
(https://github.com/bitcoin/bitcoin/pull/30344#issuecomment-2200381417)
> Re [#30344 (comment)](https://github.com/bitcoin/bitcoin/pull/30344#issuecomment-2199482117)
>
> > lgtm, iiuc you'd want #30141 merged before this one?
>
> Mmh, I don't think this was said anywhere? Looks like the two are not really related either.

It seems they have a couple acks and conflict with each other? Was taking a guess based on https://github.com/bitcoin/bitcoin/pull/30344#issuecomment-2192453113

💬 theuni commented on pull request "kernel: remove mempool_persist":
(https://github.com/bitcoin/bitcoin/pull/30344#issuecomment-2200413138)
@glozow We both noticed this independently. @TheCharlatan Had it queued up as part of his next batch of changes (not yet PR'd), but I didn't know that when I PR'd this one. He's since agreed that it makes sense to merge this on its own.

#30141 is unrelated, he's just waiting for that to be merged before PR'ing his next batch of kernel changes.

💬 ishaanam commented on pull request "#27307 follow-up: update mempool conflict tests + docs":
(https://github.com/bitcoin/bitcoin/pull/30365#discussion_r1661201441)
Done

💬 ishaanam commented on pull request "#27307 follow-up: update mempool conflict tests + docs":
(https://github.com/bitcoin/bitcoin/pull/30365#discussion_r1661201934)
Done, thanks

💬 glozow commented on pull request "kernel: remove mempool_persist":
(https://github.com/bitcoin/bitcoin/pull/30344#issuecomment-2200424459)
> https://github.com/bitcoin/bitcoin/pull/30141 is unrelated, he's just waiting for that to be merged before PR'ing his next batch of kernel changes.

I can see that they aren't related, just trying to clarify which one to merge first because the PRs conflict. I figured this would be an ok place to ask that since it's the same people. Apologies, I wasn't trying to imply that they're related at all, my previous comment was to provide an explanation for why I was asking which one to prioritize.

📝 maflcko opened a pull request: "fuzz: Mutate -max_len= during generation phase"
(https://github.com/bitcoin/bitcoin/pull/30371)
This revives https://github.com/bitcoin/bitcoin/pull/28178#issuecomment-1927243781, because it helps to find https://github.com/bitcoin/bitcoin/issues/30367, which failed to be found for more than a year on any of the existing fuzz servers.

Locally, I can find the bug with `-max_len=84 -use_value_profile=1` on a single thread on a laptop. The reason is likely that a smaller max_len results in a faster fuzzing speed (iterations). As a side-effect it may also be effective at reducing the size o
...

💬 furszy commented on pull request "#27307 follow-up: update mempool conflict tests + docs":
(https://github.com/bitcoin/bitcoin/pull/30365#discussion_r1661216674)
can now be respent without manually abandoning the transactions "when the parent transaction is dropped from the mempool"?

👍 theuni approved a pull request: "ci: Clear unused /msan/llvm-project"
(https://github.com/bitcoin/bitcoin/pull/30369#pullrequestreview-2151691321)
utACK fa6beb8cfcabd58273f15e03f781016ac610e788

Could delete the sdk tarball as well.

💬 sipa commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#issuecomment-2200478173)
Rebased after the merge of #30237.

📝 maflcko opened a pull request: "util: Use SteadyClock in RandAddSeedPerfmon"
(https://github.com/bitcoin/bitcoin/pull/30372)
`GetTime` is mockable in tests and system-changeable in production. This should be fine and not lead to issues, but using `SteadyClock` is more correct in this context to do an expensive task only so often.

💬 vasild commented on pull request "test: fix debug log assertion in p2p_i2p_ports test":
(https://github.com/bitcoin/bitcoin/pull/30345#issuecomment-2200493378)
> I'm not 100% convinced of the change in this PR

Right. This looks quite bizarre to me. `Session::Hello()` creates the socket and it shouldn't be able to proceed since it shouldn't be able connect to `127.0.0.1:60000`. However, from the log message `Missing RESULT=` it seems it managed to call `SendRequestAndGetReply(*sock, "HELLO VERSION MIN=3.1 MAX=3.1");`, write to that socket, go a reply and is upset because the reply is missing `RESULT=`, and the reply is the same as the request :raised
...