Bitcoin Core Github
44 subscribers
120K links
Download Telegram
πŸ€” janb84 reviewed a pull request: "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings"
(https://github.com/bitcoin/bitcoin/pull/33247#pullrequestreview-3241450515)
Concept ACK a9b211540b79832fb4e4f870d2f770cf03d35b11

This PR reorders the binary checks to after the installation step. And removes the code paths that are made superfluous by the reorg .

Open NIT/question on `libexec` inclusion.

Guix builds and the security tests runs now after install: βœ…
<details>

This PR:
```shell
[101%] Built target test_bitcoin
-- Install configuration: "RelWithDebInfo"
-- Installing: /distsrc-base/distsrc-a9b211540b79-riscv64-linux-gnu/installed/bitcoin-a9
...
πŸ’¬ janb84 commented on pull request "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#discussion_r2360676127)
NIT: This only covers 1 of the 2 binary locations, nowadays we also have `libexec` should this not also be checked ?
πŸš€ achow101 merged a pull request: "coins: warn on oversized `-dbcache`"
(https://github.com/bitcoin/bitcoin/pull/33333)
πŸ’¬ l0rinc commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#discussion_r2360677074)
```suggestion
tfm::format(std::cerr, "Fatal error: expected message not found in the debug log: '%s'\n", m_message);
```
πŸ’¬ l0rinc commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#discussion_r2360683563)
nit/unrelated: I'm surprised to see an `std::list` here, `std::vector` is likely faster even for random erases
πŸ‘ l0rinc approved a pull request: "test: don't throw from the destructor of DebugLogHelper"
(https://github.com/bitcoin/bitcoin/pull/33388#pullrequestreview-3241444220)
Core review ACK 2dcf0c69b8659886ecfc85b174e59cd7f210f5c0

I would also prefer adding quotes around the message but it's not a blocker.
πŸ’¬ l0rinc commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#discussion_r2360670884)
That is my understanding as well, thanks
πŸ“ john-moffett opened a pull request: "rpc: addpeeraddress: throw on invalid IP"
(https://github.com/bitcoin/bitcoin/pull/33430)
Right now we return an opaque `{"success" : false}` in `addpeeraddress` for an empty or invalid IP. This changes it to throw `RPC_CLIENT_INVALID_IP_OR_SUBNET` with the error message `Invalid IP address`. Tests updated to match.
πŸ’¬ enirox001 commented on pull request "test: Replace legacy wallet with MiniWallet in rpc_getblockstats.py":
(https://github.com/bitcoin/bitcoin/pull/33184#issuecomment-3309277841)
Looked into this @achow101 and you're right about the setup issues - they weren't legacy wallet specific but rather configuration problems.
I've addressed the three issues you identified:

1. Added self.uses_wallet = True to prevent -disablewallet
2. MiniWallet naturally avoids deprecated settxfee by using modern fee handling
3. MiniWallet sidesteps multiwallet complexity entirely

Regarding your concern about the --gen-test-data path being rarely executed: I've ensured the test runs wit
...
πŸ’¬ hebasto commented on pull request "Modernize use of UTF-8 in Windows code":
(https://github.com/bitcoin/bitcoin/pull/32380#issuecomment-3309288797)
Rebased to resolve conflict with https://github.com/bitcoin/bitcoin/pull/33333.
πŸ’¬ achow101 commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2360894055)
Looks like the revert is in the wrong commit.
πŸ€” furszy reviewed a pull request: "index: remove unnecessary locator cleaning in BaseIndex::Init()"
(https://github.com/bitcoin/bitcoin/pull/32882#pullrequestreview-3241749692)
utACK facd01e6ffbbd019312f370a3807de0b95bbd745

At least for me, there’s no need to add me as a co-author when the code or suggestion is simple. Generally, only add people when they contribute a substantial change.
Just a small thought.
πŸ’¬ hodlinator commented on pull request "build: Remove lingering Windows registry & shortcuts (#32132 follow-up)":
(https://github.com/bitcoin/bitcoin/pull/33422#issuecomment-3309491269)
Parallel uninstall entries:
<img width="1358" height="508" alt="image" src="https://github.com/user-attachments/assets/3681dbae-127e-44ab-9031-7b6515fd020c" />

Error when trying to uninstall the second entry after the first one has already been uninstalled:
<img width="1357" height="509" alt="image" src="https://github.com/user-attachments/assets/cae121b8-d388-4ae7-9c1b-91d675420b8d" />
πŸ’¬ hodlinator commented on pull request "build: Remove lingering Windows registry & shortcuts (#32132 follow-up)":
(https://github.com/bitcoin/bitcoin/pull/33422#discussion_r2361035808)
Apart from the added ` (64-bit)` suffix, these are copied from the uninstaller sections below, excluding items which would not originally have had the suffix.
πŸ’¬ achow101 commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#issuecomment-3309585706)
Given that the scope of the changes in this PR has expanded quite a bit since it was added to the milestone, and since the changes take place within consensus validation, I think this should be removed from the milestone. I don't think this will be able to get sufficient review for this release, and I don't think the changes here are necessary for the upcoming release.
πŸ’¬ Crypt-iQ commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#issuecomment-3309591741)
ACK 2dcf0c69b8659886ecfc85b174e59cd7f210f5c0
πŸ’¬ achow101 commented on pull request "Remove unnecessary casts when calling socket operations":
(https://github.com/bitcoin/bitcoin/pull/33378#issuecomment-3309658082)
ACK 67f632b6deb8b4aa190c458b71d2bc8c793626d5
πŸš€ achow101 merged a pull request: "Remove unnecessary casts when calling socket operations"
(https://github.com/bitcoin/bitcoin/pull/33378)
πŸ’¬ fanquake commented on pull request "build: Remove lingering Windows registry & shortcuts (#32132 follow-up)":
(https://github.com/bitcoin/bitcoin/pull/33422#issuecomment-3309713059)
cc @sipsorcery
πŸ’¬ achow101 commented on pull request "p2p: Increase tx relay rate":
(https://github.com/bitcoin/bitcoin/pull/28592#issuecomment-3309717927)
ACK b81f37031c8f2ccad9346f1b65ee0f8083c44796