🤔 rkrux reviewed a pull request: "tests: fix `OP_1NEGATE` handling in `CScriptOp`"
(https://github.com/bitcoin/bitcoin/pull/29589#pullrequestreview-1986101030)
1. Make is successful
2. All functional tests pass

Would it be possible to share where this error pops up in the branch specified in the description?
> "When we don't encode -1 as OP_1NEGATE then we end up getting errors inside of CheckMinimalPush() saying -1 was not minimally encoded."

👍 dergoegge approved a pull request: "refactor: Use typesafe Wtxid in compact block encodings"
(https://github.com/bitcoin/bitcoin/pull/29752#pullrequestreview-1986159858)
ACK a8203e94123b6ea6e4f4a6320e3ad20457f44a28

💬 sr-gi commented on pull request "test: Cleans some manual checks/drops when using wait_for_getdata":
(https://github.com/bitcoin/bitcoin/pull/29748#issuecomment-2042562089)
> Concept ACK, nit: Could you also take a look at `announce_tx_and_wait_for_getdata` in `p2p_segwit.py` for a similar cleanup?

Unfortunately, that's not the case given how the tests using `announce_tx_and_wait_for_getdata` are built. That is, the same transaction may be sent multiple times using `announce_tx_and_wait_for_getdata`, so you need to potentially pop the old one to make sure the assert is not using old data.

The only way of working around this would be making `wait_for_getdata`
...

💬 sipa commented on pull request "Enable HW-accelerated implementations of SHA256 for MSVC builds":
(https://github.com/bitcoin/bitcoin/pull/24773#discussion_r1555701766)
@theuni I think what is going on is that before this PR, `HAVE_GETCPUID` wasn't defined for MSVC builds, but with this PR, there now is such a feature. However, this PR still doesn't enable `rdrand`/`rdseed` support for Windows (it would also need intrinsics versions), so this define conditional is needed to exclude support for that despite having getcpuid.

🤔 murchandamus reviewed a pull request: "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure"
(https://github.com/bitcoin/bitcoin/pull/29735#pullrequestreview-1986238385)
ACK 052c67d4beb72d3fdf31fa5e584ea2fa12e6f69c

Please feel free to ignore nits

💬 murchandamus commented on pull request "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure":
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1555720689)
Nit: I don’t feel strongly about it, but the change that made me notice that there were code changes in the move originally was that `COINBASE_MATURITY - 1` had gotten replaced with a `99` magic number.

💬 murchandamus commented on pull request "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure":
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1555727656)
Nit: Subject line in commit message should not exceed 50 characters, and definitely not be more than 72: https://cbea.ms/git-commit/#limit-50

🤔 ismaelsadeeq reviewed a pull request: "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure"
(https://github.com/bitcoin/bitcoin/pull/29735#pullrequestreview-1986192988)
Concept ACK


I will run the new fuzz test with and without 72425bd699dd33d57794b3339f6266c4c9df443d

💬 ismaelsadeeq commented on pull request "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure":
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1555729721)
Maybe assert for this at the beginning of the `fill_mempool`
```python
assert_equal(node.getnetworkinfo()['relayfee'], Decimal('0.00001000'))
```

💬 ismaelsadeeq commented on pull request "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure":
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1555723093)
In c93f790cad1558dcf0b8fb5fd0cf3a276368d656 " Move fill_mempool to util function"

There should be a commit that will first pull out these two lines from this scope to
https://github.com/bitcoin/bitcoin/blob/f0794cbd405636a7f528a60f2873050b865cf7e8/test/functional/mempool_limit.py#L231

Moving `fill_mempool` to `test_framework/util.py` makes it clear that the comments do not belong to this scope.

💬 ismaelsadeeq commented on pull request "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure":
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1555694194)
In a466686c6e97d47e3f4718eed0f781acb88da7fa "test: expand docstring to fill_mempool"

nit:
```suggestion
Allows for simpler testing of scenarios with floating mempoolminfee > minrelayfee
```

💬 laanwj commented on pull request "tracing: Only prepare tracepoint arguments when actually tracing":
(https://github.com/bitcoin/bitcoin/pull/26593#discussion_r1555738343)
Attaching by pid is a good idea, even aside from semaphores, as it attaches a specific instance instead of all processes sharing the binary, which i think it does if you provide the path.

💬 brunoerg commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1555757479)
Also, maybe we do not need this verification anymore?
```py
if res["success"]:
self.log.info(f"Added {addr} to tx_originator's addrman")
else:
self.log.info(f"Could not add {addr} to tx_originator's addrman (collision?)")
```

💬 cbergqvist commented on pull request "test: Bump timeouts in feature_index_prune and wallet_importdescriptors":
(https://github.com/bitcoin/bitcoin/pull/29791#issuecomment-2042641779)
Happy for my first merged PR! :)

## Post-merge post-mortem

### Working stupidly, not smart or hard

I have spent too many evenings focusing on this:
> Agree that it might be fruitful to investigate the history of wallet_importdescriptors.py to pinpoint regressions, didn't have time today.

In over >110 runs, I have observed how the chance of `wallet_importdescriptors.py --descriptors` failure has increased with time. As patterns emerged around certain commits, I've tested further only
...

💬 0xB10C commented on issue "Continuous benchmark tracking":
(https://github.com/bitcoin/bitcoin/issues/27284#issuecomment-2042651718)
I just learned that nanobench is able to fill in an [output format template](https://nanobench.ankerl.com/tutorial.html#rendering-mustache-like-templates). It might make sense to try that route first.

👍 theuni approved a pull request: "depends: add new LLVM debug macro"
(https://github.com/bitcoin/bitcoin/pull/29781#pullrequestreview-1986389340)
ACK 5efebc0edbb479d2041b3fb2d43be3a77e817b3e

💬 laanwj commented on pull request "doc: 25.2 historical release notes":
(https://github.com/bitcoin/bitcoin/pull/29830#issuecomment-2042709408)
ACK 93bd2e2f6c9672fbf1120d1e25349f6edd29cfef
Matches the file on tag `v25.2`.

💬 cbergqvist commented on pull request "p2p: gives seednode priority over dnsseed if both are provided":
(https://github.com/bitcoin/bitcoin/pull/28016#issuecomment-2042723373)
ACK 2842e51a246b162a586941184b7694f187d7aee7

Diffed top 2 commits in that and 78482a09e06beb841e70781eb509c2b2fdea8bd9 which I previously tested & acked. Only scope of `start`-variable was narrowed as suggested by @davidgumberg in https://github.com/bitcoin/bitcoin/pull/28016#discussion_r1526788396.

💬 instagibbs commented on pull request "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure":
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1555830543)
Made it shorter :)

💬 instagibbs commented on pull request "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure":
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1555831622)
bit of an annoying circular reference issue(blocktools requiring utils) so for now at least make it grep-able what the magic value should be(and more greppable)