💬 fjahr commented on issue "ci: failure in `wallet_basic.py --descriptors`":
(https://github.com/bitcoin/bitcoin/issues/27974#issuecomment-1611648809)
Looks like a dublicate of #27249 ?

💬 jonatack commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1245423693)
Thanks for looking into it. I like the idea of keeping them separate and avoiding including unneeded code, in general and for building with slower, older CPUs. So 👍 for your diff in https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1244795380. Great work so far AFAICT. Will continue reviewing the remaining commits.

💬 jonatack commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1245427231)
> In both cases all tests would be recompiled

Yes, but future changes to `util/net` would not require compiling all the tests.

💬 fjahr commented on pull request "Prevent file descriptor exhaustion from too many RPC calls":
(https://github.com/bitcoin/bitcoin/pull/27731#discussion_r1245435268)
Hm, how would that happen? I didn't have any issues when testing this code with 2.2.1-alpha and 2.1.12-stable.

📝 sipa opened a pull request: "Make poly1305 support incremental computation + modernize"
(https://github.com/bitcoin/bitcoin/pull/27993)
Our current Poly1305 code (src/crypto/poly1305.*) only supports computing the entire tag in one go (the `poly1305_auth` function takes a key and message, and outputs the tag). However, the RFC8439 authenticated encryption (as used in BIP324, see #27634) scheme makes use of Poly1305 in a way where the message consists of 3 different pieces:
* The additionally authenticated data (AAD), padded to 16 bytes.
* The ciphertext, padded to 16 bytes.
* The length of the AAD and the length of the cipher
...

🤔 jonatack reviewed a pull request: "test: remove race in the user-agent reception check"
(https://github.com/bitcoin/bitcoin/pull/27986#pullrequestreview-1503489729)
Light tested ACK c1a169082ac1147ec2f3cc329f23bc0bfc28cd6b

💬 jonatack commented on pull request "test: remove race in the user-agent reception check":
(https://github.com/bitcoin/bitcoin/pull/27986#discussion_r1245457894)
Suggest while touching this, to make it clearer and more coherent with the documentation just above.

```diff
- # Consistency check that the Bitcoin Core has received our user agent string.
- # Find our connection in "getpeerinfo" by our address:port, it is unique.
+ # Consistency check that the node received our user agent string.
+ # Find our connection in getpeerinfo by our address:port, as it is unique.
```

💬 vasild commented on pull request "test: remove race in the user-agent reception check":
(https://github.com/bitcoin/bitcoin/pull/27986#issuecomment-1611751064)
`c1a169082a...28d26c4f37`: take https://github.com/bitcoin/bitcoin/pull/27986#discussion_r1245457894

💬 vasild commented on pull request "test: remove race in the user-agent reception check":
(https://github.com/bitcoin/bitcoin/pull/27986#discussion_r1245488264)
Done.

💬 ryanofsky commented on pull request "index: make startup more efficient":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1245477180)
In commit "refactor: simplify pruning violation check" (2412ffdce69ce2b164e5303ae4b610ab0d00fd5c)

Looking at this code, it's not really clear that `block_to_test` is going to be non-null, and that the `CheckBlockDataAvailability` check is going to pass. If it were null, the pruning check would fail even if there were no pruned data.

So I think it would be good add an assert with an explanation of why `block_to_test` can't be null here, and I implemented this below.

While implementing th
...

👍 ryanofsky approved a pull request: "index: make startup more efficient"
(https://github.com/bitcoin/bitcoin/pull/27607#pullrequestreview-1503519460)
Code review ACK 2f830d1de8f0b57f46f1d7da3cb7b9ab4aa1e2ff. I left a long comment below about one of the intermediate commits after I got hung up on whether `block_to_test` could be null before it was passed to the CheckBlockData function. I think it is not too important since the code is deleted in the end, but I did suggest some improvements to CheckBlockData that I think would make it safer and easier to use.

👍 jamesob approved a pull request: "test: Fuzz on macOS"
(https://github.com/bitcoin/bitcoin/pull/27932#pullrequestreview-1503552896)
Github ACK https://github.com/bitcoin/bitcoin/pull/27932/commits/fae7c50d201726f605938c3511dd9119efeea5ec

Adds fuzzing to a new architecture, right? Seems like an easy win?

💬 jamesob commented on pull request "util: Don't derive secure_allocator from std::allocator":
(https://github.com/bitcoin/bitcoin/pull/27930#issuecomment-1611769447)
Concept ACK. Thanks for filing the change @CaseyCarter, and welcome to the project!

👍 jamesob approved a pull request: "test: Fix intermittent issue in mining_getblocktemplate_longpoll.py"
(https://github.com/bitcoin/bitcoin/pull/27941#pullrequestreview-1503585748)
Github ACK https://github.com/bitcoin/bitcoin/pull/27941/commits/fa748c6f2ac2f9cac7ce42fd745ed3c3eae093b7

Seems like a commonsense fix? Can't test the RPC method if the Bitcoin Core HTTP handler hasn't booted.

👍 vasild approved a pull request: "p2p: Restrict self-advertisements with privacy networks to avoid fingerprinting"
(https://github.com/bitcoin/bitcoin/pull/27411#pullrequestreview-1503590039)
ACK e7cf8657e1165ea5da3911a9e543837cd8938f97

💬 jonatack commented on pull request "test: remove race in the user-agent reception check":
(https://github.com/bitcoin/bitcoin/pull/27986#discussion_r1245517426)
Verified per https://docs.python.org/3/whatsnew/3.8.html that [asyncio.BaseTransport.get_extra_info()](https://docs.python.org/3/library/asyncio-protocol.html#asyncio.BaseTransport.get_extra_info) was added in Python 3.8, the minimum supported version per `doc/dependencies.md`.

I'm not sure if `import asyncio` should be added to this file.

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

modulo question in https://github.com/bitcoin/bitcoin/pull/27986#discussion_r1245517426

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