💬 l0rinc commented on pull request "build: improve cxx and linker flag caching":
(https://github.com/bitcoin/bitcoin/pull/30732#issuecomment-2353313839)
Thanks for checking @hebasto!

> Is this scenario possible in the master branch? If not, perhaps this functionality can be dropped?

There's only a single case where SOURCE is defined now - in which case the related flags were also unique:
> LINKER_SUPPORTS__FSANITIZE_UNDEFINED_ADDRESS_FUZZER__WL__FATAL_WARNINGS_

We could drop the hash, but that means that whoever calls that method again has to change the implementation, since it's not safe to call it without knowing who else called it,
...

💬 achow101 commented on pull request "qt: Translations update":
(https://github.com/bitcoin/bitcoin/pull/30899#issuecomment-2353314794)
Backported in #30827

💬 kevkevinpal commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change and removed libtools from CI":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1761429706)
no problem removed in [03fd466](https://github.com/bitcoin/bitcoin/pull/30875/commits/03fd466581666cfd637e79a8594a9a0e7d097524)[03fd466](https://github.com/bitcoin/bitcoin/pull/30875/commits/03fd466581666cfd637e79a8594a9a0e7d097524)[03fd466](https://github.com/bitcoin/bitcoin/pull/30875/commits/03fd466581666cfd637e79a8594a9a0e7d097524)

💬 achow101 commented on pull request "streams: cache file position within AutoFile":
(https://github.com/bitcoin/bitcoin/pull/30884#issuecomment-2353325069)
ACK a240e150e837b5a95ed19765a2e8b7c5b6013f35

💬 fanquake commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change and removed libtools from CI":
(https://github.com/bitcoin/bitcoin/pull/30875#issuecomment-2353336442)
https://github.com/bitcoin/bitcoin/blob/37679b856ce183ec256c12a680b93ad53ed94da6/doc/design/libraries.md?plain=1#L11

📝 jonatack opened a pull request: "doc: update Eclipser fuzzing documentation"
(https://github.com/bitcoin/bitcoin/pull/30908)
Simplify and update the dependencies installation instructions by referring to those in the Eclipser repository.

💬 jonatack commented on pull request "doc: update Eclipser fuzzing documentation":
(https://github.com/bitcoin/bitcoin/pull/30908#issuecomment-2353339619)
(That project doesn't appear to have been updated since 3-4 years, and I don't know its current status and relevance to us.)

📝 fjahr opened a pull request: "AssumeUTXO: Don't Assume m_chain_tx_count in GuessVerificationProgress"
(https://github.com/bitcoin/bitcoin/pull/30909)
A test that is added as part of #30455 uncovered this issue: The `GuessVerificationProgress` function is used during during descriptor import and relies on `m_chain_tx_count`. In #29370 an [`Assume` was added](https://github.com/bitcoin/bitcoin/pull/29370/commits/0fd915ee6bef63bb360ccc5c039a3c11676c38e3) expecting the `m_chaint_tx_count` to be set. However, as the test uncovered, `GuessVerificationProgress` is called with snapshot blocks that have `m_chaint_tx_count = 0` when the assumeutxo back
...

📝 achow101 converted_to_draft a pull request: "[28.x] Further backports and rc2"
(https://github.com/bitcoin/bitcoin/pull/30827)
* #30834
* #30807
* #30880
* https://github.com/bitcoin-core/gui/pull/835
* #30899

💬 ryanofsky commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change and removed libtools from CI":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1761460346)
re: https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1759786194

Thanks for the ping! Yes, this should say `bitcoin-node` not `bitcoind` to test the multiprocess binary. Either command line will work, but `bitcoin-node` will make the test more meaningful, especially with changes from #10102 merged, because `bitcoind` will run tests with the node and wallet in the same process, while `bitcoin-node` will spawn separate `bitcoin-wallet` processes

💬 fjahr commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#issuecomment-2353371964)
I have opened a separate PR #30909 with my suggested fix and your test commit cherry-picked on top of it @alfonsoromanz .

💬 jonatack commented on pull request "doc: update Eclipser fuzzing documentation":
(https://github.com/bitcoin/bitcoin/pull/30908#issuecomment-2353373061)
cc https://github.com/agroce

🤔 ismaelsadeeq reviewed a pull request: "wallet: Write best block to disk before backup"
(https://github.com/bitcoin/bitcoin/pull/30678#pullrequestreview-2307141540)
Concept ACK, and ACK after resolving https://github.com/bitcoin/bitcoin/pull/30678#pullrequestreview-2260723558


Should also add a test for when a wallet is migrated, I left a suggested for that.

💬 ismaelsadeeq commented on pull request "wallet: Write best block to disk before backup":
(https://github.com/bitcoin/bitcoin/pull/30678#discussion_r1761464897)
It would be beneficial to have this in a separate test method so that we can add more test cases cleanly without associating it with the assume UTXO test. For example, we can test https://github.com/furszy/bitcoin-core/commit/5b618f1b024f16640a2a05f1e269f61b33b86577 by creating a non-descriptor wallet, migrating it, and attempt to restore it.

💬 jonatack commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change and removed libtools from CI":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1761472002)
@ryanofsky `bitcoin-wallet` is built in any case; should `doc/multiprocess.md` describe this differently?

```
Configure summary
=================
Executables:
bitcoind ............................ ON
bitcoin-node (multiprocess) ......... OFF
bitcoin-qt (GUI) .................... ON
bitcoin-gui (GUI, multiprocess) ..... OFF
bitcoin-cli ......................... ON
bitcoin-tx .......................... ON
bitcoin-util ........................ ON
bitcoin-wallet ......
...

💬 kevkevinpal commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change and removed libtools from CI":
(https://github.com/bitcoin/bitcoin/pull/30875#issuecomment-2353391005)
> https://github.com/bitcoin/bitcoin/blob/37679b856ce183ec256c12a680b93ad53ed94da6/doc/design/libraries.md?plain=1#L11

Thanks! updated in [c3d68e1](https://github.com/bitcoin/bitcoin/pull/30875/commits/c3d68e11191e036e154916f27f21200d2bf16756)

👍 hebasto approved a pull request: "[RFC] Switch and/or align debugging flags (back) to `-Og`"
(https://github.com/bitcoin/bitcoin/pull/29796#pullrequestreview-2307158543)
ACK 296def1f6fd936d72d49ce98f9473b4a5d2f9c4b, I have reviewed the code and it looks OK.

This PR effectively aligns flags between the depends build subsystem and the main build system. This change makes the current undocumented behaviour--where flags from depends override flags form the main build system--more flexible, and it can be considered in the ongoing [discussion](https://github.com/bitcoin/bitcoin/issues/30813).

Users will still be able to adjust their debugging experience by manua
...

💬 sdaftuar commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2353413849)
ACK 9ad2fe7e69e9e69949ebbb280a15756dc3301f09

🤔 sipa reviewed a pull request: "fuzz: Add HeadersSyncState target"
(https://github.com/bitcoin/bitcoin/pull/26662#pullrequestreview-1212243962)
Apparently I never submitted this feedback. Maybe useful for follow-ups.

💬 sipa commented on pull request "fuzz: Add HeadersSyncState target":
(https://github.com/bitcoin/bitcoin/pull/26662#discussion_r1044846771)
It'd be a bit easier to follow if instead of this conditional there were a `if (!result.request_more) break;`. The `requested_more` variable can then also go away, as it'd always be true any time it's checked. Lastly, it'd make it more obvious that `redownloaded_it` is always initialized in iterations after the first.