💬 hebasto commented on issue "build: use UCRT runtime for Windows (release) binaries":
(https://github.com/bitcoin/bitcoin/issues/30210#issuecomment-2446405138)
FWIW, here is a way to cross-build UCRT-linked Windows binaries on Fedora 41:
```
$ sudo dnf install ucrt64-gcc-c++
$ make -C depends HOST=x86_64-w64-mingw32ucrt mingw32_CC=/usr/bin/x86_64-w64-mingw32ucrt-gcc mingw32_CXX=/usr/bin/x86_64-w64-mingw32ucrt-g++
$ cmake -B build --toolchain depends/x86_64-w64-mingw32ucrt/toolchain.cmake
$ cmake --build build
```
(https://github.com/bitcoin/bitcoin/issues/30210#issuecomment-2446405138)
FWIW, here is a way to cross-build UCRT-linked Windows binaries on Fedora 41:
```
$ sudo dnf install ucrt64-gcc-c++
$ make -C depends HOST=x86_64-w64-mingw32ucrt mingw32_CC=/usr/bin/x86_64-w64-mingw32ucrt-gcc mingw32_CXX=/usr/bin/x86_64-w64-mingw32ucrt-g++
$ cmake -B build --toolchain depends/x86_64-w64-mingw32ucrt/toolchain.cmake
$ cmake --build build
```
💬 hebasto commented on pull request "build: increase minimum supported Windows to 10.0":
(https://github.com/bitcoin/bitcoin/pull/31172#issuecomment-2446416679)
> ... and suggest linking against UCRT instead.
I've built this branch on Fedora 41, linking against UCRT (see [this](https://github.com/bitcoin/bitcoin/issues/30210#issuecomment-2446405138) workflow), but the issue persists.
(https://github.com/bitcoin/bitcoin/pull/31172#issuecomment-2446416679)
> ... and suggest linking against UCRT instead.
I've built this branch on Fedora 41, linking against UCRT (see [this](https://github.com/bitcoin/bitcoin/issues/30210#issuecomment-2446405138) workflow), but the issue persists.
📝 maflcko opened a pull request: "ci: Place datadirs for tests under tmpfs "
(https://github.com/bitcoin/bitcoin/pull/31182)
(https://github.com/bitcoin/bitcoin/pull/31182)
💬 laanwj commented on pull request "Cleanups to port mapping module post UPnP drop":
(https://github.com/bitcoin/bitcoin/pull/31157#discussion_r1822298782)
DispatchMapPort is local and only called in one place; might as well roll it into `StartMapPort`?
(https://github.com/bitcoin/bitcoin/pull/31157#discussion_r1822298782)
DispatchMapPort is local and only called in one place; might as well roll it into `StartMapPort`?
💬 laanwj commented on pull request "Cleanups to port mapping module post UPnP drop":
(https://github.com/bitcoin/bitcoin/pull/31157#discussion_r1822306060)
Maybe i'm missing something but the intent of `g_mapport_current` is hazy to me. It's turned on in `ThreadMapPort` while `ProcessPCP` is running, then turned off temporarily when `ProcessPCP` returns false.
- If we want to know if the mapport thread is running, `g_mapport_thread.joinable()` seems to suffice.
- If we want to know if mapport thread is supposed to be running, or stop it, there's `g_mapport_enabled`.
(https://github.com/bitcoin/bitcoin/pull/31157#discussion_r1822306060)
Maybe i'm missing something but the intent of `g_mapport_current` is hazy to me. It's turned on in `ThreadMapPort` while `ProcessPCP` is running, then turned off temporarily when `ProcessPCP` returns false.
- If we want to know if the mapport thread is running, `g_mapport_thread.joinable()` seems to suffice.
- If we want to know if mapport thread is supposed to be running, or stop it, there's `g_mapport_enabled`.
💬 fanquake commented on pull request "build: Unify `-logsourcelocations` format":
(https://github.com/bitcoin/bitcoin/pull/30811#issuecomment-2446497505)
I think you could just remove the doc block being added here, and then this can go ahead (also rebase). Doesn't seem like it needs to be blocked on #30861, if that is why it was drafted.
(https://github.com/bitcoin/bitcoin/pull/30811#issuecomment-2446497505)
I think you could just remove the doc block being added here, and then this can go ahead (also rebase). Doesn't seem like it needs to be blocked on #30861, if that is why it was drafted.
🤔 hodlinator reviewed a pull request: "tinyformat: Add compile-time checking for literal format strings"
(https://github.com/bitcoin/bitcoin/pull/31174#pullrequestreview-2404320459)
Concept ACK e53829d3952c6ed275507a66e77636aad67d106b
Cleanest attempt at increased compile time validation of format so far. When reviewing #31149 I had the gnawing feeling that more complete format string support would have reduced the diff, but pushed it away for expediency (an earlier attempt at more complete support was attempted in #30999).
(https://github.com/bitcoin/bitcoin/pull/31174#pullrequestreview-2404320459)
Concept ACK e53829d3952c6ed275507a66e77636aad67d106b
Cleanest attempt at increased compile time validation of format so far. When reviewing #31149 I had the gnawing feeling that more complete format string support would have reduced the diff, but pushed it away for expediency (an earlier attempt at more complete support was attempted in #30999).
💬 hodlinator commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1822308585)
```suggestion
}
```
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1822308585)
```suggestion
}
```
👍 laanwj approved a pull request: "test: Don't enforce BIP94 on regtest unless specified by arg"
(https://github.com/bitcoin/bitcoin/pull/31156#pullrequestreview-2404394664)
Code review ACK e60cecc8115d3b28be076792baa5e4ea26d353a6
(https://github.com/bitcoin/bitcoin/pull/31156#pullrequestreview-2404394664)
Code review ACK e60cecc8115d3b28be076792baa5e4ea26d353a6
💬 willcl-ark commented on pull request "tracing: Only prepare tracepoint arguments when actually tracing":
(https://github.com/bitcoin/bitcoin/pull/26593#issuecomment-2446546723)
utACK 0de3e96e333090548a43e5e870c4cb8941d6baf1
Based on `git range-diff 38e24df...0de3e96e333090548a43e5e870c4cb8941d6baf1` .
The cmake tidy-up looks good to me, and I examined newly built binaries for probe and semaphore info. I didn't re-run the testing from my previous ACK.
(https://github.com/bitcoin/bitcoin/pull/26593#issuecomment-2446546723)
utACK 0de3e96e333090548a43e5e870c4cb8941d6baf1
Based on `git range-diff 38e24df...0de3e96e333090548a43e5e870c4cb8941d6baf1` .
The cmake tidy-up looks good to me, and I examined newly built binaries for probe and semaphore info. I didn't re-run the testing from my previous ACK.
💬 fanquake commented on pull request "cmake: Revamp `FindLibevent` module":
(https://github.com/bitcoin/bitcoin/pull/31181#issuecomment-2446546898)
Concept ACK - we don't have to restrict CMake usage to `vcpkg`, and this will let us make more cleanups.
(https://github.com/bitcoin/bitcoin/pull/31181#issuecomment-2446546898)
Concept ACK - we don't have to restrict CMake usage to `vcpkg`, and this will let us make more cleanups.
💬 l0rinc commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1822380551)
Found the PR thanks to @hodlinator, this is how I handled that case: https://github.com/bitcoin/bitcoin/pull/30999/files#diff-71badc1cc71ba46244f7841a088251bb294265f4fe9e662c0ad6b15be540eee4R60
We could a test case for the incomplete trailing number (or any other test that seems relevant here): https://github.com/bitcoin/bitcoin/pull/30999/files#diff-718d0d85269ec81595bd9f9181eea3a74b20244b07f14c546e3e07520b2b5f82R81
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1822380551)
Found the PR thanks to @hodlinator, this is how I handled that case: https://github.com/bitcoin/bitcoin/pull/30999/files#diff-71badc1cc71ba46244f7841a088251bb294265f4fe9e662c0ad6b15be540eee4R60
We could a test case for the incomplete trailing number (or any other test that seems relevant here): https://github.com/bitcoin/bitcoin/pull/30999/files#diff-718d0d85269ec81595bd9f9181eea3a74b20244b07f14c546e3e07520b2b5f82R81
💬 hodlinator commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1822392320)
Isn't that done here?
https://github.com/bitcoin/bitcoin/blob/ad37073f2e6ab1f39a59109692f84cc85809f53e/src/test/util_string_tests.cpp#L90
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1822392320)
Isn't that done here?
https://github.com/bitcoin/bitcoin/blob/ad37073f2e6ab1f39a59109692f84cc85809f53e/src/test/util_string_tests.cpp#L90
💬 l0rinc commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1822395900)
It is indeed now, thanks for checking.
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1822395900)
It is indeed now, thanks for checking.
💬 Gary19751957 commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#issuecomment-2446696590)
I want whoever you are off of my emails,and away from my crypto and
assets,I do not know you and have given no permissions for any one to
Access my account or any other information being my GitHub repositorys or
other wise.The FBI has been notified and given the names along with a copy
of this email to further this investigation.Gary Rollins
Gary Rollins
On Wed, Oct 30, 2024, 7:15 AM l0rinc ***@***.***> wrote:
> ***@***.**** commented on this pull request.
> -----------------------
...
(https://github.com/bitcoin/bitcoin/pull/31174#issuecomment-2446696590)
I want whoever you are off of my emails,and away from my crypto and
assets,I do not know you and have given no permissions for any one to
Access my account or any other information being my GitHub repositorys or
other wise.The FBI has been notified and given the names along with a copy
of this email to further this investigation.Gary Rollins
Gary Rollins
On Wed, Oct 30, 2024, 7:15 AM l0rinc ***@***.***> wrote:
> ***@***.**** commented on this pull request.
> -----------------------
...
💬 laanwj commented on pull request "net: option to disallow v1 connection on ipv4 and ipv6 peers":
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-2446708418)
> I don't think just sending transactions over encrypted connections gives that much extra privacy.
i'm not advicating to send *just transactions*, v2 connections would have blocks+transactions, all of it. Not sending transactions over v1 makes sure that transactions aren't sent over the network in plaintext.
In the long term, if the idea is to phase out P2P v1, i think demotiing it to a blocksonly-protocol makes sense. It holds the network together against eclipse attacks and accidental
...
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-2446708418)
> I don't think just sending transactions over encrypted connections gives that much extra privacy.
i'm not advicating to send *just transactions*, v2 connections would have blocks+transactions, all of it. Not sending transactions over v1 makes sure that transactions aren't sent over the network in plaintext.
In the long term, if the idea is to phase out P2P v1, i think demotiing it to a blocksonly-protocol makes sense. It holds the network together against eclipse attacks and accidental
...
📝 allanlealluz opened a pull request: "aa"
(https://github.com/bitcoin/bitcoin/pull/31183)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/31183)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
✅ fanquake closed a pull request: "aa"
(https://github.com/bitcoin/bitcoin/pull/31183)
(https://github.com/bitcoin/bitcoin/pull/31183)
📝 fanquake locked a pull request: "aa"
(https://github.com/bitcoin/bitcoin/pull/31183)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/31183)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
💬 brunoerg commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1822430311)
In baac72581b1c99a41ab16a00a3f25fe938eb0610: "This prevents adversarial nodes from proving
our reconciliation set by spamming reconciliation requests": Couldn't we just check the last time the node did a reconciliation request and simply ignore the request?
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1822430311)
In baac72581b1c99a41ab16a00a3f25fe938eb0610: "This prevents adversarial nodes from proving
our reconciliation set by spamming reconciliation requests": Couldn't we just check the last time the node did a reconciliation request and simply ignore the request?