⚠️ sreekv143 opened an issue: "Sree"
(https://github.com/bitcoin/bitcoin/issues/30179)
Ntg

✅ pinheadmz closed an issue: "Sree"
(https://github.com/bitcoin/bitcoin/issues/30179)

💬 S3RK commented on pull request "wallet, rpc: document and update `sendall` behavior around unconfirmed inputs":
(https://github.com/bitcoin/bitcoin/pull/28979#issuecomment-2132788146)
ACK 71aae72e1fc998b2629d68a7301d85dc1b65641e

💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1615586509)
I guess this could work if `timeout` isn't too long. Since if the router doens't support NAT-PMP / PCP it's not going to reply, it delays when we fall back to UPNP. But a few seconds seems fine.

💬 S3RK commented on pull request "Fix waste calculation in SelectionResult":
(https://github.com/bitcoin/bitcoin/pull/28366#issuecomment-2132825213)
Happy to reack, but you need to fix clang-tidy first

```
wallet/test/coinselector_tests.cpp:893:43: error: argument name 'current_fee' in comment does not match parameter name 'fee' [bugprone-argument-comment,-warnings-as-errors]
893 | add_coin(1 * COIN, 1, selection1, /*current_fee=*/fee, /*long_term_fee=*/fee - fee_diff);
| ^
wallet/test/coinselector_tests.cpp:61:90: note: 'fee' declared here
61 | static void add_coin(const C
...

💬 S3RK commented on pull request "tests: improve wallet multisig descriptor test and docs":
(https://github.com/bitcoin/bitcoin/pull/29154#issuecomment-2132844324)
Code Review ACK d93b79470916b1e6f85c55cc6beb1e41b382196f

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1615628209)
The timeout is still one second per try (so three seconds in total maximum, given current retries), it's just not possible to extend it indefinitely anymore by sending rejected packets.

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

💬 Sjors commented on pull request "[PoC] ci: Add FreeBSD GitHub Actions job":
(https://github.com/bitcoin/bitcoin/pull/30164#issuecomment-2132883060)
We could pick 13 (13.3 if you have to specify) and then `pkg install llvm16`.

I'm able to compile on a VM running 13.2 and with either clang 15 or 16 manually installed:

```
./configure CC=/usr/local/bin/clang16 CXX=/usr/local/bin/clang++16 MAKE=gmake
```

It seems that the 13.* point releases are maintained for 3 months each, but 13 in general is supported until April 30, 2026.

https://www.freebsd.org/security/#sup

💬 ajtowns commented on pull request "cli: restrict multiple exclusive argument usage in bitcoin-cli":
(https://github.com/bitcoin/bitcoin/pull/30148#issuecomment-2132902588)
Rather than checking each one of these options individually, why not have an `OptionsCategory` for them, and iterate over that? That has the advantage that it lists them separately when you invoke `bitcoin-cli -help`, eg:

```
...
-testnet
Use the test chain. Equivalent to -chain=test.

CLI Commands:

-addrinfo
Get the number of addresses known to the node, per network and total,
after filtering for quality and recency. The total number of
addresses kn
...

⚠️ kosuodhmwa opened an issue: ""bitcoin-cli" does not exist, while "bitcoind" does in ~/bitcoin/src folder"
(https://github.com/bitcoin/bitcoin/issues/30180)
All manuals to create a wallet are either:

- GUI (click there, click ehere)
OR
- On console with "bitcoin-cli" command so it seems


But i don't have a GUI desktop and also "bitcoin-cli" seems to be missing.

So how to create / configure a new local wallet when wallet support is compiled in (as it is) on "bitcoind" ?

Thank you very much for your feedback(s).


With best regards,
Jan

💬 kosuodhmwa commented on issue ""bitcoin-cli" does not exist, while "bitcoind" does in ~/bitcoin/src folder":
(https://github.com/bitcoin/bitcoin/issues/30180#issuecomment-2132914198)
Or do i need to set other compile settings for that?
https://github.com/bitcoin/bitcoin/issues/30158

💬 kosuodhmwa commented on issue "Log: "no wallet support compiled in" when i start bitcoind":
(https://github.com/bitcoin/bitcoin/issues/30158#issuecomment-2132915629)
"Missing wallet support" log message is gone but there are some other questions
https://github.com/bitcoin/bitcoin/issues/30180

💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2132952090)
> Do you know what's the minimum FreeBSD version it can be compiled on? Let's bump the version bound to that.

Just tried on 14.0 with its default clang 16 and I get the same error. So we should either find a workaround or disable FreeBSD for this feature and a TODO comment. I don't know how popular this is as a desktop distro?

💬 carnhofdaki commented on pull request "Feature: Use different datadirs for different signets":
(https://github.com/bitcoin/bitcoin/pull/29838#issuecomment-2132979950)
Concept ACK

💬 carnhofdaki commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-2132996407)
Concept ACK

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2133003388)
> Just tried on 14.0 with its default clang 16 and I get the same error. So we should either find a workaround or disable FreeBSD for this feature and a TODO comment. I don't know how popular this is as a desktop distro?

i would feel bad disabling FreeBSD support after @vasild contributed the code for that, but if this gets close to merge and FreeBSD is still broken i'll remove it.
i expect `#define typeof __typeof__` would go a long way to work around this error.

💬 rkrux commented on pull request "test: MiniWallet: respect passed feerate for padded txs (using `target_weight`)":
(https://github.com/bitcoin/bitcoin/pull/30162#discussion_r1615708591)
Non blocking nit: This seems to be testing for a specific piece and if you don't anticipate it to be updated later, WDYT about either adding this test in an existing functional test, or adding this in the unit tests of the functional test framework?

🤔 rkrux reviewed a pull request: "test: MiniWallet: respect passed feerate for padded txs (using `target_weight`)"
(https://github.com/bitcoin/bitcoin/pull/30162#pullrequestreview-2080359105)
Concept ACK [93527b8](https://github.com/bitcoin/bitcoin/pull/30162/commits/93527b82e70c8e19d7317ce5287b0ee2a0020f1b)

Make is successful, so are all functional tests.

I am in support of the changes in this PR because it fixes the tx fee calculation and improves the caller code in various places. Asked few questions and provided suggestion, would like to take another look later and provide ACK.

💬 rkrux commented on pull request "test: MiniWallet: respect passed feerate for padded txs (using `target_weight`)":
(https://github.com/bitcoin/bitcoin/pull/30162#discussion_r1615703491)
In the previous implementation, it seemed a little unusual that `b'a'` was added first, and then more padding was added later. The newer approach seems cleaner where padding is added only once.