💬 maflcko commented on pull request "ci: Enable experimental kernel stuff in ASan task":
(https://github.com/bitcoin/bitcoin/pull/33845#issuecomment-3512701642)
Continue from https://github.com/bitcoin/bitcoin/pull/33824#issuecomment-3511082065:
> > I was thinking about permitting nullptr when size=0, or even just permitting nullptr for any size, when the data pointer is followed by a size?
>
> Afaict our de-serialization can handle this case (nullptr when size=0) correctly, but fails when passed a nullptr with non-zero size. Maybe a null check in `btck_block_create(...)` would be better? What do you think?
Yes, either should be fine. To clarif
...
(https://github.com/bitcoin/bitcoin/pull/33845#issuecomment-3512701642)
Continue from https://github.com/bitcoin/bitcoin/pull/33824#issuecomment-3511082065:
> > I was thinking about permitting nullptr when size=0, or even just permitting nullptr for any size, when the data pointer is followed by a size?
>
> Afaict our de-serialization can handle this case (nullptr when size=0) correctly, but fails when passed a nullptr with non-zero size. Maybe a null check in `btck_block_create(...)` would be better? What do you think?
Yes, either should be fine. To clarif
...
💬 maflcko commented on pull request "ci: Enable experimental kernel stuff in most CI tasks":
(https://github.com/bitcoin/bitcoin/pull/33824#issuecomment-3512704107)
Split the failing CI tasks out to https://github.com/bitcoin/bitcoin/pull/33845 and https://github.com/bitcoin/bitcoin/pull/33842
(https://github.com/bitcoin/bitcoin/pull/33824#issuecomment-3512704107)
Split the failing CI tasks out to https://github.com/bitcoin/bitcoin/pull/33845 and https://github.com/bitcoin/bitcoin/pull/33842
💬 achow101 commented on pull request "p2p: Advance pindexLastCommonBlock early after connecting blocks":
(https://github.com/bitcoin/bitcoin/pull/32180#issuecomment-3512710368)
ACK 01cc20f3307c532f402cdf5dc17f2f14920b725b
(https://github.com/bitcoin/bitcoin/pull/32180#issuecomment-3512710368)
ACK 01cc20f3307c532f402cdf5dc17f2f14920b725b
💬 kevkevinpal commented on pull request "test: Ignore error message give from python because of PYTHON_GIL":
(https://github.com/bitcoin/bitcoin/pull/33795#issuecomment-3512720950)
Added a TODO to remove the warning suppression once `capnp` can run safely without the GIL
[5b7778c](https://github.com/bitcoin/bitcoin/pull/33795/commits/5b7778ca6d00db3fef92384c373e67261abb65a0)
(https://github.com/bitcoin/bitcoin/pull/33795#issuecomment-3512720950)
Added a TODO to remove the warning suppression once `capnp` can run safely without the GIL
[5b7778c](https://github.com/bitcoin/bitcoin/pull/33795/commits/5b7778ca6d00db3fef92384c373e67261abb65a0)
✅ fanquake closed an issue: "log: "Replaying blocks" / "Rolling forward" logging is rate-limited"
(https://github.com/bitcoin/bitcoin/issues/33769)
(https://github.com/bitcoin/bitcoin/issues/33769)
💬 fanquake commented on issue "log: "Replaying blocks" / "Rolling forward" logging is rate-limited":
(https://github.com/bitcoin/bitcoin/issues/33769#issuecomment-3512779755)
Closing given #33443.
(https://github.com/bitcoin/bitcoin/issues/33769#issuecomment-3512779755)
Closing given #33443.
✅ achow101 closed an issue: "p2p: Inefficiency in block download / stalling algorithm"
(https://github.com/bitcoin/bitcoin/issues/32179)
(https://github.com/bitcoin/bitcoin/issues/32179)
🚀 achow101 merged a pull request: "p2p: Advance pindexLastCommonBlock early after connecting blocks"
(https://github.com/bitcoin/bitcoin/pull/32180)
(https://github.com/bitcoin/bitcoin/pull/32180)
💬 achow101 commented on pull request "rpc: add "ischange: true" to decoded tx outputs in wallet gettransaction response":
(https://github.com/bitcoin/bitcoin/pull/32517#issuecomment-3512800884)
ACK 060bb55508245776bb6a39c8b7849769ee588d69
(https://github.com/bitcoin/bitcoin/pull/32517#issuecomment-3512800884)
ACK 060bb55508245776bb6a39c8b7849769ee588d69
💬 achow101 commented on pull request "crypto: Use secure_allocator for `AES256_ctx`":
(https://github.com/bitcoin/bitcoin/pull/31774#discussion_r2511292626)
In 26442fec572da263765c576c080186c84cee1615 "bench: Add wallet encryption benchmark"
We've generally not had "measure overhead" in the benchmarks. If this is because encryption is actually a very small amount of time, I would suggest generating significantly more keys that need to actually be encrypted.
(https://github.com/bitcoin/bitcoin/pull/31774#discussion_r2511292626)
In 26442fec572da263765c576c080186c84cee1615 "bench: Add wallet encryption benchmark"
We've generally not had "measure overhead" in the benchmarks. If this is because encryption is actually a very small amount of time, I would suggest generating significantly more keys that need to actually be encrypted.
💬 achow101 commented on pull request "crypto: Use secure_allocator for `AES256_ctx`":
(https://github.com/bitcoin/bitcoin/pull/31774#discussion_r2511269902)
In 7176b26cde7cbaffdd92af9c25f85f8e5233e78a "refactor: Generalize derivation target calculation"
This comment does not seem helpful, I'm not sure what information it is trying to convey.
(https://github.com/bitcoin/bitcoin/pull/31774#discussion_r2511269902)
In 7176b26cde7cbaffdd92af9c25f85f8e5233e78a "refactor: Generalize derivation target calculation"
This comment does not seem helpful, I'm not sure what information it is trying to convey.
💬 achow101 commented on pull request "crypto: Use secure_allocator for `AES256_ctx`":
(https://github.com/bitcoin/bitcoin/pull/31774#discussion_r2511273130)
In 26442fec572da263765c576c080186c84cee1615 "bench: Add wallet encryption benchmark"
This part of setup is unnecessary. Encryption does not encrypt destinations or transactions. Generating new destinations does not result in more things to be encrypted.
If the idea was to have a bunch of keys to be encrypted for the benchmark, then several independent descriptors would need to be imported.
(https://github.com/bitcoin/bitcoin/pull/31774#discussion_r2511273130)
In 26442fec572da263765c576c080186c84cee1615 "bench: Add wallet encryption benchmark"
This part of setup is unnecessary. Encryption does not encrypt destinations or transactions. Generating new destinations does not result in more things to be encrypted.
If the idea was to have a bunch of keys to be encrypted for the benchmark, then several independent descriptors would need to be imported.
💬 fanquake commented on pull request "build: add `-W*-whitespace`":
(https://github.com/bitcoin/bitcoin/pull/32482#issuecomment-3512823435)
> Thanks. I've pulled those two in here.
Looking at the CI. Those commits are not going to work as implemented (essentially anywhere we are using Clang).
(https://github.com/bitcoin/bitcoin/pull/32482#issuecomment-3512823435)
> Thanks. I've pulled those two in here.
Looking at the CI. Those commits are not going to work as implemented (essentially anywhere we are using Clang).
💬 achow101 commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#issuecomment-3512825537)
> Needs rebase.
Doesn't seem like it does?
(https://github.com/bitcoin/bitcoin/pull/29136#issuecomment-3512825537)
> Needs rebase.
Doesn't seem like it does?
🚀 achow101 merged a pull request: "rpc: add "ischange: true" to decoded tx outputs in wallet gettransaction response"
(https://github.com/bitcoin/bitcoin/pull/32517)
(https://github.com/bitcoin/bitcoin/pull/32517)
⚠️ hebasto opened an issue: "`test_kernel` fails to build on Ubuntu 22.04"
(https://github.com/bitcoin/bitcoin/issues/33846)
```
$ sudo apt update
$ sudo apt install git build-essential cmake pkgconf python3 libevent-dev libboost-dev libsqlite3-dev
$ git clone https://github.com/bitcoin/bitcoin.git && cd bitcoin
$ cmake -B build -DENABLE_IPC=OFF -DBUILD_KERNEL_LIB=ON
$ cmake --build build -j $(nproc)
<snip>
[ 76%] Building CXX object src/test/kernel/CMakeFiles/test_kernel.dir/test_kernel.cpp.o
/bitcoin/src/test/kernel/test_kernel.cpp:17:10: fatal error: format: No such file or directory
17 | #include <format>
...
(https://github.com/bitcoin/bitcoin/issues/33846)
```
$ sudo apt update
$ sudo apt install git build-essential cmake pkgconf python3 libevent-dev libboost-dev libsqlite3-dev
$ git clone https://github.com/bitcoin/bitcoin.git && cd bitcoin
$ cmake -B build -DENABLE_IPC=OFF -DBUILD_KERNEL_LIB=ON
$ cmake --build build -j $(nproc)
<snip>
[ 76%] Building CXX object src/test/kernel/CMakeFiles/test_kernel.dir/test_kernel.cpp.o
/bitcoin/src/test/kernel/test_kernel.cpp:17:10: fatal error: format: No such file or directory
17 | #include <format>
...
💬 hebasto commented on issue "`test_kernel` fails to build on Ubuntu 22.04":
(https://github.com/bitcoin/bitcoin/issues/33846#issuecomment-3512866067)
cc @TheCharlatan
(https://github.com/bitcoin/bitcoin/issues/33846#issuecomment-3512866067)
cc @TheCharlatan
💬 achow101 commented on pull request "refactor: inline constant return values from `dbwrapper` write methods":
(https://github.com/bitcoin/bitcoin/pull/33042#issuecomment-3512874868)
ACK 743abbcbde9e5a2db489bca461c98df461eff7d0
(https://github.com/bitcoin/bitcoin/pull/33042#issuecomment-3512874868)
ACK 743abbcbde9e5a2db489bca461c98df461eff7d0
💬 fanquake commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3512913726)
Forgot to re-add the fuzz test from #33843 (https://github.com/bitcoin/bitcoin/pull/33843/commits/8e4084f7b6d51b3fd1e5f299c5e7531f5885b2cd) back here?
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3512913726)
Forgot to re-add the fuzz test from #33843 (https://github.com/bitcoin/bitcoin/pull/33843/commits/8e4084f7b6d51b3fd1e5f299c5e7531f5885b2cd) back here?
💬 davidgumberg commented on pull request "crypto: Use secure_allocator for `AES256_ctx`":
(https://github.com/bitcoin/bitcoin/pull/31774#discussion_r2511344463)
https://en.wikipedia.org/wiki/Ratio#Notation_and_terminology ([archive link](https://web.archive.org/web/20250918113648/https://en.wikipedia.org/wiki/Ratio#Notation_and_terminology))
This comment explains why the assignment below is correct. This can be read as "target_iterations is to elapsed_iterations as target_time is to elapsed_time", this is mostly equivalent to: $\frac{\text{targetiterations}}{\text{elapsediterations}} = \frac{\text{targettime}}{\text{elapsedtime}}$
(https://github.com/bitcoin/bitcoin/pull/31774#discussion_r2511344463)
https://en.wikipedia.org/wiki/Ratio#Notation_and_terminology ([archive link](https://web.archive.org/web/20250918113648/https://en.wikipedia.org/wiki/Ratio#Notation_and_terminology))
This comment explains why the assignment below is correct. This can be read as "target_iterations is to elapsed_iterations as target_time is to elapsed_time", this is mostly equivalent to: $\frac{\text{targetiterations}}{\text{elapsediterations}} = \frac{\text{targettime}}{\text{elapsedtime}}$