: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.

💬 rkrux commented on pull request "test: MiniWallet: respect passed feerate for padded txs (using `target_weight`)":
(https://github.com/bitcoin/bitcoin/pull/30162#discussion_r1615705400)
Though it's older implementation, for my understanding, why is the actual weight more than the `target_weight`. Doesn't `target_weight` become a misnomer with this?

💬 rkrux commented on pull request "test: MiniWallet: respect passed feerate for padded txs (using `target_weight`)":
(https://github.com/bitcoin/bitcoin/pull/30162#discussion_r1615712714)
Nice to see the removal of many of these repetitive fee calculation code!

💬 rkrux commented on pull request "test: MiniWallet: respect passed feerate for padded txs (using `target_weight`)":
(https://github.com/bitcoin/bitcoin/pull/30162#discussion_r1615734654)
The previous implementation is 10^-5, and the newer one is 10^-6.
IMO, it's worth creating it a constant with an expressive name to make reading this piece easier.

💬 rkrux commented on pull request "test: MiniWallet: respect passed feerate for padded txs (using `target_weight`)":
(https://github.com/bitcoin/bitcoin/pull/30162#discussion_r1615711645)
As per the code in `_bulk_tx`, the actual weight is indeed off by 3 WUs, consider getting rid of `might be` here, it adds a bit of uncertainty imo.

💬 rkrux commented on pull request "test: MiniWallet: respect passed feerate for padded txs (using `target_weight`)":
(https://github.com/bitcoin/bitcoin/pull/30162#discussion_r1615710140)
Should there also be an assertion disallowing passing both `target_weight` and `fee`?

💬 rkrux commented on pull request "test: MiniWallet: respect passed feerate for padded txs (using `target_weight`)":
(https://github.com/bitcoin/bitcoin/pull/30162#discussion_r1615736403)
Preference style: Although this is not a fresh test, but IMHO it makes it far easier for the reviewer to read when the variable name contains the unit as well such as `child_feerate_satvb` or `child_fee_sat`.

💬 rkrux commented on pull request "test: MiniWallet: respect passed feerate for padded txs (using `target_weight`)":
(https://github.com/bitcoin/bitcoin/pull/30162#discussion_r1615739344)
Nice catch here that fixes the calculation!