Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 l0rinc commented on pull request "net/rpc: Report inv information for debugging":
(https://github.com/bitcoin/bitcoin/pull/33448#discussion_r2366371242)
The CI failures indicate that we need to add the other field as well:
```suggestion
"last_inv_sequence": 0,
"inv_to_send": 0,
```
---
tested locally that it fixes the tests:
```
TEST | STATUS | DURATION

rpc_net.py --v1transport | ✓ Passed | 12 s
rpc_net.py --v2transport | ✓ Passed | 10 s
```
💬 TheCharlatan commented on pull request "depends: static libxcb-cursor":
(https://github.com/bitcoin/bitcoin/pull/33434#issuecomment-3316249701)
Concept ACK

I'm not familiar enough with X to say why it's seemingly not possible to link all the `xcb_utils` statically, but this change seems sane to me.
📝 hebasto opened a pull request: "Release: 30.0rc2 translations update"
(https://github.com/bitcoin/bitcoin/pull/33452)
This PR updates Spanish (es) and Czech (cs) translations and addresses the following comments:
- https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3315273628
- https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3316206549

Updates for other languages were skipped, as I believe the review effort would not be worthwhile at this stage of the release process.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads 10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-3316319902)
I benchmarked the latest branch with default dbcache up to 912683. Results are a speedup of 14% - 5:06 vs 5:47.

| Command | Mean [s] | Min [s] | Max [s] | Relative |
|:---|---:|---:|---:|---:|
| `echo 688c03597afb0b76077f1ffc4608eef19481056e && /usr/bin/time ./build/bin/bitcoind -printtoconsole=0 -connect=192.168.2.171 -stopatheight=912683` | 18430.672 ± 19.856 | 18416.631 | 18444.712 | 1.00 |
| `echo 1444ed855f438f1270104fca259ce61b99ed5cdb && /usr/bin/time ./build/bin/bitcoind -printtoco
...
💬 polespinasa commented on issue "Cleanup CFeeRate constructor (sat/vB vs BTC/kvB)":
(https://github.com/bitcoin/bitcoin/issues/23129#issuecomment-3316478295)
Is this still relevant after #32750 is merged?
💬 ajtowns commented on pull request "Exponentially-decaying tx inventory queue":
(https://github.com/bitcoin/bitcoin/pull/33449#issuecomment-3316552769)
It's not the queue that's exponentially decaying, but the inv queue drain rate tracking the relay rate with an exponential decay? *shrug*

The previous PR, #33448, may be helpful for observing the behaviour of this PR in the wild. I use the following command occassionally:

```sh
bitcoin-cli getpeerinfo | jq -j '.[] | (.inv_to_send, " ", .last_inv_sequence, " ", .connection_type, " ", .id, " ", .relaytxes, " ", .synced_blocks, " ", (.network|sub("not_publicly_routable";"onion")), " inv-rx:"
...
💬 sipa commented on pull request "Exponentially-decaying tx inventory queue":
(https://github.com/bitcoin/bitcoin/pull/33449#issuecomment-3316566982)
@ajtowns What I meant is actually the queue itself whose size decays exponentially, though only in a fairly constrained setting.

I believe the following statement holds:

If at time $T$ the size of the queue is $N$, and after that, no more than $B$ inv elements get added to the queue per trickle, then after any trickle occurring at time $T+t$, the size of the queue is bounded by $\left\lceil N \exp(-t/A) \right\rceil$, regardless of how many trickles happened in between, or when those happened.
...
💬 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.