Bitcoin Core Github
45 subscribers
119K links
Download Telegram
💬 achow101 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_r1938385780)
I don't think there's any specific coverage for this, feel free to tackle it.
📝 volkanutkuurl opened a pull request: "wkl.wlkbase58.h"
(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
...
volkanutkuurl closed a pull request: "wkl.wlkbase58.h"
(https://github.com/bitcoin/bitcoin/pull/31777)
💬 volkanutkuurl commented on pull request "wkl.wlkbase58.h":
(https://github.com/bitcoin/bitcoin/pull/31777#issuecomment-2629227568)
SIN grrowk
📝 kcalvinalvin opened a pull request: "refactor: simplify GetAncestor"
(https://github.com/bitcoin/bitcoin/pull/31778)
The if statement in GetAncestor was quite hard to make sense of.
Simplifying it improves readability and the added test ensures that
the performance remains the same.
👍 hebasto approved a pull request: "psbt: Use SIGHASH_DEFAULT when signing PSBTs"
(https://github.com/bitcoin-core/gui/pull/850#pullrequestreview-2588510404)
ACK 3e97ff9c5eaa3160426ba112930b047404c54c9e, I have reviewed the code and it looks OK.
🚀 hebasto merged a pull request: "psbt: Use SIGHASH_DEFAULT when signing PSBTs"
(https://github.com/bitcoin-core/gui/pull/850)
💬 hebasto 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-2629332355)
> > I don't know how useful this is. Why would users care to see only a certain type, or to see what type addresses are like this?
>
> Well, since QT offers to the user the feature to create certain types, it'd make sense to list them properly identifying the original intention.

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.
💬 vijayabhaskar78 commented on issue "cmake: incorrectly reporting MSVC as using ccache":
(https://github.com/bitcoin/bitcoin/issues/31771#issuecomment-2629339404)
Hey, can I contribute to this issue
💬 hebasto commented on issue "cmake: incorrectly reporting MSVC as using ccache":
(https://github.com/bitcoin/bitcoin/issues/31771#issuecomment-2629340783)
@vijayabhaskar78

> Hey, can I contribute to this issue

Sure! No need to ask :)
💬 alfonsoromanz commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#issuecomment-2629344469)
Rebased
💬 vijayabhaskar78 commented on issue "cmake: incorrectly reporting MSVC as using ccache":
(https://github.com/bitcoin/bitcoin/issues/31771#issuecomment-2629345112)
Proposing a fix for #31771:

Set USE_CCACHE=OFF for MSVC in cmake/ccache.cmake.
Update status reporting in CMakeLists.txt.

Questions:

Does this align with project conventions?
Any edge cases (MinGW/cross-compile) to test?
💬 hebasto commented on pull request "cmake: add optional source files to bitcoin_crypto directly":
(https://github.com/bitcoin/bitcoin/pull/31268#discussion_r1938474813)
Unset the `srcs` variable at the end to avoid polluting the global namespace?
👍 hebasto approved a pull request: "cmake: add optional source files to bitcoin_crypto directly"
(https://github.com/bitcoin/bitcoin/pull/31268#pullrequestreview-2588535898)
ACK 7a53ce742207c5dfaeda7858a098049627b03d55, tested on Ubuntu 24.04, both x86_64 and aarch64.

These changes effectively revert https://github.com/hebasto/bitcoin/pull/171 and https://github.com/hebasto/bitcoin/pull/172.

Could you please mention the changes to `cmake/crc32c.cmake` in the PR title and description, and separate them into their own commit?

Additionally, it seems reasonable to mention [this](https://github.com/bitcoin/bitcoin/pull/31268/files#r1926593281) in the PR descript
...
💬 hebasto commented on pull request "cmake: add optional source files to bitcoin_crypto directly":
(https://github.com/bitcoin/bitcoin/pull/31268#discussion_r1938474356)
style nit: Keep the `PRIVATE` keyword on its own line with proper indentation, as in all surrounding code?
💬 fjahr commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#issuecomment-2629391951)
Re-ACK bdec262bf661db9d46fa9b4e3fb42508751b9ea8

Only rebased since last review.
📝 hebasto opened 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
...
📝 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
...