💬 mzumsande commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1190113427)
Yes, no need to do change it in this PR.

💬 achow101 commented on pull request "refactor: Replace global find_value function with UniValue::find_value method":
(https://github.com/bitcoin/bitcoin/pull/27605#issuecomment-1542448136)
ACK fa266c4bbf564308ddbc12653527226506902084

💬 michaelfolkson commented on pull request "add ryanofsky to trusted-keys":
(https://github.com/bitcoin/bitcoin/pull/27604#issuecomment-1542478462)
For posterity: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2023-May/021618.html

💬 TheCharlatan commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1190136977)
`CZMQPublishRawBlockNotifier` takes `get_block_by_index` by value (meaning instead of borrowing, copies its value), so I think that is why this works out.

💬 TheCharlatan commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#issuecomment-1542522098)
Updated 886a473fc48f2c7d67436b5d9ac5643cd007e27f -> e3311d649d9785ca7bc3a29ad7470c7cd69951bb ([removeBlockstorageArgs_24](https://github.com/TheCharlatan/bitcoin/tree/removeBlockstorageArgs_24) -> [removeBlockstorageArgs_25](https://github.com/TheCharlatan/bitcoin/tree/removeBlockstorageArgs_25), [compare](https://github.com/TheCharlatan/bitcoin/compare/removeBlockstorageArgs_24..removeBlockstorageArgs_25))
* Addressed @ryanofsky's [comment](https://github.com/bitcoin/bitcoin/pull/27125#discuss
...

🚀 achow101 merged a pull request: "refactor: Replace global find_value function with UniValue::find_value method"
(https://github.com/bitcoin/bitcoin/pull/27605)

💬 ryanofsky commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1190102588)
In commit "zmq: Pass lambda to zmq's ZMQPublishRawBlockNotifier" (6247b3c5e3c9e70097243a85b3d883b279a473cc)

Would probably be a little more efficient to make the get_block_by_index non-const and use `m_get_block_by_index{std::move(get_block_by_index)}` to move the function instead of copying it.

👍 ryanofsky approved a pull request: "refactor, kernel: Decouple ArgsManager from blockstorage"
(https://github.com/bitcoin/bitcoin/pull/27125#pullrequestreview-1420938880)
Code review ACK 886a473fc48f2c7d67436b5d9ac5643cd007e27f

re: https://github.com/bitcoin/bitcoin/pull/27125#issuecomment-1542522098

> * Addressed @ryanofsky's [comment](https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1190097934), moving the lambda instead of borrowing.

Actually I think my suggestion won't work. There can be multiple notifiers, so the function needs to be copied into all of them not moved into the first one. ACKed previous version of the PR 886a473fc48f2c7d6
...

💬 ryanofsky commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1190113760)
In commit "refactor/iwyu: Complete includes for blockmanger_args" (c065e093b1c786476d9bc9769e42c2a54ff68816)

"blockmanger_args" spelling in commit title

💬 ryanofsky commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1190174923)
re: https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1190097934

Yeah sorry, I saw the "factories" map and assumed the factories would be long-lived, not destroyed before the function returns. This code is pretty strange, because it creates a map and then never looks anything up in the map. Could be cleaned up later, though. Probably a more sane way to write it would be to drop the map, change the outer for loop into an `add_notifiers` lambda, and do

```c++
add_notifiers("pubhashb
...

💬 fanquake commented on issue "GCC 13: `-Wdangling-reference` output":
(https://github.com/bitcoin/bitcoin/issues/26926#issuecomment-1542538866)
Closing, now that #27605 has been merged.

✅ fanquake closed an issue: "GCC 13: `-Wdangling-reference` output"
(https://github.com/bitcoin/bitcoin/issues/26926)

💬 fanquake commented on pull request "rpc: allow submitpackage to be called outside of regtest":
(https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1542546467)
Concept ACK - when you get a chance, can you put together a backport PR for `25.x`, so we can see the changes. I think if we want to do that, it'd be preferable to get it into `rc2`.

💬 fanquake commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#issuecomment-1542551108)
https://github.com/bitcoin/bitcoin/pull/27125/checks?check_run_id=13380672723
```bash
clang-tidy-16 -p=/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu -quiet /tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/zmq/zmqnotificationinterface.cpp
zmq/zmqnotificationinterface.cpp:48:62: error: std::move of the const variable 'gb' has no effect; remove std::move() or make the variable non-const [performance-move-const-arg,-warnings-as-errors]
return std:
...

💬 fanquake commented on pull request "ci: Remove CI_EXEC bloat":
(https://github.com/bitcoin/bitcoin/pull/27616#issuecomment-1542551991)
Concept ACK

💬 TheCharlatan commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#issuecomment-1542559776)
Updated 87479b2c3a6e5044baf41343e5287ff4bef0547a -> 5ff63a09a9edd1204b2cc56cf6f48a44adab7bb3 ([removeBlockstorageArgs_26](https://github.com/TheCharlatan/bitcoin/tree/removeBlockstorageArgs_27) -> [removeBlockstorageArgs_27](https://github.com/TheCharlatan/bitcoin/tree/removeBlockstorageArgs_27), [compare](https://github.com/TheCharlatan/bitcoin/compare/removeBlockstorageArgs_26..removeBlockstorageArgs_27))
* Reverted previous change as suggested by @ryanofsky in [comment](https://github.com/bi
...

💬 fanquake commented on issue "Intermittent failures in interface_usdt_mempool.py":
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-1542571731)
master @ e0a70c5b4f2c691e8d6b507c8ce879f0b0424254. https://cirrus-ci.com/task/5247512164958208:
```bash
node0 2023-05-10T17:29:06.449483Z (mocktime: 2023-05-24T17:29:09Z) [httpworker.1] [rpc/request.cpp:181] [parse] [rpc] ThreadRPCServer method=getrawmempool user=__cookie__
test 2023-05-10T17:29:06.450000Z TestFramework (INFO): Polling buffer...
test 2023-05-10T17:29:06.486000Z TestFramework (INFO): Ensuring mempool:rejected event was handled successfully...
test 2023-05-10T17:29:
...

👍 ryanofsky approved a pull request: "refactor, kernel: Decouple ArgsManager from blockstorage"
(https://github.com/bitcoin/bitcoin/pull/27125#pullrequestreview-1421110102)
Code review ACK 5ff63a09a9edd1204b2cc56cf6f48a44adab7bb3. Since last ACK just added std::move and fixed commit title. Sorry for the noise!

⚠️ Spyson opened an issue: "Recovering BTC sent to this Bitcoin address bc1q45puhw8zc5qpgzuuxnefakxckmnldulpgmp0la"
(https://github.com/bitcoin/bitcoin/issues/27619)

✅ pinheadmz closed an issue: "Recovering BTC sent to this Bitcoin address bc1q45puhw8zc5qpgzuuxnefakxckmnldulpgmp0la"
(https://github.com/bitcoin/bitcoin/issues/27619)