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

💬 maflcko commented on issue "Bug Report RescanWallet backup.cpp":
(https://github.com/bitcoin/bitcoin/issues/31791#issuecomment-2633113741)
ai slop

:lock: fanquake locked an issue: "Bug Report RescanWallet backup.cpp"
(https://github.com/bitcoin/bitcoin/issues/31791)

💬 maflcko commented on pull request "ci: run in worktrees":
(https://github.com/bitcoin/bitcoin/pull/31787#discussion_r1940677521)
> 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?

It should only install once (otherwise there will be issues). Yes, it is for `DANGER_RUN_CI_ON_HOST`.

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

💬 maflcko commented on pull request "ci: run in worktrees":
(https://github.com/bitcoin/bitcoin/pull/31787#discussion_r1940680325)
Would be good to explain why this is needed (with an example error log) and why this is the correct fix. My preference would be to fix it in the lint test_runner itself, so that everyone benefits from the fix, not just people running this ci bash script in a workdir.

💬 Sjors commented on pull request "ci: build multiprocess on most jobs":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2633222863)
Rebased on the latest #31741, replaced `-DBUILD_MULTIPROCESS` with `-DENABLE_IPC`.

I dropped 188cae8df38a83b08b104992c5c84c910b01f4b9 "ci: build multiprocess with Tsan" since this is a non-depends job, so IIUC it should also build multiprocess with the same flags as the rest of the code.

I also dropped 743ae9b4e2746f48d707d49fc2c8ba908995034d "build: add bitcoin-{node,gui} to Maintenance.cmake" which can go in a followup that turns it on by default.

On my own macOS 15.3 with Xcode 16.2
...

💬 hebasto commented on pull request "build: simplify by flattening the dependency graph":
(https://github.com/bitcoin/bitcoin/pull/30911#issuecomment-2633228974)
> Without that commit, the headers must be generated late in the build, after a bunch of other libs have been built:
>
> `base58_encode_decode.json /home/cory/dev/bitcoin/cmake/script/GenerateHeaderFromJson.cmake || libcrc32c.a libcrc32c_sse42.a libleveldb.a libminisketch.a minisketch_clmul src/bitcoin_clientversion src/crypto/libbitcoin_crypto.a src/crypto/libbitcoin_crypto_avx2.a src/crypto/libbitcoin_crypto_sse41.a src/crypto/libbitcoin_crypto_x86_shani.a src/generate_build_info src/libbit
...

👍 hebasto approved a pull request: "build: simplify by flattening the dependency graph"
(https://github.com/bitcoin/bitcoin/pull/30911#pullrequestreview-2592139055)
ACK 12fa9511b5fba18e83f88b7b831906595bcf2116.

💬 hebasto commented on pull request "ci: build multiprocess on most jobs":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2633281600)
> Rebased on the latest #31741, replaced `-DBUILD_MULTIPROCESS` with `-DENABLE_IPC`.
>
> I dropped [8a7bcdb](https://github.com/bitcoin/bitcoin/commit/8a7bcdbaa5a45b40ac451439106e78ade7194e37) "ci: build multiprocess with Tsan" since this is a non-depends job, so IIUC it should also build multiprocess with the same flags as the rest of the code.
>
> I also dropped [743ae9b](https://github.com/bitcoin/bitcoin/commit/743ae9b4e2746f48d707d49fc2c8ba908995034d) "build: add bitcoin-{node,gui} to
...

💬 maflcko commented on pull request "test: check `scanning` field from `getwalletinfo`":
(https://github.com/bitcoin/bitcoin/pull/31768#discussion_r1940777811)
Won't this race with `importing` and intermittently fail?