👍 dergoegge approved a pull request: "Several randomness improvements"
(https://github.com/bitcoin/bitcoin/pull/29625#pullrequestreview-2066132059)
Code review ACK 636e4864516782b7e6e33be2b2ec7743afaae8ba

💬 maflcko commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1606723428)
nit: Now that this is merged, it could say "in 27.0 and prior releases." Otherwise, on 29.x it will read as-if 28.0 had it missing.

💬 maflcko commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1606724711)
nit instead of duplicating the section from `doc/JSON-RPC-interface.md` here, it seems better to link/refer to it. Otherwise, if the section is updated, this may go stale or must be updated at the same time.

💬 josibake commented on pull request "refactor: Model the bech32 charlimit as an Enum":
(https://github.com/bitcoin/bitcoin/pull/30047#issuecomment-2120390457)
@theuni https://github.com/bitcoin/bitcoin/pull/30047#discussion_r1597108664 has been resolved by https://github.com/bitcoin/bitcoin/pull/30047/commits/d196442c015c4cbf490751a650ecb3aa29321442, if you have time for a second look/reACK

🤔 furszy reviewed a pull request: "assumeutxo, rpc: Improve EOF error when reading snapshot metadata in loadtxoutset"
(https://github.com/bitcoin/bitcoin/pull/28670#pullrequestreview-2066175552)
utACK 4a2df05e

💬 furszy commented on pull request "assumeutxo, rpc: Improve EOF error when reading snapshot metadata in loadtxoutset":
(https://github.com/bitcoin/bitcoin/pull/28670#discussion_r1606753156)
nit: should use `fs::PathToString(path)`

💬 maflcko commented on pull request "assumeutxo, rpc: Improve EOF error when reading snapshot metadata in loadtxoutset":
(https://github.com/bitcoin/bitcoin/pull/28670#discussion_r1606762456)
> PathToString

That is only for internal use. However, the result here will be returned on RPC, so the dev-notes apply:

```
- Use `fs::path::u8string()`/`fs::path::utf8string()` and `fs::u8path()` functions when converting path
to JSON strings, not `fs::PathToString` and `fs::PathFromString`

- *Rationale*: JSON strings are Unicode strings, not byte strings, and
RFC8259 requires JSON to be encoded as UTF-8.

🤔 furszy reviewed a pull request: "validation: improve performance of CheckBlockIndex"
(https://github.com/bitcoin/bitcoin/pull/28339#pullrequestreview-2066200941)
ACK 5bc2077e8f592442b089affdf0b5795fbc053bb8

💬 ajtowns commented on pull request "test: create assert_not_equal util":
(https://github.com/bitcoin/bitcoin/pull/29500#discussion_r1606763359)
`assert_equal` is helpful because when `a == 3` fails, it can be helpful to know what the value of `a` actually is without having to add debug code and rerun. If `a != 3` fails, though, you already know that `a == 3`, so this doesn't seem very helpful.

There are cases where you're comparing `a != b` and may not be sure which ones getting the wrong value where this could perhaps be helpful, but those seem pretty rare.

Concept -0 for me.

💬 ajtowns commented on pull request "test: create assert_not_equal util":
(https://github.com/bitcoin/bitcoin/pull/29500#discussion_r1606772606)
This error message doesn't make sense when `*args` is non-empty: if you say `assert_not_equal(a, 1, 2)` and `a` is 1 or 2, it will report `1 == 1 == 2` or `2 == 1 == 2`.

💬 marcofleon commented on pull request "fuzz: add more coverage for `ScriptPubKeyMan`":
(https://github.com/bitcoin/bitcoin/pull/30134#issuecomment-2120491389)
Got it, thanks @brunoerg. I'll redo and get back to you then. I also realized I was looking at the wrong line in the coverage report. I should probably be looking at `wallet/scriptpubkeyman.cpp` not `wallet/test/fuzz/scriptpubkeyman.cpp`.

💬 maflcko commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1606802910)
Looks like this is unused?

👍 willcl-ark approved a pull request: "net: make the list of known message types a compile time constant"
(https://github.com/bitcoin/bitcoin/pull/29421#pullrequestreview-2066265255)
ACK b3efb486732f3caf8b8a8e9d744e6d20ae4255ef

Based on range diff from previous ACK, all looks good!
`range-diff 2e312ea...b3efb48`

💬 stickies-v commented on pull request "indexes: Don't wipe indexes again when continuing a prior reindex":
(https://github.com/bitcoin/bitcoin/pull/30132#discussion_r1606818901)
As discussed offline, I think this change means we're ignoring the GUI-based reindex from https://github.com/bitcoin/bitcoin/pull/30132/files#diff-b1e19192258d83199d8adaa5ac31f067af98f63554bfdd679bd8e8073815e69dR1609, both for these optional indices but also for the block index

💬 maflcko commented on pull request "wallet: Allow user to navigate options while encrypting at creation":
(https://github.com/bitcoin-core/gui/pull/722#issuecomment-2120525642)
I don't understand this change. It looks like a "Back" button is added to the warning pop-up, but it will only take you back to enter a different password, not to disable the password selection in the create wallet dialog. Wouldn't it be better to add the "Back" button to the password dialog?

Also, wouldn't it be better if the text from the two warning pop-ups was added to the top of the password dialog? Otherwise, it seems odd to first ask for a password from the user, then print two warning
...

💬 maflcko commented on pull request "wallet: Allow user to navigate options while encrypting at creation":
(https://github.com/bitcoin-core/gui/pull/722#issuecomment-2120529217)
Also, this doesn't need release notes, does it?

💬 TheCharlatan commented on pull request "indexes: Don't wipe indexes again when continuing a prior reindex":
(https://github.com/bitcoin/bitcoin/pull/30132#issuecomment-2120534220)
Updated 991f50acad6809c83c0c44cf3c55fb9d2c0107e3 -> 9de8b263dabd6dd2f86f1f0253c6ee3fac7a4407 ([preserveIndexOnRestart_0](https://github.com/TheCharlatan/bitcoin/tree/preserveIndexOnRestart_0) -> [preserveIndexOnRestart_1](https://github.com/TheCharlatan/bitcoin/tree/preserveIndexOnRestart_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/preserveIndexOnRestart_0..preserveIndexOnRestart_1))

* Addressed @stickies-v's [comment](https://github.com/bitcoin/bitcoin/pull/30132#discussion
...

💬 instagibbs commented on pull request "refactor prep for package rbf":
(https://github.com/bitcoin/bitcoin/pull/30072#discussion_r1606835018)
done

💬 instagibbs commented on pull request "refactor prep for package rbf":
(https://github.com/bitcoin/bitcoin/pull/30072#discussion_r1606836006)
added an explicit `m_allow_sibling_eviction` ATMPArg and had it being true imply `m_allow_replacements`.

👍 willcl-ark approved a pull request: "chainparams: Add achow101 DNS seeder"
(https://github.com/bitcoin/bitcoin/pull/30007#pullrequestreview-2066317042)
ACK ee218aa9a9eaac53030c31b099b4afe354197ba7

I checked that these seeds return diverse and (mainly) active node addresses (although not in a scripted way, but by `addnode`-ing a random selection of each).

I also checked DNSSEC setup using Verisign's DNSSEC [debugger](https://dnssec-debugger.verisignlabs.com) tool for each of them, which they all passed.