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

💬 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 :)