💬 pablomartin4btc commented on pull request "wallet: init, don't error out when loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/32449#issuecomment-2887006977)
> > Is it worth it to verify that, previous to this PR, the node with the legacy wallet in its `settings.json` was failing to restart (using `assert_start_raises_init_error`)?
>
> That's what is being tested. You can run the test on master and it will fail due to that behavior.

I meant without switching to `master` and once this PR gets merged, I thought there was a release with this failure and your fix will be in the next one, but both changes will be part of the same release so the answ
...

💬 pablomartin4btc commented on pull request "wallet: init, don't error out when loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/32449#discussion_r2093241973)
I've left a [suggestion](https://github.com/bitcoin/bitcoin/pull/32449#discussion_r2093187305).

💬 mzumsande commented on issue "avoid using invalidateblock to directly test reorg behavior":
(https://github.com/bitcoin/bitcoin/issues/32531#issuecomment-2887013040)
> > Why not simply submit the fork chain at the right point in time
>
> Sure, that works too, it just seemed easiest since you can use ~all the tooling to generate blockchains. e.g., if you want stuff in the fork blocks (and make as many blocks as you like) it's trivial to do via rpcs like generateblock.

Oh ok I see now, as in https://github.com/bitcoin/bitcoin/pull/32516/commits/16af944a547bfea8f1f910938491a8ad11a47de5. Could also prepare the fork chain on a second node using whatever toolin
...

📝 fanquake opened a pull request: "Update leveldb subtree to latest upstream"
(https://github.com/bitcoin/bitcoin/pull/32534)
Pulls in
* https://github.com/bitcoin-core/leveldb-subtree/pull/51

Remove the related warning suppression.

💬 rkrux commented on pull request "wallet: Ensure best block matches wallet scan state":
(https://github.com/bitcoin/bitcoin/pull/30221#discussion_r2093245598)
> Not sure what would be verified? After invalidateblock and before the crash, we do verify that the coinbase transaction is marked abandoned.

I wanted to check that the best block locator is indeed persisted to disk after invalidation but the `getwalletinfo` RPC reads from memory, which can't be used for verifying this PR's change. The coinbase transaction verification was done before this PR as well, so that also not verifies.

Previously, I had in mind some functionality that reads from
...

💬 maflcko commented on pull request "rpc: Round verificationprogress to 1 for a recent tip":
(https://github.com/bitcoin/bitcoin/pull/32528#issuecomment-2887022304)
> > After this PR, it will mean your chain tip's timestamp is within 2 hours of now.
>
> I think that's a worse cure than the disease. In case we **know** there are unprocessed blocks in the chain (because we have their headers), we shouldn't say progress: 1.

I don't think this is making anything worse. This is already possible on current master, when a miner sets the timestamp to a future one (compared to your time), even if there are still blocks ahead.

💬 pablomartin4btc commented on pull request "wallet, refactor: Remove Legacy wallet unused warnings and errors":
(https://github.com/bitcoin/bitcoin/pull/32481#discussion_r2093251942)
maybe neither do this one:

https://github.com/bitcoin/bitcoin/blob/39090518f62cca5432413d1669a85924ba721278/src/wallet/wallet.cpp#L448

(just to confirm and do both in 1 go)

💬 l0rinc commented on pull request "Update leveldb subtree to latest upstream":
(https://github.com/bitcoin/bitcoin/pull/32534#issuecomment-2887030315)
utACK 7015052eba23368539dcd1a9b4217ce1cacd2999

For the record, there are other `LevelDB` changes that could use some reviews: https://github.com/bitcoin-core/leveldb-subtree/pulls

💬 furszy commented on pull request "wallet: init, don't error out when loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/32449#issuecomment-2887030525)
The issue is caused by the stderr warning message not being consumed. The framework expects stderr to be empty at the end of the test, without accounting for warning messages. Will update the test to clear the buffer after startup.

💬 mzumsande commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2093270847)
I meant the 2100 (two weeks) of of blocks. I didn't test this, but is there a reason this couldn't just be a few blocks (plus a few more that aren't sent to the node)?

💬 maflcko commented on pull request "[28.x] build: suppress `-Wunterminated-string-initialization` in secp256k1":
(https://github.com/bitcoin/bitcoin/pull/32484#issuecomment-2887069576)
An alternative would be to just ignore the warning? CI on 28.x isn't using gcc 15 and users aren't really expected to compile with werror?

🤔 darosior reviewed a pull request: "test: properly check for per-tx sigops limit"
(https://github.com/bitcoin/bitcoin/pull/32533#pullrequestreview-2847007268)
Concept ACK

💬 fanquake commented on pull request "[28.x] build: suppress `-Wunterminated-string-initialization` in secp256k1":
(https://github.com/bitcoin/bitcoin/pull/32484#issuecomment-2887084027)
We could also do that. I guess it depends where we expect this branch to be compiled. Any downstream CIs, build systems, packagers etc may be turning warnings into errors. It doesn't seem bad for us to suppress known non-issues.

📝 maflcko opened a pull request: "rev_32343_wip_nomerge_ci"
(https://github.com/bitcoin/bitcoin/pull/32535)
pls ignore this is just a #32524 playground

💬 fanquake commented on pull request "build: Introduce internal kernel library":
(https://github.com/bitcoin/bitcoin/pull/28690#discussion_r2093340350)
Should also be easy to get `bitcoin_clientversion` out of here.

⚠️ hebasto opened an issue: "cmake: Cannot find Qt 6 on SunOS / illumos (OpenIndiana Distribution)"
(https://github.com/bitcoin/bitcoin/issues/32536)
Qt 6 was installed as follows:
```
$ sudo pkg install library/qt6
```

As a workaround, the `Qt6_DIR` CMake variable can be set manually:
```
$ cmake -B build -DBUILD_GUI=ON -DQt6_DIR=/usr/lib/qt/6.6/lib/amd64/cmake/Qt6
```

Alternatively, the [`Qt6_ROOT`](https://cmake.org/cmake/help/latest/variable/PackageName_ROOT.html) CMake or [environment](https://cmake.org/cmake/help/latest/envvar/PackageName_ROOT.html) variable can be used:
```
$ env Qt6_DIR=/usr/lib/qt/6.6/lib/amd64 cmake -B build -DBUI
...

💬 furszy commented on pull request "wallet: init, don't error out when loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/32449#discussion_r2093346688)
sure, updated.

💬 furszy commented on pull request "wallet: init, don't error out when loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/32449#discussion_r2093349150)
Would have liked to do this, but it's not enough. The stderr pipe still needs to be cleared before the test finishes, or the framework will treat the warning message as an error during cleanup.

📝 hebasto opened a pull request: "250516 win min ver"
(https://github.com/bitcoin/bitcoin/pull/32537)
Set minimum supported Windows version to 1903 (May 2019 Update).

This version is the minimum [required](https://learn.microsoft.com/en-us/windows/apps/design/globalizing/use-utf8-code-page#set-a-process-code-page-to-utf-8) to support the UTF-8 code page (CP_UTF8), which is necessary for https://github.com/bitcoin/bitcoin/pull/32380.

Additionally, the `symbol-check.py` script has been amended to verify application manifests for supported OS value.

💬 hebasto commented on pull request "Set minimum supported Windows version to 1903 (May 2019 Update)":
(https://github.com/bitcoin/bitcoin/pull/32537#issuecomment-2887193874)
My Guix build:
```
aarch64
db31eebbbd5ed99567eea6d1e692130815e0b6e6c7041e8fe20918eb743ce3db guix-build-4748c63c1d0f/output/aarch64-linux-gnu/SHA256SUMS.part
05ad41b64f57a027124138e3e5baf3325e4bcb433c851e9086526568068694f2 guix-build-4748c63c1d0f/output/aarch64-linux-gnu/bitcoin-4748c63c1d0f-aarch64-linux-gnu-debug.tar.gz
f230295bc79a313acaa16090609b28c6df6021213b8bd8dea9938b8e48037f17 guix-build-4748c63c1d0f/output/aarch64-linux-gnu/bitcoin-4748c63c1d0f-aarch64-linux-gnu.tar.gz
acc5897b
...