Bitcoin Core Github
45 subscribers
118K links
Download Telegram
💬 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.
💬 hebasto commented on pull request "build: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#discussion_r2390237740)
So people on macOS 14.4 with Xcode 15.4 and Apple Clang 1500.3.9.4 based on LLVM 16.0 will fail to compile `fuzz.cpp`, right?
💬 luke-jr commented on pull request "depends: Update URL for `qrencode` package source tarball":
(https://github.com/bitcoin/bitcoin/pull/33494#discussion_r2390259053)
Seems kind of ugly to lose the package name in the filename. How about `$(package)-$($(package)_version)-github.tar.gz` or something?
💬 maflcko commented on pull request "build: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#discussion_r2390276369)
Yes. My guess is that it should be easy and harmless to upgrade Xcode, but I haven't verified this or tried it.
🤔 starkshade reviewed a pull request: "test: sock: Enable socket pair tests on Windows"
(https://github.com/bitcoin/bitcoin/pull/33506#pullrequestreview-3283180075)
The changes refactor the `CreateSocketPair` function to provide a cross-platform implementation and improve the handling of `Sock` objects using `std::unique_ptr` and move semantics, enhancing test robustness and memory management.

**Key Changes:**
- Introduced a Windows-compatible implementation for `CreateSocketPair` using TCP loopback connections, as `socketpair(2)` is not available on Windows.
- Modified `CreateSocketPair` to return `std::pair<Sock, Sock>` instead of modifying an intege
...
💬 starkshade commented on pull request "test: sock: Enable socket pair tests on Windows":
(https://github.com/bitcoin/bitcoin/pull/33506#discussion_r2390282956)
## ⚠️ Code Issue

**Lines:** 107-117

```cpp
Sock* sock0 = new Sock(s[0]);
Sock* sock1 = new Sock(s[1]);

SendAndRecvMessage(*sock0, *sock1);

Sock* sock0moved = new Sock(std::move(*sock0));
Sock* sock1moved = new Sock(INVALID_SOCKET);
*sock1moved = std::move(*sock1);

delete sock0;
delete sock1;
```

This code snippet demonstrates manual memory management using `new` and `delete` for `Sock` objects. This is error-prone and can lead to memory lea
...
💬 starkshade commented on pull request "test: sock: Enable socket pair tests on Windows":
(https://github.com/bitcoin/bitcoin/pull/33506#discussion_r2390282864)
## ⚠️ Code Issue

**Lines:** 86-89

```cpp
static void CreateSocketPair(int s[2])
{
BOOST_REQUIRE_EQUAL(socketpair(AF_UNIX, SOCK_STREAM, 0, s), 0);
}
```

The original `CreateSocketPair` function modified an array passed by reference. The new implementation returns a `std::pair<Sock, Sock>` by value, which is a better approach for managing socket resources, but the original function signature and implementation are removed, and the new implementation for non-Windows is now the pri
...
💬 starkshade commented on pull request "test: sock: Enable socket pair tests on Windows":
(https://github.com/bitcoin/bitcoin/pull/33506#discussion_r2390283007)
## ⚠️ Code Issue

**Lines:** 147-151

```cpp
int s[2];
CreateSocketPair(s);

Sock sock_send(s[0]);
Sock sock_recv(s[1]);
```

Similar to the `wait` test case, this code initializes `Sock` objects directly from integer file descriptors. This bypasses RAII and could lead to resource management issues. Using `std::move` to transfer ownership is a more robust approach.
🤔 starkshade reviewed a pull request: "RPC: add sendrawtransactiontopeer"
(https://github.com/bitcoin/bitcoin/pull/33507#pullrequestreview-3283182194)
This change introduces a new RPC command, `sendrawtransactiontopeer`, which allows users to send a raw transaction directly to a specified peer without storing it in the local node's mempool. This is a valuable addition for testing transaction propagation and peer-to-peer interactions.

**Key Changes:**
- Added a new RPC command `sendrawtransactiontopeer` to submit a raw transaction to a specific peer.
- The new command takes `hexstring`, `peer_id`, and an optional `maxburnamount` as argumen
...
💬 starkshade commented on pull request "RPC: add sendrawtransactiontopeer":
(https://github.com/bitcoin/bitcoin/pull/33507#discussion_r2390284640)
## 💡 Improvement Needed

**Line:** 170

The `sendrawtransactiontopeer` command, similar to `sendmsgtopeer` (line 168), involves network interaction. For clarity and consistency within the `RPC_COMMANDS_SAFE_FOR_FUZZING` list, it would be beneficial to add a comment explaining its safety considerations or potential side effects during fuzzing, especially if its behavior or safety profile is conditional (e.g., when peers are connected versus not connected).

**Example Fix:**
```diff
--- a
...