💬 sipa commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1710554112)
I've simplified the change a bit (now much closer to older deserialization code) and moved it to a separate commit (together with the introduction of the `DepGraph` reordering constructor).

💬 sipa commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2277019007)
Unsure where this should live, but I have a Python reimplementation of the depgraph serialization code + mermaid formatter: https://gist.github.com/sipa/822f60db6608a26bb4aae75fd31bcb12

💬 ajtowns commented on pull request "contrib/signet/miner updates":
(https://github.com/bitcoin/bitcoin/pull/28417#discussion_r1710716168)
`next_block_delta` can be somewhat small if you have a high difficulty target (ie 2.5m instead of 10m) and if `--poisson` is specified will be smaller again (the deterministic random calculation used against the genesis block reduces the target by ~73%). The `--poisson` result alone is already annoying -- it will generally only take 25 blocks before it's caught up to real time; but even without that if you switch from mining with a high nbits target to a minimal nbits target, then even without `
...

💬 maflcko commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2277190642)
(The drahtbot guix build failed due to a silent merge conflict, I presume)

💬 maflcko commented on pull request "assumeutxo: Drop block height from metadata":
(https://github.com/bitcoin/bitcoin/pull/30598#issuecomment-2277205856)
The CI failure is known and can be ignored (https://github.com/bitcoin/bitcoin/actions/runs/10310090865/job/28541115356?pr=30598#step:27:229)

Only change since my last review is a small style-only cleanup to remove the unused parameter from the constructor.

re-ACK 00618e8745192d209c23e3ae873c077e58168957 🎌

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

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_fi
...

💬 maflcko commented on pull request "lint: Find function calls in default arguments":
(https://github.com/bitcoin/bitcoin/pull/30553#discussion_r1710808023)
This will also drop the "All checks passed!" on the success case (https://cirrus-ci.com/task/4866693351079936?logs=lint#L656), which I did not like when writing this.

💬 maflcko commented on pull request "lint: Find function calls in default arguments":
(https://github.com/bitcoin/bitcoin/pull/30553#discussion_r1710809136)
I copy-pasted this from the mlc linter, so maybe this can be done in a follow-up for both. Otherwise, it seems inconsistent.

💬 maflcko commented on pull request "lint: Find function calls in default arguments":
(https://github.com/bitcoin/bitcoin/pull/30553#discussion_r1710811261)
I am trying to preserve the behavior of the previous check based on `git grep --extended-regexp`, which didn't exclude subtrees either.

However, I'll apply your suggestion in a follow-up.

💬 maflcko commented on pull request "lint: Find function calls in default arguments":
(https://github.com/bitcoin/bitcoin/pull/30553#discussion_r1710817315)
Thanks, fixed typo (in another way)

🤔 maflcko reviewed a pull request: "Fixes for GCC 15 compatibility"
(https://github.com/bitcoin/bitcoin/pull/30612#pullrequestreview-2229402700)
> * Two are related to missing includes. You can't use `uint16_t` et al. without including `<cstdint>`.

Probably a good time to enable iwyu. :sweat_smile:

> This manifests as the following compile-time mess:

Are you sure this is not a GCC bug?

💬 maflcko commented on pull request "Fixes for GCC 15 compatibility":
(https://github.com/bitcoin/bitcoin/pull/30612#discussion_r1710836374)
```suggestion
template<std::integral I> // Disallow silent float -> int conversion
```

Please keep the comment

💬 maflcko commented on pull request "Fixes for GCC 15 compatibility":
(https://github.com/bitcoin/bitcoin/pull/30612#discussion_r1710833459)
iwyu:

```
node/interface_ui.h should add these lines:
#include <vector> // for vector

node/interface_ui.h should remove these lines:
- #include <memory> // lines 11-11

💬 whitslack commented on pull request "Fixes for GCC 15 compatibility":
(https://github.com/bitcoin/bitcoin/pull/30612#discussion_r1710848701)
With all due respect, that's a semantically void comment. `std::integral I` already means "disallow floats."

💬 whitslack commented on pull request "Fixes for GCC 15 compatibility":
(https://github.com/bitcoin/bitcoin/pull/30612#issuecomment-2277260451)
> Are you sure this is not a GCC bug?

The compile-time error is of a class of errors that is not specific to GCC. Maybe the stdlibc++ in GCC 15 changed the implementation of `std::optional` so that the previously valid code in `feerate.h` is no longer valid. In general, if, in the course of checking a type constraint, the compiler instantiates a template that invokes the same type constraint, then there is an infinite recursion, and the compiler will reject the code with an error saying that
...

💬 maflcko commented on pull request "C++20 std::views::reverse":
(https://github.com/bitcoin/bitcoin/pull/28687#issuecomment-2277264560)
ACK 2925bd537cbd8c70594e23f6c4298b7101f7f73d 🎷

<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 2925bd537cbd8c70594e23f6c4
...

💬 Sjors commented on pull request "doc, chainparams: 29775 release notes and follow-ups":
(https://github.com/bitcoin/bitcoin/pull/30604#discussion_r1710890448)
35ad61a6bde14a74cc07dc8ecf3bb1cd49590e46: let's define a separate constant and make it clear these are different but related:

```cpp
/**
* Maximum number of seconds that the timestamp of the first
* block of a difficulty adjustment period is allowed to
* be earlier than the last block of the previous period.
*/
static constexpr int64_t MAX_TIMEWARP{MAX_FUTURE_BLOCK_TIME};

/**
* If the timestamp of the last block in a difficulty period is set
* MAX_FUTURE_BLOCK_TIME seconds in
...

💬 maflcko commented on pull request "Fixes for GCC 15 compatibility":
(https://github.com/bitcoin/bitcoin/pull/30612#issuecomment-2277287178)
> > Are you sure this is not a GCC bug?
>
> The compile-time error is of a class of errors that is not specific to GCC. Maybe the stdlibc++ in GCC 15 changed the implementation of `std::optional` so that the previously valid code in `feerate.h` is no longer valid.

Correct. Though, GCC 15 is so far out that there is a chance the change will be reverted again internally, because I assume it breaks compilation of more code than just this single instance. I am wondering if their change was int
...

⚠️ Sjors opened an issue: "UpdateTime() needs to account for timewarp fix on testnet4 "
(https://github.com/bitcoin/bitcoin/issues/30614)
In (`miner.cpp`) we set the block (template) timestamp as follows:

```cpp
pblock->nTime = TicksSinceEpoch<std::chrono::seconds>(NodeClock::now());
...
UpdateTime(pblock, ...)


int64_t UpdateTime(CBlockHeader* pblock, ...)
{
int64_t nOldTime = pblock->nTime;
int64_t nNewTime{std::max<int64_t>(pindexPrev->GetMedianTimePast() + 1, TicksSinceEpoch<std::chrono::seconds>(NodeClock::now()))};

if (nOldTime < nNewTime) {
pblock->nTime = nNewTime;
}
```

In oth
...

💬 Sjors commented on pull request "doc, chainparams: 29775 release notes and follow-ups":
(https://github.com/bitcoin/bitcoin/pull/30604#discussion_r1710919235)
And see #30614.

✅ maflcko closed an issue: "getmempoolentry returns "incorrect" bip125-replaceable status"
(https://github.com/bitcoin/bitcoin/issues/22209)