💬 edilmedeiros commented on pull request "doc: Fixup bitcoin-wallet manpage chain selection args":
(https://github.com/bitcoin/bitcoin/pull/31264#issuecomment-2466218323)
Concept ACK.

I understand and like the rational for not being repetitive, but don't you think the terminology might be confusing? I could not think of any better alternatives, though.

💬 edilmedeiros commented on pull request "doc: Fix dead links to mailing list archives":
(https://github.com/bitcoin/bitcoin/pull/31240#issuecomment-2466219877)
I personally prefer to update the links because I can't imagine the mess of updating this again in the future if gnusha is taken down and we end up with two indirections on the URLs.

💬 theStack commented on pull request "Package validation: accept packages of size 1":
(https://github.com/bitcoin/bitcoin/pull/31096#issuecomment-2466227961)
Concept ACK

💬 furszy commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1835445136)
sure. Done as suggested.

💬 furszy commented on issue "importdescriptors always rescans":
(https://github.com/bitcoin/bitcoin/issues/31263#issuecomment-2466279297)
> The docs say that '"now" can be specified to bypass scanning' but that doesn't appear to be true

The message also specify that "Blocks up to 2 hours before the earliest timestamp of all descriptors being imported will be scanned as well as the mempool".
This occurs regardless of the provided timestamp. It serves as a grace period to account for possible variations in block times.

👍 tdb3 approved a pull request: "bench: add support for custom data directory"
(https://github.com/bitcoin/bitcoin/pull/31000#pullrequestreview-2425519599)
re ACK b842ad61c7d9507db415b5d6225f5444f6bf2654

Liking the use of time rather than random.

Re-ran https://github.com/bitcoin/bitcoin/pull/31000#pullrequestreview-2401711134

```
'tmp/test_common bitcoin/BlockAssemblerAddPackageTxns/1731173028231274080'
```

💬 tdb3 commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1835472395)
non-blocking nit:
Since time is being used now, the name isn't quite as random as before. Could update the comment and `rand_str` name.

💬 furszy commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1835476601)
sure, pushed. Thanks

👍 tdb3 approved a pull request: "test: Shut down framework cleanly on RPC connection failure"
(https://github.com/bitcoin/bitcoin/pull/30660#pullrequestreview-2425524929)
Code Review re ACK 68b0530d5fe867dbee5e551ecf6d491fecbc77f1

👍 tdb3 approved a pull request: "bench: add support for custom data directory"
(https://github.com/bitcoin/bitcoin/pull/31000#pullrequestreview-2425525188)
re ACK 5adb4a44776de3f9295cd6bf965f8beba0e4e368

Change since last push is a simple removal of a semi-accurate comment.

👍 tdb3 approved a pull request: "test: Introduce ensure_for helper"
(https://github.com/bitcoin/bitcoin/pull/30893#pullrequestreview-2425525972)
Code Review re ACK f04529fc40f2f7c8e4e63d892cdcbe6c91dfbf96

Would also review again if the suggestion in https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1834183665 was implemented.

✅ achow101 closed an issue: "importdescriptors always rescans"
(https://github.com/bitcoin/bitcoin/issues/31263)

💬 achow101 commented on pull request "test: Rework wallet_migration.py to use previous releases":
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1835479468)
Oops yes.

💬 secp512k2 commented on pull request "doc: Fix missing comma in JSON example in REST-interface.md":
(https://github.com/bitcoin/bitcoin/pull/31259#issuecomment-2466381262)
> LLM-PR?
>
> Not sure how useful it is to fix the example output in an interface doc. LLM operator and reviewer time is probably spent better elsewhere.

Not a bot. Just thought having accurate docs would be helpful for a newcomer troubleshooting. I understand the limited PR review capability though, and will limit to more major things.

💬 luke-jr commented on pull request "util: Narrow scope of args (-color, -version, -conf, -datadir)":
(https://github.com/bitcoin/bitcoin/pull/31212#issuecomment-2466406625)
>Disallow -noconf since a config file is required

But it's not. Seems like `-noconf` should work to ignore the config file.

🤔 tdb3 reviewed a pull request: "rpc: increase the defaults for -rpcthreads and -rpcworkqueue"
(https://github.com/bitcoin/bitcoin/pull/31215#pullrequestreview-2425588523)
Approach ACK

Performed a very simplistic sanity check comparing 6463117a29294f6ddc9fafecfd1e9023956cc41b (commit under head) to head (e56fc7ce6a92eae7e80657d9f57a148cc002358d).

Ran `bitcoin-cli gettxoutsetinfo` 8x simultaneously on a system with 4 cores and timed the result (not claiming this to be the best test, just a quick check). The idea was to see if a core-constrained system would see a performance degradation from the PR change.

Without the increase of rpcthreads and rpcworkqueu
...

💬 maciejsszmigiero commented on pull request "dbwrapper: Bump LevelDB max file size to 128 MiB to avoid system slowdown from high disk cache flush rate":
(https://github.com/bitcoin/bitcoin/pull/30039#issuecomment-2466468043)
@l0rinc
> are you still working on this or should we take over?

I can obviously change the default in this PR to 16 MiB but I think having #30059 is important too: as you measured here on Oct 2 the best performing size on HDD storage actually seems to be 32 MiB.

💬 fjahr commented on pull request "scripted-diff: get rid of remaining "command" terminology in protocol.{h,cpp}":
(https://github.com/bitcoin/bitcoin/pull/31163#issuecomment-2466520047)
Code review ACK 4120c7543ee32efed7396d7153411ecbbd588ad3

Did some light checks to see if anything was missed and reviewed changes.

💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1835675175)
Wouldn't I need to make `AddFlags` static as well in that case?
Which would trigger `this` removal and rewrite of the whole `AddFlags` method which is done in the next commit. Or you mean squash the first two commits?

💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1835681246)
Done