💬 ceffikhan commented on pull request "test: create assert_not_equal util":
(https://github.com/bitcoin/bitcoin/pull/29500#issuecomment-2132348090)
> In the functional tests there are lots of cases where we assert != which we now swap with assert_not_equal to be more readable
>
> This is motivated/uses logic from this PR which was closed #28528 This partially helps #23119
>
> I've broken it up to just `assert_not_equal` to keep the PR smaller as suggested in [#28528 (comment)](https://github.com/bitcoin/bitcoin/pull/28528#issuecomment-1959945805)
>
> I can create follow up PR's if this is wanted

I also had the motivation to do t
...

🤔 tdb3 reviewed a pull request: "test: Set mocktime in p2p_disconnect_ban.py to avoid intermittent test failure"
(https://github.com/bitcoin/bitcoin/pull/30174#pullrequestreview-2079853732)
Concept ACK

Thank you. It's nice to make tests more robust even in slower environments/configurations.
Did you run into this specific failure scenario, or simply being proactive?



nit: If another push happens, these look like typos:

https://github.com/bitcoin/bitcoin/blob/fa4f0decb9106b5046fda76baf2f7ed44a5d3c62/test/functional/p2p_disconnect_ban.py#L21

```diff
diff --git a/test/functional/p2p_disconnect_ban.py b/test/functional/p2p_disconnect_ban.py
index e50fc78056d..e47f9c7
...

💬 tdb3 commented on pull request "test: Set mocktime in p2p_disconnect_ban.py to avoid intermittent test failure":
(https://github.com/bitcoin/bitcoin/pull/30174#discussion_r1615381582)
At first glance, this seems like it may allow for node 0 and node 1 to diverge in time (node 1's time is forced to a point potentially in the past, while node 0 isn't). Had an initial thought about the ramifications of allowing this to happen. Since this test is exercising banning rather than block generation, I'm not sure this would matter.

⚠️ 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.