✅ alex-lt-kong closed an issue: "Bitcoind crashed, how to debug"
(https://github.com/bitcoin/bitcoin/issues/27156)

💬 jonatack commented on pull request "Handle CJDNS from LookupSubNet()":
(https://github.com/bitcoin/bitcoin/pull/27071#issuecomment-1443962823)
> I will try to mimic the proxy in order to address the latter.

In case you plan to do that in a follow-up:

re-ACK c2a86ba6674008996552830877d66a1bf888fad1 per `git range-diff be2e748 8991ed2c6e c2a86ba667`

💬 fanquake commented on issue "contrib/install_db4.sh script fails on OpenBSD 7.2 (RPi 3) (error: Unable to find a mutex implementation)":
(https://github.com/bitcoin/bitcoin/issues/26860#issuecomment-1443978351)
@m-kubik were you compiling the master branch? We have since removed this script, in favour of using depends. You can see the updated OpenBSD build instructions here: https://github.com/bitcoin/bitcoin/blob/master/doc/build-openbsd.md.

👍 pinheadmz approved a pull request: "bumpfee: Allow the user to choose which output is change"
(https://github.com/bitcoin/bitcoin/pull/26467)
ACK 4167843e0729994954317e2a4ac8a96a453bad79

reviewed code, ran tests. Played with the feature on regtest and in particular using `reduce_output` to intentionally reduce the recipient output instead of the original change output added automatically by `sendtoaddress`. I don't think is explicitly covered in the test but could be. One other quesiton about the test below but otherwise LGTM

<details><summary>Show Signature</summary>

```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

A
...

💬 pinheadmz commented on pull request "bumpfee: Allow the user to choose which output is change":
(https://github.com/bitcoin/bitcoin/pull/26467#discussion_r1117235969)
Will `new_change_pos == change_pos` always here? Or are the outputs of the replacement re-randomized?

💬 fanquake commented on pull request "Fix various libbitcoinkernel DLL build problems":
(https://github.com/bitcoin/bitcoin/pull/27146#issuecomment-1444014647)
This also fixes #19772.

👍 stickies-v approved a pull request: "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`"
(https://github.com/bitcoin/bitcoin/pull/26261)
re-ACK [0566f17](https://github.com/bitcoin/bitcoin/commit/0566f171771e37dca02adc328e0385edbb412073)

I verified that the only changes since https://github.com/bitcoin/bitcoin/commit/84b3ab40d5480862f3f7e08c4ad1f9124990ba74 are:
- resolving the merge conflict from https://github.com/bitcoin/bitcoin/pull/25619
- in `ProcessUpnp()`, [replace](https://github.com/bitcoin/bitcoin/compare/4b0c5ca1315cee243280cbaf2afea6fe5d26d9db..ece6012cf18e096e053e45ebbb31898c6fb7871a?diff=unified#diff-641555124
...

💬 john-moffett commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1117323095)
`m_untouched_memory_end` now called `m_available_memory_end`

👍 john-moffett approved a pull request: "Add pool based memory resource"
(https://github.com/bitcoin/bitcoin/pull/25325)
ACK d87cb99bb37637e26a9e00b9f7de4bc6f44cb79d

Tested and code reviewed. Very nice improvement.

Left tiny nits in documentation, but please ignore unless updating for other reasons.

💬 john-moffett commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1117328292)
Extra "is" after "memory".

👍 pinheadmz approved a pull request: "Handle invalid hex encoding in ParseHex"
(https://github.com/bitcoin/bitcoin/pull/25227)
ACK fad0c892c34c30cf8f50e832425210e24d45837e

Built and ran all tests, reviewed code. I also inserted a few other test cases but nothing broke so, take em or leave em:

` Ff aA ` (mixed case / multiple spaces, succeeds)
` F F ` (spaces between nibbles, fails as expected)


<details><summary>Show Signature</summary>

```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK fad0c892c34c30cf8f50e832425210e24d45837e
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiE
...

👋 ryanofsky's pull request is ready for review: "Deduplicate bitcoind and bitcoin-qt init code"
(https://github.com/bitcoin/bitcoin/pull/27150)

💬 ryanofsky commented on pull request "Deduplicate bitcoind and bitcoin-qt init code":
(https://github.com/bitcoin/bitcoin/pull/27150#issuecomment-1444334598)
Marking ready for review since I was able to merge the duplicate InitSettings functions after #27073.

---

Rebased eeff271a693baa6fb013b6b007306d0c288c811a -> e248926c73ce6f707343501fb42b31090fed202b ([`pr/oneconfig.1`](https://github.com/ryanofsky/bitcoin/commits/pr/oneconfig.1) -> [`pr/oneconfig.2`](https://github.com/ryanofsky/bitcoin/commits/pr/oneconfig.2), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/oneconfig.1-rebase..pr/oneconfig.2)) after #27073 merge

📝 mzumsande opened a pull request: " 25574 followups (VerifyDB error handling)"
(https://github.com/bitcoin/bitcoin/pull/27157)
This addresses two outstanding comments by ryanofsky from #25574:
* return `ChainstateLoadStatus::INTERRUPTED` if verification was interrupted, avoiding a misleading log entry in `init`:
https://github.com/bitcoin/bitcoin/pull/25574#discussion_r1110050930
* add documentation for `require_full_verification` https://github.com/bitcoin/bitcoin/pull/25574#discussion_r1110031541

💬 jamesob commented on pull request "assumeutxo: keep cache when flushing snapshot (#17487 followup)":
(https://github.com/bitcoin/bitcoin/pull/27008#discussion_r1117597926)
`AbortNode()` is used elsewhere in the function that this function is called from, although I did change another reference that you pointed out what inconsistent.

💬 jonatack commented on pull request "bench: update logging benchmarks":
(https://github.com/bitcoin/bitcoin/pull/26957#discussion_r1117702466)
Thanks @kouloumos -- updated to "Disable writing the log to a file, as used for unit tests and fuzzing in `MakeNoLogFileContext`."

💬 jonatack commented on pull request "bench: update logging benchmarks":
(https://github.com/bitcoin/bitcoin/pull/26957#discussion_r1117704951)
Thanks, @LarryRuane! Added a commit by you at the start of the changes here to fix the order dependency.

💬 jonatack commented on pull request "bench: update logging benchmarks":
(https://github.com/bitcoin/bitcoin/pull/26957#discussion_r1117709540)
Great points. Fixed the -debug, ensured that we do it where needed, and added this comment at the top of the benchmarks, along with co-author to both of you in this commit (LMK if a name/email is incorrect).

```diff
+// The test framework currently enables all categories by default, but in case
+// that changes, we set -debug=category in the benchmarks below when we expect a
+// category to be logged.
```

💬 jonatack commented on pull request "bench: update logging benchmarks":
(https://github.com/bitcoin/bitcoin/pull/26957#issuecomment-1444580916)
Thank you @kouloumos and @LarryRuane for the excellent reviewing. Updated per `git diff 9d708db a0e092e`

💬 rserranon commented on pull request "wallet: finish addressbook encapsulation":
(https://github.com/bitcoin/bitcoin/pull/26836#discussion_r1117586765)
Nit: Shouldn't be better to use the capital B in `LoadAddrBookEntryLablel` and `LoadAddrBookEntryPurpose` as in `GetAddrBookSize` and `ListAddrBookAddreses` ?

💬 rserranon commented on pull request "wallet: finish addressbook encapsulation":
(https://github.com/bitcoin/bitcoin/pull/26836#discussion_r1117807285)
Nice optimization!