👍 TheCharlatan approved a pull request: "serialization: Support for multiple parameters"
(https://github.com/bitcoin/bitcoin/pull/28929#pullrequestreview-2056418308)
Nice, reviewing this was fun.
ACK 8d491ae9ecf1948ea29f67b50ca7259123f602aa

💬 TheCharlatan commented on pull request "serialization: Support for multiple parameters":
(https://github.com/bitcoin/bitcoin/pull/28929#discussion_r1600619857)
Just a comment: This does indeed not trigger clang-tidy's recursion check (I'm guessing because of the constexpr condition).

💬 paplorinc commented on pull request "blockstorage: Separate reindexing from saving new blocks":
(https://github.com/bitcoin/bitcoin/pull/29975#discussion_r1600629260)
Would it still make sense to call this method and discard the return ed value, like before, when the output param was mutated?

💬 paplorinc commented on pull request "blockstorage: Separate reindexing from saving new blocks":
(https://github.com/bitcoin/bitcoin/pull/29975#discussion_r1600629456)
Ah, no, I just like this change

💬 paplorinc commented on pull request "blockstorage: Separate reindexing from saving new blocks":
(https://github.com/bitcoin/bitcoin/pull/29975#issuecomment-2111128757)
ACK e41667b720372dae8438ea86e9819027e62b54e0
Great job, love the results of these untangling tasks!

💬 theuni commented on pull request "rpc: move UniValue in blockToJSON":
(https://github.com/bitcoin/bitcoin/pull/30094#issuecomment-2111151797)
@willcl-ark I added a quick/dirty bench to my branch that shows the difference between copies and moves. It seems correct to me.

Do you have any interest in doing the same for memory usage, just as a sanity check that nothing funky is happening low-level? Beyond that, I'm not really sure where to start with your graph... it really doesn't make any sense to me :)

💬 mzumsande commented on pull request "blockstorage: Separate reindexing from saving new blocks":
(https://github.com/bitcoin/bitcoin/pull/29975#discussion_r1600675693)
I don't think so, with the way the function is now only called if we don't know the BlockPos already it seems logical for callers to do something with the result. Of course, if someone comes up with such a use case in the future, they can just remove the `[[nodiscard]]` - after all its point is just to prevent future programming errors.

🤔 ryanofsky reviewed a pull request: "serialization: Support for multiple parameters"
(https://github.com/bitcoin/bitcoin/pull/28929#pullrequestreview-2056500485)
Thanks for the review!

💬 ryanofsky commented on pull request "serialization: Support for multiple parameters":
(https://github.com/bitcoin/bitcoin/pull/28929#discussion_r1600674320)
re: https://github.com/bitcoin/bitcoin/pull/28929#discussion_r1600617406

> Are you going to be using this method for other purposes besides tests and as an internal getter?

I could see some other uses. Initially this was just added for testing, but then marco pointed out it could be used as an internal getter in https://github.com/bitcoin/bitcoin/pull/28929#discussion_r1497229478 to avoid recursion in read, write, size, etc methods. The other case it might be useful is if you have have a c
...

💬 theuni commented on pull request "crypto: disable asan for sha256_sse4 with clang and -O0":
(https://github.com/bitcoin/bitcoin/pull/30097#issuecomment-2111184589)
> Concept ACK - Think I prefer fixing this inline, than in global flags / build. I guess we didn't end up making an issue upstream (https://github.com/llvm/llvm-project/issues)? If there is something to link to, would be good to add it here.

Upstream issue filed here: https://github.com/llvm/llvm-project/issues/92182

🤔 mzumsande reviewed a pull request: "p2p: detect addnode cjdns peers in GetAddedNodeInfo()"
(https://github.com/bitcoin/bitcoin/pull/30085#pullrequestreview-2056511498)
utACK d0b047494c28381942c09d0cca45baa323bfcffc

💬 jonatack commented on pull request "[27.x] Backports and probably finalize":
(https://github.com/bitcoin/bitcoin/pull/30092#issuecomment-2111186256)
Propose https://github.com/bitcoin/bitcoin/pull/30085 if merged in time.

💬 naiyoma commented on pull request "Test/rpc whitelistdefault test":
(https://github.com/bitcoin/bitcoin/pull/29858#issuecomment-2111204089)
> Thanks for adding these tests, concept ACK.
>
> Seems like this is in progress because there is test failure and a debugger is added. Added few suggestions for now, will review again when the tests pass.

Thanks for the review! The initial tests passed successfully. But I pushed an update that implements the suggested alternative approach from this [discussion](https://github.com/bitcoin/bitcoin/pull/29858#discussion_r1568092353)

Unfortunately, the tests are currently failing with this
...

💬 naiyoma commented on pull request "Test/rpc whitelistdefault test":
(https://github.com/bitcoin/bitcoin/pull/29858#discussion_r1600721946)
The idea here was to create a separate path for testing whitelisted users and a different path for testing users with no whitelist. The issue is that once a user is whitelisted, clearing the bitcoin.conf and writing new permissions still shows the user as whitelisted. I've provided a detailed explanation [here](https://github.com/bitcoin/bitcoin/pull/29858#discussion_r1589320996).

💬 naiyoma commented on pull request "Test/rpc whitelistdefault test":
(https://github.com/bitcoin/bitcoin/pull/29858#discussion_r1600728213)
Not sure if I should do this in this PR, since the main scope of this one is to cover the whitelistdefault test case.

💬 naiyoma commented on pull request "Test/rpc whitelistdefault test":
(https://github.com/bitcoin/bitcoin/pull/29858#discussion_r1600735279)
Ive provided an explanation of why this is happening [here](https://github.com/bitcoin/bitcoin/pull/29858#discussion_r1589320996)

💬 achow101 commented on pull request "refactor: Simply include CTxMemPool::Options in CTxMemPool directly rather than duplicating definition":
(https://github.com/bitcoin/bitcoin/pull/29086#issuecomment-2111326109)
ACK cc67d33fdac45357b593b1faff3d1735e5fe91ba

⚠️ SleepTheGod opened an issue: "Unexpected Termination of WebSocket Connection after Valid Subscription Request on ws.blockchain.info"
(https://github.com/bitcoin/bitcoin/issues/30103)
Summary:
A WebSocket client encounters an abrupt termination after successfully connecting to the WebSocket service at ws.blockchain.info and sending a valid subscription request for transaction updates related to a specific Bitcoin address.

Environment:
WebSocket Client: Custom Python script using websocket-client library.
Server: ws.blockchain.info
Date/Time of Occurrence: May 14, 2024, 23:06:34 GMT
Python Version: 3.x
Operating System: Not specified, but applicable to all.
Steps to
...

✅ achow101 closed an issue: "Unexpected Termination of WebSocket Connection after Valid Subscription Request on ws.blockchain.info"
(https://github.com/bitcoin/bitcoin/issues/30103)

💬 achow101 commented on issue "Unexpected Termination of WebSocket Connection after Valid Subscription Request on ws.blockchain.info":
(https://github.com/bitcoin/bitcoin/issues/30103#issuecomment-2111329558)
Issues with blockchain.info should be reported to them. They are not affiliated with this project in any way.