💬 murchandamus commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2220508773)
In "wallet: throw error at conflicting tx versions in pre-selected inputs" (cf712ab56e3256db5dfcf422860bdb3cf87cd03e):
Is `m_version` always set?
Otherwise, what would happen if `coin_control.m_version` is not set, and one preselected input is TRUC, but another one is not?

💬 murchandamus commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2220523528)
In "wallet: don't include unconfirmed v3 txs with children in available coins" (bff006b7ae7fa120fac4f2118e891d2c88d38f9a):

I find this comment hard to parse. Do you mean something along the lines of:

```suggestion
// this marks all outputs of any unconfirmed v3 parent transactions as unspendable
// if any of its outputs are spent by this transaction
```

📝 marshallshen opened a pull request: "test: functional test for incomplete PSBT with additional signatures required"
(https://github.com/bitcoin/bitcoin/pull/33035)
## Summary

This PR adds a new functional test `test_psbt_incomplete_needs_additional_signature` to verify wallet behavior when a PSBT requires additional signatures and cannot be completed.

## What's Added
- Test scenario: Creates a 2-of-3 multisig transaction with only 1 signature provided
- Behavior validation: Confirms the wallet correctly identifies the transaction as incomplete and refuses to finalize

This test ensures that Bitcoin Core's wallet properly handles the common scenar
...

💬 l0rinc commented on pull request "tests: speed up coins_tests by parallelizing":
(https://github.com/bitcoin/bitcoin/pull/32945#issuecomment-3100621871)
reACK 06ab3a394ade1e0d4fdb1fef636ff0d6bad71948

Only difference since last review was applying https://github.com/bitcoin/bitcoin/pull/32945#discussion_r2218430945

💬 ajtowns commented on pull request "tests: speed up coins_tests by parallelizing":
(https://github.com/bitcoin/bitcoin/pull/32945#discussion_r2221068746)
Done

💬 Eunovo commented on pull request "wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key":
(https://github.com/bitcoin/bitcoin/pull/32471#discussion_r2221456230)
Nice idea. Added you as Co-author in the relevant commit.

💬 josibake commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2221476916)
> The silent payments wallet needs to scan entire blocks since it doesn't know ahead of time which scriptPubKeys to use in a GCS filter

This is correct. I was imagining a world where we can pre-compute the scriptPubKeys, but realised we have to iterate the blocks anyways to get the tweaks so its likely faster to just check directly. In the future, if the wallet has some sort of silent payments index, then I think fast rescans would be possible (and beneficial).

💬 Eunovo commented on pull request "wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key":
(https://github.com/bitcoin/bitcoin/pull/32471#discussion_r2221504575)
I forgot about the guard I added in https://github.com/bitcoin/bitcoin/pull/32471/commits/4202dca524aeef13e09380804c483d7c5402092d

You are right in the sense that it's not possible for a watch-only to even get to `ToPrivateString()` through the rpc; we could assert `has_priv_key` here. However, the descriptor unit tests often call `ToPrivateString()` on watch-only descriptors; assert `has_priv_key` causes the tests to fail.

🤔 vasild reviewed a pull request: "net: Fix Discover() not running when using -bind=0.0.0.0:port"
(https://github.com/bitcoin/bitcoin/pull/32757#pullrequestreview-3041523676)
Almost ACK a9b7542b6ea1af770d93cbbf5bb55dae6841334f

> I encountered port conflicts because the test framework automatically adds an onion bind to node 2,

Hmm, I think it is not the test framework but `bitcoind` itself that adds the onion bind. To figure out how `bitcoind` is started, e.g. for `nodes[2]` I look at `/tmp/bitcoin_func_test_.../node2/regtest/debug.log` for the lines like:
```
...
Config file arg: [regtest] shrinkdebugfile="0"
Config file arg: [regtest] unsafesqlitesync="1
...

💬 vasild commented on pull request "net: Fix Discover() not running when using -bind=0.0.0.0:port":
(https://github.com/bitcoin/bitcoin/pull/32757#discussion_r2221491082)
The testing framework is designed so that it can run parallel executions on the same machine. For example, running two separate tests independently at the same time. To avoid port collisions between such independent executions is has `p2p_port(n)` and `tor_port(n)` which allocate ports in a range which (hopefully) will not overlap with the range from another test that executes concurrently (e.g. in another terminal). This `+100` seems a odds with this, maybe it could cause collisions.

I would
...

💬 Eunovo commented on pull request "wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key":
(https://github.com/bitcoin/bitcoin/pull/32471#discussion_r2221513192)
To add this assertion, we will have to remove those checks, and I haven't seen the advantage in doing that.

💬 Eunovo commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2221561291)
Yes, it does. The ImportingNow object is destroyed at the end of the function.

💬 maflcko commented on pull request "wallet, sqlite: Encapsulate SQLite statements in a RAII class":
(https://github.com/bitcoin/bitcoin/pull/33033#discussion_r2221607401)
nit: you can use `ALWAYS_FALSE` for this, so that the workaround docs for the C++23 defect report are in one place only.

💬 maflcko commented on pull request "tests: speed up coins_tests by parallelizing":
(https://github.com/bitcoin/bitcoin/pull/32945#issuecomment-3101567218)
lgtm ACK 06ab3a394ade1e0d4fdb1fef636ff0d6bad71948

💬 maflcko commented on pull request "test: functional test for incomplete PSBT with additional signatures required":
(https://github.com/bitcoin/bitcoin/pull/33035#issuecomment-3101578061)
https://corecheck.dev/bitcoin/bitcoin/pulls/33035 does not show any new coverage. What is the new coverage and how to observe that it is actually new?

💬 Eunovo commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2221693565)
Done.

💬 Sjors commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#issuecomment-3101643309)
Perhaps there should be a (continuously updated) BIP of transaction standardness rules? It can track which clients and versions support them. And list which ones are configurable in clients. And finally which clients have different defaults.

💬 Sjors commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#issuecomment-3101665965)
@Eunovo that's an interesting trade-off, and we should probably be consistent with how the rest of the wallet behaves. Otherwise we'd leave a fingerprint of which coins were sent using silent payments.

So in that case I think the bevavior should be:
- if coins are sent to a bech32m address, use a silent payment change address (don't use the regular bech32m descriptor)
- otherwise, use a matching output type
- when there's no descriptor for it, fall back to silent payment change address

📝 fanquake opened a pull request: "Update secp256k1 subtree to latest master"
(https://github.com/bitcoin/bitcoin/pull/33036)
Updates the subtree to https://github.com/bitcoin-core/secp256k1/commit/b9313c6e1a6082a66b4c75777e18ca4b176fcf9d. See https://github.com/bitcoin-core/secp256k1/blob/master/CHANGELOG.md#070---2025-07-21 for the most relevant changes.

💬 Sjors commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-3101714725)
This needed a release note :-)

Maybe we can still mention it in the next v29 update?