💬 Sjors commented on issue "depens: bdb build fails on Intel macOS 13.6.6 ":
(https://github.com/bitcoin/bitcoin/issues/29792#issuecomment-2039881846)
I'm still running the test suite to see if it's not broken. The machine is quite slow.

At minimum we should add gnu patch to the list of requirements / recommendations, at least for macOS < 14.

🚀 fanquake merged a pull request: "depends: build libqrencode with CMake"
(https://github.com/bitcoin/bitcoin/pull/29725)

💬 m3dwards commented on issue "IBD performance regression in 27.0rc1 on Windows":
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2039911554)
I can partially replicate this regression using pre-built binaries on Windows 11. For me, `27.0rc1` is **15% slower** than `26.0`. I can't quite replicate the higher variability of `27.0rc1` as it had a slightly higher range but a lower standard deviation.

**26.0**

Mean: 35.21 seconds
Range: 9 seconds
Standard Deviation: 2.25


**27.0rc1**

Mean: 40.77 second
...

👍 ryanofsky approved a pull request: "Logging cleanup"
(https://github.com/bitcoin/bitcoin/pull/29798#pullrequestreview-1983292855)
Code review ACK d98ca056a82264884bc1aa2da3af540a73ee8f26

Nice cleanups! Main feedback I have is to more clearly indicate which commits are refactoring, and which have behavior changes. Would suggest putting "logging, refactor:" in refactoring commit titles and "logging:" in commits that change behavior. Also I would consider dropping any behavior changes that aren't obvious improvments and doing them in separate PRs so they have more visibility and are not lost in "logging cleanups."

💬 ryanofsky commented on pull request "Logging cleanup":
(https://github.com/bitcoin/bitcoin/pull/29798#discussion_r1553726631)
In commit "rpc: remove misleading text from logging help" (e78866a10973c64a59e8a71eed737328b6bc0ca3)

Right now this returns NONE/true when the empty string "" is passed. Should return false in that case too and never return NONE?

Also, this is a nice change, but I found description confusing because the whole thing sounds it this like a help text change, but then the last sentence mentions it also adds a new error. Would suggest a description more like:

```
rpc: make logging method rej
...

💬 ryanofsky commented on pull request "Logging cleanup":
(https://github.com/bitcoin/bitcoin/pull/29798#discussion_r1553719054)
In commit "logging: minor encapsulation improvement and use BCLog::NONE instead of 0" (1fae74149507e5914a7bb7986e4906053b8dbbbb)

Would be good to add "refactor:" prefix to commit message so it is obvious this is not supposed to change behavior.

💬 ryanofsky commented on pull request "Logging cleanup":
(https://github.com/bitcoin/bitcoin/pull/29798#discussion_r1553715178)
In commit "logging: move special cases to GetLogCategory()" (9fb321ad2b0176955718317b796715250eac01f4)

Would be good if commit title had a "refactor:" prefix so it is clear this is not intended to change behavior.

Also, I think this commit should remove ALL and NONE cases from the map, so special cases are handled completely within the GetLogCategory() and LogCategoryToStr() functions, and there are no more special cases in the map and LogCategoriesList(). Would suggest:

```diff
--- a/
...

💬 ryanofsky commented on pull request "Logging cleanup":
(https://github.com/bitcoin/bitcoin/pull/29798#discussion_r1553729155)
re: https://github.com/bitcoin/bitcoin/pull/29798#discussion_r1549650210
In commit "logging: remove unused code" (d98ca056a82264884bc1aa2da3af540a73ee8f26)

> Would you prefer an assert

I'd suggest just dropping this commit entirely. If you adopt my suggestion from the first commit the `{"", BCLog::NONE}` map entry already disappears so this does not provide an additional simplification to the map, and I don't think there's really another reason to change GetLogPrefix behavior here, at le
...

💬 dergoegge commented on pull request "build: Enable fuzz binary in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29774#issuecomment-2039930366)
> One example on line 282 of transaction_tests.cpp:

I believe this is unrelated to this PR, see https://github.com/bitcoin/bitcoin/issues/29051. `make clean` and rebuilding should fix it.

👋 fanquake's pull request is ready for review: "depends: build miniupnpc with CMake"
(https://github.com/bitcoin/bitcoin/pull/29707)

💬 glozow commented on pull request "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure":
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1553735891)
"or similarly small" doesn't really make sense to me. AFAICT this makes just enough transactions to go past the 5MB mempool.

💬 glozow commented on pull request "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure":
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1553744185)
Mempool is already empty?

💬 glozow commented on pull request "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure":
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1553749001)
Mempool is already empty
```suggestion
# Reset maxmempool, datacarriersize, and dynamic mempool minimum feerate
```

💬 glozow commented on pull request "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure":
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1553736451)
nit
```suggestion
Allows for simpler testing of scenarios with minfee > minrelay
```

💬 glozow commented on pull request "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure":
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1553746827)
Note that this can change the mempool min fee

📝 Sjors opened a pull request: "depends: suggest GNU patch"
(https://github.com/bitcoin/bitcoin/pull/29814)
Fixes #29792

Adds a note to `depends/README.md` that on older versions of macOS you _may_ need to use GNU patch. I phrased as a troubleshooting hint, rather than make it a default recommendation, because:

1. so far only I ran into it, and @fanquake couldn't reproduce
2. BDB is only used for the legacy wallet, which itself is deprecated
3. it seems better to replace as few default tools on macOS as possible

The instructions in `build-osx.md` don't even mention depends, so I don't touch
...

💬 TheCharlatan commented on pull request "depends: swap some cctools tools for LLVM tools":
(https://github.com/bitcoin/bitcoin/pull/29739#issuecomment-2039959065)
Tested the cross-compiled binaries and deployed bundle on macos 12.7.4.

🤔 glozow reviewed a pull request: "feefrac: avoid explicitly computing diagram; compare based on chunks"
(https://github.com/bitcoin/bitcoin/pull/29757#pullrequestreview-1983373180)
reACK c2fdcf4

💬 instagibbs commented on pull request "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure":
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1553766100)
mm yeah let me make this a bit more bullet proof

💬 Sjors commented on pull request "net: update comment for service bit support info for seed.bitcoin.sipa.be":
(https://github.com/bitcoin/bitcoin/pull/29809#issuecomment-2039962940)
You can't know which configuration @sipa - or anyone else running that code - is using. And keeping track of all permutations gets quite noisy.

For my own seed I've occasionally added more filters for new features that were being developed. I wouldn't want to keep making pull requests to change the comment (mine doesn't mention any).

Perhaps this tool can be modified to scan which filters are in use: https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/main/check-dnsseeds.py