💬 furszy commented on pull request "index: Compare deserialized block hash with the block hash from the blockindex":
(https://github.com/bitcoin/bitcoin/pull/26390#discussion_r1181268939)
Could use structured bindings:
```c++
for (const auto& [block_hash, filter] : entries) {
if (!ReadFilterFromDisk(filter.pos, filter.hash, block_hash, *filter_pos_it)) {
```

(Same below)

💬 furszy commented on pull request "index: Compare deserialized block hash with the block hash from the blockindex":
(https://github.com/bitcoin/bitcoin/pull/26390#discussion_r1181270062)
Wouldn't hurt to add a comment for this. By reading only the function's signature, it's not clear what the uin256 is.

💬 furszy commented on pull request "rpc: simplify scan blocks":
(https://github.com/bitcoin/bitcoin/pull/26780#issuecomment-1529099811)
Thanks for the review TheCharlatan.

> Does this require a release note for the s/filtertype/filter_type/ change?

Yeah.. I made this in December, just after #23549 got merged. Now that v25 is released, we have to add release notes for it.
Will just drop that commit.

💬 furszy commented on pull request "rpc: simplify scan blocks":
(https://github.com/bitcoin/bitcoin/pull/26780#discussion_r1181271406)
yeah, thanks.

💬 furszy commented on pull request "rpc: simplify scan blocks":
(https://github.com/bitcoin/bitcoin/pull/26780#discussion_r1181271614)
sounds good

💬 furszy commented on pull request "rpc: simplify scan blocks":
(https://github.com/bitcoin/bitcoin/pull/26780#discussion_r1181272996)
yeah, the user could have aborted the process at the very beginning.

maybe could change it to:
```
end_range ? end_range->nHeight : start_index->nHeight;
```
but I'm not so sure that this makes code easier to read.

👍 TheCharlatan approved a pull request: "[23.x] Additional backports for 23.x"
(https://github.com/bitcoin/bitcoin/pull/27475#pullrequestreview-1407167093)
ACK f0919339bfd983975fe3b85f51423302a1d8a5a0

💬 TheCharlatan commented on pull request "rpc,rest,zmq: faster getblock, NotifyBlock and rest_block by reading raw block":
(https://github.com/bitcoin/bitcoin/pull/26415#discussion_r1181281332)
I'm guessing the commit message should say "rest:..."?

🤔 TheCharlatan reviewed a pull request: "rpc,rest,zmq: faster getblock, NotifyBlock and rest_block by reading raw block"
(https://github.com/bitcoin/bitcoin/pull/26415#pullrequestreview-1407169071)
Concept ACK

Code looks good to me, but still want to run some tests.

💬 TheCharlatan commented on pull request "build: Check usages of #if defined(...)":
(https://github.com/bitcoin/bitcoin/pull/25302#discussion_r1181287798)
I think this is only defined conditionally.

📝 aureleoules opened a pull request: "ci: Add test coverage job"
(https://github.com/bitcoin/bitcoin/pull/27547)
This PR adds a new Cirrus job that generates test coverage reports for all pull requests and master using [Codecov](https://about.codecov.io/). It is free for open-source projects.
We can now quickly determine whether a feature that a pull request adds is properly tested.

I've ran a few tests on my own fork, you can check it out here: https://app.codecov.io/gh/aureleoules/bitcoin and a test pull request: https://app.codecov.io/gh/aureleoules/bitcoin/pull/5.

Codecov has a nice feature that
...

💬 achow101 commented on issue "Consider unconfirmed unspent outputs from external wallet as safe for spending for "sendtoaddress" and "fundrawtransaction" RPC":
(https://github.com/bitcoin/bitcoin/issues/17936#issuecomment-1529141419)
Such inputs can be manually selected when using `fundrawtransaction` or `walletcreatefundedpsbt`. However allowing coin selection to select such inputs introduces a potential footgun and we generally are not interested in doing that.

The feature request didn't seem to attract much attention in the past. Also, the issue seems not important enough right now to keep it sitting around idle in the list of open issues.

Closing due to lack of interest. Pull requests with improvements are always w
...

✅ achow101 closed an issue: "Consider unconfirmed unspent outputs from external wallet as safe for spending for "sendtoaddress" and "fundrawtransaction" RPC"
(https://github.com/bitcoin/bitcoin/issues/17936)

✅ achow101 closed an issue: "Prevent possible footgun caused by decodescript converting P2PKH with uncompressed pubkey into SegWit addresses"
(https://github.com/bitcoin/bitcoin/issues/19383)

💬 achow101 commented on issue "Prevent possible footgun caused by decodescript converting P2PKH with uncompressed pubkey into SegWit addresses":
(https://github.com/bitcoin/bitcoin/issues/19383#issuecomment-1529141656)
We generally avoid showing things to users that can result in funds being "stuck", even if technically still valid.

The feature request didn't seem to attract much attention in the past. Also, the issue seems not important enough right now to keep it sitting around idle in the list of open issues.

Closing due to lack of interest. Pull requests with improvements are always welcome.

💬 ariard commented on issue "Package Relay Project Tracking":
(https://github.com/bitcoin/bitcoin/issues/27463#issuecomment-1529256915)
If the state-of-art implementation of BIP331 can be linked here or in the BIP (Like there is https://github.com/glozow/bitcoin/pull/8 and https://github.com/glozow/bitcoin/pull/9) ?

From a reviewer perspective, ideally it’s better to have the proposed specification changes, the proposed Core code changes and the backend code of broadcaster client (e.g Lightning) to grasp a correct mental model of all the interactions.

👍 TheCharlatan approved a pull request: "rpc: simplify scan blocks"
(https://github.com/bitcoin/bitcoin/pull/26780#pullrequestreview-1407409471)
ACK b922f6b5262884f42d7483f1e9af35650bdb53a7

💬 TheCharlatan commented on pull request "rpc: Optimize serialization disk space of dumptxoutset":
(https://github.com/bitcoin/bitcoin/pull/26045#issuecomment-1529437357)
ACK 7acfc2a221237877aa3522266cac6c326ae7fe8e

Since we are changing the format of dumptxoutset anyway here in a non backwards compatible fashion, I'd like to suggest moving the metadata to the end of the file. This would take care of the double iteration described in https://github.com/bitcoin/bitcoin/pull/26045#pullrequestreview-1403015467. In my eyes, this does not significantly hurt the integrity of the file. If an exception occurs during writing, only the temporary file remains. Other comm
...

💬 MarcoFalke commented on issue "wallet: Imports with pre-existing balance somtimes don't have any balance after a rescan":
(https://github.com/bitcoin/bitcoin/issues/19808#issuecomment-1529480218)
Is this still an issue with a recent version of Bitcoin Core? If yes, what are the steps to reproduce?

💬 josibake commented on issue "wallet: Imports with pre-existing balance somtimes don't have any balance after a rescan":
(https://github.com/bitcoin/bitcoin/issues/19808#issuecomment-1529482079)
> If yes, what are the steps to reproduce?

I am attempting to reproduce this week, will post by the end of the week if I’m successful. If I can’t reproduce, we can probably close