💬 fjahr commented on pull request "contrib: Add asmap-tool":
(https://github.com/bitcoin/bitcoin/pull/28793#issuecomment-2077562279)
Addressed comments from @virtu , thanks for reviewing!

💬 sr-gi commented on pull request "net: Decrease nMaxIPs when learning from DNS seeds":
(https://github.com/bitcoin/bitcoin/pull/29850#issuecomment-2077565724)
Post-merge ACK f2e3662e57eca1330962faf38ff428a564d50a11

💬 twosatsmaxi commented on pull request "policy: Allow non-standard scripts with -acceptnonstdtxn=1 (test nets only)":
(https://github.com/bitcoin/bitcoin/pull/29843#issuecomment-2077629808)
Following.

📝 theStack opened a pull request: "refactor: remove remaining unused code from cpp-subprocess"
(https://github.com/bitcoin/bitcoin/pull/29961)
This PR removes remaining code that is unused within the cpp-subprocess module (templates and commented out code). Happy to add more removals if anyone finds more unused parts. Note that there are some API functions of the `Popen` class that we don't use, e.g. `wait()`, `pid()`, `poll()`, `kill()`, but they sound IMHO common enough to be useful in the future, so not sure how deep we should go there.

🤔 tdb3 reviewed a pull request: "contrib: rpcauth.py - Add new option (-json) to output text in json format"
(https://github.com/bitcoin/bitcoin/pull/29433#pullrequestreview-2022987855)
Thanks for squashing commits.
ACK for 9adf949d2aa6d199b85295b18c08967395b5570a

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

💬 achow101 commented on pull request "lint: scripted-diff verification also requires GNU grep":
(https://github.com/bitcoin/bitcoin/pull/29689#issuecomment-2077705790)
ACK 3bf4f8db669e1e274ce2633cf84add2938b9914b

🚀 achow101 merged a pull request: "lint: scripted-diff verification also requires GNU grep"
(https://github.com/bitcoin/bitcoin/pull/29689)

🤔 brunoerg reviewed a pull request: "net: Make AddrFetch connections to fixed seeds"
(https://github.com/bitcoin/bitcoin/pull/26114#pullrequestreview-2023068219)
Just tested with `-nodnsseed -blocksonly -debug=net` with an empty `peers.dat` and checked the behaviour is as expected. Some logs:

1. Adding fixed seeds
```
2024-04-25T13:02:59Z Adding fixed seeds as -dnsseed=0 (or IPv4/IPv6 connections are disabled via -onlynet) and neither -addnode nor -seednode are provided
2024-04-25T13:02:59Z [net] Added hardcoded seed: 1.253.159.19:8333
2024-04-25T13:02:59Z [net] Added hardcoded seed: 2.152.74.211:8333
2024-04-25T13:02:59Z [net] Added hardcoded se
...

💬 achow101 commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#issuecomment-2077749583)
ACK cb8f83df1916308013a6ae4b8e4914eb805c5343

💬 theuni commented on pull request "depends: pass verbose through to cmake based makefiles":
(https://github.com/bitcoin/bitcoin/pull/29960#issuecomment-2077756588)
Concept ACK. I just ran into this locally. I solved it using `V=1 VERBOSE=1`, but this makes sense too.

✅ ryanofsky closed an issue: "util-{util,wallet}: undefined reference to `evhttp_uridecode'"
(https://github.com/bitcoin/bitcoin/issues/29654)

🚀 ryanofsky merged a pull request: "refactor: Use our own implementation of urlDecode"
(https://github.com/bitcoin/bitcoin/pull/29904)

💬 laanwj commented on issue "Add IPv6 pinhole support using UPnP / NAT-PMP":
(https://github.com/bitcoin/bitcoin/issues/17012#issuecomment-2077766337)
Manually creating an IPv6 pinhole seems to work here:
```sh
$ export ROUTER_IPV6_ADDR=...
$ export MY_IPV6_ADDR=...
$ upnpc -6 -u 'http://[${ROUTER_IPV6_ADDR}]:5000/rootDesc.xml' -A "" 0 $MY_IPV6_ADDR 1234 tcp 30
upnpc : miniupnpc library test client, version 2.2.4.
(c) 2005-2022 Thomas Bernard.
Go to http://miniupnp.free.fr/ or https://miniupnp.tuxfamily.org/
for more information.
Found valid IGD : http://[...]:5000/ctl/IPConn
Local LAN ip address : ...
AddPinhole: ([]:0 -> [...]:1
...

💬 ryanofsky commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#issuecomment-2077766606)
I went ahead and merged this, despite NACK from luke https://github.com/bitcoin/bitcoin/pull/29904#issuecomment-2067871490 about costs of forking shared code. It seems like the NACK was adequately responded to, with responses acknowledging the tradeoff and giving reasons why it seemed justified in this case (even fixing a bug luke reported https://github.com/bitcoin/bitcoin/issues/29654)

📝 theuni opened a pull request: "RFC: depends: add release type to CMake builds"
(https://github.com/bitcoin/bitcoin/pull/29962)
RFC because I'm not sure if this is the right thing to do in combination with our CFLAGS/CXXFLAGS/etc env overrides.

I believe it was suggested by @laanwj at some point.

💬 theuni commented on pull request "RFC: depends: add release type to CMake builds":
(https://github.com/bitcoin/bitcoin/pull/29962#issuecomment-2077772971)
Ping @hebasto

💬 achow101 commented on pull request "test: Extends wait_for_getheaders so a specific block hash can be checked":
(https://github.com/bitcoin/bitcoin/pull/29736#issuecomment-2077785663)
ACK c4f857cc301d856f3c60acbe6271d3fe19441c7a

💬 hebasto commented on pull request "RFC: depends: add release type to CMake builds":
(https://github.com/bitcoin/bitcoin/pull/29962#issuecomment-2077790003)
The defaults for the "Makefile" generator are:
- "RelWithDebInfo" : `-O2 -g -DNDEBUG`
- "Debug': `-g`

That differs from our flags, which are `-O2` and `-O1 -g` for Linux respectively.

And CMake can changes its defaults at any time (

However, it would be nice to be explicit about the used build type, for example, `-DCMAKE_BUILD_TYPE=None`.

✅ achow101 closed an issue: "test: Check hash in wait_for_get* helpers"
(https://github.com/bitcoin/bitcoin/issues/18614)