💬 kevkevinpal commented on pull request "test: adds coverage to src/validation for invalid tx coinbase":
(https://github.com/bitcoin/bitcoin/pull/32253#issuecomment-2818358588)
> I don't think there is any use in keeping this open? The test is strictly redundant, as Marco demonstrated it's already covered both in unit and functional tests.

Sounds I can go ahead and close it

✅ kevkevinpal closed a pull request: "test: adds coverage to src/validation for invalid tx coinbase"
(https://github.com/bitcoin/bitcoin/pull/32253)

📝 TheCharlatan opened a pull request: "kernel: Seperate UTXO set access from validation functions"
(https://github.com/bitcoin/bitcoin/pull/32317)
The current code makes it hard to call functions for checking the consensus-correctness of a block without having a full UTXO set at hand. This makes implementing features such as Utreexo and non-assumevalid swiftsync, where maintaining a full UTXO set defeats their purpose, through the kernel API difficult. Checks like the coinbase subsidy, or block sigops are only available through `ConnectBlock`, which requires a complete coins cache.

Solve this by moving accessing coins and the responsibi
...

💬 pinheadmz 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-2818394106)
NACK, it may not be common sense for new contributors. Also the tip about adding passing tests before code changes in commit history is useful. I'm not gonna push back if others really want to merge this change, it just doesn't seem useful.

💬 EthanHeilman commented on pull request "tests: improves tapscript unit tests":
(https://github.com/bitcoin/bitcoin/pull/31640#issuecomment-2818410640)
Any reason this hasn't been merged despite 2 approvals?

💬 instagibbs commented on pull request "test: test MAX_SCRIPT_SIZE for block validity":
(https://github.com/bitcoin/bitcoin/pull/32304#issuecomment-2818420304)
@Sjors It doesn't apply because the first clause of the `IsUnspendable` check is if it starts with an OP_RETURN.

💬 andrewtoth commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2052396373)
nit: could be `const` method.

💬 instagibbs commented on pull request "test: test MAX_SCRIPT_SIZE for block validity":
(https://github.com/bitcoin/bitcoin/pull/32304#discussion_r2052398185)
Pushed a change to use `MAX_SCRIPT_ELEMENT_SIZE`, but I feel that the code with asserts is already a good explanation?

👍 rkrux approved a pull request: "test: add test for decoding PSBT with MuSig2 PSBT key types (BIP 373)"
(https://github.com/bitcoin/bitcoin/pull/32305#pullrequestreview-2781251768)
tACK 23ee07dd0f81930f46d2f4910cc92cb0eab61cc3

I believe this is a value add as it sets the base for the MuSig2 functional testing. I left few minor suggestions, I would be quick to re-ack if they are implemented.

💬 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_r2052391023)
Since this is a new check that has been added & the current use case is of a list of only bytes (in participant pubkeys), maybe we can assert this on all instead of any to keep the checking strict to start with that can be eased in the future if needed?

💬 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_r2052391602)
Are asserts like on this line really needed? If the decoded psbt is malformed then the test would fail in the following assert anyway.

💬 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)