📝 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

💬 maflcko commented on issue "./contrib/guix/guix-build does not work on riscv64":
(https://github.com/bitcoin/bitcoin/issues/29020#issuecomment-1854061826)
https://git.savannah.gnu.org/cgit/guix.git/commit/?id=d5ca4d4fd713a9f7e17e074a1e37dda99bbb09fc

👍 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

💬 fanquake commented on issue "./contrib/guix/guix-build does not work on riscv64":
(https://github.com/bitcoin/bitcoin/issues/29020#issuecomment-1854097278)
Guess it's time to guix pull and find the next bug.

🤔 stickies-v reviewed a pull request: "Policy: Report reason inputs are non standard"
(https://github.com/bitcoin/bitcoin/pull/29060#pullrequestreview-1779669082)
Concept ACK

💬 stickies-v commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r1425373263)
A function called `AreInputsStandard` that returns a falsy value if inputs are standard is counterintuitive. Would suggest renaming to e.g. `HasNonStandardInput`.

<details>
<summary>git diff on 6ffb9569b7</summary>

```diff
diff --git a/src/bench/ccoins_caching.cpp b/src/bench/ccoins_caching.cpp
index c677fb1c16..9a31cecd7a 100644
--- a/src/bench/ccoins_caching.cpp
+++ b/src/bench/ccoins_caching.cpp
@@ -44,7 +44,7 @@ static void CCoinsCaching(benchmark::Bench& bench)
// Benchmar
...

💬 stickies-v commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r1425375934)
nit: no need for `.has_value()`, idiomatic enough as is imo (here and in a bunch of other places)
```suggestion
BOOST_CHECK(!::AreInputsStandard(CTransaction(txTo), coins));
```

💬 stickies-v commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r1425377178)
Since we've validated `has_value()` just above, can just derefence safely for conciseness:

```suggestion
return state.Invalid(TxValidationResult::TX_INPUTS_NOT_STANDARD, inputs_standardness_result->reason, inputs_standardness_result->debug);
```

💬 stickies-v commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r1425431635)
nit: this is the reason for a single input
```suggestion
struct NonStandardInputReason {
```

💬 stickies-v commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r1425494016)
nit: requires `#include <optional>` (in both files)

💬 stickies-v commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r1425378615)
nit: more concise, easier to read (+ in other places)
```suggestion
assert(!AreInputsStandard(tx_1, coins));
```

💬 stickies-v commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r1425491585)
This causes unnecessary input validation if `!m_require_standard`, would suggest changing to e.g.

<details>
<summary>git diff on 6ffb9569b7</summary>

```diff
diff --git a/src/validation.cpp b/src/validation.cpp
index 96204b2ab4..bf1c578384 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -822,9 +822,10 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
return false; // state filled in by CheckTxInputs
}

- const auto inputs_standardness_r
...

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