jedisct1 / rust-ed25519-compact

Small, wasm-friendly, zero-dependencies Ed25519 and X25519 implementation for Rust.
MIT License
121 stars 22 forks source link

Possible bug in master/src/ed25519.rs#L241 #31

Closed dima-brook closed 1 year ago

dima-brook commented 1 year ago

Hi,

There seems to be a bug in this line that causes the if-else clause to always resolve in the else option: https://github.com/jedisct1/rust-ed25519-compact/blob/master/src/ed25519.rs#L241

Please, look into it. Transactions that require signatures fail because of this error.

Calling this smart contract function: https://github.com/XP-NETWORK/xp-casper-integration/blob/production/src/main.rs#L179-L209

Getting this error: https://testnet.cspr.live/deploy/f5b081d9eaa2a860ea1b593ce65fdd85153539cfbefebe9cedfd0c8386b67680

jedisct1 commented 1 year ago

An error is returned if the signature is not valid.

All the tests are passing, and this crate is used by quite a few applications so it's unlikely that the line you refer to always return an error when the signature is valid.

There may be a bug in your application or in the way the crate is used.

Here's an equivalent of the test_ed25519 test from that crate, in Zig, that has much cleaner Ed25519 implementation:

const std = @import("std");

pub fn main() !void {
    const kp = try std.crypto.sign.Ed25519.KeyPair.create([_]u8{42} ** 32);
    const msg = "Hello, World!";
    const sig = try kp.sign(msg, null);
    std.debug.print("{any}\n", .{sig});
    try sig.verify(msg, kp.public_key);
}

The computed signature is exactly the same, so they verify the same thing, and verification passes in the Rust crate, too.

While I'd be happy to fix bugs in that code, I unfortunately can't do it in other people's code especially code that I'm not familiar with and don't use.