💬 pinheadmz commented on pull request "net: Allow inbound whitebind connections to more aggressively evict peers when slots are full":
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1226883074)
ok I like this, will do
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1226883074)
ok I like this, will do
🚀 fanquake merged a pull request: "Add public Boost headers explicitly"
(https://github.com/bitcoin/bitcoin/pull/27783)
(https://github.com/bitcoin/bitcoin/pull/27783)
💬 fanquake commented on pull request "kernel: Remove args, settings, chainparams, chainparamsbase from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27576#discussion_r1226884126)
@TheCharlatan did you want to open a followup for this?
(https://github.com/bitcoin/bitcoin/pull/27576#discussion_r1226884126)
@TheCharlatan did you want to open a followup for this?
👋 TheCharlatan's pull request is ready for review: "kernel: Add interrupt class"
(https://github.com/bitcoin/bitcoin/pull/27861)
(https://github.com/bitcoin/bitcoin/pull/27861)
💬 TheCharlatan commented on pull request "Return EXIT_FAILURE on post-init fatal errors":
(https://github.com/bitcoin/bitcoin/pull/27708#discussion_r1226886073)
FWIW, this could be de-globalized again by the interrupt class introduced in https://github.com/bitcoin/bitcoin/pull/27861.
(https://github.com/bitcoin/bitcoin/pull/27708#discussion_r1226886073)
FWIW, this could be de-globalized again by the interrupt class introduced in https://github.com/bitcoin/bitcoin/pull/27861.
💬 TheCharlatan commented on pull request "kernel: Remove args, settings, chainparams, chainparamsbase from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27576#discussion_r1226890229)
Yes.
(https://github.com/bitcoin/bitcoin/pull/27576#discussion_r1226890229)
Yes.
💬 pinheadmz commented on pull request "net: Allow inbound whitebind connections to more aggressively evict peers when slots are full":
(https://github.com/bitcoin/bitcoin/pull/27600#issuecomment-1587634263)
@LarryRuane updated PR description to clarify where we get the random peer, added comment about the possible nullopt return value, and removed the default `force` value in `SelectNodeToEvict()`
(https://github.com/bitcoin/bitcoin/pull/27600#issuecomment-1587634263)
@LarryRuane updated PR description to clarify where we get the random peer, added comment about the possible nullopt return value, and removed the default `force` value in `SelectNodeToEvict()`
💬 TheBlueMatt commented on pull request "[WIP] add a stratum v2 template provider":
(https://github.com/bitcoin/bitcoin/pull/27854#issuecomment-1587650914)
> None of these deficiencies require the full StratumV2 approach to address;
This protocol basically is a protocol designed exactly for this use-case. The fact that it comes under a "StratumV2" banner is somewhat unrelated, there's only some common framing. It's also not a substantial protocol, arguably simpler than getblocktemplate in full.
This protocol exists basically to create an alternative to getblocktemplate which solves key problems in it, and can be used with or without any other Str
...
(https://github.com/bitcoin/bitcoin/pull/27854#issuecomment-1587650914)
> None of these deficiencies require the full StratumV2 approach to address;
This protocol basically is a protocol designed exactly for this use-case. The fact that it comes under a "StratumV2" banner is somewhat unrelated, there's only some common framing. It's also not a substantial protocol, arguably simpler than getblocktemplate in full.
This protocol exists basically to create an alternative to getblocktemplate which solves key problems in it, and can be used with or without any other Str
...
💬 ryanofsky commented on pull request "Return EXIT_FAILURE on post-init fatal errors":
(https://github.com/bitcoin/bitcoin/pull/27708#discussion_r1226913429)
> FWIW, this could be de-globalized again by the interrupt class introduced in #27861.
Yes #27861 replaces the `AbortNode` function with a `KernelNotifications::fatalError` method that can access whatever state it needs through the `KernelNotifications` class without needing any globals. `AbortNode` is the only function using `g_exit_status` so it should just disappear then.
(https://github.com/bitcoin/bitcoin/pull/27708#discussion_r1226913429)
> FWIW, this could be de-globalized again by the interrupt class introduced in #27861.
Yes #27861 replaces the `AbortNode` function with a `KernelNotifications::fatalError` method that can access whatever state it needs through the `KernelNotifications` class without needing any globals. `AbortNode` is the only function using `g_exit_status` so it should just disappear then.
💬 ryanofsky commented on pull request "Return EXIT_FAILURE on post-init fatal errors":
(https://github.com/bitcoin/bitcoin/pull/27708#discussion_r1226914676)
> Does this make `InitShutdownState()` required before calling `AbortNode()` ? I suppose that could be added to the comment here.
It could be mentioned but `AbortNode` is about to be removed in https://github.com/bitcoin/bitcoin/pull/27861, so it isn't too important
(https://github.com/bitcoin/bitcoin/pull/27708#discussion_r1226914676)
> Does this make `InitShutdownState()` required before calling `AbortNode()` ? I suppose that could be added to the comment here.
It could be mentioned but `AbortNode` is about to be removed in https://github.com/bitcoin/bitcoin/pull/27861, so it isn't too important
💬 brunoerg commented on pull request "rest: bugfix, fix crash error when calling `/deploymentinfo`":
(https://github.com/bitcoin/bitcoin/pull/27853#discussion_r1226915435)
> Also just querying the previous hash seems more efficient than running generate?
For sure, and for what we'd like to test may be enough, I'm gonna change it.
(https://github.com/bitcoin/bitcoin/pull/27853#discussion_r1226915435)
> Also just querying the previous hash seems more efficient than running generate?
For sure, and for what we'd like to test may be enough, I'm gonna change it.
💬 Xekyo commented on pull request "Bump unconfirmed ancestor transactions to target feerate":
(https://github.com/bitcoin/bitcoin/pull/26152#issuecomment-1587664581)
Rebased on master, incorporated outstanding follow-ups from https://github.com/bitcoin/bitcoin/pull/27021#issuecomment-1554675156
(https://github.com/bitcoin/bitcoin/pull/26152#issuecomment-1587664581)
Rebased on master, incorporated outstanding follow-ups from https://github.com/bitcoin/bitcoin/pull/27021#issuecomment-1554675156
📝 ryanofsky opened a pull request: "validation: Stricter assumeutxo error handling when renaming chainstates"
(https://github.com/bitcoin/bitcoin/pull/27862)
There are two places in assumeutxo code where it is calling `AbortNode` to trigger asynchronous shutdowns without returning errors to calling functions.
One case, in `LoadChainstate`, happens when snapshot validation succeeds, and there is an error trying to replace the background chainstate with the snapshot chainstate.
The other case, in `InvalidateCoinsDBOnDisk`, happens when snapshot validatiion fails, and there is an error trying to remove the snapshot chainstate.
In both cases the
...
(https://github.com/bitcoin/bitcoin/pull/27862)
There are two places in assumeutxo code where it is calling `AbortNode` to trigger asynchronous shutdowns without returning errors to calling functions.
One case, in `LoadChainstate`, happens when snapshot validation succeeds, and there is an error trying to replace the background chainstate with the snapshot chainstate.
The other case, in `InvalidateCoinsDBOnDisk`, happens when snapshot validatiion fails, and there is an error trying to remove the snapshot chainstate.
In both cases the
...
💬 brunoerg commented on pull request "rest: bugfix, fix crash error when calling `/deploymentinfo`":
(https://github.com/bitcoin/bitcoin/pull/27853#issuecomment-1587672780)
Force-pushed addressing https://github.com/bitcoin/bitcoin/pull/27853/files#r1226690678
Thanks @stickies-v for the review.
(https://github.com/bitcoin/bitcoin/pull/27853#issuecomment-1587672780)
Force-pushed addressing https://github.com/bitcoin/bitcoin/pull/27853/files#r1226690678
Thanks @stickies-v for the review.
🤔 TheBlueMatt reviewed a pull request: "[WIP] add a stratum v2 template provider"
(https://github.com/bitcoin/bitcoin/pull/27854#pullrequestreview-1475323709)
Just a few random notes, I'm not qualified to review modern C++ anymore :joy:
(https://github.com/bitcoin/bitcoin/pull/27854#pullrequestreview-1475323709)
Just a few random notes, I'm not qualified to review modern C++ anymore :joy:
💬 TheBlueMatt commented on pull request "[WIP] add a stratum v2 template provider":
(https://github.com/bitcoin/bitcoin/pull/27854#discussion_r1226927976)
Why is this a config flag? The message future_template bit indicates that its a predicted template, and presumably we'll have a stream of both predicted and current templates on the wire, not something in the options?
(https://github.com/bitcoin/bitcoin/pull/27854#discussion_r1226927976)
Why is this a config flag? The message future_template bit indicates that its a predicted template, and presumably we'll have a stream of both predicted and current templates on the wire, not something in the options?
💬 TheBlueMatt commented on pull request "[WIP] add a stratum v2 template provider":
(https://github.com/bitcoin/bitcoin/pull/27854#discussion_r1226928617)
Is there a reason not to hard-code this and optional_features until we actually have a need for them? Just less code if we do.
(https://github.com/bitcoin/bitcoin/pull/27854#discussion_r1226928617)
Is there a reason not to hard-code this and optional_features until we actually have a need for them? Just less code if we do.
💬 TheBlueMatt commented on pull request "[WIP] add a stratum v2 template provider":
(https://github.com/bitcoin/bitcoin/pull/27854#discussion_r1226922409)
Why bother making this optional? Can just have a config flag.
(https://github.com/bitcoin/bitcoin/pull/27854#discussion_r1226922409)
Why bother making this optional? Can just have a config flag.
💬 TheBlueMatt commented on pull request "[WIP] add a stratum v2 template provider":
(https://github.com/bitcoin/bitcoin/pull/27854#discussion_r1226924386)
Shouldnt this be non-0? 1KB would match the current GBT logic. More generally, there doesn't seem to be a way to set this, somehow I'd thought it was in the exposed protocol, but if not it probably should be.
(https://github.com/bitcoin/bitcoin/pull/27854#discussion_r1226924386)
Shouldnt this be non-0? 1KB would match the current GBT logic. More generally, there doesn't seem to be a way to set this, somehow I'd thought it was in the exposed protocol, but if not it probably should be.
💬 dimitaracev commented on pull request "validation: Replace MinBIP9WarningHeight with MinBIP9WarningStartTime":
(https://github.com/bitcoin/bitcoin/pull/27427#issuecomment-1587686012)
Went with the approach proposed by @ajtowns . Commit 8d39803
(https://github.com/bitcoin/bitcoin/pull/27427#issuecomment-1587686012)
Went with the approach proposed by @ajtowns . Commit 8d39803