💬 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():`

💬 stratospher commented on pull request "Introduce secp256k1 module with field and group classes to test framework":
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1207999486)
never mind, just reviewed key.py and saw how this is being used in `verify_schnorr`, `tweak_add_pubkey`. agree that it's better/simpler to keep it as it is.

💬 stratospher commented on pull request "Introduce secp256k1 module with field and group classes to test framework":
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1207758013)
I thought of "The result is cached." as:
1. someone calls `int()`
2. we do operations inside `int()`, compute what num and den should be and store it locally+lazily without updating actual `self.num`, `self.den`
3. someone calls another FE function
4. the newly computed value of `self.num`, `self.den` gets actually updated only here

isn't this what caching a variable conceptually means? (even though it doesn't matter here where `self.num`, `self.den` gets actually updated) what did y
...

💬 brunoerg commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1208033507)
Instead of pushing the node to `vNodes` and removing it if it's not routable, I think you could do something like:
```cpp
static void AddRandomOutboundPeer(NodeId& id, std::vector<CNode*>& vNodes, PeerManager& peerLogic, ConnmanTestMsg& connman, ConnectionType connType, bool onion_peer = false)
{
CAddress addr;

if (onion_peer) {
auto tor_addr{g_insecure_rand_ctx.randbytes(ADDR_TORV3_SIZE)};
BOOST_REQUIRE(addr.SetSpecial(OnionToString(tor_addr)));
}

wh
...

💬 furszy 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-1565503469)
Ok nice, great. Thanks for the details.
We should code a test exercising exactly that behavior.