💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1509003472)
> I agree with suggestions from @stickies-v, would be happy to re-review.

I'll let you both know ones the new PR is ready, very soon. Thanks.

💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1167123408)
Yeah, the intention was to add the param key on top of the previous error, adding more details at this level but that info is also available from the caller of `GetQueryParameterFromUri`. Ok, I'll leave it out.

📝 jonatack opened a pull request: "p2p: skip netgroup diversity follow-up"
(https://github.com/bitcoin/bitcoin/pull/27467)
In #27374 the role of the `setConnected` data structure in `CConnman::ThreadOpenConnections` changed from the distinct set of outbound peer netgroups to those of outbound IPv4/6 peers only.

In accordance with the changed semantics, this pull updates a code comment regarding feeler connections and proposes to update the naming of `setConnected` to `outbound_ipv46_peer_netgroups_set`.

🤔 jonatack reviewed a pull request: "p2p: skip netgroup diversity of new connections for tor/i2p/cjdns"
(https://github.com/bitcoin/bitcoin/pull/27374#pullrequestreview-1386023175)
Post-merge ACK.

I am not completely sure the additional complexity involved is worth it versus reverting [`72e8ffd` (#27264)](https://github.com/bitcoin/bitcoin/pull/27264/commits/72e8ffd7f8dbf908e65da6d012ede914596737ab), but the change itself here seems fine, modulo a couple of minor loose ends.

> if we run a node only on tor/i2p/cjdns

Via interactions with users, my understanding is that it may possibly be fairly often the case that node operators are running on non-clearnet network
...

💬 jonatack commented on pull request "p2p: skip netgroup diversity of new connections for tor/i2p/cjdns":
(https://github.com/bitcoin/bitcoin/pull/27374#discussion_r1167172725)
The code doc at https://github.com/bitcoin/bitcoin/pull/27374/files#diff-00021eed586a482abdb09d6cdada1d90115abe988a91421851960e26658bed02R1858 is now out of date.

The naming of `setConnected` should probably be updated to its new semantics.

Addressed in #27467.

💬 pinheadmz commented on pull request "Add wallet method to detect if a key is "active"":
(https://github.com/bitcoin/bitcoin/pull/27216#issuecomment-1509119182)
thanks again @jonatack
rebase to 8139075244f1f361a14f80920c45d8c4f41d553e
- refactor commits to separate tests in hygenic order!
- address review nits

💬 pinheadmz commented on issue "Cannot change passphrase even if it is correct":
(https://github.com/bitcoin/bitcoin/issues/19036#issuecomment-1509158547)
Closing this due to inactivity and lack of follow-up and reproducibility. Feel free to add comments if more details become available.

✅ pinheadmz closed an issue: "Cannot change passphrase even if it is correct"
(https://github.com/bitcoin/bitcoin/issues/19036)

💬 jonatack commented on pull request "assumeutxo: net_processing changes":
(https://github.com/bitcoin/bitcoin/pull/24008#issuecomment-1509171387)
> Approach ACK [0ef0057](https://github.com/bitcoin/bitcoin/commit/0ef00576cc9a4aac27310b9fc29c534ad829aad9) the logic seems good AFAICS

Will re-review after my last review 14 months age.

Finishing the 2 remaining PRs in the parent https://github.com/bitcoin/bitcoin/pull/15606 after v25 branch-off, to target v26 might be a reasonable goal.

💬 pinheadmz commented on issue "Transactions in stale blocks still show in txindex":
(https://github.com/bitcoin/bitcoin/issues/6787#issuecomment-1509182160)
> Eh, sounds like "works as intended" to me.

Agreed. This is what I expect the index to do, I think we can close this issue

✅ pinheadmz closed an issue: "Transactions in stale blocks still show in txindex"
(https://github.com/bitcoin/bitcoin/issues/6787)

💬 instagibbs commented on pull request "[POLICY] Ephemeral anchors":
(https://github.com/bitcoin/bitcoin/pull/26403#issuecomment-1509185568)
I reworked this PR with changes incorporated from https://github.com/bitcoin-inquisition/bitcoin/pull/23

💬 fakhroshumailasabri786 commented on issue "Crash using getnewaddress in the console":
(https://github.com/bitcoin-core/gui/issues/725#issuecomment-1509232041)

💬 ishaanam commented on pull request "bumpfee: avoid making bumped transactions with too low fee when replacing outputs":
(https://github.com/bitcoin/bitcoin/pull/27308#issuecomment-1509354550)
reACK d52fa1b0a5a8eecbe1e296a44b72965717e9235b

📝 pablomartin4btc opened a pull request: "bugfix: rest: avoid segfault for invalid URI"
(https://github.com/bitcoin/bitcoin/pull/27468)
Minimal fix to get it promptly into 25.0 release (suggested by [@stickies-v](https://github.com/bitcoin/bitcoin/pull/27253#pullrequestreview-1385130381) and supported by [@vasild](https://github.com/bitcoin/bitcoin/pull/27253#pullrequestreview-1385842606) )

Please check #27253 for reviewers comments and acks regarding this PR and read the commit comment message body for more details about the fix.

💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1509401910)
Updated changes:
- Created new PR #27468 with a minimal bugfix and single commit in order to merge it faster for 25.0 release, as suggested above by [@stickies-v](https://github.com/bitcoin/bitcoin/pull/27253#pullrequestreview-1385130381) and supported by [@vasild](https://github.com/bitcoin/bitcoin/pull/27253#pullrequestreview-1385842606).
- The new PR contains also the suggested changes from @stickies-v.

💬 rsafier commented on pull request "Allow configuring target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#issuecomment-1509467328)
ACK d8434da3c14ed6723d86ef2cd266008d366e1413
Tested against parameters for Mutinynet above, Plebnet Playground, ZBD signet as a client, synced up no issues.
Tested mining on throw away random signet at 1s block time target for 2300 blocks, seems good, didn't suddenly get harder.

💬 Xekyo commented on pull request "bumpfee: avoid making bumped transactions with too low fee when replacing outputs":
(https://github.com/bitcoin/bitcoin/pull/27308#issuecomment-1509552056)
reACK https://github.com/bitcoin/bitcoin/commit/d52fa1b0a5a8eecbe1e296a44b72965717e9235b

👍 Ayush170-Future approved a pull request: "p2p: skip netgroup diversity follow-up"
(https://github.com/bitcoin/bitcoin/pull/27467#pullrequestreview-1386334439)
ACK on the concept.

The code also looks good to me.

💬 pinheadmz commented on pull request "Make GUI and CLI tools use the same datadir":
(https://github.com/bitcoin/bitcoin/pull/27409#issuecomment-1509735704)
This PR probably closes https://github.com/bitcoin/bitcoin/issues/8106