💬 rkrux commented on pull request "psbt: MuSig2 Fields":
(https://github.com/bitcoin/bitcoin/pull/31247#discussion_r2049044328)
This portion is exactly same as the one used in the nonces case, can be extracted into a `DeserializeMuSig2PartialSigOrPubNonceKey` function because the format of the key is same in both the cases.

Though the duplication is not enough but the deduplicated version would enforce the similarity of the key format to the reader easily.

💬 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.