💬 fanquake commented on pull request "build: document why we check for `std::system`":
(https://github.com/bitcoin/bitcoin/pull/32491#issuecomment-2886365558)
> i'm not sure any other such platforms exist right now, i'm fine with removing the check. But adding a comment makes sense nevertheless.
Yea. Wanting to remove the check is what led me to end up documenting it, given it's unclear wether we support these builds or not.
(https://github.com/bitcoin/bitcoin/pull/32491#issuecomment-2886365558)
> i'm not sure any other such platforms exist right now, i'm fine with removing the check. But adding a comment makes sense nevertheless.
Yea. Wanting to remove the check is what led me to end up documenting it, given it's unclear wether we support these builds or not.
🚀 fanquake merged a pull request: "build: document why we check for `std::system`"
(https://github.com/bitcoin/bitcoin/pull/32491)
(https://github.com/bitcoin/bitcoin/pull/32491)
💬 rkrux commented on pull request "refactor: bdb removals":
(https://github.com/bitcoin/bitcoin/pull/32511#issuecomment-2886368804)
> Generally, when it comes to providing helper functions for tests, my preference would be to focus on porting the real and user-exposed (wallet) RPCs to similarly-named or similarly-featured helper functions to be used in tests.
Interesting, something like this is done in this PR #32452 for `importprivkey`.
(https://github.com/bitcoin/bitcoin/pull/32511#issuecomment-2886368804)
> Generally, when it comes to providing helper functions for tests, my preference would be to focus on porting the real and user-exposed (wallet) RPCs to similarly-named or similarly-featured helper functions to be used in tests.
Interesting, something like this is done in this PR #32452 for `importprivkey`.
💬 fanquake commented on issue "intermittent issue in rpc_signer.py (enumeratesigners timeout)":
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2886372110)
Maybe #32343? @hebasto @laanwj
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2886372110)
Maybe #32343? @hebasto @laanwj
🚀 fanquake merged a pull request: "test: Remove unused verify_flags suppression"
(https://github.com/bitcoin/bitcoin/pull/32527)
(https://github.com/bitcoin/bitcoin/pull/32527)
💬 fanquake commented on pull request "wallet: Remove watchonly behavior and isminetypes":
(https://github.com/bitcoin/bitcoin/pull/32523#issuecomment-2886387048)
```bash
/home/runner/work/_temp/src/wallet/spend.cpp:474:12: error: calling function 'AvailableCoins' requires holding mutex 'wallet.cs_wallet' exclusively [-Werror,-Wthread-safety-analysis]
474 | return AvailableCoins(wallet, coinControl, /*feerate=*/ std::nullopt, params);
| ^
1 error generated.
```
(https://github.com/bitcoin/bitcoin/pull/32523#issuecomment-2886387048)
```bash
/home/runner/work/_temp/src/wallet/spend.cpp:474:12: error: calling function 'AvailableCoins' requires holding mutex 'wallet.cs_wallet' exclusively [-Werror,-Wthread-safety-analysis]
474 | return AvailableCoins(wallet, coinControl, /*feerate=*/ std::nullopt, params);
| ^
1 error generated.
```
💬 fanquake commented on pull request "scripted-diff: Remove unused leading newline in RPC docs":
(https://github.com/bitcoin/bitcoin/pull/32514#issuecomment-2886391559)
Concept ACK - did you want to fix any more of the [Grammar] here?
(https://github.com/bitcoin/bitcoin/pull/32514#issuecomment-2886391559)
Concept ACK - did you want to fix any more of the [Grammar] here?
💬 laanwj commented on pull request "doc: Add hint about avoiding spaces in paths when building on Windows":
(https://github.com/bitcoin/bitcoin/pull/32397#issuecomment-2886410945)
It would be useful to document, but not in the way it is here. It's not a problem when cloning the repository.
> I believe it would be helpful to mention a workaround in the build documentation. [Here](https://github.com/hebasto/bitcoin/commit/6a2a5428346634c80e6856e17b2c737bb523fd14)'s a suggestion you could consider including.
+1
(https://github.com/bitcoin/bitcoin/pull/32397#issuecomment-2886410945)
It would be useful to document, but not in the way it is here. It's not a problem when cloning the repository.
> I believe it would be helpful to mention a workaround in the build documentation. [Here](https://github.com/hebasto/bitcoin/commit/6a2a5428346634c80e6856e17b2c737bb523fd14)'s a suggestion you could consider including.
+1
👍 brunoerg approved a pull request: "fuzz: Delete wallet_notifications"
(https://github.com/bitcoin/bitcoin/pull/32526#pullrequestreview-2846316142)
ACK fad2faf6c5d8f09a91fb291e30b4989b06a6fe86
I agree to remove it.
(https://github.com/bitcoin/bitcoin/pull/32526#pullrequestreview-2846316142)
ACK fad2faf6c5d8f09a91fb291e30b4989b06a6fe86
I agree to remove it.
💬 laanwj commented on issue "intermittent issue in rpc_signer.py (enumeratesigners timeout)":
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2886420662)
If it's #32343 i don't understand why. It was changed so that the brute force fd-closing path isn't even *compiled* on Linux.
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2886420662)
If it's #32343 i don't understand why. It was changed so that the brute force fd-closing path isn't even *compiled* on Linux.
📝 maflcko opened a pull request: " rpc: Round verificationprogress to 1 for a recent tip "
(https://github.com/bitcoin/bitcoin/pull/32528)
Some users really seem to care about this. While it shouldn't matter much, the diff is so trivial that it is probably worth doing.
Fixes #31127
This implements my own suggestion from https://github.com/bitcoin/bitcoin/pull/31177#issuecomment-2639659890
(https://github.com/bitcoin/bitcoin/pull/32528)
Some users really seem to care about this. While it shouldn't matter much, the diff is so trivial that it is probably worth doing.
Fixes #31127
This implements my own suggestion from https://github.com/bitcoin/bitcoin/pull/31177#issuecomment-2639659890
💬 maflcko commented on pull request "rpc, logging: return "verificationprogress" of 1 when up to date":
(https://github.com/bitcoin/bitcoin/pull/31177#issuecomment-2886452195)
> Just a simple one-line patch to offset `GetBlockTime` with a constant number of hours on top of the code in current master. I don't think there needs to be any special casing?
Went ahead and just implemented that in https://github.com/bitcoin/bitcoin/pull/32528
(https://github.com/bitcoin/bitcoin/pull/31177#issuecomment-2886452195)
> Just a simple one-line patch to offset `GetBlockTime` with a constant number of hours on top of the code in current master. I don't think there needs to be any special casing?
Went ahead and just implemented that in https://github.com/bitcoin/bitcoin/pull/32528
💬 maflcko commented on pull request "refactor: bdb removals":
(https://github.com/bitcoin/bitcoin/pull/32511#issuecomment-2886465274)
The pull here only touches C++ code, so I meant to have such helpers in C++ code. The python side can just use the RPCs directly (or use one of the pre-existing helpers)
(https://github.com/bitcoin/bitcoin/pull/32511#issuecomment-2886465274)
The pull here only touches C++ code, so I meant to have such helpers in C++ code. The python side can just use the RPCs directly (or use one of the pre-existing helpers)
💬 maflcko commented on issue "getblockchaininfo `verificationprogress` never reaches 1.0":
(https://github.com/bitcoin/bitcoin/issues/31127#issuecomment-2886468585)
See #32528
(https://github.com/bitcoin/bitcoin/issues/31127#issuecomment-2886468585)
See #32528
💬 laanwj commented on issue "intermittent issue in rpc_signer.py (enumeratesigners timeout)":
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2886473596)
It would be reasonaly useful to if it logged a full backtrace of bitcoind (gdb `thread apply all bt`) in these case of timeouts.
Though, i'm not sure that's enough to debug this.
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2886473596)
It would be reasonaly useful to if it logged a full backtrace of bitcoind (gdb `thread apply all bt`) in these case of timeouts.
Though, i'm not sure that's enough to debug this.
💬 i-am-yuvi commented on pull request "qa: feature_framework_startup_failures.py fixes & improvements (#30660 follow-up)":
(https://github.com/bitcoin/bitcoin/pull/32509#issuecomment-2886527364)
re-ACK bf950c4544d3a8478b46faf0b93c0dc647274c9b
(https://github.com/bitcoin/bitcoin/pull/32509#issuecomment-2886527364)
re-ACK bf950c4544d3a8478b46faf0b93c0dc647274c9b
🚀 fanquake merged a pull request: "fuzz: Delete wallet_notifications"
(https://github.com/bitcoin/bitcoin/pull/32526)
(https://github.com/bitcoin/bitcoin/pull/32526)
💬 kevkevinpal commented on pull request "test: added fuzz coverage for consensus/merkle.cpp":
(https://github.com/bitcoin/bitcoin/pull/32243#issuecomment-2886623528)
I pushed [caaed67](https://github.com/bitcoin/bitcoin/pull/32243/commits/caaed67232f701341d98cc14d8a014b8c232e2ff) which does not include `ConsumeTransaction` but I am still trying to see if this will add more coverage compared to [f99fd0b](https://github.com/bitcoin/bitcoin/commit/f99fd0b87dcfaf84784ce423f78a950ad377b36b)
(https://github.com/bitcoin/bitcoin/pull/32243#issuecomment-2886623528)
I pushed [caaed67](https://github.com/bitcoin/bitcoin/pull/32243/commits/caaed67232f701341d98cc14d8a014b8c232e2ff) which does not include `ConsumeTransaction` but I am still trying to see if this will add more coverage compared to [f99fd0b](https://github.com/bitcoin/bitcoin/commit/f99fd0b87dcfaf84784ce423f78a950ad377b36b)
📝 laanwj opened a pull request: "[DONOTMERGE] subprocess: replace `fs::directory_iterator` with `readdir`"
(https://github.com/bitcoin/bitcoin/pull/32529)
Alternate implementation of fd iteration for linux.
Do not merge, this is to help troubleshoot CI issue #32524 if it is caused by #32343.
(https://github.com/bitcoin/bitcoin/pull/32529)
Alternate implementation of fd iteration for linux.
Do not merge, this is to help troubleshoot CI issue #32524 if it is caused by #32343.
💬 laanwj commented on pull request "[DONOTMERGE] subprocess: replace `fs::directory_iterator` with `readdir`":
(https://github.com/bitcoin/bitcoin/pull/32529#issuecomment-2886681039)
ARM run failure is interesting:
```
fatal error: in "system_tests/run_command": subprocess::CalledProcessError: /proc/<pid>/fd iteration failed: No such file or directory (2)
```
(proabably `/proc` isn't mounted there, but why wasn't it erroring with the previous implementation)
(https://github.com/bitcoin/bitcoin/pull/32529#issuecomment-2886681039)
ARM run failure is interesting:
```
fatal error: in "system_tests/run_command": subprocess::CalledProcessError: /proc/<pid>/fd iteration failed: No such file or directory (2)
```
(proabably `/proc` isn't mounted there, but why wasn't it erroring with the previous implementation)