💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1670655362)
dunno: https://sonarcloud.io/project/issues?id=aureleoules_bitcoin&branch=28280-e1076541e62972464193afb1f8d90ff178665ecb&resolved=false&open=AZCVwk_WMNEdWlV7XnLo

💬 glozow commented on pull request "system: use %LOCALAPPDATA% as default datadir on windows":
(https://github.com/bitcoin/bitcoin/pull/27064#discussion_r1670656006)
Btw, this file should go in the `doc/` dir instead of `doc/release-notes/` (which is the archive of old release notes by version)

📝 glozow opened a pull request: "[doc] archive v26.2 release notes"
(https://github.com/bitcoin/bitcoin/pull/30414)

💬 dergoegge commented on pull request "fuzz: Version handshake":
(https://github.com/bitcoin/bitcoin/pull/30413#issuecomment-2217941499)
> Are you sure it is completely non-existent?

The coverage we do have comes from the `process_messages` harness, but the harness doesn't fuzz the handshake it only makes sure that the handshake completes, so that the other messages can be fuzzed.

I'll generate a coverage report to show the difference

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2217946806)
The [benchmarks](https://corecheck.dev/bitcoin/bitcoin/pulls/28280) indicate that `BlockToJsonVerboseWrite` was slowed down

<img src="https://github.com/bitcoin/bitcoin/assets/1841944/8b5d5299-a4c2-4b26-9a74-44dd2504a20c">

I ran a before/after against `master`, found only a minor (4%) diff - is this expected?
> make -j10 && ./src/bench/bench_bitcoin --filter=BlockToJsonVerboseWrite --min-time=10000


before:
| ns/op | op/s | err% | total | benchmark
...

🤔 glozow reviewed a pull request: "net: fix race condition in self-connect detection"
(https://github.com/bitcoin/bitcoin/pull/30394#pullrequestreview-2166427591)
lgtm, do you plan to take any of the suggestions?

💬 glozow commented on pull request "net: fix race condition in self-connect detection":
(https://github.com/bitcoin/bitcoin/pull/30394#discussion_r1670590969)
nitty nit: this diff should be in f8594553fab6f865b13be4d5873b4ff55aae62d2, not e1349beed448cbecb276991019e78f0fccf579cd

💬 glozow commented on issue "Mini miner `AncestorFeerateComparator` Signed Integer Overflow":
(https://github.com/bitcoin/bitcoin/issues/30284#issuecomment-2217978085)
Ah, I think #30412 might fix this since `FeeFrac::Mul` handles the overflow? I'm not sure how to get the input from that CI task so I can check.

💬 glozow commented on pull request "Fee Estimation: Ignore all transactions that are CPFP'd":
(https://github.com/bitcoin/bitcoin/pull/30079#discussion_r1670728220)
instead of b1b28a21e185dfbc0c75f92ac55401a9ba8a5b08, I'd recommend rebasing on #30412
I was able to reproduce the ub crash and verify that 6254c253e995e77196b853784cfd0f42619b324d fixes it.

🤔 ismaelsadeeq reviewed a pull request: "MiniMiner: use FeeFrac in AncestorFeerateComparator"
(https://github.com/bitcoin/bitcoin/pull/30412#pullrequestreview-2166670402)
Concept ACK 271469b8bcb84af8f7a64c530e9cee7e0bf96352

💬 ismaelsadeeq commented on pull request "Fee Estimation: Ignore all transactions that are CPFP'd":
(https://github.com/bitcoin/bitcoin/pull/30079#discussion_r1670741214)
thanks I will rebase on it.

📝 ryanofsky opened a pull request: "contrib: fix check-deps.sh to check for weak symbols"
(https://github.com/bitcoin/bitcoin/pull/30415)
Fix check-deps.sh to check for weak symbols so it can detect when an exported template function like `TryParseHex` is used from another library.

Also add suppression because consensus library is currently using the `TryParseHex` function from util, when the consensus library is not supposed to depend on the util library.

Problem was reported by hebasto in https://github.com/bitcoin/bitcoin/pull/29015#issuecomment-2209258843

💬 darosior commented on pull request "PoC: fuzz chainstate and block managers":
(https://github.com/bitcoin/bitcoin/pull/29158#discussion_r1670749288)
When debating this with myself my weak argument in favour of it was that the target was very slow and this could help discover interesting paths faster. I also wanted your feedback on this, so thanks.

💬 theStack commented on pull request "net: fix race condition in self-connect detection":
(https://github.com/bitcoin/bitcoin/pull/30394#issuecomment-2218045821)
> lgtm, do you plan to take any of the suggestions?

Tackled all of the ordinary and nitty nits (thanks Marco and Gloria), re-review should be trivial.

💬 ismaelsadeeq commented on pull request "Fee Estimation: Ignore all transactions that are CPFP'd":
(https://github.com/bitcoin/bitcoin/pull/30079#discussion_r1670763260)
done

💬 sipa commented on pull request "net: fix race condition in self-connect detection":
(https://github.com/bitcoin/bitcoin/pull/30394#issuecomment-2218076997)
utACK a3122d38c65577cc13d88f8690da1e8f9d959c74, only reviewed the test change lightly.

🤔 sipa reviewed a pull request: "MiniMiner: use FeeFrac in AncestorFeerateComparator"
(https://github.com/bitcoin/bitcoin/pull/30412#pullrequestreview-2166777545)
utACK 271469b8bcb84af8f7a64c530e9cee7e0bf96352

💬 sipa commented on pull request "MiniMiner: use FeeFrac in AncestorFeerateComparator":
(https://github.com/bitcoin/bitcoin/pull/30412#discussion_r1670783767)
Nit: could be `return std::min(self_feerate, ancestor_feerate);`

🤔 mzumsande reviewed a pull request: "net: fix race condition in self-connect detection"
(https://github.com/bitcoin/bitcoin/pull/30394#pullrequestreview-2166824948)
I wonder if this could this lead to problems with "non-shy" software that would sends VERSION immediately on incoming connection (which bitcoin core doesn't do thanks to Hal Finney's #101, but alternative clients might).

When Bitcoin Core makes an outgoing connection after this patch, it won't send out the version message immediately, so it could receive a version message before sending one. In that case, I think that `ProcessMessage` could send out other messages (WTXIDRELAY etc.) before sen
...

🤔 BrandonOdiwuor reviewed a pull request: "rpc: doc: use "output script" terminology consistently in "asm"/"hex" results"
(https://github.com/bitcoin/bitcoin/pull/30408#pullrequestreview-2166843016)
ACK 29eafd5733d77b3e8f3f3ab6cd65c61ac0e8536b

LGTM