💬 achow101 commented on pull request "wallet: improve IBD sync time by skipping block scanning prior birth time":
(https://github.com/bitcoin/bitcoin/pull/27469#issuecomment-1565133194)
re-ACK 82bb7831fa6052620998c7eef47e48ed594248a8

💬 amitiuttarwar commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1565134780)
> Ah, I think the problem with the test is that `AddRandomOutboundPeer` creates random IPv4 addresses, and sometimes these aren't routable (e.g. protected ranges), so that they are not assigned to `NET_IPV4` but `NET_UNROUTABLE`, which then makes the test fail.

great find @mzumsande ! I was able to reproduce & observe this behavior, so I added a loop in `AddRandomOutboundPeer` to try again if the selected address is not routable. hopefully this works, I ran the current version >1000 times and
...

🚀 achow101 merged a pull request: "wallet: improve IBD sync time by skipping block scanning prior birth time"
(https://github.com/bitcoin/bitcoin/pull/27469)

💬 ishaanam commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#issuecomment-1565156770)
> If mapTxSpends.count(outpoint) == 1, the outpoint was spent only once, so the wallet shouldn't contain any conflicting tx.

This is not necessarily always true, because even though the conflicted wallet transaction must always have its spends added to `mapTxSpends`, this is not the case for all conflicting transactions.

For example, let's say that Alice has a utxo. She creates a transaction, tx1, spending this utxo to Bob. `AddToSpends` will get called on Bob's node for this transaction
...

💬 mmienko commented on pull request "update osx build docs":
(https://github.com/bitcoin/bitcoin/pull/27769#issuecomment-1565181500)
> Thanks, however steps for how to (re-)install your package manager...not something we want to maintain.

That's a fair point and I agree. Appreciate the review. If I see issues for macOS like this, I'll leave a comment there.

✅ mmienko closed a pull request: "update osx build docs"
(https://github.com/bitcoin/bitcoin/pull/27769)

💬 fanquake commented on pull request "fuzz: Change LIMIT_TO_MESSAGE_TYPE from a compile-time to a run-time setting":
(https://github.com/bitcoin/bitcoin/pull/27766#issuecomment-1565301478)
Nice. Should help with oss-fuzz disk issues. Related to https://github.com/bitcoin-core/qa-assets/pull/131

🚀 fanquake merged a pull request: "fuzz: Change LIMIT_TO_MESSAGE_TYPE from a compile-time to a run-time setting"
(https://github.com/bitcoin/bitcoin/pull/27766)

💬 brunoerg commented on pull request "fuzz: Change LIMIT_TO_MESSAGE_TYPE from a compile-time to a run-time setting":
(https://github.com/bitcoin/bitcoin/pull/27766#issuecomment-1565384526)
post-merge ACK 1111c9ac97ca0f0afeb5df45bc0970b761c3c9ff

💬 Sjors commented on pull request "assumeutxo: net_processing changes":
(https://github.com/bitcoin/bitcoin/pull/24008#issuecomment-1565416944)
@fjahr thanks for the write up! It's on my reading list. Will also look at @sdaftuar's PR.

💬 Sjors commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1207968175)
`> 0` is fine too, but I guess nobody will this to zero accidentally.

💬 Sjors commented on pull request "[25.x] Parallel compact block downloads":
(https://github.com/bitcoin/bitcoin/pull/27752#issuecomment-1565420134)
utACK 50c86f57af5c6a9aa2e4828aeb67d641340b0860

💬 MarcoFalke commented on pull request "Fix rpc_getblockfrompeer intermittency by introducing 'getblockfileinfo' RPC command":
(https://github.com/bitcoin/bitcoin/pull/27770#issuecomment-1565431844)
Not sure. My preference would be to figure out why the blocks are of different size and then make that deterministic instead.

💬 pablomartin4btc commented on pull request "httpserver, rest: improving URI validation":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1208014798)
I can't remember tbh, try to find it in the history with no success, not sure if I did it due to matching the consumers(?) or @vasild suggested it at some point...

💬 pablomartin4btc commented on pull request "httpserver, rest: improving URI validation":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1208014857)
Well, I think that an empty string wouldn't be a valid URI, at least the `scheme component` (the first part of it which is followed by a ":", which is again followed by the `hier-part` component (which may be empty) must be present according to the [RFC](https://www.rfc-editor.org/rfc/rfc3986#section-3). I thought it was perhaps validated inside the `evhttp_uri_parse` but I couldn't find it [there](https://github.com/libevent/libevent/blob/bca26524fc4cd7a9e79d210c1079baaa7d29835d/http.c#L4464-L4
...

💬 pablomartin4btc commented on pull request "httpserver, rest: improving URI validation":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1208015048)
Yeah missed them, done now, thanks!

💬 pablomartin4btc commented on pull request "httpserver, rest: improving URI validation":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1208015177)
Yeah and also it'll be more consistent with the comments above in the private declaration section.
https://github.com/bitcoin/bitcoin/blob/c72967f48ab9eb02b8dc9231fdcc6ee85d66b7b9/src/httpserver.h#L60-L63

💬 pablomartin4btc commented on pull request "httpserver, rest: improving URI validation":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1208015277)
Ok, done.

💬 pablomartin4btc commented on pull request "httpserver, rest: improving URI validation":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1565460712)
Updates:
- Incorporated most of @stickies-v suggestions
- I'll remove the `httpserver` private member `replySent` (as @vasild suggested and @stickies-v agrees with), the argument from the constructor and all the logic around it in a separated commit. I can squash it later into 1 if you like.

💬 stratospher commented on pull request "Introduce secp256k1 module with field and group classes to test framework":
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1207644980)
when defining classes in python, don't we skip parentheses?
using `class ECPubKey:` and `class ECKey:` instead of `class ECPubKey():` and `class ECKey():`