Timeline On Updating To V4 Ietf Draft from herumi/bls

[herumi]

herumi/bls#63

[herumi]

@nisdas
Is it okay that sec.GetPublicKey() returns nil if sec.IsZero()?

[nisdas]

Wouldn't it be better if we returned an error here ? since in this case the secret key is zero and not allowed as of v4

[herumi]

func (sec *SecretKey) GetPublicKey() (pub *PublicKey) {
+  if sec.IsZero() {
+    return nil
+  }
  pub = new(PublicKey)
  C.blsGetPublicKey(&pub.v, &sec.v)
  return pub
}

I think that it is simple, but if returing an error, then it will break the backward compatibility. Is it okay?

[mcdee]

Returning nil would, in my opinion, be worse than changing the method signature. Existing code such as:

key.GetPublicKey().Serialize()

would be liable to panic.

[nisdas]

@herumi That is fine, otherwise we would have to always check for a nil pubkey whenever using the serialize function.

if pub == nil 
{
return errInvalidPubkey
}

Returning an error seems like the correct thing to do in this situation

[herumi]

@nisdas What APIs will you use before sec.GetPublicKey()?
If

sec.SetByCSPRNG()
pub = sec.GetPublicKey()

then, SetByCSPRNG() checks whether sec is zero and causes panic. https://github.com/herumi/bls-eth-go-binary/blob/master/bls/bls.go#L294-L302.
So it is unnecessary to check it in GetPublicKey.

If you write

sec.Deserialize() or something
pub, err := sec.GetPublicKey(); if err ...

then it is almost equaivalent to

sec.Deserialize() or something
if sec.IsZero() { // error }
pub := sec.GetPublicKey()

So how about the latter?

[nisdas]

Dont feel too strongly on this, if the latter will be easier for you I dont mind following it. We only using it in one place in our wrapper so it wont be too difficult to abstract away.

[herumi]

I see.
How about appending new api sec.GetSafePublicKey() (*PublicKey, error) without changing the current GetPublicKey()?

[nisdas]

Sounds good to me in that case 👌

[herumi]

I've appended the api.
ab4d637