Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 l0rinc commented on issue "v30.0 Testing":
(https://github.com/bitcoin/bitcoin/issues/33368#issuecomment-3316923745)
I also just finished a full `-reindex` until `-stopatheight=915303` compiled with `AppleClang 17.0.0.17000319` against `master` with ` -assumevalid=0` (to test full script validation) and `-DCMAKE_BUILD_TYPE=Debug` (to test every assertion) and `-DHAVE_ARM_SHANI=OFF` (to test software hashing). Reindexing until block 1 took a day and reindexing the chainstate took 3 more (with default settings a reindex takes about 4-5 hours). I have cancelled it a few times, rolled the blocks forward successf
...
💬 ajtowns commented on pull request "Exponentially-decaying tx inventory queue":
(https://github.com/bitcoin/bitcoin/pull/33449#issuecomment-3317060161)
Oh! I hadn't noticed this was a different formula to what (I thought) we were discussing earlier.

I think our exponential-based inv timing implies that 1% of invs will be at least 23s after the previous inv, and the formula above will mean we immediately send 32% of our current queue at that point, which seems excessive to me? Compared to the previous sim I ran, I get 45% of the max queue size (2045 vs 4500), but 270%-580% of the max inv size.
💬 0xB10C commented on pull request "net/rpc: Report inv information for debugging":
(https://github.com/bitcoin/bitcoin/pull/33448#issuecomment-3317216893)
Concept ACK. I had exposing the inv-to-send size on my todo list too.
💬 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.