Don't use from_ed25519_bytes when making keypairs

[ameba23]

Currently we use the schnorrkel crate's SecretKey::from_ed25519_bytes when creating a sr25519 keypair from a given secret key.

According to schorrkel's documentation this method is to 'Construct an SecretKey from a slice of bytes, corresponding to an Ed25519 expanded secret key.'

I'm not sure why we are using this. I know substrate allows using ed25519, but to my knowledge we are using sr25519 for signature request accounts. In entropy-js we set the key type to sr25519 here: https://github.com/entropyxyz/entropy-js/blob/c4c21042929cc0072c7be297b743f212b3f12607/src/keys/index.ts#L6

I've added a test which shows that this gives a different keypair than if we use from_bytes. And when i have tried sending a UserTransactionRequest with this as it is, i get an error that the account is not registered.

I don't understand how this has been working for us at all. This means that we sign UserTransactionRequest message with effectively a different keypair to the one we use to sign register transactions. So it seems impossible that the entropy-js tests pass, but they do.

Does anyone have an idea why we are using ed25519 here, and why this seems to be working in the entropy-js tests?

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
x25519-chacha20poly1305	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 23, 2023 8:40am

[ameba23]

I think i get where this comes from. sp-core::sr25519::from::seed::slice does use ed25519 key expansion when creating a 64 byte secret key from a 32 byte seed: https://docs.rs/sp-core/latest/src/sp_core/sr25519.rs.html#466-480

But still, that is in my understanding not how we are using this. We pass it a 64 byte secret key.

[JesseAbram]

I dont see a problem with this, I think the issue with me here is that this should be a shared package between core and here, like the protocol package so we don't accidentally get our states out of wack

[ameba23]

I dont see a problem with this, I think the issue with me here is that this should be a shared package between core and here, like the protocol package so we don't accidentally get our states out of wack

I agree that it would be a good idea to do that, i think there is a card for it already. But its not possible:

For the CLI test client i have been playing around with i tried to use SignedMessage from here, but there are problems because this uses sp-core v6, so the AccountId type is different. We cant upgrade sp-core because the new versions don't work on wasm.

So i tried to use encrypt_and_sign from here, just as we do in entropy-js. Without the changes in this PR, we get a completely different signing keypair than we do from SignedMessage::new, as this test shows:

x25519-chacha20poly1305/src/lib.rs

Line 349 in 0bcdcdd

fn test_keypair_parsing() {

Since it is a different keypair, i kept getting an error that the user was not yet registered, because the keypair they used to sign the registration tx was different from that in the SignedMessage.

[ameba23]

Thanks @frankiebee for help figuring this out. I am closing this and have made a related issue: #15