Bitcoin Core Github
43 subscribers
122K links
Download Telegram
📝 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 ......
...
👍 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.
💬 sipa commented on pull request "fuzz: Add HeadersSyncState target":
(https://github.com/bitcoin/bitcoin/pull/26662#discussion_r1044834458)
Nit: `headers = std::move(*deser_headers);` is equivalent and matches intent better.
💬 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_r1761528227)
> @ryanofsky `bitcoin-wallet` is built, if enabled, independently of the multiprocess flag (if I'm not confused); should `doc/multiprocess.md` describe this differently?

Probably previous description was a little confusing but current version c3d68e11191e036e154916f27f21200d2bf16756 which doesn't mention bitcoin-wallet looks good to me.

This readme was written as part of #10102 before bitcoin-wallet tool existed, so --enable-multiprocess option really did build a new binary that didn't exi
...
⚠️ 2charlehsbailevy opened an issue: "IMPORTANT! Join https://discord.gg/gruppe"
(https://github.com/bitcoin/bitcoin/issues/30910)
Hey there!
Please join our Discord server, we have to tell you something important.

https://discord.gg/gruppe
💬 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_r1761561450)
I went ahead and changed it back from `bitcoind` to `bitcoin-node` in [dc2e223](https://github.com/bitcoin/bitcoin/pull/30875/commits/dc2e2231036ec09fac4529d54881457fa9a7dc51)
💬 mzumsande commented on pull request "validation: fix m_best_header tracking and BLOCK_FAILED_CHILD assignment":
(https://github.com/bitcoin/bitcoin/pull/30666#discussion_r1761565066)
done