👍 sedited approved a pull request: "move-only: MAX_BLOCK_TIME_GAP to src/qt"
(https://github.com/bitcoin-core/gui/pull/919#pullrequestreview-3587827623)
ACK fa5ed16aa4d9dbe3ed47cb53f3cb15b0685a2b96
Seems appropriate to move this.
(https://github.com/bitcoin-core/gui/pull/919#pullrequestreview-3587827623)
ACK fa5ed16aa4d9dbe3ed47cb53f3cb15b0685a2b96
Seems appropriate to move this.
💬 stickies-v commented on pull request "log: Use `__func__` for -logsourcelocations":
(https://github.com/bitcoin/bitcoin/pull/34088#discussion_r2627183691)
I'm not sure we should replace all `std::source_location` usage. There is value in uniformity, but I think it's also good to keep places that don't require the shorter function name to stick with stdlib? This is fairly trivial to achieve with e.g.:
<details>
<summary>git diff on fa41dd987e</summary>
```diff
diff --git a/src/logging.cpp b/src/logging.cpp
index b5baac0721..b321991930 100644
--- a/src/logging.cpp
+++ b/src/logging.cpp
@@ -388,7 +388,7 @@ std::shared_ptr<BCLog::LogRateLi
...
(https://github.com/bitcoin/bitcoin/pull/34088#discussion_r2627183691)
I'm not sure we should replace all `std::source_location` usage. There is value in uniformity, but I think it's also good to keep places that don't require the shorter function name to stick with stdlib? This is fairly trivial to achieve with e.g.:
<details>
<summary>git diff on fa41dd987e</summary>
```diff
diff --git a/src/logging.cpp b/src/logging.cpp
index b5baac0721..b321991930 100644
--- a/src/logging.cpp
+++ b/src/logging.cpp
@@ -388,7 +388,7 @@ std::shared_ptr<BCLog::LogRateLi
...
💬 ritoban23 commented on issue "doc: Use multi-path descriptors in descriptors.md and linked tests.":
(https://github.com/bitcoin/bitcoin/issues/34086#issuecomment-3665495043)
@BenWestgate Hi I would like to work on this! will submit a PR soon
(https://github.com/bitcoin/bitcoin/issues/34086#issuecomment-3665495043)
@BenWestgate Hi I would like to work on this! will submit a PR soon
📝 hodlinator opened a pull request: "contrib: asmap-tool.py - Don't write binary to TTY"
(https://github.com/bitcoin/bitcoin/pull/34089)
Verify that we wouldn't be writing encoded asmap binary data directly to the TTY since it is the default but makes no sense. (Having stdout as default does make sense when piping to other applications however).
Found while exploring the ASMap data pipeline (https://github.com/asmap/asmap-data/pull/38#pullrequestreview-3547352533) from Kartograf into Bitcoin Core.
(https://github.com/bitcoin/bitcoin/pull/34089)
Verify that we wouldn't be writing encoded asmap binary data directly to the TTY since it is the default but makes no sense. (Having stdout as default does make sense when piping to other applications however).
Found while exploring the ASMap data pipeline (https://github.com/asmap/asmap-data/pull/38#pullrequestreview-3547352533) from Kartograf into Bitcoin Core.
💬 fanquake commented on pull request "contrib: asmap-tool.py - Don't write binary to TTY":
(https://github.com/bitcoin/bitcoin/pull/34089#issuecomment-3665521922)
cc @fjahr
(https://github.com/bitcoin/bitcoin/pull/34089#issuecomment-3665521922)
cc @fjahr
🤔 brunoerg reviewed a pull request: "fuzz: Add tests for `CCoinControl` methods"
(https://github.com/bitcoin/bitcoin/pull/34026#pullrequestreview-3587941467)
The improved harness from this PR increased the mutation score of the `coin_control` target compared to the master branch. See:
<img width="1211" height="534" alt="Screenshot 2025-12-17 at 11 03 36" src="https://github.com/user-attachments/assets/39e8a407-4d86-4800-b777-e073cc09ca97" />
The only uncaught mutant is:
```diff
diff --git a/src/wallet/coincontrol.cpp b/src/wallet/coincontrol.cpp
index 873c5ab383..e83f8d451a 100644
--- a/src/wallet/coincontrol.cpp
+++ b/src/wallet/coincon
...
(https://github.com/bitcoin/bitcoin/pull/34026#pullrequestreview-3587941467)
The improved harness from this PR increased the mutation score of the `coin_control` target compared to the master branch. See:
<img width="1211" height="534" alt="Screenshot 2025-12-17 at 11 03 36" src="https://github.com/user-attachments/assets/39e8a407-4d86-4800-b777-e073cc09ca97" />
The only uncaught mutant is:
```diff
diff --git a/src/wallet/coincontrol.cpp b/src/wallet/coincontrol.cpp
index 873c5ab383..e83f8d451a 100644
--- a/src/wallet/coincontrol.cpp
+++ b/src/wallet/coincon
...
💬 maflcko commented on pull request "ci: Pin native tests on cross-builds to same commit":
(https://github.com/bitcoin/bitcoin/pull/34080#issuecomment-3665643550)
> I'm surprised the Windows runners are able to find the commit resulting from a merge between PR & master that happened on an Ubuntu runner. But looking at the CI logs that does appear to be the case.
The merge happens on the github infra, and anyone can fetch any commit. I have a local function to do that:
```sh
$ type gfbb
gfbb is a function
gfbb ()
{
git fetch https://github.com/bitcoin/bitcoin "$1"
}
```
(https://github.com/bitcoin/bitcoin/pull/34080#issuecomment-3665643550)
> I'm surprised the Windows runners are able to find the commit resulting from a merge between PR & master that happened on an Ubuntu runner. But looking at the CI logs that does appear to be the case.
The merge happens on the github infra, and anyone can fetch any commit. I have a local function to do that:
```sh
$ type gfbb
gfbb is a function
gfbb ()
{
git fetch https://github.com/bitcoin/bitcoin "$1"
}
```
💬 0xB10C commented on pull request "test: address self-announcement":
(https://github.com/bitcoin/bitcoin/pull/34039#discussion_r2627328886)
Thanks rkrux, included!
(https://github.com/bitcoin/bitcoin/pull/34039#discussion_r2627328886)
Thanks rkrux, included!
💬 0xB10C commented on pull request "test: address self-announcement":
(https://github.com/bitcoin/bitcoin/pull/34039#discussion_r2627331006)
done!
(https://github.com/bitcoin/bitcoin/pull/34039#discussion_r2627331006)
done!
💬 0xB10C commented on pull request "test: address self-announcement":
(https://github.com/bitcoin/bitcoin/pull/34039#issuecomment-3665661325)
Included both suggestions, thanks!
- https://github.com/bitcoin/bitcoin/pull/34039#discussion_r2607756886
- https://github.com/bitcoin/bitcoin/pull/34039#discussion_r2610717344
(https://github.com/bitcoin/bitcoin/pull/34039#issuecomment-3665661325)
Included both suggestions, thanks!
- https://github.com/bitcoin/bitcoin/pull/34039#discussion_r2607756886
- https://github.com/bitcoin/bitcoin/pull/34039#discussion_r2610717344
👍 rkrux approved a pull request: "rpc: Add optional peer_ids param to filter getpeerinfo"
(https://github.com/bitcoin/bitcoin/pull/32741#pullrequestreview-3588075627)
lgtm re-ACK 9b6090d8d985f3ce61eaa587511bbeeeedf28cb9
(https://github.com/bitcoin/bitcoin/pull/32741#pullrequestreview-3588075627)
lgtm re-ACK 9b6090d8d985f3ce61eaa587511bbeeeedf28cb9
💬 Crypt-iQ commented on pull request "log: Use `__func__` for -logsourcelocations":
(https://github.com/bitcoin/bitcoin/pull/34088#issuecomment-3665671949)
Concept ACK. The debug logs are harder to read now.
(https://github.com/bitcoin/bitcoin/pull/34088#issuecomment-3665671949)
Concept ACK. The debug logs are harder to read now.
🤔 sipa reviewed a pull request: "contrib: asmap-tool.py - Don't write binary to TTY"
(https://github.com/bitcoin/bitcoin/pull/34089#pullrequestreview-3588088416)
ACK e7e51952dc24531932b6c06e4599be3a3d6bede8
(https://github.com/bitcoin/bitcoin/pull/34089#pullrequestreview-3588088416)
ACK e7e51952dc24531932b6c06e4599be3a3d6bede8
📝 hebasto opened a pull request: "net: Fix `-Wmissing-braces`"
(https://github.com/bitcoin/bitcoin/pull/34090)
On some non-POSIX platforms, Clang emits `-Wmissing-braces` warnings for the `IN6ADDR_ANY_INIT` and `IN6ADDR_LOOPBACK_INIT` macros. For example, on OpenIndiana / illumos:
```
$ uname -srv
SunOS 5.11 illumos-325e0fc8bb
$ clang --version
clang version 21.1.7 (https://github.com/OpenIndiana/oi-userland.git 36a81bf5e5d307d4e85893422600678d46328010)
Target: x86_64-pc-solaris2.11
Thread model: posix
InstalledDir: /usr/clang/21/bin
$ cmake -B build -DCMAKE_GENERATOR=Ninja -DCMAKE_C_COMPILER=cl
...
(https://github.com/bitcoin/bitcoin/pull/34090)
On some non-POSIX platforms, Clang emits `-Wmissing-braces` warnings for the `IN6ADDR_ANY_INIT` and `IN6ADDR_LOOPBACK_INIT` macros. For example, on OpenIndiana / illumos:
```
$ uname -srv
SunOS 5.11 illumos-325e0fc8bb
$ clang --version
clang version 21.1.7 (https://github.com/OpenIndiana/oi-userland.git 36a81bf5e5d307d4e85893422600678d46328010)
Target: x86_64-pc-solaris2.11
Thread model: posix
InstalledDir: /usr/clang/21/bin
$ cmake -B build -DCMAKE_GENERATOR=Ninja -DCMAKE_C_COMPILER=cl
...
💬 gmaxwell commented on pull request "fees: Introduce Mempool Based Fee Estimation to reduce overestimation":
(https://github.com/bitcoin/bitcoin/pull/34075#issuecomment-3665733085)
I think enabling it by default probably makes sense, that particular sub comment was more about having a ready way to choose not to use it when transacting.
As far as the initial defaults I think there I meant that that it shouldn't target too close to the bottom of the mempool as even if that currently gets high success that may not be true after wide deployment. Though on reflection I'm not sure that thats true as this only lowers feerates, so other people using this should only increase y
...
(https://github.com/bitcoin/bitcoin/pull/34075#issuecomment-3665733085)
I think enabling it by default probably makes sense, that particular sub comment was more about having a ready way to choose not to use it when transacting.
As far as the initial defaults I think there I meant that that it shouldn't target too close to the bottom of the mempool as even if that currently gets high success that may not be true after wide deployment. Though on reflection I'm not sure that thats true as this only lowers feerates, so other people using this should only increase y
...
🚀 fanquake merged a pull request: "A few followups after introducing `/rest/blockpart/` endpoint"
(https://github.com/bitcoin/bitcoin/pull/34074)
(https://github.com/bitcoin/bitcoin/pull/34074)
💬 fanquake commented on pull request "net: Fix `-Wmissing-braces`":
(https://github.com/bitcoin/bitcoin/pull/34090#issuecomment-3665787525)
https://github.com/bitcoin/bitcoin/actions/runs/20307057596/job/58327346295?pr=34090#step:11:495:
```bash
net_processing.cpp
netgroup.cpp
D:\a\bitcoin\bitcoin\src\net.cpp(3309,41): error C2440: '<function-style-cast>': cannot convert from 'initializer list' to 'in6_addr' [D:\a\bitcoin\bitcoin\build\src\bitcoin_node.vcxproj]
D:\a\bitcoin\bitcoin\src\net.cpp(3309,41):
'in6_addr::in6_addr': no overloaded function could convert all the argument types
C:\Program File
...
(https://github.com/bitcoin/bitcoin/pull/34090#issuecomment-3665787525)
https://github.com/bitcoin/bitcoin/actions/runs/20307057596/job/58327346295?pr=34090#step:11:495:
```bash
net_processing.cpp
netgroup.cpp
D:\a\bitcoin\bitcoin\src\net.cpp(3309,41): error C2440: '<function-style-cast>': cannot convert from 'initializer list' to 'in6_addr' [D:\a\bitcoin\bitcoin\build\src\bitcoin_node.vcxproj]
D:\a\bitcoin\bitcoin\src\net.cpp(3309,41):
'in6_addr::in6_addr': no overloaded function could convert all the argument types
C:\Program File
...
💬 fjahr commented on pull request "contrib: asmap-tool.py - Don't write binary to TTY":
(https://github.com/bitcoin/bitcoin/pull/34089#issuecomment-3665802808)
tACK e7e51952dc24531932b6c06e4599be3a3d6bede8
Thanks!
```
$ ./contrib/asmap/asmap-tool.py encode ../../python/kartograf/out/1765638546/final_result.txt
Not much use in writing binary to a TTY. Please specify an output file or pipe output to another process.
```
(https://github.com/bitcoin/bitcoin/pull/34089#issuecomment-3665802808)
tACK e7e51952dc24531932b6c06e4599be3a3d6bede8
Thanks!
```
$ ./contrib/asmap/asmap-tool.py encode ../../python/kartograf/out/1765638546/final_result.txt
Not much use in writing binary to a TTY. Please specify an output file or pipe output to another process.
```
💬 maflcko commented on pull request "log: Use `__func__` for -logsourcelocations":
(https://github.com/bitcoin/bitcoin/pull/34088#discussion_r2627467520)
Not sure if `__func__` is guaranteed to be consteval/constexpr?
(https://github.com/bitcoin/bitcoin/pull/34088#discussion_r2627467520)
Not sure if `__func__` is guaranteed to be consteval/constexpr?
💬 maflcko commented on pull request "log: Use `__func__` for -logsourcelocations":
(https://github.com/bitcoin/bitcoin/pull/34088#discussion_r2627471486)
Not sure about the implicit `operator const std::source_location&(){ return m_loc; };`. This seems like it could silently make some places use the wrong function name.
> function_name_short
Thx, done
(https://github.com/bitcoin/bitcoin/pull/34088#discussion_r2627471486)
Not sure about the implicit `operator const std::source_location&(){ return m_loc; };`. This seems like it could silently make some places use the wrong function name.
> function_name_short
Thx, done
🚀 fanquake merged a pull request: "ci: Pin native tests on cross-builds to same commit"
(https://github.com/bitcoin/bitcoin/pull/34080)
(https://github.com/bitcoin/bitcoin/pull/34080)