Several ergonomic improvements #27
Conversation
thomaseizinger
force-pushed
the
remove-private-public-key-indirection
branch
5 times, most recently
from
6ca74bf to
691b9c4
Compare
Not sure why we would need this and it causes problems ...
Functions that allocate should start with `to_`. `as_` is used for cheap conversions.
To minimise breaking changes, we introduce extension traits that provide most of the old functionality that was defined on the wrapper types. Fixes #13.
thomaseizinger
force-pushed
the
remove-private-public-key-indirection
branch
from
9868b59 to
fee1f70
Compare
luckysori
left a comment
luckysori
left a comment
There was a problem hiding this comment.
Nice solution with the extension trait! I never consider that one.
| addr_type: AddressType::Integrated(payment_id), | ||
| public_spend, | ||
| public_view, | ||
| public_spend: public_spend.compress(), |
There was a problem hiding this comment.
❓ As a general question, why should we store compressed points? I would assume that one could wait until serialisation.
There was a problem hiding this comment.
Unfortunately, EdwardsPoints don't implement Hash so we would have to find a solution around that. Compressing is not cheap but I guess doing it inside Hash might be fine?
There was a problem hiding this comment.
True, you have to compress before you hash.
On principle, I would delay compressing until it's strictly necessary. The thing about CompressedEdwardsY is that it's no different to 32 bytes, so it can hold data that doesn't actually map to an EdwardsPoint (see decompress).
There was a problem hiding this comment.
Yeah I think moving the compressing to a manual implementation of Hash is probably the better idea.
Thank you. It is good if you want to keep the type hierarchy flat. In our case, both |
Draft until dalek-cryptography/curve25519-dalek#353 is accepted.
Additionally, the changes to private/publickey are controversal and should probably go somewhere else and/or be delayed.