💬 achow101 commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2663965111)
> It might be worth trying to notarize the binaries (instead of only the GUI bundle). This [forum thread](https://forums.developer.apple.com/forums/thread/721117) suggests it can be done by temporarily zipping them and sending them Apple in that form.
The binaries can be notarized, but the notarization cannot be stapled, which is why I elected to not notarize them. This means that anyone who runs those binaries will be phoning home to apple, but I guess that will happen regardless of notariza
...
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2663965111)
> It might be worth trying to notarize the binaries (instead of only the GUI bundle). This [forum thread](https://forums.developer.apple.com/forums/thread/721117) suggests it can be done by temporarily zipping them and sending them Apple in that form.
The binaries can be notarized, but the notarization cannot be stapled, which is why I elected to not notarize them. This means that anyone who runs those binaries will be phoning home to apple, but I guess that will happen regardless of notariza
...
💬 achow101 commented on pull request "consensus: Remove checkpoints (take 2)":
(https://github.com/bitcoin/bitcoin/pull/31649#issuecomment-2663967294)
> rfm?
I think this should be merged after v29.x branch off so that it can be tested in master for several months before being released in v30.0
(https://github.com/bitcoin/bitcoin/pull/31649#issuecomment-2663967294)
> rfm?
I think this should be merged after v29.x branch off so that it can be tested in master for several months before being released in v30.0
🤔 zaidmstrr reviewed a pull request: "fuzz: split `coinselection` harness"
(https://github.com/bitcoin/bitcoin/pull/31870#pullrequestreview-2621909024)
Hey, I'm reviewing this PR, as the description says that we can test each target separately, (`coinselection_bnb`, `coinselection_knapsack`, `coinselection_srd`) which is fine, but for the combined test I can't find any fuzz target there like `coinselection`.
(https://github.com/bitcoin/bitcoin/pull/31870#pullrequestreview-2621909024)
Hey, I'm reviewing this PR, as the description says that we can test each target separately, (`coinselection_bnb`, `coinselection_knapsack`, `coinselection_srd`) which is fine, but for the combined test I can't find any fuzz target there like `coinselection`.
💬 Crypt-iQ commented on issue "compact block fingerprinting":
(https://github.com/bitcoin/bitcoin/issues/28272#issuecomment-2664024685)
I meant to reply but guess I never did. The point about blocks-only nodes is a good one. In the happy path, they aren't sent compact blocks since they never send `SENDCMPCT` to the peer again to set the high-bandwidth mode. But they can still be sent unsolicited compact blocks and because they can't reconstruct the blocks, it lets them be fingerprinted if anything is in their mempool. In most cases I don't think we can send `GETDATA` to fetch txns from blocksonly nodes since they never `INV` us
...
(https://github.com/bitcoin/bitcoin/issues/28272#issuecomment-2664024685)
I meant to reply but guess I never did. The point about blocks-only nodes is a good one. In the happy path, they aren't sent compact blocks since they never send `SENDCMPCT` to the peer again to set the high-bandwidth mode. But they can still be sent unsolicited compact blocks and because they can't reconstruct the blocks, it lets them be fingerprinted if anything is in their mempool. In most cases I don't think we can send `GETDATA` to fetch txns from blocksonly nodes since they never `INV` us
...
💬 brunoerg commented on pull request "fuzz: split `coinselection` harness":
(https://github.com/bitcoin/bitcoin/pull/31870#issuecomment-2664030173)
> Hey, I'm reviewing this PR, as the description says that we can test each target separately, (`coinselection_bnb`, `coinselection_knapsack`, `coinselection_srd`) which is fine, but for the combined test I can't find any fuzz target there like `coinselection`.
I just updated the description. I removed the `coinselection` target, so there are 3 ones.
(https://github.com/bitcoin/bitcoin/pull/31870#issuecomment-2664030173)
> Hey, I'm reviewing this PR, as the description says that we can test each target separately, (`coinselection_bnb`, `coinselection_knapsack`, `coinselection_srd`) which is fine, but for the combined test I can't find any fuzz target there like `coinselection`.
I just updated the description. I removed the `coinselection` target, so there are 3 ones.
💬 mahmoudeA commented on issue "Bitcoin Core-[test] : Text overflows during the sync process.":
(https://github.com/bitcoin-core/gui/issues/847#issuecomment-2664040590)
تم اصلاح المشكلھ
(https://github.com/bitcoin-core/gui/issues/847#issuecomment-2664040590)
تم اصلاح المشكلھ
💬 fjahr commented on pull request "psbt: MuSig2 Fields":
(https://github.com/bitcoin/bitcoin/pull/31247#issuecomment-2664050910)
I compared the test vectors here to those in the BIP and I couldn't find the ["Spend of a Taproot output where a key in a script is a MuSig2 Aggregate Pubkey - With participant pubkeys only"](https://github.com/bitcoin/bips/pull/1764/files#diff-1cfac4ca4a777bedf8e9f21117e9a9849652e513828314234e3f7f81db1aae64R234) case here.
e.g. this Base64 String
```
cHNidP8BAFICAAAAAZqLSlB5a5YAmQ9/4R36ALxw79KWBIr8hnGa8PsfqRk3AQAAAAD9////ARjd9QUAAAAAFgAUyRI+BujX8JZsXRzQ+TMALU63V80AAAAAAAEBKwDh9QUAAAAAIlE
...
(https://github.com/bitcoin/bitcoin/pull/31247#issuecomment-2664050910)
I compared the test vectors here to those in the BIP and I couldn't find the ["Spend of a Taproot output where a key in a script is a MuSig2 Aggregate Pubkey - With participant pubkeys only"](https://github.com/bitcoin/bips/pull/1764/files#diff-1cfac4ca4a777bedf8e9f21117e9a9849652e513828314234e3f7f81db1aae64R234) case here.
e.g. this Base64 String
```
cHNidP8BAFICAAAAAZqLSlB5a5YAmQ9/4R36ALxw79KWBIr8hnGa8PsfqRk3AQAAAAD9////ARjd9QUAAAAAFgAUyRI+BujX8JZsXRzQ+TMALU63V80AAAAAAAEBKwDh9QUAAAAAIlE
...
💬 achow101 commented on pull request "psbt: MuSig2 Fields":
(https://github.com/bitcoin/bitcoin/pull/31247#issuecomment-2664065518)
> Am I missing something or did this one get lost?
Looks like it got lost. Added it now.
(https://github.com/bitcoin/bitcoin/pull/31247#issuecomment-2664065518)
> Am I missing something or did this one get lost?
Looks like it got lost. Added it now.
💬 mlori commented on pull request "optimization: batch XOR operations 12% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2664077362)
## Benchmark Results for Bitcoin Core Optimization
Based on the guide: [https://gist.github.com/l0rinc/83d2bdfce378ad7396610095ceb7bed5](https://gist.github.com/l0rinc/83d2bdfce378ad7396610095ceb7bed5), I executed the following benchmark tests on the commits listed below, with **4 runs per commit**.
- **caa68f79c11e5c444977ce8dee8a43020b7b3c5a** (Optimization: XOR 64 bits together instead of byte-by-byte)
- **5acf12bafeb126f2190b3f401f95199e0eea90c9** (master)
I began this quest on Jan
...
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2664077362)
## Benchmark Results for Bitcoin Core Optimization
Based on the guide: [https://gist.github.com/l0rinc/83d2bdfce378ad7396610095ceb7bed5](https://gist.github.com/l0rinc/83d2bdfce378ad7396610095ceb7bed5), I executed the following benchmark tests on the commits listed below, with **4 runs per commit**.
- **caa68f79c11e5c444977ce8dee8a43020b7b3c5a** (Optimization: XOR 64 bits together instead of byte-by-byte)
- **5acf12bafeb126f2190b3f401f95199e0eea90c9** (master)
I began this quest on Jan
...
💬 1440000bytes commented on issue "Wallet passpharse":
(https://github.com/bitcoin/bitcoin/issues/31852#issuecomment-2664080542)
Now that I look back this feels like social engineering attempt or a troll.
Anyways if @InnDe you are able to access your wallet, please let us know.
(https://github.com/bitcoin/bitcoin/issues/31852#issuecomment-2664080542)
Now that I look back this feels like social engineering attempt or a troll.
Anyways if @InnDe you are able to access your wallet, please let us know.
💬 hodlinator commented on issue "qa: `wallet_importdescriptors` failure "TypeError: 'bool' object is not subscriptable"":
(https://github.com/bitcoin/bitcoin/issues/31881#issuecomment-2664092403)
> But... this test is not deterministic. If the scanning process runs slower, the "0" duration check will also fail. I'm not sure we should keep it.
Yeah, it seems like an overly harsh check to me. Curious what was the impulse behind #31768 @brunoerg?
(https://github.com/bitcoin/bitcoin/issues/31881#issuecomment-2664092403)
> But... this test is not deterministic. If the scanning process runs slower, the "0" duration check will also fail. I'm not sure we should keep it.
Yeah, it seems like an overly harsh check to me. Curious what was the impulse behind #31768 @brunoerg?
💬 fjahr commented on pull request "psbt: MuSig2 Fields":
(https://github.com/bitcoin/bitcoin/pull/31247#issuecomment-2664099485)
re-ACK debacd1f59641c1e9e268c20820f4c8f0fc583f7
Confirmed all the BIP373 test vectors are present in the PR.
(https://github.com/bitcoin/bitcoin/pull/31247#issuecomment-2664099485)
re-ACK debacd1f59641c1e9e268c20820f4c8f0fc583f7
Confirmed all the BIP373 test vectors are present in the PR.
💬 brunoerg commented on issue "qa: `wallet_importdescriptors` failure "TypeError: 'bool' object is not subscriptable"":
(https://github.com/bitcoin/bitcoin/issues/31881#issuecomment-2664104230)
> I haven't tried, but https://github.com/bitcoin/bitcoin/pull/31768#discussion_r1954481759 by @hodlinator may fix it
Yes, it fixes it.
> Yeah, it seems like an overly harsh check to me. Curious what was the impulse behind https://github.com/bitcoin/bitcoin/pull/31768 @brunoerg?
I've tried it on different setups and besides the duration being different as expected for every setup/run I tried I didn't get a case where it's 0. Anyway, I'm fine on removing it or addressing the @hodlinator's sugg
...
(https://github.com/bitcoin/bitcoin/issues/31881#issuecomment-2664104230)
> I haven't tried, but https://github.com/bitcoin/bitcoin/pull/31768#discussion_r1954481759 by @hodlinator may fix it
Yes, it fixes it.
> Yeah, it seems like an overly harsh check to me. Curious what was the impulse behind https://github.com/bitcoin/bitcoin/pull/31768 @brunoerg?
I've tried it on different setups and besides the duration being different as expected for every setup/run I tried I didn't get a case where it's 0. Anyway, I'm fine on removing it or addressing the @hodlinator's sugg
...
💬 hodlinator commented on issue "qa: `wallet_importdescriptors` failure "TypeError: 'bool' object is not subscriptable"":
(https://github.com/bitcoin/bitcoin/issues/31881#issuecomment-2664114454)
Is the added code protecting some other code? Or just documenting the interface of the RPC, perhaps based on surviving mutants (from mutation testing)? If it's something like the former, I think it should be fixed. If it's the latter, I would lean towards reverting unless people agree we should start testing every aspect of the RPC interface (gradually or not).
(https://github.com/bitcoin/bitcoin/issues/31881#issuecomment-2664114454)
Is the added code protecting some other code? Or just documenting the interface of the RPC, perhaps based on surviving mutants (from mutation testing)? If it's something like the former, I think it should be fixed. If it's the latter, I would lean towards reverting unless people agree we should start testing every aspect of the RPC interface (gradually or not).
📝 brunoerg opened a pull request: "test: remove scanning check on `wallet_importdescriptors`"
(https://github.com/bitcoin/bitcoin/pull/31893)
May cause intermittent issues. See: https://github.com/bitcoin/bitcoin/issues/31881
(https://github.com/bitcoin/bitcoin/pull/31893)
May cause intermittent issues. See: https://github.com/bitcoin/bitcoin/issues/31881
💬 brunoerg commented on issue "qa: `wallet_importdescriptors` failure "TypeError: 'bool' object is not subscriptable"":
(https://github.com/bitcoin/bitcoin/issues/31881#issuecomment-2664125319)
I just opened #31893 to remove it. It's better to avoid any possibility of intermittent issue.
> Is the added code protecting some other code? Or just documenting the interface of the RPC, perhaps based on surviving mutants (from mutation testing)? If it's something like the former, I think it should be fixed. If it's the latter, I would lean towards reverting unless people agree we should start testing every aspect of the RPC interface (gradually or in larger PR(s)).
It's just a test covera
...
(https://github.com/bitcoin/bitcoin/issues/31881#issuecomment-2664125319)
I just opened #31893 to remove it. It's better to avoid any possibility of intermittent issue.
> Is the added code protecting some other code? Or just documenting the interface of the RPC, perhaps based on surviving mutants (from mutation testing)? If it's something like the former, I think it should be fixed. If it's the latter, I would lean towards reverting unless people agree we should start testing every aspect of the RPC interface (gradually or in larger PR(s)).
It's just a test covera
...
👍 hodlinator approved a pull request: "doc: clarify loadwallet path loading for wallets"
(https://github.com/bitcoin/bitcoin/pull/30302#pullrequestreview-2621999617)
re-ACK 80e695e6f1edd1a6d9aaab0748a5b573c4492cb4
<details><summary>Tested, looks good.</summary>
```
₿ build/src/bitcoin-cli -regtest loadwallet
error code: -1
error message:
loadwallet "filename" ( load_on_startup )
Loads a wallet from a wallet file or directory.
Note that all wallet command-line options used when starting bitcoind will be
applied to the new wallet.
Arguments:
1. filename (string, required) The path to the wallet directory (absolute or relative to th
...
(https://github.com/bitcoin/bitcoin/pull/30302#pullrequestreview-2621999617)
re-ACK 80e695e6f1edd1a6d9aaab0748a5b573c4492cb4
<details><summary>Tested, looks good.</summary>
```
₿ build/src/bitcoin-cli -regtest loadwallet
error code: -1
error message:
loadwallet "filename" ( load_on_startup )
Loads a wallet from a wallet file or directory.
Note that all wallet command-line options used when starting bitcoind will be
applied to the new wallet.
Arguments:
1. filename (string, required) The path to the wallet directory (absolute or relative to th
...
💬 hodlinator commented on issue "qa: `wallet_importdescriptors` failure "TypeError: 'bool' object is not subscriptable"":
(https://github.com/bitcoin/bitcoin/issues/31881#issuecomment-2664135447)
> It's just a test coverage addition but not based on surviving mutants since it does not even have test coverage.
Ah, forgot that mutants are focused on modifying test code, not implementation.
(https://github.com/bitcoin/bitcoin/issues/31881#issuecomment-2664135447)
> It's just a test coverage addition but not based on surviving mutants since it does not even have test coverage.
Ah, forgot that mutants are focused on modifying test code, not implementation.
💬 brunoerg commented on issue "qa: `wallet_importdescriptors` failure "TypeError: 'bool' object is not subscriptable"":
(https://github.com/bitcoin/bitcoin/issues/31881#issuecomment-2664137413)
> Ah, forgot that mutants are focused on modifying test code, not implementation.
No, mutants are focused on mutating the implementation to verify the tests. However, if there is no test coverage for the implementation, it does not make sense to modify it, there is nothing to verify.
(https://github.com/bitcoin/bitcoin/issues/31881#issuecomment-2664137413)
> Ah, forgot that mutants are focused on modifying test code, not implementation.
No, mutants are focused on mutating the implementation to verify the tests. However, if there is no test coverage for the implementation, it does not make sense to modify it, there is nothing to verify.
👍 theStack approved a pull request: "psbt: MuSig2 Fields"
(https://github.com/bitcoin/bitcoin/pull/31247#pullrequestreview-2622017122)
re-ACK debacd1f59641c1e9e268c20820f4c8f0fc583f7
Only a [missing valid test vector](https://github.com/bitcoin/bitcoin/pull/31247#issuecomment-2664050910) has been added since my previous ACK.
(https://github.com/bitcoin/bitcoin/pull/31247#pullrequestreview-2622017122)
re-ACK debacd1f59641c1e9e268c20820f4c8f0fc583f7
Only a [missing valid test vector](https://github.com/bitcoin/bitcoin/pull/31247#issuecomment-2664050910) has been added since my previous ACK.