Bitcoin Core Github
43 subscribers
122K links
Download Telegram
💬 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?
💬 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
👍 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.
💬 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.
📝 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
💬 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
💬 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)
💬 maflcko commented on issue "getblockchaininfo `verificationprogress` never reaches 1.0":
(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.
💬 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
🚀 fanquake merged a pull request: "fuzz: Delete wallet_notifications"
(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)
📝 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.
💬 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)
💬 instagibbs commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2093026925)
might be worth getting this one mined to show it's consensus-valid still
📝 darosior opened a pull request: "node: cap `-maxmempool` and `-dbcache` values for 32-bit"
(https://github.com/bitcoin/bitcoin/pull/32530)
32-bit architecture is limited to 4GiB of RAM, so it doesn't make sense to set a too high value.
🤔 brunoerg reviewed a pull request: "Remove legacy `Parse(U)Int*`"
(https://github.com/bitcoin/bitcoin/pull/32520#pullrequestreview-2846644623)
Concept ACK
🤔 rkrux reviewed a pull request: "test: Remove legacy wallet RPC overloads"
(https://github.com/bitcoin/bitcoin/pull/32452#pullrequestreview-2846350523)
ACK b104d442277090337ce405d92f1398b7cc9bcdb7

`RPCOverloadWrapper` did catch my eye and made me wonder why it was added in the first place when I reviewed #32360 as it added an additional layer for calling the RPCs in the test framework - good that it can be removed after removal of legacy wallets, makes it slightly easier to track the flow in the framework.

> On the second commit, [test: Replace importprivkey with wallet_importprivkey](https://github.com/bitcoin/bitcoin/pull/32452/commits/
...
💬 rkrux commented on pull request "test: Remove legacy wallet RPC overloads":
(https://github.com/bitcoin/bitcoin/pull/32452#discussion_r2093072864)
Some function documentation here could prove to be helpful for others who are navigating the tests. Might even encourage to create more such helper/wrapper functions as needed.
💬 rkrux commented on pull request "test: Remove legacy wallet RPC overloads":
(https://github.com/bitcoin/bitcoin/pull/32452#discussion_r2092869629)
Mentioning for clarity:
Fine to remove but as this test stood before this PR, `w1` doesn't seem to be a legacy wallet as I can see `descriptors` argument was not set to false in its creation. Maybe it was in some older diff.