Bitcoin Core Github
44 subscribers
120K links
Download Telegram
πŸ’¬ 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.
πŸ’¬ bitschmidty commented on pull request "datacarrier: Undeprecate configuration option":
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3318166185)
I open this PR with the goal of having the deprecation removed for the upcoming v30 release, in order to minimize any additional confusion around this option.

I ask that any Bitcoin Core users that want to indicate support or opposition to this PR without providing technical feedback use the πŸ‘ or πŸ‘Ž reactions rather than an ACK/NACK in hopes of keeping this PR easy and expeditious to review.

_Disclaimer: I am the executive director of Brink, an organization that funds some Bitcoin Core de
...
πŸ’¬ sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2367587173)
Done.
βœ… sipa closed a pull request: "Exponentially-decaying tx inventory queue"
(https://github.com/bitcoin/bitcoin/pull/33449)