Bitcoin Core Github
44 subscribers
121K links
Download Telegram
👍 AngusP approved a pull request: "test: switch from curl to urllib for HTTP requests"
(https://github.com/bitcoin/bitcoin/pull/29970#pullrequestreview-2079184785)
ACK 588cad38f7813d88f022d0304aa20a0b96d184ed
⚠️ lcharles123 opened an issue: "Add "maxuploadtargettimeframe" to change the timeframe considered by "maxuploadtarget""
(https://github.com/bitcoin/bitcoin/issues/30176)
### Please describe the feature you'd like to see added.

The "maxuploadtarget"
# Tries to keep outbound traffic under the given target per 24h (one day).

But many ISPs that does not have unlimited traffic account it by month. So it will be a handy setting to avoid the needing to calculate the amount transferred by the billing period. This new setting can be specified in days.

### Is your feature related to a problem, if so please describe it.

_No response_

### Describe the solution you'
...
💬 sipa commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#issuecomment-2131728449)
Apparently `std::is_trivially_default_constructible_v<T>` does not imply you can just memset 0 to construct the objects (or at least, I can't find evidence of that). So I've dropped that branch.
💬 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
💬 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.