Bitcoin Core Github
43 subscribers
123K links
Download Telegram
📝 virtu opened a pull request: "contrib: asmap-tool - Compare ASMaps with respect to specific addresses"
(https://github.com/bitcoin/bitcoin/pull/30246)
Right now, we have no way to quantify the "degradation" of an ASMap over time in the context of Bitcoin's P2P network in a meaningful way. However, such data would be useful for:
1. Making sure the minimum shelf life of ASMaps is compatible with the release cycle (we wouldn't want to start shipping ASMaps with releases before making sure ASMaps typically do not become obsolete before the time of the next release)
2. Node operators eager to keep their ASMaps up-to-date between releases.

Whil
...
👍 maflcko approved a pull request: "ci: parse TEST_RUNNER_EXTRA into an array"
(https://github.com/bitcoin/bitcoin/pull/30244#pullrequestreview-2104485950)
I can't review this, because it is written in bash, but testing seems to indicate that this works as intended for some reason.

ACK b9c0b79ab7a0d4dafd5386a78f4fd747236c8f88
💬 maflcko commented on pull request "ci: parse TEST_RUNNER_EXTRA into an array":
(https://github.com/bitcoin/bitcoin/pull/30244#discussion_r1631166504)
Is this still needed?
💬 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