💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2138569907)
c21466b83d725ab38e8b2b6c5b3e01815b300745

it's not actually bytes; we we want to just refer to Usage() directly here

💬 glozow commented on pull request "config: allow setting -proxy per network":
(https://github.com/bitcoin/bitcoin/pull/32425#issuecomment-2960426526)
Needs release note?

💬 pablomartin4btc commented on pull request "wallet, refactor: Remove Legacy wallet unused warnings and errors":
(https://github.com/bitcoin/bitcoin/pull/32481#issuecomment-2960434399)
_<ins>Updates</ins>_:
* Addressed [feedback](https://github.com/bitcoin/bitcoin/pull/32481#discussion_r2138353475) from @maflcko [and](https://github.com/bitcoin/bitcoin/pull/32481#discussion_r2138470337) @achow101.
* Removed also wallet descriptor check from `CWallet::EncryptWallet()` and `CWallet::Create()`.

✅ glozow closed an issue: "Apparently CJDNS network does not work with Tor on mainnet."
(https://github.com/bitcoin/bitcoin/issues/24450)

🚀 glozow merged a pull request: "config: allow setting -proxy per network"
(https://github.com/bitcoin/bitcoin/pull/32425)

💬 glozow commented on pull request "package validation: relax the package-not-child-with-unconfirmed-parents rule":
(https://github.com/bitcoin/bitcoin/pull/31385#issuecomment-2960446263)
Rebased since it's been a while. Still worth doing imo! Getting this out of the way would be good before we try to do subpackage evaluation / package RBF stuff, fwiw.

💬 hodlinator commented on pull request "build: Add resource file and manifest to `bitcoin.exe`":
(https://github.com/bitcoin/bitcoin/pull/32634#discussion_r2138681467)
Followed up on in #32719.

⚠️ Christewart opened an issue: "`signrawtransactionwithkey` doesn't work with non segwit p2sh scripts"
(https://github.com/bitcoin/bitcoin/issues/32722)
### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current behaviour

I'm attempting to reproduce the transaction specified by AJ Towns that pointed out BitVM bridges with CTV+CSFS for p2sh outputs are vulnerable to theft. You can read more about the topic [here](https://delvingbitcoin.org/t/how-ctv-csfs-improves-bitvm-bridges/1591/8).

The TLDR is i'm attempting to sign a `p2sh(p2pk)` script.

I've attempted to create this transaction via the `signrawtransa
...

💬 ariard commented on pull request "BIP-119 (OP_CHECKTEMPLATEVERIFY) (regtest only)":
(https://github.com/bitcoin/bitcoin/pull/31989#issuecomment-2960477093)
@jamesob You can break some CTV use-cases by abusing the segwit block limits (i.e the 80k limit sigops).

Here a test case on some 28.x branch, i.e `getblocktemplate` won’t accept more txn, once 80k limit reached)
https://github.com/ariard/bitcoin/commit/b85a426c43cb7000788a55ea140b73a68da9ce4e

An adversary can be break-even by targeting multiples “contract protocols” in overflowing a single sequence of blocks.

The easy fix is to move CTV as an OP_SUCCESS, as like it’s done for OP_CSFS.
...

🤔 pablomartin4btc reviewed a pull request: "wallet, rpc: Remove deprecated balances from getwalletinfo and getunconfirmedbalance"
(https://github.com/bitcoin/bitcoin/pull/32721#pullrequestreview-2915057715)
Concept ACK

There are still functional test references to the fields removed in the first commit (also in `wallet_multiwallet.py`, `wallet_reorgsrestore.py`). Should references in the wallet interface be updated as well?

This would need release notes at some point, no?

🤔 mzumsande reviewed a pull request: "index: move disk read lookups to base class"
(https://github.com/bitcoin/bitcoin/pull/32694#pullrequestreview-2914928430)
Concept ACK

💬 mzumsande commented on pull request "index: move disk read lookups to base class":
(https://github.com/bitcoin/bitcoin/pull/32694#discussion_r2138697822)
6f1392cc42cde638773f2b697d7d2c58abcdc860:
As a result of this commit, `CopyHeightIndexToHashIndex` (which doesn't have any other callers) could be simpler: No nee to pass two heights, no need for a loop if it only copies one element.

Besides, I wonder if the code duplication (it's identical code in `coinstatsindex` and `blockfilterindex`) could be resolved.

💬 sipa commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r2138805235)
Nice catch! I've added a commit that always performs the check for SimpleCandidateFinder (even if non-optimal).

💬 sipa commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r2138805358)
Indeed.

💬 sipa commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r2138806088)
The chunking won't necessarily match exactly, but the number of elements in the chunking must match if both are optimal.

💬 achow101 commented on issue "`signrawtransactionwithkey` doesn't work with non segwit p2sh scripts":
(https://github.com/bitcoin/bitcoin/issues/32722#issuecomment-2960680889)
Your test has a bug:

```
script_b = script_to_p2sh_script(redeem_script)
address = script_to_p2sh(script_b)
```

`address` is computed incorrectly. `script_to-p2sh` is like `script_to_p2sh_script` in that it takes the redeem script. When you pass it `script_b`, what you're really creating is a `p2sh(p2sh())` which is invalid. I believe if you modify your test to try to broadcast the transaction, it would fail to validate.

🤔 w0xlt reviewed a pull request: "wallet, rpc: Remove deprecated balances from getwalletinfo and getunconfirmedbalance"
(https://github.com/bitcoin/bitcoin/pull/32721#pullrequestreview-2915146785)
ACK https://github.com/bitcoin/bitcoin/pull/32721/commits/19a2f0ca09b9b56485d21e309486906c26e8afcf

💬 sipa commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r2138844396)
Huh, I hit fuzz failures with this if I let it run for a while. Weird, will investigate.

📝 theuni opened a pull request: "Refactor: Redefine CTransaction equality to include witness data"
(https://github.com/bitcoin/bitcoin/pull/32723)
I stumbled upon the `CTransaction` comparison operators while refactoring some nearby code. I found it surprising and not at all obvious that two transactions would test equal even if their witness data differed. It seems like an unnecessary potential footgun. Fix that by comparing against wtxid rather than txid.

Outside of tests, there were only 3 users of these functions in the code-base:
- Its use in the mempool has been replaced with an explicit txid comparison, as that's a tighter const
...

💬 furszy commented on pull request "index: move disk read lookups to base class":
(https://github.com/bitcoin/bitcoin/pull/32694#discussion_r2138853635)
This reminds me to my own [comment](https://github.com/bitcoin/bitcoin/pull/24230/commits/707ff84981d7f61d467093cb0aec7eee55276c52#r1307972744) hehe. Will do it here.

💬 theuni commented on pull request "Refactor: Redefine CTransaction equality to include witness data":
(https://github.com/bitcoin/bitcoin/pull/32723#issuecomment-2960706099)
Ping @achow101 and @glozow to check my assumptions about the wallet/mempool uses.