💬 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.

💬 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-2632362189)

> 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. Progress increase per hour says "calculating...". Should I reinstall Bitcoin Core?

I managed to resolve this. Updated to V28.1 and reset all settings to default. Downloads started again OK.

**One change I noticed was that the po
...

✅ tdb3 closed a pull request: "test: simplify timewarp test"
(https://github.com/bitcoin/bitcoin/pull/30941)

💬 tdb3 commented on pull request "test: simplify timewarp test":
(https://github.com/bitcoin/bitcoin/pull/30941#issuecomment-2632422222)
Closed, as #31600 was merged

💬 ryanofsky commented on issue "test: 32-bit Clang `ipc_test` failure at `-O0`":
(https://github.com/bitcoin/bitcoin/issues/31772#issuecomment-2632608138)
I tried those steps except I left off the ` -DCMAKE_C_COMPILER='clang;-m32' -DCMAKE_CXX_COMPILER='clang++;-m32'` part and the test passed without a problem.

Is the clang part important? It seems odd to use gcc for the depends build and clang for the bitcoin build, and when when I tried the adding the clang arguments specified, cmake complained about not being able to find libstdc++, which maybe makes sense because, I don't know if it is supposed to work with the gnu library.

If there are any e
...

✅ maflcko closed an issue: "Bug Report RescanWallet backup.cpp"
(https://github.com/bitcoin/bitcoin/issues/31791)