👍 sedited approved a pull request: "script: Add a separate ScriptError for empty pubkeys encountered in Tapscript"
(https://github.com/bitcoin/bitcoin/pull/33961#pullrequestreview-3542392743)
ACK 9d5021a05bd33c73276909eec961777867ddb412
(https://github.com/bitcoin/bitcoin/pull/33961#pullrequestreview-3542392743)
ACK 9d5021a05bd33c73276909eec961777867ddb412
👍 andrewtoth approved a pull request: "Broadcast own transactions only via short-lived Tor or I2P connections"
(https://github.com/bitcoin/bitcoin/pull/29415#pullrequestreview-3542841874)
ACK 2d398050ee31db05e9222149b5e60572ac31d803
(https://github.com/bitcoin/bitcoin/pull/29415#pullrequestreview-3542841874)
ACK 2d398050ee31db05e9222149b5e60572ac31d803
💬 andrewtoth commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2591174137)
nit
```suggestion
const auto pushed_tx_opt{m_tx_for_private_broadcast.GetTxForNode(pfrom.GetId())};
```
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2591174137)
nit
```suggestion
const auto pushed_tx_opt{m_tx_for_private_broadcast.GetTxForNode(pfrom.GetId())};
```
💬 andrewtoth commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2591173094)
nit
```suggestion
const auto opt_tx{m_tx_for_private_broadcast.PickTxForSend(node.GetId())};
```
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2591173094)
nit
```suggestion
const auto opt_tx{m_tx_for_private_broadcast.PickTxForSend(node.GetId())};
```
💬 andrewtoth commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2591176187)
nit
```suggestion
if (const auto num_broadcasted{m_tx_for_private_broadcast.Remove(ptx)}) {
```
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2591176187)
nit
```suggestion
if (const auto num_broadcasted{m_tx_for_private_broadcast.Remove(ptx)}) {
```
⚠️ fugatecorey22 opened an issue: "Admin Dashboard"
(https://github.com/bitcoin/bitcoin/issues/34009)
npm create vite@latest myapp --template react
cd myapp
npm install
npm run dev
(https://github.com/bitcoin/bitcoin/issues/34009)
npm create vite@latest myapp --template react
cd myapp
npm install
npm run dev
📝 rkrux opened a pull request: "wallet: check for `agg_pub` validity in MuSig2 signing"
(https://github.com/bitcoin/bitcoin/pull/34010)
Otherwise conversion of an invalid `agg_pub` to `XOnlyPubKey` in a later step few lines below crashes the flow.
Should fix #33999.
<!--
*** 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 improv
...
(https://github.com/bitcoin/bitcoin/pull/34010)
Otherwise conversion of an invalid `agg_pub` to `XOnlyPubKey` in a later step few lines below crashes the flow.
Should fix #33999.
<!--
*** 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 improv
...
👍 sedited approved a pull request: "test: interface_ipc.py minor fixes and cleanup"
(https://github.com/bitcoin/bitcoin/pull/34003#pullrequestreview-3543353009)
ACK 6ef092c0e4343fc421c323ff09d3c791fb4bc33a
Thanks for the cleanup! Have another suggestion for saving a few lines and naming clarity:
<details>
<summary>Diff</summary>
```diff
diff --git a/test/functional/interface_ipc.py b/test/functional/interface_ipc.py
index 804f7f8f64..f8f3a76ec7 100755
--- a/test/functional/interface_ipc.py
+++ b/test/functional/interface_ipc.py
@@ -176,2 +176 @@ class IPCInterfaceTest(BitcoinTestFramework):
- template3 = await template2.wa
...
(https://github.com/bitcoin/bitcoin/pull/34003#pullrequestreview-3543353009)
ACK 6ef092c0e4343fc421c323ff09d3c791fb4bc33a
Thanks for the cleanup! Have another suggestion for saving a few lines and naming clarity:
<details>
<summary>Diff</summary>
```diff
diff --git a/test/functional/interface_ipc.py b/test/functional/interface_ipc.py
index 804f7f8f64..f8f3a76ec7 100755
--- a/test/functional/interface_ipc.py
+++ b/test/functional/interface_ipc.py
@@ -176,2 +176 @@ class IPCInterfaceTest(BitcoinTestFramework):
- template3 = await template2.wa
...
💬 sedited commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#issuecomment-3615621559)
> What is wrong with the most obvious approach to error handling?
I am guessing you are asking why not just use exceptions? If so, I think they could be an alternative for these situations too. I'd be curious how a similar function contract could be enforced with an exception.
(https://github.com/bitcoin/bitcoin/pull/34006#issuecomment-3615621559)
> What is wrong with the most obvious approach to error handling?
I am guessing you are asking why not just use exceptions? If so, I think they could be an alternative for these situations too. I'd be curious how a similar function contract could be enforced with an exception.
👍 sedited approved a pull request: "depends: Propagate native C compiler to `sqlite` package"
(https://github.com/bitcoin/bitcoin/pull/33995#pullrequestreview-3543468882)
ACK 710031ebef838d2f0a1effa19170bef7b130bbeb
(https://github.com/bitcoin/bitcoin/pull/33995#pullrequestreview-3543468882)
ACK 710031ebef838d2f0a1effa19170bef7b130bbeb
💬 plebhash commented on issue "Block template memory management (for IPC clients)":
(https://github.com/bitcoin/bitcoin/issues/33899#issuecomment-3615719206)
during some IRL discussions with @ismaelsadeeq and @Shourya742 we brainstormed some potential alternative approach to this
instead of trying to establish boundaries on how much memory would be consumed with templates, we could simply start from the assumption that any economically rational implementation always aims to be hashing on whatever is the latest (and therefore most profitable) template
while the most conservative approach would try to keep as many templates as possible in memory (bec
...
(https://github.com/bitcoin/bitcoin/issues/33899#issuecomment-3615719206)
during some IRL discussions with @ismaelsadeeq and @Shourya742 we brainstormed some potential alternative approach to this
instead of trying to establish boundaries on how much memory would be consumed with templates, we could simply start from the assumption that any economically rational implementation always aims to be hashing on whatever is the latest (and therefore most profitable) template
while the most conservative approach would try to keep as many templates as possible in memory (bec
...
💬 maflcko commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#issuecomment-3615779397)
> just use exceptions?
I don't think exceptions are a suitable alternative for low-level code in this repo, where `std::expected` is appropriate. In places where exceptions are used today, it is tedious to review the code and its behavior. For example, the same exception from an IO error in a database will sometimes terminate the program unclean, and sometimes cleanly shut down the program (https://github.com/bitcoin/bitcoin/pull/33042#issuecomment-3116540946), depending on which thread or wi
...
(https://github.com/bitcoin/bitcoin/pull/34006#issuecomment-3615779397)
> just use exceptions?
I don't think exceptions are a suitable alternative for low-level code in this repo, where `std::expected` is appropriate. In places where exceptions are used today, it is tedious to review the code and its behavior. For example, the same exception from an IO error in a database will sometimes terminate the program unclean, and sometimes cleanly shut down the program (https://github.com/bitcoin/bitcoin/pull/33042#issuecomment-3116540946), depending on which thread or wi
...
📝 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)