💬 remyers commented on issue "build: Boost 1.74.0 incompatible with Clang 18":
(https://github.com/bitcoin/bitcoin/issues/30751#issuecomment-2328622998)
> > Fwiw, I also ran across this problem (Ubuntu 20.04)
>
> I guess you also installed a different boost version, because the 1.71 boost version in Ubuntu Focal 20.04 is not supported starting with #29066 and shouldn't even pass the `cmake -B` command.

Yup, I have boost 1.74.0.3 installed.
```sh
dpkg -s libboost-dev | grep 'Version'
Version: 1.74.0.3ubuntu7
```

💬 tdb3 commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#issuecomment-2328685708)
> Concept ACK
>
> I agree that this would be interesting. The the orphan pool is mostly a black box to me (and probably others too). To discuss improvements/changes, having a better visual and mental model would be helpful. Maybe even similar to `getrawaddrman` and https://addrman.observer.

Thanks. That web app is gorgeous! Any particular tx details you'd like to see added (e.g. orphan expiration, etc.)?

👋 tdb3's pull request is ready for review: "rpc: add getorphantxs"
(https://github.com/bitcoin/bitcoin/pull/30793)

💬 petertodd commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#issuecomment-2328715995)
> Today's mempool does a suboptimal job of dealing with chain limits and RBFs. Assuming cluster mempool, we still have the issue of what to do with cluster limits that are exceeded even after removing RBF conflicts, if any. This has been called "sibling eviction" and in general seems doable in a DoS-resistant way but is left to future work. It's a problem I'm very interested in solving post-cluster mempool.

What I mean is, if you are doing an RBF replacement of one transaction with another tr
...

👍 stickies-v approved a pull request: "test: support std::optional in BOOST_CHECK_* and increase FromUserHex fuzz feature coverage"
(https://github.com/bitcoin/bitcoin/pull/30618#pullrequestreview-2279873765)
ACK 6fa4c92e4c9bf5ea90e25d41b525c88f9c2d834d , modulo my lack of fuzzing experience to have a good view on whether the `fuzz/hex.cpp` changes indeed make the fuzz test better (the code looks correct and safe to me), as per https://github.com/bitcoin/bitcoin/pull/30618/files#r1742175853

I [maintain](https://github.com/bitcoin/bitcoin/pull/30618/files#r1742093199) that the `setup_common.cpp` changes should be dropped from dec011bdaaeb000a9f22a31e3fe9415ec7bf97dc, but it is not a blocker.

💬 stickies-v commented on pull request "test: support std::optional in BOOST_CHECK_* and increase FromUserHex fuzz feature coverage":
(https://github.com/bitcoin/bitcoin/pull/30618#discussion_r1743607635)
simplification nit
```suggestion
BOOST_CHECK_EQUAL(get_valid_opts({"-minimumchainwork=0x1234"}).minimum_chain_work, arith_uint256{0x1234});
```

💬 stickies-v commented on pull request "test: support std::optional in BOOST_CHECK_* and increase FromUserHex fuzz feature coverage":
(https://github.com/bitcoin/bitcoin/pull/30618#discussion_r1743620137)
> (though in this case that wouldn't even be true since `random_hex_string` can start with `0x` for `FromUserHex` which isn't added back via `ToString`)

Ah yes good point thanks, perhaps that would be good to clarify in a docstring?

> isn't the point of these tests that we shouldn't have know the internals (i.e. black box)?

I'm out of my depth here, so will probably refrain from commenting further and hopefully someone with more fuzzing experience can chime in. I think the point of the
...

👍 vasild approved a pull request: "tracing: Only prepare tracepoint arguments when actually tracing"
(https://github.com/bitcoin/bitcoin/pull/26593#pullrequestreview-2279921795)
ACK 5be6cc771faea00915a31240d9b5afdcdd4aa52f

💬 maflcko commented on pull request "rpc: dumptxoutset height parameter follow-ups (29553)":
(https://github.com/bitcoin/bitcoin/pull/30808#discussion_r1743546975)
```suggestion
ChainstateManager& m_chainman;
const CBlockIndex& m_invalidate_index;
public:
TemporaryRollback(ChainstateManager& chainman, const CBlockIndex& index) : m_chainman{chainman}, m_invalidate_index{index} {
InvalidateBlock(m_chainman, m_invalidate_index.GetBlockHash());
};
~TemporaryRollback() {
ReconsiderBlock(m_chainman, m_invalidate_index.GetBlockHash());
```

nit in e6812ffec338035a1ebe949015e95ba335c94d08: Doesn't change the behavior
...

👍 maflcko approved a pull request: "rpc: dumptxoutset height parameter follow-ups (29553)"
(https://github.com/bitcoin/bitcoin/pull/30808#pullrequestreview-2279769878)
review ACK e6812ffec338035a1ebe949015e95ba335c94d08 💦

<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: review ACK e6812ffec338
...

💬 maflcko commented on pull request "rpc: dumptxoutset height parameter follow-ups (29553)":
(https://github.com/bitcoin/bitcoin/pull/30808#discussion_r1743586494)
nit in e6812ffec338035a1ebe949015e95ba335c94d08: Wouldn't it be easier to just `throw` the error on creation, instead of creating a temporary symbol and check whether it is null and then `throw` it? With `TemporaryRollback` this should be possible now.

💬 maflcko commented on pull request "rpc: dumptxoutset height parameter follow-ups (29553)":
(https://github.com/bitcoin/bitcoin/pull/30808#discussion_r1743571147)
```suggestion
// process which may not be what the user wants.
```

nit in9467de52d3befd831b16b01c332793de18cb29cc : typo

Also "Don't do this if ..." can be replaced by "Skip if ...", but either is fine.

💬 instagibbs commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1743671441)
I feel like the ability to fetch the whole transaction (set) optionally is useful, otherwise we just have a couple hashes of the transactions, no ability to estimate how many vbytes the orphanage is using, do any analysis on the data given?

Maybe have an optional argument via txid/wtxid that fetches the full serialized transaction in hex?

💬 tdb3 commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1743675879)
Agreed, will refinea

💬 instagibbs commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1743687732)
Follow-up PRs can probably add more interesting fields as well, such as entry time, announcer peerid, etc.

I'll try not to scope creep this PR too bad

💬 glozow commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1743691588)
For originator, I was thinking `fromPeer` (the NodeId). Currently each orphan tracks 1 sender, but hoping to have multiple in the future (#28031 if you're curious).

For this PR, I think it would suffice to take an approach similar to `getrawmempool` but with an integer verbosity.
verbose=0: list of txids (it's ok if there are duplicates, it should be rare anyway)
verbose=1: list of objects with txid, wtxid, size, senders (a list of NodeIds which as of now has length 1), expiration

I thin
...

💬 hebasto commented on issue "guix: Build fails for `x86_64-apple-darwin`":
(https://github.com/bitcoin/bitcoin/issues/30810#issuecomment-2328862764)
> Has this ever worked on your system? `guix shell: error` sounds like it is an error unrelated to Bitcoin Core?
>
> Can you use guix and guix shell normally?

Both command work just fine. For example:
```
$ guix package -i hello
$ guix shell python -- python3
```

Moreover, every other non-Darwin host is built successfully with `./contrib/guix/guix-build`.

💬 glozow commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1743697939)
> ability to estimate how many vbytes the orphanage is using

A `getorphanageinfo` can be added for overall stats, perhaps after #28031 which adds size tracking.

💬 fanquake commented on issue "guix: Build fails for `x86_64-apple-darwin`":
(https://github.com/bitcoin/bitcoin/issues/30810#issuecomment-2328969074)
I think you'll need to debug this further. It's odd that building for all HOSTS (except one) would work, but building for `x86_64-apple-darwin` currently works fine, as far as I'm aware (just tested on two different systems).

> Both command work just fine. For example:

It's not clear that those are actually invoking the issue-causing behaviour. The error message points to `guix container`. i.e `guix shell --container`

🚀 fanquake merged a pull request: "fuzz: Rename fuzz_seed_corpus to fuzz_corpora"
(https://github.com/bitcoin/bitcoin/pull/30804)