Update docs to mention ECDSA #643
Conversation
|
@apoelstra is this PR correct? If it is, why do we provide conversion from one pubkey to another ( |
|
The PR is fine, but public keys are used for other things (and were used for other things even when this text was written). Do you want it to reflect the exact set of all things public keys are used for? If so, it needs to be significantly broadened. |
tcharding
force-pushed
the
08-09-pubkey-docs
branch
from
1e23d27 to
037860c
Compare
|
That was cool, I just got lost, as they say, down the rabbit hole, reading cryptography stuff. How'd I go? |
src/key.rs
Outdated
| /// Secret key - a 256-bit key used to create ECDSA and schnorr signatures. | ||
| /// | ||
| /// A point on the secp256k1 curve i.e., a positive 256-bit integer guaranteed to be less than the | ||
| /// secp256k1 curve order. |
There was a problem hiding this comment.
A scalar isn't a point -- and while we often encode scalars as integers, mathematically they aren't integers, so I think describing them as such is a little misleading.
There was a problem hiding this comment.
Gee I botched that up. I removed the whole line. They aren't referred to as integers because we don't do integer math on them? Is it because they are members of a field, is that right? (Not adding as docs, just for my understanding.)
There was a problem hiding this comment.
@tcharding integers are in Z, scalars are in Zn where n is the curve order.
src/key.rs
Outdated
| /// A Secp256k1 public key, used for verification of signatures. | ||
| /// Public key - a point on the secp256k1 curve. | ||
| /// | ||
| /// Obtained by multiplying a secret key scalar by G (the generator point). |
There was a problem hiding this comment.
This is true, but I think it's a little too technical for a doccomment.
src/key.rs
Outdated
| /// An x-only public key - the X co-ordinate of a point on the secp256k1 curve. | ||
| /// | ||
| /// To convert to a point (`PublicKey`) one must choose the [`Parity`] of the Y co-ordinate. When | ||
| /// used to create schnorr signatures the Y co-ordinate is always positive. |
There was a problem hiding this comment.
I think coordinate shouldn't have a -. I would also say "Taproot signature" rather than "schnorr signature".
There was a problem hiding this comment.
re: schnorr vs taproot I thought here in secp we used schnorr and in bitcoin used "taproot". There are zero other occurrences of "taproot" currently in this crate (except some secp-sys test code)? I'm not arguing for or against just asking.
|
This is low priority @apoelstra, I'm basically taking free lessons from you :) |
tcharding
force-pushed
the
08-09-pubkey-docs
branch
2 times, most recently
from
e8e9ce9 to
7fd90f7
Compare
|
O boy, there are many uses x vs X (y vs Y) and coordinate vs co-ordinate all over this crate. Leaving that for another brain fried Friday afternoon job. |
|
I dunno about describing keys as "ponits on the secp256k1 curve". I guess that's fine, and technically correct, but it's really not relevant to users and it doesn't expose any methods to do generic EC operations on it. |
|
Ok, I think you better tell me, or point me to some docs please about what exactly public keys can be used for so I can write these docs. Again, its totally non-urgent though. I can also look it up later on, its Saturday morning here, I was just doing a quick thing or two before the kids woke up. |
|
I think it's fine to say that public keys can be used to verify signatures. They can also be used for ECDH handshakes and for Taproot tweaks. |
tcharding
force-pushed
the
08-09-pubkey-docs
branch
from
7fd90f7 to
f764a7f
Compare
|
Another attempt at the worlds hardest trivial PR :) |
|
Note that I change the docs for |
|
@tcharding I think this one goes in the wrong direction. What's important about a public key is that it can be used to verify signatures, not that it's a point on an elliptic curve. If you want to mention EC stuff that's probably fine but it shouldn't be the primary thing. |
tcharding
force-pushed
the
08-09-pubkey-docs
branch
2 times, most recently
from
844277e to
f140750
Compare
|
Removed the changes to |
|
Looking better :). I would replace |
tcharding
force-pushed
the
08-09-pubkey-docs
branch
from
f140750 to
ecb5260
Compare
|
Now includes one change of schnorr -> Taproot, there are more places we could do this but I think that is scope creep. I'm not sure if we want the "and serialized according to BIP-340" bit on |
|
I think it's fine to mention the encoding. We arguably could mention that other public keys are serialized according to SEC 2, but it's less informative since SEC2 is the standard (and pretty-much only) serialization for normal public keys. Vs x-only keys which are a creation of BIP340. |
|
@tcharding can you rebase this since #591 was merged? |
Crypto is _hard_. Make an effort to improve the docs with a minimum of exactly correct information.
tcharding
force-pushed
the
08-09-pubkey-docs
branch
from
ecb5260 to
aa4489c
Compare
|
Rebase only, no other changes. |
PublicKeytypes are for verifying ECDSA signatures, when these docs where written there were no other types of signatures. With the addition of taproot these docs have become stale.