💬 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.
💬 sipa commented on pull request "crypto: more `Span<std::byte>` modernization & follow-ups":
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1292897098)
I've dropped it.
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1292897098)
I've dropped it.
💬 sipa commented on pull request "crypto: more `Span<std::byte>` modernization & follow-ups":
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1292897136)
I've dropped it.
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1292897136)
I've dropped it.
💬 sipa commented on pull request "crypto: more `Span<std::byte>` modernization & follow-ups":
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1292897165)
Done.
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1292897165)
Done.
💬 sipa commented on pull request "crypto: more `Span<std::byte>` modernization & follow-ups":
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1292897357)
I have instead used different argument names (`in_bytes` and `out_bytes`) to avoid a collision with the member variable altogether.
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1292897357)
I have instead used different argument names (`in_bytes` and `out_bytes`) to avoid a collision with the member variable altogether.
💬 sipa commented on pull request "crypto: more `Span<std::byte>` modernization & follow-ups":
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1292897379)
Done.
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1292897379)
Done.
💬 sipa commented on pull request "crypto: more `Span<std::byte>` modernization & follow-ups":
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1292898612)
Indeed, I'm aware.
Just brainstorming at this point, but perhaps we don't want to actually add an overload for an STL class operator. Perhaps `std::ranges::equal` can be used instead.
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1292898612)
Indeed, I'm aware.
Just brainstorming at this point, but perhaps we don't want to actually add an overload for an STL class operator. Perhaps `std::ranges::equal` can be used instead.
💬 ajtowns commented on issue "meta: Isolated fuzzing of net processing":
(https://github.com/bitcoin/bitcoin/issues/27502#issuecomment-1676562940)
What is this going to fuzz exactly? The possible behaviour of an individual p2p message in isolation? The p2p protocol state machine for a single peer? Is this intended to be abstracted away from validation/mempool behaviours (ie, stubbing them out)?
(https://github.com/bitcoin/bitcoin/issues/27502#issuecomment-1676562940)
What is this going to fuzz exactly? The possible behaviour of an individual p2p message in isolation? The p2p protocol state machine for a single peer? Is this intended to be abstracted away from validation/mempool behaviours (ie, stubbing them out)?
💬 sipa commented on pull request "crypto: more `Span<std::byte>` modernization & follow-ups":
(https://github.com/bitcoin/bitcoin/pull/28100#issuecomment-1676582994)
Rebased after merge of #28008, and addresses feedback. I've dropped the `Crypt` -> `Encrypt` / `Decrypt` change as I felt the duplication that resulted wasn't worth it anymore.
(https://github.com/bitcoin/bitcoin/pull/28100#issuecomment-1676582994)
Rebased after merge of #28008, and addresses feedback. I've dropped the `Crypt` -> `Encrypt` / `Decrypt` change as I felt the duplication that resulted wasn't worth it anymore.
💬 ajtowns commented on pull request "[no merge, meta] refactor: net/net processing split":
(https://github.com/bitcoin/bitcoin/pull/28252#issuecomment-1676598108)
> I think we're just gonna disagree on what the right direction is
Isn't the point of opening this to discuss that and at least try to come to an agreement? Being immediately dismissed out of hand feels pretty frustrating.
> It's challenging right now because we are passing internal pointers around.
I don't really think that's at all true? Passing pointers to objects is a completely normal thing; and whether something is "internal" or not is just a matter of how you define the api -- i
...
(https://github.com/bitcoin/bitcoin/pull/28252#issuecomment-1676598108)
> I think we're just gonna disagree on what the right direction is
Isn't the point of opening this to discuss that and at least try to come to an agreement? Being immediately dismissed out of hand feels pretty frustrating.
> It's challenging right now because we are passing internal pointers around.
I don't really think that's at all true? Passing pointers to objects is a completely normal thing; and whether something is "internal" or not is just a matter of how you define the api -- i
...
💬 sipa commented on pull request "net: transport abstraction":
(https://github.com/bitcoin/bitcoin/pull/28165#discussion_r1292945676)
The `header` argument no longer exists (as the caller doesn't have any notion of headers anymore, that's local to the transport implementation). Instead there is an `m_header_to_send` variable, but it's reused across messages, so (repeated) reserving makes no sense.
(https://github.com/bitcoin/bitcoin/pull/28165#discussion_r1292945676)
The `header` argument no longer exists (as the caller doesn't have any notion of headers anymore, that's local to the transport implementation). Instead there is an `m_header_to_send` variable, but it's reused across messages, so (repeated) reserving makes no sense.
📝 stratospher opened a pull request: "crypto: BIP324 ciphersuite follow-up"
(https://github.com/bitcoin/bitcoin/pull/28267)
follow-up to #28008.
* move `dummy_tag` variable in FSChaCha20Poly1305 crypto_tests outside of the loop to be reused every time
* use easy to read `cipher.last()` in `AEADChaCha20Poly1305::Decrypt()`
* comment for initiator in `BIP324Cipher::Initialize()`
* systematically damage ciphertext with bit positions in bip324_tests
* use 4095 max bytes for `aad` in bip324 fuzz test
(https://github.com/bitcoin/bitcoin/pull/28267)
follow-up to #28008.
* move `dummy_tag` variable in FSChaCha20Poly1305 crypto_tests outside of the loop to be reused every time
* use easy to read `cipher.last()` in `AEADChaCha20Poly1305::Decrypt()`
* comment for initiator in `BIP324Cipher::Initialize()`
* systematically damage ciphertext with bit positions in bip324_tests
* use 4095 max bytes for `aad` in bip324 fuzz test
💬 stratospher commented on pull request "Add fuzz test for FSChaCha20Poly1305, AEADChacha20Poly1305":
(https://github.com/bitcoin/bitcoin/pull/28263#issuecomment-1676634167)
so split off the last commit into #28267 since they aren't related to the fuzz test. (included https://github.com/bitcoin/bitcoin/pull/28008#discussion_r1277460913 and https://github.com/bitcoin/bitcoin/pull/28008#discussion_r1283327643 there)
thinking about https://github.com/bitcoin/bitcoin/pull/28263#discussion_r1292869677. will address it soon.
(https://github.com/bitcoin/bitcoin/pull/28263#issuecomment-1676634167)
so split off the last commit into #28267 since they aren't related to the fuzz test. (included https://github.com/bitcoin/bitcoin/pull/28008#discussion_r1277460913 and https://github.com/bitcoin/bitcoin/pull/28008#discussion_r1283327643 there)
thinking about https://github.com/bitcoin/bitcoin/pull/28263#discussion_r1292869677. will address it soon.