💬 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.
(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.
(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?
(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
(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
(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
(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
(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
(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
(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
(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!
(https://github.com/bitcoin/bitcoin/pull/29686#issuecomment-2466758707)
@willcl-ark Thanks!
👍 willcl-ark approved a pull request: "doc: Fixup bitcoin-wallet manpage chain selection args"
(https://github.com/bitcoin/bitcoin/pull/31264#pullrequestreview-2425778116)
utACK fa729ab4a276c3462e390bf2fab6cad93d3a590d
Thanks Marco, this makes sense to me.
(https://github.com/bitcoin/bitcoin/pull/31264#pullrequestreview-2425778116)
utACK fa729ab4a276c3462e390bf2fab6cad93d3a590d
Thanks Marco, this makes sense to me.
💬 Sjors commented on pull request "Prune mining interface":
(https://github.com/bitcoin/bitcoin/pull/31196#discussion_r1835720824)
Fixed
(https://github.com/bitcoin/bitcoin/pull/31196#discussion_r1835720824)
Fixed
💬 Sjors commented on pull request "Prune mining interface":
(https://github.com/bitcoin/bitcoin/pull/31196#discussion_r1835720992)
I updated the commit message to include a reference to a74b0f93efa1d9eaf5abc2f6591c44a632aec6ed where this check was added.
(will push shortly)
(https://github.com/bitcoin/bitcoin/pull/31196#discussion_r1835720992)
I updated the commit message to include a reference to a74b0f93efa1d9eaf5abc2f6591c44a632aec6ed where this check was added.
(will push shortly)
📝 Sjors opened a pull request: "GitHub skip branch"
(https://github.com/bitcoin/bitcoin/pull/31265)
Testing https://github.com/bitcoin/bitcoin/pull/30487
(https://github.com/bitcoin/bitcoin/pull/31265)
Testing https://github.com/bitcoin/bitcoin/pull/30487
✅ Sjors closed a pull request: "GitHub skip branch"
(https://github.com/bitcoin/bitcoin/pull/31265)
(https://github.com/bitcoin/bitcoin/pull/31265)
👍 tdb3 approved a pull request: "doc: Fixup bitcoin-wallet manpage chain selection args"
(https://github.com/bitcoin/bitcoin/pull/31264#pullrequestreview-2425820823)
Code Review ACK fa729ab4a276c3462e390bf2fab6cad93d3a590d
Would also support adding an example:
```diff
- "To change the target wallet, use the -datadir, -wallet and (test)chain selection arguments.\n"
+ "To change the target wallet, use the -datadir, -wallet and (test)chain selection arguments (e.g. -signet).\n"
```
(https://github.com/bitcoin/bitcoin/pull/31264#pullrequestreview-2425820823)
Code Review ACK fa729ab4a276c3462e390bf2fab6cad93d3a590d
Would also support adding an example:
```diff
- "To change the target wallet, use the -datadir, -wallet and (test)chain selection arguments.\n"
+ "To change the target wallet, use the -datadir, -wallet and (test)chain selection arguments (e.g. -signet).\n"
```
💬 Sjors commented on pull request "ci: skip Github CI on branch pushes for forks":
(https://github.com/bitcoin/bitcoin/pull/30487#issuecomment-2466833320)
Github documentation is pretty terrible. The change here should work, but the jobs are not skipped in https://github.com/Sjors/bitcoin/pull/70
(https://github.com/bitcoin/bitcoin/pull/30487#issuecomment-2466833320)
Github documentation is pretty terrible. The change here should work, but the jobs are not skipped in https://github.com/Sjors/bitcoin/pull/70
👍 andrewtoth approved a pull request: "rpc: increase the defaults for -rpcthreads and -rpcworkqueue"
(https://github.com/bitcoin/bitcoin/pull/31215#pullrequestreview-2425823091)
ACK e56fc7ce6a92eae7e80657d9f57a148cc002358d
Bumping the default threads and work queue enables a lot of workflows by default, like using it with a local address indexer. Right now these users have to manually configure the higher limits.
The higher limits won't increase memory unless they are being used. And these defaults can be set lower on memory constrained systems to disable using them.
(https://github.com/bitcoin/bitcoin/pull/31215#pullrequestreview-2425823091)
ACK e56fc7ce6a92eae7e80657d9f57a148cc002358d
Bumping the default threads and work queue enables a lot of workflows by default, like using it with a local address indexer. Right now these users have to manually configure the higher limits.
The higher limits won't increase memory unless they are being used. And these defaults can be set lower on memory constrained systems to disable using them.
👍 tdb3 approved a pull request: "ci: make ctest stop on failure"
(https://github.com/bitcoin/bitcoin/pull/31257#pullrequestreview-2425829015)
code review and test ACK 36a22e5683375b7925767de9daa9df4c48831c15
- Grepped for `ctest` in other directories (in case other files should be adjusted). Didn't see anything else to update.
- Sanity checked by inserting an intentional failure (`BOOST_CHECK_EQUAL(1, 2)`) into `addrman_simple` and ran `ctest` locally as defined in the CI ymls to confirm the commands with `--stop-on-failure` behave as expected (i.e. stop early). Behaved as expected.
(https://github.com/bitcoin/bitcoin/pull/31257#pullrequestreview-2425829015)
code review and test ACK 36a22e5683375b7925767de9daa9df4c48831c15
- Grepped for `ctest` in other directories (in case other files should be adjusted). Didn't see anything else to update.
- Sanity checked by inserting an intentional failure (`BOOST_CHECK_EQUAL(1, 2)`) into `addrman_simple` and ran `ctest` locally as defined in the CI ymls to confirm the commands with `--stop-on-failure` behave as expected (i.e. stop early). Behaved as expected.