💬 vasild commented on pull request "ci: run test_bitcoin with DEBUG_LOG_OUT in RUN_UNIT_TESTS_SEQUENTIAL":
(https://github.com/bitcoin/bitcoin/pull/28736#issuecomment-1893896190)
`2b7888ec3e...57e8bc6b31`: rebase due to conflicts

💬 furszy commented on issue "Use of a wallet shouldn't be blocked in prune mode ("wallet loading failed... beyond pruned data")":
(https://github.com/bitcoin/bitcoin/issues/27188#issuecomment-1893908170)
@shovas, I have created #29183 to focus our efforts and delineate the working path in the clearest possible manner. It should help pushing the project forward.

💬 vasild commented on pull request "Avoid returning references to mutex guarded members":
(https://github.com/bitcoin/bitcoin/pull/28774#issuecomment-1893912633)
@achow101, this is now solely in the wallet. Maybe you want to take a look?

👍 TheCharlatan approved a pull request: "depends: add NM output to gen_id"
(https://github.com/bitcoin/bitcoin/pull/29249#pullrequestreview-1823792580)
ACK a1db512f069039530b9fb70f8c004a4f25c3e257

Guix build (x86)
```
580df4b97c2e045d93e98c2bf2fbccad05117aa54594ec8849969ae18dd0d865 guix-build-a1db512f0690/output/aarch64-linux-gnu/SHA256SUMS.part
d8ee47f067d42f087f1c2f01d0613d57c3f887eb2be746498b674b27671c2678 guix-build-a1db512f0690/output/aarch64-linux-gnu/bitcoin-a1db512f0690-aarch64-linux-gnu-debug.tar.gz
77655b1a8946a131c5bd0c5083d9faeae83d08d9d718ac43a79d1872c98a27e0 guix-build-a1db512f0690/output/aarch64-linux-gnu/bitcoin-a1db51
...

💬 fanquake commented on pull request "build: depends move macOS C(XX) FLAGS out of C & CXX":
(https://github.com/bitcoin/bitcoin/pull/29233#issuecomment-1893982810)
Guix Build (aarch64):
```bash
94d2fe95101baf9ba74e5c1be6a93708bd55b3700567c768e72ceb4af4951bb9 guix-build-cbc9bf11fe84/output/aarch64-linux-gnu/SHA256SUMS.part
3cacc8c3d726e78b1db8ea07c25186e589f32c991cd7fd45d4390135adf199f2 guix-build-cbc9bf11fe84/output/aarch64-linux-gnu/bitcoin-cbc9bf11fe84-aarch64-linux-gnu-debug.tar.gz
39f42920ea452ea67f0f8df3199c50d73660009368fccfab900204d27b8e50cc guix-build-cbc9bf11fe84/output/aarch64-linux-gnu/bitcoin-cbc9bf11fe84-aarch64-linux-gnu.tar.gz
dc1a2b
...

🚀 fanquake merged a pull request: "contrib: add macho branch protection check"
(https://github.com/bitcoin/bitcoin/pull/29170)

💬 TheCharlatan commented on pull request "kernel: Remove key module from kernel library":
(https://github.com/bitcoin/bitcoin/pull/29252#issuecomment-1894004822)
Updated 67058d4a82e222f770ede96dc948f758b0a8386c -> 2cb38d93a8fea16bf84b072a90ac023ef345134d ([kernelRmKey_0](https://github.com/TheCharlatan/bitcoin/tree/kernelRmKey_0) -> [kernelRmKey_1](https://github.com/TheCharlatan/bitcoin/tree/kernelRmKey_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelRmKey_0..kernelRmKey_1))

* Implemented @maflcko's [suggestion](https://github.com/bitcoin/bitcoin/pull/29252#issuecomment-1893335214), calling `ECC_Stop` instead of relaxing the `ECC_
...

💬 maflcko commented on pull request "kernel: Remove key module from kernel library":
(https://github.com/bitcoin/bitcoin/pull/29252#discussion_r1453627000)
Removing this will lead to compile errors when a `unique_ptr` member is added with a fwd decl, no?

💬 maflcko commented on pull request "kernel: Remove key module from kernel library":
(https://github.com/bitcoin/bitcoin/pull/29252#discussion_r1453627918)
need to remove include as well, no?

💬 TheCharlatan commented on pull request "kernel: Remove key module from kernel library":
(https://github.com/bitcoin/bitcoin/pull/29252#discussion_r1453636097)
Yes.

💬 Sjors commented on pull request "build: depends move macOS C(XX) FLAGS out of C & CXX":
(https://github.com/bitcoin/bitcoin/pull/29233#issuecomment-1894038057)
I was able to make and run a depends build on Intel macOS 14.2.1 for cbc9bf11fe84deb96daf9b97a8e7499979360db2.

I'm also baking a Guix build.

💬 maflcko commented on pull request "kernel: Remove key module from kernel library":
(https://github.com/bitcoin/bitcoin/pull/29252#discussion_r1453639815)
Seems better to leave as-is, or remove the comment, if it is no longer accurate?

💬 maflcko commented on pull request "kernel: Remove key module from kernel library":
(https://github.com/bitcoin/bitcoin/pull/29252#issuecomment-1894042338)
lgtm ACK 2cb38d93a8fea16bf84b072a90ac023ef345134d 🍂

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: lgtm ACK 2cb38d93a8fea16b
...

💬 TheCharlatan commented on pull request "kernel: Remove key module from kernel library":
(https://github.com/bitcoin/bitcoin/pull/29252#discussion_r1453643762)
Will leave as is then. Seems likely that something will end up here.

👍 fanquake approved a pull request: "refactor: C++20: Use std::rotl"
(https://github.com/bitcoin/bitcoin/pull/29085#pullrequestreview-1824029448)
ACK 60446285436da62adef1c0a9b11c3336d82b4d89

Had a look at the difference in master vs this PR (rebased) on aarch64. Seems like for the most part, this gives the same output +- a handful of instructions. (Identical is identical minus difference in addresses / argument ordering etc).

| Function | GCC 13.2 | Clang 17.0.6 |
| --- | --- | --- |
| MurmurHash3 | Identical | PR gives 5 less instructions: https://www.diffchecker.com/Jmlpl4rG/ |
| ChaCha20Aligned::Keystream | PR gives 1 less in
...

💬 sipa commented on pull request "refactor: C++20: Use std::rotl":
(https://github.com/bitcoin/bitcoin/pull/29085#issuecomment-1894084086)
@fanquake Which side of the diff is which version?

💬 fanquake commented on pull request "refactor: C++20: Use std::rotl":
(https://github.com/bitcoin/bitcoin/pull/29085#issuecomment-1894088819)
> Which side of the diff is which version?

Left is master @ f1fcc9638cde7664b9642018fe6872148bbb0172 , right was the PR based on f1fcc9638cde7664b9642018fe6872148bbb0172.

💬 murchandamus commented on pull request "test: Add algo assert to bnb_search_test":
(https://github.com/bitcoin/bitcoin/pull/29206#issuecomment-1894112305)
In the test case the `9` is pre-selected and therefore has to be used. This is why BnB returns 9+1 instead of 10.

💬 TheCharlatan commented on pull request "kernel: Remove key module from kernel library":
(https://github.com/bitcoin/bitcoin/pull/29252#discussion_r1453696973)
clang-tidy doesn't like it being left there. Its suggestion of defaulting it in the header seems counter to what we want to achieve here.
```
/home/drgrid/bitcoin/src/./kernel/context.h:22:5: error: class 'Context' can be made trivially destructible by defaulting the destructor on its first declaration [performance-trivially-destructible,-warnings-as-errors]
~Context();
^
= default
kernel/context.cpp:23:10: note: destructor definition is here
Context::~Context() = d
...

🤔 murchandamus reviewed a pull request: "test: Add algo assert to bnb_search_test"
(https://github.com/bitcoin/bitcoin/pull/29206#pullrequestreview-1824147084)
Concept NACK

Tests should ensure that the expected outcome is achieved. The expected outcome here is that the coin selection process returns the input set that has the best waste score.

I don’t think it’s useful to require that the best input set was produced by BnB. While in our current implementation the best input set is guaranteed to be found by BnB, it is possible that future coin selection improvements would enable another algorithm to produce the same result. As such, requiring tha
...