:lock: fanquake locked an issue: "."
(https://github.com/bitcoin/bitcoin/issues/30280)

👍 dergoegge approved a pull request: "refactor: Add explicit cast to expected_last_page to silence fuzz ISan"
(https://github.com/bitcoin/bitcoin/pull/30248#pullrequestreview-2115150107)
utACK fa9cb101cf33b57b2c043b29f1f3d55b990ba4c6

💬 dergoegge commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#issuecomment-2165010123)
#30229 was merged if you want to rebase

💬 brunoerg commented on pull request "test: cover more errors for `signrawtransactionwithkey` RPC":
(https://github.com/bitcoin/bitcoin/pull/30278#discussion_r1637893489)
To avoid calling same stuff twice. e.g. `createrawtransaction`.

💬 theStack commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1637812555)
```suggestion
// This takes the place of ReplacementChecks()'s PaysMoreThanConflicts() in the package RBF setting.
```

💬 theStack commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1637815039)
nit: slightly easier to reason for the reader imho

```suggestion
if (package_feerate <= parent_feerate) {
```

💬 theStack commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1637859001)
nit: it might be worth to add a belt-and-suspenders check that txns has size two (either directly or indirectly by asserting `txns.size() == workspaces.size()` above)

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