💬 ajtowns commented on pull request "doc: note that you can assume C++20.":
(https://github.com/bitcoin/bitcoin/pull/30136#discussion_r1614990655)
> "you can assume c++20"

Note that it depends on [compiler support](https://en.cppreference.com/w/cpp/compiler_support/20) for the individual features; for instance [modules](https://en.cppreference.com/w/cpp/language/modules) are nominally part of C++20, but still have pretty poor compiler support, so aren't something we can make use of. Conversely, sometimes if a feature is widely supported by compilers/libs, we'll use it even if it's not part of the current language standard (eg #24531 #25
...

💬 laanwj commented on pull request "doc: note that you can assume C++20.":
(https://github.com/bitcoin/bitcoin/pull/30136#discussion_r1615128529)
Right, in practice not even all C++11 features are fully supported, nor always desirable, e.g. don't `thread_local` unless it's for a very good reason and only for POD types. But as starting point before listing all the exceptions i think it still makes sense to mention.

💬 Amarrufo-1 commented on pull request "doc: note that you can assume C++20.":
(https://github.com/bitcoin/bitcoin/pull/30136#issuecomment-2132135446)
I don't even know wat that means

On Sun, May 26, 2024, 1:24 AM laanwj ***@***.***> wrote:

> ***@***.**** commented on this pull request.
> ------------------------------
>
> In doc/developer-notes.md
> <https://github.com/bitcoin/bitcoin/pull/30136#discussion_r1615128529>:
>
> > @@ -771,6 +771,8 @@ Wallet
> General C++
> -------------
>
> +As of v27.0, we require a compiler which meets at least the C++20 standard.
>
> Right, in practice not even all C++11 features are fully s
...

💬 maflcko commented on pull request "doc: add guidance for RPC to developer notes":
(https://github.com/bitcoin/bitcoin/pull/30142#discussion_r1615135575)
how is this different from "- Try to make the RPC response a JSON object."?

The `"warnings"` example here doesn't make much sense, because both are equally flexible to expansion. (See the plural `s`.) The reason why array should be preferred over string is that array is the correct type to represent an array. (But that is obvious, isn't it?)

⚠️ foolbear opened an issue: "show error "could not sign any more inputs" when sign PSBT for multisig"
(https://github.com/bitcoin/bitcoin/issues/30177)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

show error "could not sign any more inputs" when sign PSBT for multisig

### Expected behaviour

will sign succ and broadcast.

1. testnet
2. descriptor wallet
3. [create 2/3 multisig wallet and operation](https://github.com/bitcoin/bitcoin/blob/master/doc/multisig-tutorial.md)
4. succ if using walletprocesspsbt/sendrawtransaction RPC

### Steps to reproduce

1. create unsigned from a
...

✅ maflcko closed an issue: "Restore wallet taking forever to load"
(https://github.com/bitcoin/bitcoin/issues/30108)

💬 maflcko commented on pull request "build, test, doc: Temporarily remove Android-related stuff":
(https://github.com/bitcoin/bitcoin/pull/30049#issuecomment-2132188239)
concept ACK

💬 maflcko commented on pull request "test: switch from curl to urllib for HTTP requests":
(https://github.com/bitcoin/bitcoin/pull/29970#discussion_r1615192257)
https://github.com/bitcoin/bitcoin/pull/29970#discussion_r1614761341

💬 pythcoiner commented on issue "rpc: actually deprecate `rpcuser` & `rpcpass`":
(https://github.com/bitcoin/bitcoin/issues/29240#issuecomment-2132223951)
> Asking users who have always been able to just run the binaries + a config file, to now run a python script from `share/` to generate auth credentials feels a bit, messy?
> * Perhaps `bitcoin-cli` should get a `generaterpcauth` command?

good point, especially for windows users that get the binaries from `bitcoin.org`, they get only the `.exe`, i don't know if they have access to `/share` after installation?
Maybe a feature in bitcoin-qt is also needed?

💬 ceffikhan commented on pull request "test: create assert_not_equal util":
(https://github.com/bitcoin/bitcoin/pull/29500#issuecomment-2132341237)
> follow



> 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

cdcd

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