💬 sdaftuar commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1817094524)
Is this logging correct? The `parent_txid` here is really the txid of the child that didn't spend all of some parent's dust. I don't think we know which actual parent of that transaction had the unspent dust, unless we add more information to the return value of `CheckEphemeralSpends`.

💬 sdaftuar commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1817374367)
Just want to make sure I understand what you mean by "sibling" here. Is the idea that if we have a topology like this:

```mermaid
flowchart TD
TxA --> TxC
TxB --> TxC
```

Where Tx A has ephemeral dust outputs and is in the mempool, and Tx C spends them and is in the mempool, than you want to return an outpoint of Tx B, whether or not Tx B itself is in the mempool, and whether or not Tx B itself has ephemeral dust outputs?

💬 sdaftuar commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1817315017)
I think all the parents in-mempool at this point?

💬 sdaftuar commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1817772690)
Comment needs to be updated I think?

💬 sdaftuar commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1817223902)
Is it also worth testing that this fails if submitted via `sendrawtransaction` as well, so we're not just testing the `AcceptPackage()` code path?

💬 sdaftuar commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1817771844)
If it's not too hard, I was thinking it would be nice to have this fuzz test cover the package RBF case as well.

💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1817839632)
Fixed, thanks.

💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1817839644)
Done.

💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#issuecomment-2439552457)
Rebased.

👍 TheCharlatan approved a pull request: "build: Rename `PACKAGE_*` variables to `CLIENT_*`"
(https://github.com/bitcoin/bitcoin/pull/31042#pullrequestreview-2397236697)
ACK 36a3de0af9a9120aa98a07ca4abbb77ee6e58ef9

The last two commits could be squashed together though? I don't think that would complicate the scripted diff.

💬 hebasto commented on pull request "build: Rename `PACKAGE_*` variables to `CLIENT_*`":
(https://github.com/bitcoin/bitcoin/pull/31042#issuecomment-2439562411)
> The last two commits could be squashed together though? I don't think that would complicate the scripted diff.

Thanks! Squashed.

👍 TheCharlatan approved a pull request: "build: Rename `PACKAGE_*` variables to `CLIENT_*`"
(https://github.com/bitcoin/bitcoin/pull/31042#pullrequestreview-2397244727)
Re-ACK 2076a0863ce6e7fcd5a1f2c29c38c1305816e57d

👍 1440000bytes approved a pull request: "Drop miniupnp dependency"
(https://github.com/bitcoin/bitcoin/pull/31130#pullrequestreview-2397245584)
Code Review ACK https://github.com/bitcoin/bitcoin/pull/31130/commits/40e5f26a3ff77e50df808f6f850c617aec2df203

💬 andrewtoth commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r1817847707)
This would make sense if we wanted to reuse this for CCheckQueue. Then, we could just loop and `ProcessTask` on the main thread once when we call `Wait`.

🤔 glozow reviewed a pull request: "functional test: Additional package evaluation coverage"
(https://github.com/bitcoin/bitcoin/pull/31152#pullrequestreview-2397255278)
reACK f32c34d0c3d4041a301822b27e88d6db4cbf631e

👍 instagibbs approved a pull request: "test: added test to assert TX decode rpc error on submitpackage rpc"
(https://github.com/bitcoin/bitcoin/pull/31139#pullrequestreview-2397254972)
ACK 1fab1d601e40edc7296cf3d60a7e917b0e6493b3

with minor suggestion

💬 instagibbs commented on pull request "test: added test to assert TX decode rpc error on submitpackage rpc":
(https://github.com/bitcoin/bitcoin/pull/31139#discussion_r1817851813)
```suggestion
assert txid_list[0] not in node.getrawmempool()
assert txid_list[1] not in node.getrawmempool()
```

🚀 glozow merged a pull request: "functional test: Additional package evaluation coverage"
(https://github.com/bitcoin/bitcoin/pull/31152)

💬 TheCharlatan commented on pull request "build, ci: Fix linking `bitcoin-chainstate.exe` to `bitcoinkernel.dll` on Windows":
(https://github.com/bitcoin/bitcoin/pull/31158#discussion_r1817854236)
I checked if this could also be a problem for other extern symbols that are currently not used in `bitcoin-chainstate`, but still part of the kernel library. For example in `src/validation.h` we have `extern const std::vector<std::string> CHECKLEVEL_DOC;`, and it looks like using it is indeed problematic too if it were used externally: https://github.com/TheCharlatan/bitcoin/actions/runs/11531552497/job/32102377337#step:10:425.
There are a bunch of other such cases, where I'm not entirely sure
...

📝 hebasto opened a pull request: "cmake: Set top-level target output locations"
(https://github.com/bitcoin/bitcoin/pull/31161)
This PR sets the target output locations to the `bin` and `lib` subdirectories within the build tree, creating a directory structure that mirrors that of the installed targets.

This approach is widely adopted by the large projects, such as [LLVM](https://github.com/llvm/llvm-project/blob/e146c1867e8decfd423034f63a3a863733e03f04/lldb/cmake/modules/LLDBStandalone.cmake#L128-L130):
```cmake
set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)
set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_
...