💬 fanquake commented on pull request "[28.x] Backport #31407":
(https://github.com/bitcoin/bitcoin/pull/32563#discussion_r2120548419)
Thanks, is should be fixed now. It's unrelated to #31161, was a bad cherry-pick fixup by me.

👋 fanquake's pull request is ready for review: "[28.x] Backport #31407"
(https://github.com/bitcoin/bitcoin/pull/32563)

👍 willcl-ark approved a pull request: "policy: uncap datacarrier by default"
(https://github.com/bitcoin/bitcoin/pull/32406#pullrequestreview-2887515593)
ACK a189d636184b1c28fa4a325b56c1fab8f44527b1

I would have also had a slight preference for "removing the (ineffective) knob" a.k.a the previous iteration of this PR, but removing the limited default along with a notice of intended deprecation works for me too.

💬 rleed commented on issue "Add config option to set external P2P port to facilitate incoming connections":
(https://github.com/bitcoin/bitcoin/issues/32657#issuecomment-2929709339)
> I see. I believe this already works, you can add a port number to the `externalip=` argument.

Wonderful! I had tried this before, but I had overlooked a detail in my configuration....my bad. Now it's working! Thank you!

💬 Sjors commented on pull request "init: deprecate `-blockmaxweight` startup option":
(https://github.com/bitcoin/bitcoin/pull/32654#issuecomment-2929711294)
I don't think we should recommend `-blockreservedweight` as an alternative to `-blockmaxweight`, as that just adds confusion.

At this point `-blockmaxweight` seems useless in production, but it appears to be useful in test networks and our owns tests. We could hide the option under `-help-debug` in order to unclutter the `--help` output. But I wouldn't drop it.

💬 fanquake commented on pull request "deps: Bump lief to 0.16.6":
(https://github.com/bitcoin/bitcoin/pull/32431#discussion_r2120625826)
Actually, looking at this again, with 0.16.6 we can just use `has_nx`, as that does include both checks?

💬 fanquake commented on issue "cmake: Replace f`ile(GLOB ...)` command with an explicit list of `*.ts` files":
(https://github.com/bitcoin/bitcoin/issues/32653#issuecomment-2929790543)
What's the actual blocker to just making this change?

✅ maflcko closed an issue: "Add config option to set external P2P port to facilitate incoming connections"
(https://github.com/bitcoin/bitcoin/issues/32657)

💬 maflcko commented on issue "Add config option to set external P2P port to facilitate incoming connections":
(https://github.com/bitcoin/bitcoin/issues/32657#issuecomment-2929801124)
I guess the docs could be updated for the setting in `init.cpp`, to clarify this.

💬 Sjors commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#issuecomment-2929810137)
There might be a silent conflict with #32514.

💬 Sjors commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#discussion_r2120666028)
First `\n` needs to be dropped.

💬 maflcko commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#discussion_r2120693164)
> cc @maflcko is there no test that calls `help` on every method?

There should be one. However, I don't think it can catch this right now, because the `help` command will internally iterate over the individual help calls and ignore the type of the exception (the help is returned via an exception).


See:

```cpp
catch (const std::exception& e)
{
// Help text is returned in an exception
std::string strHelp = std::string(e.what());
```

It wou
...

💬 maflcko commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#discussion_r2120696631)
https://github.com/bitcoin/bitcoin/pull/32588 may also help to catch it, but the code should probably still be made more type-safe.

💬 m3dwards commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#discussion_r2120700485)
Should be addressed

💬 m3dwards commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#issuecomment-2929886599)
> While touching this code, it seems reasonable to also address the other @davidgumberg's [comment](https://github.com/bitcoin/bitcoin/pull/32634#issuecomment-2918081325):

Added

💬 fanquake commented on pull request "depends: don't install & then delete sqlite pkgconf":
(https://github.com/bitcoin/bitcoin/pull/32656#issuecomment-2929895945)
Guix Build:
```bash
545db2ab3bf28143eda2307089bb7266ae64802ea4a15371bacccf4574ee2c67 guix-build-72a5aa9b791e/output/aarch64-linux-gnu/SHA256SUMS.part
660030fb8c70d09e11f35e2ccdde4187f6bd90009bc1da2826be1cf8a73e6f4b guix-build-72a5aa9b791e/output/aarch64-linux-gnu/bitcoin-72a5aa9b791e-aarch64-linux-gnu-debug.tar.gz
e4a702b1d4f589cd9a1d6a0534eee7ddab725a9aa753b5a276510ee9b956ed94 guix-build-72a5aa9b791e/output/aarch64-linux-gnu/bitcoin-72a5aa9b791e-aarch64-linux-gnu.tar.gz
932b1525b03ed1ad
...

💬 hebasto commented on issue "cmake: Replace f`ile(GLOB ...)` command with an explicit list of `*.ts` files":
(https://github.com/bitcoin/bitcoin/issues/32653#issuecomment-2929957366)
> What's the actual blocker to just making this change?

There are no blockers.

However, [`bitcoin-maintainer-tools/update-translations.py`](https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/main/update-translations.py) should be updated first.

💬 mabu44 commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2120739704)
Moving these lines directly before the check on sigops (e.g. to line 228) can improve performances because we avoid performing the calculation when a "return false" is executed for another reason in the block between lines 206 and 227.

💬 mabu44 commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2120745163)
Moving these lines directly before the check on sigops (e.g. to line 228) can improve performances because we avoid performing the calculation when a "return false" is executed for another reason in the block between lines 206 and 227.

💬 hebasto commented on issue "windows: depends config fails":
(https://github.com/bitcoin/bitcoin/issues/32578#issuecomment-2929989264)
> Oh, no. I'm so sorry. I modify my ci.yml as this:

What CI are you using? Is the `VCPKG_INSTALLATION_ROOT` environment variable defined there?