💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1643278127)
wrote a better boundary test in this vein in follow-up, thanks. You actually need incremental added, not 1 sat.

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1643278301)
took as inspiration to make an actual boundary-condition test in follow-up, thanks

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#issuecomment-2174159709)
opened followup at https://github.com/bitcoin/bitcoin/pull/30295

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1643279044)
done in follow-up

💬 achow101 commented on pull request "test: write functional test results to csv":
(https://github.com/bitcoin/bitcoin/pull/30291#issuecomment-2174186974)
ACK ad06e68399da71c615db0dbf5304d0cd46bc1f40

💬 achow101 commented on pull request "test: Validate oversized transactions or without inputs":
(https://github.com/bitcoin/bitcoin/pull/29862#discussion_r1643298389)
In 0aa7db42564408edb41b0d42103d39ba4c2787dc "Validate oversized transaction"

I think it would be more interesting for this test to be on the threshold exactly. The rest of the tx is 70 bytes, so `MAX_BLOCK_WEIGHT / WITNESS_SCALE_FACTOR - 70` should pass `CheckTransaction`, while `- 69` does not.

🚀 achow101 merged a pull request: "test: write functional test results to csv"
(https://github.com/bitcoin/bitcoin/pull/30291)

💬 achow101 commented on pull request "test: Added test coverage to listsinceblock rpc":
(https://github.com/bitcoin/bitcoin/pull/30195#issuecomment-2174217522)
ACK 881724d443d11f984a721ef1edd5777c24d1ed29

💬 achow101 commented on pull request "test: fix `keys_to_multisig_script` (P2MS) helper for n/k > 16":
(https://github.com/bitcoin/bitcoin/pull/28312#issuecomment-2174231098)
ACK 5cf0a1f230389ef37e0ff65de5fc98394f32f60c

💬 achow101 commented on pull request "test: Added test coverage to listsinceblock rpc":
(https://github.com/bitcoin/bitcoin/pull/30195#discussion_r1643329094)
There is a reorg happening, nodes 0 and 1 mine 6 blocks on top of the common ancestor, while 2 and 3 mine 7 blocks. When the network is rejoined, the 7 block branch reorgs the 6 block branch. However, `nodes1_last_blockhash` is the tip of the 6 block branch which is not in the main chain.

🚀 achow101 merged a pull request: "test: fix `keys_to_multisig_script` (P2MS) helper for n/k > 16"
(https://github.com/bitcoin/bitcoin/pull/28312)

💬 achow101 commented on pull request "ci: move ASan job to GitHub Actions from Cirrus CI":
(https://github.com/bitcoin/bitcoin/pull/30193#issuecomment-2174273029)
ACK 9eea51d9058ad638861aa4b94c1c6e71caeb8765

🚀 achow101 merged a pull request: "test: Added test coverage to listsinceblock rpc"
(https://github.com/bitcoin/bitcoin/pull/30195)

🚀 achow101 merged a pull request: "ci: move ASan job to GitHub Actions from Cirrus CI"
(https://github.com/bitcoin/bitcoin/pull/30193)

💬 paplorinc commented on pull request "test: Validate oversized transactions or without inputs":
(https://github.com/bitcoin/bitcoin/pull/29862#discussion_r1643367751)
I wanted a value that's definitely oversized, to avoid being close to the threshold - but I think I managed to make it work, thanks for the hint, let me know if you'd like me to reduce the duplication in what I've pushed.

💬 achow101 commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#issuecomment-2174342443)
ACK 0e0c422aedd4009ab34eca127e4904d15e81f5be

Not as familiar with locking in their area, so not super confident on the `cs_main` related changes, but the rest (removing other mutexes and consolidating under `m_tx_download_mutex`) looks fine.

💬 rkrux commented on pull request "test: Added test coverage to listsinceblock rpc":
(https://github.com/bitcoin/bitcoin/pull/30195#discussion_r1643385636)
Understood now!
Thanks @achow101, this is very helpful.

💬 AngusP commented on pull request "test: Add Compact Block Encoding test `ReceiveWithExtraTransactions` covering non-empty `extra_txn`":
(https://github.com/bitcoin/bitcoin/pull/30237#discussion_r1643388352)
I think that should be OK, assuming it doesn't happen to hit a case where there's a short-ID collision

💬 AngusP commented on pull request "test: Add Compact Block Encoding test `ReceiveWithExtraTransactions` covering non-empty `extra_txn`":
(https://github.com/bitcoin/bitcoin/pull/30237#discussion_r1643388594)
Sure, seems sensible to me

👍 rkrux approved a pull request: "test: fix MiniWallet script-path spend (missing parity bit in leaf version)"
(https://github.com/bitcoin/bitcoin/pull/30076#pullrequestreview-2123842586)
reACK [e4b0dab](https://github.com/bitcoin/bitcoin/pull/30076/commits/e4b0dabb2115dc74e9c5794ddca3822cd8301c72)

Applied the shared patch again and the following tests fail on master with the mentioned error. Though I've not been able to go through the corresponding control block C++ code yet :(

```
raise JSONRPCException(response['error'], status)
test_framework.authproxy.JSONRPCException: mandatory-script-verify-flag-failed (Witness program hash mismatch) (-26)
```

```
mempool_limi
...