💬 maflcko commented on pull request "scripted-diff: test: remove 'descriptors=True' argument for `createwallet` calls":
(https://github.com/bitcoin/bitcoin/pull/32544#issuecomment-2890062931)
lgtm ACK 86de8c1668005304b2c630ca2ad4a8ca8e348e90
ci failure is obviously intermittent and unrelated
(https://github.com/bitcoin/bitcoin/pull/32544#issuecomment-2890062931)
lgtm ACK 86de8c1668005304b2c630ca2ad4a8ca8e348e90
ci failure is obviously intermittent and unrelated
💬 laanwj commented on issue "intermittent issue in rpc_signer.py (enumeratesigners timeout)":
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2890123116)
That backtrace is exactly what i'd expected. It's the parent process waiting for the forked child there.
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2890123116)
That backtrace is exactly what i'd expected. It's the parent process waiting for the forked child there.
💬 laanwj commented on pull request "rpc: Undeprecate rpcuser/rpcpassword, store all credentials hashed in memory":
(https://github.com/bitcoin/bitcoin/pull/32423#discussion_r2095198381)
i intentionally didn't do that, as that would change the semantics. It currently breaks on the first `:`, which would allow semicolons in the password.
(https://github.com/bitcoin/bitcoin/pull/32423#discussion_r2095198381)
i intentionally didn't do that, as that would change the semantics. It currently breaks on the first `:`, which would allow semicolons in the password.
💬 hebasto commented on issue "intermittent issue in qt addressBookTests: (table_view->model()->rowCount()): [3 != 2] Loc: [./qt/test/addressbooktests.cpp(183)]":
(https://github.com/bitcoin/bitcoin/issues/32558#issuecomment-2890196882)
A duplicate of https://github.com/bitcoin-core/gui/issues/874?
(https://github.com/bitcoin/bitcoin/issues/32558#issuecomment-2890196882)
A duplicate of https://github.com/bitcoin-core/gui/issues/874?
👍 laanwj approved a pull request: "wallet: Fix logging of wallet version"
(https://github.com/bitcoin/bitcoin/pull/32553#pullrequestreview-2850007943)
Code review ACK 4b2cd0b41ff4800c8801f2c44883eaec60a035fa
Logging the value immediately after reading it, makes it clear the right value is logged.
(https://github.com/bitcoin/bitcoin/pull/32553#pullrequestreview-2850007943)
Code review ACK 4b2cd0b41ff4800c8801f2c44883eaec60a035fa
Logging the value immediately after reading it, makes it clear the right value is logged.
💬 laanwj commented on pull request "node: cap `-maxmempool` and `-dbcache` values for 32-bit":
(https://github.com/bitcoin/bitcoin/pull/32530#issuecomment-2890261145)
i wonder how many people are running bitcoin nodes on 32-bit systems these days. Is this still worth special casing?
(https://github.com/bitcoin/bitcoin/pull/32530#issuecomment-2890261145)
i wonder how many people are running bitcoin nodes on 32-bit systems these days. Is this still worth special casing?
💬 Sjors commented on pull request "kernel: Remove dependency on clientversion":
(https://github.com/bitcoin/bitcoin/pull/32543#issuecomment-2890264852)
utACK d316a75f1fb807f5384ec9d4ed0b8aad6df7ed5f
This causes the version to be missing from some of the "Please report this issue here" messages. But if someone is running a node, rather than some kernel based application, they can easily find its version when reporting the bug.
(https://github.com/bitcoin/bitcoin/pull/32543#issuecomment-2890264852)
utACK d316a75f1fb807f5384ec9d4ed0b8aad6df7ed5f
This causes the version to be missing from some of the "Please report this issue here" messages. But if someone is running a node, rather than some kernel based application, they can easily find its version when reporting the bug.
💬 laanwj commented on pull request "RFC: bench: replace embedded raw block with configurable block generator":
(https://github.com/bitcoin/bitcoin/pull/32554#issuecomment-2890272937)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/32554#issuecomment-2890272937)
Concept ACK
💬 l0rinc commented on pull request "RFC: script: short-circuit `GetLegacySigOpCount` for known scripts":
(https://github.com/bitcoin/bitcoin/pull/32532#discussion_r2095258063)
It exercises both `GetSigOpCount` and `GetLegacySigOpCount`, so I kept it.
But the new tests can indeed be renamed, thanks!
(https://github.com/bitcoin/bitcoin/pull/32532#discussion_r2095258063)
It exercises both `GetSigOpCount` and `GetLegacySigOpCount`, so I kept it.
But the new tests can indeed be renamed, thanks!
💬 l0rinc commented on pull request "RFC: script: short-circuit `GetLegacySigOpCount` for known scripts":
(https://github.com/bitcoin/bitcoin/pull/32532#discussion_r2095258155)
Indeed, added, thanks.
(https://github.com/bitcoin/bitcoin/pull/32532#discussion_r2095258155)
Indeed, added, thanks.
💬 l0rinc commented on pull request "RFC: script: short-circuit `GetLegacySigOpCount` for known scripts":
(https://github.com/bitcoin/bitcoin/pull/32532#discussion_r2095258344)
I have tried including `interpreter.h` before, but it results in ugly weird compiler mess.
But it seems we can move those constant over to `script.h` instead - done, added you as a co-author.
(https://github.com/bitcoin/bitcoin/pull/32532#discussion_r2095258344)
I have tried including `interpreter.h` before, but it results in ugly weird compiler mess.
But it seems we can move those constant over to `script.h` instead - done, added you as a co-author.
💬 l0rinc commented on pull request "RFC: script: short-circuit `GetLegacySigOpCount` for known scripts":
(https://github.com/bitcoin/bitcoin/pull/32532#discussion_r2095258456)
I took these definitions from `solver.cpp` - the point here is not necessarily to fully validate the internal structure of the script templates, but rather to check if it follows a general template's layout (opcode, followed by a version, followed by a hash, etc). Validating the existing versions is done in other places, it's not important for e.g. SigOp count. If you think I'm mistaken here, please let me know.
(https://github.com/bitcoin/bitcoin/pull/32532#discussion_r2095258456)
I took these definitions from `solver.cpp` - the point here is not necessarily to fully validate the internal structure of the script templates, but rather to check if it follows a general template's layout (opcode, followed by a version, followed by a hash, etc). Validating the existing versions is done in other places, it's not important for e.g. SigOp count. If you think I'm mistaken here, please let me know.
💬 l0rinc commented on pull request "RFC: script: short-circuit `GetLegacySigOpCount` for known scripts":
(https://github.com/bitcoin/bitcoin/pull/32532#discussion_r2095258535)
This wasn't checked in `MatchPayToPubkey` either.
(https://github.com/bitcoin/bitcoin/pull/32532#discussion_r2095258535)
This wasn't checked in `MatchPayToPubkey` either.
💬 l0rinc commented on pull request "RFC: script: short-circuit `GetLegacySigOpCount` for known scripts":
(https://github.com/bitcoin/bitcoin/pull/32532#issuecomment-2890294507)
Thanks for the observations @tapcrafter, addressed your concerns in the latest push, adding you as a co-author in one of the commits.
@maflcko, to clarify the effect on the overall behavior, I slimmed down the `DeserializeAndCheckBlockTest` similarly to https://github.com/bitcoin/bitcoin/pull/32457 and added the results to the PR's description.
---
I ran
```bash
cmake -B build -DBUILD_BENCH=ON -DCMAKE_BUILD_TYPE=Release
cmake --build build -j$(nproc)
build/bin/bench_bitcoin -filte
...
(https://github.com/bitcoin/bitcoin/pull/32532#issuecomment-2890294507)
Thanks for the observations @tapcrafter, addressed your concerns in the latest push, adding you as a co-author in one of the commits.
@maflcko, to clarify the effect on the overall behavior, I slimmed down the `DeserializeAndCheckBlockTest` similarly to https://github.com/bitcoin/bitcoin/pull/32457 and added the results to the PR's description.
---
I ran
```bash
cmake -B build -DBUILD_BENCH=ON -DCMAKE_BUILD_TYPE=Release
cmake --build build -j$(nproc)
build/bin/bench_bitcoin -filte
...
🚀 fanquake merged a pull request: "scripted-diff: test: remove 'descriptors=True' argument for `createwallet` calls"
(https://github.com/bitcoin/bitcoin/pull/32544)
(https://github.com/bitcoin/bitcoin/pull/32544)
💬 Fakruradzi commented on something "":
(https://github.com/bitcoin/bitcoin/commit/7710a31f0cb69a04529f39840196826d0b9770ab#r157354157)
Ok!
(https://github.com/bitcoin/bitcoin/commit/7710a31f0cb69a04529f39840196826d0b9770ab#r157354157)
Ok!
💬 Fakruradzi commented on something "":
(https://github.com/bitcoin/bitcoin/commit/7710a31f0cb69a04529f39840196826d0b9770ab#r157354175)
Seed phrase
(https://github.com/bitcoin/bitcoin/commit/7710a31f0cb69a04529f39840196826d0b9770ab#r157354175)
Seed phrase
💬 maflcko commented on pull request "RFC: script: short-circuit `GetLegacySigOpCount` for known scripts":
(https://github.com/bitcoin/bitcoin/pull/32532#issuecomment-2890357028)
Ah, ok. So from extrapolating the previous comment and https://github.com/bitcoin/bitcoin/pull/31682#issuecomment-2606765689, the IBD speedup should be around 1%?
(https://github.com/bitcoin/bitcoin/pull/32532#issuecomment-2890357028)
Ah, ok. So from extrapolating the previous comment and https://github.com/bitcoin/bitcoin/pull/31682#issuecomment-2606765689, the IBD speedup should be around 1%?
📝 willcl-ark opened a pull request: "doc: add alpine build instructions"
(https://github.com/bitcoin/bitcoin/pull/32559)
Closes: #32512
- Provide Alpine build instructions in build-unix.md.
- Instructions cover building all components except USDT, which requires some `libc`-specific functionality.
Marked as draft as I have not tested the build GUI under a real graphical environment yet. We could alternatively omit support for the GUI on Alpine.
(https://github.com/bitcoin/bitcoin/pull/32559)
Closes: #32512
- Provide Alpine build instructions in build-unix.md.
- Instructions cover building all components except USDT, which requires some `libc`-specific functionality.
Marked as draft as I have not tested the build GUI under a real graphical environment yet. We could alternatively omit support for the GUI on Alpine.
💬 maflcko commented on issue "intermittent issue in rpc_signer.py (enumeratesigners timeout)":
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2890411766)
So far it looks like this can only happen with gcc-13 (or older) *and* GLIBCXX debug mode. I don't think any real user is running in production with the std lib debug mode, so an alternative would be to just fixup the CI?
For reference, the end-user setting `-D_GLIBCXX_ASSERTIONS` passed 25 times: https://cirrus-ci.com/task/5739615073599488
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2890411766)
So far it looks like this can only happen with gcc-13 (or older) *and* GLIBCXX debug mode. I don't think any real user is running in production with the std lib debug mode, so an alternative would be to just fixup the CI?
For reference, the end-user setting `-D_GLIBCXX_ASSERTIONS` passed 25 times: https://cirrus-ci.com/task/5739615073599488