📝 Pro2x9 opened a pull request: "Create c-cpp.yml"
(https://github.com/bitcoin/bitcoin/pull/34011)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/34011)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
💬 jsarenik commented on issue "RFC: when to drop testnet3":
(https://github.com/bitcoin/bitcoin/issues/31975#issuecomment-3615858503)
What about asking Wiz to kindly turn off `mempool.space/testnet` which is testnet3 public explorer? Yes, there are many more but this would be a gesture.
(https://github.com/bitcoin/bitcoin/issues/31975#issuecomment-3615858503)
What about asking Wiz to kindly turn off `mempool.space/testnet` which is testnet3 public explorer? Yes, there are many more but this would be a gesture.
👍 hebasto approved a pull request: "[30.x] Backports & 30.1rc1"
(https://github.com/bitcoin/bitcoin/pull/33997#pullrequestreview-3543750729)
ACK 62af018015abfafaf248cb54001dee78228c19da.
nit: A typo in the commit message in 1dbc46bf9cf2c978b09ff60c5c399e1f8d074916:
"Github-Pull: 33528" --> "Github-Pull: #33528"
(https://github.com/bitcoin/bitcoin/pull/33997#pullrequestreview-3543750729)
ACK 62af018015abfafaf248cb54001dee78228c19da.
nit: A typo in the commit message in 1dbc46bf9cf2c978b09ff60c5c399e1f8d074916:
"Github-Pull: 33528" --> "Github-Pull: #33528"
💬 maflcko commented on pull request "ci: Add IWYU job":
(https://github.com/bitcoin/bitcoin/pull/33810#discussion_r2591966035)
nit: Looking at the output, I think this is already done correctly?
If so, sorting can be done as well via:
```diff
diff --git a/src/.clang-format b/src/.clang-format
index c5fcd0b48c..4a1692a4bb 100644
--- a/src/.clang-format
+++ b/src/.clang-format
@@ -97,7 +97,7 @@ ForEachMacros:
- BOOST_FOREACH
IfMacros:
- KJ_IF_MAYBE
-IncludeBlocks: Preserve
+IncludeBlocks: Regroup
IncludeCategories:
- Regex: '^<bitcoin-build-config\.h>'
Priority: -1
```
and then runn
...
(https://github.com/bitcoin/bitcoin/pull/33810#discussion_r2591966035)
nit: Looking at the output, I think this is already done correctly?
If so, sorting can be done as well via:
```diff
diff --git a/src/.clang-format b/src/.clang-format
index c5fcd0b48c..4a1692a4bb 100644
--- a/src/.clang-format
+++ b/src/.clang-format
@@ -97,7 +97,7 @@ ForEachMacros:
- BOOST_FOREACH
IfMacros:
- KJ_IF_MAYBE
-IncludeBlocks: Preserve
+IncludeBlocks: Regroup
IncludeCategories:
- Regex: '^<bitcoin-build-config\.h>'
Priority: -1
```
and then runn
...
💬 Sjors commented on pull request "mining: add getMemoryLoad() and track template non-mempool memory footprint":
(https://github.com/bitcoin/bitcoin/pull/33922#discussion_r2591987168)
This just checks that bench logging happens.
(https://github.com/bitcoin/bitcoin/pull/33922#discussion_r2591987168)
This just checks that bench logging happens.
⚠️ fanquake opened an issue: "ci: failure in Windows native job"
(https://github.com/bitcoin/bitcoin/issues/34012)
Has been re-run multiple times.
https://github.com/bitcoin/bitcoin/actions/runs/19800578606/job/57234826268?pr=33973#step:9:900:
```bash
Completed submission of libevent[core,thread]:x64-windows@2.1.12#7 to 1 binary cache(s) in 248 ms
Waiting for 1 remaining binary cache submissions...
Completed submission of sqlite3[core,json1]:x64-windows@3.50.4 to 1 binary cache(s) in 399 ms (1/1)
All requested installations completed successfully in: 2.4 min
-- Running vcpkg install - done
-- Selecting Wind
...
(https://github.com/bitcoin/bitcoin/issues/34012)
Has been re-run multiple times.
https://github.com/bitcoin/bitcoin/actions/runs/19800578606/job/57234826268?pr=33973#step:9:900:
```bash
Completed submission of libevent[core,thread]:x64-windows@2.1.12#7 to 1 binary cache(s) in 248 ms
Waiting for 1 remaining binary cache submissions...
Completed submission of sqlite3[core,json1]:x64-windows@3.50.4 to 1 binary cache(s) in 399 ms (1/1)
All requested installations completed successfully in: 2.4 min
-- Running vcpkg install - done
-- Selecting Wind
...
💬 fanquake commented on pull request "test: Add DERSIG unit tests to script_tests.json":
(https://github.com/bitcoin/bitcoin/pull/33973#issuecomment-3616123044)
Windows failure is unrelated: #34012.
(https://github.com/bitcoin/bitcoin/pull/33973#issuecomment-3616123044)
Windows failure is unrelated: #34012.
✅ maflcko closed a pull request: "test: Add DERSIG unit tests to script_tests.json"
(https://github.com/bitcoin/bitcoin/pull/33973)
(https://github.com/bitcoin/bitcoin/pull/33973)
📝 maflcko reopened a pull request: "test: Add DERSIG unit tests to script_tests.json"
(https://github.com/bitcoin/bitcoin/pull/33973)
I'd like to add a few `DERSIG` variants of existing `STRICTENC` tests, since `DERSIG` is a consensus flag.
I ran:
```
cmake -B build -DENABLE_WALLET=OFF
cmake --build build --parallel 8
ctest --test-dir build --parallel 8
```
```
6/133 Test #93: script_tests ......................... Passed 1.57 sec
```
(https://github.com/bitcoin/bitcoin/pull/33973)
I'd like to add a few `DERSIG` variants of existing `STRICTENC` tests, since `DERSIG` is a consensus flag.
I ran:
```
cmake -B build -DENABLE_WALLET=OFF
cmake --build build --parallel 8
ctest --test-dir build --parallel 8
```
```
6/133 Test #93: script_tests ......................... Passed 1.57 sec
```
✅ diegoviola closed an issue: "Consider enabling plugin=wayland for bitcoincore.org builds"
(https://github.com/bitcoin-core/gui/issues/916)
(https://github.com/bitcoin-core/gui/issues/916)
💬 diegoviola commented on issue "Consider enabling plugin=wayland for bitcoincore.org builds":
(https://github.com/bitcoin-core/gui/issues/916#issuecomment-3616188091)
> The main concern raised was the introduction of additional dependencies: [bitcoin/bitcoin#22708 (comment)](https://github.com/bitcoin/bitcoin/pull/22708#issuecomment-1100861599).
Ah, I understand. As someone who doesn't use `depends` and compiles their own builds this doesn't affect me. I guess not a lot has changed when it comes to "additional dependencies", so I'll just close this.
(https://github.com/bitcoin-core/gui/issues/916#issuecomment-3616188091)
> The main concern raised was the introduction of additional dependencies: [bitcoin/bitcoin#22708 (comment)](https://github.com/bitcoin/bitcoin/pull/22708#issuecomment-1100861599).
Ah, I understand. As someone who doesn't use `depends` and compiles their own builds this doesn't affect me. I guess not a lot has changed when it comes to "additional dependencies", so I'll just close this.
💬 maflcko commented on issue "ci: failure in Windows native job":
(https://github.com/bitcoin/bitcoin/issues/34012#issuecomment-3616198218)
I haven't looked here, but generally CI does not work well with re-running tasks, when the code is merged with current master, but the ci config is stale.
The solution is to minimize the ci config (yaml) and pull out all ci settings and ci scripts into stand-alone files (similar to the files `.github/ci-lint-exec.py .github/ci-test-each-commit-exec.py`)
As the error is zmq related, it could be due to 49c672853503e561cd1e7fed19a32f21ad345370, but again, I haven't checked this closely.
(https://github.com/bitcoin/bitcoin/issues/34012#issuecomment-3616198218)
I haven't looked here, but generally CI does not work well with re-running tasks, when the code is merged with current master, but the ci config is stale.
The solution is to minimize the ci config (yaml) and pull out all ci settings and ci scripts into stand-alone files (similar to the files `.github/ci-lint-exec.py .github/ci-test-each-commit-exec.py`)
As the error is zmq related, it could be due to 49c672853503e561cd1e7fed19a32f21ad345370, but again, I haven't checked this closely.
💬 maflcko commented on pull request "Feature: Use different datadirs for different signets":
(https://github.com/bitcoin/bitcoin/pull/29838#issuecomment-3616207470)
Could turn into draft, while CI is red?
(https://github.com/bitcoin/bitcoin/pull/29838#issuecomment-3616207470)
Could turn into draft, while CI is red?
💬 fanquake commented on pull request "qa: Remove no longer needed `feature_dirsymlinks.py`":
(https://github.com/bitcoin/bitcoin/pull/33924#issuecomment-3616226134)
~0. Seems fine to leave this for now.
(https://github.com/bitcoin/bitcoin/pull/33924#issuecomment-3616226134)
~0. Seems fine to leave this for now.
🚀 fanquake merged a pull request: "net: fix use-after-free with v2->v1 reconnection logic"
(https://github.com/bitcoin/bitcoin/pull/33956)
(https://github.com/bitcoin/bitcoin/pull/33956)
💬 maflcko commented on pull request "util: generalize `util::Result` to support custom errors":
(https://github.com/bitcoin/bitcoin/pull/34005#issuecomment-3616239972)
> I don't see that as a concern for this PR, since the current `Result` class could just be renamed to `Expected` in #25665 to avoid that if desired. So #34005 and #34006 both seem like good solutions to me, and I'd be happy with either.
Either is fine by me as well. It just seems more churn to (1) introduce the Expected alias based on Result, (2) move it a different file, (3) likely rework it to remove the implicit bilingual_string error stuff (because it is not needed and confusing). It see
...
(https://github.com/bitcoin/bitcoin/pull/34005#issuecomment-3616239972)
> I don't see that as a concern for this PR, since the current `Result` class could just be renamed to `Expected` in #25665 to avoid that if desired. So #34005 and #34006 both seem like good solutions to me, and I'd be happy with either.
Either is fine by me as well. It just seems more churn to (1) introduce the Expected alias based on Result, (2) move it a different file, (3) likely rework it to remove the implicit bilingual_string error stuff (because it is not needed and confusing). It see
...
💬 maflcko commented on pull request "util: generalize `util::Result` to support custom errors":
(https://github.com/bitcoin/bitcoin/pull/34005#discussion_r2592174675)
if this alias is added, one should also be added for `std::unexpected`.
(https://github.com/bitcoin/bitcoin/pull/34005#discussion_r2592174675)
if this alias is added, one should also be added for `std::unexpected`.
✅ maflcko closed an issue: "ASAN use-after-free in `m_reconnections`"
(https://github.com/bitcoin/bitcoin/issues/33615)
(https://github.com/bitcoin/bitcoin/issues/33615)
⚠️ Sjors opened an issue: "rpc: getrawtransaction lookup by witness txid"
(https://github.com/bitcoin/bitcoin/issues/34013)
For Stratum v2 custom job declaration to be bandwidth efficient, the pool can request only the transactions that it doesn't know about.
The spec doesn't specify how this is achieved, but one method is to call `getrawtransaction` on each of the transaction ids listed in [DeclareMiningJob](https://stratumprotocol.org/specification/06-Job-Declaration-Protocol?query=DeclareMiningJob#644-declareminingjob-client-server).
Unfortunately this RPC only supports `txid`, not `wtxid`.
I think the easiest
...
(https://github.com/bitcoin/bitcoin/issues/34013)
For Stratum v2 custom job declaration to be bandwidth efficient, the pool can request only the transactions that it doesn't know about.
The spec doesn't specify how this is achieved, but one method is to call `getrawtransaction` on each of the transaction ids listed in [DeclareMiningJob](https://stratumprotocol.org/specification/06-Job-Declaration-Protocol?query=DeclareMiningJob#644-declareminingjob-client-server).
Unfortunately this RPC only supports `txid`, not `wtxid`.
I think the easiest
...
🤔 hodlinator reviewed a pull request: "rest: allow reading partial block data from storage"
(https://github.com/bitcoin/bitcoin/pull/33657#pullrequestreview-3543967284)
> Could you provide a patch for me which would go through every single transaction in mainchain history and query it both ways (full block + new RPC) and assert that they're equal? I expect it to be heavy, I can run it on my benchmark servers to make sure it works for every real scenario we have.
What value would that provide? The new API allows fetching arbitrary byte-ranges from within blocks. The current unit tests checking the bytes match seem sufficient for validating the logic (testing wi
...
(https://github.com/bitcoin/bitcoin/pull/33657#pullrequestreview-3543967284)
> Could you provide a patch for me which would go through every single transaction in mainchain history and query it both ways (full block + new RPC) and assert that they're equal? I expect it to be heavy, I can run it on my benchmark servers to make sure it works for every real scenario we have.
What value would that provide? The new API allows fetching arbitrary byte-ranges from within blocks. The current unit tests checking the bytes match seem sufficient for validating the logic (testing wi
...