💬 Xekyo commented on pull request "Bump unconfirmed ancestor transactions to target feerate":
(https://github.com/bitcoin/bitcoin/pull/26152#discussion_r1254760855)
Did about 40M executions, and no issues! Gonna calculate the Waste score below here as discussed.

💬 achow101 commented on pull request "kernel: Rm ShutdownRequested and AbortNode from validation code.":
(https://github.com/bitcoin/bitcoin/pull/27861#issuecomment-1624141412)
light ACK 6eb33bd0c21b3e075fbab596351cacafdc947472

I'm not particularly experienced with this area of the code, so I can't really comment on approach taken here. However mechanically, the code looks find and correct to me and seems to achieve the stated objectives.

💬 jonatack commented on pull request "test: remove race in the user-agent reception check":
(https://github.com/bitcoin/bitcoin/pull/27986#issuecomment-1624158995)
re-ACK 20b49460b35268db59f7efcb02736b0e31191a74

📝 furszy opened a pull request: "wallet: address book migration bug fixes"
(https://github.com/bitcoin/bitcoin/pull/28038)
Addressing two specific bugs encountered during the wallet migration process, related to the address book, and improves the test coverage for it.

Bug 1: Non-Cloning of External 'Send' Records
The external 'send' records were not being correctly cloned to all wallets.

Bug 2: Persistence of Empty Labels
As address book entries without associated db label records can be treated as change (the `label` field inside the `CAddressBookData` class is optional, `nullopt` labels make `CAddressBookD
...

💬 TheCharlatan commented on pull request "blockstorage: Return on fatal flush errors":
(https://github.com/bitcoin/bitcoin/pull/27866#discussion_r1254840912)
> Maybe your goal is to enhance libbitcoinkernel applications ability to handle and report errors, and my goal is to improve clarify of validation code and find to potential bugs in it.

I think that hits it on the nail. In any case I updated the PR description with much of the body of the commit message to make it clearer what I am trying to achieve here and why I'd consider it an improvement.

📝 theuni opened a pull request: "wallet: don't include bdb files from our headers"
(https://github.com/bitcoin/bitcoin/pull/28039)
Only `#include` upstream bdb headers from our cpp files.

It's generally good practice to avoid including 3rd party deps in headers as otherwise they tend to sneak into new compilation units. IMO this makes for a nice cleanup.

There's a good bit of code movement here, but each commit is small and _should_ be obviously correct.

Note: in the future, the buildsystem can add the bdb include path for `bdb.cpp` and `salvage.cpp` only, rather than all wallet sources.

📝 theuni opened a pull request: "wallet: sqlite: don't include sqlite files from our headers"
(https://github.com/bitcoin/bitcoin/pull/28040)
Only `#include` upstream sqlite headers from our cpp files.

Like #28039 but simpler :)

💬 Xekyo commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#issuecomment-1624254517)
Concept ACK

📝 1proprogrammerchant opened a pull request: "Merge pull request #1 from bitcoin/master"
(https://github.com/bitcoin/bitcoin/pull/28041)
pull_1

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests t
...

✅ hebasto closed a pull request: "Merge pull request #1 from bitcoin/master"
(https://github.com/bitcoin/bitcoin/pull/28041)

📝 hebasto locked a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/28041)
pull_1

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests t
...

💬 mzumsande commented on issue "EstimateMedianVal returns higher fee for higher confTarget":
(https://github.com/bitcoin/bitcoin/issues/20725#issuecomment-1624279055)
> @ismaelsadeeq Can you try with the commit specified in OP as well? If it fails, can you try bisecting?

Maybe not necessary:
There are newer crashes reported in #23165 (which seems to be the same problem). The seed `7288975409214300634` from https://api.cirrus-ci.com/v1/task/6110951150190592/logs/ci.log leads to a crash on current master (the test passes with #21161 rebased on master).

💬 achow101 commented on pull request "wallet: address book migration bug fixes":
(https://github.com/bitcoin/bitcoin/pull/28038#issuecomment-1624292395)
ACK 7ecc29a0b7a23d8f5d3c1e6a0dad29b3ad839eb9

Nice catch!

🚀 ryanofsky merged a pull request: "kernel: Rm ShutdownRequested and AbortNode from validation code."
(https://github.com/bitcoin/bitcoin/pull/27861)

💬 achow101 commented on pull request "wallet: sqlite: don't include sqlite files from our headers":
(https://github.com/bitcoin/bitcoin/pull/28040#issuecomment-1624315734)
ACK bea9fc2600635020fd28ec7a6613c92a6f349a86

💬 furszy commented on pull request "index: make startup more efficient":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1254925898)
Sure, applied. Thanks!

💬 sipa commented on pull request "Add feerate histogram to getmempoolinfo":
(https://github.com/bitcoin/bitcoin/pull/21422#issuecomment-1624321895)
@santochibtc "Feerate histogram" here is a bytes vs fee diagram, not a #tx vs fee diagram.

🤔 furszy reviewed a pull request: "index: make startup more efficient"
(https://github.com/bitcoin/bitcoin/pull/27607#pullrequestreview-1517325528)
Updated per feedback, thanks. [Small diff](https://github.com/bitcoin/bitcoin/compare/30b2511f39d32e29f9f05859aa8a97b84c22376b..988f3f692acabf0eedfed38e4704fe355f995e72).

💬 furszy commented on pull request "index: make startup more efficient":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1254926452)
done

💬 furszy commented on pull request "index: make startup more efficient":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1254926965)
removed.

💬 achow101 commented on pull request "wallet: don't include bdb files from our headers":
(https://github.com/bitcoin/bitcoin/pull/28039#issuecomment-1624325625)
ACK 2d09d0f50408f9c522b9efa4f386072502c7b3d1

Changes are primarily moves and appear otherwise obviously correct.