💬 pinheadmz commented on pull request "Make bitcoin-tx replaceable value optional":
(https://github.com/bitcoin/bitcoin/pull/29022#discussion_r1420776229)
I think you can just remove this last sentence. The command DOES take an action if no inputs are provided (you added a test for this!), and the impact of this option in that case I think is handled already by the first two sentences.

📝 theStack opened a pull request: "test: detect OS in functional tests consistently using `platform.system()`"
(https://github.com/bitcoin/bitcoin/pull/29034)
There are at least three ways to detect the operating system in Python3:
- `os.name` (https://docs.python.org/3.9/library/os.html#os.name)
- `sys.platform` (https://docs.python.org/3.9/library/sys.html#sys.platform)
- `platform.system()` (https://docs.python.org/3.9/library/platform.html#platform.system)

We are currently using all of them in functional tests (both in individual tests and shared test framework code), which seems a bit messy. This PR consolidates into using `platform.system(
...

💬 LarryRuane commented on pull request "test: test_bitcoin: allow -testdatadir=<datadir>":
(https://github.com/bitcoin/bitcoin/pull/26564#discussion_r1420802246)
We do remove the custom directory at the beginning of the test:
https://github.com/bitcoin/bitcoin/pull/26564/files#diff-6a8ef76c60f30a6ca67d9f0e478fd02989c4b7fbc4c3116f80e13d873d5775e6R164
so I think this restriction is still needed. I just verified that `/tmp/test_common_Bitcoin\ Core` can be a symlink, so that would allow you to use any location, right? (I could add that to the README but not sure it's worth it.)

📝 theStack opened a pull request: "test: fix `addnode` functional test failure on OpenBSD"
(https://github.com/bitcoin/bitcoin/pull/29035)
This is the functional test counterpart of PR #28891 / commit 007d6f0e85bc329040bb405ef6016339518caa66 (unfortunately, I missed it back then and only ran the unit tests -- sorry for the noise).

master branch on OpenBSD 7.4:
```
$ ./test/functional/rpc_net.py
2023-12-08T17:29:05.057000Z TestFramework (INFO): PRNG seed is: 6024296850131317403
2023-12-08T17:29:05.058000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_au3zchif
2023-12-08T17:29:05.618000Z TestFramew
...

💬 theStack commented on issue "test: Intermittent issue in rpc_net.py":
(https://github.com/bitcoin/bitcoin/issues/29030#issuecomment-1847572942)
Seems like asserting for the debug log message "Added connection peer=" is insufficient for ensuring that this new connection will show up in a following `getpeerinfo()` call, as the debug message is written in the `CNode` ctor, which means it hasn't been added to `CConnman.m_nodes` at this point yet. If I'm not missing anything, then using the newly introduced `wait_for_new_connection` helper instead of asserting the debug log should fix that.

💬 theuni commented on pull request "[POC] C++20 `std::endian`":
(https://github.com/bitcoin/bitcoin/pull/28674#issuecomment-1847574299)
Hmm. I have something working, but as usual, MSVC is the odd case. Looking at the current code, it's not clear to me how MSVC ends up doing byteswaps because we don't have any detection for it. But I also doubt that it's finding the functions in `<byteswap.h>`.

So I think it's possible that MSVC is always taking the slow path? If that's the case, fixing this here would give it a big speedup.

@hebasto could you check with MSVC? It should be easy to check with master with something like:
`
...

💬 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.