π¬ achow101 commented on pull request "gui: Add a menu action to restore then migrate a legacy wallet":
(https://github.com/bitcoin-core/gui/pull/877#issuecomment-3432697596)
Are you still working on this?
(https://github.com/bitcoin-core/gui/pull/877#issuecomment-3432697596)
Are you still working on this?
π¬ furszy commented on pull request "http: limit RPC server threads to available cores":
(https://github.com/bitcoin/bitcoin/pull/33678#issuecomment-3432697958)
Answered without reading https://github.com/bitcoin/bitcoin/pull/31215#discussion_r1828025349, interesting. Maybe having a bit more is good but maybe not 16 fixed? Like a factor of 2 of the number of cores.
(https://github.com/bitcoin/bitcoin/pull/33678#issuecomment-3432697958)
Answered without reading https://github.com/bitcoin/bitcoin/pull/31215#discussion_r1828025349, interesting. Maybe having a bit more is good but maybe not 16 fixed? Like a factor of 2 of the number of cores.
β
achow101 closed a pull request: "Including exception what() in Runaway dialog box"
(https://github.com/bitcoin-core/gui/pull/876)
(https://github.com/bitcoin-core/gui/pull/876)
π¬ achow101 commented on pull request "Including exception what() in Runaway dialog box":
(https://github.com/bitcoin-core/gui/pull/876#issuecomment-3432701682)
This PR does not seem to have attracted much attention from reviewers. As such, it does not seem important enough right now to keep it sitting idle in the list of open PRs.
Closing due to lack of interest.
(https://github.com/bitcoin-core/gui/pull/876#issuecomment-3432701682)
This PR does not seem to have attracted much attention from reviewers. As such, it does not seem important enough right now to keep it sitting idle in the list of open PRs.
Closing due to lack of interest.
π¬ darosior commented on pull request "miniscript: fixes #29098 by only use first k valid signatures":
(https://github.com/bitcoin/bitcoin/pull/31719#issuecomment-3432713975)
While it may be desirable, i don't think this is a priority (as demonstrated by the lack of reviewer interest). Concept NACK.
(https://github.com/bitcoin/bitcoin/pull/31719#issuecomment-3432713975)
While it may be desirable, i don't think this is a priority (as demonstrated by the lack of reviewer interest). Concept NACK.
π theuni approved a pull request: "depends: Avoid `warning: "_FORTIFY_SOURCE" redefined` for `libevent`"
(https://github.com/bitcoin/bitcoin/pull/32266#pullrequestreview-3366133624)
utACK fe71a4b139f3a142468c2e931775813bc8f9d2ad
(https://github.com/bitcoin/bitcoin/pull/32266#pullrequestreview-3366133624)
utACK fe71a4b139f3a142468c2e931775813bc8f9d2ad
β
achow101 closed a pull request: "tests: Added progress tracker when running fuzz test_runner.py"
(https://github.com/bitcoin/bitcoin/pull/32354)
(https://github.com/bitcoin/bitcoin/pull/32354)
π¬ achow101 commented on pull request "tests: Added progress tracker when running fuzz test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/32354#issuecomment-3432732029)
This PR does not seem to have attracted much attention from reviewers. As such, it does not seem important enough right now to keep it sitting idle in the list of open PRs.
Closing due to lack of interest.
(https://github.com/bitcoin/bitcoin/pull/32354#issuecomment-3432732029)
This PR does not seem to have attracted much attention from reviewers. As such, it does not seem important enough right now to keep it sitting idle in the list of open PRs.
Closing due to lack of interest.
π¬ maflcko commented on pull request "depends: Avoid `warning: "_FORTIFY_SOURCE" redefined` for `libevent`":
(https://github.com/bitcoin/bitcoin/pull/32266#issuecomment-3432734644)
lgtm ACK fe71a4b139f3a142468c2e931775813bc8f9d2ad
ref: https://github.com/bitcoin/bitcoin/blob/7d27af98c7cf858b5ab5a02e64f89a857cc53172/CMakeLists.txt#L508
(https://github.com/bitcoin/bitcoin/pull/32266#issuecomment-3432734644)
lgtm ACK fe71a4b139f3a142468c2e931775813bc8f9d2ad
ref: https://github.com/bitcoin/bitcoin/blob/7d27af98c7cf858b5ab5a02e64f89a857cc53172/CMakeLists.txt#L508
π¬ l0rinc commented on pull request "Add option dbfilesize to control LevelDB target ("max") file size":
(https://github.com/bitcoin/bitcoin/pull/30059#issuecomment-3432739650)
Since the merge of the original PR changing this value from 2MB to 32MB, based on surfaced user feedback and secondary effects, it seems obvious to me this isn't something we should make configurable (it just makes it harder for us to reason about behavior).
https://github.com/bitcoin/bitcoin/issues/32455#issuecomment-2925382555 revealed that while this change does support dynamically changing file sizes (we've documented this in the original PR), it is understandably surprising to the users
...
(https://github.com/bitcoin/bitcoin/pull/30059#issuecomment-3432739650)
Since the merge of the original PR changing this value from 2MB to 32MB, based on surfaced user feedback and secondary effects, it seems obvious to me this isn't something we should make configurable (it just makes it harder for us to reason about behavior).
https://github.com/bitcoin/bitcoin/issues/32455#issuecomment-2925382555 revealed that while this change does support dynamically changing file sizes (we've documented this in the original PR), it is understandably surprising to the users
...
π¬ pablomartin4btc commented on pull request "Bugfix - don't allow multiple dialogs for same tx in TransactionView":
(https://github.com/bitcoin-core/gui/pull/817#issuecomment-3432742271)
Temporarily drafting it since I've found some issues on Qt 6 while reviewing #904. WIP.
(https://github.com/bitcoin-core/gui/pull/817#issuecomment-3432742271)
Temporarily drafting it since I've found some issues on Qt 6 while reviewing #904. WIP.
π¬ achow101 commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-3432744483)
Are you still working on this?
Are any of the Concept ACKers interested in actually reviewing this?
(https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-3432744483)
Are you still working on this?
Are any of the Concept ACKers interested in actually reviewing this?
π l0rinc opened a pull request: "validation: do not wipe utxo cache for stats/scans/snapshots"
(https://github.com/bitcoin/bitcoin/pull/33680)
Revival of https://github.com/bitcoin/bitcoin/pull/30610#issuecomment-3432564955
> Since #28280, the cost of a non-wiping sync of the UTXO cache is only proportional to the number of dirty entries, rather than proportional to the size of the entire cache. Because of that, there is no reason to perform a wiping flush in case the contents of the cache is still useful.
>
> Split the `FlushStateMode::ALWAYS` mode into a `FORCE_SYNC` (non-wiping) and a `FORCE_FLUSH` (wiping), and then use the f
...
(https://github.com/bitcoin/bitcoin/pull/33680)
Revival of https://github.com/bitcoin/bitcoin/pull/30610#issuecomment-3432564955
> Since #28280, the cost of a non-wiping sync of the UTXO cache is only proportional to the number of dirty entries, rather than proportional to the size of the entire cache. Because of that, there is no reason to perform a wiping flush in case the contents of the cache is still useful.
>
> Split the `FlushStateMode::ALWAYS` mode into a `FORCE_SYNC` (non-wiping) and a `FORCE_FLUSH` (wiping), and then use the f
...
π¬ l0rinc commented on pull request "validation: do not wipe utxo cache for stats/scans/snapshots":
(https://github.com/bitcoin/bitcoin/pull/30610#issuecomment-3432754607)
Revived in https://github.com/bitcoin/bitcoin/pull/33680, let's continue review there
(https://github.com/bitcoin/bitcoin/pull/30610#issuecomment-3432754607)
Revived in https://github.com/bitcoin/bitcoin/pull/33680, let's continue review there
π¬ achow101 commented on pull request "miner: don't needlessly append dummy OP_0 to bip34":
(https://github.com/bitcoin/bitcoin/pull/32420#issuecomment-3432756742)
Are you still working on this?
(https://github.com/bitcoin/bitcoin/pull/32420#issuecomment-3432756742)
Are you still working on this?
π¬ ryanofsky commented on pull request "init: Split file path handling out of -asmap option":
(https://github.com/bitcoin/bitcoin/pull/33631#issuecomment-3432763010)
@laanwj do you agree this PR would be unnecessary if the `ip_asn.map` file were loaded by default (or if asmap data were embedded in the binary and enabled by default)? Those seem like cleaner solutions to me.
Or if it is not safe to load asmap data be default, it would seem cleaner not to have a default'filename that is not actually used by default.
(https://github.com/bitcoin/bitcoin/pull/33631#issuecomment-3432763010)
@laanwj do you agree this PR would be unnecessary if the `ip_asn.map` file were loaded by default (or if asmap data were embedded in the binary and enabled by default)? Those seem like cleaner solutions to me.
Or if it is not safe to load asmap data be default, it would seem cleaner not to have a default'filename that is not actually used by default.
π¬ achow101 commented on pull request "doc: improve NODE_NETWORK_LIMITED documentation per BIP159":
(https://github.com/bitcoin/bitcoin/pull/31805#issuecomment-3432765461)
Are you still working on this?
(https://github.com/bitcoin/bitcoin/pull/31805#issuecomment-3432765461)
Are you still working on this?
β
furszy closed a pull request: "http: limit RPC server threads to available cores"
(https://github.com/bitcoin/bitcoin/pull/33678)
(https://github.com/bitcoin/bitcoin/pull/33678)
π¬ andrewtoth commented on pull request "http: limit RPC server threads to available cores":
(https://github.com/bitcoin/bitcoin/pull/33678#issuecomment-3432772787)
> I think it makes sense to not exceed that number, as having more threads than cores doesnβt really help.
This would make sense if RPCs are purely CPU bound. Many RPCs are mainly IO bound, and the OS can suspend a thread while it waits on IO. Absent coroutines that can suspend execution on the same thread for IO, more threads than cores will be a benefit.
(https://github.com/bitcoin/bitcoin/pull/33678#issuecomment-3432772787)
> I think it makes sense to not exceed that number, as having more threads than cores doesnβt really help.
This would make sense if RPCs are purely CPU bound. Many RPCs are mainly IO bound, and the OS can suspend a thread while it waits on IO. Absent coroutines that can suspend execution on the same thread for IO, more threads than cores will be a benefit.
π andrewtoth approved a pull request: "test: set number of RPC server threads to 2"
(https://github.com/bitcoin/bitcoin/pull/33679#pullrequestreview-3366198055)
ACK e9cd45e3d3c7592265ebf67387090b3df1501df4
Makes sense, similar change to https://github.com/bitcoin/bitcoin/pull/33485.
(https://github.com/bitcoin/bitcoin/pull/33679#pullrequestreview-3366198055)
ACK e9cd45e3d3c7592265ebf67387090b3df1501df4
Makes sense, similar change to https://github.com/bitcoin/bitcoin/pull/33485.