Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 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
💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1835681261)
Done
💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1835681273)
Done
💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1835681298)
Done
💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1835681312)
Done
💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1835681334)
> check that pointers are non-null if flags are set, or go further and check prev->next == self, and next->prev == self

the latter seems redundant, but the first is simple to check - done
💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#issuecomment-2466714671)
Thanks @ryanofsky & @andrewtoth, applied most nits, please re-review: `git range-diff a6921049..aa2f3139 a6921049..9edbe5a4` or https://github.com/bitcoin/bitcoin/compare/3b5b2457f713014fd5073da34b76a5a8cd796e4a..9edbe5a48fe9a2c0d364882fcb7d44ae6aac6d24
💬 BebeSparkelSparkel commented on pull request "Update manpage descriptions":
(https://github.com/bitcoin/bitcoin/pull/29686#issuecomment-2466758707)
@willcl-ark Thanks!