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

🚀 achow101 merged a pull request: "Introduce secp256k1 module with field and group classes to test framework"
(https://github.com/bitcoin/bitcoin/pull/26222)

💬 mzumsande commented on pull request "p2p: Restrict self-advertisements with privacy networks to avoid fingerprinting":
(https://github.com/bitcoin/bitcoin/pull/27411#issuecomment-1612067010)
> > Note that this affects only our own self-advertisements, the rules to forward other people's addrs are not changed.
>
> Would this make our own addr stand out too? Or could we still forward our own addr by coincidence outside of self-advertising?

Yes, we could still forward it: When we receive a gossipped address we choose 1-2 random peers (that stay the same for 24 hours, see [RelayAddress](https://github.com/bitcoin/bitcoin/blob/7952a5934a1e071ca24a51483d058174e7b6de43/src/net_proces
...

🤔 jonatack reviewed a pull request: "test: Use same timeout for all index sync"
(https://github.com/bitcoin/bitcoin/pull/27988#pullrequestreview-1503945589)
Approach ACK

(review beg for #27425 doing a ~similar test util extraction change)

💬 jonatack commented on pull request "test: Use same timeout for all index sync":
(https://github.com/bitcoin/bitcoin/pull/27988#discussion_r1245722202)
Perhaps call it `WaitUntilIndexSynced`

and `s/Block/Wait/`, or drop the Doxygen comment if the naming is clear enough

💬 jonatack commented on pull request "test: Use same timeout for all index sync":
(https://github.com/bitcoin/bitcoin/pull/27988#discussion_r1245731192)
Here and in the function declaration
```suggestion
void IndexWaitSynced(const BaseIndex& index)
```