check_sig from common.rs seems to reject some valid signatures about multi-party-ecdsa

[1ooper]

1. Why does Zengo use check_sig to validate the signature?
   check_sig validates the signature with the help of the Rust dependency secp256k1, as detailed here: https://docs.rs/secp256k1/latest/secp256k1/. This dependency is a binding to Pieter Wuille’s secp256k1 C library, which finds its use in the Bitcoin network and other related applications. By using a third-party or "official" check, we ensure that the signature is valid in the blockchain after output_signature.

2. Why does the check_sig function verify "sig is msg32 || pk[..32]" ?
   The code you've pointed to seems to come from the fuzzy_dummy mod: https://docs.rs/secp256k1-sys/0.4.2/src/secp256k1_sys/lib.rs.html#664. Please note, this module is designed for fuzz testing and isn't used in signing or verification processes. As I mentioned earlier, the secp256k1 dependency is a binding to a C library, and the verification codes can be found here: https://github.com/rust-bitcoin/rust-secp256k1/blob/7c8270a8506e31731e540fab7ee1abde1f48314e/secp256k1-sys/depend/secp256k1/src/secp256k1.c#L381.
   For your demo codes, I think you should adjust the s to the canonical s like

        let s_tag_bn = Scalar::<Secp256k1>::group_order() - &s_bn;
        if s_bn > s_tag_bn {
            s = Scalar::<Secp256k1>::from(&s_tag_bn);
        }

[1ooper]

I think you might ignore the convention of s in ECDSA in the cryptocurrency field. Both s and -s are valid from the perspective of the algorithm but when s > n/2, where n is the order of the group, we should adjust s into -s i.e. n-s by convention . This is the Zengo code for your reference:

multi-party-ecdsa/src/protocols/multi_party_ecdsa/gg_2018/party_i.rs

Line 674 in 3e711c7

pub fn output_signature(&self, s_vec: &[Scalar<Secp256k1>]) -> Result<SignatureRecid, Error> {

. Also, these two BIPs are about this issue: https://github.com/bitcoin/bips/blob/master/bip-0146.mediawiki#low_s and https://github.com/bitcoin/bips/blob/master/bip-0062.mediawiki#low-s-values-in-signatures

[traffictse]

I think you might ignore the convention of s in ECDSA in the cryptocurrency field. Both s and -s are valid from the perspective of the algorithm but when s > n/2, where n is the order of the group, we should adjust s into -s i.e. n-s by convention . This is the Zengo code for your reference:

multi-party-ecdsa/src/protocols/multi_party_ecdsa/gg_2018/party_i.rs

Line 674 in 3e711c7

pub fn output_signature(&self, s_vec: &[Scalar<Secp256k1>]) -> Result<SignatureRecid, Error> {

. Also, these two BIPs are about this issue: https://github.com/bitcoin/bips/blob/master/bip-0146.mediawiki#low_s and https://github.com/bitcoin/bips/blob/master/bip-0062.mediawiki#low-s-values-in-signatures

Thx for the reply, but I am afraid that using low s by replacing high s with n-s is not exactly what I was doubting. As in your reference of ZenGo's code:

multi-party-ecdsa/src/protocols/multi_party_ecdsa/gg_2018/party_i.rs

Line 674 in 3e711c7

pub fn output_signature(&self, s_vec: &[Scalar<Secp256k1>]) -> Result<SignatureRecid, Error> {

Within this function output_signature, a sub-function verify is invoked (at

multi-party-ecdsa/src/protocols/multi_party_ecdsa/gg_2018/party_i.rs

Line 705 in 3e711c7

let ver = verify(&sig, &self.y, &self.m).is_ok();

) to check the validity of signature.

This sub-function verify is actually the classic verification of ECDSA scheme. This is the code reference:

multi-party-ecdsa/src/protocols/multi_party_ecdsa/gg_2018/party_i.rs

Line 714 in 3e711c7

pub fn verify(sig: &SignatureRecid, y: &Point<Secp256k1>, message: &BigInt) -> Result<(), Error> {

What I was doubting is the reasons:
(1) Why ZenGo used a second sub-function check_sig (at

multi-party-ecdsa/examples/gg18_sign_client.rs

Line 503 in 3e711c7

check_sig(&sig.r, &sig.s, &message_bn, &y_sum);

) to check the validity of signature again right after invoking the function output_signature (at

multi-party-ecdsa/examples/gg18_sign_client.rs

Line 487 in 3e711c7

.output_signature(&s_i_vec)

)?

This is the code reference for check_sig:

multi-party-ecdsa/examples/common.rs

Line 192 in 3e711c7

pub fn check_sig(

(2) According to my demo, ZenGo's verify and check_sig are different in filtering out "invalid" signatures. Apparently, verify complies with the ECDSA standard while check_sig doesn't. Digging deeper, I saw that check_sig invokes this function (https://docs.rs/secp256k1/0.20.0/src/secp256k1/lib.rs.html#813)

   #[inline]
    pub fn verify(&self, msg: &Message, sig: &Signature, pk: &key::PublicKey) -> Result<(), Error> {
        unsafe {
            if ffi::secp256k1_ecdsa_verify(self.ctx, sig.as_c_ptr(), msg.as_c_ptr(), pk.as_c_ptr()) == 0 {
                Err(Error::IncorrectSignature)
            } else {
                Ok(())
            }
        }
    }

, which further invokes this one (https://docs.rs/secp256k1-sys/0.4.2/src/secp256k1_sys/lib.rs.html#933):

    // ECDSA
    /// Verifies that sig is msg32||pk[..32]
    pub unsafe fn secp256k1_ecdsa_verify(cx: *const Context,
                                         sig: *const Signature,
                                         msg32: *const c_uchar,
                                         pk: *const PublicKey)
                                         -> c_int {
        check_context_flags(cx, SECP256K1_START_VERIFY);
        // Actually verify
        let sig_sl = slice::from_raw_parts(sig as *const u8, 64);
        let msg_sl = slice::from_raw_parts(msg32 as *const u8, 32);
        if &sig_sl[..32] == msg_sl && sig_sl[32..] == (*pk).0[0..32] {
            1
        } else {
            0
        }
    }

As you may see, check_sig seems to verify "sig is msg32 || pk[..32]". Why?

[traffictse]

1. Why does Zengo use check_sig to validate the signature?
   check_sig validates the signature with the help of the Rust dependency secp256k1, as detailed here: https://docs.rs/secp256k1/latest/secp256k1/. This dependency is a binding to Pieter Wuille’s secp256k1 C library, which finds its use in the Bitcoin network and other related applications. By using a third-party or "official" check, we ensure that the signature is valid in the blockchain after output_signature.

This does make a lot of sense, considering the wide use of Pieter Wuille’s secp256k1 C library in the Bitcoin network.

2. Why does the check_sig function verify "sig is msg32 || pk[..32]" ?
   The code you've pointed to seems to come from the fuzzy_dummy mod: https://docs.rs/secp256k1-sys/0.4.2/src/secp256k1_sys/lib.rs.html#664. Please note, this module is designed for fuzz testing and isn't used in signing or verification processes. As I mentioned earlier, the secp256k1 dependency is a binding to a C library, and the verification codes can be found here: https://github.com/rust-bitcoin/rust-secp256k1/blob/7c8270a8506e31731e540fab7ee1abde1f48314e/secp256k1-sys/depend/secp256k1/src/secp256k1.c#L381.

You are so right about being within the fuzzy_dummy mod, which I didn't realize previously.

For your demo codes, I think you should adjust the s to the canonical s like

        let s_tag_bn = Scalar::<Secp256k1>::group_order() - &s_bn;
        if s_bn > s_tag_bn {
            s = Scalar::<Secp256k1>::from(&s_tag_bn);
        }

This modification from the original s to the canonical (low) s in my demo codes has brought a consistent output from all three verifications.

Thank you for your patience and help.