💬 instagibbs commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1196823777)
done
(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
(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.
(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
(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
(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
(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.
(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
...
(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!
(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
(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)
(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)
(https://github.com/bitcoin/bitcoin/issues/27558)
💬 instagibbs commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1196858330)
> If in the situation where an outbound (non-hb) peer is the first to announce that we still always send the getblocktxn if compact blocks fail, then this new logic would not be making anything worse, which seems sufficient to me.
Exactly. Fixed, I think!
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1196858330)
> If in the situation where an outbound (non-hb) peer is the first to announce that we still always send the getblocktxn if compact blocks fail, then this new logic would not be making anything worse, which seems sufficient to me.
Exactly. Fixed, I think!
💬 instagibbs commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1196858467)
Looks like some lost in translation logic from prior PRs. Indeed, we can just peek at the first entry, if it exists. Fixed, I think.
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1196858467)
Looks like some lost in translation logic from prior PRs. Indeed, we can just peek at the first entry, if it exists. Fixed, I think.
💬 sdaftuar commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#issuecomment-1551819825)
Thanks for the quick updates -- the code looks right to me now; will test.
(https://github.com/bitcoin/bitcoin/pull/27626#issuecomment-1551819825)
Thanks for the quick updates -- the code looks right to me now; will test.
💬 sinetek commented on issue "Option to prevent sleep":
(https://github.com/bitcoin/bitcoin/issues/27692#issuecomment-1551831106)
I don't think we should start messing with the user's sleep settings, it will just be confusing if every app did that. They can set themselves the power/conservative profile depending on their needs.
(https://github.com/bitcoin/bitcoin/issues/27692#issuecomment-1551831106)
I don't think we should start messing with the user's sleep settings, it will just be confusing if every app did that. They can set themselves the power/conservative profile depending on their needs.
💬 brunoerg commented on pull request "test: fix intermittent issue in `feature_bip68_sequence`":
(https://github.com/bitcoin/bitcoin/pull/27177#discussion_r1196911811)
@achow101 I agree on removing the fallback to use any UTXO when there are no mature coins. Going to address it.
(https://github.com/bitcoin/bitcoin/pull/27177#discussion_r1196911811)
@achow101 I agree on removing the fallback to use any UTXO when there are no mature coins. Going to address it.
💬 brunoerg commented on pull request "test: fix intermittent issue in `feature_bip68_sequence`":
(https://github.com/bitcoin/bitcoin/pull/27177#discussion_r1196923956)
> The confirmation counts for the UTXOs can be very outdated when blocks are generated by things other than the MiniWallet. It would be nice if these could be kept up to date so that this filter is accurate.
So, can I update `get_utxos` as well?
(https://github.com/bitcoin/bitcoin/pull/27177#discussion_r1196923956)
> The confirmation counts for the UTXOs can be very outdated when blocks are generated by things other than the MiniWallet. It would be nice if these could be kept up to date so that this filter is accurate.
So, can I update `get_utxos` as well?
🤔 mzumsande reviewed a pull request: "net: Allow inbound whitebind connections to more aggressively evict peers when slots are full"
(https://github.com/bitcoin/bitcoin/pull/27600#pullrequestreview-1431340479)
As I suggested in the review club, an alternative, more simple approach would be to just pick a random peer after removing NoBan and outbound connections when in force-mode. Then, if at the end of the usual eviction algorithm, we don't have a evict-able peer, we would evict the random one instead.
This would make the code simpler (no need to change `EraseLastKElements` or keep track of `last`), and I don't really see a major downside:
* The `EraseLastKElements` eviction criteria don't really s
...
(https://github.com/bitcoin/bitcoin/pull/27600#pullrequestreview-1431340479)
As I suggested in the review club, an alternative, more simple approach would be to just pick a random peer after removing NoBan and outbound connections when in force-mode. Then, if at the end of the usual eviction algorithm, we don't have a evict-able peer, we would evict the random one instead.
This would make the code simpler (no need to change `EraseLastKElements` or keep track of `last`), and I don't really see a major downside:
* The `EraseLastKElements` eviction criteria don't really s
...
💬 TheCharlatan commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#issuecomment-1551909232)
Updated 0b88c307a8ed81705cf8e6fb6332fdf969eb0e2e -> a6612baf6aed92f74f96fa4bc04d0ee359f5cc3f ([chainstateRmKernelUiInterface_6](https://github.com/TheCharlatan/bitcoin/commits/chainstateRmKernelUiInterface_6) -> [chainstateRmKernelUiInterface_7](https://github.com/TheCharlatan/bitcoin/commits/chainstateRmKernelUiInterface_7), [compare](https://github.com/TheCharlatan/bitcoin/compare/chainstateRmKernelUiInterface_6..chainstateRmKernelUiInterface_7)).
* Addressed @ryanofsky's [comment](https://gi
...
(https://github.com/bitcoin/bitcoin/pull/27636#issuecomment-1551909232)
Updated 0b88c307a8ed81705cf8e6fb6332fdf969eb0e2e -> a6612baf6aed92f74f96fa4bc04d0ee359f5cc3f ([chainstateRmKernelUiInterface_6](https://github.com/TheCharlatan/bitcoin/commits/chainstateRmKernelUiInterface_6) -> [chainstateRmKernelUiInterface_7](https://github.com/TheCharlatan/bitcoin/commits/chainstateRmKernelUiInterface_7), [compare](https://github.com/TheCharlatan/bitcoin/compare/chainstateRmKernelUiInterface_6..chainstateRmKernelUiInterface_7)).
* Addressed @ryanofsky's [comment](https://gi
...