-
Notifications
You must be signed in to change notification settings - Fork 32
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
Improve performance by using bcoin-org/bcrypto #29
Comments
Looks like you use the same test fixtures as they do: |
Looks like the vast majority of tests do pass with bcrypto. Only 18 fail, while 600 pass. src/schnorr.js const { schnorr: bcrypto } = require("bcrypto");
const check = require("./check");
function sign(privateKey, message) {
check.checkSignParams(privateKey, message);
if (typeof privateKey === "string") {
privateKey = Buffer.from(privateKey, "hex");
}
return bcrypto.sign(message, privateKey);
}
function verify(pubKey, message, signature) {
check.checkVerifyParams(pubKey, message, signature);
return bcrypto.verify(message, signature, pubKey);
}
function batchVerify(pubKeys, messages, signatures) {
check.checkBatchVerifyParams(pubKeys, messages, signatures);
const batch = [];
for (let i = 0; i < pubKeys.length; i++) {
batch.push([messages[i], signatures[i], pubKeys[i]]);
}
return bcrypto.verifyBatch(batch);
}
module.exports = {
sign,
verify,
batchVerify,
}; Tests
|
It looks like the library doesn't support BIP340 x-only public keys. At least that's what I think from the test failures. But I'm happy to review a PR if you're able to get the tests working. |
Looking at https://github.com/bcoin-org/bcrypto/blob/master/lib/native/schnorr-libsecp256k1.js and searching for |
Looks like it would be more involved than just swapping some methods because bcrypto seems to not be 100% compliant with BIP340. Will fork instead and make changes as needed for my use-case as I don't need the parts that cause tests to fail. Thanks for the solid base! |
Hey, have you considered using https://github.com/bcoin-org/bcrypto to replace the functions in https://github.com/guggero/bip-schnorr/blob/master/src/schnorr.js? I would have to check if it is 100% compatible with the way BIP340 is implemented here but if it is you could gain a massive performance boost through it, especially for verification.
Code
Result
The text was updated successfully, but these errors were encountered: