💬 ErikDeSmedt commented on pull request "wallet: bugfix, always use apostrophe for spkm descriptor ID":
(https://github.com/bitcoin/bitcoin/pull/27920#issuecomment-1611795857)
ACK: I've tested the changes and the bug is resolved.

I've been struggling with back-porting the test-code to v25.0.
You might need to change some descriptors to use `h` instead of `'` and I manually removed some checksums.

@Sjors : Thanks for helping me out with this

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

⚠️ sipa opened an issue: "Improving fee estimation accuracy"
(https://github.com/bitcoin/bitcoin/issues/27995)
The current fee estimation algorithm suffers from a number of issues that make it hard to use in the real world:
* Because it's solely based on historical data (looking at how long mempool transactions take to confirm), it cannot react quickly to changing conditions.
* Because it's aiming to match seen behavior rather than requirement, if some non-negligible fraction of users keeps paying a certain high feerate, it may try to match that, even if unnecessary for confirmation.

There exist alt
...

💬 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