💬 maflcko commented on issue "fuzz: Fix stability, determinism issues":
(https://github.com/bitcoin/bitcoin/issues/29018#issuecomment-1845683402)
> oss-fuzz has a table for the afl++ jobs

Nice. Though, I wonder if there is something public available. Similar to the coverage report (https://github.com/bitcoin/bitcoin/blame/fcdb39d3ee17015776c0759e4742334a962219db/doc/fuzzing.md#L350) or the inputs zip (https://github.com/bitcoin-core/qa-assets/blob/38d7a06e9544bada01d558e6f85129334c228076/download_oss_fuzz_inputs.py#L35)

💬 TheCharlatan commented on pull request "refactor: rpc: Pass CBlockIndex by reference instead of pointer":
(https://github.com/bitcoin/bitcoin/pull/29021#discussion_r1419213113)
The `chainman.ActiveChain().Tip()` is checked here, but there are a few other places where the `tip` pointer is not checked (e.g. [here](https://github.com/bitcoin/bitcoin/pull/29021/files#diff-decae4be02fb8a47ab4557fe74a9cb853bdfa3ec0fa1b515c0a1e5de91f4ad0bL559), [here](https://github.com/bitcoin/bitcoin/pull/29021/files#diff-decae4be02fb8a47ab4557fe74a9cb853bdfa3ec0fa1b515c0a1e5de91f4ad0bR730), and [here](https://github.com/bitcoin/bitcoin/pull/29021/files#diff-decae4be02fb8a47ab4557fe74a9cb85
...

👍 TheCharlatan approved a pull request: "doc: Add link to needs-release-notes label"
(https://github.com/bitcoin/bitcoin/pull/29025#pullrequestreview-1770616702)
ACK fa88953d6fb54fdb47485981279632c693534108

💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1845689097)
re: https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1835850051

`LimitMempoolSize` is only called upon successful transaction inclusions, so a v3 tx that is negatively prioritized later may not be evicted yet.

💬 maflcko commented on pull request "refactor: rpc: Pass CBlockIndex by reference instead of pointer":
(https://github.com/bitcoin/bitcoin/pull/29021#discussion_r1419285212)
The weren't previously checked either (I presume because if the tip is nullptr, then pblockindex should be nullptr as well, which is checked).

However, I am happy to review a pull request adding a check, or push any changes that compile to this pull request.

💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1419287189)
I'm not sure the best way to document things, but I added a to-do item to the OP so that we don't lose track of figuring out how we want the docs to look. (Will have to conform all the code comments as well.)

💬 pablomartin4btc commented on pull request "Fix: Ensure 'Transaction View' remains disabled if no wallet is selected":
(https://github.com/bitcoin-core/gui/pull/780#issuecomment-1845700681)
> I'm not sure if this is the right approach: user might want to enable `Mask values` before opening his wallet?

I was just giving it a second thought and if we follow the original approach a user could still have the desired behaviour as the following:

<details>
<summary>check display here</summary>


</details>

First time the user opens or creates a wallet can tick
...

💬 dergoegge commented on issue "fuzz: Fix stability, determinism issues":
(https://github.com/bitcoin/bitcoin/issues/29018#issuecomment-1845701335)
> Nice. Though, I wonder if there is something public available.

I'm not aware of something like that hosted by oss-fuzz, all the per fuzzer stats seem to require auth.

I've been primarily fuzzing with afl++ lately, I can look at hosting some stats from that.

💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1419291403)
Chunks for a given cluster are already sorted in descending feerate order, is that what you're asking about or is there another issue I'm overlooking?

💬 TheCharlatan commented on pull request "refactor: rpc: Pass CBlockIndex by reference instead of pointer":
(https://github.com/bitcoin/bitcoin/pull/29021#discussion_r1419294898)
Right, you only added checks to the lines you already were touching, so I guess that makes sense.

👍 TheCharlatan approved a pull request: "refactor: rpc: Pass CBlockIndex by reference instead of pointer"
(https://github.com/bitcoin/bitcoin/pull/29021#pullrequestreview-1770634925)
ACK fa5989d514d246e56977c528b2dd2abe6dc8efcc

💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1419299493)
I believe I (finally) actually fixed this behavior to count the number of direct conflicts.

💬 maflcko commented on issue "fuzz: Fix stability, determinism issues":
(https://github.com/bitcoin/bitcoin/issues/29018#issuecomment-1845718292)
Yeah, or alternatively steps to reproduce the stability output with afl, so that non-afl people can have some fun, too.

💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1419303051)
Should be fixed now.

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#issuecomment-1845736014)
re-ACK [f56ec8a](https://github.com/bitcoin/bitcoin/pull/28765/commits/f56ec8a3b086b0f2f8a0fbde861447e40ffbc3d9)

💬 Sjors commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1419345302)
That's why it says "This changes after a new descriptor is imported".

But right now the logic behind populating this field is not documented. Without reading a bunch of pull request comments, one might wonder why we don't just store the xpriv, why we go through all descriptors instead of one, etc.

👍 pablomartin4btc approved a pull request: "refactor: rpc: Pass CBlockIndex by reference instead of pointer"
(https://github.com/bitcoin/bitcoin/pull/29021#pullrequestreview-1770724880)
tACK fa5989d514d246e56977c528b2dd2abe6dc8efcc

I've re-tested #29003 & `getblockchaininfo` on `mainnet`.

💬 achow101 commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1419392876)
Sure, but that's not what your suggested text documents. I've added a comment earlier during the actual loading that describes it.

💬 maflcko commented on issue "./contrib/guix/guix-build does not work on riscv64":
(https://github.com/bitcoin/bitcoin/issues/29020#issuecomment-1845865501)
Sure, sent the fix: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=67697

💬 mzumsande commented on pull request "p2p: attempt to fill full outbound connection slots with peers that support tx relay":
(https://github.com/bitcoin/bitcoin/pull/28538#discussion_r1419441534)
It is enforced by the line
`if (!node_to_evict.has_value() || *node_to_evict < node->GetId()) {` (second condition),
not through `m_connected` but by picking the eligible node with the highest NodeId. I'll add to the comment that "newest" means "highest `Node Id`", do you think that would be sufficient?