Bitcoin Core Github
45 subscribers
118K links
Download Telegram
🤔 pablomartin4btc reviewed a pull request: "wallet: reduce unconditional logging during load"
(https://github.com/bitcoin/bitcoin/pull/33299#pullrequestreview-3282443179)
Not sure about this, if there are no descriptor wallets during startup (no default, no one in settings), we would be logging the sqlite version but if the user had legacy wallets in settings, that would print a warning (`FAILED_LEGACY_DISABLED` -> `initWarning`) not showing the berkeley version of the DB instantiated (which code is not in `master` either but it doesn't look consistent). If we have a diff DB format in the future we'd need to change this LogDBInfo() (once someone finds it).

I'd
...
💬 pablomartin4btc commented on pull request "wallet: reduce unconditional logging during load":
(https://github.com/bitcoin/bitcoin/pull/33299#discussion_r2389748193)
nit: if you re-touch... please update the copyright (also to match its header <code>walletdb.h</code>)
<code>// Copyright (c) 2009-present The Bitcoin Core developers</code>
💬 maflcko commented on pull request "build: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#discussion_r2389936885)
Ah, I guess I could have written the comment clearer. I meant `Use the earliest Xcode requiring at least the version of macOS denoted in ...`. Otherwise, we'd unnecessarily limit our Xcode features to EOL versions of macOS.

Whether to pick the earliest minor version of Xcode or earliest major version of Xcode is not specified, but I think going with the earliest major version here seems fine, to be able to remove the fuzz.cpp workaround.
💬 maflcko commented on pull request "build: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#issuecomment-3350089468)
> The minimum supported version in the release notes should only be changed with the along with changing the minimum supported version in depends.

Thx, done. However, I can't test the resulting depends change on macOS myself.
🤔 Eunovo reviewed a pull request: "test: Fix rpc_getblockstats for no-wallet environments"
(https://github.com/bitcoin/bitcoin/pull/33184#pullrequestreview-3282780757)
I think this test could be simplified to always generate data with the MiniWallet and test with generated data, instead of dumping it to a file and loading it later. I don't see the advantage of "generate" and "load" paths, and it's creating unnecessary complexity.
💬 Eunovo commented on pull request "test: Fix rpc_getblockstats for no-wallet environments":
(https://github.com/bitcoin/bitcoin/pull/33184#discussion_r2389993284)
https://github.com/bitcoin/bitcoin/pull/33184/commits/12c9c7d927091f3be72f4acd99c166a41c789ae9:
IIUC, `MiniWallet` doesn't require wallet rpc, so what caused the "-disablewallet" error?
💬 Eunovo commented on pull request "test: Fix rpc_getblockstats for no-wallet environments":
(https://github.com/bitcoin/bitcoin/pull/33184#discussion_r2389995639)
https://github.com/bitcoin/bitcoin/pull/33184/commits/316cf41c25438d61f4e9568e20878fa374476a5a:
I see that the CI fails because this argument conflicts with the argument defined in `rpc_blockstats`.
💬 maflcko commented on pull request "RPC: add sendrawtransactiontopeer":
(https://github.com/bitcoin/bitcoin/pull/33507#discussion_r2390009870)
You'll also have to check the fee? Otherwise, this just seems like a duplicate of `sendmsgtopeer 0 "tx" "$txhex"`?
💬 maflcko commented on pull request "test: Fix rpc_getblockstats for no-wallet environments":
(https://github.com/bitcoin/bitcoin/pull/33184#issuecomment-3350177873)
> I don't see the advantage of "generate" and "load" paths, and it's creating unnecessary complexity.

I guess it depends on how long it takes to generate, but if it is just a few seconds, it seems easier not to cache.
💬 Eunovo commented on pull request "test: Fix rpc_getblockstats for no-wallet environments":
(https://github.com/bitcoin/bitcoin/pull/33184#issuecomment-3350187225)
@maflcko This is from my 8-core laptop
```
bitcoin [2e1836d744] build/test/functional/test_runner.py rpc_getblockstats --gen-test-data
Temporary test directory at /tmp/test_runner_₿_🏃_20250930_083649
Remaining jobs: [rpc_getblockstats.py]
1/1 - rpc_getblockstats.py passed, Duration: 1 s

TEST | STATUS | DURATION

rpc_getblockstats.py | ✓ Passed | 1 s

ALL | ✓ Passed | 1 s (accumulated)
Runtime: 1 s

```
💬 hebasto commented on pull request "depends: Update URL for `qrencode` package source tarball":
(https://github.com/bitcoin/bitcoin/pull/33494#issuecomment-3350214088)
My Guix build:
```
aarch64
b286e83a69f60494b86be866ce3b4aaa7c58aeb2e9410a38b898c2afe6946f59 guix-build-980e4987db04/output/aarch64-linux-gnu/SHA256SUMS.part
c9bcfe3c18dd5a7130c3ccbb17f8f51f7486a9bbfdb57c9ee59164003013de39 guix-build-980e4987db04/output/aarch64-linux-gnu/bitcoin-980e4987db04-aarch64-linux-gnu-debug.tar.gz
f72be7de9bb5d037e9e27db645d8578f031e6a7b6b9096c573dfba56cbcb8a0c guix-build-980e4987db04/output/aarch64-linux-gnu/bitcoin-980e4987db04-aarch64-linux-gnu.tar.gz
ff9ff073
...
💬 maflcko commented on pull request "ci: use latest versions of lint deps":
(https://github.com/bitcoin/bitcoin/pull/33487#issuecomment-3350234120)
lgtm ACK d4f47f97715c7b6a2879e99c62f09ccead8cc4cd
🤔 janb84 reviewed a pull request: "ci: use latest versions of lint deps"
(https://github.com/bitcoin/bitcoin/pull/33487#pullrequestreview-3282863818)
lgtm ACK d4f47f97715c7b6a2879e99c62f09ccead8cc4cd
💬 hebasto commented on pull request "depends: static libxcb-cursor":
(https://github.com/bitcoin/bitcoin/pull/33434#issuecomment-3350313336)
@davidgumberg
> Trying to do a guix build on this branch I get the following error:

Is this consistently reproducible, or does it occur intermittently?
👍 hebasto approved a pull request: "ci: use latest versions of lint deps"
(https://github.com/bitcoin/bitcoin/pull/33487#pullrequestreview-3282958097)
ACK d4f47f97715c7b6a2879e99c62f09ccead8cc4cd, I have reviewed the code and it looks OK.
🚀 hebasto merged a pull request: "ci: use latest versions of lint deps"
(https://github.com/bitcoin/bitcoin/pull/33487)
👍 vasild approved a pull request: "depends: Update URL for `qrencode` package source tarball"
(https://github.com/bitcoin/bitcoin/pull/33494#pullrequestreview-3282991826)
ACK 980e4987db0450cabb8761433001841f7e9f4c73

I confirm the diff from the OP. The old archive is not available anymore at https://fukuchi.org/works/qrencode/qrencode-4.1.1.tar.gz, so I downloaded it from https://bitcoincore.org/depends-sources/qrencode-4.1.1.tar.gz instead. The new one is at https://github.com/fukuchi/libqrencode/archive/refs/tags/v4.1.1.tar.gz.

Then I extracted each archive, did
```sh
for f in $(find ./ -type f |sort) ; do sha256sum "$f" ; done
```
and compared the resulting d
...
💬 vasild commented on pull request "net: support overriding the proxy selection in ConnectNode()":
(https://github.com/bitcoin/bitcoin/pull/33454#issuecomment-3350439700)
`82e46a7248...2ae966b222`: address suggestions, thanks!
💬 vasild commented on pull request "net: support overriding the proxy selection in ConnectNode()":
(https://github.com/bitcoin/bitcoin/pull/33454#discussion_r2390185247)
Right! Changed to `target_addr`.
💬 vasild commented on pull request "net: support overriding the proxy selection in ConnectNode()":
(https://github.com/bitcoin/bitcoin/pull/33454#discussion_r2390186651)
I like `proxy_override`, changed.