💬 ryanofsky commented on pull request "Disable util::Result copying and assignment":
(https://github.com/bitcoin/bitcoin/pull/29906#discussion_r1579632793)
re: https://github.com/bitcoin/bitcoin/pull/29906#discussion_r1579500335

It seems like the goal of the code is to get a new address without unlocking. But I haven't looked into this. It definitely looks a little clumsy, though, because it isn't actually using the error message that is returned.

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