💬 maflcko commented on pull request "ci: move ASAN job to GitHub Actions from Cirrus CI":
(https://github.com/bitcoin/bitcoin/pull/30193#discussion_r1631169842)
The gui tests and benchmarks should also be running under the sanitizer, otherwise bugs may be missed in them.

💬 m3dwards commented on pull request "ci: move ASAN job to GitHub Actions from Cirrus CI":
(https://github.com/bitcoin/bitcoin/pull/30193#discussion_r1631171633)
Oh you are right. This was a late change after I had already had the USDT tests running and didn't check that they were still running 🤦

💬 hebasto commented on pull request "ci: parse TEST_RUNNER_EXTRA into an array":
(https://github.com/bitcoin/bitcoin/pull/30244#issuecomment-2154804383)
> This change allows proper parsing of quotes and complex values such as:
>
> ```shell
> TEST_RUNNER_EXTRA='--exclude "rpc_bind.py --ipv6, feature_proxy.py"'
> ```

Does such an example work on Windows in Command Prompt and PowerShell?

💬 maflcko commented on pull request "ci: parse TEST_RUNNER_EXTRA into an array":
(https://github.com/bitcoin/bitcoin/pull/30244#issuecomment-2154809254)
> Does such an example work on Windows in Command Prompt and PowerShell?

PowerShell and Command Prompt are not supported by `./ci/`. Only `bash`, according to https://github.com/bitcoin/bitcoin/tree/master/ci#running-a-stage-locally

💬 hebasto commented on pull request "ci: parse TEST_RUNNER_EXTRA into an array":
(https://github.com/bitcoin/bitcoin/pull/30244#issuecomment-2154812052)
> > Does such an example work on Windows in Command Prompt and PowerShell?
>
> PowerShell and Command Prompt are not supported by `./ci/`. Only `bash`, according to https://github.com/bitcoin/bitcoin/tree/master/ci#running-a-stage-locally

I'm aware of that. I'm asking about `TEST_RUNNER_EXTRA` variable content given in the example.

💬 m3dwards commented on pull request "ci: parse TEST_RUNNER_EXTRA into an array":
(https://github.com/bitcoin/bitcoin/pull/30244#issuecomment-2154819308)
> Does such an example work on Windows in Command Prompt and PowerShell?

I will test it.

🤔 glozow reviewed a pull request: "util: add VecDeque"
(https://github.com/bitcoin/bitcoin/pull/30161#pullrequestreview-2104539101)
ACK 7b8eea067f

💬 maflcko commented on pull request "test: Add Compact Block Encoding test `ReceiveWithExtraTransactions` covering non-empty `extra_txn`":
(https://github.com/bitcoin/bitcoin/pull/30237#issuecomment-2154833376)
Would it be difficult to make the test deterministic?

💬 pinheadmz commented on pull request "json-rpc 2.0 followups: docs, tests, cli":
(https://github.com/bitcoin/bitcoin/pull/30238#discussion_r1631204799)
Yeah good idea here thanks, updating

💬 pinheadmz commented on pull request "json-rpc 2.0 followups: docs, tests, cli":
(https://github.com/bitcoin/bitcoin/pull/30238#discussion_r1631206987)
hmmm I think I'm going to pass on this, there's a few other files with the `;` and if it doesn't break anything I'll just leave them all the same... but if @fanquake agrees I'll add a commit to this PR.

💬 pinheadmz commented on pull request "json-rpc 2.0 followups: docs, tests, cli":
(https://github.com/bitcoin/bitcoin/pull/30238#discussion_r1631209745)
Great work thanks! I monitored with Wireshark as well. The other local test I did is to build bitcoin with legacy support removed. Only one test should fail, the `interface_rpc` test from #27101 which explicitly makes a legacy request.

```diff
diff --git a/src/rpc/request.cpp b/src/rpc/request.cpp
index 87b9f18b33..e72e349bf6 100644
--- a/src/rpc/request.cpp
+++ b/src/rpc/request.cpp
@@ -200,9 +200,7 @@ void JSONRPCRequest::parse(const UniValue& valRequest)
// The "jsonrpc" key
...

🚀 glozow merged a pull request: "util: add VecDeque"
(https://github.com/bitcoin/bitcoin/pull/30161)

💬 pinheadmz commented on pull request "json-rpc 2.0 followups: docs, tests, cli":
(https://github.com/bitcoin/bitcoin/pull/30238#discussion_r1631214784)
There also would be no `error` or `result` fields in the 204 case! I think it's self explanatory that the authproxy call function expects a very specific type of response.

💬 tdb3 commented on pull request "test: Added test coverage to listsinceblock rpc":
(https://github.com/bitcoin/bitcoin/pull/30195#discussion_r1631217975)
The `exists()` asserts are a nice improvement/sanity check.

💬 tdb3 commented on pull request "json-rpc 2.0 followups: docs, tests, cli":
(https://github.com/bitcoin/bitcoin/pull/30238#discussion_r1631249895)
As a follow-up, checked that bitcoind does actually send the exact reason phrase "No Content", so the updates made are correct.

💬 Sjors commented on pull request "Introduce Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30200#issuecomment-2154903353)
All the above should be fixed now, except for `g_best_block_cv.wait_until` (but that's not blocking).

The biggest change is in 914c26bbd4d7e87af4e0d0edf9af69ae76668b7e. It now makes the `options` argument mandatory for `BlockAssembler` constructor, dropping implicit handling of `ArgsManager`. The caller i.e. the `Mining` interface implementation now handles this.

In Stratum v2 the pool communicates how many extra bytes it needs for its own outputs (payouts, extra commitments, etc). This ne
...

💬 instagibbs commented on pull request "test: expand LimitOrphan and EraseForPeer coverage":
(https://github.com/bitcoin/bitcoin/pull/30082#discussion_r1631271509)
done in first commit now

💬 instagibbs commented on pull request "test: expand LimitOrphan and EraseForPeer coverage":
(https://github.com/bitcoin/bitcoin/pull/30082#discussion_r1631271559)
done

💬 sdaftuar commented on pull request "policy: bump TX_MAX_STANDARD_VERSION to 3":
(https://github.com/bitcoin/bitcoin/pull/29496#issuecomment-2154918042)
utACK 30a01134c

💬 instagibbs commented on pull request "test: expand LimitOrphan and EraseForPeer coverage":
(https://github.com/bitcoin/bitcoin/pull/30082#issuecomment-2154934386)
rebased due to silent merge conflict, with cleanups suggested by @glozow :+1: