💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1966264980)
It's mostly to avoid consuming 12 bytes from the fuzz data for every transaction; added a comment.

💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1966264497)
This has been rewritten already.

💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1966266270)
Fixed.

💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1966266151)
I've added a comment. `~Ref` triggers observable behavior in TxGraph, so the simulation controls when it gets called (and its ordering w.r.t. other operations).

💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1966265505)
This dates from a time when the fuzz test allowed up to 1000 steps, and it felt unnecessary to support cases with unboundedly growing numbers of removed transactions. I've just dropped it.

💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1966267798)
Done, largely.

💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1966270811)
Indeed, done.

✅ yancyribbens closed an issue: "Fuzzing Bitcoin Core with clang-16"
(https://github.com/bitcoin/bitcoin/issues/31922)

💬 yancyribbens commented on issue "Fuzzing Bitcoin Core with clang-16":
(https://github.com/bitcoin/bitcoin/issues/31922#issuecomment-2675759929)
> What are the exact steps to reproduce?

Following the docs here: https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md. `cmake --preset=libfuzzer` was fine, it was the next `cmake --build build_fuzz` that was failing.

Looks like nuking `./bld-fzz` did the trick. Should have thought to try a fresh checkout. Thanks!

💬 instagibbs commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1966322708)
major reading comprehension issues it seems

💬 instagibbs commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1966324252)
yes, thanks

💬 instagibbs commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1966333596)
hah, was my first guess that I doubted :)

💬 instagibbs commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1966335575)
saw the updated comment/code in SanityCheck, makes sense

💬 instagibbs commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#issuecomment-2675849453)
verified changes, just waiting on further PostLinearization commentary

via `git range-diff master 11135464c5c3aef1ce8d2823120467a522ce2c87 d5abb86439e79d4adfbfbd46f833268bbca0bf6e`

💬 yancyribbens commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1966342676)
```suggestion
- A new wallet startup option `-maxfeerate` is added.
```

💬 yancyribbens commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1966343083)
```suggestion
// m_max_tx_fee. This is merely a belt-and-suspenders check).
```

👋 kevkevinpal's pull request is ready for review: "test: create assert_not_equal util"
(https://github.com/bitcoin/bitcoin/pull/29500)

📝 Prabhat1308 opened a pull request: "doc: Update documentation to include Clang/llvm based coverage report generation "
(https://github.com/bitcoin/bitcoin/pull/31933)
Followed up from the [comment](https://github.com/bitcoin/bitcoin/issues/31927#issuecomment-2674522975) on the issue [#31927](https://github.com/bitcoin/bitcoin/issues/31927) , issues have been observed building coverage reports with `gcov` in MacOs and NixOs (reported by @hodlinator). This PR adds the steps to generate a coverage report based on the default llvm/clang tooling.

💬 kevkevinpal commented on pull request "test: create assert_not_equal util":
(https://github.com/bitcoin/bitcoin/pull/29500#issuecomment-2675949599)
> @kevkevinpal what's the status of this?

Just rebased and addressed @vasild comment

---

> Almost ACK [dcf9a45](https://github.com/bitcoin/bitcoin/commit/dcf9a45aab4c4a7f9f900d5dafcdfe0e9a598a38), modulo squash the two commits into one and rebase.

Just rebased and squashed the commits, I removed the scripted-diff because it was getting more difficult to review the diff than the actual code. But now all the changes are in [d9cb032](https://github.com/bitcoin/bitcoin/pull/29500/commits
...

💬 hugohn commented on pull request "Revert merge of PR #31826":
(https://github.com/bitcoin/bitcoin/pull/31908#issuecomment-2675997995)
I agree completely, @sipa. From a process perspective, it might be worth adopting a policy that any code changes not covered by CI must be verified on the relevant hardware—or at least a suitable emulator—before merging. In this particular case, the original changes were tested, but the subsequent refactor was not. Had it been tested, we would have caught the compile error.