💬 hebasto commented on pull request "depends: bump boost to 1.86.0 and use new CMake buildsystem":
(https://github.com/bitcoin/bitcoin/pull/30434#discussion_r1810740623)
It seems that the previous variant is correct:
```suggestion
$(package)_config_opts_darwin=-DCMAKE_INSTALL_NAME_TOOL=true
```

💬 TheCharlatan commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#issuecomment-2429305648)
Concept ACK

💬 hebasto commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#issuecomment-2429319684)
TO fix CI failure:
```diff
--- a/src/mapport.cpp
+++ b/src/mapport.cpp
@@ -2,8 +2,6 @@
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

-#include <bitcoin-build-config.h> // IWYU pragma: keep
-
#include <mapport.h>

#include <clientversion.h>
```

💬 hebasto commented on pull request "depends: add *FLAGS to gen_id":
(https://github.com/bitcoin/bitcoin/pull/31125#issuecomment-2429329095)
My Guix build:
```
aarch64
6361934f5c9bd884aff2ef18000ca718974d8ea564939db6acf3f3ac6faa5e35 guix-build-cd048e03e258/output/aarch64-linux-gnu/SHA256SUMS.part
d3be037f7cd976b315f96303a56952e3eb5bb8f1963d01c0ede701af2cfee83a guix-build-cd048e03e258/output/aarch64-linux-gnu/bitcoin-cd048e03e258-aarch64-linux-gnu-debug.tar.gz
7c6e75134c8b6614758c024b7bdfcd3bddd0b652d374cbb852e365f00b360a18 guix-build-cd048e03e258/output/aarch64-linux-gnu/bitcoin-cd048e03e258-aarch64-linux-gnu.tar.gz
4514a22f
...

💬 laanwj commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#issuecomment-2429342997)
Big concept ACK on this. As mentioned, MiniUPnP is some pretty dangerous code that resulted in the only known exploitable RCE in bitcoin core in history. It does brittle C-based string manipulation to generate and parse XML and HTTP which still has a high risk of having further bugs.

For that reason it's been disabled by default for years, there is no path toward doing so, and meanwhile i doubt many people have been going through the hassle of enabling UPnP in the settings to open a port.


...

💬 hebasto commented on pull request "depends: add *FLAGS to gen_id":
(https://github.com/bitcoin/bitcoin/pull/31125#discussion_r1810790477)
I'm not sure about that as none of the `{CPP,C,CXX,LD}FLAGS` variables is propagated to the native packages:
```
$ cd depends
$ make HOST=arm64-apple-darwin MULTIPROCESS=1 print-native_libmultiprocess_cxxflags CXXFLAGS=-some-fancy-flag
native_libmultiprocess_cxxflags=
```

💬 hebasto commented on pull request "depends: add *FLAGS to gen_id":
(https://github.com/bitcoin/bitcoin/pull/31125#discussion_r1810791006)
nit:
```suggestion
# [ CPPFLAGS=... ] [CFLAGS=...] [CXXFLAGS=...] [LDFLAGS=...] \
```

---

While touching this code, the comment can be improved:
```diff
--- a/depends/gen_id
+++ b/depends/gen_id
@@ -3,7 +3,7 @@
# Usage: env [ CC=... ] [ C_STANDARD=...] [ CXX=... ] [CXX_STANDARD=...] \
# [ CPPFLAGS=... ] [CFLAGS=...] [CXXFLAGS=...] [LDFLAGS=...]
# [ AR=... ] [ NM=... ] [ RANLIB=... ] [ STRIP=... ] [ DEBUG=... ] \
-# [ LTO=... ] [ NO_
...

💬 TheCharlatan commented on pull request "depends: add *FLAGS to gen_id":
(https://github.com/bitcoin/bitcoin/pull/31125#discussion_r1810793581)
Nit: While you are at it maybe break before `./gen_id` too for readability purposes?

💬 TheCharlatan commented on pull request "depends: add *FLAGS to gen_id":
(https://github.com/bitcoin/bitcoin/pull/31125#discussion_r1810796779)
Mmh, isn't that intentional?

💬 mzumsande commented on pull request "Halt processing of unrequested transactions v2":
(https://github.com/bitcoin/bitcoin/pull/30572#issuecomment-2429390190)
> In my understanding, what you're describing is the following:

This looks like a separate issue. I was talking about orphan resolution removing the tx, you are talking about `TxRequestTracker` expiration removing it.

> I can go to any of your PRs too, change the code with one line, and say the code is failing dozen of tests

I wasn't changing a random line of code: the added code is guarded by a future P2P version upgrade and therefore unreachable and untestable as is - one way to expos
...

💬 fanquake commented on pull request "depends: add *FLAGS to gen_id":
(https://github.com/bitcoin/bitcoin/pull/31125#discussion_r1810818502)
Broken.

💬 fanquake commented on pull request "depends: add *FLAGS to gen_id":
(https://github.com/bitcoin/bitcoin/pull/31125#discussion_r1810818914)
Added.

⚠️ edilmedeiros opened an issue: "[TestFramework] TestShell.reset() will always fail"
(https://github.com/bitcoin/bitcoin/issues/31131)
[TestShell documentation](https://github.com/bitcoin/bitcoin/blob/28ce159bc327e6dfec34077ff2e379b23a95db65/test/functional/test-shell.md?plain=1#L167) mentions a `TestShell.reset()` method.

When experimenting with it, it will always fail with:

```
Traceback (most recent call last):
File "/Users/jose.edil/2-development/bitcoin/vinteum/foss-program/infra-signet-server/signet-server.py", line 169, in <module>
TestShell().reset()
File "/Users/jose.edil/2-development/bitcoin/bitcoin
...

📝 andrewtoth opened a pull request: "validation: fetch block inputs parallel threads ~17% faster IBD"
(https://github.com/bitcoin/bitcoin/pull/31132)
When fetching inputs in ConnectBlock, each input is fetched from the cache in series. A cache miss means a round trip to the disk db to fetch the outpoint and insert it into the cache. Since the db is locked from being written during ConnectTip, we can fetch all block inputs missing from the cache on parallel threads before entering ConnectBlock. Using this strategy resulted in a ~17% faster IBD (or master was ~21% slower).

Doing IBD from a local peer with default settings, stopping at height
...

🤔 hebasto reviewed a pull request: "Add Signet and testnet4 launch shortcuts for Windows"
(https://github.com/bitcoin/bitcoin/pull/26334#pullrequestreview-2385433421)
Post-merge ACK cfd03de965a081facbd72316c76603dd7aa511bd.

👍 fanquake approved a pull request: "ci: Approximate MAKEJOBS in image build phase"
(https://github.com/bitcoin/bitcoin/pull/30935#pullrequestreview-2385444373)
ACK fa71bedf8609f06618aa85342ea6f5c4d2c5fea0

🚀 fanquake merged a pull request: "ci: Approximate MAKEJOBS in image build phase"
(https://github.com/bitcoin/bitcoin/pull/30935)

🚀 fanquake merged a pull request: "[27.x] Prep for 27.2"
(https://github.com/bitcoin/bitcoin/pull/31101)

💬 murchandamus commented on issue "Testnet4 consensus failure due to timewarp related "softfork"":
(https://github.com/bitcoin/bitcoin/issues/30786#issuecomment-2429522291)
I have the impression that the person that was selling testnet3 coins may be the one that has monopolized mining testnet4 now

⚠️ laanwj opened an issue: "net: Tor service target port collides when running multiple nodes, making bitcoind error out"
(https://github.com/bitcoin/bitcoin/issues/31133)
Various people have reported this on IRC after the 28.0 release, but have not created a GH issue, so i'm creating one.

To be able to distinguish incoming localhost connections from connections on the Tor hidden (onion) service, we automatically create a separate "Tor service target" P2P port. This port is always opened, even if `-listenonion=0`, because the user might manually configure a Tor hidden service and wants the same behavior.

[DefaultOnionServiceTarget()](https://github.com/bitco
...