📝 hebasto converted_to_draft a pull request: "util, refactor: Switch to value-initialization"
(https://github.com/bitcoin/bitcoin/pull/30040)
This PR allows to avoid false positive `-Wmaybe-uninitialized` warnings when cross-compiling for Windows.
(https://github.com/bitcoin/bitcoin/pull/30040)
This PR allows to avoid false positive `-Wmaybe-uninitialized` warnings when cross-compiling for Windows.
✅ hebasto closed a pull request: "util, refactor: Switch to value-initialization"
(https://github.com/bitcoin/bitcoin/pull/30040)
(https://github.com/bitcoin/bitcoin/pull/30040)
💬 hebasto commented on pull request "util, refactor: Switch to value-initialization":
(https://github.com/bitcoin/bitcoin/pull/30040#issuecomment-2094918672)
Closing.
> CI still failing:
FWIW, those false positive warnings are fixed in GCC 13.
(https://github.com/bitcoin/bitcoin/pull/30040#issuecomment-2094918672)
Closing.
> CI still failing:
FWIW, those false positive warnings are fixed in GCC 13.
💬 laanwj commented on pull request "util, refactor: Switch to value-initialization":
(https://github.com/bitcoin/bitcoin/pull/30040#discussion_r1590393699)
Concept ACK on adding value initialization here, seems like a good precaution in any case.
(https://github.com/bitcoin/bitcoin/pull/30040#discussion_r1590393699)
Concept ACK on adding value initialization here, seems like a good precaution in any case.
💬 laanwj commented on issue "Possible to Ban Clients by Name?":
(https://github.com/bitcoin/bitcoin/issues/30036#issuecomment-2094939151)
NACK on adding functionality for banning by user agent. The user agent (subversion) an arbitrary string that clients can send, so this is super easy to circumvent, and a potential footgun (generally, you'd want to connect to as many different clients as possible to reduce the chance of the node ending up on an isolated "island").
(https://github.com/bitcoin/bitcoin/issues/30036#issuecomment-2094939151)
NACK on adding functionality for banning by user agent. The user agent (subversion) an arbitrary string that clients can send, so this is super easy to circumvent, and a potential footgun (generally, you'd want to connect to as many different clients as possible to reduce the chance of the node ending up on an isolated "island").
💬 andrewtoth 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-2094958100)
> Might partially address https://github.com/bitcoin/bitcoin/issues/29662
That issue is complaining about long compaction times. From https://github.com/bitcoin/bitcoin/blob/master/src/leveldb/include/leveldb/options.h#L111-L112:
> The downside will be longer compactions and hence longer latency/performance hiccups.
it seems this change would make compaction times longer, so would exacerbate that issue?
(https://github.com/bitcoin/bitcoin/pull/30039#issuecomment-2094958100)
> Might partially address https://github.com/bitcoin/bitcoin/issues/29662
That issue is complaining about long compaction times. From https://github.com/bitcoin/bitcoin/blob/master/src/leveldb/include/leveldb/options.h#L111-L112:
> The downside will be longer compactions and hence longer latency/performance hiccups.
it seems this change would make compaction times longer, so would exacerbate that issue?
👍 laanwj approved a pull request: "chainparams: Add achow101 DNS seeder"
(https://github.com/bitcoin/bitcoin/pull/30007#pullrequestreview-2039807786)
All good now!
ACK ee218aa9a9eaac53030c31b099b4afe354197ba7
(https://github.com/bitcoin/bitcoin/pull/30007#pullrequestreview-2039807786)
All good now!
ACK ee218aa9a9eaac53030c31b099b4afe354197ba7
💬 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-2094981028)
> Do you mind sharing the methods used so far to test this?
I am simply watching the disk cache flush rate in `iostat(1)`.
In addition to that, the difference in the system interactivity is also pretty apparent.
> Does the change from 2MB to 128MB have any impact on consistent or transient RAM usage (i.e. for resource-constrained nodes)?
Did not observe any such effect, the RAM usage of the Bitcoin process seems to vary within roughly the same bounds when syncing with the Bitcoin net
...
(https://github.com/bitcoin/bitcoin/pull/30039#issuecomment-2094981028)
> Do you mind sharing the methods used so far to test this?
I am simply watching the disk cache flush rate in `iostat(1)`.
In addition to that, the difference in the system interactivity is also pretty apparent.
> Does the change from 2MB to 128MB have any impact on consistent or transient RAM usage (i.e. for resource-constrained nodes)?
Did not observe any such effect, the RAM usage of the Bitcoin process seems to vary within roughly the same bounds when syncing with the Bitcoin net
...
💬 andrewtoth 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#discussion_r1590461048)
Should we make this a constant? Would it be appropriate to reuse `MAX_BLOCKFILE_SIZE`?
(https://github.com/bitcoin/bitcoin/pull/30039#discussion_r1590461048)
Should we make this a constant? Would it be appropriate to reuse `MAX_BLOCKFILE_SIZE`?
✅ IAmAdamRest closed an issue: "Possible to Ban Clients by Name?"
(https://github.com/bitcoin/bitcoin/issues/30036)
(https://github.com/bitcoin/bitcoin/issues/30036)
💬 IAmAdamRest commented on issue "Possible to Ban Clients by Name?":
(https://github.com/bitcoin/bitcoin/issues/30036#issuecomment-2095040466)
It is BTF and it is still happening. The subnets have been all over the place and even in many data centers. I'm not going to bother updating this because I have my own theories about who is behind this and where they are and spoke to the secret service today and turned over all of my logs for them to review and I was told I am NOT the only party to report this exact issue in the last week to them.
(https://github.com/bitcoin/bitcoin/issues/30036#issuecomment-2095040466)
It is BTF and it is still happening. The subnets have been all over the place and even in many data centers. I'm not going to bother updating this because I have my own theories about who is behind this and where they are and spoke to the secret service today and turned over all of my logs for them to review and I was told I am NOT the only party to report this exact issue in the last week to them.
🚀 fanquake merged a pull request: "msvc: Compile `test\fuzz\miniscript.cpp`"
(https://github.com/bitcoin/bitcoin/pull/30031)
(https://github.com/bitcoin/bitcoin/pull/30031)
👍 fanquake approved a pull request: "depends: pass verbose through to cmake based makefiles"
(https://github.com/bitcoin/bitcoin/pull/29960#pullrequestreview-2039871257)
ACK 7c69baf227252511455bc06e315f6a3c7fc5a398
(https://github.com/bitcoin/bitcoin/pull/29960#pullrequestreview-2039871257)
ACK 7c69baf227252511455bc06e315f6a3c7fc5a398
🚀 fanquake merged a pull request: "depends: pass verbose through to cmake based makefiles"
(https://github.com/bitcoin/bitcoin/pull/29960)
(https://github.com/bitcoin/bitcoin/pull/29960)
💬 fjahr commented on pull request "rpc: improve submitpackage documentation and other improvements":
(https://github.com/bitcoin/bitcoin/pull/29292#issuecomment-2095068570)
re-ACK 311f52371f37123f1f6186ac818db0741f643aed
Apologies for even slower re-review :)
(https://github.com/bitcoin/bitcoin/pull/29292#issuecomment-2095068570)
re-ACK 311f52371f37123f1f6186ac818db0741f643aed
Apologies for even slower re-review :)
💬 andrewtoth 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-2095086792)
Benchmarked IBD with an SSD to block 800k, `dbcache=450`, `prune=0` with a local node serving the blocks. This branch is 27% (!) faster than master :rocket:
```
commit 7f15e71f7e762645dbd1ea5eba9ecc6f9ad60236 (branch)
Time (mean ± σ): 14711.490 s ± 225.376 s [User: 19465.517 s, System: 1147.712 s]
Range (min … max): 14552.125 s … 14870.854 s 2 runs
commit eb0bdbdd753bca97120247b921fd29d606fea6e9 (master)
Time (mean ± σ): 20274.276 s ± 106.042 s [User: 21
...
(https://github.com/bitcoin/bitcoin/pull/30039#issuecomment-2095086792)
Benchmarked IBD with an SSD to block 800k, `dbcache=450`, `prune=0` with a local node serving the blocks. This branch is 27% (!) faster than master :rocket:
```
commit 7f15e71f7e762645dbd1ea5eba9ecc6f9ad60236 (branch)
Time (mean ± σ): 14711.490 s ± 225.376 s [User: 19465.517 s, System: 1147.712 s]
Range (min … max): 14552.125 s … 14870.854 s 2 runs
commit eb0bdbdd753bca97120247b921fd29d606fea6e9 (master)
Time (mean ± σ): 20274.276 s ± 106.042 s [User: 21
...
💬 fjahr commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2095109309)
> I'm happy to continue generating as many genesis blocks as is required. Here's a new one with a bitcoin block hash
I have included this one in the latest push, thank you!
> Does the genesis block need a message? It seems like a neutral stance to me.
It could be empty but as @ajtowns has [expressed here](https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2094003382), to prevent a long pre-mine a current block hash should be included and that's not the first time I have heard th
...
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2095109309)
> I'm happy to continue generating as many genesis blocks as is required. Here's a new one with a bitcoin block hash
I have included this one in the latest push, thank you!
> Does the genesis block need a message? It seems like a neutral stance to me.
It could be empty but as @ajtowns has [expressed here](https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2094003382), to prevent a long pre-mine a current block hash should be included and that's not the first time I have heard th
...
💬 ajtowns commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2095213347)
> nudge miners to reorg min-difficulty blocks
Maybe something to consider would be to have the min-difficulty interval vary. eg, after a real difficulty block, min-difficulty applies if the delta is 1h10m; but it reduces to 20m for consecutive min difficulty blocks. So you could mine perhaps 3 min-difficulty blocks immediately, but would then have to wait 10m before you can do another one, and if a real difficulty block is mined in the meantime, you're back to a long-ish wait before you can t
...
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2095213347)
> nudge miners to reorg min-difficulty blocks
Maybe something to consider would be to have the min-difficulty interval vary. eg, after a real difficulty block, min-difficulty applies if the delta is 1h10m; but it reduces to 20m for consecutive min difficulty blocks. So you could mine perhaps 3 min-difficulty blocks immediately, but would then have to wait 10m before you can do another one, and if a real difficulty block is mined in the meantime, you're back to a long-ish wait before you can t
...
💬 maflcko commented on issue "ci: Enable bpfcc-tools":
(https://github.com/bitcoin/bitcoin/issues/29804#issuecomment-2095363054)
> Wait for 24.04 GHA image, Move the task over to that image
Still waiting for it to appear here: https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners/about-github-hosted-runners#standard-github-hosted-runners-for-public-repositories
(https://github.com/bitcoin/bitcoin/issues/29804#issuecomment-2095363054)
> Wait for 24.04 GHA image, Move the task over to that image
Still waiting for it to appear here: https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners/about-github-hosted-runners#standard-github-hosted-runners-for-public-repositories
💬 hebasto commented on pull request "kernel: Remove key module from kernel library":
(https://github.com/bitcoin/bitcoin/pull/29252#issuecomment-2095369013)
Concept ACK.
(https://github.com/bitcoin/bitcoin/pull/29252#issuecomment-2095369013)
Concept ACK.