💬 fanquake commented on pull request "update osx build docs":
(https://github.com/bitcoin/bitcoin/pull/27769#issuecomment-1564760572)
> (I didn't becuase I think I installed Homebrew from a guide rather than official site).

Concept NACK - Thanks, however steps for how to (re-)install your package manager, are off-topic for these installation instructions, and not something we want to maintain. I'd suggest always reading official (up-to-date) documentation, than random, third-party guides.

💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#issuecomment-1564803705)
@vasild did an overhaul of this PR and implemented more how we discussed on IRC. Edited the description to explain. I ended up not using `std::variant` because I felt it added uneeded complexity. Current approach is `Proxy` just has a `is_unix` flag and abstracts away the `CService` methods like `IsValid()` etc to support TCP or UNIX sockets

💬 stratospher commented on pull request "Introduce secp256k1 module with field and group classes to test framework":
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1207054125)
8990668: s/infinite/infinity

💬 stratospher commented on pull request "Introduce secp256k1 module with field and group classes to test framework":
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1207054357)
8990668: s/infinite/infinity
maybe make infinity representation in str and repr consistent too.

💬 stratospher commented on pull request "Introduce secp256k1 module with field and group classes to test framework":
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1207066672)
8990668: isn't the result updated and not cached here?

💬 stratospher commented on pull request "Introduce secp256k1 module with field and group classes to test framework":
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1207067694)
89906684: any particular reason why `v >= FE.SIZE` isn't supported? (have only thought about `ellswift_decode` [scenario](https://github.com/bitcoin/bips/blob/master/bip-0324/ellswift_decode_test_vectors.csv#L77) where v could be greater than `FE.SIZE`)

💬 sipa commented on pull request "Introduce secp256k1 module with field and group classes to test framework":
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1207264222)
I don't understand.

💬 sipa commented on pull request "Introduce secp256k1 module with field and group classes to test framework":
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1207265123)
__repr__ is supposed to produce a string that is valid Python code.

💬 amitiuttarwar commented on pull request "addrman: select addresses by network follow-up":
(https://github.com/bitcoin/bitcoin/pull/27745#discussion_r1207282099)
good catch! I intended to bundle this assume with the callers, think I accidentally reverted to an in-between state. will fix

✅ evansmj closed a pull request: "doc: Add Python install notes to build-osx.md."
(https://github.com/bitcoin/bitcoin/pull/27728)

💬 TheCharlatan commented on pull request "Return EXIT_FAILURE on post-init fatal errors":
(https://github.com/bitcoin/bitcoin/pull/27708#discussion_r1207346355)
> Which.. isn't bad as it will remove another ui_interface dependency.

Can you elaborate on this point? `AbortNode` still relies on `ui_interface` from what I can tell.

From my current understanding both the `StartShutdown` in `ThreadImport` and in the check are call shutdown and node `AbortNode`, because `BlockImport` might run before the ui is available (this is me guessing out of historical reasons). I am not sold on this point though, since this runs in another thread and we also prov
...

📝 furszy opened a pull request: "Fix rpc_getblockfrompeer intermittency by introducing 'getblockfileinfo' RPC command"
(https://github.com/bitcoin/bitcoin/pull/27770)
Attempt to fix #27749.

Implemented a `getblockfileinfo` RPC command to obtain block files related data.
So it can be used to fix the intermittency presented inside `rpc_getblockfrompeer.py`.

#27749 issue is the number of blocks contained by the first block file, which is not fixed
and can vary depending on the inner block sizes (what is fixed is the total size of the file
but not the number of blocks that the file contains).

Basically, `pruneblockchain(height)` tries to prune blocks
...

💬 furszy commented on issue "rpc_getblockfrompeer.py intermittent failure: assert_equal(pruneheight, 248); not(249 == 248)":
(https://github.com/bitcoin/bitcoin/issues/27749#issuecomment-1564966458)
I wasn't able to reproduce it but my spider sense tells me that it will be fixed by #27770.

💬 jamesob commented on pull request "assumeutxo: net_processing changes":
(https://github.com/bitcoin/bitcoin/pull/24008#issuecomment-1564974163)
Closing in lieu of some simpler changes that @sdaftuar has written, currently integrated in #27596.

✅ jamesob closed a pull request: "assumeutxo: net_processing changes"
(https://github.com/bitcoin/bitcoin/pull/24008)

💬 furszy commented on pull request "Return EXIT_FAILURE on post-init fatal errors":
(https://github.com/bitcoin/bitcoin/pull/27708#discussion_r1207406938)
> > Which.. isn't bad as it will remove another ui_interface dependency.
>
> Can you elaborate on this point? `AbortNode` still relies on `ui_interface` from what I can tell.

Ok, I confused you. Sorry.

The ui_interface dependency that can be removed is the `base.cpp` one. Thus why said to "bubble up" the error string up to `init.cpp` instead of calling `InitError` from the index base class `Start` method.

> From my current understanding both the `StartShutdown` in `ThreadImport` and
...

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

💬 LarryRuane commented on pull request "util: generalize accounting of system-allocated memory in pool resource":
(https://github.com/bitcoin/bitcoin/pull/27748#issuecomment-1565039269)
I decided to reopen this; I think it's a good generalization improvement to the pool allocator. See the description for details. Thanks, @martinus, for the suggested improvement!

💬 LarryRuane commented on pull request "util: generalize accounting of system-allocated memory in pool resource":
(https://github.com/bitcoin/bitcoin/pull/27748#issuecomment-1565042192)
GitHub isn't allowing me to reopen this; @achow101 could you re-open this for me, please? Or, if you'd prefer, I could make a new PR. Thanks!

TL;DR on this PR's history: When I created this PR, I thought I was fixing a bug. But @martinus pointed out that there is no bug, so I closed this PR. I then decided this would be a worthwhile improvement after all (although of course not trying to fix a bug), so I'd like it to be reopened.

💬 amitiuttarwar commented on pull request "addrman: select addresses by network follow-up":
(https://github.com/bitcoin/bitcoin/pull/27745#discussion_r1207469865)
should be ok now (both here and in `GetEntry`)