💬 fanquake commented on issue "build: compiler flags in linker flags output":
(https://github.com/bitcoin/bitcoin/issues/31487#issuecomment-2573006198)
Same for stuff like:
`[09:29:46.367] Linker flags .......................... -Wno-error=return-type -Wno-error=maybe-uninitialized -Wno-error=array-bounds`
Showing this in "Linker flags" output doesn't make sense.

📝 kehiy opened a pull request: "doc: upgrade license to 2025."
(https://github.com/bitcoin/bitcoin/pull/31611)

💬 glozow commented on pull request "[28.x] 28.1 backports and final changes":
(https://github.com/bitcoin/bitcoin/pull/31594#issuecomment-2573019102)
reACK 36314b8da2ee65afd5636fa830d436c5c22bd260

💬 maflcko commented on pull request "fuzz: Abort if system time is called without mock time being set":
(https://github.com/bitcoin/bitcoin/pull/31549#issuecomment-2573048815)
Tested that no superflous ones were added via:

<details><summary>diff</summary>

```diff
diff --git a/src/test/fuzz/fuzz.cpp b/src/test/fuzz/fuzz.cpp
index e4e4723c74..eda5ff4cc6 100644
--- a/src/test/fuzz/fuzz.cpp
+++ b/src/test/fuzz/fuzz.cpp
@@ -78,11 +78,13 @@ void FuzzFrameworkRegisterTarget(std::string_view name, TypeTestOneInput target,

static std::string_view g_fuzz_target;
static const TypeTestOneInput* g_test_one_input{nullptr};
+bool g_ever_used_g_mocktime{false};

...

💬 maflcko commented on pull request "fuzz: Abort if system time is called without mock time being set":
(https://github.com/bitcoin/bitcoin/pull/31549#issuecomment-2573049796)
ACK 28e374f0a7dc1bda1c71e14eb896b2d9d2f23b48 🍭

<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: ACK 28e374f0a7dc1bda1c71e14eb8
...

💬 rkrux commented on pull request "BlockAssembler: return selected packages virtual size and fee":
(https://github.com/bitcoin/bitcoin/pull/30391#discussion_r1904134687)
Thank you for sharing that PR. The term `vFeerateHistogram` in itself is fine and in the #21422 PR a histogram is indeed created.
What I find confusing here is calling a simple vector of feefrac's a histogram. If a histogram will be created using this field later, let's call **that field** a histogram and not this one?

💬 hebasto commented on issue "build: compiler flags in linker flags output":
(https://github.com/bitcoin/bitcoin/issues/31487#issuecomment-2573088492)
> Same for stuff like: `[09:29:46.367] Linker flags .......................... -Wno-error=return-type -Wno-error=maybe-uninitialized -Wno-error=array-bounds` Showing this in "Linker flags" output doesn't make sense.

Replacing `CMAKE_CXX_FLAGS` with `APPEND_CXXFLAGS` to provide those flags will help.

💬 fanquake commented on issue "cmake inconsistently overriding `-O3` (sometimes)":
(https://github.com/bitcoin/bitcoin/issues/31491#issuecomment-2573089576)
@hebasto Given that your suggestion of

[> If a user wants to override any of these, they should use the CMAKE_CXX_FLAGS_<CONFIG> ](https://github.com/bitcoin/bitcoin/issues/31491#issuecomment-2541620375)

doesn't work. What should users do? Are we disallowing `-O3` wholesale? If this is the case, it should probably be documented (with the rationale), and likely warned about at configure time (if they have set `-O3` via `CMAKE_CXX_FLAGS_` or any other way), rather than sliently swapping user
...

💬 rkrux commented on pull request "BlockAssembler: return selected packages virtual size and fee":
(https://github.com/bitcoin/bitcoin/pull/30391#discussion_r1904148406)
Related to [this comment](https://github.com/bitcoin/bitcoin/pull/30391/files#r1904134687), I find the language here confusing as well. A fee rate histogram element, which I would expect to be a feeRate interval/group, is being compared to a feeFrac element, which is just a pair of fee and size.

💬 TheCharlatan commented on pull request "Miner: never create a template which exploits the timewarp bug":
(https://github.com/bitcoin/bitcoin/pull/31376#discussion_r1904153448)
I don't think the risk to the miner is big enough to warrant holding this up, especially since the timewarp fix is not actually deployed. A lot of things have to align and the miner needs to be doing weird things with setting their timestamp.

👍 TheCharlatan approved a pull request: "Miner: never create a template which exploits the timewarp bug"
(https://github.com/bitcoin/bitcoin/pull/31376#pullrequestreview-2532144755)
ACK 733fa0b0a140fc1e40c644a29953db090baa2890

🚀 fanquake merged a pull request: "lint: Move assertion linter into lint runner"
(https://github.com/bitcoin/bitcoin/pull/31435)

💬 vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2573130715)
`48843ec694...bbfc58a0af`: rebase and "If there are only a few tasks that do not work, it may be better to just carve them out, instead of enumerating all the ones that work", see [above](https://github.com/bitcoin/bitcoin/pull/31349#discussion_r1896679919).

💬 TheCharlatan commented on pull request "rpc: fix mintime field testnet4, expand timewarp attack test":
(https://github.com/bitcoin/bitcoin/pull/31600#discussion_r1904163181)
I think this should also say that the next block miner also has to use their own wall clock time and not the one provided by the template?

💬 vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r1904166046)
You are right! Flipped the logic to exclude only macOS.

No strong opinion on "tidy" - I added it because those tests should not generate internet traffic. Would drop it if requested.

💬 Sjors commented on pull request "Miner: never create a template which exploits the timewarp bug":
(https://github.com/bitcoin/bitcoin/pull/31376#issuecomment-2573155058)
ACK 733fa0b0a140fc1e40c644a29953db090baa2890

A test would be nice.

After some discussion in https://delvingbitcoin.org/t/timewarp-attack-600-second-grace-period/1326 the main concern I have left, is about miners that ignore the timestamp. This PR has no impact on those miners (because they're not using the timestamp that this PR modifies).

My initial concern was that the eventual soft fork would have a _lower_ `MAX_TIMEWARP`, but this seems very unlikely: https://github.com/bitcoin/bitc
...

💬 Sjors commented on pull request "rpc: fix mintime field testnet4, expand timewarp attack test":
(https://github.com/bitcoin/bitcoin/pull/31600#discussion_r1904186696)
I expanded the comment.

💬 glozow commented on pull request "p2p: track and use all potential peers for orphan resolution":
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1904183854)
Nice, thanks!

💬 glozow commented on pull request "p2p: track and use all potential peers for orphan resolution":
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1904184387)
Of course, done

💬 glozow commented on pull request "p2p: track and use all potential peers for orphan resolution":
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1904124892)
Could you elaborate on how we would get missing ancestors beyond parents? We could iterate through the vin of parents we already have, but I don't see how any of those could be missing. And we can't look through the vin of a missing parent.