Wrong bip32 version for ExtendedKey

zargarzadehm commented 5 months ago

https://github.com/bnb-chain/tss-lib/blob/7113b68867a703940aa0c2150fa80ea63ab8d07b/ecdsa/signing/key_derivation_util.go#L55

Is there a specific reason for setting HDPrivateKeyID as the Version for the extended PublicKey in the function derivingPubkeyFromPath? It seems like it should be HDPublicKeyID.

    extendedParentPk := &ckd.ExtendedKey{
        PublicKey:  pk,
        Depth:      0,
        ChildIndex: 0,
        ChainCode:  chainCode[:],
        ParentFP:   []byte{0x00, 0x00, 0x00, 0x00},
        Version:    net.HDPublicKeyID[:],
    }

For instance, in this unit test, you are deriving using keys[0].ECDSAPub, while the Version set in derivingPubkeyFromPath is HDPrivateKeyID, which implies the response would involve xprv instead of xpub.

ackratos commented 5 months ago

Hi @zargarzadehm , we are taking look at it

yycen commented 5 months ago

Hi @zargarzadehm, there could be some misuse of the field. But currently the (non-hardened) child key derivation only use the publickey, chainocode to compute next level child, the version seems not impact anywhere. I guess the whole structure kept mainly to be compatible with test suite from e.g. btcsuite

zargarzadehm commented 5 months ago

Hi @zargarzadehm, there could be some misuse of the field. But currently the (non-hardened) child key derivation only use the publickey, chainocode to compute next level child, the version seems not impact anywhere. I guess the whole structure kept mainly to be compatible with test suite from e.g. btcsuite

Do you mean for production purpose we should use DeriveChildKeyFromHierarchy method directly, instead of derivingPubkeyFromPath?

bnb-chain / tss-lib

Wrong bip32 version for ExtendedKey #288