💬 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.

💬 instagibbs commented on pull request "refactor prep for package rbf":
(https://github.com/bitcoin/bitcoin/pull/30072#discussion_r1606840132)
@sr-gi I reverted the logic change here to reduce review surface, just shortening the comment here instead.

💬 maflcko commented on pull request "Lint: support running individual rust linters and improve subtree exclusion":
(https://github.com/bitcoin/bitcoin/pull/29965#issuecomment-2120556703)
Looks like the conflicting https://github.com/bitcoin/bitcoin/pull/30034 is close(r) to merge, if you want to review it

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#issuecomment-2120592803)
rebased on latest #30072

💬 maflcko commented on pull request "rpc: Avoid getchaintxstats invalid results":
(https://github.com/bitcoin/bitcoin/pull/29720#issuecomment-2120602326)
@fjahr Thanks! I've pushed your test commit with small fixups.

💬 edilmedeiros commented on pull request "Update manpage descriptions":
(https://github.com/bitcoin/bitcoin/pull/29686#discussion_r1606890727)
I just realized that `bitcoin-util` does not have a `help` command like `bitcoin-cli`. Looks silly now as it has only one command, but if the tool is to increase, this seems to be a fairly easy improvement someone else can work on. In case there's another PR for that, let's try to make this more close to the `bitcoin-cli` message mentioning the help command only.

💬 maflcko commented on pull request "rpc: avoid copying into UniValue":
(https://github.com/bitcoin/bitcoin/pull/30115#issuecomment-2120649012)
Maybe rebase and re-bench after 75118a608fc22a57567743000d636bc1f969f748, which replaced some copies with moves as well?

💬 theuni commented on pull request "rpc: avoid copying into UniValue":
(https://github.com/bitcoin/bitcoin/pull/30115#issuecomment-2120681426)
> Maybe rebase and re-bench after [75118a6](https://github.com/bitcoin/bitcoin/commit/75118a608fc22a57567743000d636bc1f969f748), which replaced some copies with moves as well?

Ah, great, thanks for pointing me to that. I had pretty much this same commit locally to address the third point in this PR's description:
> - Refactoring functions to accept UniValues by value rather than by const reference

Though I think there are still other functions that need the same treatment.

💬 instagibbs commented on pull request "refactor prep for package rbf":
(https://github.com/bitcoin/bitcoin/pull/30072#issuecomment-2120685605)
win64 test failure is minisketch https://github.com/bitcoin/bitcoin/pull/30072/checks?check_run_id=25181774728

💬 ryanofsky commented on pull request "indexes: Don't wipe indexes again when continuing a prior reindex":
(https://github.com/bitcoin/bitcoin/pull/30132#issuecomment-2120741121)
> fixing a bug introduced in b47bd959207e82555f07e028cc2246943d32d4c3: "kernel: De-globalize fReindex".

Is this true? That commit, which was part of #29817, should not have changed any previous behavior

💬 edilmedeiros commented on pull request "Update manpage descriptions":
(https://github.com/bitcoin/bitcoin/pull/29686#discussion_r1606982074)
What about, instead of this, we just remove it altogether in favor of the `Commands` section that's already there? That way, it will follow the same pattern as `bitcoin-wallet`.

💬 edilmedeiros commented on pull request "Update manpage descriptions":
(https://github.com/bitcoin/bitcoin/pull/29686#discussion_r1606976568)
```suggestion
"The -named option allows specifying parameters using key=value instead of positional style.\n"
```

💬 edilmedeiros commented on pull request "Update manpage descriptions":
(https://github.com/bitcoin/bitcoin/pull/29686#discussion_r1606988664)
```suggestion
"It is suitable for desktop users who prefer a graphical over a command-line interface.\n\n"
```

💬 edilmedeiros commented on pull request "Update manpage descriptions":
(https://github.com/bitcoin/bitcoin/pull/29686#discussion_r1606986060)
This is not rendering nicely in the terminal:

```
Usage: bitcoin-tx [options] <hex-tx> [commands]
or: bitcoin-tx [options] -create [commands]
```

When it should be
```
Usage: bitcoin-tx [options] <hex-tx> [commands]
or: bitcoin-tx [options] -create [commands]
```

🤔 edilmedeiros reviewed a pull request: "Update manpage descriptions"
(https://github.com/bitcoin/bitcoin/pull/29686#pullrequestreview-2066570836)
Other than those comments, I compiled everything and ran the tests. Man pages seem fine too.

🤔 sr-gi reviewed a pull request: "test/BIP324: disconnection scenarios during v2 handshake"
(https://github.com/bitcoin/bitcoin/pull/29431#pullrequestreview-2066388767)
I like this approach much better, it is way easier to follow and reason about. I left some comments inline.

> because we don't send a version message to ensure that the disconnection happens in the v2 handshake phase. in your case (different from what we are testing here), we send the correct garbage terminator but because we don't send a version message - disconnection is still expected.

What is weird is that the test is expecting a specific log message, which seems to also match in this
...