💬 hebasto commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2631926352)
> IMO, there is isn't a good reason to break compatibility here at all, if a shim that just adds one new line and one new file to the build [#31161 (comment)](https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2623350369) can provide it.

Thanks! Applied.

💬 mzumsande commented on issue ""Rolling forward" at startup can take a long time, and is not interruptible":
(https://github.com/bitcoin/bitcoin/issues/11600#issuecomment-2631941438)
> Any more on this? I've been stuck on 90% for hours. Wondering if a smaller dbcache is more preventative, albeit slower? My internet provider shut off at midnight over the weekend for maintenance, causing this issue.

This shouldn't be caused by internet issues - it happens in case of an unclean shutdown.
A smaller dbcache doesn't prevent the need to roll forward, but it should reduce the time rolling forward takes - as does #30611, after which we would flush more often even if running with a
...

⚠️ BartzZin opened an issue: "Bug Report RescanWallet backup.cpp"
(https://github.com/bitcoin/bitcoin/issues/31791)
### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current behaviour

static void RescanWallet(CWallet& wallet, const WalletRescanReserver& reserver, int64_t time_begin = TIMESTAMP_MIN, bool update = true)
{
int64_t scanned_time = wallet.RescanFromTime(time_begin, reserver, update);
if (wallet.IsAbortingRescan()) {
throw JSONRPCError(RPC_MISC_ERROR, "Rescan aborted by user.");
} else if (scanned_time > time_begin) {
throw JSONRPCE
...

📝 darosior opened a pull request: "Double check all block rules in `ConnectBlock`, not only `CheckBlock`"
(https://github.com/bitcoin/bitcoin/pull/31792)
We currently only sanity check `CheckBlock()` in `ConnectBlock()`. Not running the rest of the block
checks makes it hard to reason about upgrade edge cases if we were to introduce new consensus rules
(which would necessarily be context-dependent checks). For instance a timewarp fix or a BIP34
supplementation.

This is fine to do as the expensive check in `ContextualCheckBlock()` is cached since
1ec6bbeb8d27d31647d1433ccb87b362f6d81f90 and the future-timestamp check is split out of
`Conte
...

💬 Oztayls commented on issue ""Rolling forward" at startup can take a long time, and is not interruptible":
(https://github.com/bitcoin/bitcoin/issues/11600#issuecomment-2632008462)
Thanks. You are correct, when I looked into it some more, I had set windows to update automatically, and it rebooted during the night. I've now turned that feature off, and uninstalled nearly all the software on the machine.

The rolling forward process finally completed last night, but now Bitcoin Core (V27) is not connecting to peers and has not recommenced downloading. Network traffic is zero. I'm now at 54% of the initial download after 1 week. I rebooted it, but it's not responding. Progre
...

💬 willcl-ark commented on pull request "ci: run in worktrees":
(https://github.com/bitcoin/bitcoin/pull/31787#discussion_r1940003059)
Is the (possible) running of 01_base_install.sh more than once deliberate? If so, then I'm curious why we ever do that? Is it just for the `DANGER_RUN_CI_ON_HOST` path or is there another use?

If not then it seems like we could just mark install as done when building the image, something like [this](https://github.com/bitcoin/bitcoin/commit/410b01403346553a0aab9ac3d8347fa69398257c)...

> It is also used in tidy via git --no-pager diff.

Does this not mean that it requires the actual .git
...

💬 pinheadmz commented on issue "Bug Report RescanWallet backup.cpp":
(https://github.com/bitcoin/bitcoin/issues/31791#issuecomment-2632046659)
@BartzZin Can you please rewrite the "current behavior" and "expected behavior" fields in your issue in English so we can understand what you are experiencing? Otherwise this issue may be closed as off-topic.

🤔 jonatack reviewed a pull request: "rpc, logging: return "verificationprogress" of 1 when up to date"
(https://github.com/bitcoin/bitcoin/pull/31177#pullrequestreview-2590980640)
Approach ACK

💬 jonatack commented on pull request "rpc, logging: return "verificationprogress" of 1 when up to date":
(https://github.com/bitcoin/bitcoin/pull/31177#discussion_r1940010147)
May I suggest the following cleanup and locking improvement:

- Use Clang thread safety annotation with `GuessVerificationProgress` to ensure ::cs_main is held by callers

- Drop unneeded lock already held by most callers

- Remove redundant doxygen; the convention is to document the declaration

- Move `const ChainTxData& data{GetParams().TxData()};` to where it is used


<details><summary>diff</summary><p>

```diff
diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
i
...

💬 jonatack commented on pull request "rpc, logging: return "verificationprogress" of 1 when up to date":
(https://github.com/bitcoin/bitcoin/pull/31177#discussion_r1940021075)
style pico-nit, make const or maybe save an allocation

```diff
- int64_t header_age = time(nullptr) - m_best_header->GetBlockTime();
- if (header_age < 24 * 60 * 60) {
+ if (int64_t{time(nullptr) - m_best_header->GetBlockTime()} < 24 * 60 * 60) {
```

or

```diff
- int64_t header_age = time(nullptr) - m_best_header->GetBlockTime();
- if (header_age < 24 * 60 * 60) {
+ if (const int64_t header_age{time(nullptr) - m_best_header->GetBlockTime(
...

💬 theuni commented on pull request "build: simplify by flattening the dependency graph":
(https://github.com/bitcoin/bitcoin/pull/30911#issuecomment-2632082441)
Actually, @hebasto , what guarantees that the headers are built before the `test_bitcoin` objects? It seems to work ok but I'm not sure why it doesn't race?

Seems the headers should be an interface library that test_bitcoin depends on?

👍 Prabhat1308 approved a pull request: "test: added additional coverage to waitforblock and waitforblockheight rpc's"
(https://github.com/bitcoin/bitcoin/pull/31784#pullrequestreview-2591018801)
ACK [7e0db87](https://github.com/bitcoin/bitcoin/pull/31784/commits/7e0db87d4fff996c086f6e86b62338c98ef30c55)

Good to have error handling for negative timeout scenarios since both of the functions depend on it.

💬 sipa commented on pull request "rpc, logging: return "verificationprogress" of 1 when up to date":
(https://github.com/bitcoin/bitcoin/pull/31177#discussion_r1940034947)
Use our built-in time functionality in `util/time.h`, which is type-safe (using the standard library's chrono types) and mockable.

💬 ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2632098689)
Updated 1c10bfc167b0f3c17780b2ae3747999ee56c4e1c -> e4289ddc3cf361cfc2ec8c5c64b96e57950d17ab ([`pr/subtree.5`](https://github.com/ryanofsky/bitcoin/commits/pr/subtree.5) -> [`pr/subtree.6`](https://github.com/ryanofsky/bitcoin/commits/pr/subtree.6), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/subtree.5..pr/subtree.6)) to fix ci link error https://cirrus-ci.com/task/6281383380779008 caused by applying core_interface debug flags to multiprocess target but not mptest target depending
...

💬 polespinasa commented on pull request "rpc, logging: return "verificationprogress" of 1 when up to date":
(https://github.com/bitcoin/bitcoin/pull/31177#discussion_r1940060516)
Thanks for reviewing.
I'm not sure about using it as requested. Is this what you're referring to? (the code works as expected)
```
int64_t end_of_chain_timestamp = TicksSinceEpoch<std::chrono::seconds>(NodeClock::time_point{std::chrono::seconds{pindex->GetBlockTime()}});

if (m_best_header && m_best_header->nChainWork > pindex->nChainWork) {
int64_t header_age = TicksSinceEpoch<std::chrono::seconds>(Now<NodeSeconds>()) - m_best_header->GetBlockTime();
```

💬 mzumsande commented on issue "wallet: lastprocessedblock can be inconsistent with internal best block":
(https://github.com/bitcoin/bitcoin/issues/30686#issuecomment-2632200455)
#30678 was merged, should this issue be closed or updated?

If I understand it correctly, the current status is that `m_last_block_processed` can still go out of sync with its saved entry (#30221 suggests to change it) - but that this is no longer a user-facing issue in the context of backups / unloads.

💬 purpleKarrot commented on pull request "build: simplify by flattening the dependency graph":
(https://github.com/bitcoin/bitcoin/pull/30911#discussion_r1940109904)
`1`, `ON`, `YES`, `TRUE`, `Y`, or any non-zero number (including floating point numbers) are treated as true. See: [basic-expressions](https://cmake.org/cmake/help/latest/command/if.html#basic-expressions). Sticking to `ON`/`OFF` consistently avoids this confusion. But yeah, it is a nit.

💬 polespinasa commented on pull request "rpc, logging: return "verificationprogress" of 1 when up to date":
(https://github.com/bitcoin/bitcoin/pull/31177#discussion_r1940129489)
thanks for reviewing. Suggestions applied here ef15351b7605a1b93d0c0f77607802553219f258.
Will squash once other reviews are finished.

💬 purpleKarrot commented on pull request "build: simplify by flattening the dependency graph":
(https://github.com/bitcoin/bitcoin/pull/30911#issuecomment-2632239233)
I have to dig deeper what "header generation" actually is in this context here. But generally, `add_custom_target` and `add_dependencies` are not useful for code generation. You just need a `add_custom_command` that generates the files and you use those files in an `add_library`/`add_executable`/`target_sources`. As long as the custom command and the library/executable are defined in the same directory, dependencies will be resolved.

Custom commands are only useful for things that are built e
...

💬 theuni commented on pull request "build: simplify by flattening the dependency graph":
(https://github.com/bitcoin/bitcoin/pull/30911#issuecomment-2632267200)
Thanks, yeah, ignore that question. You got it exactly right. We're covered by the generated headers being an output of an add_custom_command and being added to the target.