๐ฌ pinheadmz commented on pull request "[28.x] Backport #31407":
(https://github.com/bitcoin/bitcoin/pull/32563#issuecomment-2891325049)
codesigning hung forever at one point. I SIGINT it and got a possibly helpful error:
```
--> ./detached-sig-create.sh <redacted>
WARNING: Part of the file was not parsed: 37803 bytes
Enter the passphrase for <redacted>:
Enter the passphrase for <redacted>:
WARNING: Part of the file was not parsed: 37803 bytes
Code signature created
WARNING: Part of the file was not parsed: 37803 bytes
WARNING: Part of the file was not parsed: 37803 bytes
Code signature applied
WARNING: Part of the f
...
(https://github.com/bitcoin/bitcoin/pull/32563#issuecomment-2891325049)
codesigning hung forever at one point. I SIGINT it and got a possibly helpful error:
```
--> ./detached-sig-create.sh <redacted>
WARNING: Part of the file was not parsed: 37803 bytes
Enter the passphrase for <redacted>:
Enter the passphrase for <redacted>:
WARNING: Part of the file was not parsed: 37803 bytes
Code signature created
WARNING: Part of the file was not parsed: 37803 bytes
WARNING: Part of the file was not parsed: 37803 bytes
Code signature applied
WARNING: Part of the f
...
๐ฌ hebasto commented on pull request "Set minimum supported Windows version to 1903 (May 2019 Update)":
(https://github.com/bitcoin/bitcoin/pull/32537#discussion_r2095901559)
You're right. I've misread docs...
(https://github.com/bitcoin/bitcoin/pull/32537#discussion_r2095901559)
You're right. I've misread docs...
๐ hebasto converted_to_draft a pull request: "Set minimum supported Windows version to 1903 (May 2019 Update)"
(https://github.com/bitcoin/bitcoin/pull/32537)
This PR sets the minimum supported Windows version to 1903 (May 2019 Update).
This version is the minimum [required](https://learn.microsoft.com/en-us/windows/apps/design/globalizing/use-utf8-code-page#set-a-process-code-page-to-utf-8) to support the UTF-8 code page (CP_UTF8), which is necessary for https://github.com/bitcoin/bitcoin/pull/32380.
Additionally, the `symbol-check.py` script has been amended to verify application manifests for supported OS value.
(https://github.com/bitcoin/bitcoin/pull/32537)
This PR sets the minimum supported Windows version to 1903 (May 2019 Update).
This version is the minimum [required](https://learn.microsoft.com/en-us/windows/apps/design/globalizing/use-utf8-code-page#set-a-process-code-page-to-utf-8) to support the UTF-8 code page (CP_UTF8), which is necessary for https://github.com/bitcoin/bitcoin/pull/32380.
Additionally, the `symbol-check.py` script has been amended to verify application manifests for supported OS value.
๐ laanwj opened a pull request: "Use subprocess library for notifications"
(https://github.com/bitcoin/bitcoin/pull/32566)
Builds on #29868 (to have windows support).
This switches the `-*notify` options to use the subprocess library, which is currently used for only the external signer. The main advantage of this is that file descriptor leaks are avoided (#32343). This could also add future flexibility in notifications, for example to bypass the shell (and launch commands with their arguments directly, for safety), or do something with command output (like log it). But for now, this is meant to be one-to-one.
...
(https://github.com/bitcoin/bitcoin/pull/32566)
Builds on #29868 (to have windows support).
This switches the `-*notify` options to use the subprocess library, which is currently used for only the external signer. The main advantage of this is that file descriptor leaks are avoided (#32343). This could also add future flexibility in notifications, for example to bypass the shell (and launch commands with their arguments directly, for safety), or do something with command output (like log it). But for now, this is meant to be one-to-one.
...
๐ฌ l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2095911064)
This is the error I got previously for constexpr string: https://cirrus-ci.com/task/4531348515323904?logs=ci#L1428
Changed it back to private, thanks.
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2095911064)
This is the error I got previously for constexpr string: https://cirrus-ci.com/task/4531348515323904?logs=ci#L1428
Changed it back to private, thanks.
๐ฌ l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2095911559)
I'm against throwing in general - especially for constructors, but we're already reading files in there and already throwing via `HandleError` - changed and moved to the related refactoring commit.
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2095911559)
I'm against throwing in general - especially for constructors, but we're already reading files in there and already throwing via `HandleError` - changed and moved to the related refactoring commit.
๐ฌ l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2095911656)
โจ โโ๐แตข๐ธ โจ
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2095911656)
โจ โโ๐แตข๐ธ โจ
๐ฌ l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2095911786)
In my defense, I was hungry ๐ฅฉ
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2095911786)
In my defense, I was hungry ๐ฅฉ
๐ฌ l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2095912020)
The `new`/`old` are wrong indeed - but the explanation for why we're using a different constructor here needs local explanation - reworded, thanks
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2095912020)
The `new`/`old` are wrong indeed - but the explanation for why we're using a different constructor here needs local explanation - reworded, thanks
๐ฌ hebasto commented on pull request "Set minimum supported Windows version to 1903 (May 2019 Update)":
(https://github.com/bitcoin/bitcoin/pull/32537#discussion_r2095916084)
In its current state, this PR is not necessary for introducing the UTF-8 code page support.
(https://github.com/bitcoin/bitcoin/pull/32537#discussion_r2095916084)
In its current state, this PR is not necessary for introducing the UTF-8 code page support.
โ
hebasto closed a pull request: "Set minimum supported Windows version to 1903 (May 2019 Update)"
(https://github.com/bitcoin/bitcoin/pull/32537)
(https://github.com/bitcoin/bitcoin/pull/32537)
๐ stickies-v approved a pull request: "doc: remove // for ... comments"
(https://github.com/bitcoin/bitcoin/pull/32562#pullrequestreview-2851071402)
ACK e4aee0b3a23a9609e16bd7c8eda618dcf0a8b1db
The new includes seem arbitrary, they usually still don't represent the full include list (according to [IWYU](https://api.cirrus-ci.com/v1/task/4921334028828672/logs/ci.log)), and could be dropped, but I verified them against the IWYU run and mostly seem fine (I added comments for the ones that seem incorrect.
Found a couple more include comments in `util/fs_helpers.cpp`
(https://github.com/bitcoin/bitcoin/pull/32562#pullrequestreview-2851071402)
ACK e4aee0b3a23a9609e16bd7c8eda618dcf0a8b1db
The new includes seem arbitrary, they usually still don't represent the full include list (according to [IWYU](https://api.cirrus-ci.com/v1/task/4921334028828672/logs/ci.log)), and could be dropped, but I verified them against the IWYU run and mostly seem fine (I added comments for the ones that seem incorrect.
Found a couple more include comments in `util/fs_helpers.cpp`
๐ฌ stickies-v commented on pull request "doc: remove // for ... comments":
(https://github.com/bitcoin/bitcoin/pull/32562#discussion_r2095904411)
nit: `#include <limits.h>` can be removed according to IWYU I suspect because we also `#include <limits>`
(https://github.com/bitcoin/bitcoin/pull/32562#discussion_r2095904411)
nit: `#include <limits.h>` can be removed according to IWYU I suspect because we also `#include <limits>`
๐ฌ stickies-v commented on pull request "doc: remove // for ... comments":
(https://github.com/bitcoin/bitcoin/pull/32562#discussion_r2095909033)
nit: `chrono` and `string` are not required according to [IWYU](https://api.cirrus-ci.com/v1/task/4921334028828672/logs/ci.log)
(https://github.com/bitcoin/bitcoin/pull/32562#discussion_r2095909033)
nit: `chrono` and `string` are not required according to [IWYU](https://api.cirrus-ci.com/v1/task/4921334028828672/logs/ci.log)
๐ฌ stickies-v commented on pull request "doc: remove // for ... comments":
(https://github.com/bitcoin/bitcoin/pull/32562#discussion_r2095917052)
nit: `cstring` and `string` includes can just be dropped according to [IWYU](https://api.cirrus-ci.com/v1/task/4921334028828672/logs/ci.log)
(https://github.com/bitcoin/bitcoin/pull/32562#discussion_r2095917052)
nit: `cstring` and `string` includes can just be dropped according to [IWYU](https://api.cirrus-ci.com/v1/task/4921334028828672/logs/ci.log)
๐ฌ stickies-v commented on pull request "doc: remove // for ... comments":
(https://github.com/bitcoin/bitcoin/pull/32562#discussion_r2095910935)
nit: the `span.h` include can now be removed according to [IWYU](https://api.cirrus-ci.com/v1/task/4921334028828672/logs/ci.log)
(https://github.com/bitcoin/bitcoin/pull/32562#discussion_r2095910935)
nit: the `span.h` include can now be removed according to [IWYU](https://api.cirrus-ci.com/v1/task/4921334028828672/logs/ci.log)
๐ l0rinc's pull request is ready for review: "script: short-circuit `GetLegacySigOpCount` for known scripts"
(https://github.com/bitcoin/bitcoin/pull/32532)
(https://github.com/bitcoin/bitcoin/pull/32532)
๐ฌ maflcko commented on pull request "ci: Move DEBUG=1 to centos task":
(https://github.com/bitcoin/bitcoin/pull/32560#discussion_r2095935838)
No reason. I'd say anything is fine here.
Initially this was set in 14788fbadac3aa352eb51da81ecdfa01208ca33c, but we have a larger cache now.
(https://github.com/bitcoin/bitcoin/pull/32560#discussion_r2095935838)
No reason. I'd say anything is fine here.
Initially this was set in 14788fbadac3aa352eb51da81ecdfa01208ca33c, but we have a larger cache now.
๐ hebasto's pull request is ready for review: "Modernize use of UTF-8 in Windows code"
(https://github.com/bitcoin/bitcoin/pull/32380)
(https://github.com/bitcoin/bitcoin/pull/32380)
๐ฌ maflcko commented on pull request "Use WitnessV0KeyHash in TestAddAddressesToSendBook":
(https://github.com/bitcoin-core/gui/pull/875#issuecomment-2891405513)
can someone pls assign 30.0 milestone?
(https://github.com/bitcoin-core/gui/pull/875#issuecomment-2891405513)
can someone pls assign 30.0 milestone?