💬 MarcoFalke commented on issue ""error reading from database. shutting down"":
(https://github.com/bitcoin/bitcoin/issues/22426#issuecomment-1460887481)
We get a few data corruption reports regularly, however, no developer could reproduce them yet. So fixing them is hard and for debugging one can only throw blind guesses.

My next guess would be to check if Anti-Virus Software was involved.

If you can reliably reproduce this, it can help us if you shared the steps. Though, your Windows installation might largely differ from a fresh install of Windows. So if you can reproduce on a fresh install of Windows, that'd be ideal for finding the bug
...

💬 mzumsande commented on pull request "gui: use the stored CSubNet entry when unbanning":
(https://github.com/bitcoin-core/gui/pull/717#discussion_r1130039145)
Not a major issue, but this looks like it's designed to supports unbanning multiple nodes with one click, but the GUI doesn't allow me to select multiple entries when I try (unlike in the peertable where banning multiple nodes with once click works).

💬 mzumsande commented on pull request "gui: use the stored CSubNet entry when unbanning":
(https://github.com/bitcoin-core/gui/pull/717#discussion_r1130020981)
If this comment is fixed anyway (it was copied from `peertablemodel.h`), could also replace "getpeerinfo" with "listbanned".

💬 ryanofsky commented on pull request "refactor / kernel: Move non-gArgs chainparams functionality to kernel":
(https://github.com/bitcoin/bitcoin/pull/26177#discussion_r1130015697)
In commit "Decouple RegTestChainParams from ArgsManager" (66b17894185e0cda3ea7cbec77cc771a7ba41d56)

Probably better to merge the option instead of overwriting it:

```c++
if (auto value = args.GetBoolArg("-fastprune")) options.fastprune = *value;
```

This would avoid needing to hardcode the default `false` value here, and would be more more consistent with `version_bits_parameters` and `activation_heights` handling below which keep preexisting values if arguments are unset.

Alternat
...

💬 ryanofsky commented on pull request "refactor / kernel: Move non-gArgs chainparams functionality to kernel":
(https://github.com/bitcoin/bitcoin/pull/26177#discussion_r1130052484)
re: https://github.com/bitcoin/bitcoin/pull/26177#discussion_r1126829031

> Thanks, updated. [Diff](https://github.com/TheCharlatan/bitcoin/compare/tc/2022-09-libbitcoinkernel-chainparams-args_1..tc/2022-09-libbitcoinkernel-chainparams-args_2)

This looks the same as before. I was suggesting dropping the `chainparams_ptr` variable and just writing `*chainparams` directly below.

💬 ryanofsky commented on pull request "refactor / kernel: Move non-gArgs chainparams functionality to kernel":
(https://github.com/bitcoin/bitcoin/pull/26177#discussion_r1130039156)
In commit "split non/kernel chainparamsbase" (341be7fa680e9dcdbaa46db0194833b5d0229394)

I still don't think it makes sense to have `CBaseChainParams` in the kernel. Like the comment above says, the point of the `CBaseChainParams` class and the reason it is separate from `CChainParams` is that it is shared between bitcoin-cli and bitcoind. I think of `CBaseChainParams` as client parameters, and `CChainParams` as server parameters. Practically speaking, `bitcoin-cli` or other clients that needs
...

💬 pablomartin4btc commented on pull request "cli: add validation to cli side commands besides when it's used with -rpcwallet":
(https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1130071627)
done

💬 pablomartin4btc commented on pull request "cli: add validation to cli side commands besides when it's used with -rpcwallet":
(https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1130071906)
done

💬 pablomartin4btc commented on pull request "cli: add validation to cli side commands besides when it's used with -rpcwallet":
(https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1130104615)
On this one, when I raise the error or when I have to pass the strings for the insertion into the chain, it doesn't look right to pass twice the same variable (e.g.: `INVALID_CLI_COMMAND_ARGUMENT.format(rpcwallet3, rpcwallet3, "-generate"`).

Would you accept:

`INVALID_CLI_COMMAND_ARGUMENT = 'Error: invalid cli-command argument. If \"{}\" is a valid option try passing it before the \"{}\" command.'`

💬 pablomartin4btc commented on pull request "cli: add validation to cli side commands besides when it's used with -rpcwallet":
(https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1130114475)
fixed

💬 achow101 commented on pull request "Switch hardened derivation marker to h (in normalized descriptors and new wallets)":
(https://github.com/bitcoin/bitcoin/pull/26076#issuecomment-1460962371)
reACK 4677f1923c9509025de0e77892638d5013f044bf

📝 sipa opened a pull request: "Update src/secp256k1 subtree to upstream release v0.3.0"
(https://github.com/bitcoin/bitcoin/pull/27230)
This updates the libsecp256k1 subtree to v0.3.0. I don't believe there are code changes that are particularly important to Bitcoin Core, apart from the added CMake build system support.

💬 TheCharlatan commented on pull request "refactor / kernel: Move non-gArgs chainparams functionality to kernel":
(https://github.com/bitcoin/bitcoin/pull/26177#issuecomment-1460991593)
Thanks for the review,

Updated c44983faa02b4845cd7605abe99c332980a07af1 -> a7afa98ca38fd66fc69b77b95a6a57d50207911b ([tc/2022-09-libbitcoinkernel-chainparams-args_4](https://github.com/TheCharlatan/bitcoin/commits/tc/2022-09-libbitcoinkernel-chainparams-args_4) -> [tc/2022-09-libbitcoinkernel-chainparams-args_5](https://github.com/TheCharlatan/bitcoin/commits/tc/2022-09-libbitcoinkernel-chainparams-args_5), [compare](https://github.com/TheCharlatan/bitcoin/compare/tc/2022-09-libbitcoinkernel-
...

💬 TheCharlatan commented on pull request "refactor / kernel: Move non-gArgs chainparams functionality to kernel":
(https://github.com/bitcoin/bitcoin/pull/26177#discussion_r1130159494)
This makes sense, I wasn't connecting the fact that the util library includes `chainparamsbase` anyway with segregating the kernel code. I dropped the commit.

💬 TheCharlatan commented on pull request "refactor / kernel: Move non-gArgs chainparams functionality to kernel":
(https://github.com/bitcoin/bitcoin/pull/26177#discussion_r1130159566)
Merging with the option seems more in tune with how the options are handled in other places in the code, so I went with that.

💬 TheCharlatan commented on pull request "refactor / kernel: Move non-gArgs chainparams functionality to kernel":
(https://github.com/bitcoin/bitcoin/pull/26177#discussion_r1130159703)
I dropped this by mistake. Included now.

💬 mzumsande commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1461041818)
> I think it goes like this: (...)
>
> I think this is enough to ditch the idea of random-chance `Select()`. Also, maybe it would not be so much more simpler than this PR.

I agree with that conclusion, but I think you can't just add the probabilities like that. I think the correct formula is
$$p={k! S(n,k) \over k^n}$$
where k is the number of networks, $n=8$ the number of outbounds and $S(n,k)$ the Stirling number of the second kind, see [here](https://stats.stackexchange.com/question
...

💬 Xekyo commented on issue "Small unspent can get removed from OutputGroup for being uneconomical probably leading to later partial spending":
(https://github.com/bitcoin/bitcoin/issues/20287#issuecomment-1461138806)
I agree, that we still have this issue with potentially making a partial spend, when we have an uneconomical UTXO that’s part of a group. I think there may be a confusion here between dust and uneconomical UTXOs, though: in Bitcoin Core, dust pertains to an output whose value is less than `dustrelayfee × (input_weight + output_weight)` (where dustrelayfee is a feerate) with the default `dustrelayfee` being 3 ṩ/vB. So, dust outputs will always be dust (unless the dustrelayfee is changed). We only
...

📝 jonatack opened a pull request: "rpc: fix logging RPC when "none" values are passed, add test coverage, improve docs"
(https://github.com/bitcoin/bitcoin/pull/27231)
The logging RPC doesn't seem to behave as described in its help when `0` or `none` are passed, including not recognizing `none`:

```
$ ./src/bitcoin-cli logging '["none"]'
error code: -8
error message:
unknown logging category none
```

This pull adds unit and functional test coverage, the latter of which fails on master and passes on this branch, fixes the behavior as described in the help, and improves the documentation and examples.

💬 vostrnad commented on pull request "BIP324: Enable v2 P2P encrypted transport":
(https://github.com/bitcoin/bitcoin/pull/24545#issuecomment-1461152689)
After updating, node is stuck in a loop trying to connect to manually added nodes that haven't been updated yet, without downgrading to a v1 connection. It might be just fine, but I thought I'd mention it.