Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runtime-sdk: Add sr25519 support in EVM precompiles #2073

Merged
merged 3 commits into from
Dec 8, 2024

Conversation

kostko
Copy link
Member

@kostko kostko commented Nov 20, 2024

Fixes #1968

Copy link

netlify bot commented Nov 20, 2024

Deploy Preview for oasisprotocol-oasis-sdk canceled.

Name Link
🔨 Latest commit 7621921
🔍 Latest deploy log https://app.netlify.com/sites/oasisprotocol-oasis-sdk/deploys/6752d08487f5ec000844d190

@kostko kostko requested a review from CedarMist November 20, 2024 07:52
@kostko kostko force-pushed the kostko/feature/sr25519-sign branch 2 times, most recently from 087bccb to f8d82ba Compare November 20, 2024 08:40
runtime-sdk/src/crypto/signature/sr25519.rs Show resolved Hide resolved
@@ -60,6 +61,27 @@ impl PublicKey {
.map_err(|_| Error::VerificationFailed)
}

/// Verify a signature.
pub fn verify_raw(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not in line with other signers, where you have /// Verify signature without using any domain separation scheme., so optionally we could think about renaming this method.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This matches the polkadot implementation.

runtime-sdk/src/crypto/signature/sr25519.rs Outdated Show resolved Hide resolved
@kostko kostko force-pushed the kostko/feature/sr25519-sign branch from f8d82ba to 1a596b9 Compare November 20, 2024 10:26
@CedarMist
Copy link
Member

Constructing STROBE/Merlin transcript requires access to the underlying KeccakF[1600] permutation function, I'm wary that implementing sr25519 signing alone may not be enough if people end up reverting to Solidity to construct the message to be signed, or if the sr25519 signer makes assumption about the transcript which makes it impossible to use for other things.

@kostko
Copy link
Member Author

kostko commented Nov 20, 2024

We can always introduce additional signature modes with different transcripts if needed. This is the easiest way to add into the existing framework, it would be good to know if it is sufficient for the use cases at hand.

@CedarMist
Copy link
Member

Good news: current implementation can verify test case signature.

Bad news:

  1. Key derivation from seed doesn't match Polkadot implementation
  2. micro-sr25519 can't verify messages it signs with a keypair generated by on-chain implementation, neither can on-chain implementation verify a message signed by micro-sr25519 with keypair generated by on-chain implementation (basic round-trip tests)
    // Key derivation from polkadot test cases
    // See: https://github.com/polkadot-js/wasm/blob/10010830094e7d033bd11b16c5e3bc01a7045309/packages/wasm-crypto/src/rs/sr25519.rs#L176
    const secretSeed = getBytes("0xfac7959dbfe72f052e5a0c3c8d6530f202b02fd8f9f5ca3580ec8deb7797479e");
    const secretKey = sr25519.secretFromSeed(secretSeed);
    const publicKey = sr25519.getPublicKey(secretKey);
    expect(hexlify(publicKey)).eq("0x46ebddef8cd9bb167dc30878d7113b7e168e6f0646beffd77d69d39bad76b47a");

    // Known valid signature
    const msg = new TextEncoder().encode("<Bytes>message to sign</Bytes>");
    const sig = getBytes("0x48ce2c90e08651adfc8ecef84e916f6d1bb51ebebd16150ee12df247841a5437951ea0f9d632ca165e6ab391532e75e701be6a1caa88c8a6bcca3511f55b4183");
    const sigSigner = getBytes("0xf84d048da2ddae2d9d8fd6763f469566e8817a26114f39408de15547f6d47805");

    // Verify JS implementation matches polkadot test case signature
    const isValid = sr25519.verify(msg, sig, sigSigner);
    expect(isValid).eq(true);

    const CONTEXT = new TextEncoder().encode('substrate');

    // Verify on-chain implementation also works
    const result = await se.testVerify(6, sigSigner, CONTEXT, msg, sig);
    expect(result).eq(true);

    // Test key generation
    const generatedKey = await se.testKeygen(6, secretSeed);
    // 64 byte secret, appended with 32 byte public key
    expect(getBytes(generatedKey.secretKey).length).eq(96);
    expect(hexlify(getBytes(generatedKey.secretKey).slice(64))).eq(generatedKey.publicKey);

    // JS can verify on-chain signed message
    const onchainSigned = await se.testSign(6, generatedKey.secretKey, CONTEXT, msg);
    const jsVerify = sr25519.verify(msg, getBytes(onchainSigned), getBytes(generatedKey.publicKey));
    expect(jsVerify).eq(true);
    // And on-chain can verify on-chain signed message
    expect(await se.testVerify(6, generatedKey.publicKey, CONTEXT, msg, onchainSigned)).eq(true);

    // JS roundtrip with on-chain generated keypair
    const jsSigned = sr25519.sign(getBytes(generatedKey.secretKey).slice(0, 64), msg);
    expect(sr25519.verify(msg, jsSigned, getBytes(generatedKey.publicKey))).eq(false); // FAIL

    // on-chain verify JS signed message
    const onchainVerify = await se.testVerify(6, generatedKey.publicKey, CONTEXT, msg, jsSigned);
    expect(onchainVerify).eq(false); // FAIL

@kostko kostko force-pushed the kostko/feature/sr25519-sign branch from 1a596b9 to 2af15ac Compare December 6, 2024 08:13
@kostko
Copy link
Member Author

kostko commented Dec 6, 2024

The other thing seems to be that there are two ways to encode a keypair:

  • Keypair::to_bytes: Serialize Keypair to bytes.
  • Keypair::to_half_ed25519_bytes: Serialize Keypair to bytes with Ed25519 secret key format.

It looks like we are using to_bytes but the Polkadot libraries are using to_half_ed25519_bytes? We can change that as well.

@kostko kostko force-pushed the kostko/feature/sr25519-sign branch 4 times, most recently from 4b8a0fe to 7af2344 Compare December 6, 2024 09:59
Copy link
Member

@CedarMist CedarMist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This matches the Polkadot implementation and passes the tests I made in sapphire-contracts: oasisprotocol/sapphire-paratime#469

@kostko kostko merged commit f3c7873 into main Dec 8, 2024
26 checks passed
@kostko kostko deleted the kostko/feature/sr25519-sign branch December 8, 2024 10:19
github-actions bot added a commit that referenced this pull request Dec 8, 2024
github-actions bot added a commit that referenced this pull request Dec 8, 2024
github-actions bot added a commit to OasisUnofficial/oasis-sdk that referenced this pull request Dec 8, 2024
github-actions bot added a commit to OasisUnofficial/oasis-sdk that referenced this pull request Dec 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sr25519 precompile for EVM sdk
4 participants