ed25519_generate_key_pair should return decoded strings

[sohkai]

From bigchaindb/bigchaindb-common#4. May affect #22.

As far as I can tell, there's no need to return the byte version and it isn't required by the Cryptoconditions spec. We typically use .decode() on them anyway.

[diminator]

@r-marques is this resolved by #28 ?

hence we can close this?

[r-marques]

No.
I'm skeptical in doing this. I would like to the crypto code to be consistent throughout and just handle receive/return byte strings.
Encoding and decoding would be handled outside in the bigchaindb wrappers for cryptoconditions

[diminator]

so we can close this?

it's not a cc issue

[sohkai]

The thing is though, I think most people (or maybe just me?) would expect that the key generation results in a string, not a byte array. We've already had one instance internally where this assumption caused a bug.

Encoding and decoding would be handled outside in the bigchaindb wrappers for cryptoconditions

But this consistency has already been broken by some fulfillments' .to_dict()s.

[diminator]

The thing is though, I think most people (or maybe just me?) would expect that the key generation results in a string, not a byte array.

The original spec states that these are JS buffers, hence Python bytes. so that assumption can be mitigated by refering to the right docs and spec.

But this consistency has already been broken by some fulfillment's .to_dict()s.

This is routine is custom to our implementation and not part of the original spec.
we could provide bytes in the dict, but then we'll need to clean that when using it...

[sohkai]

I guess if we assume the JS implementation of the w3 spec is the "original" that we should keep ourselves consistent to, then that's fine (although I did find that the RSA signing takes in a string as a private key on a cursory glance).

At the end of the day, it just feels like a very awkward interface, but maybe there are good reasons for doing this.

[diminator]

although I did find that the RSA signing takes in a string as a private key on a cursory glance

good find, maybe report this?

feels like a very awkward interface, but maybe there are good reasons for doing this.

yeah, I know - but I think there are some reasons for that. You could ask

[diminator]

feels like a very awkward interface

If we want we can be opinionated about this outside of the 'raw' cc package.
for example, when defining the bdb-driver API we can make sure the user only works with strings

[sohkai]

Might be just a docs typo, this is also incorrectly stated as a "string".

[sohkai]

If we want we can be opinionated about this outside of the 'raw' cc package.

Ok. The more I look around, the more I see that these crypto libs usually return byte arrays so I'm ok if we keep this package 'raw'. I haven't look around too much in terms of higher level libs that use these crypto libs though, so I'm not sure if they usually give back raw results.

[diminator]

can we close this with #28?

[sohkai]

I think with the discussion today, we'll keep to the js implementation and leave this as-is here.