-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
Bip Draft: Discrete Log Equality Proofs (DLEQ) #1689
base: master
Are you sure you want to change the base?
Conversation
There was some previous discussion on this gist before making this PR |
|
||
TBD | ||
|
||
== Changelog == |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a section on backwards compatibility, run git grep -A2 Backward
on the repo root for ideas.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure there's anything to be backwards compatible with? This would be a new standard?
Perhaps it could discuss compatibility with https://github.com/BlockstreamResearch/secp256k1-zkp/blob/master/src/modules/ecdsa_adaptor/dleq_impl.h or any other widely used implementations (Joinmarket)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps
==Backwards Compatibility==
This proposal is compatible with all older clients.
Perhaps it could discuss compatibility with BlockstreamResearch/secp256k1-zkp@master/src/modules/ecdsa_adaptor/dleq_impl.h or any other widely used implementations (Joinmarket)?
Yes, it might be useful to add in a Compatibility
section (perhaps somewhat like BIP197) with respect to those.
==Compatibility==
BIP 197 is compatible with [https://github.com/ethereum/EIPs/pull/1850 ERC 1850] for [https://arxiv.org/pdf/1901.05117.pdf atomic loans] with Ethereum. Can be extended in the future to be compatible with other HTLC and smart contract compatible chains.
Input: | ||
* The secret key ''a'': a 256-bit unsigned integer | ||
* The public key ''B'': a point on the curve | ||
* Auxiliary random data ''r'': a 32-byte array |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it recommended to use a different r
every time? I guess there's no risk of a
being leaked with same r
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could recommend this, perhaps as a footnote?
Thanks for your comments @jonatack, @stratospher, @theStack. I've also updated the BIP to include the generator G as an input, and so the BIP is no longer specific to secp256k1. This was mentioned on the mailing list as an improvement to make this standard work with other curves as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From an editorial standpoint this looks good so far. As mentioned by Jon, please include a Backwards Compatibility section, if only to state that there are no concerns.
I did not verify the cryptography of the proof, but after staring at it for a few minutes, I and perhaps other would perhaps benefit from a couple sentences of why/how the proof works e.g. as a footnote. I was also wondering whether you might want to expand on related work, alternate designs, and design decisions in this document. For example you might want to mention some of the things from the opening comment on the PR here, in the footnotes.
Fwiw, I've written a reference implementation of this BIP for secp256k1 in Python, see: https://github.com/theStack/bips/blob/bip-DLEQ-add_reference_impl/bip-DLEQ/reference.py
|
* Fail if ''k = 0''. | ||
* Let ''R<sub>1</sub> = k⋅G''. | ||
* Let ''R<sub>2</sub> = k⋅B''. | ||
* Let ''e = int(hash<sub>BIP0???/challenge</sub>(cbytes(A) || cbytes(B) || cbytes(C) || cbytes(G) || cbytes(R<sub>1</sub>) || cbytes(R<sub>2</sub>)))''. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I wonder if it is really needed to also include the generator point in the challenge hash? Seems excessive to me as its implicitly included in all other points. Generally I'm not sure what are the best practices here, since this seems to be the first BIP where the generator point can be generic and is not defined as the one in secp256k1.
This BIP specifies a standard way to generate and verify DLEQ proofs. This is motivated by sending to silent payments in PSBTs. However, there are also other uses where DLEQs could be useful, so it would be good to have this BIP for others to reference.
This is inspired by https://github.com/discreetlogcontracts/dlcspecs/blob/master/ECDSA-adaptor.md#proof-of-discrete-logarithm-equality, but is a little more specific.
There is an implementation of that already at https://github.com/BlockstreamResearch/secp256k1-zkp/blob/master/src/modules/ecdsa_adaptor/dleq_impl.h, which this BIP attempts to be compatible with.
Inital ML post: https://groups.google.com/g/bitcoindev/c/MezoKV5md7s