👍 brunoerg approved a pull request: "wallet: warn against accidental unsafe older() import"
(https://github.com/bitcoin/bitcoin/pull/33135#pullrequestreview-3206472463)
reACK 5e9737ff493a96566f4fc11b1733b4b5a5393f6b
(https://github.com/bitcoin/bitcoin/pull/33135#pullrequestreview-3206472463)
reACK 5e9737ff493a96566f4fc11b1733b4b5a5393f6b
🤔 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-3206223831)
Partial Code review 2 - a06017dfce7ce72afbebe6f68d9a29cf72d26593
`wallet_musig.py` & `descriptor_tests.cpp` tests pass with these suggestions.
(https://github.com/bitcoin/bitcoin/pull/29675#pullrequestreview-3206223831)
Partial Code review 2 - a06017dfce7ce72afbebe6f68d9a29cf72d26593
`wallet_musig.py` & `descriptor_tests.cpp` tests pass with these suggestions.
💬 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_r2336962962)
In 7c085554dce336eb1597ab2fc482163876a49270 "sign: Create MuSig2 signatures for known MuSig2 aggregate keys"
Easy candidate to deduplicate and improve readability.
```diff
diff --git a/src/musig.cpp b/src/musig.cpp
index 28de6dc819..d5da34feb7 100644
--- a/src/musig.cpp
+++ b/src/musig.cpp
@@ -61,6 +61,16 @@ std::optional<CPubKey> MuSig2AggregatePubkeys(const std::vector<CPubKey>& pubkey
return MuSig2AggregatePubkeys(pubkeys, keyagg_cache, std::nullopt);
}
+CExtPubKey Crea
...
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2336962962)
In 7c085554dce336eb1597ab2fc482163876a49270 "sign: Create MuSig2 signatures for known MuSig2 aggregate keys"
Easy candidate to deduplicate and improve readability.
```diff
diff --git a/src/musig.cpp b/src/musig.cpp
index 28de6dc819..d5da34feb7 100644
--- a/src/musig.cpp
+++ b/src/musig.cpp
@@ -61,6 +61,16 @@ std::optional<CPubKey> MuSig2AggregatePubkeys(const std::vector<CPubKey>& pubkey
return MuSig2AggregatePubkeys(pubkeys, keyagg_cache, std::nullopt);
}
+CExtPubKey Crea
...
💬 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_r2336873078)
In 6b01d7379b9e29760abd7b549a04db43937a7b8d "sign: Add CreateMuSig2Nonce"
In 10b7148efa754169cc625e93c98fa54f7b375e5d "sign: Add CreateMuSig2PartialSig"
To avoid reading these very long types multiple times, also to avoid duplication.
An alternative is to have these 3 objects in a struct that could be used in psbt.h and sign.h, but that'd increase the diff.
```diff
diff --git a/src/musig.h b/src/musig.h
index 95f495a40a..020fdc09c3 100644
--- a/src/musig.h
+++ b/src/musig.h
@@ -14,6
...
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2336873078)
In 6b01d7379b9e29760abd7b549a04db43937a7b8d "sign: Add CreateMuSig2Nonce"
In 10b7148efa754169cc625e93c98fa54f7b375e5d "sign: Add CreateMuSig2PartialSig"
To avoid reading these very long types multiple times, also to avoid duplication.
An alternative is to have these 3 objects in a struct that could be used in psbt.h and sign.h, but that'd increase the diff.
```diff
diff --git a/src/musig.h b/src/musig.h
index 95f495a40a..020fdc09c3 100644
--- a/src/musig.h
+++ b/src/musig.h
@@ -14,6
...
💬 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_r2336806292)
In 6b01d7379b9e29760abd7b549a04db43937a7b8d "sign: Add CreateMuSig2Nonce"
Nit: Reordering to defer getting the private key.
```diff
diff --git a/src/script/sign.cpp b/src/script/sign.cpp
index b183be8939..d208f44626 100644
--- a/src/script/sign.cpp
+++ b/src/script/sign.cpp
@@ -106,10 +106,6 @@ std::vector<uint8_t> MutableTransactionSignatureCreator::CreateMuSig2Nonce(const
{
assert(sigversion == SigVersion::TAPROOT || sigversion == SigVersion::TAPSCRIPT);
- // Retrieve
...
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2336806292)
In 6b01d7379b9e29760abd7b549a04db43937a7b8d "sign: Add CreateMuSig2Nonce"
Nit: Reordering to defer getting the private key.
```diff
diff --git a/src/script/sign.cpp b/src/script/sign.cpp
index b183be8939..d208f44626 100644
--- a/src/script/sign.cpp
+++ b/src/script/sign.cpp
@@ -106,10 +106,6 @@ std::vector<uint8_t> MutableTransactionSignatureCreator::CreateMuSig2Nonce(const
{
assert(sigversion == SigVersion::TAPROOT || sigversion == SigVersion::TAPSCRIPT);
- // Retrieve
...
💬 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_r2337004847)
It was mostly to put the related code in a block, not for reusability; calling the inline sub-functions is an alternate imho.
```python
def do_test(self, ...):
self.log.info(f"Testing {comment}")
def prepare_musig_wallets_for_spending(...):
...
def spend_from_musig_address(...):
...
musig_spending_data = prepare_musig_wallets_for_spending()
spend_from_musig_address(musig_spending_data)
```
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2337004847)
It was mostly to put the related code in a block, not for reusability; calling the inline sub-functions is an alternate imho.
```python
def do_test(self, ...):
self.log.info(f"Testing {comment}")
def prepare_musig_wallets_for_spending(...):
...
def spend_from_musig_address(...):
...
musig_spending_data = prepare_musig_wallets_for_spending()
spend_from_musig_address(musig_spending_data)
```
⚠️ Zws758857 opened an issue: "把虚拟的“挖矿”货币应用到真实的实体“挖有机矿”,采用IoT设备记录真实的生产数据构成NFT,实现可追溯,不可簒改,去中心化的区块链Pi公链分布式"
(https://github.com/bitcoin/bitcoin/issues/33357)
### Please describe the feature you'd like to see added.
[常见的] 服务器地址 = 47.238.221.199 服务器端口 = 5443 代币 = Nndu0WLR74D6daAF303zwstls_启用 = true[ 3389 ] 类型 = TCP 本地IP = 127.0.0.1 本地端口 = 3389 远程端口 = 3389[ 31400 ] 类型 = TCP 本地IP = 127.0.0.1 本地端口 = 31400 远程端口 = 31400 [ 31401 ] 类型 = TCP 本地IP = 127.0.0.1 本地端口 = 31401 远程端口 = 31401 [ 31402 ] 类型 = TCP 本地IP = 127.0.0.1 本地端口 = 31402 远程端口 = 31402 [ 31403 ] 类型 = TCP 本地IP = 127.0.0.1 本地端口 = 31403 远程端口 = 31403 [ 31404 ] 类型 = TCP 本地IP = 127.0.0.1 本地端口 = 31404 远程端口
...
(https://github.com/bitcoin/bitcoin/issues/33357)
### Please describe the feature you'd like to see added.
[常见的] 服务器地址 = 47.238.221.199 服务器端口 = 5443 代币 = Nndu0WLR74D6daAF303zwstls_启用 = true[ 3389 ] 类型 = TCP 本地IP = 127.0.0.1 本地端口 = 3389 远程端口 = 3389[ 31400 ] 类型 = TCP 本地IP = 127.0.0.1 本地端口 = 31400 远程端口 = 31400 [ 31401 ] 类型 = TCP 本地IP = 127.0.0.1 本地端口 = 31401 远程端口 = 31401 [ 31402 ] 类型 = TCP 本地IP = 127.0.0.1 本地端口 = 31402 远程端口 = 31402 [ 31403 ] 类型 = TCP 本地IP = 127.0.0.1 本地端口 = 31403 远程端口 = 31403 [ 31404 ] 类型 = TCP 本地IP = 127.0.0.1 本地端口 = 31404 远程端口
...
✅ fanquake closed an issue: "把虚拟的“挖矿”货币应用到真实的实体“挖有机矿”,采用IoT设备记录真实的生产数据构成NFT,实现可追溯,不可簒改,去中心化的区块链Pi公链分布式"
(https://github.com/bitcoin/bitcoin/issues/33357)
(https://github.com/bitcoin/bitcoin/issues/33357)
💬 glozow commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2337076285)
This should be placed earlier, i.e. before `CheckEphemeralSpends`, to bound the number of mempool anestors before we start looking up whether they have dust to sweep.
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2337076285)
This should be placed earlier, i.e. before `CheckEphemeralSpends`, to bound the number of mempool anestors before we start looking up whether they have dust to sweep.
💬 mzumsande commented on pull request "help: enrich help text for `-loadblock`":
(https://github.com/bitcoin/bitcoin/pull/33343#issuecomment-3275391542)
> I wrote that before I found the `contrib/linearize` tool, which seems to be designed for unobfuscated block sharing anyway
It was originally designed to to something else (create a linear, in order chain), so using the tool has side effects (maybe the users doesn't want to lose historical forks for some reason). Could mention it as an example though.
I'm not convinced that changing the interface to support adding a xor-key per `-loadblock` arg (or one for all?) is really worth the effort
...
(https://github.com/bitcoin/bitcoin/pull/33343#issuecomment-3275391542)
> I wrote that before I found the `contrib/linearize` tool, which seems to be designed for unobfuscated block sharing anyway
It was originally designed to to something else (create a linear, in order chain), so using the tool has side effects (maybe the users doesn't want to lose historical forks for some reason). Could mention it as an example though.
I'm not convinced that changing the interface to support adding a xor-key per `-loadblock` arg (or one for all?) is really worth the effort
...
💬 rkrux commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-3275442471)
In bff8e053f4e6de3ce39e87a4faea161b1b84d850 "pubkey: Return tweaks from BIP32 derivation"
It would be helpful to mention why this is done in the commit message - specifically to mention around its usage in the "[sign: Create MuSig2 signatures for known MuSig2 aggregate keys](https://github.com/bitcoin/bitcoin/pull/29675/commits/7c085554dce336eb1597ab2fc482163876a49270)" commit pertaining to BIP 328/327.
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-3275442471)
In bff8e053f4e6de3ce39e87a4faea161b1b84d850 "pubkey: Return tweaks from BIP32 derivation"
It would be helpful to mention why this is done in the commit message - specifically to mention around its usage in the "[sign: Create MuSig2 signatures for known MuSig2 aggregate keys](https://github.com/bitcoin/bitcoin/pull/29675/commits/7c085554dce336eb1597ab2fc482163876a49270)" commit pertaining to BIP 328/327.
🤔 mzumsande reviewed a pull request: "test: Fix CLI_MAX_ARG_SIZE issues"
(https://github.com/bitcoin/bitcoin/pull/33243#pullrequestreview-3206763025)
Code Review ACK fa96a4afea2a9bf90c843198e75a00acef02c32d
(https://github.com/bitcoin/bitcoin/pull/33243#pullrequestreview-3206763025)
Code Review ACK fa96a4afea2a9bf90c843198e75a00acef02c32d
💬 glozow commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2337280911)
Is it problematic that there is no limit?
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2337280911)
Is it problematic that there is no limit?
📝 amishhaa opened a pull request: "contrib: fix for macOS deployment build failing on Qt translations even though it is optional."
(https://github.com/bitcoin/bitcoin/pull/33358)
From what I deciphered reading the line https://github.com/bitcoin/bitcoin/blob/master/contrib/macdeploy/macdeployqtplus#L390 is that qt translations are optional to have hence we should be able to build without it but the case where the flag translations_dir falls back to its default Null value it raises this error.
I have moved the code which adds language files under the if statement that first checks if the value of the flag is not Null before referencing it.
This PR assumes that having Q
...
(https://github.com/bitcoin/bitcoin/pull/33358)
From what I deciphered reading the line https://github.com/bitcoin/bitcoin/blob/master/contrib/macdeploy/macdeployqtplus#L390 is that qt translations are optional to have hence we should be able to build without it but the case where the flag translations_dir falls back to its default Null value it raises this error.
I have moved the code which adds language files under the if statement that first checks if the value of the flag is not Null before referencing it.
This PR assumes that having Q
...
👍 theStack approved a pull request: "test/refactor: use test deque to avoid quadratic iteration"
(https://github.com/bitcoin/bitcoin/pull/33313#pullrequestreview-3206983200)
ACK 8c1f10233dc9a843147772c40973031f5bfdbb7c
While I agree with others than very likely there won't be a notable difference in run-time any time soon (if ever), it seems conceptually the right approach.
(https://github.com/bitcoin/bitcoin/pull/33313#pullrequestreview-3206983200)
ACK 8c1f10233dc9a843147772c40973031f5bfdbb7c
While I agree with others than very likely there won't be a notable difference in run-time any time soon (if ever), it seems conceptually the right approach.
💬 l0rinc commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#issuecomment-3276022733)
Not pruned, but did many reindex-chainstates (it's a rpi4b benchmarking server), maybe a few `reindex`es as well. I don't know how to reliably reproduce it, but it always happen on that server with the given state. I will try to copy the data over when the other servers free up.
(https://github.com/bitcoin/bitcoin/pull/31615#issuecomment-3276022733)
Not pruned, but did many reindex-chainstates (it's a rpi4b benchmarking server), maybe a few `reindex`es as well. I don't know how to reliably reproduce it, but it always happen on that server with the given state. I will try to copy the data over when the other servers free up.
💬 l0rinc commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2337572396)
Excellent, added you as a coathor, thanks.
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2337572396)
Excellent, added you as a coathor, thanks.
💬 instagibbs commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2337582862)
not just the number, but also size as a proxy for number of parent outputs which are possibly scanned
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2337582862)
not just the number, but also size as a proxy for number of parent outputs which are possibly scanned
👍 hodlinator approved a pull request: "log: always print initial signature verification state"
(https://github.com/bitcoin/bitcoin/pull/33336#pullrequestreview-3207641440)
ACK 8cb75ee0a63880b55ac695fe884748aa7d604c38
While it is unsettling that we don't fully understand what causes script checking to occur prior to the assumevalid height (https://github.com/bitcoin/bitcoin/pull/32975#discussion_r2329269037) - having an initial "`Enabling signature validations at block #1 ...`" in `-assumevalid=0` cases as per this PR seems fine regardless.
(Empty `std::optional` does not match `bool fScriptChecks` until set).
https://github.com/bitcoin/bitcoin/blob/8cb75e
...
(https://github.com/bitcoin/bitcoin/pull/33336#pullrequestreview-3207641440)
ACK 8cb75ee0a63880b55ac695fe884748aa7d604c38
While it is unsettling that we don't fully understand what causes script checking to occur prior to the assumevalid height (https://github.com/bitcoin/bitcoin/pull/32975#discussion_r2329269037) - having an initial "`Enabling signature validations at block #1 ...`" in `-assumevalid=0` cases as per this PR seems fine regardless.
(Empty `std::optional` does not match `bool fScriptChecks` until set).
https://github.com/bitcoin/bitcoin/blob/8cb75e
...
💬 hodlinator commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2337774261)
nit: In case we are ever able to send away all blocks before detecting the expected log message, we could also wait for the block height before stopping the search of the log:
```suggestion
self.wait_until(lambda: self.nodes[0].getblockcount() >= COINBASE_MATURITY + 1)
```
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2337774261)
nit: In case we are ever able to send away all blocks before detecting the expected log message, we could also wait for the block height before stopping the search of the log:
```suggestion
self.wait_until(lambda: self.nodes[0].getblockcount() >= COINBASE_MATURITY + 1)
```