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

Signing and verifying a message using ECDSA sometimes fails #1898

Open
4 of 10 tasks
rjwebb opened this issue Nov 22, 2023 · 3 comments · May be fixed by #1973
Open
4 of 10 tasks

Signing and verifying a message using ECDSA sometimes fails #1898

rjwebb opened this issue Nov 22, 2023 · 3 comments · May be fixed by #1973
Labels
Bug Tracks issues causing errors or unintended behavior, critical to fix for reliability. Help Wanted Tasks open for external contributions, suitable for independent or collaborative work. P1 - High Essential for progress, blocks other tasks. Must be completed soon to maintain project flow.

Comments

@rjwebb
Copy link

rjwebb commented Nov 22, 2023

  • I'm submitting a ...

    • Bug report
    • Feature request
    • Support request
    • Other
  • What is the current behavior and expected behavior?

Expected behaviour: I should be able to generate a KeyringPair that signs messages using the ECDSA algorithm and verify the signatures against the message + the KeyringPair's public key

Current behaviour: This works most of the time, approx 99/100 times, but sometimes the verification fails.

  • What is the motivation for changing the behavior?

I'm working on a project that relies on being able to sign and verify data from the KeyringPair or from Polkadot wallets so I want to be able to rely on this functionality.

  • Please tell us about your environment:

I'm using @polkadot/keyring, @polkadot/util and @polkadot/util-crypto at the latest version as of writing (12.6.1).

I've created a minimal repository here that reproduces the bug: https://github.com/rjwebb/polkadot-ecdsa-bug-reproduction

  • Version: 12.6.1

  • Environment: Node v18.17.1

    • Node.js
    • Browser
    • Other (limited support for other environments)
  • Language:

    • JavaScript
    • TypeScript (include tsc --version) 5.2.2
    • Other
@TarikGul TarikGul added Bug Tracks issues causing errors or unintended behavior, critical to fix for reliability. P1 - High Essential for progress, blocks other tasks. Must be completed soon to maintain project flow. labels Oct 28, 2024
@TarikGul
Copy link
Member

Thanks for the reproduction, I was able to reproduce it. Added to the project board as High, and putting a Help wanted tag on it to hopefully garner a little bit of attention. If bandwidth frees up I will investigate this further.

@TarikGul TarikGul added the Help Wanted Tasks open for external contributions, suitable for independent or collaborative work. label Oct 28, 2024
@valentinfernandez1
Copy link
Contributor

valentinfernandez1 commented Jan 30, 2025

I noticed that signatureVerify() occasionally identifies the signature as ed25519 or sr25519 instead of ecdsa. This results in random verification failures, even though the signing process works as expected.

const signAndVerify = (keyringPair: KeyringPair, message, publicKey) => {
  const encodedMessage = stringToU8a(message);
  const signature = keyringPair.sign(encodedMessage);

  const result = signatureVerify(encodedMessage, signature, publicKey);
  // -- Log the signature type that was used for verification --
  console.log(result.crypto);
  return result.isValid;
};

const main = async () => {
  const mnemonic = mnemonicGenerate();

  console.log(`generating a keypair with mnemonic: "${mnemonic}"`);
  const keyring = new Keyring({
    type: "ecdsa",
  });
  const keyringpair = keyring.addFromMnemonic(mnemonic);
  const publicKey = keyringpair.publicKey;

  // try signing a load of strings and verifying the signature
  for (let i = 0; i < 1000; i++) {
    const message = `message ${i}`;
    console.log(`signing "${message}"`);

    if (!signAndVerify(keyringpair, message, publicKey)) {
      throw new Error(`signature verification failed for message ${i}`);
    }
  }

  console.log("all signatures verified successfully!");
};

main();

The output was

signing "message 103"
ecdsa
signing "message 104"
ecdsa
signing "message 105"
ecdsa
signing "message 106"
ed25519
/Development/pjs-testing/dist/index.js:30
            throw new Error(`signature verification failed for message ${i}`);
                  ^

Error: signature verification failed for message 106

As shown, the first 105 signatures were correctly detected as ecdsa, but for message 106, the verification unexpectedly switched to ed25519, causing the failure.

@valentinfernandez1
Copy link
Contributor

Addressed by #1973

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Tracks issues causing errors or unintended behavior, critical to fix for reliability. Help Wanted Tasks open for external contributions, suitable for independent or collaborative work. P1 - High Essential for progress, blocks other tasks. Must be completed soon to maintain project flow.
Projects
Development

Successfully merging a pull request may close this issue.

3 participants