💬 hebasto commented on pull request "refactor: remove remaining unused code from cpp-subprocess":
(https://github.com/bitcoin/bitcoin/pull/29961#discussion_r1582021758)
ed39d261dc056e700b79da769e085c8af389451d:

The `Popen::poll()` function is used in #29868. I hope that an improved/fixed Windows implementation of the `Popen::retcode()`, which I don't have now, will allow to drop it.

📝 hebasto opened a pull request: "build, msvc: Compile `test\fuzz\bitdeque.cpp`"
(https://github.com/bitcoin/bitcoin/pull/29983)
This PR resolves one point from the https://github.com/bitcoin/bitcoin/pull/29774#issuecomment-2028808614:
> What is the issue with the bitdeque... ?

💬 hebasto commented on pull request "msvc: Compile `test\fuzz\bitdeque.cpp`":
(https://github.com/bitcoin/bitcoin/pull/29983#issuecomment-2081363960)
cc @sipa

💬 hebasto commented on pull request "build: Enable fuzz binary in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29774#issuecomment-2081364269)
> What is the issue with the bitdeque and miniscript fuzz tests?

The https://github.com/bitcoin/bitcoin/pull/29983 resolves the issue with the former.

💬 laanwj commented on issue "depends: Cross-compiling `qt` for `arm-linux-gnueabihf` fails":
(https://github.com/bitcoin/bitcoin/issues/29980#issuecomment-2081381459)
This is caused by the 32 to 64 bit time_t transition (for the 2038 epoch problem). It will affect all 32-bit systems.

I would guess solving this requires a minimal patch to the zlib inside Qt to make the right combination of defines. i'll take a look at it.

💬 laanwj commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#issuecomment-2081398215)
New tests LGTM
Re-ACK a0943b812ef38826a4ee2732af5f24e470281117

📝 laanwj opened a pull request: "net: Replace ifname check with IFF_LOOPBACK in Discover"
(https://github.com/bitcoin/bitcoin/pull/29984)
Checking the interface name is kind of brittle. In the age of network namespaces and containers, there is no reason a loopback interface can't be called differently.

Check for the `IFF_LOOPBACK` flag to detect loopback interface instead.

💬 laanwj commented on issue ""Migrate Wallet" is unclear to translators":
(https://github.com/bitcoin/bitcoin/issues/29979#issuecomment-2081416263)
A better user-facing word would be just "upgrade wallet"--but for technical reasons we use a different word, as we already had a concept of upgrading inside the wallet format.

💬 laanwj commented on issue "Add IPv6 pinhole support using UPnP / NAT-PMP":
(https://github.com/bitcoin/bitcoin/issues/17012#issuecomment-2081424660)
The UPnP side of this is progressing, though i've ran into a possible bug/limitaton in miniupnp: https://github.com/miniupnp/miniupnp/issues/731#issuecomment-2081257515

> Meanwhile NAT-PMP support is there, but it has no IPv6 pinhole support.

i looked into this too, we'll actually have to implement PCP support ourselves to do this. The good part is that it's just a matter of sending one fixed-size binary UDP packet to the default gateway and parsing the result. Not worth taking on a depend
...

💬 laanwj commented on pull request "depends: Remove Qt build-time dependencies":
(https://github.com/bitcoin/bitcoin/pull/29923#issuecomment-2081443414)
Looks like we could drop 'bison' as a dependency here, it was used by the `xkbcommon` build which is dropped.

📝 laanwj opened a pull request: "depends: Fix build of Qt for 32-bit platforms"
(https://github.com/bitcoin/bitcoin/pull/29985)
The 32 to 64-bit `time_t` transition causes a build failure in the built-in gzip about conflicting `_TIME_BITS` and `_FILE_OFFSET_BITS`.

Note that gzip doesn't use time_t at all, so it is a false alarm.

Take the following patch from upstream gzip:
https://github.com/madler/zlib/commit/a566e156b3fa07b566ddbf6801b517a9dba04fa3.patch

Closes #29980.

💬 hebasto commented on pull request "depends: Fix build of Qt for 32-bit platforms":
(https://github.com/bitcoin/bitcoin/pull/29985#issuecomment-2081445659)
Concept ACK. I was thinking about picking up the same patch :)

🤔 hebasto reviewed a pull request: "depends: Fix build of Qt for 32-bit platforms with recent glibc"
(https://github.com/bitcoin/bitcoin/pull/29985#pullrequestreview-2026996605)
Approach ACK e4c1a0c62900e8e560b99ee2749460c15355c556, reviewed and tested on Ubuntu 24.04.

I'm going to submit Guix build hashes shortly.

💬 laanwj commented on pull request "depends: Fix build of Qt for 32-bit platforms with recent glibc":
(https://github.com/bitcoin/bitcoin/pull/29985#discussion_r1582109676)
whoops, should probably do s/gzip/zlib here too

💬 laanwj commented on pull request "depends: Fix build of Qt for 32-bit platforms with recent glibc":
(https://github.com/bitcoin/bitcoin/pull/29985#issuecomment-2081463967)
Force-pushed because somehow had gzip and zlib names mixed up in my head, only the branch name remains 😄

💬 theStack commented on pull request "refactor: remove remaining unused code from cpp-subprocess":
(https://github.com/bitcoin/bitcoin/pull/29961#discussion_r1582144839)
Oh sorry, I missed that. Force-pushed now with a commit where only `Popen::kill()` is removed (and `Popen::poll()` is kept as-is).

💬 sipa commented on pull request "net: Replace ifname check with IFF_LOOPBACK in Discover":
(https://github.com/bitcoin/bitcoin/pull/29984#issuecomment-2081471875)
utACK a68fed111be393ddbbcd7451f78bc63601253ee0

📝 sdaftuar opened a pull request: "test: Don't rely on incentive incompatible replacement in mempool_accept_v3.py"
(https://github.com/bitcoin/bitcoin/pull/29986)
In the sibling eviction test, we're currently testing that a transaction with ancestor feerate of 179 is able to replace a transaction with ancestor feerate of 300, due to a shortcoming in our current RBF rules.

In preparation for fixing our RBF rules to not allow such replacements, fix the test by bumping the fee of the replacement to be a bit higher.

💬 sdaftuar commented on pull request "test: Don't rely on incentive incompatible replacement in mempool_accept_v3.py":
(https://github.com/bitcoin/bitcoin/pull/29986#issuecomment-2081474305)
@glozow @instagibbs This was something I noticed when rebasing #28676.

📝 fanquake opened a pull request: "guix: build with glibc 2.31"
(https://github.com/bitcoin/bitcoin/pull/29987)
Set minimum required glibc to 2.31.
The glibc 2.31 branch is still maintained: https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/release/2.31/master.

Remove the stack-protector check from test-security-check, as the test
no-longer fails, and given the control we have of the end, the actual
security-check test seems sufficient (this might also be applied to some
of the other checks).