💬 PeterWrighten commented on issue "bitcoin-wallet requires write permissions when unneeded":
(https://github.com/bitcoin/bitcoin/issues/29559#issuecomment-2946134773)
> [@PeterWrighten](https://github.com/PeterWrighten) Any luck?

I worked on it and pulled a request for this issue. You could try and test it, as for me it performs well.

📝 PeterWrighten converted_to_draft a pull request: "wallet: Allow read-only database access for info and dump commands"
(https://github.com/bitcoin/bitcoin/pull/32685)
## Problem

Fixes #29559

The `bitcoin-wallet info` and `bitcoin-wallet dump` commands currently fail when wallet files are write-protected, throwing "Database opened in readonly mode but read-write permissions are needed" errors. This prevents users from safely inspecting write-protected wallet files, which is important for:

- Security-conscious setups where wallets are stored on read-only filesystems
- Forensic analysis of wallet files without risk of modification
- Backup verificatio
...

💬 achow101 commented on pull request "wallet, rpc: Move (Un)LockCoin WalletBatch creation out of RPC":
(https://github.com/bitcoin/bitcoin/pull/32593#discussion_r2130306601)
The naming follows our common pattern where `Load` is used to set things in memory because it is called by the wallet loading code.

💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2130337922)
Fixed

💬 brunoerg commented on issue "doc: references to unshipped documentation":
(https://github.com/bitcoin/bitcoin/issues/32565#issuecomment-2946395220)
> [@fanquake](https://github.com/fanquake) why not just updating the the docs to point to the online versions. e.g:
>
> **Replace:** `see doc/cjdns.md`
>
> With: `see https://github.com/bitcoin/bitcoin/blob/master/doc/cjdns.md`
>
> This make's sure that:
>
> 1. **Accessibility**: Users can easily access the documentation regardless of how they obtained the binaries.
> 2. **Up-to-Date Information**: Linking to the online repository ensures that users are directed to the most current version o
...

💬 PeterWrighten commented on issue "bitcoin-wallet requires write permissions when unneeded":
(https://github.com/bitcoin/bitcoin/issues/29559#issuecomment-2946552932)
@Sjors I have worked for this issue and pulled request. Can you review it? Thanks so much! #32685

👋 PeterWrighten's pull request is ready for review: "wallet: Allow read-only database access for info and dump commands"
(https://github.com/bitcoin/bitcoin/pull/32685)

📝 theStack opened a pull request: "depends: fix multiprocess build on OpenBSD (apply capnp patch, correct SHA256SUM command)"
(https://github.com/bitcoin/bitcoin/pull/32690)
This PR fixes the multiprocess depends build for OpenBSD by applying upstream patch https://github.com/capnproto/capnproto/pull/2308 and switching the SHA256SUM command to output hash sums in the expected format (the default is BSD format [1], but we need GNU format [2], see commit message for details).

The first commit can be replaced with a simple capnp version bump once this is available in a release.

Tested on OpenBSD 7.7 (x86_64) via
```
$ gmake -C depends MULTIPROCESS=1 NO_BOOST=1
...

💬 theStack commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-2947231102)
> [capnproto/capnproto#2308](https://github.com/capnproto/capnproto/pull/2308) - has landed, so you can pull that in here.

Opened #32690 since it turned out that only applying the patch was not enough, as another issue regarding the SHA256SUM command was revealed.

🤔 davidgumberg reviewed a pull request: "wallet: Keep track of the wallet's own transaction outputs in memory"
(https://github.com/bitcoin/bitcoin/pull/27286#pullrequestreview-2901675322)
ACK https://github.com/bitcoin/bitcoin/pull/27286/commits/39734259fc10b1339db2ae80c5fab12916cf0aff

Left a lot of non-blocking nits/questions/observations, many of which are out-of-scope, and all of which (except for commit hygiene nits) can be addressed in a follow up.

Benchmarked this branch with the hard-coded `nEpochIterations` removed as suggested by @rkrux (https://github.com/bitcoin/bitcoin/pull/27286#pullrequestreview-2850393076) against master with this commit cherry-picked on top,
...

💬 davidgumberg commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r2129798416)
In https://github.com/bitcoin/bitcoin/pull/27286/commits/41c82afdfc94d7e0e1daa5ac4f4e5814e113c398 (_wallet: Keep track of transaction outputs owned by the wallet_)

Feel-free-to-disregard-big-picture-out-of-PR-scope-question-comment:

I feel like one of the challenges in reading `CWallet` is how much state exists, and how many places that state is modified/read/accessed from, maybe most of this is unavoidable, but I wonder if it would be a benefit to have some of these wallet state members
...

💬 davidgumberg commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r2130604690)
https://github.com/bitcoin/bitcoin/pull/27286/commits/ae50dcb3a49eaf40fb64fd227c447ba90c45db34 (_wallet: Change balance calculation to use m_txos_)

---

nit: here and above, I think `GetTxOut()` and `GetWalletTx()` are used enough to justify a variable, also minimizes diff if names are reused, but I'm ~0 on changing them:

```suggestion
for (const auto& [outpoint, txo] : wallet.GetTXOs()) {
const CWalletTx& wtx{txo.GetWalletTx()};
const CTxOut& output{tx
...

💬 davidgumberg commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r2130659438)
https://github.com/bitcoin/bitcoin/pull/27286/commits/ae50dcb3a49eaf40fb64fd227c447ba90c45db34 (_wallet: Change balance calculation to use m_txos_)

nit: moving this check up would make the diff smaller

💬 davidgumberg commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r2130634083)
ae50dcb3a49eaf40fb64fd227c447ba90c45db34 (_wallet: Change balance calculation to use m_txos_)

Nit that can be addressed in a follow up, this commit leaves `ISMINE_USED` and related logic as dead code.

💬 davidgumberg commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r2129872942)
In commit https://github.com/bitcoin/bitcoin/pull/27286/commits/ad76a44c3d9f300d5d230af442d36f90b349bc12 (_wallet: Recompute wallet TXOs after descriptor migration_)

nit: This commit should be moved to before `m_txos` is used in `GetBalance()`

💬 davidgumberg commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r2129958056)
https://github.com/bitcoin/bitcoin/pull/27286/commits/8d9c7c3461d088211dde0551909898b8018059bf (_wallet: Recalculate the wallet's txos after any imports_)

----
Just an observation:

It's interesting to me that it's expected behavior that even without a rescan, previously untracked outputs of tracked transactions that become spendable are known to the wallet. I suppose this is a side effect of the logic before this PR, and this PR preserves that behavior. This is tested for in the earlier h
...

💬 davidgumberg commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r2130703872)
```suggestion
if (wallet.IsTxImmatureCoinBase(txo.GetWalletTx()) && txo.GetWalletTx().isConfirmed() && tx_depth >= min_depth) {
```

or alternately, nesting the immature and trusted checks together, maybe like:

```cpp
if (is_trusted && tx_depth >= min_depth) {
if (wallet.IsTxImmatureCoinBase(txo.GetWalletTx()) && txo.GetWalletTx().isConfirmed()) {
ret.m_mine_immature += credit_mine;
ret.m_
...

💬 davidgumberg commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r2130512343)
https://github.com/bitcoin/bitcoin/pull/27286/commits/ae50dcb3a49eaf40fb64fd227c447ba90c45db34 (_wallet: Change balance calculation to use m_txos_)

----
Why is this check added?

💬 davidgumberg commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r2130673491)
Out of scope observation:

It seems strange that there isn't a balance type for outputs that are trusted and confirmed in a block, but below `min_depth` confirmations, you see some balance when the tx is in the mempool, and then when it's got enough confirmations, but in between it disappears from your balances seems odd to me.

💬 why19850 commented on issue "Release Schedule for 30.0":
(https://github.com/bitcoin/bitcoin/issues/32275#issuecomment-2947734879)
1CKk5YAEqzrsTci9iPCoahnZ2dbEnyWZRi