💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1762117778)
done

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1762118393)
done

✅ glozow closed a pull request: "TxDownloadManager refactor followups"
(https://github.com/bitcoin/bitcoin/pull/30820)

💬 glozow commented on pull request "TxDownloadManager refactor followups":
(https://github.com/bitcoin/bitcoin/pull/30820#issuecomment-2354268095)
closing, squashed into #30110. will reopen if followups need to happen again.

💬 furszy commented on pull request "cli: improve error message on multiwallet and add validation to cli-side commands":
(https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1762123098)
I think it would be beneficial to explain the alternatives. The `-rpcwallet` parameter depends on how the wallet is loaded. It can either be the wallet directory name located inside the `-walletdir` (the default value is `datadir/net/wallets/wallet_name`, in which case only `wallet_name` is used in this option), or the absolute path to the wallet directory, which is the complete path `datadir/net/wallets/wallet_name`.

You can note the difference by loading an existing wallet in two different
...

👋 theuni's pull request is ready for review: "build: speed up by flattening the dependency graph"
(https://github.com/bitcoin/bitcoin/pull/30911)

💬 mzumsande commented on pull request "cli: improve error message on multiwallet and add validation to cli-side commands":
(https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1762156524)
... and I think in both of these cases `walletname` would be correct and `filename` incorrect, it would just be a different wallet name depending on how it's loaded.

It could actually also be a filename: Put a legacy wallet `oldwallet.dat` into the datadir, run with `-deprecatedrpc=create_bdb` and `loadwallet "oldwallet.dat"` will load it successfully, in this special case the filename is the walletname. But even here, where `filename` would make sense, `walletname` is correct too, so it just
...

💬 davidgumberg commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#issuecomment-2354365190)
Concept ACK

Nit: could also fix

```diff
--- a/test/util/test_runner.py
+++ b/test/util/test_runner.py
@@ -5,7 +5,7 @@
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
"""Test framework for bitcoin utils.

-Runs automatically during `make check`.
+Runs automatically during `ctest --test-dir build/`.

Can also be run manually."""
```

✅ achow101 closed an issue: "28.0rc1 synchronizes much slower on Windows"
(https://github.com/bitcoin/bitcoin/issues/30833)

🚀 achow101 merged a pull request: "streams: cache file position within AutoFile"
(https://github.com/bitcoin/bitcoin/pull/30884)

👋 achow101's pull request is ready for review: "[28.x] Further backports and rc2"
(https://github.com/bitcoin/bitcoin/pull/30827)

💬 achow101 commented on pull request "streams: cache file position within AutoFile":
(https://github.com/bitcoin/bitcoin/pull/30884#issuecomment-2354420266)
Backported in #30827

💬 maflcko commented on pull request "AssumeUTXO: Don't Assume m_chain_tx_count in GuessVerificationProgress":
(https://github.com/bitcoin/bitcoin/pull/30909#issuecomment-2354677700)
> However, as the test uncovered, `GuessVerificationProgress` is called with snapshot blocks that have `m_chaint_tx_count = 0` when the assumeutxo background sync is in progress.

Can you explain this a bit better? IIRC the snapshot base block itself has the count set, as well as any validated blocks after it. See `src/node/blockstorage.cpp: base->m_chain_tx_count = au_data.m_chain_tx_count;
`. So I think calling `GuessVerificationProgress` in this case shouldn't lead to issues. Wherea
...

💬 maflcko commented on pull request "ci: honor ci bypass prefix in test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/30898#issuecomment-2354696900)
> I don't see a suggestion in the previous conversation to skip these.

All but a few linters apply to all files (this includes doc-only, or markdown-only files). See for example the subtree check: https://github.com/bitcoin/bitcoin/runs/30239893248


```
src/crc32c in HEAD currently refers to tree 81b971cac88189a11a3d7491d8e57437f534d195
src/crc32c in HEAD was last updated in commit 5d45552fd4303f8d668ffbc50cce1053485aeead (tree 454691a9b89ee8b9e1f71a48a7398edba49c3805)
diff --git a/REA
...

💬 maflcko commented on issue "CI timeouts":
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2354720389)
I did a macOS 13 GHA run (https://github.com/maflcko/bitcoin-core-with-ci/actions/runs/10886978505/job/30208236022) and it failed with: `2024-09-16T15:55:29.8666110Z [0;34m node0 stderr Error: A fatal internal error occurred, see debug.log for details: Corrupt block found indicating potential hardware failure. [0m` in `feature_block.py` on ~master.

So possibly Github could be facing hardware issues on macOS, leading to other issues down the line?

✅ Sjors closed a pull request: "ci: honor ci bypass prefix in test-each-commit"
(https://github.com/bitcoin/bitcoin/pull/30898)

💬 Sjors commented on pull request "ci: honor ci bypass prefix in test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/30898#issuecomment-2354725686)
The only use case I have in mind is a large refactor where, e.g. to make the diff easier to follow, an intermediate commit can't build.

I'm not seeing much excitement for that functionality. Such PR can always cherry-pick this commit, so closing for now.

💬 Sjors commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange, drop CRPCSignals & g_best_block":
(https://github.com/bitcoin/bitcoin/pull/30409#issuecomment-2354761241)
Rebased after #30440. Just some `#include` and `using` conflicts since that PR also touched the mining interface.

📝 maflcko opened a pull request: "ci: Use macos-14 GHA image"
(https://github.com/bitcoin/bitcoin/pull/30913)
There shouldn't be any downside, because XCode remains pinned to the same version.

However, builds are expected to be a bit faster with M1, which seems nice.

💬 Sjors commented on pull request "Remove Taproot activation height":
(https://github.com/bitcoin/bitcoin/pull/26201#issuecomment-2354780061)
#27433 needed the rebase here