💬 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.
💬 laanwj 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_r1590651112)
+1 on a constant, but i don't think it's approprioate to reuse `MAX_BLOCKFILE_SIZE`, better to define a new one
(https://github.com/bitcoin/bitcoin/pull/30039#discussion_r1590651112)
+1 on a constant, but i don't think it's approprioate to reuse `MAX_BLOCKFILE_SIZE`, better to define a new one
💬 laanwj 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-2095395201)
> This branch is 27% (!) faster than master
That's impressive!
> From what I understand, this patch reduces the frequency of flushes
Not only the frequency of flushes; another potential advantage here is that leveldb will spend less time open()ing and close()ing files to maintain its allowed number of open files (eg the `fd_limiter` stuff).
(https://github.com/bitcoin/bitcoin/pull/30039#issuecomment-2095395201)
> This branch is 27% (!) faster than master
That's impressive!
> From what I understand, this patch reduces the frequency of flushes
Not only the frequency of flushes; another potential advantage here is that leveldb will spend less time open()ing and close()ing files to maintain its allowed number of open files (eg the `fd_limiter` stuff).
💬 TheCharlatan commented on pull request "rpc: return warnings as an array instead of just a single one":
(https://github.com/bitcoin/bitcoin/pull/29845#discussion_r1590664391)
I think it is better off eventually in the kernel library eventually: https://github.com/bitcoin/bitcoin/pull/28690/files#diff-4cb884d03ebb901069e4ee5de5d02538c40dd9b39919c615d8eaa9d364bbbd77R735
(https://github.com/bitcoin/bitcoin/pull/29845#discussion_r1590664391)
I think it is better off eventually in the kernel library eventually: https://github.com/bitcoin/bitcoin/pull/28690/files#diff-4cb884d03ebb901069e4ee5de5d02538c40dd9b39919c615d8eaa9d364bbbd77R735
👍 hebasto approved a pull request: "kernel: Remove key module from kernel library"
(https://github.com/bitcoin/bitcoin/pull/29252#pullrequestreview-2040262030)
ACK a885a166cec6d84d08600f12b25d912bd28af80e.
(https://github.com/bitcoin/bitcoin/pull/29252#pullrequestreview-2040262030)
ACK a885a166cec6d84d08600f12b25d912bd28af80e.
💬 hebasto commented on pull request "kernel: Remove key module from kernel library":
(https://github.com/bitcoin/bitcoin/pull/29252#discussion_r1590699911)
nit: Add `#include <key.h>`?
(https://github.com/bitcoin/bitcoin/pull/29252#discussion_r1590699911)
nit: Add `#include <key.h>`?
💬 maflcko commented on pull request "Prevent file descriptor exhaustion from too many RPC calls":
(https://github.com/bitcoin/bitcoin/pull/27731#issuecomment-2095457721)
> removing libevent
Nice. Is there a tracking issue?
(https://github.com/bitcoin/bitcoin/pull/27731#issuecomment-2095457721)
> removing libevent
Nice. Is there a tracking issue?
👍 TheCharlatan approved a pull request: "rpc: return warnings as an array instead of just a single one"
(https://github.com/bitcoin/bitcoin/pull/29845#pullrequestreview-2040301855)
ACK 42fb5311b19582361409d65c6fddeadbee14bb97
(https://github.com/bitcoin/bitcoin/pull/29845#pullrequestreview-2040301855)
ACK 42fb5311b19582361409d65c6fddeadbee14bb97
👍 TheCharlatan approved a pull request: "kernel: Streamline util library"
(https://github.com/bitcoin/bitcoin/pull/29015#pullrequestreview-2040303535)
Re-ACK 5eeafeb302dcf56dd5c16c1e4785a41a132344c7
(https://github.com/bitcoin/bitcoin/pull/29015#pullrequestreview-2040303535)
Re-ACK 5eeafeb302dcf56dd5c16c1e4785a41a132344c7
💬 fanquake commented on pull request "build, bench, msvc: Add missing benchmarks":
(https://github.com/bitcoin/bitcoin/pull/29773#issuecomment-2095507661)
Is the intention to try and merge this as-is, or are the Windows issues going to be investigated and fixed (can draft if so)?
(https://github.com/bitcoin/bitcoin/pull/29773#issuecomment-2095507661)
Is the intention to try and merge this as-is, or are the Windows issues going to be investigated and fixed (can draft if so)?
💬 TheCharlatan commented on pull request "kernel: Streamline util library":
(https://github.com/bitcoin/bitcoin/pull/29015#issuecomment-2095510288)
Re https://github.com/bitcoin/bitcoin/pull/29015#issuecomment-2083201174
> But that is not the case. The libbitcoin_util still depends on libbitcoin_crypto, libbitcoin_consensus and libbitcoin_common libraries.
> If such an outcome is expected, maybe update the doc/design/libraries.md accordingly?
I understand this PR more as providing clarity of what should be in the util library and #28690 as actually realizing the diagram.
(https://github.com/bitcoin/bitcoin/pull/29015#issuecomment-2095510288)
Re https://github.com/bitcoin/bitcoin/pull/29015#issuecomment-2083201174
> But that is not the case. The libbitcoin_util still depends on libbitcoin_crypto, libbitcoin_consensus and libbitcoin_common libraries.
> If such an outcome is expected, maybe update the doc/design/libraries.md accordingly?
I understand this PR more as providing clarity of what should be in the util library and #28690 as actually realizing the diagram.
📝 josibake opened a pull request: "Compare `COutPoints` lexicographically"
(https://github.com/bitcoin/bitcoin/pull/30046)
Broken out from #28122
---
BIP352 requires the smallest output lexicographically, which is my primary motivation for adding this as the default way of compare `COutPoint`s (e.g. https://github.com/bitcoin/bitcoin/blob/5b06ccf1dc63f56dbc35368a37b9a72af671f15f/src/common/bip352.cpp#L149)
Outside of needing this for #28122 , I think it makes more sense to compare `COutPoints` by their serialization for any use case that needs ordering since the serialization is what appears in the final t
...
(https://github.com/bitcoin/bitcoin/pull/30046)
Broken out from #28122
---
BIP352 requires the smallest output lexicographically, which is my primary motivation for adding this as the default way of compare `COutPoint`s (e.g. https://github.com/bitcoin/bitcoin/blob/5b06ccf1dc63f56dbc35368a37b9a72af671f15f/src/common/bip352.cpp#L149)
Outside of needing this for #28122 , I think it makes more sense to compare `COutPoints` by their serialization for any use case that needs ordering since the serialization is what appears in the final t
...
👋 fanquake's pull request is ready for review: "[27.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/29888)
(https://github.com/bitcoin/bitcoin/pull/29888)
💬 TheCharlatan commented on pull request "depends: swap some cctools tools for LLVM tools":
(https://github.com/bitcoin/bitcoin/pull/29739#issuecomment-2095519873)
Guix build (aarch64):
```
6a215012d268b68b25d3120c63f7154169061d71bef8e3a434dc44d6e2c8bf06 guix-build-4313febae66b/output/aarch64-linux-gnu/SHA256SUMS.part
7469b754ce7632c175116ffb1490771b9a4b635c33ccf711fd5df2cf08c6d6d4 guix-build-4313febae66b/output/aarch64-linux-gnu/bitcoin-4313febae66b-aarch64-linux-gnu-debug.tar.gz
7ffd30c4e1894555a1172e13106d8fbf6291f357073fb17840e653f98fe3599f guix-build-4313febae66b/output/aarch64-linux-gnu/bitcoin-4313febae66b-aarch64-linux-gnu.tar.gz
f8d0a35036
...
(https://github.com/bitcoin/bitcoin/pull/29739#issuecomment-2095519873)
Guix build (aarch64):
```
6a215012d268b68b25d3120c63f7154169061d71bef8e3a434dc44d6e2c8bf06 guix-build-4313febae66b/output/aarch64-linux-gnu/SHA256SUMS.part
7469b754ce7632c175116ffb1490771b9a4b635c33ccf711fd5df2cf08c6d6d4 guix-build-4313febae66b/output/aarch64-linux-gnu/bitcoin-4313febae66b-aarch64-linux-gnu-debug.tar.gz
7ffd30c4e1894555a1172e13106d8fbf6291f357073fb17840e653f98fe3599f guix-build-4313febae66b/output/aarch64-linux-gnu/bitcoin-4313febae66b-aarch64-linux-gnu.tar.gz
f8d0a35036
...
💬 hebasto commented on pull request "build, bench, msvc: Add missing benchmarks":
(https://github.com/bitcoin/bitcoin/pull/29773#issuecomment-2095522393)
> Is the intention to try and merge this as-is, or are the Windows issues going to be investigated and fixed (can draft if so)?
The former. The issue is tracked in https://github.com/bitcoin/bitcoin/issues/29816.
(https://github.com/bitcoin/bitcoin/pull/29773#issuecomment-2095522393)
> Is the intention to try and merge this as-is, or are the Windows issues going to be investigated and fixed (can draft if so)?
The former. The issue is tracked in https://github.com/bitcoin/bitcoin/issues/29816.