💬 ismaelsadeeq commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2175685307)
This should be in doc?

💬 pablomartin4btc commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2175739001)
nit: extra line here?
```suggestion
assert_equal(magic, BTREE_MAGIC)

def test_blank(self):
```

🤔 ishaanam reviewed a pull request: "wallet: Keep track of the wallet's own transaction outputs in memory"
(https://github.com/bitcoin/bitcoin/pull/27286#pullrequestreview-2972194802)
reACK 215e5999e2070a38c68e343c5c3f1dc37d567f58

💬 ishaanam commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r2175625170)
In 5cc32ee2a7addb38ae4a4c97d306d0c5d9cc2d5e "test: Test for balance update due to untracked output becoming spendable"

very minor nit: it is not clear to me as to why the time needs to be moved forward for this test to be effective, so perhaps a sentence could be added to explain that?

🤔 w0xlt reviewed a pull request: "wallet: Keep track of the wallet's own transaction outputs in memory"
(https://github.com/bitcoin/bitcoin/pull/27286#pullrequestreview-2972458146)
reACK https://github.com/bitcoin/bitcoin/pull/27286/commits/215e5999e2070a38c68e343c5c3f1dc37d567f58

💬 kevkevinpal commented on pull request "threading: use correct mutex name in reverse_lock fatal error messages":
(https://github.com/bitcoin/bitcoin/pull/32829#issuecomment-3020576505)
So one thing I noticed is that the tests passed, but the previous commit [85c2848](https://github.com/bitcoin/bitcoin/pull/32829/commits/85c2848eb575f4abaa81fdd4e8f3b2048693dd98) had failures but CI / test each commit (pull_request) passed

not sure this is a followup someone can look at to fix?

📝 instagibbs opened a pull request: "feature_taproot: sample tx version border values more"
(https://github.com/bitcoin/bitcoin/pull/32841)
Currently if the version 3 is selected for an otherwise standard spender, the test will fail. It's unlikely but possible, so change the test to update expectations and sample more aggressively on border values to instigate failures much quicker in the future if another version is made standard.

📝 theStack opened a pull request: "doc: add `/spenttxouts` to REST-interface.md"
(https://github.com/bitcoin/bitcoin/pull/32842)
Seems like adding the `spenttxouts` endpoint to the REST interface description was forgotten in #32540.

💬 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_r2176008565)
I agree that this part is pretty confusing, it's because a rescan timestamp of `now` means rescan all blocks from the MTP of the most recent block:

https://github.com/bitcoin/bitcoin/blob/6e5b67a370bbac16882938bf3f07af2519b4e9e7/src/wallet/rpc/backup.cpp#L378

https://github.com/bitcoin/bitcoin/blob/6e5b67a370bbac16882938bf3f07af2519b4e9e7/src/wallet/rpc/backup.cpp#L383

https://github.com/bitcoin/bitcoin/blob/6e5b67a370bbac16882938bf3f07af2519b4e9e7/src/wallet/rpc/backup.cpp#L399-L401

...

🤔 pablomartin4btc reviewed a pull request: "doc: add `/spenttxouts` to REST-interface.md"
(https://github.com/bitcoin/bitcoin/pull/32842#pullrequestreview-2972854934)
ACK dd99cedc0bfe7d7eee0f543bb27dab005c426c66

💬 murchandamus commented on issue "Cleanup CFeeRate constructor (sat/vB vs BTC/kvB)":
(https://github.com/bitcoin/bitcoin/issues/23129#issuecomment-3020948847)
@willcl-ark: I don’t think this is resolved. The state in `src/policy/feerate.cpp` still matches @maflcko’s comment from the linked PR that predates this issue by a day. I think #32750 is making some progress towards improvement, though.

📝 mzumsande opened a pull request: "validation: invalid block handling followups"
(https://github.com/bitcoin/bitcoin/pull/32843)
Some follow-ups from #31405:

- avoid duplicate recalculation of failure flags and `m_best_header` in `InvalidateBlock()`, which already does the accounting for these while disconnecting blocks, so recalcuation is not needed in the final `InvalidChainFound` call (https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2138368309)
- make it clearer that all block indexes with a `CBlockIndexWorkComparator` score at least as good as the tip (and no others) are expected in `setBlockIndexCandi
...

👍 theStack approved a pull request: "test: Fix wait_for_getheaders() call in test_outbound_eviction_blocks_relay_only()"
(https://github.com/bitcoin/bitcoin/pull/32823#pullrequestreview-2972945567)
lgtm ACK ec004cdb86e6471915e1033f390c76ee0428e415 :hash:

The second commit is a test bugfix very similar to #32742 / dd8447f70faf6419b4617da3c1b57098e9cd66a6 and should be backported (fwiw, I checked all other `wait_for_...` calls with hash parameters in this test file and think we caught them all now). The other commit improves the code comments w.r.t. consistent peer/node naming and getting rid of the "we" phrasing, agree that this is clearer now.

📝 luke-jr opened a pull request: "RPC/txoutproof: Support including (and verifying) proofs of wtxid"
(https://github.com/bitcoin/bitcoin/pull/32844)
Feature requested by @SomberNight for Electrum's Lightning stuff. (Please confirm this does what you need)

Unfortunately WIP because the wtxid merkle root calculation doesn't match and I don't have time to figure out why not yet (maybe after concept ACK? or if anyone else wants to take a look...)

Mostly backward compatible, but a segwit-enabled proof will verify the generation tx in old versions.

📝 pablomartin4btc opened a pull request: "rpc, test: Fix JSON parsing errors in RPC calls (unloadwallet and getdescriptoractivity) raising RPC_INVALID_PARAMETER with appropriate description"
(https://github.com/bitcoin/bitcoin/pull/32845)
Currently, `unloadwallet` RPC call fails with a JSON parsing error when no `wallet_name` argument is provided. This behavior is misleading because the error originates from a low-level JSON type mismatch, rather than clearly indicating that the wallet name or RPC endpoint (`-rpcwallet=...`) is missing. Also, found out that the [issue](https://github.com/bitcoin/bitcoin/pull/13111#issuecomment-398831543) was noticed during its implementation but never addressed.

In addition, I've verified all
...

💬 davidgumberg commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2176170471)
Fixed, thanks.

💬 SomberNight commented on pull request "RPC/txoutproof: Support including (and verifying) proofs of wtxid":
(https://github.com/bitcoin/bitcoin/pull/32844#discussion_r2176171672)
Is the cause of the mismatch that you thought the scriptPubKey directly contains the wtxid tree root?
It actually contains `Double-SHA256(witness root hash|witness reserved value)`
So you also have to extract the `witness reserved value` from the coinbase input witness.
see https://github.com/bitcoin/bips/blob/77b0bb297ad4e4e6b04b2c078c5e3b620a7090e2/bip-0141.mediawiki#commitment-structure

💬 SomberNight commented on pull request "RPC/txoutproof: Support including (and verifying) proofs of wtxid":
(https://github.com/bitcoin/bitcoin/pull/32844#issuecomment-3021260691)
> Feature requested by @SomberNight for Electrum's Lightning stuff. (Please confirm this does what you need)

Wow, thank you for working on this! I was meaning to open an issue to ask for this, but did not even get to that yet.

The request I would like an electrum server, assuming txindex=1, to be able to serve is:

`bc.tx.get_merkle_witness(txid)`, (so given just a `txid`,)

return a dict containing:
- `wtxid`
- `block_height`
- `block_hash`
- `pos` (index of tx in block)
- `mer
...

💬 SomberNight commented on pull request "RPC/txoutproof: Support including (and verifying) proofs of wtxid":
(https://github.com/bitcoin/bitcoin/pull/32844#discussion_r2176194870)
My cpp is beyond rusty, so maybe I misunderstand, but note that just because the tx to be proved does not have a witness, we would still want a witness-SPV proof for that tx.

The attack vector is that a malicious electrum server can notify the client about a tx touching their address and strip away the witness. That is, the actual tx has a witness, but the server sends it to the client using the old serialization format without it. Then when the client asks for the SPV proof, the server (stil
...

💬 bigshiny90 commented on pull request "RPC/txoutproof: Support including (and verifying) proofs of wtxid":
(https://github.com/bitcoin/bitcoin/pull/32844#discussion_r2176203977)
so, something like this?

`// Extract witness nonce from coinbase input
if (merkleBlock.m_gentx->vin.empty() || merkleBlock.m_gentx->vin[0].scriptWitness.stack.empty()) {
return {};
}
const auto& witness_nonce = merkleBlock.m_gentx->vin[0].scriptWitness.stack[0];
if (witness_nonce.size() != 32) {
return {};
}

// Calculate the witness
...