💬 marshallshen commented on pull request "test: functional test for incomplete PSBT with additional signatures required":
(https://github.com/bitcoin/bitcoin/pull/33035#issuecomment-3103097218)
@kevkevinpal - thanks! I will use it to ensure it does increase the test coverage.
@maflcko - I used LLM to help me understand the codebase and write parts of the test, my workflow is:
- understand structure of the TestFramework
- have a goal of what to test (in this case, PSBT is incomplete if some signature is missing)
- reusing the existing test artifacts, and try to write the test to match my goal above

Please let me know if there's anything I am missing as I'm still new to the codeba
...

👍 stickies-v approved a pull request: "Update secp256k1 subtree to latest master"
(https://github.com/bitcoin/bitcoin/pull/33036#pullrequestreview-3043431053)
> Yes that is what I say in the PR description.

Sorry, the link to the 0.7.0 release notes led me to believe the intention was to upgrade to the latest release instead of master, but I now understand (and as indicated in the PR title) that's not the usual update process used for secp256k1.

---

ACK 336b8be37b2260d8e902b93f1761265a0aefa496

Verified that the subtree pull matches https://github.com/bitcoin-core/secp256k1/commit/b9313c6e1a6082a66b4c75777e18ca4b176fcf9d, and briefly skimme
...

👍 theStack approved a pull request: "test: add test cases to wallet_signer.py"
(https://github.com/bitcoin/bitcoin/pull/33020#pullrequestreview-3043484317)
ACK da318fe53fa954cc3eadd7c39e17eb3da7c3e09e

nit: could update the PR title to also indicate that also (commented out) tests get removed

🤔 marcofleon reviewed a pull request: "[29.x] Backport #32521"
(https://github.com/bitcoin/bitcoin/pull/33013#pullrequestreview-3043558663)
ACK 716b4362c0b93dc666464088ed81742c4abf7732

Made sure the changes here match https://github.com/bitcoin/bitcoin/pull/32521. All unit and functional tests pass on 29.x.

💬 enirox001 commented on pull request "test: check tx is final when there is no locktime":
(https://github.com/bitcoin/bitcoin/pull/33030#issuecomment-3103441542)
ACK 065e429: Valuable test case that explicitly demonstrates `IsFinalTx` behavior when nLockTime is 0, including at genesis block height.

💬 Sjors commented on pull request "Update secp256k1 subtree to latest master":
(https://github.com/bitcoin/bitcoin/pull/33036#issuecomment-3103533914)
We have in the past updated to release tags, see e.g. #31216. Though that seems mostly a matter of how you phrase the commit title.

📝 frankomosh opened a pull request: "fuzz: add mempool_dag fuzzer for transaction dependency testing"
(https://github.com/bitcoin/bitcoin/pull/33038)
adds a new target, `mempool_dag`, to test mempool DAG invariants by constructing transaction chains with controlled parent/child dependencies. main focus areas include:

a. ancestor / descendant count and size limits (`-limitancestorcount,` `-limitancestorsize`, `-limitdescendantcount`, `-limitdescendantsize`)

b. TRUC-policy interaction with non-TRUC transactions

c. mempool eviction behaviour under shrinking `-maxmempool`

d. fee-rate consistency after partial package eviction

Inva
...

🤔 darosior reviewed a pull request: "Introduce per-txin sighash midstate cache for legacy/p2sh/segwitv0 scripts"
(https://github.com/bitcoin/bitcoin/pull/32473#pullrequestreview-3038560723)
I've been playing with this code for the past couple of days and this is looking good. I wrote a unit test which sanity checks the caching and notably exercises false positive cache hits, which appears to not currently be covered (see LINK): https://github.com/darosior/bitcoin/commit/f303c6d68b5b9b3b23af34c8ef9efa7822fbe38b. I ran IBD on mainnet and signet with caching for blocks enabled and it did not result in any historical block being rejected on either of these networks. I'll do testnet3 an
...

💬 darosior commented on pull request "Introduce per-txin sighash midstate cache for legacy/p2sh/segwitv0 scripts":
(https://github.com/bitcoin/bitcoin/pull/32473#discussion_r2219495270)
nit: no need to special-case when `kwargs` is empty since `ctx == {**ctx, **{}}`.
```suggestion
return lambda ctx: get({**ctx, **kwargs}, name)
```

💬 darosior commented on pull request "Introduce per-txin sighash midstate cache for legacy/p2sh/segwitv0 scripts":
(https://github.com/bitcoin/bitcoin/pull/32473#discussion_r2220018190)
nit: why is that needed? Looks like it may have been necessary for an earlier version of the test that had a `0` sighash type in the `ops` list for the legacy signature caching test, but it is not necessary anymore.

💬 darosior commented on pull request "Introduce per-txin sighash midstate cache for legacy/p2sh/segwitv0 scripts":
(https://github.com/bitcoin/bitcoin/pull/32473#discussion_r2220165555)
Various modifications of this code does not make any test fail. For instance:
```diff
diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
index 528d8851d5f..81fbc4ab151 100644
--- a/src/script/interpreter.cpp
+++ b/src/script/interpreter.cpp
@@ -1569,8 +1569,7 @@ int SigHashCache::CacheIndex(int32_t hash_type) const noexcept
// Note that we do not distinguish between BASE and WITNESS_V0 to determine the cache index,
// because no input can simultaneously use bo
...

💬 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#issuecomment-3103685291)
Better squash the last two commits into one.

💬 darosior commented on pull request "script: return verification flag responsible for error upon validation failure":
(https://github.com/bitcoin/bitcoin/pull/33012#discussion_r2223176014)
Well, apparently that was the only place. Fixed.

💬 maflcko commented on pull request "util: Abort on failing CHECK_NONFATAL in debug builds":
(https://github.com/bitcoin/bitcoin/pull/32588#discussion_r2223179634)
restored this, and extended the workaround in the fuzz test, and wrote unit tests.

💬 josibake commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#issuecomment-3103829182)
Looks like the most recent change is causing a failure in the unit tests:

> 2025-07-22T13:09:52.932304Z (mocktime: 2020-08-31T15:34:12Z) [test] [txmempool.cpp:699] [void CTxMemPool::check(const CCoinsViewCache &, int64_t) const] [mempwallet/spend.cpp:1343 CreateTransactionInternal: Assertion `!change_script.empty()' failed.

💬 achow101 commented on pull request "wallet, sqlite: Encapsulate SQLite statements in a RAII class":
(https://github.com/bitcoin/bitcoin/pull/33033#discussion_r2223248647)
Done

💬 Sjors commented on issue "guix: Zip file non-determinism when building in WSL":
(https://github.com/bitcoin/bitcoin/issues/32931#issuecomment-3103923012)
Maybe copy the build over and then zip it?

💬 pablomartin4btc commented on pull request "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs":
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2223286810)
Added!

💬 pablomartin4btc commented on pull request "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs":
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2223287249)
Done!

💬 pablomartin4btc commented on pull request "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs":
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2223288339)
Done!