💬 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.
💬 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
...