💬 sipa commented on pull request "Add feerate histogram to getmempoolinfo":
(https://github.com/bitcoin/bitcoin/pull/21422#issuecomment-1611839519)
I think the discussion here is conflating two things:
* Problems with Bitcoin Core's fee estimation algorithm.
* The lack of a fee histogram RPC.

It's clear that the existing fee estimation algorithms make it inaccurate for many real use cases. There are good reasons for why it works the way it does, but that doesn't change the fact that the outcome is bad. We can certainly discuss what can be done about that. I've posted about one possible approach in #27995.

However, this PR is about s
...

💬 sipa commented on pull request "test: add python implementation of Elligator swift":
(https://github.com/bitcoin/bitcoin/pull/24005#issuecomment-1611844399)
@stratospher Want to address the comments here, and rebase? It'd be good to get rid of the merge commit in this PR's history.

🤔 john-moffett reviewed a pull request: "util: Don't derive secure_allocator from std::allocator"
(https://github.com/bitcoin/bitcoin/pull/27930#pullrequestreview-1503703565)
Concept ACK and existing code ACK. Thanks for catching this!

I agree that `zero_after_free_allocator` also ought to be modified. FWIW, here's what I did:

```diff
diff --git a/src/support/allocators/zeroafterfree.h b/src/support/allocators/zeroafterfree.h
index 2dc644c242a..35312e9704d 100644
--- a/src/support/allocators/zeroafterfree.h
+++ b/src/support/allocators/zeroafterfree.h
@@ -12,31 +12,46 @@
#include <vector>

template <typename T>
-struct zero_after_free_allocator : pu
...

📝 Brotcrunsher converted_to_draft a pull request: "DRAFT: Checking for multi/single-value types in UniValue."
(https://github.com/bitcoin/bitcoin/pull/27994)
Previously it was possible to call getValStr() on a VOBJ and VARR, which silently resulted in the return of an empty string. However, such a call would be most likely a bug. We are now throwing in case of such types.

Similarly, calling empty() and size() didn't make sense for none VOBJ/VARR UniValues.

**Please note:** This is a draft and not meant to be merged just yet. I created the pull request already because I wanted to see what the CI-Pipeline tells me about this. I also want to give
...

🤔 PRADACANDI18 reviewed a pull request: "util: Safer MakeByteSpan with ByteSpanCast"
(https://github.com/bitcoin/bitcoin/pull/27973#pullrequestreview-1503741432)
General feed back have I have not given approval just yet on a merge. I have to review some more

💬 mzumsande commented on pull request "net: Make AddrFetch connections to fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/26114#issuecomment-1611903953)
Updated and rebased due to multiple conflicts - in particular #27577 got merged which should've fixed the problem mentioned above. I intend to move this out of draft in a few days after some manual testing / possible CI fixes.

💬 stratospher commented on pull request "test: add python implementation of Elligator swift":
(https://github.com/bitcoin/bitcoin/pull/24005#discussion_r1245625118)
interesting! done.

💬 stratospher commented on pull request "test: add python implementation of Elligator swift":
(https://github.com/bitcoin/bitcoin/pull/24005#issuecomment-1611916209)
Rebased on master, updated to address [review comment](https://github.com/bitcoin/bitcoin/pull/24005#discussion_r1239096530) and to just use commits from 26222 which this depends on.

💬 achow101 commented on pull request "wallet: bugfix, always use apostrophe for spkm descriptor ID":
(https://github.com/bitcoin/bitcoin/pull/27920#issuecomment-1611922773)
ACK 5df988b534df842ddb658ad2a7ff0f12996c8478

💬 achow101 commented on pull request "util: Allow std::byte and char Span serialization":
(https://github.com/bitcoin/bitcoin/pull/27927#issuecomment-1611928229)
ACK fa38d862358b87219b12bf31236c52f28d9fc5d6

🚀 achow101 merged a pull request: "util: Allow std::byte and char Span serialization"
(https://github.com/bitcoin/bitcoin/pull/27927)

🤔 jonatack reviewed a pull request: "test: add end-to-end tests for CConnman and PeerManager"
(https://github.com/bitcoin/bitcoin/pull/26812#pullrequestreview-1503702678)
ACK 4557cc336fd8eb321b0db024b70213f46017071c

💬 jonatack commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1245633845)
https://github.com/bitcoin/bitcoin/commit/37cd2f5a358f1d21c11efa8783be65570418c648 and 4557cc3

Have a look at https://cirrus-ci.com/task/4730456604672000 for IWYU suggestions on the new files.

💬 jonatack commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1245592304)
37cd2f5 naming: maybe call the test file `net_msg_tests.cpp` (and move this to line 113)

💬 jonatack commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1245596582)
37cd2f5

<details><summary>non-blocking suggestions: make pipes const, prefix iterator, reduce nesting</summary><p>

```diff
@@ -22,20 +22,15 @@ BOOST_FIXTURE_TEST_SUITE(netmsg_tests, NetTestingSetup)
BOOST_AUTO_TEST_CASE(initial_messages_exchange)
{
std::unordered_map<std::string, size_t> count_sent_messages;
- auto pipes = m_sockets_pipes.PopFront();
+ const auto pipes{m_sockets_pipes.PopFront()};

// Wait for all messages due to the initial handshake to be Send(
...

💬 jonatack commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1245634333)
https://github.com/bitcoin/bitcoin/commit/4557cc336fd8eb321b0db024b70213f46017071c

<details><summary>non-blocking suggestions</summary><p>

```diff
@@ -57,7 +57,7 @@ void initialize_process_message_e2e()
/*chain_type=*/ChainType::REGTEST,
/*extra_args=*/{"-txreconciliation"});
g_setup = testing_setup.get();
- for (int i = 0; i < 2 * COINBASE_MATURITY; i++) {
+ for (int i = 0; i < 2 * COINBASE_MATURITY; ++i) {
MineBlock(g_setup->m_node, CS
...

💬 ryanofsky commented on pull request "refactor: Drop unsafe AsBytePtr function":
(https://github.com/bitcoin/bitcoin/pull/27978#issuecomment-1611965447)
Updated 650ca0d937c2c90adeeac60adae090902618d771 -> 7c853619ee9ea17a79f1234b6c7871a45ceadcb9 ([`pr/noptr.3`](https://github.com/ryanofsky/bitcoin/commits/pr/noptr.3) -> [`pr/noptr.4`](https://github.com/ryanofsky/bitcoin/commits/pr/noptr.4), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/noptr.3..pr/noptr.4)) switching from `AsWritableBytes` to `MakeWritableByteSpan`, making `SpanFromDbt` a static function, and adding a `SpanFromBlob` function to simplify sqlite code

💬 dooglus commented on issue "bitcoind hangs waiting for `g_requests.empty()`":
(https://github.com/bitcoin/bitcoin/issues/27722#issuecomment-1611979215)
> I was able to fix this with my branch here [Crypt-iQ@4fd7adb](https://github.com/Crypt-iQ/bitcoin/commit/4fd7adb30f584664421b6431bce8ebcbf7ceef2f) by adding a `evhttp_connection_set_closecb` callback.

This fixed the issue for me too, but I just saw a crash caused by an assert() just 4 lines before the modified code:

[Thread 0x7ffec0f4b700 (LWP 1828755) exited]
bitcoin-qt: httpserver.cpp:223: http_request_cb(evhttp_request*, void*)::<lambda(evhttp_request*, void*)>: Assertion `n
...

💬 lontivero commented on issue "Improving fee estimation accuracy":
(https://github.com/bitcoin/bitcoin/issues/27995#issuecomment-1611989703)
Given the current fee rate estimation algorithm has no prediction power when the mempool conditions change, it is common to pay too little or too much, what is especially true while we increase the confirmation target.

In Wasabi we use the feerate histogram to "correct" the fee rate estimations provided by our node and to keep them between an acceptable range. There are many things that can be done but we just want to be sure our users participating in coinjoins don't pay more than every
...

💬 jonatack commented on pull request "refactor: Drop unsafe AsBytePtr function":
(https://github.com/bitcoin/bitcoin/pull/27978#issuecomment-1612028668)
re-ACK 7c853619ee9ea17a79f1234b6c7871a45ceadcb9

The OP and commit message might need to be updated a bit.

💬 achow101 commented on pull request "Introduce secp256k1 module with field and group classes to test framework":
(https://github.com/bitcoin/bitcoin/pull/26222#issuecomment-1612048492)
ACK d4fb58ae8ae9772d025ead184ef8f2c0ea50df3e

Much simpler to understand, and the more complicated stuff matches up with the descriptions in WIkipedia.