💬 josibake commented on pull request "wallet: Use CTxDestination in CRecipient instead of just scriptPubKey":
(https://github.com/bitcoin/bitcoin/pull/28246#discussion_r1292841564)
in https://github.com/bitcoin/bitcoin/pull/28246/commits/34d7ab05b3f30e2827d9358a212f25865a71cce8:
Looks like this is causing the fuzzer to fail in `src/test/fuzz/util.cpp` because this new type is not handled.
(https://github.com/bitcoin/bitcoin/pull/28246#discussion_r1292841564)
in https://github.com/bitcoin/bitcoin/pull/28246/commits/34d7ab05b3f30e2827d9358a212f25865a71cce8:
Looks like this is causing the fuzzer to fail in `src/test/fuzz/util.cpp` because this new type is not handled.
⚠️ achow101 opened an issue: "`-min` does not minimize wallet loading dialog"
(https://github.com/bitcoin-core/gui/issues/748)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
A user on the Bitcoin stackexchange [reported](https://bitcoin.stackexchange.com/questions/119283/is-it-possible-to-run-bitcoin-core-gui-at-the-same-time-as-bitcoind) that starting `-min` still results in the `Loading wallets` dialog box appearing.
### Expected behaviour
`-min` should minimize all startup dialogs.
### Steps to reproduce
Start with `-min` with wallets automatically load
...
(https://github.com/bitcoin-core/gui/issues/748)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
A user on the Bitcoin stackexchange [reported](https://bitcoin.stackexchange.com/questions/119283/is-it-possible-to-run-bitcoin-core-gui-at-the-same-time-as-bitcoind) that starting `-min` still results in the `Loading wallets` dialog box appearing.
### Expected behaviour
`-min` should minimize all startup dialogs.
### Steps to reproduce
Start with `-min` with wallets automatically load
...
📝 hebasto opened a pull request: "ci: Run "macOS 11.0 [gui, no tests] [jammy]" job on GitHub Actions"
(https://github.com/bitcoin/bitcoin/pull/28265)
A draft for now. PR description update and undrafting come shortly.
(https://github.com/bitcoin/bitcoin/pull/28265)
A draft for now. PR description update and undrafting come shortly.
💬 hebasto commented on pull request "ci: Move tidy to persistent worker":
(https://github.com/bitcoin/bitcoin/pull/28214#issuecomment-1676456625)
> I'll create a separate pull/thread for macOS-cross next week, so that it can be discussed.
For a proposal to move macOS-cross to GitHub Actions, please see https://github.com/bitcoin/bitcoin/pull/28265.
(https://github.com/bitcoin/bitcoin/pull/28214#issuecomment-1676456625)
> I'll create a separate pull/thread for macOS-cross next week, so that it can be discussed.
For a proposal to move macOS-cross to GitHub Actions, please see https://github.com/bitcoin/bitcoin/pull/28265.
👋 hebasto's pull request is ready for review: "ci: Run "macOS 11.0 [gui, no tests] [jammy]" job on GitHub Actions"
(https://github.com/bitcoin/bitcoin/pull/28265)
(https://github.com/bitcoin/bitcoin/pull/28265)
💬 sipa commented on pull request "BIP324 ciphersuite":
(https://github.com/bitcoin/bitcoin/pull/28008#discussion_r1292864586)
The (FS)ChaCha20Poly1305 cipher really supports up to $2^{64}-1$ bytes of AAD. It just so happens that we only use it for garbage, and the garbage is limited to 4095 bytes. We need some kind of limit anyway, but it's a fair point that for our purposes, 4095 would be a more natural limit.
(https://github.com/bitcoin/bitcoin/pull/28008#discussion_r1292864586)
The (FS)ChaCha20Poly1305 cipher really supports up to $2^{64}-1$ bytes of AAD. It just so happens that we only use it for garbage, and the garbage is limited to 4095 bytes. We need some kind of limit anyway, but it's a fair point that for our purposes, 4095 would be a more natural limit.
💬 sipa commented on pull request "BIP324 ciphersuite":
(https://github.com/bitcoin/bitcoin/pull/28008#discussion_r1292866052)
@achow101 We have a `zero_after_free_allocator`, which sounds like what you're describing. The `secure_allocator` goes a step further an allocates from a pool of non-swappable memory pages on supporting systems.
* The `secure_allocator`'s behavior does sound overkill for the BIP324 purpose, I feel, and it may be counterproductive even because non-swappable memory is limited, and we really want it for our high-quality RNG and wallet keys.
* The `zero_after_free_allocator`'s behavior is what w
...
(https://github.com/bitcoin/bitcoin/pull/28008#discussion_r1292866052)
@achow101 We have a `zero_after_free_allocator`, which sounds like what you're describing. The `secure_allocator` goes a step further an allocates from a pool of non-swappable memory pages on supporting systems.
* The `secure_allocator`'s behavior does sound overkill for the BIP324 purpose, I feel, and it may be counterproductive even because non-swappable memory is limited, and we really want it for our high-quality RNG and wallet keys.
* The `zero_after_free_allocator`'s behavior is what w
...
🤔 sipa reviewed a pull request: "Add fuzz test for FSChaCha20Poly1305, AEADChacha20Poly1305"
(https://github.com/bitcoin/bitcoin/pull/28263#pullrequestreview-1575907129)
Mostly comments on the fuzz tests. The last commit looks good to me (and maybe we want to split it off).
(https://github.com/bitcoin/bitcoin/pull/28263#pullrequestreview-1575907129)
Mostly comments on the fuzz tests. The last commit looks good to me (and maybe we want to split it off).
💬 sipa commented on pull request "Add fuzz test for FSChaCha20Poly1305, AEADChacha20Poly1305":
(https://github.com/bitcoin/bitcoin/pull/28263#discussion_r1292869936)
The same comment as I've given on the other test about mixing "exercise API" and "test correctness" fuzzing applies here: `ok` will never be true (except with negligible probability), and if it does, `plain` and `contents` won't match.
(https://github.com/bitcoin/bitcoin/pull/28263#discussion_r1292869936)
The same comment as I've given on the other test about mixing "exercise API" and "test correctness" fuzzing applies here: `ok` will never be true (except with negligible probability), and if it does, `plain` and `contents` won't match.
💬 sipa commented on pull request "Add fuzz test for FSChaCha20Poly1305, AEADChacha20Poly1305":
(https://github.com/bitcoin/bitcoin/pull/28263#discussion_r1292867394)
This will always construct an AAD of 512 bytes, if that much data is available in the fuzzer input. I think it's better to make the AAD size dynamic, and a function of the fuzzer input data too.
(https://github.com/bitcoin/bitcoin/pull/28263#discussion_r1292867394)
This will always construct an AAD of 512 bytes, if that much data is available in the fuzzer input. I think it's better to make the AAD size dynamic, and a function of the fuzzer input data too.
💬 sipa commented on pull request "Add fuzz test for FSChaCha20Poly1305, AEADChacha20Poly1305":
(https://github.com/bitcoin/bitcoin/pull/28263#discussion_r1292869677)
This will never trigger, as the AEAD advances between between the preceding Encrypt call and the Decrypt call here, so encryption and decryption will never use the same key. With distinct keys, it should be cryptographically impossible for *anyone* (let alone a fuzzer) to get a match (and if, with extremely low probability, ok=true is returned, it'll be accidental, and plaintext and decrypted result won't match).
There are two possible styles of fuzz tests we could write here, and this feels
...
(https://github.com/bitcoin/bitcoin/pull/28263#discussion_r1292869677)
This will never trigger, as the AEAD advances between between the preceding Encrypt call and the Decrypt call here, so encryption and decryption will never use the same key. With distinct keys, it should be cryptographically impossible for *anyone* (let alone a fuzzer) to get a match (and if, with extremely low probability, ok=true is returned, it'll be accidental, and plaintext and decrypted result won't match).
There are two possible styles of fuzz tests we could write here, and this feels
...
💬 sipa commented on pull request "Add fuzz test for FSChaCha20Poly1305, AEADChacha20Poly1305":
(https://github.com/bitcoin/bitcoin/pull/28263#discussion_r1292870252)
Nit: `.` at the end of the sentence so it's clear the lines are about different arguments.
Also perhaps say "if we are the initiator" or; every v2 P2P connection involves initiating somehow; the question is whether it's us or the remote side doing that.
(https://github.com/bitcoin/bitcoin/pull/28263#discussion_r1292870252)
Nit: `.` at the end of the sentence so it's clear the lines are about different arguments.
Also perhaps say "if we are the initiator" or; every v2 P2P connection involves initiating somehow; the question is whether it's us or the remote side doing that.
💬 sipa commented on pull request "Add fuzz test for FSChaCha20Poly1305, AEADChacha20Poly1305":
(https://github.com/bitcoin/bitcoin/pull/28263#issuecomment-1676490059)
@stratospher Want to also address https://github.com/bitcoin/bitcoin/pull/28008#discussion_r1277460913 and https://github.com/bitcoin/bitcoin/pull/28008#discussion_r1283327643 here?
(https://github.com/bitcoin/bitcoin/pull/28263#issuecomment-1676490059)
@stratospher Want to also address https://github.com/bitcoin/bitcoin/pull/28008#discussion_r1277460913 and https://github.com/bitcoin/bitcoin/pull/28008#discussion_r1283327643 here?
📝 furszy opened a pull request: "gui: make '-min' minimize wallet loading dialog"
(https://github.com/bitcoin/bitcoin/pull/28266)
Simple fix for #748.
When '-min' is enabled, no loading dialog should
be presented on screen during startup.
(https://github.com/bitcoin/bitcoin/pull/28266)
Simple fix for #748.
When '-min' is enabled, no loading dialog should
be presented on screen during startup.
💬 ariard commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1676497083)
@zkfrio Thanks for your thoughts and I'm staying available to explain any technical concepts laid out on a public communication channels such as the Bitcoin mailing list or IRC, and offer my time in a true FOSS spirit.
In matters of ad hominems and personal threats, I'm following the personal policy to process the ones coming from pseudonymous GH users (minding all the practice GH is following to bind users to a social persona) as "cheap talk", as there is no track records on your statements.
...
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1676497083)
@zkfrio Thanks for your thoughts and I'm staying available to explain any technical concepts laid out on a public communication channels such as the Bitcoin mailing list or IRC, and offer my time in a true FOSS spirit.
In matters of ad hominems and personal threats, I'm following the personal policy to process the ones coming from pseudonymous GH users (minding all the practice GH is following to bind users to a social persona) as "cheap talk", as there is no track records on your statements.
...
💬 furszy commented on pull request "gui: make '-min' minimize wallet loading dialog":
(https://github.com/bitcoin/bitcoin/pull/28266#issuecomment-1676497207)
meh, wrong repo.. closing.
(https://github.com/bitcoin/bitcoin/pull/28266#issuecomment-1676497207)
meh, wrong repo.. closing.
✅ furszy closed a pull request: "gui: make '-min' minimize wallet loading dialog"
(https://github.com/bitcoin/bitcoin/pull/28266)
(https://github.com/bitcoin/bitcoin/pull/28266)
📝 furszy opened a pull request: "gui: make '-min' minimize wallet loading dialog"
(https://github.com/bitcoin-core/gui/pull/749)
Simple fix for #748.
When '-min' is enabled, no loading dialog should
be presented on screen during startup.
(https://github.com/bitcoin-core/gui/pull/749)
Simple fix for #748.
When '-min' is enabled, no loading dialog should
be presented on screen during startup.
💬 sipa commented on pull request "crypto: more `Span<std::byte>` modernization & follow-ups":
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1292896983)
Done.
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1292896983)
Done.
💬 sipa commented on pull request "crypto: more `Span<std::byte>` modernization & follow-ups":
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1292897040)
Done in a separate commit.
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1292897040)
Done in a separate commit.