💬 rkrux commented on pull request "test: add test for decoding PSBT with MuSig2 PSBT key types (BIP 373)":
(https://github.com/bitcoin/bitcoin/pull/32305#discussion_r2052392744)
Nit: Maybe consider putting this in a separate subtest as the current `run_test` is quite long already.

💬 rkrux commented on pull request "test: add test for decoding PSBT with MuSig2 PSBT key types (BIP 373)":
(https://github.com/bitcoin/bitcoin/pull/32305#discussion_r2052395285)
Fine for now but maybe we can consider making an `assert_musig2_fields()` later when more musig2 specific tests are added - if needed though.

💬 instagibbs commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#issuecomment-2818435940)
reACK d64bd31f9b6903f0a335a78b519d0b52e84cca1b

Simple rebase on top of stability fixes.

`git range-diff master 27a0c93abb7e70b93214eb857e2046f848139e68 d64bd31f9b6903f0a335a78b519d0b52e84cca1b`

💬 Sjors commented on pull request "kernel: Separate UTXO set access from validation functions":
(https://github.com/bitcoin/bitcoin/pull/32317#discussion_r2052401069)
2be3c59151bcb840eb8cbd045a3dada6775ebc9e: this or the final commit would be a good time to document both `SpendBlock` and `ConnectBlock`.

💬 saikiran57 commented on issue "signrawtransactionwithkey command shouldn't output the "Witness program was passed an empty witness" error for a TapRoot transaction":
(https://github.com/bitcoin/bitcoin/issues/27017#issuecomment-2818446195)
Hi @GregTonoski
Example:
`./bitcoin-cli.exe -testnet signrawtransactionwithkey 020000000165ef750aac862b0177cadb77961bf1a936e2bec0376b286f5b4e1b6255cf3a960000000000fdffffff012c1a0000000000002251203b82b2b2a9185315da6f80da5f06d0440d8a5e1457fa93387c2d919c86ec878600000000 '["cV628xvqToz45dwdPmTcJ9RgEVnWMwP8dpZBGzb9LfTk3sBHFNwc"]' '[{"txid":"963acf55621b4e5b6f286b37c0bee236a9f11b9677dbca77012b86ac0a75ef65","vout":0,"scriptPubKey":"512055355ca83c973f1d97ce0e3843c85d78905af16b4dc531bc488e57212d230116",
...

💬 GregTonoski commented on issue "signrawtransactionwithkey command shouldn't output the "Witness program was passed an empty witness" error for a TapRoot transaction":
(https://github.com/bitcoin/bitcoin/issues/27017#issuecomment-2818504035)
Is it related to the bug bitcoin#31589, perhaps?

💬 glozow commented on pull request "tests: improves tapscript unit tests":
(https://github.com/bitcoin/bitcoin/pull/31640#issuecomment-2818509467)
I only see 1 ack, ping @instagibbs @0xBEEFCAF3 for re-review

💬 instagibbs commented on pull request "tests: improves tapscript unit tests":
(https://github.com/bitcoin/bitcoin/pull/31640#issuecomment-2818539349)
reACK 62b6071fd6ac2aad2fc42f135b3df662ab0eee7a

removed the assert in favor of the single BOOST_ERROR check

✅ maflcko closed a pull request: "docs: remove passing CI section in guidelines for PR merging as it's common sense"
(https://github.com/bitcoin/bitcoin/pull/32006)

💬 maflcko commented on pull request "docs: remove passing CI section in guidelines for PR merging as it's common sense":
(https://github.com/bitcoin/bitcoin/pull/32006#issuecomment-2818560151)
lgtm ACK 17a1630f226e1c8eb13eb7c7425290e933ad9368

Makes sense to remove this, because it is common sense, and also redundant with the readme sections https://github.com/bitcoin/bitcoin/blob/master/README.md#automated-testing and https://github.com/bitcoin/bitcoin/blob/master/README.md#manual-quality-assurance-qa-testing.

📝 maflcko reopened a pull request: "docs: remove passing CI section in guidelines for PR merging as it's common sense"
(https://github.com/bitcoin/bitcoin/pull/32006)
Removing passing CI section in guidelines for PR merging as it's common sens

💬 saikiran57 commented on issue "signrawtransactionwithkey command shouldn't output the "Witness program was passed an empty witness" error for a TapRoot transaction":
(https://github.com/bitcoin/bitcoin/issues/27017#issuecomment-2818575632)
Its not related to bug https://github.com/bitcoin/bitcoin/issues/31589,

you can see this line https://github.com/bitcoin/bitcoin/blob/master/src/script/sign.cpp#L477 it is consider the XOnlyPubKey{vSolutions[0]} instead of tweaked public key. So that's why when signing https://github.com/bitcoin/bitcoin/blob/master/src/script/sign.cpp#L67 it returns false because output is locked to tweaked public key where as we are searching for x-only-public key.

👋 mzumsande's pull request is ready for review: "test: allow all functional tests to be run or skipped with --usecli"
(https://github.com/bitcoin/bitcoin/pull/32290)

💬 sipa commented on pull request "tests: improves tapscript unit tests":
(https://github.com/bitcoin/bitcoin/pull/31640#issuecomment-2818686077)
Concept & Approach ACK

Code nits:
```diff
--- a/src/test/script_tests.cpp
+++ b/src/test/script_tests.cpp
@@ -930,11 +930,11 @@ BOOST_AUTO_TEST_CASE(script_json_test)
auto element = test[pos][i].get_str();
// We use #SCRIPT# to flag a non-hex script that we can read using ParseScript
// Taproot script must be third from the last element in witness stack
- std::string scriptFlag = std::string("#SCRIPT#");
-
...

💬 EniRox001 commented on pull request "test: Fix feature_pruning test after nTime typo fix":
(https://github.com/bitcoin/bitcoin/pull/32312#issuecomment-2818701974)
> Code change looks good to me. The commit message doesn't seem to say anything about the typo fix though. It should be mentioned there and that the rest of the change is fixing a secondary issue caused by this. You could also split this in two separate commits.

Thanks for the review! I've split the changes into two logical commits as suggested:

1. First commit fixes the nTimes -> nTime typo and explains its impact on the test behavior
2. Second commit addresses the test failure that was
...

📝 VolodymyrBg opened a pull request: "Fix failing util_time_GetTime test on Windows"
(https://github.com/bitcoin/bitcoin/pull/32318)
Remove unreliable steady clock time checking from the test that was causing CI failures primarily on Windows. The test previously tried to verify that steady_clock time increases after a 1ms sleep, but this approach is not reliable on all platforms where such a short sleep interval may not consistently result in observable clock changes.

This addresses issue #32197 where the test was reporting failures in the cross-built Windows CI environment. As noted in the discussion, the test is not cr
...

👍 ryanofsky approved a pull request: "doc: Add deps install notes for multiprocess"
(https://github.com/bitcoin/bitcoin/pull/32293#pullrequestreview-2781680324)
Code review ACK 7f5a35cf4b31f176ff9138d21f3ec677ae7d1bcf just dropping dependencies.md update since last review

💬 romanz commented on pull request "Replace libevent with our own HTTP and socket-handling implementation":
(https://github.com/bitcoin/bitcoin/pull/32061#discussion_r2052036849)
nit: consider using `string_view` instead of `string` here and below.

💬 1440000bytes commented on pull request "rpc: Allow fullrbf fee bump in (psbt)bumpfee":
(https://github.com/bitcoin/bitcoin/pull/31953#issuecomment-2818913559)
reACK https://github.com/bitcoin/bitcoin/commit/fa86190e6ed2aeda7bcceaf96f52403816bcd751

👍 TheCharlatan approved a pull request: "test: test MAX_SCRIPT_SIZE for block validity"
(https://github.com/bitcoin/bitcoin/pull/32304#pullrequestreview-2781694117)
ACK b1ea542ae651cec433910d8c727abc41f17a7678