Bitcoin Core Github
43 subscribers
122K links
Download Telegram
👍 TheCharlatan approved a pull request: "guix: (explicitly) build Linux GCC with `--enable-cet`"
(https://github.com/bitcoin/bitcoin/pull/30438#pullrequestreview-2307061040)
ACK 89bf11b807252fe5839b5b18742e24568dfe7bbd

Guix build (aarch64)
```
94142f4399e6b57ae5d95364685cf545a20e1974eb3e6061e62b77af57e59a6b guix-build-89bf11b80725/output/aarch64-linux-gnu/SHA256SUMS.part
1bbbbc9c2818eb6c87a7d7164afc61eee299d8417d986832eb0d503484994f4b guix-build-89bf11b80725/output/aarch64-linux-gnu/bitcoin-89bf11b80725-aarch64-linux-gnu-debug.tar.gz
0601f57454694181473f03bc8ff6c6f23f6ff50f38190a9ff42bd351ff739460 guix-build-89bf11b80725/output/aarch64-linux-gnu/bitcoin-89
...
💬 maflcko commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#issuecomment-2353289836)
> I am thinking if this should still be fixed in v28, so that users don't see this scary `Internal bug detected` log, what do you think @maflcko ?

Yeah, it should probably be fixed or worked around in v28. But AU is marked experimental (if it isn't it should be done, for at least one release), so a fix for a debug log warning doesn't have to be rushed and can be done for rc3, or even 28.1, imo.
💬 l0rinc commented on pull request "ci: Print inner env, Make ccache config more flexible":
(https://github.com/bitcoin/bitcoin/pull/30869#issuecomment-2353296446)
utACK fa99e4521b6fc0e7f6636d40bc0d6a7325227374
💬 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#discussion_r1761424826)
This line can remain: https://github.com/bitcoin/bitcoin/pull/30902#discussion_r1759727845.
💬 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 ......
...