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

Move ./lib/runtime/sig_verifier.go to ./lib/crypto package #1930

Closed
EclesioMeloJunior opened this issue Oct 27, 2021 · 4 comments · Fixed by #2057
Closed

Move ./lib/runtime/sig_verifier.go to ./lib/crypto package #1930

EclesioMeloJunior opened this issue Oct 27, 2021 · 4 comments · Fixed by #2057

Comments

@EclesioMeloJunior
Copy link
Member

Issue summary

  • The file ./lib/runtime/sig_verifier.go placed at the runtime package does not depend on runtime stuff, and it just acts like a worker that verifies batch signatures in a concurrently way.
  • We need to move the file to ./lib/crypto package as make more sense to that file be there.

Other information and links

  • N/A
@kevinliao852
Copy link

Can I take this

@EclesioMeloJunior
Copy link
Member Author

Can I take this

Go for it!

@kevinliao852
Copy link

@EclesioMeloJunior
Hi, I found that if I move sig_verifier.go to ./lib/crypto directory, it would have an importing cycle issue. I think it is because there are two packages, ed25519 and ed25519, which are the dependencies of sig_verifier.go.
Do you have any suggestions for dealing with it?

@EclesioMeloJunior
Copy link
Member Author

@kevinliao852, sorry for the delay!
ok, yeah, there is one way to avoid the cycle importing.
I would suggest doing:

  1. Create a new type called SigVerifyFunc of func(pubkey, sig, msg []byte) (bool, error)
type SigVerifyFunc func(pubkey, sig, msg []byte) (bool, error)
  1. Change the Signature field KeyTypeID crypto.KeyType to VerifyFunc SigVerifyFunc
  2. For each lib/crypto/ed25519, lib/crypto/sr25519, lib/crypto/secp256k1 package create a function called BatchSignVerify that satisfy the signature of the type SigVerifyFunc we created in step 1. In other words each of these function will do what has been done in the Signature.verify() method
// lib/crypto/secp256k1
func BatchSignVerify(pubkey, sig, msg []byte) (bool, error) {
	result := secp256k1.VerifySignature(pubkey, msg, sig)
	return result, nil
}

// lib/crypto/sr25519
func BatchSignVerify(pubkey, sig, msg []byte) (bool, error) {
	pubKey, err := NewPublicKey(pubkey)
	if err != nil {
		return false, fmt.Errorf("failed to fetch sr25519 public key: %s", err)
	}

	return pubKey.Verify(msg, sig)
}

// lib/crypto/ed25519
...
  1. Remove the Signature.verify() method and in the Signature.Start() instead of call sig.verify you will call:
ok, err := sign.VerifyFunc(sign.PubKey, sign.Sign, sign.Msg)
if err != nil || !ok {
	log.Error("[ext_crypto_start_batch_verify_version_1]", "error", err)
	sv.Invalid()
	return
}
  1. Just adjust the lib/runtime/wasmer/imports.go to
// instead of use the `crypto.Secp256k1Type`
signature := runtime.Signature{
	PubKey:    pub.Encode(),
	Sign:      signature,
	Msg:       hash[:],
	KeyTypeID: crypto.Secp256k1Type,
}

// use the `secp256k1.BatchSignVerify`
signature := crytpo.Signature{
	PubKey:    pub.Encode(),
	Sign:      signature,
	Msg:       hash[:],
	VerifyFunc: secp256k1.BatchSignVerify,
}

this way you can move the batch signature verification to lib/crypto package without the cycle issue.
If you need more help don't hesitate to reach me out on gossamer discord

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 a pull request may close this issue.

2 participants