Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 stickies-v commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r1425375326)
nit: No need for the intermediary var, here and in a bunch of other places

```suggestion
return NonStandardInputsReason{"bad-txns-input-script-nonstandard", strprintf("input %u", i)};
```
👋 hebasto's pull request is ready for review: "msvc: Fix `test\config.ini` content"
(https://github.com/bitcoin/bitcoin/pull/29075)
💬 hebasto commented on pull request "msvc: Fix `test\config.ini` content":
(https://github.com/bitcoin/bitcoin/pull/29075#issuecomment-1854135616)
Undrafted.
💬 mohamedawnallah commented on pull request "test: add TestNode wait_until helper":
(https://github.com/bitcoin/bitcoin/pull/29070#issuecomment-1854137832)
LGTM! Code Review ACK https://github.com/bitcoin/bitcoin/commit/bf0f7dbec6590a54ec890e7a2ca5d85427995334
💬 aureleoules commented on pull request "Replace non-standard CLZ builtins with c++20's bit_width":
(https://github.com/bitcoin/bitcoin/pull/29057#issuecomment-1854139729)
> I don't think the benchmarks will come. I guess the machine has been preempted by AWS? @aureleoules

I've re-ran the job. I actually found a way to re-run preempted jobs automatically!
A lot of benchmark jobs failed yesterday because of #29061 including this pull because I rebase all pulls on master before running jobs and I don't properly handle failures 😬
💬 hebasto commented on pull request "Stratum v2 Template Provider (take 2)":
(https://github.com/bitcoin/bitcoin/pull/28983#issuecomment-1854145231)
> @hebasto any idea how to handle `D:\a\bitcoin\bitcoin\src\common\sv2_noise.h(92,13): error C3646: 'WriteMsg': unknown override specifier `?

> It's not easy being green

Happy to see CI is green :)
💬 mzumsande commented on pull request "net, cli: use v2transport for manual/addrfetch connections, add to -netinfo":
(https://github.com/bitcoin/bitcoin/pull/29058#discussion_r1425547546)
Yes, `ShouldReconnectV1` (which is called in `DisconnectNode()`) will result in the peer only being put on the list of reconnections if it's in `RecvState::KEY`. That is impossible if they ever sent us something, in which case `PerformReconnections()` will do nothing.
💬 mzumsande commented on pull request "net, cli: use v2transport for manual/addrfetch connections, add to -netinfo":
(https://github.com/bitcoin/bitcoin/pull/29058#issuecomment-1854197471)
Also worth noting that while this PR deals with manual connections, we absolutely depend on the reconnection mechanism for automatic connections too:
The logic that we try with v1 based on service flags is just a courtesy but cannot be relied on, since an attacker could add the v2 service flag to arbitrary v1 nodes in most addrmans by spamming the network with the addr with an added v2 service flag, and then everyone thinks it's a v2 peer and depends on the reconnection mechanism when connecti
...
👍 BrandonOdiwuor approved a pull request: "bench: wallet, fix change position out of range error"
(https://github.com/bitcoin/bitcoin/pull/29065#pullrequestreview-1779977403)
ACK 37c75c58202f89b752500f76c872d7f8caf6bdb3

Looks good to me
👍 fanquake approved a pull request: "msvc: Fix `test\config.ini` content"
(https://github.com/bitcoin/bitcoin/pull/29075#pullrequestreview-1780042081)
ACK f76e59d02ea819928b45bd18d9c3a5b1aab36fe2
🚀 fanquake merged a pull request: "msvc: Fix `test\config.ini` content"
(https://github.com/bitcoin/bitcoin/pull/29075)
fanquake closed an issue: "ci: MSVC failures"
(https://github.com/bitcoin/bitcoin/issues/29074)
💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1425606237)
this only works if prioritization happens before mempool acceptance
💬 0xB10C commented on pull request "tracing: Only prepare tracepoint arguments when actually tracing":
(https://github.com/bitcoin/bitcoin/pull/26593#issuecomment-1854284485)
Rebased. Now includes the tracepoint changes from https://github.com/bitcoin/bitcoin/pull/25273.
💬 theStack commented on pull request "net, cli: use v2transport for manual/addrfetch connections, add to -netinfo":
(https://github.com/bitcoin/bitcoin/pull/29058#issuecomment-1854344432)
> I guess in general it's less nice to add options to `-cli` args compared to rpcs: The alternative is that we could allow some construction like `-connect=1.2.3.4:8333|v1transport` (and similar to `-addnode` / `-seednode`) but then we need additional string parsing / documentation for that, and I think we should only do it if there is an actual use case for it.

I agree, this should only be done if there is a really good reason for it.

@naumenkogs: Note that my concerns were not about the
...
💬 brunoerg commented on pull request "fuzz: coinselection, improve `min_viable_change`/`change_output_size`":
(https://github.com/bitcoin/bitcoin/pull/28372#issuecomment-1854350212)
CI error seems unrelated:
```
2023-12-13T15:26:02.5983445Z 54/286 - rpc_signer.py failed, Duration: 3 s
2023-12-13T15:26:02.5983915Z
2023-12-13T15:26:02.5984128Z stdout:
2023-12-13T15:26:02.5985467Z 2023-12-13T15:25:59.249000Z TestFramework (INFO): PRNG seed is: 6112864330657271854
2023-12-13T15:26:02.5986266Z
2023-12-13T15:26:02.5994148Z 2023-12-13T15:25:59.249000Z TestFramework (INFO): Initializing test directory D:\a\_temp\test_runner_₿_🏃_20231213_151925\rpc_signer_229
2023-12-13T1
...
👍 stickies-v approved a pull request: "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly"
(https://github.com/bitcoin/bitcoin/pull/28051#pullrequestreview-1780105857)
re-ACK 6db04be102807ee0120981a9b8de62a55439dabb
💬 achow101 commented on pull request "bench: wallet, fix change position out of range error":
(https://github.com/bitcoin/bitcoin/pull/29065#issuecomment-1854429077)
> This pull does fix the crash but I am still getting valgrind errors when executing the benchmark

Unable to replicate this, so probably a separate issue.
🚀 achow101 merged a pull request: "bench: wallet, fix change position out of range error"
(https://github.com/bitcoin/bitcoin/pull/29065)
achow101 closed an issue: "Benchmark WalletCreateTxUsePresetInputsAndCoinSelection crashes due to #25273"
(https://github.com/bitcoin/bitcoin/issues/29061)