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

💬 kevkevinpal commented on pull request "doc: removed help text saying that peers may not connect automatically":
(https://github.com/bitcoin/bitcoin/pull/29994#issuecomment-2104948425)
> ACK [95897ff](https://github.com/bitcoin/bitcoin/commit/95897ff181c0757e445f0e066a2a590a0a0120d2)
>
> Why exactly was this closed?
>
> > I'm not sure this should be removed unless it has been observed to no longer be the case
>
> > According to https://bitnodes.io/dashboard/, ~85% of public nodes are running 23 or higher
>
> Those are (currently) 82.3% of the listening nodes. I think "unlikely to get incoming connections" is not true anymore and this sentence can be dropped.
>
>
...

💬 achow101 commented on pull request "kernel: Remove key module from kernel library":
(https://github.com/bitcoin/bitcoin/pull/29252#issuecomment-2104971798)
ACK 96378fe734e5fb6167eb20036d7170572a647edb

👍 kristapsk approved a pull request: "doc: removed help text saying that peers may not connect automatically"
(https://github.com/bitcoin/bitcoin/pull/29994#pullrequestreview-2050507810)
ACK 95897ff181c0757e445f0e066a2a590a0a0120d2. According to https://bitnodes.io/dashboard/#user-agents stats, most nodes on the network are v23+.

🚀 achow101 merged a pull request: "kernel: Remove key module from kernel library"
(https://github.com/bitcoin/bitcoin/pull/29252)

👍 theuni approved a pull request: "refactor: Model the bech32 charlimit as an Enum"
(https://github.com/bitcoin/bitcoin/pull/30047#pullrequestreview-2050530629)
Ok. I spent some time working on a correct-by-construction approach here with a new `Bech32Encoded` string type which knows its own size limit. Ultimately I don't think it's worth the complexity.

What's here doesn't seem ideal to me, but I can't come up with anything better and it's simple enough that it's obviously correct (without `NO_LIMIT` at least :).

utACK 696e0a5314296ca064192f580de949d5d9d8f9d4.

💬 josibake commented on pull request "crypto, refactor: add method for applying the taptweak":
(https://github.com/bitcoin/bitcoin/pull/30051#issuecomment-2105011629)
Updated to add a move constructor to the KeyPair class. I noticed this was missing while trying to use the new class in #28201 (i.e. creating a std::vector of KeyPairs). @theuni assuming this was just missed, but if not curious if you have any objections to adding a move constructor on KeyPair?