💬 fanquake commented on pull request "Add missing byteswap functions for MSVC":
(https://github.com/bitcoin/bitcoin/pull/29036#issuecomment-1852122243)
cc @sipsorcery.

> Also we should remember that MSVC build uses no optimisation

Could rebase on master now that https://github.com/bitcoin/bitcoin/pull/29045 has gone in.

💬 epompeii commented on issue "Continuous benchmark tracking":
(https://github.com/bitcoin/bitcoin/issues/27284#issuecomment-1852127121)
@aureleoules that looks really nice!

Would you be interesting in plotting those data over time? If so I can work on ingesting your results into Bencher, similar to how `rustls` is doing it: https://bencher.dev/perf/rustls-821705769

💬 glozow commented on pull request "mempool / miner: regularly flush <=0-fee entries and mine everything in the mempool":
(https://github.com/bitcoin/bitcoin/pull/27018#issuecomment-1852127137)
Planning to resurrect this after #28948.

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#issuecomment-1852127685)
@sdaftuar I took your suggestions, reordered and slightly reworded. Matches my implementation.

⚠️ aureleoules opened an issue: "Benchmark WalletCreateTxUsePresetInputsAndCoinSelection due to #"
(https://github.com/bitcoin/bitcoin/issues/29061)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

The WalletCreateTxUsePresetInputsAndCoinSelection benchmarks crashes on master due to the commit 758501b71391136c33b525b1a0109b990d4f463e introduced in #25273 (I used git bisect to verify).

```
valgrind ./src/bench/bench_bitcoin -filter=WalletCreateTxUsePresetInputsAndCoinSelection -min-time=10000
==34277== Memcheck, a memory error detector
==34277== Copyright (C) 2002-2017, and GNU
...

💬 fanquake commented on issue "Benchmark WalletCreateTxUsePresetInputsAndCoinSelection due to #":
(https://github.com/bitcoin/bitcoin/issues/29061#issuecomment-1852135389)
Was this fixed in #29055?

💬 aureleoules commented on issue "Continuous benchmark tracking":
(https://github.com/bitcoin/bitcoin/issues/27284#issuecomment-1852136094)
> Would you be interesting in plotting those data over time?

Yes I plan to display on the homepage the plot of benchmarks and test coverage ratio of master over time!

💬 maflcko commented on issue "Continuous benchmark tracking":
(https://github.com/bitcoin/bitcoin/issues/27284#issuecomment-1852136752)
Agree that a plot over time would be useful. They were on https://codespeed.bitcoinperf.com/timeline/ , but it hasn't run for some years now.

💬 fanquake commented on issue "./contrib/guix/guix-build does not work on riscv64":
(https://github.com/bitcoin/bitcoin/issues/29020#issuecomment-1852139445)
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=67697

Apparently the fix has now landed, but I don't yet see it in https://git.savannah.gnu.org/cgit/guix.git/log/ (or core-updates).

💬 maflcko commented on issue "Benchmark WalletCreateTxUsePresetInputsAndCoinSelection due to #":
(https://github.com/bitcoin/bitcoin/issues/29061#issuecomment-1852140217)
Why was this not caught by CI?

💬 fanquake commented on issue "Benchmark WalletCreateTxUsePresetInputsAndCoinSelection due to #":
(https://github.com/bitcoin/bitcoin/issues/29061#issuecomment-1852145959)
> Why was this not caught by CI?

Looks like that is a "low priority" benchmark, meaning it's not run in the CI, which only run high?

💬 epompeii commented on issue "Continuous benchmark tracking":
(https://github.com/bitcoin/bitcoin/issues/27284#issuecomment-1852146955)
Sounds great!

If you want them to be live updating, you can embed Bencher plots. Just go to the `Share` button on the Perf Page and copy the Embed Perf Plot Link for the current plot. [This is an example](https://bencher.dev/learn/benchmarking/rust/libtest-bench/#catch-performance-regressions-in-ci) of what that could look like.

💬 aureleoules commented on issue "Benchmark WalletCreateTxUsePresetInputsAndCoinSelection due to #25273":
(https://github.com/bitcoin/bitcoin/issues/29061#issuecomment-1852149860)
> Was this fixed in https://github.com/bitcoin/bitcoin/pull/29055?

Unfortunately no, it still crashes on master as of a7484be65f7617d77aff92ecf6f5fb26015d27a8

💬 dergoegge commented on pull request "fuzz: make FuzzedDataProvider usage deterministic":
(https://github.com/bitcoin/bitcoin/pull/29043#issuecomment-1852151987)
> I don't know how stable the ordering is for llvm. If it stays like that it's not a big deal I'd say, but it might change in future versions or with optimization levels?

Good point, I guess since the spec is unspecified they have the right to change it but I would still be surprised if they do. I'm not sure if there is a trivial way to check if they ever have.

> in a different project I'm working on we run the fuzzing targets with the minimized corpus as unit tests, and this is done in th
...

👍 TheCharlatan approved a pull request: "refactor: Print verbose serialize compiler error messages"
(https://github.com/bitcoin/bitcoin/pull/29056#pullrequestreview-1777459272)
`concept` ACK fa180c523a173550ab4e549788cb21d4e7994118 :tada:

This:
```
./serialize.h:763:6: note: candidate template ignored: constraints not satisfied [with Stream = DataStream, T = Span<int>]
void Serialize(Stream& os, const T& a)
^
./serialize.h:762:14: note: because 'Serializable<Span<int>, DataStream>' evaluated to false
requires Serializable<T, Stream>
^
./serialize.h:760:52: note: because 'a.Serialize(s)' would be invalid: no member named 'Serialize' in
...

💬 TheCharlatan commented on pull request "refactor: Print verbose serialize compiler error messages":
(https://github.com/bitcoin/bitcoin/pull/29056#discussion_r1423986205)
Nit: Should this rather be in `assumptions.h` if it counts for the entire codebase?

💬 aureleoules commented on issue "Benchmark WalletCreateTxUsePresetInputsAndCoinSelection due to #25273":
(https://github.com/bitcoin/bitcoin/issues/29061#issuecomment-1852155662)
> Why was this not caught by CI?

Yeah, I don't believe all the benchmarks are being run. I think they should all be executed with at least -min-time=1000 as well.
I've noticed while working on my bench tool (context https://github.com/bitcoin/bitcoin/issues/27284#issuecomment-1852136094) that a few pulls almost introduced these bugs.
And some benchs only crash with `-min-time=1000` or `-min-time=10000` for some reasons.

📝 BrandonOdiwuor opened a pull request: "Calculate used balance from GetBalance(...)"
(https://github.com/bitcoin/bitcoin/pull/29062)
Compute `used` balance from GetBalance(...)

_Check https://github.com/bitcoin/bitcoin/pull/28776#discussion_r1407875236 by @achow101_

> However, I think it would be better to move all of this into `GetBalance` itself and just have it always compute the used balance for us rather than having the caller do this extra computation.

💬 fanquake commented on issue "Benchmark WalletCreateTxUsePresetInputsAndCoinSelection crashes due to #25273":
(https://github.com/bitcoin/bitcoin/issues/29061#issuecomment-1852171394)
> Unfortunately no, it still crashes on master as of https://github.com/bitcoin/bitcoin/commit/a7484be65f7617d77aff92ecf6f5fb26015d27a8

cc @achow101 @furszy

🤔 stickies-v reviewed a pull request: "[refactor] Check CTxMemPool options in ctor"
(https://github.com/bitcoin/bitcoin/pull/28830#pullrequestreview-1775679380)
LGTM 3aa41a31690da2ae22b6a15a9a5de0de78a01757 when fuzzer is fixed