Bitcoin Core Github
44 subscribers
119K links
Download Telegram
💬 hodlinator commented on pull request "build, ci: Fix linking `bitcoin-chainstate.exe` to `bitcoinkernel.dll` on Windows":
(https://github.com/bitcoin/bitcoin/pull/31158#discussion_r1981338733)
Isn't *symbol_visibility.h* this dedicated header?
💬 hebasto commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#discussion_r1981361215)
Thanks! Taken.

In my initial implementation, I relied on the [fail-fast behavior](https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#exit-codes-and-error-action-preference) in GitHub-hosted runners. However, it turns out that `$ErrorActionPreference = 'stop'` only applies to PowerShell cmdlets.
💬 hebasto commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2700881250)
Addressed the recent feedback from @davidgumberg.
💬 fanquake commented on pull request "[28.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/31648#issuecomment-2700914514)
@willcl-ark I've just changed it here.
💬 fanquake commented on pull request "ci: rename imagefile to Dockerfile to follow docker file's convention.":
(https://github.com/bitcoin/bitcoin/pull/31995#issuecomment-2700918781)
> And some docker image files are named as ".Dockerfile".

These are from subtrees.

I don't think there's anything to do here. Note that also don't need to use Docker, you can also use podman.
💬 fanquake commented on issue "Trying to run bitcoin qt on Windows and getting an AV":
(https://github.com/bitcoin/bitcoin/issues/30825#issuecomment-2700928581)
Is there anything for us to do here? Anything to be fixed? Should it just be closed? @hebasto.
hebasto closed an issue: "Trying to run bitcoin qt on Windows and getting an AV"
(https://github.com/bitcoin/bitcoin/issues/30825)
💬 eval-exec commented on pull request "ci: rename imagefile to Dockerfile to follow docker file's convention.":
(https://github.com/bitcoin/bitcoin/pull/31995#issuecomment-2700945396)
Thanks for your response!

> These are from subtrees.

Where is subtrees.

> I don't think there's anything to do here. Note that also don't need to use Docker, you can also use podman.

I just think it would be better to keep the image file names consistent. And, `podman` call it `Containerfile`. https://docs.podman.io/en/v5.3.0/markdown/podman-build.1.html
💬 fanquake commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2700952489)
@Sjors My main question is why are the unsigned binaries in this PR, behaving differently to the unsigned binaries currently produced by master, and why that new failure message/behaviour [is expected](https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2699767246)?
💬 fanquake commented on pull request "ci: rename imagefile to Dockerfile to follow docker file's convention.":
(https://github.com/bitcoin/bitcoin/pull/31995#issuecomment-2700964152)
> Where is subtrees.

https://github.com/bitcoin-core/secp256k1
https://github.com/sipa/minisketch


> I just think it would be better to keep the image file names consistent.

Ours are consistently named `{name}_imagefile`. There's no requirement that we are consistent with files from other repositories.
eval-exec closed a pull request: "ci: rename imagefile to Dockerfile to follow docker file's convention."
(https://github.com/bitcoin/bitcoin/pull/31995)
💬 eval-exec commented on pull request "ci: rename imagefile to Dockerfile to follow docker file's convention.":
(https://github.com/bitcoin/bitcoin/pull/31995#issuecomment-2700973611)
Thanks for clarifying. I appreciate it.
fanquake closed an issue: "MSVC 17.12.0 internal compiler error "
(https://github.com/bitcoin/bitcoin/issues/31303)
💬 fanquake commented on issue "MSVC 17.12.0 internal compiler error ":
(https://github.com/bitcoin/bitcoin/issues/31303#issuecomment-2700974579)
Closing, as there's nothing left to do here. Once @hebasto get's a notification from https://developercommunity.visualstudio.com/t/C1001:-Internal-compiler-error-with-st/10830350 that a new release containing the fix is available, he could follow up with dropping the workaround.
💬 willcl-ark commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2700985550)
My guix hashes:

```
d043dbd79662ee324ab809061405c71ecb4dbd9c1100ea7fcfc72bfb8e775a20 guix-build-e181bda061ca/output/aarch64-linux-gnu/SHA256SUMS.part
09160970e038df29d6c2ae3da69adf77ac496f65e114e3bbb56272465b9c230a guix-build-e181bda061ca/output/aarch64-linux-gnu/bitcoin-e181bda061ca-aarch64-linux-gnu-debug.tar.gz
f94c5f7734858dfc77717849934112cceef44a80b06fb1c21f41a78733092c6a guix-build-e181bda061ca/output/aarch64-linux-gnu/bitcoin-e181bda061ca-aarch64-linux-gnu.tar.gz
a28ae11a558562
...
👍 stickies-v approved a pull request: "[28.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/31648#pullrequestreview-2661168707)
ACK 2c372fcd5fe4f38aafa7095b990b8b805ee41ab7

All backports are clean, except;
- 4e7dd0ac2079dc04005c2d226eab9566807f146d backported from b537a2c02a9921235d1ecf8c3c7dc1836ec68131 (due to switching to CMake), looks okay to me

Backports seem sensible.

nit:
- #31611 missing from PR description
- dd32974dda0782798bf0e0b61f32aef745302663 doesn't have the `Github-Pull` and `Rebased-From` metadata
- commit authors 0xB10C and kehiy missing from release notes
👍 willcl-ark approved a pull request: "[28.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/31648#pullrequestreview-2661180973)
ACK 2c372fcd5fe4f38aafa7095b990b8b805ee41ab7
👍 laanwj approved a pull request: "torcontrol: Limit reconnect timeout to max seconds and log delay in whole seconds"
(https://github.com/bitcoin/bitcoin/pull/31979#pullrequestreview-2661185365)
Code review ACK f708498293c27f63507cfa08c74909ba3d1aa675
📝 fanquake locked a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/31990)
<!--
*** 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
...
💬 hodlinator commented on pull request "RFC: Add implicit constructor for `_hex` to `uint256` and remove `consteval_ctor`":
(https://github.com/bitcoin/bitcoin/pull/31991#discussion_r1981454718)
I'd lean towards defining `operator""_uint256` in *uint256.h*, but don't have a strong preference. *uint256.h* currently has `#include <util/strencodings.h>`, so doing it the other way would introduce cyclic include issues.