💬 ryanofsky commented on pull request "kernel: Streamline util library":
(https://github.com/bitcoin/bitcoin/pull/29015#issuecomment-1847634783)
Updated 778c0214d25e5a012c61251d592257ae19805255 -> 8b21f41335dc837b36ea863156605d092f47a0ab ([`pr/rmutil.2`](https://github.com/ryanofsky/bitcoin/commits/pr/rmutil.2) -> [`pr/rmutil.3`](https://github.com/ryanofsky/bitcoin/commits/pr/rmutil.3), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/rmutil.2..pr/rmutil.3)) to fix clang-tidy error https://cirrus-ci.com/task/4602567305461760. Also switched to `using` declarations more places to avoid creating conflicts and make the PR easier to
...

💬 hebasto commented on pull request "[POC] C++20 `std::endian`":
(https://github.com/bitcoin/bitcoin/pull/28674#issuecomment-1847650626)
@theuni

https://github.com/bitcoin/bitcoin/blob/3e691258d8789a4a89cce42e7e71b130491594d7/build_msvc/bitcoin_config.h.in#L69

💬 LarryRuane commented on pull request "test: test_bitcoin: allow -testdatadir=<datadir>":
(https://github.com/bitcoin/bitcoin/pull/26564#issuecomment-1847650629)
Force pushed to remove the fix to `doc/benchmarking.md` as suggested by https://github.com/bitcoin/bitcoin/pull/26564#discussion_r1420176846

💬 theuni commented on pull request "[POC] C++20 `std::endian`":
(https://github.com/bitcoin/bitcoin/pull/28674#issuecomment-1847657957)
> @theuni
>
> https://github.com/bitcoin/bitcoin/blob/3e691258d8789a4a89cce42e7e71b130491594d7/build_msvc/bitcoin_config.h.in#L69

Thanks! https://github.com/theuni/bitcoin/commit/bbc72033128b6496dd3a92432eadbc4e75aaffe2 ready for PR if it indeed provides a speedup. I don't see how it couldn't :)

💬 josibake commented on pull request "Silent Payments: Implement BIP352":
(https://github.com/bitcoin/bitcoin/pull/28122#issuecomment-1847658969)
rebased on master and updated to fix a silent merge conflict in the tests, related to the new `Txid` type.

📝 theuni opened a pull request: "Add missing byteswap functions for MSVC"
(https://github.com/bitcoin/bitcoin/pull/29036)
This should give a speedup across the board for MSVC builds.

While working on modernizing our byteswapping code for c++20, we noticed that MSVC uses our hand-written byteswap functions, as opposed to using libc/compiler versions like almost all other platforms.

@aureleoules did some great benchmarks in #28674 which show that these hand-written byteswaps often compile down to a slow mess.
@hebasto confirmed that we're indeed hitting these paths for MSVC.

Quick tests with godbolt show th
...

📝 murchandamus opened a pull request: "Add multiplication operator to CFeeRate"
(https://github.com/bitcoin/bitcoin/pull/29037)

📝 theuni converted_to_draft a pull request: "Add missing byteswap functions for MSVC"
(https://github.com/bitcoin/bitcoin/pull/29036)
This should give a speedup across the board for MSVC builds.

While working on modernizing our byteswapping code for c++20, we noticed that MSVC uses our hand-written byteswap functions, as opposed to using libc/compiler versions like almost all other platforms.

@aureleoules did some great benchmarks in #28674 which show that these hand-written byteswaps often compile down to a slow mess.
@hebasto confirmed that we're indeed hitting these paths for MSVC.

Quick tests with godbolt show th
...

💬 kevkevinpal commented on pull request "test: fix `addnode` functional test failure on OpenBSD":
(https://github.com/bitcoin/bitcoin/pull/29035#issuecomment-1847673352)
ACK [fd0bde2](https://github.com/bitcoin/bitcoin/pull/29035/commits/fd0bde2793239bd6d60a2435fa28df915cedd7e6)

💬 theuni commented on pull request "Add missing byteswap functions for MSVC":
(https://github.com/bitcoin/bitcoin/pull/29036#issuecomment-1847674787)
Converted this to draft because I haven't actually tested the compile for MSVC (relying on c-i :( ), and because we'll need solid benchmarks before making any decisions.

👋 murchandamus's pull request is ready for review: "Add multiplication operator to CFeeRate"
(https://github.com/bitcoin/bitcoin/pull/29037)

💬 murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1420907612)
How about this? https://github.com/bitcoin/bitcoin/pull/29037

⚠️ RocketNodeLN opened an issue: "26.0 - Released binaries hash mismatch"
(https://github.com/bitcoin/bitcoin/issues/29038)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

The sha256sum for v26 release binaries [1](https://github.com/bitcoin/bitcoin/archive/refs/tags/v26.0.tar.gz) & [2](https://bitcoincore.org/bin/bitcoin-core-26.0/bitcoin-26.0-x86_64-linux-gnu.tar.gz) produce a hash of `e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855`


### Expected behaviour

The hash for the released binary from https://bitcoincore.org/bin/bitcoin-core-2
...

💬 hebasto commented on pull request "Add missing byteswap functions for MSVC":
(https://github.com/bitcoin/bitcoin/pull/29036#issuecomment-1847681276)
Concept ACK on removing another MSVC-specific performance pessimization from the code.

✅ achow101 closed an issue: "26.0 - Released binaries hash mismatch"
(https://github.com/bitcoin/bitcoin/issues/29038)

💬 achow101 commented on issue "26.0 - Released binaries hash mismatch":
(https://github.com/bitcoin/bitcoin/issues/29038#issuecomment-1847686434)
1. The github download links are automatically generated by github. They are not built with the deterministic build process and we do not use them for the published releases. The warning in the github release explicitly says not to use them.
2. This link points to the tarball containing the binaries for x86_64 linux systems, whereas the hash you have posted is for the source tarball. Notice how the names are different.

💬 RocketNodeLN commented on issue "26.0 - Released binaries hash mismatch":
(https://github.com/bitcoin/bitcoin/issues/29038#issuecomment-1847691317)
https://bitcoincore.org/bin/bitcoin-core-26.0/bitcoin-26.0-x86_64-linux-gnu.tar.gz is not the source tarball

💬 kevkevinpal commented on pull request "test: detect OS in functional tests consistently using `platform.system()`":
(https://github.com/bitcoin/bitcoin/pull/29034#issuecomment-1847703106)
ACK [878d914](https://github.com/bitcoin/bitcoin/pull/29034/commits/878d914777a03a04ecb84217152e8b7fd73a5062)

---

Ran this command to make sure we were not missing any, only seeing os.name show up for posix in `test_framework.py`
```
$ grep -nr "os\.name" ./test && grep -nr "sys\.platform" ./test

./test/functional/README.md:40:- Use `platform.system()` for detecting the running operating system and `os.name` to
./test/functional/test_framework/test_framework.py:918: if os.name
...

💬 ajtowns commented on pull request "build: Enable -Wunreachable-code":
(https://github.com/bitcoin/bitcoin/pull/28999#issuecomment-1847739799)
> ACK [fa8adbe](https://github.com/bitcoin/bitcoin/commit/fa8adbe7c17b16cf7ecd16eb9f3f1792e9da2876)
>
> At least on my machine, this does't catch [#28830 (comment)](https://github.com/bitcoin/bitcoin/pull/28830#discussion_r1415690508) though.

`Assert(!error)` in that case is trivially true (error was just set to nullopt, !nullopt is true), so presumably it's getting compiled to a no-op, at which point there is no unreachable code to warn about?

💬 achow101 commented on pull request "wallet: Pass through transaction locktime and preset input sequences and scripts to CreateTransaction":
(https://github.com/bitcoin/bitcoin/pull/25273#discussion_r1420969629)
The consequence is that size estimation will overestimate that input's size by 1 byte, which is up to 4 weight units of overestimation and therefore fees. However it should not be possible to accidentally set an internal input to be external as all calls to `SelectExternal` are guarded by a `!IsMine()`.