💬 ryanofsky commented on pull request "multiprocess: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2231676827)
I simplified PR as suggested so `bitcoin-mine` no longer spawns `bitcoin-node` if it can't connect to it. Instead it just prints an error message and suggests a command line to run `bitcoin-node` manually.

I also added a new commit to make the IPC code compatible with all the interfaces added in https://github.com/bitcoin/bitcoin/pull/30409, https://github.com/bitcoin/bitcoin/pull/30356, https://github.com/bitcoin/bitcoin/pull/30440, and https://github.com/bitcoin/bitcoin/pull/30443, so this
...

💬 maflcko commented on pull request "utils: replace boost::date_time usage with c++20 std::chrono":
(https://github.com/bitcoin/bitcoin/pull/30462#issuecomment-2231680026)
> Formatting in chrono is quite useful, so I don't think it makes sense to wait years until we require those compilers.

Can you give some examples where it would be useful? Right now it is only used in a single place in the wallet.

> This was noticed while working on https://github.com/bitcoin/bitcoin/pull/30434, which requires us to install boost::date_time. Though vendoring date.h here may seem heavy, keep in mind that the status quo is a forward-incompatible mess of boost headers :)


...

💬 maflcko commented on pull request "fuzz: add target for `CoinsResult`":
(https://github.com/bitcoin/bitcoin/pull/30461#issuecomment-2231685655)


Spend.cpp coverage is not too bad already: https://maflcko.github.io/b-c-cov/fuzz.coverage/src/wallet/spend.cpp.gcov.html

It may be good to clarify the new coverage a bit.

✅ maflcko closed a pull request: "fuzz: wallet, add target for Spend"
(https://github.com/bitcoin/bitcoin/pull/28236)

💬 maflcko commented on pull request "fuzz: wallet, add target for Spend":
(https://github.com/bitcoin/bitcoin/pull/28236#issuecomment-2231686907)
Picked up in https://github.com/bitcoin/bitcoin/pull/30461

💬 theuni commented on pull request "utils: replace boost::date_time usage with c++20 std::chrono":
(https://github.com/bitcoin/bitcoin/pull/30462#discussion_r1679972352)
Specifically the comparison against the epoch (0) fails. So, if the timestamp is old enough, this should fail but doesn't:
```c++
assert tp < epoch
```

You can play around here: https://godbolt.org/z/G8r837oGf

I'll add more specific info. I should probably submit a bug report upstream as well.

🤔 instagibbs reviewed a pull request: "cluster mempool: cluster linearization algorithm"
(https://github.com/bitcoin/bitcoin/pull/30126#pullrequestreview-2178360431)
another pass, through https://github.com/bitcoin/bitcoin/pull/30126/commits/23496cb4d43327c3d27401a531586bcfc985b6ad

changes since last look make sense, I would like to spend more time validating the optimizations

💬 instagibbs commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1678216187)
```Suggestion
* been serialized here so far, and in what order. */
```

💬 instagibbs commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1678277812)
```Suggestion
for (const auto topo_idx : reordering) {
```

💬 instagibbs commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1678297762)
mind if we also `Assume(select.Any())`?

💬 instagibbs commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1678263149)
This is the remaining part of logic that is hard to intuit now that the rest is a lot simpler to follow. I believe it's correct due to the harness itself but...

💬 instagibbs commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1678309127)
empircally

💬 instagibbs commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1678305787)
can we do same checks as https://github.com/bitcoin/bitcoin/pull/30126/commits/3b632d417764d79a51158aaea68586486d3b1cee#diff-1433c1fc4926a466291656ba67cf6b029523e4bd5da177ade812f25edf07343cR245 ?

💬 instagibbs commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1678345364)
lin_done doesn't exist

💬 instagibbs commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1679851817)
```Suggestion
* is guaranteed not to make the linearization worse at any point.
```

💬 instagibbs commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1679854169)
nitty renaming
```Suggestion
ClusterIndex NumChunksLeft() const noexcept { return m_chunks.size(); }
```

💬 maflcko commented on pull request "utils: replace boost::date_time usage with c++20 std::chrono":
(https://github.com/bitcoin/bitcoin/pull/30462#discussion_r1679976789)
Ah, so it is UB/crash, according to ubsan:

```
/opt/compiler-explorer/gcc-14.1.0/include/c++/14.1.0/bits/chrono.h:227:38: runtime error: signed integer overflow: -9223372037 * 1000000000 cannot be represented in type 'long int'

💬 achow101 commented on pull request "rpc: Use CHECK_NONFATAL over Assert":
(https://github.com/bitcoin/bitcoin/pull/30429#issuecomment-2231701658)
ACK fa6270737eb9655bfb4e29b7070ecb6cd2087b7f

💬 maflcko commented on pull request "utils: replace boost::date_time usage with c++20 std::chrono":
(https://github.com/bitcoin/bitcoin/pull/30462#discussion_r1679982342)
Would be good to add a fuzz target as well to https://github.com/google/oss-fuzz/tree/master/projects/libstdcpp

💬 maflcko commented on pull request "logging: Replace LogError and LogWarning with LogAlert":
(https://github.com/bitcoin/bitcoin/pull/30364#issuecomment-2231709067)
ACK 66d35a5c1384bf579e2da8e7054be7536e7a7101 🐛

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK 66d35a5c1384bf579e2da8e705
...