💬 mzumsande commented on issue "fuzz: Fix stability, determinism issues":
(https://github.com/bitcoin/bitcoin/issues/29018#issuecomment-1845641373)
How is the "stability" metric calculated?

💬 dergoegge commented on issue "fuzz: Fix stability, determinism issues":
(https://github.com/bitcoin/bitcoin/issues/29018#issuecomment-1845645132)
> How is the "stability" metric calculated?

"Stability is measured by how many percent of the edges in the target are "stable". Sending the same input again and again should take the exact same path through the target every time. If that is the case, the stability is 100%." - https://github.com/AFLplusplus/AFLplusplus/blob/stable/docs/FAQ.md

💬 furszy commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#discussion_r1419237363)
> Right, but this is my concern: a bug is found and fixed, and then a log is added that might have helped detect the already fixed bug (in this case, I'm still not convinced this log as written would have helped much). This pattern tends toward log bloat over time.

Well, thats not what occurs in practice. Isn't a pattern at all. Could check all solved bugs in the wallet the past ~2 years (since I'm around at least) and see how many log lines were added to the wallet. I'm quite confident that
...

💬 sdaftuar commented on pull request "mempool: Don't sort in entryAll":
(https://github.com/bitcoin/bitcoin/pull/29019#issuecomment-1845658601)
> Though, if the function will be removed either way, I wonder if this is worth it. Maybe wait for an OK by @sdaftuar to avoid a merge conflict with https://github.com/bitcoin/bitcoin/pull/28676 ?

I'm not necessarily planning to remove `infoAll()`, but I am planning to replace `GetSortedDepthAndScore()` with a different function that will iterate the mempool topologically based on cluster sort (ie a valid topological order in which the mempool is sorted by descending feerate chunks).

I thi
...

💬 achow101 commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1845666417)
> We have the `version` which stores the version of the last client which opened the wallet. Maybe we should use that to also determine whether an upgrade needs to be performed?

Hmm, this seems to not necessarily be true for older versions. 23.0 has it has updating only when less than, not just not equal.

💬 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)