💬 Sjors commented on pull request "test: clarify timewarp grace period griefing attack":
(https://github.com/bitcoin/bitcoin/pull/31725#discussion_r1952265961)
Changed the log statement to `<=`

🚀 fanquake merged a pull request: "build: set build type and per-build-type flags as early as possible"
(https://github.com/bitcoin/bitcoin/pull/31711)

💬 fanquake commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#discussion_r1952324188)
Do these new dependencies change what is needed for a no-substitutes/boostrap build?

👍 hodlinator approved a pull request: "ci: Test cross-built Windows executables on Windows natively"
(https://github.com/bitcoin/bitcoin/pull/31176#pullrequestreview-2611426260)
ACK 18274b677500204503a4c3853cfcd9af75531021

### Concept

Since few/no end users will be running Bitcoin Core under *Wine*, it makes sense to test cross-built binaries (similar to the Guix-produced release binaries) natively on Windows.

Left over mentions of Wine stem from eaafa23cbd83b7bda4b28779138c62446bbdea2a and 21704f6334d2a4bd140c6e3260c4bfa3f3157bad. The former seems like it could be more of a mingw issue, but the latter workaround could possibly be removed we we want to stop sup
...

🚀 fanquake merged a pull request: "depends: Make default `host` and `build` comparable"
(https://github.com/bitcoin/bitcoin/pull/30584)

💬 laanwj commented on pull request "random: Check `GetRNDRRS` is supported in `InitHardwareRand` to avoid infinite loop":
(https://github.com/bitcoin/bitcoin/pull/31826#issuecomment-2653350018)
Checked on amazon r7g.xlarge (Graviton 3):
```
2025-02-12T10:07:35Z Bitcoin Core version v28.99.0-585aba6eec85 (release build)
2025-02-12T10:07:35Z Using the 'arm_shani(1way,2way)' SHA256 implementation
2025-02-12T10:07:35Z Using RNDR and RNDRRS as additional entropy sources
```
Also verified that both `GetRNDR` and `GetRNDRRS` are still being called.

As baseline i also built and tested on rpi5 (ARM64 which has no support for RNG). It is still correctly not detected, no invalid instruct
...

🤔 vasild reviewed a pull request: "Have createNewBlock() wait for tip, make rpc handle shutdown during long poll and wait methods"
(https://github.com/bitcoin/bitcoin/pull/31785#pullrequestreview-2611342025)
Approach ACK be15f2e769987b1df2cf1c8f327c5aafe064dbb3

💬 vasild commented on pull request "Have createNewBlock() wait for tip, make rpc handle shutdown during long poll and wait methods":
(https://github.com/bitcoin/bitcoin/pull/31785#discussion_r1952373326)
Given that "no tip" is expected here (unlikely but is not a "programming logic error"), maybe avoid `CHECK_NONFATAL()` and use `JSONRPCError`:

```cpp
std::optional<BlockRef> old_block{miner.getTip()};
std::optional<BlockRef> new_block;
if (IsRPCRunning()) {
const uint256 h{old_block.has_value() ? old_block->hash : uint256::ZERO};
new_block = timeout > 0 ? miner.waitTipChanged(h, std::chrono::milliseconds(timeout)) : miner.waitTipChanged(h);
}

if (!new_block.has_value()) {

...

💬 vasild commented on pull request "Have createNewBlock() wait for tip, make rpc handle shutdown during long poll and wait methods":
(https://github.com/bitcoin/bitcoin/pull/31785#discussion_r1952303154)
```suggestion
* @retval std::nullopt if the given `timeout` passes or the node is shut down before the tip is connected (there is no tip during startup).
```

💬 vasild commented on pull request "Have createNewBlock() wait for tip, make rpc handle shutdown during long poll and wait methods":
(https://github.com/bitcoin/bitcoin/pull/31785#discussion_r1952318566)
Just before the lock and return there was a period when both `m_tip_block_mutex` and `cs_main` were unlocked, so `tip_hash` may be stale here. So this could return an inconsistent hash+height - the hash of one block and the height of another. The previous code was ok because it retrieved both the hash and the height from `chainman().ActiveChain().Tip()` under `cs_main`. I think this will fix it:

```diff
- LOCK(::cs_main);
- return BlockRef{*Assume(tip_hash), Assume(chainman().
...

💬 vasild commented on pull request "Have createNewBlock() wait for tip, make rpc handle shutdown during long poll and wait methods":
(https://github.com/bitcoin/bitcoin/pull/31785#discussion_r1952397601)
Similar here:

```cpp
std::optional<BlockRef> block{miner.getTip()};
const auto deadline{std::chrono::steady_clock::now() + 1ms * timeout};
while (IsRPCRunning() && (!block.has_value() || block->hash != hash)) {
const uint256 h{block.has_value() ? block->hash : uint256::ZERO};
if (timeout) {
const auto now{std::chrono::steady_clock::now()};
if (now >= deadline) break;
const MillisecondsDouble remaining{deadline - now};
block = miner.waitTipChang
...

💬 vasild commented on pull request "Have createNewBlock() wait for tip, make rpc handle shutdown during long poll and wait methods":
(https://github.com/bitcoin/bitcoin/pull/31785#discussion_r1952406392)
Maybe elaborate the message that we could be here if no tip was connected within the timeout: "No tip within timeout or shutting down".

💬 vasild commented on pull request "Have createNewBlock() wait for tip, make rpc handle shutdown during long poll and wait methods":
(https://github.com/bitcoin/bitcoin/pull/31785#discussion_r1952403604)
Why is comment saying "Timeout: ..." when we could be here if the tip changed (no timeout)? Is the comment wrong?

💬 vasild commented on pull request "Have createNewBlock() wait for tip, make rpc handle shutdown during long poll and wait methods":
(https://github.com/bitcoin/bitcoin/pull/31785#discussion_r1952410800)
nit: I think it is redundant to check that `maybe_tip` has value with `CHECK_NONFATAL()` just after `if (!maybe_tip)`. That's like:

```
if (A) {
throw ...
}
assert(!A);
```

`tip = maybe_tip->hash;` seems fine as well.

💬 vasild commented on pull request "Have createNewBlock() wait for tip, make rpc handle shutdown during long poll and wait methods":
(https://github.com/bitcoin/bitcoin/pull/31785#discussion_r1952422440)
The message of the first commit has a line that is 211 chars. Would be nice to use the [50/72 rule](https://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html) in commit messages.

💬 vasild commented on pull request "Have createNewBlock() wait for tip, make rpc handle shutdown during long poll and wait methods":
(https://github.com/bitcoin/bitcoin/pull/31785#discussion_r1952424790)
In commit message of ad3af401c19b8d05ce69011a359db3090b7018e1 `rpc: handle shutdown during long poll and wait methods`:

```diff
- getblocktemplate, waitfornewblock
+ getblocktemplate, waitfornewblock
```

💬 maflcko commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2653380149)
> Left over mentions of Wine stem from [eaafa23](https://github.com/bitcoin/bitcoin/commit/eaafa23cbd83b7bda4b28779138c62446bbdea2a) and [21704f6](https://github.com/bitcoin/bitcoin/commit/21704f6334d2a4bd140c6e3260c4bfa3f3157bad). The former seems like it could be more of a mingw issue,

eaafa23 can probably be removed, or the comment should be adjusted, because time isn't used anymore in GetTime since 0000a63689036dc4368d04c0648a55fdf507932f and instead chrono::system_clock is used.

💬 hebasto commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2653388069)
> > Left over mentions of Wine stem from [eaafa23](https://github.com/bitcoin/bitcoin/commit/eaafa23cbd83b7bda4b28779138c62446bbdea2a) and [21704f6](https://github.com/bitcoin/bitcoin/commit/21704f6334d2a4bd140c6e3260c4bfa3f3157bad). The former seems like it could be more of a mingw issue,
>
> [eaafa23](https://github.com/bitcoin/bitcoin/commit/eaafa23cbd83b7bda4b28779138c62446bbdea2a) can probably be removed, or the comment should be adjusted, because time isn't used anymore in GetTime since
...

📝 maflcko opened a pull request: "test: Remove stale gettime test"
(https://github.com/bitcoin/bitcoin/pull/31846)
(currently typing)

💬 maflcko commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2653417798)
Removed the stale test in https://github.com/bitcoin/bitcoin/pull/31846