👍 pablomartin4btc approved a pull request: "test: Do not pass tests on unhandled exceptions"
(https://github.com/bitcoin/bitcoin/pull/33001#pullrequestreview-3044920696)
tACK faa3e684118bffa7a98cf76eeeb59243219df900

Managed to reproduce the issue with the patch provided in the PR description, this branch fixes it. Nice detail handling `KeyboardInterrupt` for debugging purpose. Code-wise refactoring: better reading removing redundant catches.

👍 stickies-v approved a pull request: "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs"
(https://github.com/bitcoin/bitcoin/pull/32845#pullrequestreview-3044915254)
re-ACK af0ad72a0ed778c7e6f83b1a290705cefe23d78a

💬 stickies-v commented on pull request "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs":
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2223852456)
nit: what's the purpose of wrapping this in `namespace wallet`?

Relatedly, should probably make `TestWalletName` `static`.

💬 stickies-v commented on pull request "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs":
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2223857169)
nit: can keep comma to avoid touching this line

💬 stickies-v commented on pull request "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs":
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2223866061)
```suggestion
- `unloadwallet` - Return RPC_INVALID_PARAMETER when both the RPC wallet endpoint
and wallet_name parameters are unspecified. Previously the RPC failed with a JSON
parsing error.
```

💬 ismaelsadeeq commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2223903289)

Makes sense, I think we can make the bounds not just an arbitrary number but tied it to the cores of the machine.

That will prevent footgoon whereby user will spawn more threads than the machine can handle leading to degraded performance due to lots of context switching.

✅ achow101 closed a pull request: "test: functional test for incomplete PSBT with additional signatures required"
(https://github.com/bitcoin/bitcoin/pull/33035)

💬 achow101 commented on pull request "test: functional test for incomplete PSBT with additional signatures required":
(https://github.com/bitcoin/bitcoin/pull/33035#issuecomment-3104989559)
What is the purpose of this test? There are already multiple tests that do exactly what you have added, there is nothing useful being added here.

Please do not use a LLM to write your code.

💬 achow101 commented on pull request "Enable `-natpmp` by default":
(https://github.com/bitcoin/bitcoin/pull/33004#issuecomment-3105012258)
Concept ACK

💬 achow101 commented on pull request "test: delete commented-out tests and add a test case in wallet_signer":
(https://github.com/bitcoin/bitcoin/pull/33020#issuecomment-3105022393)
ACK da318fe53fa954cc3eadd7c39e17eb3da7c3e09e

💬 achow101 commented on pull request "rpc: Fix internal bug in descriptorprocesspsbt when encountering invalid signatures":
(https://github.com/bitcoin/bitcoin/pull/33014#issuecomment-3105042497)
The `CHECK_NONFATAL` is correct. `complete == true` means that the PSBT is complete and can be finalized. The invariant here is that `complete == true` means `FinalizeAndExtract` must succeed.

BIP 174 states that a finalizer must only construct valid scriptSigs and witnesses. We encounter this error because we assume that if a PSBT is finalized, the scriptSigs and witnesses are valid so we are not performing the extra step of verifying the script. However, in this situation, the finalizer has
...

🚀 achow101 merged a pull request: "test: delete commented-out tests and add a test case in wallet_signer"
(https://github.com/bitcoin/bitcoin/pull/33020)

💬 sipa commented on pull request "Enable `-natpmp` by default":
(https://github.com/bitcoin/bitcoin/pull/33004#issuecomment-3105053907)
utACK

💬 achow101 commented on pull request "init: [gui] Avoid UB/crash in InitAndLoadChainstate":
(https://github.com/bitcoin/bitcoin/pull/32987#issuecomment-3105070165)
ACK fac90e5261b811739ada56e06ea793a12f9c2c3d

📝 l0rinc opened a pull request: "doc: remove dangling toc entries from `developer-notes.md`"
(https://github.com/bitcoin/bitcoin/pull/33040)
The table of content wasn't adjusted when the corresponding entries were removed in https://github.com/bitcoin/bitcoin/pull/32572, see:
* `Ignoring IDE/editor files`: https://github.com/bitcoin/bitcoin/commit/faf65f05312be7647f485f088ba00fef97f47bf4
* `Shebang`: https://github.com/bitcoin/bitcoin/commit/7777fb8bc749e18c178ef460b65219187e676128
* `Wallet`: https://github.com/bitcoin/bitcoin/commit/fa69c5b170f56d554fcb0d0887bd27f961fe3e74

💬 l0rinc commented on pull request "doc: Remove stale sections in dev notes":
(https://github.com/bitcoin/bitcoin/pull/32572#issuecomment-3105072573)
Please see the follow-up, removing the corresponding TOC entries: https://github.com/bitcoin/bitcoin/pull/33040

📝 achow101 opened a pull request: "wallet: Set minversion to FEATURE_LATEST during migration"
(https://github.com/bitcoin/bitcoin/pull/33041)
Although the minversion is not used by descriptor wallets, we should still update it to the latest version for consistency.

No test as any test requires very old versions of Bitcoin Core.

💬 achow101 commented on pull request "wallet: remove outdated `pszSkip` arg of database `Rewrite` func":
(https://github.com/bitcoin/bitcoin/pull/32990#issuecomment-3105112040)
ACK 2dfeb6668cb2e98e3dccf946af084e8a08e1fab5

🚀 achow101 merged a pull request: "wallet: remove outdated `pszSkip` arg of database `Rewrite` func"
(https://github.com/bitcoin/bitcoin/pull/32990)

💬 ajtowns commented on pull request "script: return verification flag responsible for error upon validation failure":
(https://github.com/bitcoin/bitcoin/pull/33012#issuecomment-3105269388)
> The behaviour already exists in a limited fashion. Even without this change, an attacker can trivially create such a transaction (as discussed in OP). Here is for instance a `p2p_invalid_tx.py` case which passes both with or without this PR:

Okay, but that just means the extra work isn't providing much value today either?

> I don't think this logic is actually meant to prevent an attacker from wasting resources anyways. I think it's rather to make sure we don't split the network when int
...