Bitcoin Core Github
44 subscribers
112K links
Download Telegram
💬 achow101 commented on pull request "index: Fix coinstats overflow":
(https://github.com/bitcoin/bitcoin/pull/30469#issuecomment-3267672803)
ACK c76797481155754329ec6a6f58e8402569043944
🤔 mzumsande reviewed a pull request: "index: Fix coinstats overflow"
(https://github.com/bitcoin/bitcoin/pull/30469#pullrequestreview-3197889608)
Tested / Code Review ACK c76797481155754329ec6a6f58e8402569043944

Please adjust the PR description to the current approach ("switching most of the values tracked in the index from historical totals to values per block" is oudated).

I synced both current branch and master on signet with `undefined` sanitizer, and verified that muhash / blockstats at selected blocks are identical, and that the overflow doesn't happen anymore at height 112516.
💬 l0rinc commented on issue "use -loadblock to load blk*****.dat files, but the blocks in it are not recognized":
(https://github.com/bitcoin/bitcoin/issues/33280#issuecomment-3267757875)
> A simple workaround for the use case here should be to use -blocksxor=0 in all datadirs.

But that would only work for a pristine IBD, right? I have opened https://github.com/bitcoin/bitcoin/pull/33324 to be able to change the obfuscation of existing blocks - this should allow you to remove an existing obfuscation after block download.
💬 BenWestgate commented on pull request "doc: update multisig tutorial to use multipath descriptors":
(https://github.com/bitcoin/bitcoin/pull/33286#discussion_r2331256410)
Master returns xpub with change index so I replace() inline to maintain the same derivation depth.
This matches the method in our Tutorial and is less code. Finally, if the future wallet ever outputs a multipath descriptor, we'll get an xpub in this format and not have to add `/<0;1>/*`.
💬 Raimo33 commented on pull request "Fix benchmark CSV output":
(https://github.com/bitcoin/bitcoin/pull/33340#issuecomment-3267809025)
Code Review ACK 790b440197bde322432a5bab161f1869b667e681
💬 BenWestgate commented on pull request "doc: update multisig tutorial to use multipath descriptors":
(https://github.com/bitcoin/bitcoin/pull/33286#discussion_r2331277450)
I don't understand this nit. I tried:
```bash
for ((n=1;n<=3;n++))
do
./build/bin/bitcoin rpc -signet createwallet "participant_${n}"
done
```
and got: `bash: ./build/bin/bitcoin: No such file or directory`
💬 achow101 commented on pull request "clang-tidy: Disable `UndefinedBinaryOperatorResult` check in `src/ipc`":
(https://github.com/bitcoin/bitcoin/pull/33312#issuecomment-3267858690)
ACK 589b65f06c3376df4cde3fac5c1d39a2d3254920
achow101 closed an issue: "ci: tidy job is producing output"
(https://github.com/bitcoin/bitcoin/issues/33256)
🚀 achow101 merged a pull request: "clang-tidy: Disable `UndefinedBinaryOperatorResult` check in `src/ipc`"
(https://github.com/bitcoin/bitcoin/pull/33312)
💬 achow101 commented on pull request "net: Add interrupt to pcp retry loop":
(https://github.com/bitcoin/bitcoin/pull/33338#issuecomment-3267870639)
ACK 188de70c86414b8b2ad5143f5c607b67686526ea
🚀 achow101 merged a pull request: "net: Add interrupt to pcp retry loop"
(https://github.com/bitcoin/bitcoin/pull/33338)
👍 hebasto approved a pull request: "ci: Checkout latest merged pulls"
(https://github.com/bitcoin/bitcoin/pull/33303#pullrequestreview-3198124781)
ACK fa8f081af31cd99155c7545643e7b10beb26714d.
💬 fjahr commented on pull request "index: Fix coinstats overflow":
(https://github.com/bitcoin/bitcoin/pull/30469#issuecomment-3267981848)
> Please adjust the PR description to the current approach ("switching most of the values tracked in the index from historical totals to values per block" is outdated).

Done
💬 Raimo33 commented on issue "[IBD] Raspberry Pi: 90% CPU time for 1.5% of block processing":
(https://github.com/bitcoin/bitcoin/issues/32832#issuecomment-3268007494)
I think I found the root cause, not tested, just speculating:

New Raspberry devices (Pi 3, Pi 4, etc.) are 64-bit CPUs. They can also be setup to run in 32bit mode, but I assume both the above benchmarks and issues used them in the default 64bit mode.

the incriminated functions: `secp256k1_fe_mul_inner` and `secp256k1_fe_sqr_inner` are notably the most expensive operations of libsecp256k1...

libsecp256k1 has various versions of these functions, compiled selectively based on the target archite
...
💬 Crypt-iQ commented on pull request "net: check for empty header before calling FillBlock":
(https://github.com/bitcoin/bitcoin/pull/33296#discussion_r2331383900)
I originally decided not to, but makes sense. Done in 5e585a0fc4fd68dd7b4982054b34deae2e7aeb89
💬 Crypt-iQ commented on pull request "net: check for empty header before calling FillBlock":
(https://github.com/bitcoin/bitcoin/pull/33296#discussion_r2331384232)
Thanks, done in 5e585a0fc4fd68dd7b4982054b34deae2e7aeb89
💬 l0rinc commented on issue "[IBD] Raspberry Pi: 90% CPU time for 1.5% of block processing":
(https://github.com/bitcoin/bitcoin/issues/32832#issuecomment-3268032980)
Thanks for the hint. I'm planning on investigating in more detail, but I have noticed my intuitions were off often, I try not to speculate anymore, it's why it would help if we could back these by any concrete measurements before we attempt a fix.
💬 instagibbs commented on pull request "net: check for empty header before calling FillBlock":
(https://github.com/bitcoin/bitcoin/pull/33296#issuecomment-3268044429)
@fjahr definitely not ideal, this PR is basically only removing the Assume crash while better state machine can be worked on
💬 Raimo33 commented on issue "[IBD] Raspberry Pi: 90% CPU time for 1.5% of block processing":
(https://github.com/bitcoin/bitcoin/issues/32832#issuecomment-3268045072)
> Thanks for the hint. I'm planning on investigating in more detail, but I have noticed my intuitions were off often, I try not to speculate anymore, it's why it would help if we could back these by any concrete measurements before we attempt a fix.

Agreed, unfortunately I don't know how much evidence I can gather without having access to those machines, but will see what I can do. If I were you, I would start by checking if this holds true:

> I wouldn't be surprised if the 32bit version Pi wo
...
💬 sipa commented on issue "[IBD] Raspberry Pi: 90% CPU time for 1.5% of block processing":
(https://github.com/bitcoin/bitcoin/issues/32832#issuecomment-3268045724)
Is this before or after the assumevalid point? If before, the secp256k1 operations won't even be used.

And if you are on a 64-bit ARM system, you absolutely want to use the 64-bit secp256k1 field implementations. They perform 4x fewer multiplications than the 32-bit ones. The asm optimizations compensate somewhat for that by better pipelining, but (1) can't overcome the fact that you just need far more arithmetic operations on 32-bit, and (2) the asm optimizations aren't even enabled by default
...
💬 Raimo33 commented on issue "[IBD] Raspberry Pi: 90% CPU time for 1.5% of block processing":
(https://github.com/bitcoin/bitcoin/issues/32832#issuecomment-3268049230)
> the asm optimizations aren't even enabled by default in Bitcoin Core builds.

Is there a specific reason for this?