🤔 theStack reviewed a pull request: "Cluster size 2 package rbf"
(https://github.com/bitcoin/bitcoin/pull/28984#pullrequestreview-2115139302)
Spent most time reviewing the core commit 612847ae1a55e92bb7732905a026be96a436372a so far, which looks correct to me (though I'm not super-familiar with the details of the feerate diagram checks that are used in this PR for the first time in production code). Still planning on looking deeper at the tests and do another full review round. Left some non-blocking nits on the way.

💬 theStack commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1637914037)
```suggestion
single_coin = self.coins.pop()
```

💬 theStack commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1637918078)
could mention "analogous to regular replacement rule 5"?

💬 theStack commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1637862004)
nit: shorter (can also be done for grandchild_key)
```suggestion
CKey child_key = GenerateRandomKey();
```

💬 vasild commented on pull request "fuzz: FuzzedSock::Recv() don't lose bytes from MSG_PEEK read":
(https://github.com/bitcoin/bitcoin/pull/30273#issuecomment-2165212522)
The UB sanitizer did not like the call to `memcpy(..., nullptr, 0);` which happens when the fuzzer input is exhausted. Added an explicit check about that.

💬 stickies-v commented on pull request "Encapsulate warnings in generalized node::Warnings and remove globals":
(https://github.com/bitcoin/bitcoin/pull/30058#issuecomment-2165244787)
Force pushed to address merge conflict from #29015, no changes otherwise.

🤔 marcofleon reviewed a pull request: "i2p: fix and improve logs"
(https://github.com/bitcoin/bitcoin/pull/29833#pullrequestreview-2115402840)
ACK 7d3662fbe35032178c5a5e27e73c592268f6e41b.

🚀 fanquake merged a pull request: "Update minisketch subtree to eb37a9b8e79f9e49d73b96a49bf97a96d9eb676c"
(https://github.com/bitcoin/bitcoin/pull/30270)

👍 ryanofsky approved a pull request: "Encapsulate warnings in generalized node::Warnings and remove globals"
(https://github.com/bitcoin/bitcoin/pull/30058#pullrequestreview-2115559245)
Code review ACK 260f8da71a35232d859d7705861fc1a88bfbbe81. Only change since last review was rebasing

📝 fanquake opened a pull request: "Update leveldb subtree to latest upstream"
(https://github.com/bitcoin/bitcoin/pull/30281)
Includes https://github.com/bitcoin-core/leveldb-subtree/pull/41 which is used in #30234.

💬 benma commented on issue "docs: Wrong/outdated docs for `tr(KEY)` in doc/descriptors.md":
(https://github.com/bitcoin/bitcoin/issues/30279#issuecomment-2165532816)
I for one was confused. Imo being explicit is better than being implicit. "internal key" is ambiguous to me, as it means it is untweaked, but it does not mean it must be tweaked. BIP-341 merely says "should".

Removing the descriptors.md in in favor of BIPs is not optimal, because:

- the BIPs are not the docs for Bitcoin Core - Bitcoin Core should document itself what it supports
- it is hard to get an overview of what is supported in Bitcoin Core by scrolling BIPs

I think the best woul
...

📝 fanquake opened a pull request: "Revert "contrib: macdeploy: monkey-patch gen-sdk to be deterministic""
(https://github.com/bitcoin/bitcoin/pull/30282)
This reverts commit ba30a5407e065e9d6dd037351e83f56a43f38f19.

We no-longer support Python 3.8, so remove the monkey patching.

💬 andrewtoth commented on pull request "validation: sync chainstate to disk after syncing to tip":
(https://github.com/bitcoin/bitcoin/pull/15218#discussion_r1638246968)
Point taken for moving the explicit lock after this check, since the lock is taken in `IsInitialBlockDownload()`.

However, this check only runs once every 30 seconds. I don't see how it could possibly affect responsiveness of the software. It is a very fast check I would assume on the order of microseconds every 30 seconds.

💬 fanquake commented on pull request "build: Bump clang minimum supported version to 16":
(https://github.com/bitcoin/bitcoin/pull/30263#issuecomment-2165727772)
Looks like the Apple Clang shipping with Xcode 15, is based on Clang 15. Xcode 16 (still in beta) will ship with a Apple Clang based on Clang 16.

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1638262708)
done

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1638262828)
done

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1638262918)
added at beginning of function

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1638263066)
done

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1638263169)
done in this and one more location

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1638263303)
done