📝 fanquake opened a pull request: "[28.x] More backports"
(https://github.com/bitcoin/bitcoin/pull/33415)
Further backports for `28.x`.
(https://github.com/bitcoin/bitcoin/pull/33415)
Further backports for `28.x`.
💬 brunoerg commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2355204852)
@TheCharlatan I tried to run this target and got 0 exec/s which is pretty bad. I was expecting a bad performance due to the number of tasks and the pool overhead btw.
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2355204852)
@TheCharlatan I tried to run this target and got 0 exec/s which is pretty bad. I was expecting a bad performance due to the number of tasks and the pool overhead btw.
💬 waketraindev commented on pull request "net: Provide block templates to peers on request":
(https://github.com/bitcoin/bitcoin/pull/33191#discussion_r2355215514)
```suggestion
HelpExampleCli("gettemplateinfo", "")
```
Fix for rpc examples string
(https://github.com/bitcoin/bitcoin/pull/33191#discussion_r2355215514)
```suggestion
HelpExampleCli("gettemplateinfo", "")
```
Fix for rpc examples string
💬 waketraindev commented on pull request "net: Provide block templates to peers on request":
(https://github.com/bitcoin/bitcoin/pull/33191#discussion_r2355224390)
Prefer throwing an RPC exception here instead of returning default TemplateStats
(https://github.com/bitcoin/bitcoin/pull/33191#discussion_r2355224390)
Prefer throwing an RPC exception here instead of returning default TemplateStats
💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2355234167)
No, that's included in `sizeof(Cluster)` above.
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2355234167)
No, that's included in `sizeof(Cluster)` above.
💬 TheCharlatan commented on pull request "RFC: Riscv bare metal CI job":
(https://github.com/bitcoin/bitcoin/pull/31425#issuecomment-3302609393)
Rebased a577bbe5df766a4e0e5a36fc997b7880eec45c0e -> e254de76ac0a390569bc5b561f61babb6d021dcb ([bare_metal_support_3](https://github.com/TheCharlatan/bitcoin/tree/bare_metal_support_3) -> [bare_metal_support_4](https://github.com/TheCharlatan/bitcoin/tree/bare_metal_support_4), [compare](https://github.com/TheCharlatan/bitcoin/compare/bare_metal_support_3..bare_metal_support_4))
(https://github.com/bitcoin/bitcoin/pull/31425#issuecomment-3302609393)
Rebased a577bbe5df766a4e0e5a36fc997b7880eec45c0e -> e254de76ac0a390569bc5b561f61babb6d021dcb ([bare_metal_support_3](https://github.com/TheCharlatan/bitcoin/tree/bare_metal_support_3) -> [bare_metal_support_4](https://github.com/TheCharlatan/bitcoin/tree/bare_metal_support_4), [compare](https://github.com/TheCharlatan/bitcoin/compare/bare_metal_support_3..bare_metal_support_4))
💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2355242458)
I'd rather keep the external selector (`Level level`) and internal identifier (`int level`) separate, as it may allow adding more external ones later if need be (e.g. a `Level::SINGLE` which requires there to be only the main level and asserts otherwise, or a `Level::STAGING` which requires a staging level to exist). Also, if we'd add a third level (which may be needed to implement `testmempoolaccept` with package RBF) there may be even more level selectors to add.
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2355242458)
I'd rather keep the external selector (`Level level`) and internal identifier (`int level`) separate, as it may allow adding more external ones later if need be (e.g. a `Level::SINGLE` which requires there to be only the main level and asserts otherwise, or a `Level::STAGING` which requires a staging level to exist). Also, if we'd add a third level (which may be needed to implement `testmempoolaccept` with package RBF) there may be even more level selectors to add.
💬 TheCharlatan commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#issuecomment-3302633627)
Approach ACK
(https://github.com/bitcoin/bitcoin/pull/33336#issuecomment-3302633627)
Approach ACK
💬 ArmchairCryptologist commented on pull request "policy: lower the default blockmintxfee, incrementalrelayfee, minrelaytxfee":
(https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3302812634)
FWIW, one very well-connected node I have running with minrelaytxfee+incrementalrelayfee of 0.1 sats/vB has had 81.64% successful compact block reconstructions without any requested transactions in all of September, counting 2032 of 2489 blocks as of this writing. And of the remaining 457 blocks, 315 only requested a single transaction.
As far as I can tell, around 10% of nodes that report a minfeefilter are already using 0.1 sats/vB or less at this point, so it should improve rapidly.
(https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3302812634)
FWIW, one very well-connected node I have running with minrelaytxfee+incrementalrelayfee of 0.1 sats/vB has had 81.64% successful compact block reconstructions without any requested transactions in all of September, counting 2032 of 2489 blocks as of this writing. And of the remaining 457 blocks, 315 only requested a single transaction.
As far as I can tell, around 10% of nodes that report a minfeefilter are already using 0.1 sats/vB or less at this point, so it should improve rapidly.
💬 dergoegge commented on pull request "tor: enable PoW defenses for automatically created hidden services":
(https://github.com/bitcoin/bitcoin/pull/33414#issuecomment-3302853723)
Should we then also add PoW to the connections that we make to other nodes running behind hidden services?
(https://github.com/bitcoin/bitcoin/pull/33414#issuecomment-3302853723)
Should we then also add PoW to the connections that we make to other nodes running behind hidden services?
💬 ryanofsky commented on pull request "multiprocess: Don't require bitcoin -m argument when IPC options are used":
(https://github.com/bitcoin/bitcoin/pull/33229#issuecomment-3302921594)
Unfortunately this conflicted with #33407 due to cmake change on adjacent line, so needed to rebase.
<!-- begin push-25 -->
Rebased f9685d6a1389938b0cceb31d9eef201ab3dd2e86 -> 453b0fa286e5dce0af682b7b73684dd6415a50de ([`pr/ipc-auto.24`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-auto.24) -> [`pr/ipc-auto.25`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-auto.25), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ipc-auto.24-rebase..pr/ipc-auto.25))<!-- end --> due to
...
(https://github.com/bitcoin/bitcoin/pull/33229#issuecomment-3302921594)
Unfortunately this conflicted with #33407 due to cmake change on adjacent line, so needed to rebase.
<!-- begin push-25 -->
Rebased f9685d6a1389938b0cceb31d9eef201ab3dd2e86 -> 453b0fa286e5dce0af682b7b73684dd6415a50de ([`pr/ipc-auto.24`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-auto.24) -> [`pr/ipc-auto.25`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-auto.25), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ipc-auto.24-rebase..pr/ipc-auto.25))<!-- end --> due to
...
🤔 rkrux reviewed a pull request: "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys"
(https://github.com/bitcoin/bitcoin/pull/29675#pullrequestreview-3233829232)
More than three quarters of code review complete at 6400f7b82f4d2c22fb05f7122ce3a750938526f7
(https://github.com/bitcoin/bitcoin/pull/29675#pullrequestreview-3233829232)
More than three quarters of code review complete at 6400f7b82f4d2c22fb05f7122ce3a750938526f7
💬 rkrux commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2355125544)
In https://github.com/bitcoin/bitcoin/commit/1614a2da5ea32b3efeecf6c756036539c567b656 "sign: Create MuSig2 signatures for known MuSig2 aggregate keys"
The name of the function avoids the need for this comment.
```diff
diff --git a/src/script/sign.cpp b/src/script/sign.cpp
index b60d6dab55..a83cf82f19 100644
--- a/src/script/sign.cpp
+++ b/src/script/sign.cpp
@@ -316,7 +316,6 @@ static bool SignMuSig2(const BaseSignatureCreator& creator, SignatureData& sigda
plain_pub = twe
...
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2355125544)
In https://github.com/bitcoin/bitcoin/commit/1614a2da5ea32b3efeecf6c756036539c567b656 "sign: Create MuSig2 signatures for known MuSig2 aggregate keys"
The name of the function avoids the need for this comment.
```diff
diff --git a/src/script/sign.cpp b/src/script/sign.cpp
index b60d6dab55..a83cf82f19 100644
--- a/src/script/sign.cpp
+++ b/src/script/sign.cpp
@@ -316,7 +316,6 @@ static bool SignMuSig2(const BaseSignatureCreator& creator, SignatureData& sigda
plain_pub = twe
...
💬 rkrux commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2355100462)
In 1614a2da5ea32b3efeecf6c756036539c567b656 "sign: Create MuSig2 signatures for known MuSig2 aggregate keys"
Otherwise `part_info` acts as a dummy object.
```diff
diff --git a/src/script/sign.cpp b/src/script/sign.cpp
index b60d6dab55..f377a32750 100644
--- a/src/script/sign.cpp
+++ b/src/script/sign.cpp
@@ -277,7 +277,7 @@ static bool SignMuSig2(const BaseSignatureCreator& creator, SignatureData& sigda
XOnlyPubKey xonly_part(part_pk);
auto it = sigda
...
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2355100462)
In 1614a2da5ea32b3efeecf6c756036539c567b656 "sign: Create MuSig2 signatures for known MuSig2 aggregate keys"
Otherwise `part_info` acts as a dummy object.
```diff
diff --git a/src/script/sign.cpp b/src/script/sign.cpp
index b60d6dab55..f377a32750 100644
--- a/src/script/sign.cpp
+++ b/src/script/sign.cpp
@@ -277,7 +277,7 @@ static bool SignMuSig2(const BaseSignatureCreator& creator, SignatureData& sigda
XOnlyPubKey xonly_part(part_pk);
auto it = sigda
...
💬 rkrux commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2355117208)
In d53a70d769505b28c76d0468ea9503f7e00c1e04 "sign: Add CreateMuSig2PartialSig"
```diff
diff --git a/src/key.cpp b/src/key.cpp
index 29427689bb..a952acb260 100644
--- a/src/key.cpp
+++ b/src/key.cpp
@@ -401,7 +401,7 @@ std::optional<uint256> CKey::CreateMuSig2PartialSig(const uint256& sighash, cons
const auto& pn_it = pubnonces.find(part_pk);
if (pn_it == pubnonces.end()) return std::nullopt;
const std::vector<uint8_t> pubnonce = pn_it->second;
- if (
...
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2355117208)
In d53a70d769505b28c76d0468ea9503f7e00c1e04 "sign: Add CreateMuSig2PartialSig"
```diff
diff --git a/src/key.cpp b/src/key.cpp
index 29427689bb..a952acb260 100644
--- a/src/key.cpp
+++ b/src/key.cpp
@@ -401,7 +401,7 @@ std::optional<uint256> CKey::CreateMuSig2PartialSig(const uint256& sighash, cons
const auto& pn_it = pubnonces.find(part_pk);
if (pn_it == pubnonces.end()) return std::nullopt;
const std::vector<uint8_t> pubnonce = pn_it->second;
- if (
...
💬 rkrux commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2355118477)
In 09105c55e71d0614508c9111ed448ea1a370b38b "sign: Add CreateMuSig2AggregateSig"
```diff
diff --git a/src/musig.cpp b/src/musig.cpp
index c8dfe70ef9..686ec5e869 100644
--- a/src/musig.cpp
+++ b/src/musig.cpp
@@ -139,7 +139,7 @@ std::optional<std::vector<uint8_t>> CreateMuSig2AggregateSig(const std::vector<C
const auto& pn_it = pubnonces.find(part_pk);
if (pn_it == pubnonces.end()) return std::nullopt;
const std::vector<uint8_t> pubnonce = pn_it->second;
-
...
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2355118477)
In 09105c55e71d0614508c9111ed448ea1a370b38b "sign: Add CreateMuSig2AggregateSig"
```diff
diff --git a/src/musig.cpp b/src/musig.cpp
index c8dfe70ef9..686ec5e869 100644
--- a/src/musig.cpp
+++ b/src/musig.cpp
@@ -139,7 +139,7 @@ std::optional<std::vector<uint8_t>> CreateMuSig2AggregateSig(const std::vector<C
const auto& pn_it = pubnonces.find(part_pk);
if (pn_it == pubnonces.end()) return std::nullopt;
const std::vector<uint8_t> pubnonce = pn_it->second;
-
...
💬 rkrux commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2355537665)
In https://github.com/bitcoin/bitcoin/commit/1614a2da5ea32b3efeecf6c756036539c567b656 "sign: Create MuSig2 signatures for known MuSig2 aggregate keys"
Suggesting because I ended up spending some time to understand the need for this check.
```diff
@@ -285,6 +286,7 @@ static bool SignMuSig2(const BaseSignatureCreator& creator, SignatureData& sigda
std::vector<std::pair<uint256, bool>> tweaks;
CPubKey plain_pub = agg_pub;
+ // Check if we can derive the script p
...
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2355537665)
In https://github.com/bitcoin/bitcoin/commit/1614a2da5ea32b3efeecf6c756036539c567b656 "sign: Create MuSig2 signatures for known MuSig2 aggregate keys"
Suggesting because I ended up spending some time to understand the need for this check.
```diff
@@ -285,6 +286,7 @@ static bool SignMuSig2(const BaseSignatureCreator& creator, SignatureData& sigda
std::vector<std::pair<uint256, bool>> tweaks;
CPubKey plain_pub = agg_pub;
+ // Check if we can derive the script p
...
💬 rkrux commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2355503340)
In https://github.com/bitcoin/bitcoin/commit/1614a2da5ea32b3efeecf6c756036539c567b656 "sign: Create MuSig2 signatures for known MuSig2 aggregate keys"
Usually avoid suggesting this kind of change but in this case it seems quite helpful for readability.
```diff
diff --git a/src/script/sign.cpp b/src/script/sign.cpp
index b60d6dab55..de17219bd5 100644
--- a/src/script/sign.cpp
+++ b/src/script/sign.cpp
@@ -286,26 +286,26 @@ static bool SignMuSig2(const BaseSignatureCreator& creator, Sig
...
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2355503340)
In https://github.com/bitcoin/bitcoin/commit/1614a2da5ea32b3efeecf6c756036539c567b656 "sign: Create MuSig2 signatures for known MuSig2 aggregate keys"
Usually avoid suggesting this kind of change but in this case it seems quite helpful for readability.
```diff
diff --git a/src/script/sign.cpp b/src/script/sign.cpp
index b60d6dab55..de17219bd5 100644
--- a/src/script/sign.cpp
+++ b/src/script/sign.cpp
@@ -286,26 +286,26 @@ static bool SignMuSig2(const BaseSignatureCreator& creator, Sig
...
💬 rkrux commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2355561394)
In https://github.com/bitcoin/bitcoin/commit/1614a2da5ea32b3efeecf6c756036539c567b656 "sign: Create MuSig2 signatures for known MuSig2 aggregate keys"
There are 2 instances of returning false from this function (other on line 301) in which case it is expected that the signing process would throw an error at a later stage. Do you think adding an error log here would be helpful for debugging later?
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2355561394)
In https://github.com/bitcoin/bitcoin/commit/1614a2da5ea32b3efeecf6c756036539c567b656 "sign: Create MuSig2 signatures for known MuSig2 aggregate keys"
There are 2 instances of returning false from this function (other on line 301) in which case it is expected that the signing process would throw an error at a later stage. Do you think adding an error log here would be helpful for debugging later?
💬 rkrux commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2355476052)
In 1614a2da5ea32b3efeecf6c756036539c567b656 "sign: Create MuSig2 signatures for known MuSig2 aggregate keys"
<details open>
<summary>Can reorder to return or continue early on in the process.</summary>
```diff
diff --git a/src/script/sign.cpp b/src/script/sign.cpp
index b60d6dab55..eea6cd0a9f 100644
--- a/src/script/sign.cpp
+++ b/src/script/sign.cpp
@@ -270,19 +270,6 @@ static bool SignMuSig2(const BaseSignatureCreator& creator, SignatureData& sigda
for (const auto& [agg_pub,
...
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2355476052)
In 1614a2da5ea32b3efeecf6c756036539c567b656 "sign: Create MuSig2 signatures for known MuSig2 aggregate keys"
<details open>
<summary>Can reorder to return or continue early on in the process.</summary>
```diff
diff --git a/src/script/sign.cpp b/src/script/sign.cpp
index b60d6dab55..eea6cd0a9f 100644
--- a/src/script/sign.cpp
+++ b/src/script/sign.cpp
@@ -270,19 +270,6 @@ static bool SignMuSig2(const BaseSignatureCreator& creator, SignatureData& sigda
for (const auto& [agg_pub,
...