π¬ Sjors commented on pull request "doc: Remove stale sections in dev notes":
(https://github.com/bitcoin/bitcoin/pull/32572#discussion_r2100127903)
Not to bash rust, but maybe suggest either...
(https://github.com/bitcoin/bitcoin/pull/32572#discussion_r2100127903)
Not to bash rust, but maybe suggest either...
π¬ hebasto commented on pull request "subprocess: Handle quoted tokens properly":
(https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2897728376)
> Alternatively, can't we just pass a vector of arguments to `RunCommandParseJSON` and avoid splitting/merging?
This approach [caused](https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-1837238921) quoting-related issues on Windows, though I havenβt tested it with the current master branch.
(https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2897728376)
> Alternatively, can't we just pass a vector of arguments to `RunCommandParseJSON` and avoid splitting/merging?
This approach [caused](https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-1837238921) quoting-related issues on Windows, though I havenβt tested it with the current master branch.
π¬ laanwj commented on pull request "subprocess: Handle quoted tokens properly":
(https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2897741176)
> This approach https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-1837238921 quoting-related issues on Windows, though I havenβt tested it with the current master branch.
The thing on Windows is that `CreateProcessW` takes a single command line, instead of a list of arguments. So a "join" is unavoidable. It does do quoting on windows while joining this command line though-i don't know enough to know if it's correct: https://github.com/arun11299/cpp-subprocess/blob/master/cpp-subproce
...
(https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2897741176)
> This approach https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-1837238921 quoting-related issues on Windows, though I havenβt tested it with the current master branch.
The thing on Windows is that `CreateProcessW` takes a single command line, instead of a list of arguments. So a "join" is unavoidable. It does do quoting on windows while joining this command line though-i don't know enough to know if it's correct: https://github.com/arun11299/cpp-subprocess/blob/master/cpp-subproce
...
π¬ laanwj commented on pull request "doc: Remove stale sections in dev notes":
(https://github.com/bitcoin/bitcoin/pull/32572#discussion_r2100144262)
"Don't add any new bash scripts" would still be good advice imo. We're still running into bash insanity a decade later #32573.
(https://github.com/bitcoin/bitcoin/pull/32572#discussion_r2100144262)
"Don't add any new bash scripts" would still be good advice imo. We're still running into bash insanity a decade later #32573.
π¬ laanwj commented on pull request "fs: remove `_POSIX_C_SOURCE` defining":
(https://github.com/bitcoin/bitcoin/pull/32460#discussion_r2100186943)
Maybe `(void)posix_fallocate(fileno(file), 0, nEndPos)` to make ignoring the return value explicit.
(https://github.com/bitcoin/bitcoin/pull/32460#discussion_r2100186943)
Maybe `(void)posix_fallocate(fileno(file), 0, nEndPos)` to make ignoring the return value explicit.
π¬ vasild commented on pull request "tests: Expand HTTP coverage to assert libevent behavior":
(https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2100197725)
In that discussion @maflcko [suggested](https://www.erisian.com.au/bitcoin-core-dev/log-2025-05-07.html#l-102) to start the timer before connect and send and sliv3r__ said that [before connect it worked 15 times in a row](https://www.erisian.com.au/bitcoin-core-dev/log-2025-05-07.html#l-117). Do I miss something?
(https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2100197725)
In that discussion @maflcko [suggested](https://www.erisian.com.au/bitcoin-core-dev/log-2025-05-07.html#l-102) to start the timer before connect and send and sliv3r__ said that [before connect it worked 15 times in a row](https://www.erisian.com.au/bitcoin-core-dev/log-2025-05-07.html#l-117). Do I miss something?
π¬ laanwj commented on pull request "fs: remove `_POSIX_C_SOURCE` defining":
(https://github.com/bitcoin/bitcoin/pull/32460#issuecomment-2897839301)
OpenBSD (as of 7.4): has no `posix_fallocate`, but does have `ftruncate` https://github.com/PointCloudLibrary/pcl/pull/5957/files
FreeBSD (as of 8.3): has `posix_fallocate`: https://man.freebsd.org/cgi/man.cgi?query=posix_fallocate&sektion=2&manpath=FreeBSD+8.3-RELEASE
NetBSD (as of 7.0): has it : https://man.netbsd.org/NetBSD-7.0/posix_fallocate.2
(https://github.com/bitcoin/bitcoin/pull/32460#issuecomment-2897839301)
OpenBSD (as of 7.4): has no `posix_fallocate`, but does have `ftruncate` https://github.com/PointCloudLibrary/pcl/pull/5957/files
FreeBSD (as of 8.3): has `posix_fallocate`: https://man.freebsd.org/cgi/man.cgi?query=posix_fallocate&sektion=2&manpath=FreeBSD+8.3-RELEASE
NetBSD (as of 7.0): has it : https://man.netbsd.org/NetBSD-7.0/posix_fallocate.2
π¬ vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2897850022)
`fb12a062b8...cedbc2cd99`: rebase due to conflicts
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2897850022)
`fb12a062b8...cedbc2cd99`: rebase due to conflicts
π¬ polespinasa commented on pull request "tests: Expand HTTP coverage to assert libevent behavior":
(https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2100216282)
It was "randomly" failing. The 15 times in a row was moving it before `http.clinet.HTTPConn...`.
Moving it before `conn.connect` failed the 6th time https://www.erisian.com.au/bitcoin-core-dev/log-2025-05-07.html#l-115
(https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2100216282)
It was "randomly" failing. The 15 times in a row was moving it before `http.clinet.HTTPConn...`.
Moving it before `conn.connect` failed the 6th time https://www.erisian.com.au/bitcoin-core-dev/log-2025-05-07.html#l-115
π¬ ismaelsadeeq commented on pull request "Fees: add Fee rate Forecaster Manager":
(https://github.com/bitcoin/bitcoin/pull/31664#discussion_r2100220749)
thanks fixed.
(https://github.com/bitcoin/bitcoin/pull/31664#discussion_r2100220749)
thanks fixed.
π¬ ismaelsadeeq commented on pull request "Fees: add Fee rate Forecaster Manager":
(https://github.com/bitcoin/bitcoin/pull/31664#discussion_r2100227626)
fixed
(https://github.com/bitcoin/bitcoin/pull/31664#discussion_r2100227626)
fixed
π¬ vasild commented on pull request "tests: Expand HTTP coverage to assert libevent behavior":
(https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2100230504)
Ok, then this should work:
```diff
# Get the underlying socket from HTTP connection so we can send something unusual
+ start = time.time()
conn = http.client.HTTPConnection(urlNode2.hostname, urlNode2.port)
conn.connect()
sock = conn.sock
sock.sendall(http_request.encode("utf-8"))
# Wait for response, but expect a timeout disconnection after 1 second
- start = time.time()
res = sock.recv(1024)
stop
...
(https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2100230504)
Ok, then this should work:
```diff
# Get the underlying socket from HTTP connection so we can send something unusual
+ start = time.time()
conn = http.client.HTTPConnection(urlNode2.hostname, urlNode2.port)
conn.connect()
sock = conn.sock
sock.sendall(http_request.encode("utf-8"))
# Wait for response, but expect a timeout disconnection after 1 second
- start = time.time()
res = sock.recv(1024)
stop
...
π¬ rkrux commented on pull request "wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key":
(https://github.com/bitcoin/bitcoin/pull/32471#issuecomment-2897892091)
A similar problem happens in `gethdkeys private=true` as well and I think it's due to partial private keys when the following error is thrown:
```
β bitcoin git:(list-descriptors-with-partial-keys) β bitcoinclireg -named gethdkeys private=true
error code: -1
error message:
map::at: key not found
```
It appears to be coming from here when the `xprv` corresponding to the `xpub` can't be found: https://github.com/bitcoin/bitcoin/blob/7763e86afa045910a14ac9b2cd644927a447370b/src/wallet/
...
(https://github.com/bitcoin/bitcoin/pull/32471#issuecomment-2897892091)
A similar problem happens in `gethdkeys private=true` as well and I think it's due to partial private keys when the following error is thrown:
```
β bitcoin git:(list-descriptors-with-partial-keys) β bitcoinclireg -named gethdkeys private=true
error code: -1
error message:
map::at: key not found
```
It appears to be coming from here when the `xprv` corresponding to the `xpub` can't be found: https://github.com/bitcoin/bitcoin/blob/7763e86afa045910a14ac9b2cd644927a447370b/src/wallet/
...
β οΈ John-zhan opened an issue: "depends config fails"
(https://github.com/bitcoin/bitcoin/issues/32578)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
"cmake -B build --preset vs2022-static" execute this config, and bitcoin--9.0\build\vcpkg_installed\vcpkg\blds\libevent\config-x64-windows-static-out.log display such info:
[1/2] "D:/Program Files/CMake/bin/cmake.exe" -E chdir ".." "D:/Program Files/CMake/bin/cmake.exe" "F:/workspace/code/github/bitcoin-29.0/build/vcpkg_installed/vcpkg/blds/libevent/src/.12-stable-1d4b950f74.clean" "-G" "
...
(https://github.com/bitcoin/bitcoin/issues/32578)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
"cmake -B build --preset vs2022-static" execute this config, and bitcoin--9.0\build\vcpkg_installed\vcpkg\blds\libevent\config-x64-windows-static-out.log display such info:
[1/2] "D:/Program Files/CMake/bin/cmake.exe" -E chdir ".." "D:/Program Files/CMake/bin/cmake.exe" "F:/workspace/code/github/bitcoin-29.0/build/vcpkg_installed/vcpkg/blds/libevent/src/.12-stable-1d4b950f74.clean" "-G" "
...
π¬ vasild commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#discussion_r2100274925)
It didn't compile - couldn't convert the return value of `snap.Nodes()` which is `const std::vector<std::shared_ptr<CNode>>&` to `std::span<std::shared_ptr<CNode>>`. So I used vector in the first commit, and later when the snapshots are removed - changed it to `span`.
(https://github.com/bitcoin/bitcoin/pull/32015#discussion_r2100274925)
It didn't compile - couldn't convert the return value of `snap.Nodes()` which is `const std::vector<std::shared_ptr<CNode>>&` to `std::span<std::shared_ptr<CNode>>`. So I used vector in the first commit, and later when the snapshots are removed - changed it to `span`.
π¬ vasild commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#issuecomment-2897938845)
`cbd7bd7422...67bc008f62`: fix CI
(https://github.com/bitcoin/bitcoin/pull/32015#issuecomment-2897938845)
`cbd7bd7422...67bc008f62`: fix CI
π¬ darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2100286223)
The value of a transaction output.
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2100286223)
The value of a transaction output.
π fanquake merged a pull request: "wallet: Use `util::Error` throughout `AddWalletDescriptor` instead of returning `nullptr` for some errors"
(https://github.com/bitcoin/bitcoin/pull/32475)
(https://github.com/bitcoin/bitcoin/pull/32475)
π¬ fanquake commented on pull request "cmake: Remove `ENABLE_{SSE41,AVX2,X86_SHANI,ARM_SHANI}` from `bitcoin-build-config.h`":
(https://github.com/bitcoin/bitcoin/pull/32551#issuecomment-2897963902)
Guix Build:
```bash
d899fe5104a77d8c86d40518cb872e8200142e5711689a7bec1e116fcb1807af guix-build-800b7cc42ca6/output/aarch64-linux-gnu/SHA256SUMS.part
8f81b2cd236c0116937b950964f1a4a88c0fdf1baf0d859c055153c4fceddcdc guix-build-800b7cc42ca6/output/aarch64-linux-gnu/bitcoin-800b7cc42ca6-aarch64-linux-gnu-debug.tar.gz
43812573c4d833df3cae58332dfd632afedd428fc0c46a403b872bdfb344bfb7 guix-build-800b7cc42ca6/output/aarch64-linux-gnu/bitcoin-800b7cc42ca6-aarch64-linux-gnu.tar.gz
1e93c16fa60545e3
...
(https://github.com/bitcoin/bitcoin/pull/32551#issuecomment-2897963902)
Guix Build:
```bash
d899fe5104a77d8c86d40518cb872e8200142e5711689a7bec1e116fcb1807af guix-build-800b7cc42ca6/output/aarch64-linux-gnu/SHA256SUMS.part
8f81b2cd236c0116937b950964f1a4a88c0fdf1baf0d859c055153c4fceddcdc guix-build-800b7cc42ca6/output/aarch64-linux-gnu/bitcoin-800b7cc42ca6-aarch64-linux-gnu-debug.tar.gz
43812573c4d833df3cae58332dfd632afedd428fc0c46a403b872bdfb344bfb7 guix-build-800b7cc42ca6/output/aarch64-linux-gnu/bitcoin-800b7cc42ca6-aarch64-linux-gnu.tar.gz
1e93c16fa60545e3
...
π¬ laanwj commented on pull request "chainparams: Re-add seed.bitcoinstats.com":
(https://github.com/bitcoin/bitcoin/pull/31086#issuecomment-2897974665)
Doesn't seem this should be closed, would just be nice to have some more testing.
(https://github.com/bitcoin/bitcoin/pull/31086#issuecomment-2897974665)
Doesn't seem this should be closed, would just be nice to have some more testing.