🤔 glozow reviewed a pull request: "fuzz: txorphan tests fixups"
(https://github.com/bitcoin/bitcoin/pull/29974#pullrequestreview-2034150338)
ACK 58594c7

💬 sr-gi commented on pull request "net: don't lock cs_main while reading blocks in net processing":
(https://github.com/bitcoin/bitcoin/pull/26326#discussion_r1586586486)
Would it be worth having a static assert for this when defining `MAX_BLOCKTXN_DEPTH`?

```
static_assert(MAX_BLOCKTXN_DEPTH < MIN_BLOCKS_TO_KEEP, "MIN_BLOCKS_TO_KEEP too low");
```

This was not obvious to me either, but I'm not too familiar with this part of net_processing

💬 instagibbs commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#issuecomment-2088877822)
should be able to add fuzz coverage easily for differing wtxids but same txids in `src/test/fuzz/txorphan.cpp`

✅ laanwj closed a pull request: "doc: Remove outdated description for --port argument"
(https://github.com/bitcoin/bitcoin/pull/30014)

💬 theuni commented on pull request "build: no-longer disable WARN_CXXFLAGS when CXXFLAGS is set":
(https://github.com/bitcoin/bitcoin/pull/25972#issuecomment-2088880497)
What's the status of this?

🤔 jonatack reviewed a pull request: "doc: removed help text saying that peers may not connect automatically"
(https://github.com/bitcoin/bitcoin/pull/29994#pullrequestreview-2034177812)
I'm not sure this should be removed unless it has been observed to no longer be the case, and in the meantime it does no harm. See also https://github.com/bitcoin/bitcoin/pull/30014#pullrequestreview-2034095091.

💬 glozow commented on pull request "test: Don't rely on incentive incompatible replacement in mempool_accept_v3.py":
(https://github.com/bitcoin/bitcoin/pull/29986#discussion_r1586616806)
Oof my bad for not checking more closely. How's this?

```
diff --git a/test/functional/mempool_accept_v3.py b/test/functional/mempool_accept_v3.py
index 8285b82c19..9fc6eff453 100755
--- a/test/functional/mempool_accept_v3.py
+++ b/test/functional/mempool_accept_v3.py
@@ -533,10 +533,27 @@ class MempoolAcceptV3(BitcoinTestFramework):
tx_unrelated_replacee = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=utxo_unrelated_conflict)
assert tx_unrelated_replac
...

💬 achow101 commented on pull request "test: Add test case for spending bare multisig":
(https://github.com/bitcoin/bitcoin/pull/29120#issuecomment-2088899280)
ACK e504b1fa1fa4d014b329dea81bfdf1aa55db238f

✅ kevkevinpal closed a pull request: "doc: removed help text saying that peers may not connect automatically"
(https://github.com/bitcoin/bitcoin/pull/29994)

💬 kevkevinpal commented on pull request "doc: removed help text saying that peers may not connect automatically":
(https://github.com/bitcoin/bitcoin/pull/29994#issuecomment-2088902630)
> I'm not sure this should be removed unless it has been observed to no longer be the case, and in the meantime it does no harm. See also [#30014 (review)](https://github.com/bitcoin/bitcoin/pull/30014#pullrequestreview-2034095091).

that's fair I can close this for now, if someone does have concrete statistics feel free to pick this up

✅ achow101 closed an issue: "Test case for spending bare multisig?"
(https://github.com/bitcoin/bitcoin/issues/29113)

🚀 achow101 merged a pull request: "test: Add test case for spending bare multisig"
(https://github.com/bitcoin/bitcoin/pull/29120)

💬 glozow commented on pull request "test: Don't rely on incentive incompatible replacement in mempool_accept_v3.py":
(https://github.com/bitcoin/bitcoin/pull/29986#discussion_r1586637512)
above:
- `fee_to_beat_absolute` is 41600 (above code uses `max(tx_unrelated_replacee, tx_v3_child_2)` but Rule 3 would require us to beat the sum of their fees haha)
- `feerate_to_beat` is 300, i.e. 31200 /104
- `fee_to_beat_feerate` is 300 * (147 + 154) = 90300
- `fee_v3_child_3` is max(41600, 90300) + 5 = 90305
- this means the ancestor feerate is 90305 / (147 + 154) = 300.017

💬 instagibbs commented on pull request "fuzz: txorphan tests fixups":
(https://github.com/bitcoin/bitcoin/pull/29974#discussion_r1586607474)
micro-nit/guess: for stability I think a single boolean might be better

👍 instagibbs approved a pull request: "fuzz: txorphan tests fixups"
(https://github.com/bitcoin/bitcoin/pull/29974#pullrequestreview-2034172340)
ACK 58594c7040241f2312b0b8735a8baf0412674613

ran fuzz for a while, no issues

🤔 stickies-v reviewed a pull request: "p2p: index TxOrphanage by wtxid, allow entries with same txid"
(https://github.com/bitcoin/bitcoin/pull/30000#pullrequestreview-2033751635)
Concept ACK, but I'm concerned about the behaviour change in `AlreadyHaveTx`, which seems quite non-trivial to review so I'll need to look into further.

💬 stickies-v commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1586638555)
Or alternatively:

<details>
<summary>git diff on 4b4dfaa8f3</summary>

```diff
diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp
index 504a1f7ec3..bed92e6b32 100644
--- a/src/txorphanage.cpp
+++ b/src/txorphanage.cpp
@@ -98,13 +98,11 @@ void TxOrphanage::EraseForPeer(NodeId peer)
m_peer_work_set.erase(peer);

int nErased = 0;
- std::map<Wtxid, OrphanTx>::iterator iter = m_orphans.begin();
- while (iter != m_orphans.end())
- {
- std::map<Wtxid, Or
...

💬 stickies-v commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1586533521)
nit: can just one-line
```suggestion
Assert(orphanage.HaveTx(ref->GetWitnessHash()));
```

💬 stickies-v commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1586713579)
It's not immediately obvious to me why it is safe to just convert any `gtxid` into a `Wtxid` here, especially since there are callsites of `AlreadyHaveTx` where a ` GenTxId::Txid` is passed, such as e.g. [here](https://github.com/bitcoin/bitcoin/blob/d73245abc70346a0e8805d50a1f395706084294c/src/net_processing.cpp#L4632). If it is indeed safe, I think it could be a better approach to update `AlreadyHaveTx` to take a `Wtxid` and push the conversion further to the edge, so it can be reviewed on a c
...

💬 stickies-v commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1586372606)
```suggestion
/** Check if we already have an orphan transaction */
```