mimblewimble / secp256k1-zkp

Fork of secp256k1-zkp for the Grin/MimbleWimble project
MIT License
32 stars 42 forks source link

should not mix hash(R,P,m) and hash(P,R,m) #26

Closed garyyu closed 5 years ago

garyyu commented 5 years ago

Gary Yu @garyyu 11:00 @jaspervdm Now I remember where I saw e = hash(P,R,m) firstly. https://github.com/mimblewimble/secp256k1-zkp/blob/master/src/modules/aggsig/main_impl.h#L72-L81

And we both agree to be compatible with BIP-schnorr, that’s why we use e = hash(R,P,m): https://github.com/mimblewimble/secp256k1-zkp/blob/master/src/modules/aggsig/main_impl.h#L45-L57

We should not mix 2 different definition of e. Please let's stick to BIP-schnorr, and correct the https://github.com/mimblewimble/secp256k1-zkp/blob/master/src/modules/aggsig/main_impl.h#L72-L81

Will give a PR on this.

jaspervdm commented 5 years ago

Ah yes, I also noticed this when going over the code today. Even though this part is currently not used for grin, I agree it should be fixed to be consistent.