💬 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

💬 TheCharlatan commented on pull request "Deduplicate bitcoind and bitcoin-qt init code":
(https://github.com/bitcoin/bitcoin/pull/27150#discussion_r1119622390)
nit: s/to whether/whether to/

💬 TheCharlatan commented on pull request "Deduplicate bitcoind and bitcoin-qt init code":
(https://github.com/bitcoin/bitcoin/pull/27150#discussion_r1119655981)
More a comment in case anybody else stumbles over this than a change request, but I'm surprised this is done in `bitcoind` and not in `bitcoin-qt`. Indeed I can start bitcoin-qt with any number of positional arguments, e.g. `./qt/bitcoin-qt h j k`, but not so `bitcoind`. The historical [context](https://github.com/bitcoin/bitcoin/pull/10447) is that this is a leftover from when `bitcoind` was also used as a command line client.

👍 darosior approved a pull request: "Make miniscript_{stable,smart} fuzzers avoid too large scripts"
(https://github.com/bitcoin/bitcoin/pull/27165)
ACK b09b7a1f94885feed5d44a3858543536e3659287

Elegant fix and improvement. Just two non-blocking comments.

💬 darosior commented on pull request "Make miniscript_{stable,smart} fuzzers avoid too large scripts":
(https://github.com/bitcoin/bitcoin/pull/27165#discussion_r1119807797)
Maybe worth pointing to the explanatory comment in `Parse()`? I think the "borrowing" logic could be a little unclear to a reader with less context.

💬 darosior commented on pull request "Make miniscript_{stable,smart} fuzzers avoid too large scripts":
(https://github.com/bitcoin/bitcoin/pull/27165#discussion_r1119792156)
You can directly pass the number of sub fragments to `ComputeScriptLen` instead of passing `0` and then adding it.
```patch
diff --git a/src/test/fuzz/miniscript.cpp b/src/test/fuzz/miniscript.cpp
index 90d752b898..490fac18ce 100644
--- a/src/test/fuzz/miniscript.cpp
+++ b/src/test/fuzz/miniscript.cpp
@@ -781,7 +781,7 @@ NodeRef GenNode(F ConsumeNode, Type root_type, bool strict_valid = false) {
auto node_info = ConsumeNode(type_needed);
if (!node_info) return {
...

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

📝 fanquake opened a pull request: "guix: switch to some `minimal` versions of packages in our manifest"
(https://github.com/bitcoin/bitcoin/pull/27172)
Minimal versions of the same packages, that should still be sufficient for our use:

> (define-public bash-minimal
;; A stripped-down Bash for non-interactive use.

> (define-public coreutils-minimal
;; Coreutils without its optional dependencies.