Bitcoin Core Github
44 subscribers
121K links
Download Telegram
⚠️ fanquake opened an issue: "ci: MSVC failures"
(https://github.com/bitcoin/bitcoin/issues/29074)
The Windows native CI is failing on unrelated changes, i.e in #29072.
https://github.com/bitcoin/bitcoin/actions/runs/7194911390/job/19596459746?pr=29072:
```bash
rpc_help.py | Failed | 1 s
rpc_signer.py | Failed | 4 s
wallet_signer.py --descriptors | Failed | 3 s
```

```bash
node0 2023-12-13T12:48:04.392953Z [D:\a\bitcoin\bitcoin\src\rpc\request.cpp:187] [parse] [rpc]
...
💬 fanquake commented on issue "ci: MSVC failures":
(https://github.com/bitcoin/bitcoin/issues/29074#issuecomment-1853871903)
This is happening on master too, and is related to disabling external signer on windows.
👍 stickies-v approved a pull request: "refactor: Remove pre-C++20 code, fs::path cleanup"
(https://github.com/bitcoin/bitcoin/pull/29040#pullrequestreview-1779595471)
ACK 856c88776f8486446602476a1c9e133ac0cff510
👍 fanquake approved a pull request: "Bump minimum required Boost version due to migration to C++20"
(https://github.com/bitcoin/bitcoin/pull/29066#pullrequestreview-1779598310)
ACK 49a90915aa3ee8e3a7e163f23a55de931faf8523
💬 maflcko commented on issue "ci: MSVC failures":
(https://github.com/bitcoin/bitcoin/issues/29074#issuecomment-1853892781)
I guess the functional tests fail to see that there is no signer for some reason?
💬 sdaftuar commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#issuecomment-1853897416)
I thought of a couple issues with the current version of the PR that I wanted to summarize (longer form [here](https://delvingbitcoin.org/t/cluster-mempool-rbf-thoughts/156/23?u=sdaftuar)):

1. The mempool might not be made "strictly better", in the sense that there are more fees available for every size of transactions selected, using the RBF criteria in this PR. (I'm trying to figure out if there are any simple heuristics we can apply to resolve this issue.)

2. It occurred to me that use
...
🤔 maflcko reviewed a pull request: "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly"
(https://github.com/bitcoin/bitcoin/pull/28051#pullrequestreview-1779599067)
ACK 6db04be102807ee0120981a9b8de62a55439dabb 👗

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK 6db04be102807ee0120981a9b8
...
💬 maflcko commented on pull request "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly":
(https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1425329431)
in 73133c36aa: Just to note another thing: Now that the interrupt inside the NodeContext is unset, unit tests can not use it and must use the m_interrupt
💬 maflcko commented on pull request "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly":
(https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1425340275)
Nit in the last commit: Shouldn't new node code be in the node namespace? Otherwise it will have to be touched again when it is moved into the node namespace later?
💬 vasild commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#issuecomment-1853978983)
Would this resolve the problem?

<details>
<summary>[patch] update g_initial_block_download_completed periodically based on our tip age</summary>

```diff
diff --git i/src/net_processing.cpp w/src/net_processing.cpp
index df54a62f28..ec9bbff8f2 100644
--- i/src/net_processing.cpp
+++ w/src/net_processing.cpp
@@ -2045,13 +2045,12 @@ void PeerManagerImpl::NewPoWValidBlock(const CBlockIndex *pindex, const std::sha
* Update our best height and announce any block hashes which weren't pre
...
🚀 fanquake merged a pull request: "Bump minimum required Boost version due to migration to C++20"
(https://github.com/bitcoin/bitcoin/pull/29066)
fanquake closed an issue: "build: GCC 10.5 fails to compile `test_bitcoin` using Boost.Test 1.71"
(https://github.com/bitcoin/bitcoin/issues/29063)
📝 hebasto opened a pull request: "msvc: Fix `test\config.ini` content"
(https://github.com/bitcoin/bitcoin/pull/29075)
Closes https://github.com/bitcoin/bitcoin/issues/29074.
💬 Sjors commented on pull request "Replace Boost.Process with cpp-subprocess":
(https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-1854029824)
Tested on Ubuntu 23.10.

This slightly changes the way strings are escaped, please use this:

```diff
--- a/src/external_signer.cpp
+++ b/src/external_signer.cpp
@@ -60,43 +60,43 @@ bool ExternalSigner::Enumerate(const std::string& command, std::vector<ExternalS
return true;
}

UniValue ExternalSigner::DisplayAddress(const std::string& descriptor) const
{
- return RunCommandParseJSON(m_command + " --fingerprint " + m_fingerprint + NetworkArg() + " displayaddress --desc \
...
💬 vasild commented on pull request "sync: introduce a thread-safe generic container and use it to remove a bunch of "GlobalMutex"es":
(https://github.com/bitcoin/bitcoin/pull/25390#issuecomment-1854032522)
`92a444d0d7...f3489638dc`: rebase due to conflicts
💬 maflcko commented on pull request "Replace non-standard CLZ builtins with c++20's bit_width":
(https://github.com/bitcoin/bitcoin/pull/29057#issuecomment-1854044883)
I don't think the benchmarks will come. I guess the machine has been preempted by AWS? @aureleoules
👍 BrandonOdiwuor approved a pull request: "test: add TestNode wait_until helper"
(https://github.com/bitcoin/bitcoin/pull/29070#pullrequestreview-1779854277)
Code review ACK bf0f7dbec6590a54ec890e7a2ca5d85427995334

Looks good to me
👋 brunoerg's pull request is ready for review: "fuzz: coinselection, improve `min_viable_change`/`change_output_size`"
(https://github.com/bitcoin/bitcoin/pull/28372)
💬 brunoerg commented on pull request "fuzz: coinselection, improve `min_viable_change`/`change_output_size`":
(https://github.com/bitcoin/bitcoin/pull/28372#issuecomment-1854095552)
Rebased