💬 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

🤔 Julianabrwon reviewed a pull request: "Use the stored CSubNet entry when unbanning"
(https://github.com/bitcoin-core/gui/pull/717#pullrequestreview-1407467727)
13VYk4xFjMN8d3wXnMZzvm8cwBHp3mo3LP

📝 fanquake locked a pull request: "Use the stored CSubNet entry when unbanning"
(https://github.com/bitcoin-core/gui/pull/717)
The previous code visualized the `CSubNet` object as string, then parsed that string back to `CSubNet`. This is sub-optimal given that the original `CSubNet` object can be used directly instead.

This avoids calling `LookupSubNet()` from the GUI.

⚠️ Sjors opened an issue: "Fuzz: compare our AES implementation to AES-NI "
(https://github.com/bitcoin/bitcoin/issues/27548)
We only use AES to generate a wallet encryption key from the user password. In #7689 we ditched the OpenSSL implementation for our own. It intentionally does not use special CPU instructions like AES-NI, because performance is not an issue for our use case. Instead it is based on an existing C implementation that's known to be constant-time.

We already have a fuzzer that checks an encryption - decryption round trip.

On CPU's that support it, we could add an additional fuzz target that uses
...

💬 josibake commented on pull request "build: include example bitcoin.conf in build outputs under share/examples/ subdirectory":
(https://github.com/bitcoin/bitcoin/pull/25935#issuecomment-1529636777)
> Including the config file example in the root of the build output can suggest that editing it will have an effect on the running program.

I don't think this is true, and it is pretty clear in the documentation in the repo that you must move the config file to the datadir. For example, the file itself starts with:

```
##
## bitcoin.conf configuration file.
## Generated by contrib/devtools/gen-bitcoin-conf.sh.
##
## Lines beginning with # are comments.
## All possible configuration o
...