💬 stickies-v commented on issue "Slow memory leak in v22.0?":
(https://github.com/bitcoin/bitcoin/issues/24542#issuecomment-2104860540)
> but it is then copied into a [second UniValue](https://github.com/bitcoin/bitcoin/blob/98dd4e712efaa2b77cb168426756879c6405c3f4/src/rpc/blockchain.cpp#L196), doubling the required heap size.

And if I understand correctly, it is then copied _again_ [here](https://github.com/bitcoin/bitcoin/blob/98dd4e712efaa2b77cb168426756879c6405c3f4/src/univalue/lib/univalue.cpp#L134) since `UniValue` doesn't have a move constructor. So improving UniValue move semantics seems to make sense here indeed.

👍 hebasto approved a pull request: "build: swap cctools otool for llvm-objdump"
(https://github.com/bitcoin/bitcoin/pull/29739#pullrequestreview-2050387339)
ACK 7f5ac4520d1553170b1053a9ffcd58179386a6d2.

After migration to CMake, there is a possibility that CMake will expect to find `otool` for the "Darwin" systems. But we can deal with this in due time.

💬 achow101 commented on pull request "test: add missing comparison of node1's mempool in MempoolPackagesTest":
(https://github.com/bitcoin/bitcoin/pull/29948#issuecomment-2104876107)
ACK e912717ff63f111d8f1cd7ed1fcf054e28f36409

🤔 jonatack reviewed a pull request: "doc: removed help text saying that peers may not connect automatically"
(https://github.com/bitcoin/bitcoin/pull/29994#pullrequestreview-2050435690)
@vasild thank you, rectifying my comment.

ACK 95897ff181c0757e445f0e066a2a590a0a0120d2

New help:
```
$ ./src/bitcoind -h | grep -A2 "\-port\="
-port=<port>
Listen for connections on <port> (default: 8333, testnet: 18333, signet:
38333, regtest: 18444). Not relevant for I2P (see doc/i2p.md).
```

💬 libreisaac commented on issue "libxcb-xinerama0 Library required by bitcoin-qt":
(https://github.com/bitcoin/bitcoin/issues/30061#issuecomment-2104915543)
> Okay, thank you for your response on this. I see your points and also agree that btc core can't and shouldn't deal with os related graphics peculiarities. Just from a UX/security perspective, I was a bit surprised about it when first encountered. Maybe it could be made optional at some point or a more verbose error message can be printed. In any case, no big issue. I'll close this!

If you're concerned about trusting binaries installed by your OS' package manager, consider Gentoo. But fundamen
...

💬 josibake commented on pull request "crypto, refactor: add method for applying the taptweak":
(https://github.com/bitcoin/bitcoin/pull/30051#discussion_r1596979700)
Done.

🚀 achow101 merged a pull request: "test: add missing comparison of node1's mempool in MempoolPackagesTest"
(https://github.com/bitcoin/bitcoin/pull/29948)

💬 josibake commented on pull request "crypto, refactor: add method for applying the taptweak":
(https://github.com/bitcoin/bitcoin/pull/30051#issuecomment-2104921312)
@theuni Updated with a comment and added `KeyPair` to the BIP340 test vectors. This does test all possible merkle_root states and ensures everything is 1-to-1 with `XOnlyPubKey::ComputeTapTweakHash` and `CKey::SignSchnorr`

🤔 instagibbs reviewed a pull request: "p2p: index TxOrphanage by wtxid, allow entries with same txid"
(https://github.com/bitcoin/bitcoin/pull/30000#pullrequestreview-2050241117)
reviewed through 65f69bf533f66ecfb2695b03204bd437f1fe692d

> Given that a Bitcoin Core node doesn't ever do witness replacement, this seems like unnecessary complexity that won't ever get used between normally-operating nodes. So I did not add this.

Makes sense, seems unlikely to ever be used in a 20 minute window lifetime of an orphan given that we don't do these kinds of replacements.

also opened https://github.com/bitcoin/bitcoin/pull/30082 seeing we're touching `EraseForPeer` et al a
...

💬 instagibbs commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1596969302)
```Suggestion
# exists in orphanage, but should be re-requested due to having witness data.
```

💬 instagibbs commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1596930516)
but `wait_for_verack=False` isn't being set?

💬 instagibbs commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1596975768)
```Suggestion
assert_equal(node.getrawmempool(), [])

# 5. The parent is requested. Honest peer sends it.
```

💬 instagibbs commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1596966608)
```Suggestion
# 2. Node requests tx_grandparent by txid.
```

💬 instagibbs commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1596929262)
but you are sending verack just below?

💬 instagibbs commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1596947704)
might want to note this line is necessary for the reconsideration step to always happen

💬 instagibbs commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1596977375)
is `send_message` + `sync_with_ping` always enough to resolve two generations of orphans to ensure we hit the mempool?

💬 instagibbs commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1596981173)
```Suggestion
# 3. Honest peer announces the real child by txid (this isn't common but the node should
```

💬 instagibbs commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1596988905)
note for readers: If the child tx ended up being the same wtxid, and in that case the node would simply not add to the orphanage or request anything further.

📝 kevkevinpal reopened a pull request: "doc: removed help text saying that peers may not connect automatically"
(https://github.com/bitcoin/bitcoin/pull/29994)
Introduced in https://github.com/bitcoin/bitcoin/pull/23542 and released in version 23.0 there has been significant time since this change (2 years).

This should be removed as it is no longer relevant

💬 Sjors commented on pull request "util: check for errors after close and read in AutoFile":
(https://github.com/bitcoin/bitcoin/pull/29307#issuecomment-2104945955)
@vasild CI seems unhappy.