✅ prusnak closed an issue: "rpc: dumptxoutset as sqlite file"
(https://github.com/bitcoin/bitcoin/issues/24628)

💬 prusnak commented on issue "rpc: dumptxoutset as sqlite file":
(https://github.com/bitcoin/bitcoin/issues/24628#issuecomment-2666086329)
> I think this is resolved with the merged of [#27432](https://github.com/bitcoin/bitcoin/pull/27432)?

Yes

💬 Sjors commented on pull request "mining: drop unused -nFees and sigops from CBlockTemplate":
(https://github.com/bitcoin/bitcoin/pull/31897#discussion_r1960007754)
Changed from -1 to 0, because if someone ignores the repeated instruction to not use this value, at least they won't incorrectly sum up `vTxFees`.

💬 rkrux commented on pull request "descriptor: check whitespace in keys within fragments":
(https://github.com/bitcoin/bitcoin/pull/31603#issuecomment-2666108177)
One way to ensure code structure correctness and get rid of the brittleness noticed here (https://github.com/bitcoin/bitcoin/pull/31603#discussion_r1959636120) is to incorporate the following change. In a cursory attempt, I have this that has minimal diff that passes the tests. LMK how does this sound.

```cpp
void CheckUnparsable(const std::string& prv, const std::string& pub, const std::string& expected_error, bool check_prv_error_only = false)
{
FlatSigningProvider keys_priv, keys_pu
...

🤔 hodlinator reviewed a pull request: "refactor: CLI cleanups"
(https://github.com/bitcoin/bitcoin/pull/31887#pullrequestreview-2624027023)
Code reviewed 3252f3bab02605129a568112f650ef3cb29703b7

ACK 9184ec9e8ea6efd4d0d3625a06f223156f940243

Advise dropping 3rd commit, the others seem good.

💬 hodlinator commented on pull request "refactor: CLI cleanups":
(https://github.com/bitcoin/bitcoin/pull/31887#discussion_r1960004078)
Weird to change inheritance from `class -> class` to `struct -> class`.

💬 hodlinator commented on pull request "refactor: CLI cleanups":
(https://github.com/bitcoin/bitcoin/pull/31887#discussion_r1960014260)
Makes more sense to use `class`es for these as they use `virtual` functions. Not worth changing just to be able to remove `public` IMO.

⚠️ fanquake opened an issue: "cmake: (release) version handling is broken"
(https://github.com/bitcoin/bitcoin/issues/31898)
Create a test tag: `v99.99`
Checkout `v99.99` and perform a guix build:`./contrib/guix/guix-build`.
Take the dist-archive tarball: `/bitcoin/guix-build-99.99/output/dist-archive/bitcoin-99.99.tar.gz` and perform a build:
```bash
tar xf bitcoin-99.99.tar.gz
cd bitcoin-99.99
cmake -B build
cmake --build build
# ./build/src/bitcoind --version
Bitcoin Core daemon version v28.99.0-g43e287b3ff5f0d223b0911b6ef90054ce5eb69d2
```

👍 darosior approved a pull request: "descriptor: check whitespace in keys within fragments"
(https://github.com/bitcoin/bitcoin/pull/31603#pullrequestreview-2623967849)
ACK 35d5adbcd58e04991bb4651d2dd97af6e186d1f9

💬 darosior commented on pull request "descriptor: check whitespace in keys within fragments":
(https://github.com/bitcoin/bitcoin/pull/31603#discussion_r1959975429)
*(comment about the commit, not this line)*

nit: in commit e91a9cf9d2ca50ee31ed36a975d2e01faa666c92 you state:
> Due to Base58, pubkeys with whitespace at the beginning
end/or at the end are successfully parsed.

But pubkeys are never base58 encoded.

💬 darosior commented on pull request "descriptor: check whitespace in keys within fragments":
(https://github.com/bitcoin/bitcoin/pull/31603#discussion_r1960036391)
Interestingly this line is correct only because we check `IsHex` before `ParseHex` below, as otherwise we would need to check for any space character across the string since `ParseHex` would skip those even if they happen in the middle.

Checking for a space character at any position would also improve the error message for instance when parsing `pk(03 a34b...)`. But no need to retouch.

💬 darosior commented on pull request "descriptor: check whitespace in keys within fragments":
(https://github.com/bitcoin/bitcoin/pull/31603#issuecomment-2666139402)
> is to incorporate the following change.

Improving the descriptor unit test utilities can be done in a followup. It would be nice to get the user-facing fix in first.

💬 i-am-yuvi commented on pull request "rpc: collect transaction fees on generateblock":
(https://github.com/bitcoin/bitcoin/pull/31775#issuecomment-2666140255)
I agree with @maflcko!

> > The block reward should include both the block subsidy and the transaction fees from all transactions included in the block.
>
> Is there a use case for this? `generateblock` is a test-only RPC, so there is no need to write extra code to collect the fees. Note that the other `generate*` calls will collect the fees.

✅ maflcko closed a pull request: "rpc: collect transaction fees on generateblock"
(https://github.com/bitcoin/bitcoin/pull/31775)

💬 maflcko commented on pull request "rpc: collect transaction fees on generateblock":
(https://github.com/bitcoin/bitcoin/pull/31775#issuecomment-2666159529)
Closing for now. For future contributions, my recommendation would be to compile the code and run the tests. Also, new tests should be included for new features.

💬 hebasto commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2666164979)
My Guix build:
```
aarch64
66e46fc7610a8e3a39b6253672b5063ea0f1c3ea54df6807c8e1d36d416e9d8c guix-build-f7d8a44ce42c/output/aarch64-linux-gnu/SHA256SUMS.part
6333dc4e4233970e5ec19ac3e3b99cc30eaedf78a5ec74da57363e0c6d9993fc guix-build-f7d8a44ce42c/output/aarch64-linux-gnu/bitcoin-f7d8a44ce42c-aarch64-linux-gnu-debug.tar.gz
4225ab5948b61dcc631346c08d1a6afd6263afaaecaf9301098f1edfd2933964 guix-build-f7d8a44ce42c/output/aarch64-linux-gnu/bitcoin-f7d8a44ce42c-aarch64-linux-gnu.tar.gz
d10bfa76
...

💬 hodlinator commented on pull request "doc: Improve `dependencies.md`":
(https://github.com/bitcoin/bitcoin/pull/31895#discussion_r1960058201)
Might be so cheeky as to order these alphabetically? Also CMake/Boost at the top.

💬 hodlinator commented on pull request "doc: Improve dependencies documentation":
(https://github.com/bitcoin/bitcoin/pull/31634#discussion_r1960065455)
Looks pretty ready to me, PR description and commits are informative. Left an additional suggestion.

💬 hebasto commented on pull request "cmake: Improve robustness and usability":
(https://github.com/bitcoin/bitcoin/pull/31233#issuecomment-2666177812)
Rebased due to the conflict with the merged bitcoin/bitcoin#31892.

💬 mzumsande commented on issue "test: feature_assumeutxo.py Windows timeout":
(https://github.com/bitcoin/bitcoin/issues/31894#issuecomment-2666205108)
#31019 (maxOS) might be the same issue, in which case this would not be windows-specific.