💬 Christewart commented on pull request "net, init: derive default onion port if a user specified a -port":
(https://github.com/bitcoin/bitcoin/pull/31223#issuecomment-2457898255)
concept ACK

💬 furszy commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#issuecomment-2457975437)
Sorry for the delay.
@maflcko, the issue seems to be that the exception occurs in the test base class constructor. Which is not part of the actual test code and runs prior to it. So it might not be captured by the framework, throwing the "unknown location error".
Just dropped the fix and pushed a commit to verify it. Let's see what the CI prints now.

⚠️ Sandra-Amina-Boss opened an issue: "Sm"
(https://github.com/bitcoin/bitcoin/issues/31224)
https://github.com/bnb-chain/greenfield-whitepaper/tree/main

✅ fanquake closed an issue: "Sm"
(https://github.com/bitcoin/bitcoin/issues/31224)

:lock: fanquake locked an issue: "Sm"
(https://github.com/bitcoin/bitcoin/issues/31224)

💬 marcofleon commented on pull request "fuzz: Fix difficulty target generation in `p2p_headers_presync`":
(https://github.com/bitcoin/bitcoin/pull/31213#discussion_r1829926285)
If I wanted to do a lower target based on `MinimumChainWork`, I think I could do
```
target = (~work + 1) / work
```
where work would be `MinimumChainWork / 1600` or maybe a bit less to be safe.

👋 glozow's pull request is ready for review: "TxDownloadManager followups"
(https://github.com/bitcoin/bitcoin/pull/31190)

💬 achow101 commented on pull request "Update secp256k1 subtree to v0.6.0":
(https://github.com/bitcoin/bitcoin/pull/31216#issuecomment-2458061763)
> Should we disable the musig module in this PR and enable it only when it is needed?

Done

💬 glozow commented on pull request "BlockAssembler: return selected packages vsize and feerate":
(https://github.com/bitcoin/bitcoin/pull/30391#discussion_r1829943882)
Nonblocking, I can understand it's a larger change to use it.

💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r1829944320)
Good catch! Fixed.

📝 secp512k2 opened a pull request: "doc: Fix grammatical errors in multisig-tutorial.md"
(https://github.com/bitcoin/bitcoin/pull/31225)
This pull request fixes grammatical errors in the 'multisig-tutorial.md' document.

Corrections:

1. Incorrect Phrase "As can been seen":

- Before:

There are discussions about eliminating this redundancy, as can been seen in the issue #17190 (https://github.com/bitcoin/bitcoin/issues/17190).

- After:

There are discussions about eliminating this redundancy, as can be seen in the issue #17190 (https://github.com/bitcoin/bitcoin/issues/17190).

2. Clarity Improveme
...

💬 hebasto commented on pull request "build: Unify `-logsourcelocations` format":
(https://github.com/bitcoin/bitcoin/pull/30811#issuecomment-2458093729)
My Guix build:
```
aarch64
67b7e95e0f1d45debad8a8cac4c2d349e3e01efcf62e0b109345896f296758d9 guix-build-788c1324f3d8/output/aarch64-linux-gnu/SHA256SUMS.part
cd3c67eaa5f6fadf0a73cdb1aa03563d720fc7fdae1b59aa495a0eda9d2695af guix-build-788c1324f3d8/output/aarch64-linux-gnu/bitcoin-788c1324f3d8-aarch64-linux-gnu-debug.tar.gz
5c1eb4ff827336a9d113497459981e213844b87e232c33fc00317ce20302f761 guix-build-788c1324f3d8/output/aarch64-linux-gnu/bitcoin-788c1324f3d8-aarch64-linux-gnu.tar.gz
dd78923f
...

👍 hebasto approved a pull request: "Update secp256k1 subtree to v0.6.0"
(https://github.com/bitcoin/bitcoin/pull/31216#pullrequestreview-2416632014)
ACK 97235c446e9986ecca09c2a4b78d6c6239853fdb, verified by updating the secp256k1 subtree locally.

💬 murchandamus commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#issuecomment-2458206441)
It looks like all three dependencies got merged, is this ready for review?

💬 achow101 commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#issuecomment-2458297336)
> It looks like all three dependencies got merged, is this ready for review?

Currently it is still dependent on #30328 but I suppose it doesn't have to be.

🤔 tdb3 reviewed a pull request: "net, init: derive default onion port if a user specified a -port"
(https://github.com/bitcoin/bitcoin/pull/31223#pullrequestreview-2417053413)
Approach ACK

Seemed to work nicely with two node instances run as follows:
```
build/src/bitcoind -port=11000 -rpcport=11100 -datadir=/mnt/tmp/n1/
```
```
build/src/bitcoind -port=12000 -rpcport=12100 -datadir=/mnt/tmp/n2/
```

```
ss -tulpn
...

... 127.0.0.1:11100 0.0.0.0:* users:(("bitcoind",pid=192480,fd=12))
... 127.0.0.1:11001 0.0.0.0:* users:(("bitcoind",pid=192480,fd=29))
... 127.0.0.1:12100
...

💬 tdb3 commented on pull request "net, init: derive default onion port if a user specified a -port":
(https://github.com/bitcoin/bitcoin/pull/31223#discussion_r1830257430)
`std::optional::value_or()` might work nicely here.

e.g.
```diff
diff --git a/src/torcontrol.cpp b/src/torcontrol.cpp
index d56216a7cb0..b518cf1bb93 100644
--- a/src/torcontrol.cpp
+++ b/src/torcontrol.cpp
@@ -715,5 +715,5 @@ CService DefaultOnionServiceTarget(std::optional<uint16_t> port)
{
struct in_addr onion_service_target;
onion_service_target.s_addr = htonl(INADDR_LOOPBACK);
- return {onion_service_target, port.has_value() ? port.value() : BaseParams().OnionServ
...

💬 tdb3 commented on pull request "test: cover edge case for lockunspent rpc":
(https://github.com/bitcoin/bitcoin/pull/31209#issuecomment-2458574314)
> Not sure about this. This seems to be adding coverage for code that shouldn't exist in the first place.
>
> The integer is passed to `COutPoint`, which only takes unsigned integers, so parsing the integer as such would be more consistent. The check would then be redundant and could be removed.

Good catch. It seems a bit odd that the RPC is trying to interpret the `vout` arg as a *signed* int in the first place (`const int nOutput = o.find_value("vout").getInt<int>();`), then check for ne
...

💬 vasild commented on pull request "Fee Estimation via Fee rate Forecasters":
(https://github.com/bitcoin/bitcoin/pull/30157#issuecomment-2458703251)
Concept ACK

> Allow node users to be self-sovereign by using their node's estimates, reducing reliance on third-party fee estimations.

> having users feel the need to use external fee estimation services that they could equally have served to them by their own node, feels sub-optimal

Indeed, I do that :face_with_head_bandage:

💬 naumenkogs commented on pull request "Package validation: accept packages of size 1":
(https://github.com/bitcoin/bitcoin/pull/31096#issuecomment-2458962465)
@stickies-v i think you might be looking at the wrong commit. The hash you acked is outdated.