Bitcoin Core Github
44 subscribers
122K links
Download Telegram
🤔 i-am-yuvi reviewed a pull request: "build: Fix `macdeployqtplus` after switching to Qt 6"
(https://github.com/bitcoin/bitcoin/pull/32287#pullrequestreview-2842440076)
re-ACK 84de8c93e7d4979575161a2bb8f7eb64e1317b89
💬 hebasto commented on pull request "test: Introduce `SUPPRESS_ABORT_MESSAGE` environment variable":
(https://github.com/bitcoin/bitcoin/pull/32409#issuecomment-2882767896)
> > It is documented, and it is pre-defined by CI itself:
>
> It's not in our docs, or in our CI code, which means our CI would "work", because a third party is putting something into the environment, and the fact that this is even happening, is only discoverable if you happen to read a `.cpp` file.
>
> Would someone running these binaries locally also need/want to set this env var to get the same behaviour? If so, how would they figure that out?

Reworked to avoid relying on third-party
...
💬 Sjors commented on pull request "build: document why we check for `std::system`":
(https://github.com/bitcoin/bitcoin/pull/32491#issuecomment-2882769993)
ACK 8f4ba90b8ff47c7f90fe65d3ed37f486f9fe3a74

My understanding is that QML based QT will run fine on iOs, though I haven't tried myself.

Back when this was introduced https://github.com/bitcoin/bitcoin/pull/15457#issuecomment-466315628 it was observered that there's also more obscure platforms out there that don't have and/or sandbox `std::system`. Though nowadays we would probably direct them towards the kernel project.
💬 laanwj commented on pull request "fs: remove `_POSIX_C_SOURCE` defining":
(https://github.com/bitcoin/bitcoin/pull/32460#issuecomment-2882928200)
> _GNU_SOURCE isn't supposed to ever be defined by default, and I don't see us defining it anywhere...?

Not sure when it became that way, but it seems to be set by default, just compile anything using `g++` (first thought was it related to C++ standard, but it isn't):
```c++
#include <iostream>

int main()
{
std::cout << _GNU_SOURCE << std::endl;
std::cout << _POSIX_C_SOURCE << std::endl;
return 0;
}
```

```bash
$ g++ --version
g++ (Ubuntu 13.3.0-6ubuntu2~24.04) 13.
...
💬 luke-jr commented on pull request "doc: Add missing top-level description to pruneblockchain RPC":
(https://github.com/bitcoin/bitcoin/pull/32333#discussion_r2090613335)
...only if certain criteria are met?
💬 luke-jr commented on pull request "doc: Add missing top-level description to pruneblockchain RPC":
(https://github.com/bitcoin/bitcoin/pull/32333#discussion_r2090613797)
`getblockfrompeer` can reverse it AFAIK
🚀 fanquake merged a pull request: "doc: remove Carls substitute server from Guix docs"
(https://github.com/bitcoin/bitcoin/pull/32498)
💬 sipsorcery commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#issuecomment-2883086151)
re-tACK c0f41523973e33a8cd104a8ebf4906a4d99f3eed.
💬 hodlinator commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2090691276)
Previous thread: https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2058445224

`AlreadyConnectedToAddressPort` is significantly more verbose than master's `FindNode`. Checked for uses of `Is[A-Z]\w`-prefix and the `Already`-prefix seems to rarely work, meaning `Already` is more specific in this case. This convinced me that `Already` might be fine. :+1:
🚀 fanquake merged a pull request: "test: add skip_if_running_under_valgrind()"
(https://github.com/bitcoin/bitcoin/pull/32492)
fanquake closed an issue: "test: interface_usdt_net.py failure under --valgrind"
(https://github.com/bitcoin/bitcoin/issues/32374)
💬 fanquake commented on issue "test: interface_usdt_net.py failure under --valgrind":
(https://github.com/bitcoin/bitcoin/issues/32374#issuecomment-2883145411)
Closed via #32492.
💬 fanquake commented on pull request "build: document why we check for `std::system`":
(https://github.com/bitcoin/bitcoin/pull/32491#issuecomment-2883180819)
> it was observered that there's also more obscure platforms out there that don't have and/or sandbox std::system.

@laanwj you might remember / know of such systems?
💬 Sjors commented on pull request "Enhanced error messages for invalid network prefix during address parsing.":
(https://github.com/bitcoin/bitcoin/pull/27260#issuecomment-2883229046)
Any particular reason for marking this draft?
💬 Sjors commented on pull request "qa: make feature_assumeutxo.py test more robust":
(https://github.com/bitcoin/bitcoin/pull/32117#issuecomment-2883235805)
I'll give this PR some thought after you rebase it...
💬 polespinasa commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2090794255)
> Personally, I think it's better to think of "deprecated" as meaning just "the devs recommend you don't use this feature".

Agree on all you said, that's what I tried to argue in few words :)
👍 theStack approved a pull request: "psbt: add non-default sighash types to PSBTs and unify sighash type match checking"
(https://github.com/bitcoin/bitcoin/pull/31622#pullrequestreview-2843083473)
re-ACK ee045b61efc1479c1866b786661ef39a863677d0
(as per `$ git range-diff f09e1f94...ee045b61`)
⚠️ fanquake opened an issue: "test: feature_framework_startup_failures.py fails with `--timeout-factor=0`"
(https://github.com/bitcoin/bitcoin/issues/32506)
master @ c779ee3a4044df3a263394bbb8b104aeeaa7c727
```bash
./build/test/functional/test_runner.py feature_framework_startup_failures.py --timeout-factor=0
Temporary test directory at /tmp/test_runner_₿_🏃_20250515_101739
Remaining jobs: [feature_framework_startup_failures.py]
1/1 - feature_framework_startup_failures.py failed, Duration: 1 s

stdout:
2025-05-15T10:17:39.659000Z TestFramework (INFO): PRNG seed is: 8331063254222734517
2025-05-15T10:17:39.660000Z TestFramework (INFO): Initializing te
...
💬 fanquake commented on issue "test: feature_framework_startup_failures.py fails with `--timeout-factor=0`":
(https://github.com/bitcoin/bitcoin/issues/32506#issuecomment-2883310880)
cc @hodlinator
💬 laanwj commented on pull request "build: document why we check for `std::system`":
(https://github.com/bitcoin/bitcoin/pull/32491#issuecomment-2883318635)
> @laanwj you might remember / know of such systems?

Yes. At the time there was cloudabi, which was a sandboxed version of POSIX that removed all contextual access and ambient authority, so processes only get capability handles passed in to what they need. So there was no shell and hence no `std::system`. But that project was abandoned years ago.

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.

Aside:
...