Bitcoin Core Github
44 subscribers
119K links
Download Telegram
💬 davidgumberg commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2048325592)
Writing as a review comment since I didn't think a code comment was warranted here, the reason for putting this on a separate line even though it is only used once is to avoid the wrath of the slightly [imprecise](https://github.com/bitcoin/bitcoin/blob/e66e30c9e5383a467789574e61114b57536193b9/test/lint/lint-python-utf8-encoding.py#L31) python-utf8-encoding [lint check](https://github.com/bitcoin/bitcoin/blob/e66e30c9e5383a467789574e61114b57536193b9/test/lint/lint-python-utf8-encoding.py) which
...
🤔 shahsb reviewed a pull request: "BIP-348 (OP_CHECKSIGFROMSTACK) (regtest only)"
(https://github.com/bitcoin/bitcoin/pull/32247#pullrequestreview-2774788946)
I agree with @JeremyRubin comments..!
💬 shahsb commented on pull request "BIP-348 (OP_CHECKSIGFROMSTACK) (regtest only)":
(https://github.com/bitcoin/bitcoin/pull/32247#issuecomment-2811995385)
Concept ACK
💬 maflcko commented on issue "ci: fuzz_with_valgrind job broken":
(https://github.com/bitcoin/bitcoin/issues/32276#issuecomment-2812024548)
I tried `creduce`, but at some point it seems to have transformed the false positive warning into a true positive warning.
💬 vasild commented on pull request "rpc: add cpu_load to getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/31672#issuecomment-2812025976)
`9cc8ca3b1f...bb822a5ee1`: address suggestions

I think the `bitcoin-cli` extension in https://github.com/jonatack/bitcoin/commit/708a9502f8eca7aaa84236ea038a574f4350f298#r155516952 might be included in this PR. @i-am-yuvi, since you ACKed this PR without that extra commit, what do you think? Should it be included?
💬 vasild commented on pull request "rpc: add cpu_load to getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/31672#discussion_r2048389017)
Done.
💬 vasild commented on pull request "rpc: add cpu_load to getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/31672#discussion_r2048389405)
Done.
💬 vasild commented on pull request "rpc: add cpu_load to getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/31672#discussion_r2048392905)
Shortened a bit but not that much since this is the only help text (I assume release notes are not so useful for somebody looking into this for the first time in e.g. version 35.0, they will not go to search for past release notes).

Other help texts include `\n`, maybe do that here as well?
💬 TheCharlatan commented on pull request "[IBD] batch block reads/writes during `AutoFile` serialization":
(https://github.com/bitcoin/bitcoin/pull/31551#discussion_r2048403323)
Yes, I read that, and did not understand how this makes it more self-contained, or complete. The commit message also said nothing about adding a new error condition.
💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#issuecomment-2812047725)
`aa88c6dfb6...0e97a68e13`: fix CI
💬 l0rinc commented on pull request "[IBD] batch block reads/writes during `AutoFile` serialization":
(https://github.com/bitcoin/bitcoin/pull/31551#discussion_r2048412462)
It's closed now uniformly in https://github.com/bitcoin/bitcoin/pull/29307/files#diff-114c2880ec1ff2c5293ac65ceda0637bf92c05745b74b58410585a549464a33fR965-R972 - does that address your concern?
💬 l0rinc commented on pull request "[IBD] batch block reads/writes during `AutoFile` serialization":
(https://github.com/bitcoin/bitcoin/pull/31551#issuecomment-2812092318)
Thank you all for helping me get this over the finish line @theuni, @ryanofsky, @maflcko, @hodlinator, @achow101!
It went over quite a few iterations until we found the version that we all liked.
💬 vasild commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2048440135)
Maybe also:

```diff
- - `bin/bitcoin` (command line interface)
+ - `bin/bitcoin` (wrapper command)
```
if you retouch
👍 vasild approved a pull request: "multiprocess: Add bitcoin wrapper executable"
(https://github.com/bitcoin/bitcoin/pull/31375#pullrequestreview-2774917380)
ACK d0a2a5239d6c68d45f0ed9b113131fc4273d1214
💬 maflcko commented on pull request "test: allow all functional tests to be run or skipped with --usecli":
(https://github.com/bitcoin/bitcoin/pull/32290#discussion_r2048465023)
q in 1c2ea560300f57e6b5ed75dee2597ca55dd76c6f: Would it not be easier if this was auto-detected? That is, if the type is string, would it not be better to internally (before dispatching to the cli) add quotes to all strings?

This probably means that numbers must be passed as `int`, or `Decimal` on the python side, which seems fine.
💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#issuecomment-2812144686)
`0e97a68e13...1f71111e21`: fix AI and CI (even more)
fanquake closed an issue: "bench: WalletMigration benchmark broken"
(https://github.com/bitcoin/bitcoin/issues/32277)
🚀 fanquake merged a pull request: "bench: Fix WalletMigration benchmark"
(https://github.com/bitcoin/bitcoin/pull/32281)
💬 TheCharlatan commented on pull request "[IBD] batch block reads/writes during `AutoFile` serialization":
(https://github.com/bitcoin/bitcoin/pull/31551#issuecomment-2812207470)
Post-merge ACK
💬 TheCharlatan commented on pull request "[IBD] batch block reads/writes during `AutoFile` serialization":
(https://github.com/bitcoin/bitcoin/pull/31551#discussion_r2048519877)
Not really, but I guess we can continue over there.