📝 hebasto opened a pull request: "Revert "ci: Only run functional tests on windows in master""
(https://github.com/bitcoin/bitcoin/pull/29059)
This PR reverts commit aba4a5887b44bf7cbee9ea0a8e02bb92c1b4147b from https://github.com/bitcoin/bitcoin/pull/28567.

The Windows-specific code received [quality](https://github.com/bitcoin/bitcoin/pull/28486) and [performance](https://github.com/bitcoin/bitcoin/pull/29045) improvements recently. So there are no reasons to skip functional tests in PRs anymore.

In my own repo, I've run the GHA Windows job more than 100 times with no failure.

💬 maflcko commented on pull request "Revert "ci: Only run functional tests on windows in master"":
(https://github.com/bitcoin/bitcoin/pull/29059#discussion_r1423789241)
How much longer would it take to also run `--extended`?

💬 hebasto commented on pull request "Revert "ci: Only run functional tests on windows in master"":
(https://github.com/bitcoin/bitcoin/pull/29059#discussion_r1423803255)
> How much longer would it take to also run `--extended`?

Appr. 10 minutes.

The `feature_dbcrash.py` test takes the longest time.

🚀 fanquake merged a pull request: "refactor: rpc: Pass CBlockIndex by reference instead of pointer"
(https://github.com/bitcoin/bitcoin/pull/29021)

💬 dergoegge commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#issuecomment-1851803284)
Removing adjusted time from consensus is a big win. Having time samples received from peers be involved in consensus checks is not great and not effective against NTP based attacks (https://github.com/bitcoin/bitcoin/pull/25908#issuecomment-1264632475, https://github.com/bitcoin/bitcoin/pull/25908#issuecomment-1271239626). The warning itself is not as interesting to me but most of the review on both #25908 and this PR is about the warning and its threshold.

I don't want changing the warning (
...

👍 brunoerg approved a pull request: "doc/reduce-traffic: update/clarify max outbound connection count"
(https://github.com/bitcoin/bitcoin/pull/29052#pullrequestreview-1777200873)
ACK d58f89d355faf46b68d0ff8095699d0aff41959c

💬 dergoegge commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#issuecomment-1851813782)
> an alternative would divide each element by 2 and then add without overflow risk

Done.

💬 maflcko commented on pull request "Revert "ci: Only run functional tests on windows in master"":
(https://github.com/bitcoin/bitcoin/pull/29059#issuecomment-1851817205)
lgtm ACK 7b22cd80e050b903b5765133b8116f4b17ce0296

📝 fanquake converted_to_draft a pull request: "build: switch to using LLVM 17.x for macOS builds"
(https://github.com/bitcoin/bitcoin/pull/28880)
This is the last step before #21778. We need LLVM 17.x so that lld has `-fixup_chains`.

This is currently in draft, as there is a reproducibility issue with Qt: https://github.com/bitcoin/bitcoin/pull/28880#issuecomment-1830224605.

💬 fanquake commented on pull request "[POC] C++20 `std::endian`":
(https://github.com/bitcoin/bitcoin/pull/28674#discussion_r1423843923)
Will be possible after we pull the changes from minisketch: https://github.com/sipa/minisketch/pull/80.

💬 fanquake commented on pull request "[WIP] C++20 std::views::reverse":
(https://github.com/bitcoin/bitcoin/pull/28687#issuecomment-1851843817)
https://github.com/bitcoin/bitcoin/pull/28687/checks?check_run_id=19528202548
```bash
/usr/bin/ccache clang++-13 -stdlib=libc++ -std=c++20 -DHAVE_CONFIG_H -I. -I../src/config -fmacro-prefix-map=/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu=. -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -DHAVE_BUILD_INFO -DPROVIDE_FUZZ_MAIN_FUNCTION -I. -I./minisketch/include -I./secp256k1/include -I./univalue/include -I./leveldb/include -isystem /ci_container_base/depends/x86_64-pc-linux-gnu/includ
...

💬 Retropex commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1851853229)
@conduition is very relevant as a question.

What fundamentally bothers me with inscriptions is that they do not play the same game as those who make financial transactions.

When you pay to send 10,000 times the same funds to the same wallet, you pay to move Bitcoin from one address to another and even if it's "useless" it doesn't bother me because there is no incentive to do that.

On the other hand, inscriptions have an incentive that is external to the Bitcoin protocol, it does not pa
...

🚀 fanquake merged a pull request: "doc/reduce-traffic: update/clarify max outbound connection count"
(https://github.com/bitcoin/bitcoin/pull/29052)

✅ fanquake closed an issue: "Nit: Inconsistency in the docs regarding block-relay-only connections"
(https://github.com/bitcoin/bitcoin/issues/29046)

💬 1ma commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1851877095)
> If i publish a chain of 1000 transactions pointlessly bouncing coins around my own wallet addresses, is that spam? What about a single big (in terms of weight) transaction which only pays from myself to myself? The whole BTC network (including myself) would be better off if i didn't do that. I'm needlessly inflating the fee market for zero utility. In my humble opinion, inscriptions are similarly redundant because they could be done off-chain, so their on-chain utility to the network is near z
...

🚀 fanquake merged a pull request: "tests, bench: Fix issue with CWallet::LoadWallet() being called in the wrong places"
(https://github.com/bitcoin/bitcoin/pull/29055)

💬 maflcko commented on pull request "refactor: Remove pre-C++20 code, fs::path cleanup":
(https://github.com/bitcoin/bitcoin/pull/29040#discussion_r1423882821)
> What would be an example where it is broken by calling `string()`

Any place, which correctly calls `u8string()`, and instead incorrectly calls `string()` is broken. For example, the code may be broken on systems whose native encoding is not UTF-8.

See also

```diff
diff --git a/src/test/fs_tests.cpp b/src/test/fs_tests.cpp
index 7cfecb2b22..34168a551b 100644
--- a/src/test/fs_tests.cpp
+++ b/src/test/fs_tests.cpp
@@ -19,7 +19,7 @@ BOOST_AUTO_TEST_CASE(fsbridge_pathtostring)
{

...

✅ fanquake closed an issue: "Discussion: Upgrading to C++20"
(https://github.com/bitcoin/bitcoin/issues/23363)

💬 fanquake commented on issue "Discussion: Upgrading to C++20":
(https://github.com/bitcoin/bitcoin/issues/23363#issuecomment-1851893448)
Going to close this now that #28349 has been merged, and work is already underway to use C++20 code. I think further discussion about specific C++20 features, and their usage / compiler requirements, can continue in their own issues.

💬 maflcko commented on pull request "[WIP] C++20 std::views::reverse":
(https://github.com/bitcoin/bitcoin/pull/28687#issuecomment-1851904825)
Hmm, there's no clang-16 in the current LTS release of Ubuntu, so I guess this will have to wait for some time.

https://packages.ubuntu.com/jammy/clang-16


I wonder whether std::span will be usable, given that it internally assumes ranges?

💬 petertodd commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1851916112)
> The original Satoshi client didn't allow embedding data onchain, this has always been an exploit.

Bitcoin was released with no standardness rules at all. You absolutely could embed data onchain. You could even publish arbitrary data to the UTXO set easily.