💬 sdaftuar commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1196715533)
nit: this comment doesn't seem right

💬 sdaftuar commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1196683873)
Is it worth adding an `Assume()` here that we have strictly less than MAX_CMPCTBLOCKS_INFLIGHT_PER_BLOCK requests outstanding?

💬 sdaftuar commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1196732363)
This notion of `first_in_flight` is not correct, I think. Imagine we get a headers announcement from an outbound (non-hb) peer, and so we send a getdata(cmpctblock). In the meantime, we get a compact block announcement from an inbound peer which fails to reconstruct. Then the compact block comes back from the outbound peer; `first_in_flight` will be false because we think we have two requests outstanding.

Further down, when we decide whether to send a GETBLOCKTXN message to this non-HB out
...

💬 ddykeman1 commented on issue "Mac osx 12.6.5 ":
(https://github.com/bitcoin/bitcoin/issues/27681#issuecomment-1551672136)
Translated Report (Full Report Below)
-------------------------------------

Process: Bitcoin-Qt [844]
Path: /Applications/Bitcoin-Qt.app/Contents/MacOS/Bitcoin-Qt
Identifier: org.bitcoinfoundation.Bitcoin-Qt
Version: 24.0.1 (24.0.1)
Code Type: X86-64 (Native)
Parent Process: launchd [1]
User ID: 501

Date/Time: 2023-05-17 11:26:06.6243 -0400
OS Version: macOS 12.6.5 (21G53
...

💬 instagibbs commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#issuecomment-1551697808)
> However, it is possible that a peer starting up for the first time could have all its slots taken by inbounds.

Yeah, goes without saying, but maybe it doesn't. This PR offers no protection on fresh restart(no hb peers), and only offers 3 parallel downloads when an outbound is selected as hb and offers a block

💬 sdaftuar commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1196773469)
Actually, can we use the insertion order guarantees of multimap to solve this?

💬 sdaftuar commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1196774969)
Similarly here, can we use the insertion order of multimap to determine which peer was actually first to make the announcement? If in the situation where an outbound (non-hb) peer is the first to announce that we still always get the block, then this new logic would not be making anything worse, which seems sufficient to me.

💬 TheCharlatan commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1196781453)
Not sure about this one. The include is required for setting the default argument and it is non-const, because we mutate the message.

⚠️ Sataur opened an issue: "Option to prevent sleep"
(https://github.com/bitcoin/bitcoin/issues/27692)
### Please describe the feature you'd like to see added.

There should be option to start Core which enable system to sleep only when net. act. is disabled!

### Is your feature related to a problem, if so please describe it.

_No response_

### Describe the solution you'd like

_To [notify](https://learn.microsoft.com/en-us/windows/win32/power/system-sleep-criteria) the system that your application is busy, use the.._

### Describe any alternatives you've considered

_constant disk activity abo
...

💬 instagibbs commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1196823777)
done

💬 instagibbs commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1196823829)
done

💬 instagibbs commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1196823878)
hmm yeah, this is old code, doesn't make sense to me either. removed the change.

💬 instagibbs commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1196823938)
done

💬 instagibbs commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1196823999)
done

💬 instagibbs commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1196824091)
removed nonsense copy/paste

💬 instagibbs commented on issue "Proposal for a new mempool design":
(https://github.com/bitcoin/bitcoin/issues/27677#issuecomment-1551776409)
> does that fundamentally change with Eltoo? I guess not.

No, all pin-avoiding designs I've thought of are 0-fee parent, CPFP-ing child.

💬 ryanofsky commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1196834892)
> Not sure about this one. The include is required for setting the default argument and it is non-const, because we mutate the message.

That's a good point about the include. But changing to a `const bilingual_str&` reference and avoiding unnecessary copies would provide a more consistent interface.

For one thing, getting of the mutations would actually make current implementations of the interface more legible:

```c++
InitError(user_message.empty() ? _("A fatal internal error occurred
...

💬 TheCharlatan commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1196840346)
Thanks for the follow-up!

👍 ryanofsky approved a pull request: "indexes: Read the locator's top block during init, allow interaction with reindex-chainstate"
(https://github.com/bitcoin/bitcoin/pull/25193#pullrequestreview-1431213143)
Code review ACK 97844d9268b87b5d09b1091bfd0326ed18ce5b91. Just simple rebase since last review

🚀 ryanofsky merged a pull request: "indexes: Read the locator's top block during init, allow interaction with reindex-chainstate"
(https://github.com/bitcoin/bitcoin/pull/25193)

✅ ryanofsky closed an issue: "Coinstats index corrupted after invalidateblock and clean shutdown"
(https://github.com/bitcoin/bitcoin/issues/27558)