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

💬 mzumsande commented on issue "qa: Intermittent failure in `feature_fee_estimation.py`":
(https://github.com/bitcoin/bitcoin/issues/23165#issuecomment-1624334379)
Just noting here too (I also posted in #20725) that failures can be reproduced by using the PRNG seed from the log (`7288975409214300634` in the most recent one) and running with `--randomseed`. So it would be good to include that seed in future reports - the earlier ones are no longer accessible from cirrus.

💬 jonatack commented on pull request "p2p: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1254974082)
Thanks, makes sense, and I see that your change makes the conditional here the same as in `ProcessMessage::TX` followed by the same comment.

```cpp
if (peer->m_wtxid_relay && txid != wtxid) {
// Insert txid into m_tx_inventory_known_filter, even for
// wtxidrelay peers. This prevents re-adding of
// unconfirmed parents to the recently_announced
// filter, when a child tx is requested. See
// ProcessGetData().

...

🤔 ismaelsadeeq reviewed a pull request: "Fee estimation: extend bucket ranges consistently"
(https://github.com/bitcoin/bitcoin/pull/21161#pullrequestreview-1517440130)
The issue this is solving occurs intermittently.
I tested the test failed on recent master c325f0f with [seed](https://github.com/bitcoin/bitcoin/issues/20725#issuecomment-1624279055) from @mzumsande and passed on this PR rebased on master head c325f0f.

Did not finish understanding the PR yet

🤔 jonatack reviewed a pull request: "p2p: Drop m_recently_announced_invs bloom filter"
(https://github.com/bitcoin/bitcoin/pull/27675#pullrequestreview-1517398008)
ACK 172fa815e08b54c61b24d552f065df15046aef3c

With this simplification, test coverage for the new logic might be feasible?

💬 jonatack commented on pull request "p2p: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1254977700)
caf32d2 mentioning `m_sequence_number`'s new responsibility for p2p tx relay might be helpful (or alternatively, drop "external")

```suggestion
// In-memory counter for internal (p2p tx relay) and external (zmq) mempool tracking purposes.
```

💬 jonatack commented on pull request "p2p: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1254981645)
c219b68 nit, could add `[[nodiscard]]` while touching this

💬 jonatack commented on pull request "p2p: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1254980474)
caf32d2 nit, could be ` [[nodiscard]]`

👍 theStack approved a pull request: "Add support for RFC8439 variant of ChaCha20"
(https://github.com/bitcoin/bitcoin/pull/27985#pullrequestreview-1517469616)
Code-review ACK 200e1a292b277d6c32dbd6081d38b715cbc079cb

Verified that the implementation matches the RFC8439 specification (https://datatracker.ietf.org/doc/html/rfc8439#section-2.3) and checked that the test vectors match the ones from the linked sources (https://tools.ietf.org/html/draft-agl-tls-chacha20poly1305-04#section-7 and https://datatracker.ietf.org/doc/html/rfc8439#page-30).

Note for other reviewers (that get as easily confused as me): we don't store the constants in the `inp
...

💬 achow101 commented on pull request "Add support for RFC8439 variant of ChaCha20":
(https://github.com/bitcoin/bitcoin/pull/27985#issuecomment-1624433876)
Is the current ChaCha20 implementation used for anything that needs backwards compatibility?