💬 maflcko commented on pull request "tracing: Only prepare tracepoint arguments when actually tracing":
(https://github.com/bitcoin/bitcoin/pull/26593#discussion_r1559292092)
Thanks for the explanation. I don't see another solution right now.

💬 ajtowns commented on pull request "Feature: Use different datadirs for different signets":
(https://github.com/bitcoin/bitcoin/pull/29838#discussion_r1557976175)
I guess the hash160 will better guarantee uniqueness, but a 40 character string is kind-of long? Would it make sense to use the 8 character `HexStr(Params().MessageStart())` value instead, special-casing for when the message start magic number matches the default signet's `0a03cf40`?

💬 maflcko commented on pull request "build: Enable fuzz binary in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29774#discussion_r1559309031)
nit in 0737f4ea62dd85fe7e8914bb82086a2628ae8204: Would be nice to add the error message to the commit message

📝 fanquake opened a pull request: "guix: replace GCC unaligned VMOV patch with binutils patch"
(https://github.com/bitcoin/bitcoin/pull/29846)
Rather than invasively patching GCC, given we have binutils 2.38 available, we can patch it to flip the default for
`-muse-unaligned-vector-move`.

A 1 line binutils patch, is much more maintainable than the ~300 line patch into GCC. It's also a slight inprovement in regards to patching out ualigned instructions in the release binaries. For comparison:
Master:
```bash
objdump -D bin/*.exe | rg "vmova|vmovdqa|vmovaps|vmovapd|vmovdqa64|vmovdqa32"
141b8be20: c5 f8 28 1a vmo
...

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1559354990)
Woops yes will fix

💬 torkelrogstad commented on pull request "rpc: validate fee estimation mode case insensitive":
(https://github.com/bitcoin/bitcoin/pull/29175#issuecomment-2047432612)
It's a fair point to not want to introduce new instances of undesirable behavior. But I **definitely** don't think we should change existing behavior. That would break ALL users that use Go for interfacing with Bitcoin Core, and maybe other languages as well

📝 paplorinc opened a pull request: "refactor: Simplify base32/64 padding calculations"
(https://github.com/bitcoin/bitcoin/pull/29847)
Instead of modifying the string view by removing the suffixes manually, we trim the `ConvertBits` iteration instead by counting the trailing "=" chars.

Also added roundtrip tests for safety.

💬 willcl-ark commented on pull request "guix: replace GCC unaligned VMOV patch with binutils patch":
(https://github.com/bitcoin/bitcoin/pull/29846#issuecomment-2047459040)
Will this close #28413 ?

📝 paplorinc converted_to_draft a pull request: "refactor: Simplify base32/64 padding calculations"
(https://github.com/bitcoin/bitcoin/pull/29847)
Instead of modifying the string view by removing the suffixes manually, we trim the `ConvertBits` iteration instead by counting the trailing "=" chars.

Also added roundtrip tests for safety.

💬 RandyMcMillan commented on pull request "Feature: Use different datadirs for different signets":
(https://github.com/bitcoin/bitcoin/pull/29838#issuecomment-2047502844)
Concept ACK

💬 earonesty commented on pull request "Implement OP_CHECKTEMPLATEVERIFY":
(https://github.com/bitcoin/bitcoin/pull/29280#issuecomment-2047506633)
Lgtm. It's the simplest and least controversial and has an extremely broad use case set (not perfect for anything but useful for everything). No need to bikeshed if it's the best. It's useful and we need more useful things.

💬 RandyMcMillan commented on pull request "Feature: Use different datadirs for different signets":
(https://github.com/bitcoin/bitcoin/pull/29838#issuecomment-2047513627)
Make sure this PR is based on a passing (CI) commit.

💬 epompeii commented on issue "Continuous benchmark tracking":
(https://github.com/bitcoin/bitcoin/issues/27284#issuecomment-2047551283)
> A sample dashboard is [here](https://bencher.dev/console/projects/0xb10c-s-bitcoin-core/perf?key=true&reports_per_page=4&branches_per_page=8&testbeds_per_page=8&benchmarks_per_page=8&reports_page=1&branches_page=1&testbeds_page=1&benchmarks_page=1&report=915c1007-33dd-4d30-b9c1-531b60af8a06&branches=0ab6caf7-716b-4a6e-8101-f2b9c891a9a2&testbeds=3c0c4d9d-b40e-4d9a-a462-2379355b0785&benchmarks=993b5444-b391-43dd-a4c1-c157cb1d7209%2Cb5ce5b17-6755-449b-b205-b843a0252344%2Cd1b7da67-b22c-4d30-b946-b
...

✅ achow101 closed an issue: "Can't version 26.1 export Bech32 address private key?"
(https://github.com/bitcoin/bitcoin/issues/29836)

💬 sr-gi commented on pull request "test: Makes `wait_for_getdata` delete data on checks, plus allows to check the getdata message type":
(https://github.com/bitcoin/bitcoin/pull/29748#issuecomment-2047613487)
> Regarding this comment, I tried this refactor and the tests pass.

Thanks! I think I may have been locking the `p2p_lock` twice which made test timeout.

💬 sr-gi commented on pull request "test: Extends wait_for_getheaders so a specific block hash can be checked":
(https://github.com/bitcoin/bitcoin/pull/29736#issuecomment-2047665322)
I wonder if it may be worth splitting the `wait_for_getheaders` function so the internal function can also be used externally, so we can use that to check for whether a certain message exists without waiting for them (e.g. checking for the negative case) in a similar fashion as done in bc844fd8a7d130bfb7cf598f5b1acd87acd70e58

This way we would get rid of a bunch of manual checks like:

```python
with p2p_lock:
assert "getheaders" not in peer.last_message
```

cc/ @maflcko @stratosp
...

👍 BrandonOdiwuor approved a pull request: "test: Extends wait_for_getheaders so a specific block hash can be checked"
(https://github.com/bitcoin/bitcoin/pull/29736#pullrequestreview-1991823634)
crACK c4f857cc301d856f3c60acbe6271d3fe19441c7a

💬 murchandamus commented on pull request "[DO NOT MERGE] testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2047673316)
Since some people consider the blockstorms an interesting feature of Testnet3, it might be interesting to only raise the difficulty of the delayed block exception to 100,000 instead of 1,000,000. This would allow the network to return to the organic difficulty in fewer difficulty periods and slow down the blockstorms but not remove the feature altogether. My understanding is that this would correspond roughly a tenth of one S9 mining on the network, so if no one had mined for a while, a single S
...

💬 maflcko commented on pull request "rpc: validate fee estimation mode case insensitive":
(https://github.com/bitcoin/bitcoin/pull/29175#issuecomment-2047676950)
This is probably more than you wanted to do, but I'd say it would be better to use `FeeModeFromString` to re-use the existing (case-insensitive) parsing code, than to re-implement the parsing and use hard-coded "unset" strings in all call places.

💬 rkrux commented on pull request "test: Makes `wait_for_getdata` delete data on checks, plus allows to check the getdata message type":
(https://github.com/bitcoin/bitcoin/pull/29748#discussion_r1559533843)
Note: This will pop from last message via `check_last_getdata()` call as well, which is different from previous commits. That's why I accepted `last_data` as an argument in my suggestion.
Is popping in `check_last_getdata` also fine?