💬 hodlinator commented on pull request "qa: Improvements to debug_assert_log + busy_wait_for_debug_log":
(https://github.com/bitcoin/bitcoin/pull/33423#issuecomment-3317267505)
CI failure (https://github.com/bitcoin/bitcoin/actions/runs/17885097855/job/50857723211?pr=33423) seems unrelated:

`ERROR: failed to build: failed to solve: short read: expected 659078263 bytes but got 90259897: unexpected EOF`
`Command '['./ci/test/02_run_container.sh']' returned non-zero exit status 1.`

💬 trevarj commented on pull request "guix: update time-machine to 5cb84f2013c5b1e48a7d0e617032266f1e6059e2":
(https://github.com/bitcoin/bitcoin/pull/33185#issuecomment-3317550917)
Concept ACK, but perhaps to a more recent commit for an unrelated reason.

Anywhere after Guix commit [af9e540b7](https://codeberg.org/guix/guix/commit/af9e540b716402df983935eeabedc4572d6565ce) would allow for a developer to use the current `manifest.scm` as an isolated dev environment. I am doing this now but need to modify one line (certs package moved to nss) and it works well.

> I still don't know how to install an individual stubborn package like this from a substitute server

@Sjor
...

🤔 hodlinator reviewed a pull request: "Modernize use of UTF-8 in Windows code"
(https://github.com/bitcoin/bitcoin/pull/32380#pullrequestreview-3251179556)
Reviewed aa830373e87325f08b647d41ac3ab5f5196e46c8

Built a Guix build and tested on Windows. Verified all installed binaries worked. Also verified natively built bitcoind worked.

<details><summary>Checked embedded resources using mt</summary>

```
C:\Program Files\Bitcoin\daemon>mt -inputresource:bitcoind.exe;#1 -out:con
Microsoft (R) Manifest Tool
Copyright (c) Microsoft Corporation.
All rights reserved.
<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<assembly xmlns="urn:
...

💬 hodlinator commented on pull request "Modernize use of UTF-8 in Windows code":
(https://github.com/bitcoin/bitcoin/pull/32380#discussion_r2367130713)
(Inline comment in unrelated file)

Seems like `SetupEnvironment` is missing from the new bitcoin.cpp

```diff
diff --git a/src/bitcoin.cpp b/src/bitcoin.cpp
index c327827415..766f7bad36 100644
--- a/src/bitcoin.cpp
+++ b/src/bitcoin.cpp
@@ -59,6 +59,8 @@ static void ExecCommand(const std::vector<const char*>& args, std::string_view a

int main(int argc, char* argv[])
{
+ SetupEnvironment();
+
try {
CommandLine cmd{ParseCommandLine(argc, argv)};
if (c
...

💬 hodlinator commented on pull request "Modernize use of UTF-8 in Windows code":
(https://github.com/bitcoin/bitcoin/pull/32380#discussion_r2367127283)
Modified bitcoind I installed from a guix-generated version of this PR in an admin-prompt:
```
C:\Program Files\Bitcoin\daemon>mt -nologo -manifest C:\Users\hodlinator\Documents\cp1252.manifest -outputresource:"bitcoind.exe"
```

<details><summary>cp1552.manifest</summary>

```diff
<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<assembly xmlns="urn:schemas-microsoft-com:asm.v1" manifestVersion="1.0" xmlns:asmv3="urn:schemas-microsoft-com:asm.v3">
<assemblyIdentity type="w
...

💬 hebasto commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#discussion_r2367227845)
@davidgumberg

> I think this approach is no less invasive then what is done currently using an external script...

I disagree. I've checked c44fb4facfd3a5db8aa1bd2c7f5506a1477b0d2b. The `env:` sections in the CI logs are indeed more bloated compared to the master branch. I'd prefer to avoid this.

💬 hebasto commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#issuecomment-3317735731)
For the second commit, you might consider using a YAML anchor:
```diff
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -211,7 +211,8 @@ jobs:
steps:
- *CHECKOUT

- - name: Set up VS Developer Prompt
+ - &SET_UP_VS
+ name: Set up VS Developer Prompt
shell: pwsh -Command "$PSVersionTable; $PSNativeCommandUseErrorActionPreference = $true; $ErrorActionPreference = 'Stop'; & '{0}'"
run: |
$vswherePath = "${env:Pr
...

💬 hebasto commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#issuecomment-3317763545)
> While in this area of the code, this PR also runs some additional manifest checks on the windows binaries.

> ... there are still two places where `shell: pwsh` is used without your ` -Command "$PSVersionTable; $PSNativeCommandUseErrorActionPreference = $true; $ErrorActionPreference = 'Stop'; & '{0}'"` suffix...

While I do agree with the concerns above, it might be better to keep this PR focused on resolving https://github.com/bitcoin/bitcoin/issues/32508.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2367465700)
Brought it back in 4ead575b252becbfa18ad28a7bfa7fbf3de7129d

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2367502150)
Squashed this commit.

💬 vasild commented on pull request "system: improve handling around GetTotalRAM()":
(https://github.com/bitcoin/bitcoin/pull/33435#issuecomment-3318101491)
`8fcf71ca00...56791b5829`: take suggestions

💬 vasild commented on pull request "system: improve handling around GetTotalRAM()":
(https://github.com/bitcoin/bitcoin/pull/33435#discussion_r2367509262)
Done.

💬 vasild commented on pull request "system: improve handling around GetTotalRAM()":
(https://github.com/bitcoin/bitcoin/pull/33435#discussion_r2367509839)
Done.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2367523246)
Did the latter, thanks.

👍 hebasto approved a pull request: "system: improve handling around GetTotalRAM()"
(https://github.com/bitcoin/bitcoin/pull/33435#pullrequestreview-3251759768)
ACK 56791b582958e905e5ba5cbf172a8ea7dad1a8b0.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2367557960)
Done in 7cebfec8a6197a260e8c21f4637fad5d60163cfa

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2367560636)
Fixed.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2367572401)
Should be better now

📝 bitschmidty opened a pull request: "datacarrier: Undeprecate configuration option"
(https://github.com/bitcoin/bitcoin/pull/33453)
Removes the deprecation for the `datacarrier` and `datacarriersize` options by reverting commit 0b4048c73385166144d0b3e76beb9a2ac4cc1eca from https://github.com/bitcoin/bitcoin/pull/32406

**Many current Bitcoin Core users want to continue using this option**
This statement is based on public postings from many Bitcoin Core users and not a formal survey. AJ Towns’ observation from [#32406](https://github.com/bitcoin/bitcoin/pull/32406/commits/0b4048c73385166144d0b3e76beb9a2ac4cc1eca#r20840248
...

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2367581993)
Fixed now.