On taking a final pass in #443, we identified the following remaining issues in the PiSch proof:
[x] commitment field in PiSchProof doesn't need to be pub(crate)
[x] CommonInput field name needs to be Rust-canonical, not a capital X.
[x] CommonInput::new can put the where clause inline and remove the type parameter (param: &'a impl AsRef<CurvePoint>)
[x] ProverSecret field name could use a more descriptive rename
[x] L149 - use CurvePoint method instead of constructing it manually and calling bn_to_scalar
[x] L153 error log should contain something descriptive about what actually failed
[x] The from_message method should have validation of the challenge and response fields (both must be in the range [0, k256_order), yes.
[x] The from_message method should not use the name keygen_decommit for a proof
[x] The fill_transcript parameter name A is non-canonical; it should match the name of whatever is passed to it
[x] Won't look at proofs carefully now because #115 is in progress, but there's a capital X in with_random_schnorr_proof. IT can be renamed to whatever the common input field name gets changed to.
On taking a final pass in #443, we identified the following remaining issues in the PiSch proof:
commitmentfield inPiSchProofdoesn't need to bepub(crate)CommonInputfield name needs to be Rust-canonical, not a capitalX.CommonInput::newcan put thewhereclause inline and remove the type parameter (param: &'a impl AsRef<CurvePoint>)ProverSecretfield name could use a more descriptive renameCurvePointmethod instead of constructing it manually and callingbn_to_scalarfrom_messagemethod should have validation of thechallengeandresponsefields (both must be in the range[0, k256_order), yes.from_messagemethod should not use the namekeygen_decommitfor a prooffill_transcriptparameter nameAis non-canonical; it should match the name of whatever is passed to itXinwith_random_schnorr_proof. IT can be renamed to whatever the common input field name gets changed to.