Bitcoin Core Github
45 subscribers
119K links
Download Telegram
📝 hebasto converted_to_draft a pull request: "cmake: Improve safety and robustness during building `crc32c` subtree"
(https://github.com/bitcoin/bitcoin/pull/31779)
It was [agreed](https://github.com/bitcoin/bitcoin/pull/30905#issuecomment-2426263301) not to adopt the upstream buildsystem the `crc32c` subtree.

So this PR ensures that our custom build script properly handles potentially harmful situations, including:
1. The upstream buildsystem changes remaining unnoticed during the subtree update, even when some of those changes need to be reflected in `crc32c.cmake`.

2. The order of `include(cmake/introspection.cmake)` and `include(cmake/crc32c.cmak
...
📝 fanquake locked a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/31777)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...
📝 rhysbeynon opened a pull request: "Added new bitcoin.icns"
(https://github.com/bitcoin/bitcoin/pull/31780)
**Mac OS Minor icon change**
Apple's design guidelines have changed since the previous iteration of the Bitcoin-Core app icon. As it stands, the current icon is too large, and juts out when viewed alongside other program's icons.
My solution was to decrease the overall size to more closely fir Apple's current design documentation, keeping the main body of the icon in the body section of MacOS's icon template, while maintaining the same shape, color, and design.
My motives for this method wer
...
💬 purpleKarrot commented on pull request "cmake: Improve safety and robustness during building `crc32c` subtree":
(https://github.com/bitcoin/bitcoin/pull/31779#issuecomment-2629481474)
Whether the upstream `CMakeLists.txt` file is used or not, the `crc32c` subtree should be added with `add_subdirectory` and not `include`. The latter prevents variables from leaking into the parent directory.
💬 purpleKarrot commented on issue "cmake: incorrectly reporting MSVC as using ccache":
(https://github.com/bitcoin/bitcoin/issues/31771#issuecomment-2629506821)
I don't understand the motivation for setting `CMAKE_<lang>_COMPILER_LAUNCHER` from inside the project. Ideally, the project should not interfere with compiler launchers at all.

This gives individual users as well as CI instances the highest flexibility.

Assuming I have `ENV{CMAKE_CXX_COMPILER_LAUNCHER}` set locally to use [`sccache`](https://github.com/mozilla/sccache). What is the expected behavior when `cmake/ccache.cmake` invokes `list(APPEND CMAKE_CXX_COMPILER_LAUNCHER ${CCACHE_EXECUTABLE
...
💬 davidgumberg commented on pull request "cmake: Fix `-pthread` flags in summary":
(https://github.com/bitcoin/bitcoin/pull/31724#discussion_r1938551789)
I agree, but in that situation we would be back to printing generator expressions in the flags summary, which seems to me better than silently printing something that is not true.
💬 vijayabhaskar78 commented on issue "cmake: incorrectly reporting MSVC as using ccache":
(https://github.com/bitcoin/bitcoin/issues/31771#issuecomment-2629512226)
@purpleKarrot Should we remove the CMAKE_*_COMPILER_LAUNCHER assignments from ccache.cmake to prioritize user/CI flexibility? This would:

Avoid conflicts with external launchers like sccache set via ENV{CMAKE_CXX_COMPILER_LAUNCHER}.
Follow CMake best practices by letting users control launchers externally.
Prevent invalid command chains (e.g., sccache;ccache;cl.exe) when multiple launchers are set.

If agreed, I can update the PR to delete these lines and add documentation instead
💬 vijayabhaskar78 commented on issue "cmake: incorrectly reporting MSVC as using ccache":
(https://github.com/bitcoin/bitcoin/issues/31771#issuecomment-2629513923)
I'm a newbie to this repository. Can you help me to understand it?
💬 purpleKarrot commented on issue "cmake: incorrectly reporting MSVC as using ccache":
(https://github.com/bitcoin/bitcoin/issues/31771#issuecomment-2629517352)
@vijayabhaskar78, I am a newbe in this repository too, but I am not a newbie with CMake ;-)
My recommendation would be to remove `cmake/ccache.cmake` for the reason you wrote.
But let the original authors of that file chime in.
💬 vijayabhaskar78 commented on issue "cmake: incorrectly reporting MSVC as using ccache":
(https://github.com/bitcoin/bitcoin/issues/31771#issuecomment-2629524038)
OK let us see
💬 Oztayls commented on issue ""Rolling forward" at startup can take a long time, and is not interruptible":
(https://github.com/bitcoin/bitcoin/issues/11600#issuecomment-2629572914)
Any more on this? I've been stuck on 90% for hours. Wondering if a smaller dbcache is more preventative, albeit slower? My internet provider shut off at midnight over the weekend for maintenance, causing this issue.
pablomartin4btc closed a pull request: "Add new "address type" column to the "receiving tab" address book page"
(https://github.com/bitcoin-core/gui/pull/753)
💬 pablomartin4btc commented on pull request "Add new "address type" column to the "receiving tab" address book page":
(https://github.com/bitcoin-core/gui/pull/753#issuecomment-2629587602)
> Concept ~0 for adding a new feature that has not been vetted by designers, especially in light of development in https://github.com/bitcoin-core/gui-qml.

No problem! I originally worked on this PR as a way to get familiar with the Qt framework since I found the requirement in #646.

Thanks for your feedback - I'll close the PR to avoid any confusions.
💬 vasild commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1938953860)
Ok, I think using `MAX_MONEY` as magic value to mean "no threshold" would be nice to avoid but is not a blocker for this PR. Accompanying boolean "has" field is fine too.
🤔 rkrux reviewed a pull request: "psbt: add non-default sighash types to PSBTs and unify sighash type match checking"
(https://github.com/bitcoin/bitcoin/pull/31622#pullrequestreview-2589198945)
I reviewed range-diff, asked a question.

```
git range-diff fefe0636d4ae7c246042276cacd60b22f5fc6bb9...c99e1bc359263d44a85460032c04f7c1f3c688c7
```

All the new changes address the previous review comments, namely: removing duplication in the functional test rpc_psbt, adding Taproot input in the test, fixing comments, and changing `FillPSBT()` argument default.
💬 rkrux commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1938961739)
This is the final hex when I run the test, which is same as the one before modifying the `sighash` type as per the test:
```
0200000000010208bd94fdd97de6bc3ec454b8e0088a5341f9e9611bfbe412387e7e0df35207960000000000fdffffff08bd94fdd97de6bc3ec454b8e0088a5341f9e9611bfbe412387e7e0df35207960100000000fdffffff0280f0fa0200000000160014f57919cd765629cd0d2ba39021a146ee065d697508c2f00800000000160014c7109263e9f5db5d2162ce40a1f898f6ac3933ad02473044022067caa211634d7f25bcac48704165ca7d257ff80a7d0b764825877c7a4
...
💬 rkrux commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1938961976)
`provided`
💬 vasild commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1938960467)
nit:
```suggestion
// Helper to check if the tip has changed, and also update tip_changed.
```
💬 vasild commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1938970989)
`wait_until()` will first call the predicate, so `check_tip_changed() || ` is not needed. The above code is equivalent to:

```cpp
notifications().m_tip_block_cv.wait_until(lock, std::min(now + tick, deadline), [&]() EXCLUSIVE_LOCKS_REQUIRED(notifications().m_tip_block_mutex) {
return check_tip_changed() || chainman().m_interrupt;
});
```
hebasto closed a pull request: "MacOS updated Bitcoin-Core gui icon in accordance with Apple design docs"
(https://github.com/bitcoin/bitcoin/pull/31780)