💬 setavenger commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2077491415)
> We should probably have a BIP352 specific `COutPoint` sorter, and a test vector.

Yeah, I'm working on a test vector that incorporates this kind of scenario.

💬 fanquake commented on pull request "depends: build miniupnpc with CMake":
(https://github.com/bitcoin/bitcoin/pull/29707#issuecomment-2077505030)
> FWIW, I found another Windows-specific bug in the upstream: https://github.com/miniupnp/miniupnp/pull/727.

Rebased again, but can pull in this change + fixups on next push

💬 fjahr commented on pull request "contrib: Add asmap-tool":
(https://github.com/bitcoin/bitcoin/pull/28793#discussion_r1579698526)
done

💬 fjahr commented on pull request "contrib: Add asmap-tool":
(https://github.com/bitcoin/bitcoin/pull/28793#discussion_r1579698620)
done

💬 fjahr commented on pull request "contrib: Add asmap-tool":
(https://github.com/bitcoin/bitcoin/pull/28793#discussion_r1579699392)
Thanks for this hint but I think usage of `pathlib` is usually preferred now because it is more modern. So I will keep this as is unless there are more pros to this change that I have overlooked.

💬 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)