Bitcoin Core Github
44 subscribers
119K links
Download Telegram
💬 rkrux commented on pull request "psbt: MuSig2 Fields":
(https://github.com/bitcoin/bitcoin/pull/31247#discussion_r2049074691)
Nit here and in the `musig2_partial_sigs` section below.

```diff
- {RPCResult::Type::STR_HEX, "aggregate_pubkey", "The compressed aggregate public key for which this pubnonce is for."},
+ {RPCResult::Type::STR_HEX, "aggregate_pubkey", "The compressed aggregate public key for which this pubnonce is."},
```
💬 andrewtoth commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2049127611)
Just noting here that this is also racy (like reattempt). If we have more than 1 connection open and more than 1 succeeds, it will decrement the extra successful connections count.
💬 ismaelsadeeq commented on issue "Test Package Accept":
(https://github.com/bitcoin/bitcoin/issues/32160#issuecomment-2813204991)
> There is already no requirement of connectedness now

But it is currently enforced on master, this test fail
<details>
<summary>diff</summary>

```diff
diff --git a/test/functional/rpc_packages.py b/test/functional/rpc_packages.py
index a2f9210f94d..807ede343a5 100755
--- a/test/functional/rpc_packages.py
+++ b/test/functional/rpc_packages.py
@@ -88,6 +88,7 @@ class RPCPackagesTest(BitcoinTestFramework):
self.test_multiple_parents()
self.test_conflicting()
self.test_
...
💬 Sjors commented on pull request "doc: Add deps install notes for multiprocess":
(https://github.com/bitcoin/bitcoin/pull/32293#issuecomment-2813205001)
utACK ff4ea070aa4b1ed1ebb6dcf62e01c9dd57319f69

If this lands before #31802 then that needs to be updated to make this the default instruction.
👍 laanwj approved a pull request: "doc: Add deps install notes for multiprocess"
(https://github.com/bitcoin/bitcoin/pull/32293#pullrequestreview-2776074231)
ACK ff4ea070aa4b1ed1ebb6dcf62e01c9dd57319f69

This needs to be mentioned. i think adding paragraph after paragraph of dependencies to the build docs like this becomes a bit of a mess for someone without context "what do i actually have to install", but that's not something that needs to be resolved here.
💬 sipa commented on issue "Wallet should be able to store multiple transactions with same txid":
(https://github.com/bitcoin/bitcoin/issues/11240#issuecomment-2813238080)
I think this really calls for storing up to 2 variants of each transactions: the one we expect to confirm, and optionally the one we've seen accepted in a block.

Super hacky way of avoiding database upgrade issues: store inside the `wtx` record a concatenation of both serialized transactions. Old wallets (I think) will just read the first one, ignoring the second version if any.
💬 Sjors commented on issue "Stratum v2 via IPC Mining Interface tracking issue":
(https://github.com/bitcoin/bitcoin/issues/31098#issuecomment-2813323000)
All interface changes that I need for v30 have been merged!

Some progress has been made on enabling IPC for the release as well, but please review #31375 and https://github.com/bitcoin/bitcoin/pull/31802.
💬 maflcko commented on pull request "refactor: reenable `implicit-integer-sign-change` check for `serialize.h`":
(https://github.com/bitcoin/bitcoin/pull/32296#issuecomment-2813333039)
CI fails. Did this pass locally?
💬 fanquake commented on pull request "refactor: reenable `implicit-integer-sign-change` check for `serialize.h`":
(https://github.com/bitcoin/bitcoin/pull/32296#issuecomment-2813337803)
For reference (https://github.com/bitcoin/bitcoin/actions/runs/14517522442/job/40730094523?pr=32296#step:6:4644):
```bash
/home/runner/work/_temp/src/secp256k1/src/modinv64_impl.h:360:17: runtime error: implicit conversion from type 'uint64_t' (aka 'unsigned long') of value 7287561039273463316 (64-bit, unsigned) to type 'int' changed the value to 1893666324 (32-bit, signed)
#0 0x555bd4509ce3 in secp256k1_modinv64_posdivsteps_62_var /home/runner/work/_temp/src/secp256k1/src/modinv64_impl.h
...
👍 stickies-v approved a pull request: "qt, wallet: Convert uint256 to Txid"
(https://github.com/bitcoin/bitcoin/pull/32238#pullrequestreview-2776194854)
ACK afbb1a1bfd729dc5e92cc5a83be681999a2d3b43

I checked all the callsites of functions with changed signatures and verified that indeed `Txid` is the correct type.
💬 maflcko commented on pull request "[IBD] specialize block serialization":
(https://github.com/bitcoin/bitcoin/pull/31868#issuecomment-2813376299)
> While this wasn't the main motivation for the change, IBD on Ubuntu/GCC on SSD with i9 indicates a 2% speedup as well:

If IBD speedup is not the main motivation, what else is the motivation, given that the title mentions `[IBD]`?

Also, what code part is hitting this during IBD, given that reads and writes are now buffered?
📝 l0rinc converted_to_draft a pull request: "[IBD] specialize block serialization"
(https://github.com/bitcoin/bitcoin/pull/31868)
This change is part of [[IBD] - Tracking PR for speeding up Initial Block Download](https://github.com/bitcoin/bitcoin/pull/32043)

### Summary

This PR contain a few different optimization I found by IBD profiling, and via the newly added block seralization benchmarks. It also takes advantage of the recently merged [`std::span` changes](https://github.com/bitcoin/bitcoin/pull/31519) enabling propagating static extents.

The commits merge similar (de)serialization methods, and separates th
...
💬 l0rinc commented on pull request "[IBD] specialize block serialization":
(https://github.com/bitcoin/bitcoin/pull/31868#discussion_r2049233469)
Yes:
> typedef unsigned char uint8_t;
💬 l0rinc commented on pull request "[IBD] specialize block serialization":
(https://github.com/bitcoin/bitcoin/pull/31868#issuecomment-2813392140)
> what code part is hitting this during IBD

During serialization we have very many 1 byte writes, this change avoids the heavy allocations.
But drafting until I have time to properly remeasure everything after the recent merge.
💬 maflcko commented on pull request "[IBD] specialize block serialization":
(https://github.com/bitcoin/bitcoin/pull/31868#issuecomment-2813398761)
I understand that there are many 1-byte writes, but they go into the BufferedReader/Writer, which is not modified here. The Buffered* then passes them to AutoFile, which is modified here, but never receives 1-byte writes anymore
💬 marcofleon commented on pull request "qt, wallet: Convert uint256 to Txid":
(https://github.com/bitcoin/bitcoin/pull/32238#discussion_r2049250853)
Good call, done. Made me realize I should update `vMatch` in the parent PR, so thanks.
💬 marcofleon commented on pull request "qt, wallet: Convert uint256 to Txid":
(https://github.com/bitcoin/bitcoin/pull/32238#discussion_r2049251124)
Done.
💬 marcofleon commented on pull request "qt, wallet: Convert uint256 to Txid":
(https://github.com/bitcoin/bitcoin/pull/32238#discussion_r2049251391)
Done.
💬 marcofleon commented on pull request "qt, wallet: Convert uint256 to Txid":
(https://github.com/bitcoin/bitcoin/pull/32238#discussion_r2049251829)
Done.
💬 marcofleon commented on pull request "qt, wallet: Convert uint256 to Txid":
(https://github.com/bitcoin/bitcoin/pull/32238#discussion_r2049252754)
Done.