💬 LarryRuane commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1119099044)
nit
```suggestion
ret.push_back(it.value());
```
This may help the reader know that `it` is a `std::optional`; at first, I thought `GetIter()` may be returning a pointer. But if you prefer `*`, which is more concise, that's fine too.

💬 LarryRuane commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1119134160)
Should this function return `std::nullopt` instead?

💬 LarryRuane commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1119202243)
I'm concerned about the possible over-estimation here. In theory, there could be a combinatorial explosion if the DAG is highly interconnected and deep. I would consider removing this code, because vector `push_back()` is highly optimized when growth is needed. Or at least make sure there's a problem before adding this optimization.

If you do keep this, consider calling `cluster.shrink_to_fit()` before returning, so at least any high memory usage is temporary.

📝 ryanofsky opened a pull request: "refactor: Stop using gArgs global in system.cpp"
(https://github.com/bitcoin/bitcoin/pull/27170)
Most of the code in `util/system.cpp` that was hardcoded to use the global `ArgsManager` instance `gArgs` has been changed to stop using it (for example in https://github.com/bitcoin/bitcoin/pull/20092). But a few hardcoded references to `gArgs` remain. This commit removes the last ones so these functions aren't reading or writing global state.

Noticed these `gArgs` references while reviewing #27073

💬 S3RK commented on pull request "wallet: group outputs only once, decouple it from Coin Selection":
(https://github.com/bitcoin/bitcoin/pull/25806#discussion_r1119252440)
You're right, thanks for the explanation. This comment could be resolved.

💬 Muhammad4599 commented on pull request "refactor: wallet, remove global 'ArgsManager' dependency":
(https://github.com/bitcoin/bitcoin/pull/26889#discussion_r1119280534)
_[]()[]()_

💬 TheCharlatan commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#issuecomment-1447068021)
Updated e8b095f8ad65544f18ad69201d95e11ddf689f1f -> 899ba71e9d7caa370d8a0c4bed2e514f51024dc6 ([removeBlockstorageArgs_0](https://github.com/TheCharlatan/bitcoin/tree/removeBlockstorageArgs_0) -> [removeBlockstorageArgs_1](https://github.com/TheCharlatan/bitcoin/tree/removeBlockstorageArgs_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/removeBlockstorageArgs_0..removeBlockstorageArgs_1)) with suggested changes.
Rebased 899ba71e9d7caa370d8a0c4bed2e514f51024dc6 -> b640db3c5fbe49af5
...

💬 TheCharlatan commented on pull request "refactor: Stop using gArgs global in system.cpp":
(https://github.com/bitcoin/bitcoin/pull/27170#issuecomment-1447112820)
Concept ACK

💬 mzumsande commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1119334185)
ergonomically

💬 sipa commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#issuecomment-1447126690)
Concept ACK. Migrating to strict JSON-RPC 2.0 for `bitcoin-cli` and whatever opts into 2.0 is cool, even if it means keeping support for kinda-sorta-JSON-RPC 1.0 around on the server side.

💬 satsie commented on issue "Add per message stats to getnettotals rpc":
(https://github.com/bitcoin/bitcoin/issues/26337#issuecomment-1447193880)
Thank you everyone for your input on the concept and design for this issue! I'd like to present the work I have so far for feedback: https://github.com/satsie/bitcoin/pull/4

The PR description has some sample request/responses and summarizes the path I took to get here. I also have some open comments on areas I want to take a closer look at, or am hoping for input on.

Huge thank you to Amiti, Vasil, and AJ for already chiming in on previous iterations of the code :heart:. Since I last comm
...

💬 ryanofsky commented on pull request "Deduplicate bitcoind and bitcoin-qt init code":
(https://github.com/bitcoin/bitcoin/pull/27150#discussion_r1119383151)
re: https://github.com/bitcoin/bitcoin/pull/27150#discussion_r1118857439

Thanks! Should be fixed now

💬 furszy commented on pull request "bumpfee: Allow the user to choose which output is change":
(https://github.com/bitcoin/bitcoin/pull/26467#discussion_r1119397491)
In a902eaf5:
While you are here, could add the missing doc for the `@param[in] outputs` arg.

💬 furszy commented on pull request "bumpfee: Allow the user to choose which output is change":
(https://github.com/bitcoin/bitcoin/pull/26467#discussion_r1119402395)
In https://github.com/bitcoin/bitcoin/commit/a902eaf5be4a83dff37f28a988e5f3b2425551c0:

Could write it prettier:
```suggestion
if (reduce_output.has_value() ? reduce_output.value() == i : OutputIsChange(wallet, output)) {
```

📝 theStack opened a pull request: "test: add coverage for sigop limit policy (`-bytespersigop` setting)"
(https://github.com/bitcoin/bitcoin/pull/27171)
This PR adds missing test coverage for the `-bytespersigop` option, which determines how pre-taproot signature operations (OP_CHECKSIG{VERIFY}, OP_CHECKMULTIGSIG{VERIFY}) affect fee handling calculations. The setting was introduced in PR #7081 for mitigating the [sigop spam attack](https://bitcointalk.org/index.php?topic=1166928.0); the initial implementation rejected txs exceeding the limit, but was changed in #8365 later to account for higher sizes in the mempool (i.e. exceeding the sigop limi
...

💬 pablomartin4btc commented on pull request "Mask values on Transactions View":
(https://github.com/bitcoin-core/gui/pull/708#discussion_r1119564370)
Thanks @furszy, I took your advice on the decoupling the `dataChanged` signal and I've refactored the `updateDisplayUnit` `Q_SLOT` accordingly. As I was still thinking as you said, the call to `updateDisplayUnit` it's kind of a hack, so I've managed to add the connect on the `WalletView constructor` to trigger the `refreshAmounts` on the `setPrivacy`.

💬 koga73 commented on issue "CGminer can't connect over RPC on 0.20.0":
(https://github.com/bitcoin/bitcoin/issues/19182#issuecomment-1447573050)
> after starting ckpool, on fresh install of bitcoind and ready synced with txindex=1 I got this lines on bitcoind, and ckpool tries and tries this :
>
> `[2021-01-19 08:47:53.442] CRITICAL: No bitcoinds active! [2021-01-19 08:47:55.446] JSON failed to decode GBT 0000000000000000000048840346957d513da4037b325689baa97f5acecabd90 0000000000000000000da8a10000000000000000000000000000000000000000 536870912 1611042475 170da8a1 (null) with errno 11: Resource temporarily unavailable [2021-01-19 08:47:
...

💬 TheCharlatan commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#issuecomment-1447650893)
Updated b640db3c5fbe49af58dbc9087a96cbb493fa9574 -> b640db3c5fbe49af58dbc9087a96cbb493fa9574 ([removeBlockstorageArgs_2](https://github.com/TheCharlatan/bitcoin/tree/removeBlockstorageArgs_2) -> [removeBlockstorageArgs_3](https://github.com/TheCharlatan/bitcoin/tree/removeBlockstorageArgs_3), [compare](https://github.com/TheCharlatan/bitcoin/compare/removeBlockstorageArgs_2..removeBlockstorageArgs_3)) with suggested typo fix.

💬 martinus commented on pull request "tracing: lock contention analysis":
(https://github.com/bitcoin/bitcoin/pull/25081#issuecomment-1447740069)
Hi, I lost interest / don't have time for this unfortunately. You can close it and mark it as up-for-grabs.

✅ fanquake closed a pull request: "tracing: lock contention analysis"
(https://github.com/bitcoin/bitcoin/pull/25081)

💬 fanquake commented on pull request "refactor: Stop using gArgs global in system.cpp":
(https://github.com/bitcoin/bitcoin/pull/27170#issuecomment-1447846541)
Concept ACK