feat(signature-validation): add ed25519 signature validation to twilight-util

[Monadic-Cat]

Implementing #2169. This PR is not complete, but I felt like making what I wrote just now public quickly so people know progress exists.

[Erk-]

It would be nice if we could use a digest API for the signature so we don't have to allocate a vec for it. A quick look makes it seem like it is currently not possible but it may be possible in version 2 of ed2519-dalek. Though it may need a pr such as the below one.

dalek-cryptography/ed25519-dalek#196

[Monadic-Cat]

It would be nice if we could use a digest API for the signature so we don't have to allocate a vec for it. A quick look makes it seem like it is currently not possible but it may be possible in version 2 of ed2519-dalek. Though it may need a pr such as the below one.

dalek-cryptography/ed25519-dalek#196

I agree, it'd be really nice to have that. So, I'm following up over there to see if we can get what we need for that added.

[Monadic-Cat]

Okay, I've been a little busy for a bit, but I'm going to have a solid few days soon I can dedicate to this.

While it would be nice to have that feature on ed25519-dalek, I believe that this isn't actually blocked on that, since merging such an improvement would be entirely internal here- it wouldn't need to break this API.

So with that in mind, my goal is going to be getting this in a working state and ready to merge here, and later on pushing the stuff over at ed25519-dalek to completion.

[Monadic-Cat]

Well, I never did get that time I was expecting to for this, but it turns out the remaining work is small. I've added extract_interaction(...), so all that remains should be documentation.

[Monadic-Cat]

Side note: For avoiding the extra allocation, it seems we are waiting on one of these two PRs to be merged over at curve25519-dalek:

• Sign/verify by digest update, StreamVerifier refactoring dalek-cryptography/curve25519-dalek#583
• Sign/verify by digest update, StreamVerifier refactoring dalek-cryptography/curve25519-dalek#556

[Erk-]

Should we get it merged and then have a issue tracking the version with less allocations in a issue?

[Monadic-Cat]

Should we get it merged and then have a issue tracking the version with less allocations in a issue?

Yeah, that'd be for the best. I'm going to do some housecleaning on this PR today, and then it should be ready for final review and merge.

• Rebase to fix Cargo.toml conflict with main branch
• Move to latest release of ed25519-dalek
• Document anything not yet documented
• Fix miscellaneous broken stuff, :V

[Monadic-Cat]

It's essentially done at this point, but I'd like to suggest doing additional feature gates to support someone using only check_signature() and not getting an unused serde_json dep pulled in.

[Monadic-Cat]

Rebased to fix conflict with updates to the main branch (in particular, in the examples directory) that appeared after I made this PR.

[Erk-]

I think it looks good, only a few small nits that I would like to have changed.

[Monadic-Cat]

Added the last doc comment required by your Clippy configuration. But I have no idea what's making these tests fail.

[Erk-]

Added the last doc comment required by your Clippy configuration. But I have no idea what's making these tests fail.

They fail because of the issue solved here: #2326 so you can just look away from those.

[randomairborne]

Shouldn't this have Debug derived on it? All the other error structs do. Ditto Display and Error.

[vilgotf]

There's a few formatting and documentation issues too that I've not commented on that I'll cover in the next round of reviews. Sorry for letting this linger.

[Monadic-Cat]

There's a few formatting and documentation issues too that I've not commented on that I'll cover in the next round of reviews. Sorry for letting this linger.

...Next round of reviews? I'd rather be able to churn through all your change requests in one session. Mind if I just wait until you're done?

[vilgotf]

...Next round of reviews? I'd rather be able to churn through all your change requests in one session. Mind if I just wait until you're done?

The next round will just be documentation and formatting pointers (see CONTRIBUTING.md), if we stick with my changes. Formatting doesn't matter that much until the API is finalized.

[Monadic-Cat]

I should state this publicly. I am no longer going to be working on this PR.

The maintainers are invited to edit and bikeshed it to completion. This PR has the "Allow edits by maintainers" option checked.

If no one does anything, I'm going to change gears and build a separate crate.

[Gelbpunkt]

I don't think this PR offers any measurable advantage for users over just using dalek directly, it rather only adds new code that needs maintaining. As it is, I'm against merging this PR. The only way in which I can see this being useful is if we were to add APIs for a webserver that accepts interactions, but I think that is out of the scope of twilight.

[Monadic-Cat]

Alright then.