From 8c12e38fe893afa13f07d90580224c572274f68a Mon Sep 17 00:00:00 2001 From: Runchao Han Date: Wed, 12 Feb 2025 17:15:37 +1100 Subject: [PATCH] pop: move pop constructor functions to datagen/ (#508) Part of https://github.com/babylonlabs-io/pm/issues/186 This PR moves PoP constructor functions to `datagen/` package, in order to make it explicit that the constructor functions are only for testing purposes. --- CHANGELOG.md | 1 + test/e2e/btc_staking_e2e_test.go | 6 +- test/e2e/btc_staking_pre_approval_e2e_test.go | 2 +- .../configurer/chain/commands_btcstaking.go | 2 +- testutil/btcstaking-helper/keeper.go | 2 +- testutil/datagen/btcstaking.go | 6 +- testutil/datagen/pop.go | 120 ++++++++++++++++++ x/btcstaking/keeper/msg_server_test.go | 2 +- x/btcstaking/types/pop.go | 104 --------------- x/btcstaking/types/pop_test.go | 24 ++-- .../types/validate_parsed_message_test.go | 2 +- 11 files changed, 144 insertions(+), 127 deletions(-) create mode 100644 testutil/datagen/pop.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 6903ea5fd..8a9b58e44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ### Improvements +- [#508](https://github.com/babylonlabs-io/babylon/pull/508) Move PoP constructor functiosn to datagen/ - [#499](https://github.com/babylonlabs-io/babylon/pull/499) Add `params-by-version` CLI command ### Bug fixes diff --git a/test/e2e/btc_staking_e2e_test.go b/test/e2e/btc_staking_e2e_test.go index 1f67adcd0..454f36d87 100644 --- a/test/e2e/btc_staking_e2e_test.go +++ b/test/e2e/btc_staking_e2e_test.go @@ -410,7 +410,7 @@ func (s *BTCStakingTestSuite) Test6MultisigBTCDelegation() { // NOTE: we use the multisig address for the BTC delegation multisigStakerAddr := sdk.MustAccAddressFromBech32(multisigAddr) - pop, err := bstypes.NewPoPBTC(multisigStakerAddr, s.delBTCSK) + pop, err := datagen.NewPoPBTC(multisigStakerAddr, s.delBTCSK) s.NoError(err) // generate staking tx and slashing tx @@ -480,7 +480,7 @@ func (s *BTCStakingTestSuite) Test7BTCDelegationFeeGrant() { unbondingTime := btcStkParams.UnbondingTimeBlocks // NOTE: we use the grantee staker address for the BTC delegation PoP - pop, err := bstypes.NewPoPBTC(granteeStakerAddr, s.delBTCSK) + pop, err := datagen.NewPoPBTC(granteeStakerAddr, s.delBTCSK) s.NoError(err) // generate staking tx and slashing tx @@ -571,7 +571,7 @@ func (s *BTCStakingTestSuite) Test8BTCDelegationFeeGrantTyped() { unbondingTime := btcStkParams.UnbondingTimeBlocks // NOTE: we use the grantee staker address for the BTC delegation PoP - pop, err := bstypes.NewPoPBTC(granteeStakerAddr, s.delBTCSK) + pop, err := datagen.NewPoPBTC(granteeStakerAddr, s.delBTCSK) s.NoError(err) // generate staking tx and slashing tx diff --git a/test/e2e/btc_staking_pre_approval_e2e_test.go b/test/e2e/btc_staking_pre_approval_e2e_test.go index f6c68fc12..139cb5ca5 100644 --- a/test/e2e/btc_staking_pre_approval_e2e_test.go +++ b/test/e2e/btc_staking_pre_approval_e2e_test.go @@ -96,7 +96,7 @@ func (s *BTCStakingPreApprovalTestSuite) Test1CreateFinalityProviderAndDelegatio // NOTE: we use the node's address for the BTC delegation stakerAddr := sdk.MustAccAddressFromBech32(nonValidatorNode.PublicAddress) - pop, err := bstypes.NewPoPBTC(stakerAddr, s.delBTCSK) + pop, err := datagen.NewPoPBTC(stakerAddr, s.delBTCSK) s.NoError(err) // generate staking tx and slashing tx diff --git a/test/e2e/configurer/chain/commands_btcstaking.go b/test/e2e/configurer/chain/commands_btcstaking.go index dcbba2a95..112032503 100644 --- a/test/e2e/configurer/chain/commands_btcstaking.go +++ b/test/e2e/configurer/chain/commands_btcstaking.go @@ -406,7 +406,7 @@ func (n *NodeConfig) CreateBTCDelegationAndCheck( // NOTE: we use the node's address for the BTC delegation del1Addr := sdk.MustAccAddressFromBech32(delAddr) - popDel1, err := bstypes.NewPoPBTC(del1Addr, btcStakerSK) + popDel1, err := datagen.NewPoPBTC(del1Addr, btcStakerSK) require.NoError(t, err) testStakingInfo, stakingTx, inclusionProof, testUnbondingInfo, delegatorSig := n.BTCStakingUnbondSlashInfo(r, t, btcNet, params, fp, btcStakerSK, stakingTimeBlocks, stakingSatAmt) diff --git a/testutil/btcstaking-helper/keeper.go b/testutil/btcstaking-helper/keeper.go index 9c462c7e6..216c5a6c7 100644 --- a/testutil/btcstaking-helper/keeper.go +++ b/testutil/btcstaking-helper/keeper.go @@ -293,7 +293,7 @@ func (h *Helper) CreateDelegationWithBtcBlockHeight( staker := sdk.MustAccAddressFromBech32(datagen.GenRandomAccount().Address) // PoP - pop, err := types.NewPoPBTC(staker, delSK) + pop, err := datagen.NewPoPBTC(staker, delSK) h.NoError(err) // generate staking tx info prevBlock, _ := datagen.GenRandomBtcdBlock(r, 0, nil) diff --git a/testutil/datagen/btcstaking.go b/testutil/datagen/btcstaking.go index a39da55c0..84f336b7a 100644 --- a/testutil/datagen/btcstaking.go +++ b/testutil/datagen/btcstaking.go @@ -69,7 +69,7 @@ func GenRandomFinalityProviderWithBTCBabylonSKs( btcPK := btcSK.PubKey() bip340PK := bbn.NewBIP340PubKeyFromBTCPK(btcPK) // pop - pop, err := bstypes.NewPoPBTC(fpAddr, btcSK) + pop, err := NewPoPBTC(fpAddr, btcSK) if err != nil { return nil, err } @@ -168,7 +168,7 @@ func GenRandomBTCDelegation( require.NoError(t, err) w := uint16(100) // TODO: parameterise w - pop, err := bstypes.NewPoPBTC(sdk.MustAccAddressFromBech32(staker.Address), delSK) + pop, err := NewPoPBTC(sdk.MustAccAddressFromBech32(staker.Address), delSK) require.NoError(t, err) del := &bstypes.BTCDelegation{ @@ -346,7 +346,7 @@ func GenRandomMsgCreateBtcDelegationAndMsgAddCovenantSignatures( delSlashingTxSig, err := unbondingSlashingInfo.GenDelSlashingTxSig(delSK) require.NoError(t, err) - pop, err := bstypes.NewPoPBTC(sdk.MustAccAddressFromBech32(stakerAddr.String()), delSK) + pop, err := NewPoPBTC(sdk.MustAccAddressFromBech32(stakerAddr.String()), delSK) require.NoError(t, err) msg := &bstypes.MsgCreateBTCDelegation{ diff --git a/testutil/datagen/pop.go b/testutil/datagen/pop.go new file mode 100644 index 000000000..ac8f1ec38 --- /dev/null +++ b/testutil/datagen/pop.go @@ -0,0 +1,120 @@ +package datagen + +import ( + "encoding/hex" + + "github.com/babylonlabs-io/babylon/crypto/bip322" + "github.com/babylonlabs-io/babylon/crypto/ecdsa" + bbn "github.com/babylonlabs-io/babylon/types" + bstypes "github.com/babylonlabs-io/babylon/x/btcstaking/types" + "github.com/btcsuite/btcd/btcec/v2" + "github.com/btcsuite/btcd/btcec/v2/schnorr" + "github.com/btcsuite/btcd/btcutil" + "github.com/btcsuite/btcd/chaincfg" + "github.com/cometbft/cometbft/crypto/tmhash" + sdk "github.com/cosmos/cosmos-sdk/types" +) + +type bip322Sign[A btcutil.Address] func(sg []byte, + privKey *btcec.PrivateKey, + net *chaincfg.Params) (A, []byte, error) + +// NewPoPBTC generates a new proof of possession that sk_BTC and the address are held by the same person +// a proof of possession contains only one signature +// - pop.BtcSig = schnorr_sign(sk_BTC, bbnAddress) +func NewPoPBTC(addr sdk.AccAddress, btcSK *btcec.PrivateKey) (*bstypes.ProofOfPossessionBTC, error) { + pop := bstypes.ProofOfPossessionBTC{ + BtcSigType: bstypes.BTCSigType_BIP340, // by default, we use BIP-340 encoding for BTC signature + } + + // generate pop.BtcSig = schnorr_sign(sk_BTC, hash(bbnAddress)) + // NOTE: *schnorr.Sign has to take the hash of the message. + // So we have to hash the address before signing + hash := tmhash.Sum(addr.Bytes()) + btcSig, err := schnorr.Sign(btcSK, hash) + if err != nil { + return nil, err + } + bip340Sig := bbn.NewBIP340SignatureFromBTCSig(btcSig) + pop.BtcSig = bip340Sig.MustMarshal() + + return &pop, nil +} + +// NewPoPWithECDSABTCSig generates a new proof of possession where Bitcoin signature is in ECDSA format +// a proof of possession contains two signatures: +// - pop.BtcSig = ecdsa_sign(sk_BTC, addr) +func NewPoPBTCWithECDSABTCSig(addr sdk.AccAddress, btcSK *btcec.PrivateKey) (*bstypes.ProofOfPossessionBTC, error) { + pop := bstypes.ProofOfPossessionBTC{ + BtcSigType: bstypes.BTCSigType_ECDSA, + } + + // generate pop.BtcSig = ecdsa_sign(sk_BTC, pop.BabylonSig) + // NOTE: ecdsa.Sign has to take the message as string. + // So we have to hex addr before signing + addrHex := hex.EncodeToString(addr.Bytes()) + btcSig := ecdsa.Sign(btcSK, addrHex) + pop.BtcSig = btcSig + + return &pop, nil +} + +func newPoPBTCWithBIP322Sig[A btcutil.Address]( + addressToSign sdk.AccAddress, + btcSK *btcec.PrivateKey, + net *chaincfg.Params, + bip322SignFn bip322Sign[A], +) (*bstypes.ProofOfPossessionBTC, error) { + pop := bstypes.ProofOfPossessionBTC{ + BtcSigType: bstypes.BTCSigType_BIP322, + } + + bip322SigEncoded, err := newBIP322Sig(tmhash.Sum(addressToSign.Bytes()), btcSK, net, bip322SignFn) + if err != nil { + return nil, err + } + pop.BtcSig = bip322SigEncoded + + return &pop, nil +} + +func newBIP322Sig[A btcutil.Address]( + msgToSign []byte, + btcSK *btcec.PrivateKey, + net *chaincfg.Params, + bip322SignFn bip322Sign[A], +) ([]byte, error) { + address, witnessSignture, err := bip322SignFn( + msgToSign, + btcSK, + net, + ) + if err != nil { + return nil, err + } + + bip322Sig := bstypes.BIP322Sig{ + Address: address.EncodeAddress(), + Sig: witnessSignture, + } + + return bip322Sig.Marshal() +} + +// NewPoPBTCWithBIP322P2WPKHSig creates a proof of possession of type BIP322 +// that signs the address with the BTC secret key. +func NewPoPBTCWithBIP322P2WPKHSig( + addr sdk.AccAddress, + btcSK *btcec.PrivateKey, + net *chaincfg.Params, +) (*bstypes.ProofOfPossessionBTC, error) { + return newPoPBTCWithBIP322Sig(addr, btcSK, net, bip322.SignWithP2WPKHAddress) +} + +func NewPoPBTCWithBIP322P2TRBIP86Sig( + addrToSign sdk.AccAddress, + btcSK *btcec.PrivateKey, + net *chaincfg.Params, +) (*bstypes.ProofOfPossessionBTC, error) { + return newPoPBTCWithBIP322Sig(addrToSign, btcSK, net, bip322.SignWithP2TrSpendAddress) +} diff --git a/x/btcstaking/keeper/msg_server_test.go b/x/btcstaking/keeper/msg_server_test.go index f9d305d31..71bcba5c6 100644 --- a/x/btcstaking/keeper/msg_server_test.go +++ b/x/btcstaking/keeper/msg_server_test.go @@ -1007,7 +1007,7 @@ func TestDoNotAllowDelegationWithoutFinalityProvider(t *testing.T) { stakerAddr := sdk.MustAccAddressFromBech32(acc.Address) // PoP - pop, err := types.NewPoPBTC(stakerAddr, delSK) + pop, err := datagen.NewPoPBTC(stakerAddr, delSK) require.NoError(t, err) // generate staking tx info prevBlock, _ := datagen.GenRandomBtcdBlock(r, 0, nil) diff --git a/x/btcstaking/types/pop.go b/x/btcstaking/types/pop.go index 93b7125e9..f2f4c9f48 100644 --- a/x/btcstaking/types/pop.go +++ b/x/btcstaking/types/pop.go @@ -20,102 +20,6 @@ import ( type checkStakerKey func(stakerKey *bbn.BIP340PubKey) error -type bip322Sign[A btcutil.Address] func(sg []byte, - privKey *btcec.PrivateKey, - net *chaincfg.Params) (A, []byte, error) - -// NewPoPBTC generates a new proof of possession that sk_BTC and the address are held by the same person -// a proof of possession contains only one signature -// - pop.BtcSig = schnorr_sign(sk_BTC, bbnAddress) -func NewPoPBTC(addr sdk.AccAddress, btcSK *btcec.PrivateKey) (*ProofOfPossessionBTC, error) { - pop := ProofOfPossessionBTC{ - BtcSigType: BTCSigType_BIP340, // by default, we use BIP-340 encoding for BTC signature - } - - // generate pop.BtcSig = schnorr_sign(sk_BTC, hash(bbnAddress)) - // NOTE: *schnorr.Sign has to take the hash of the message. - // So we have to hash the address before signing - hash := tmhash.Sum(addr.Bytes()) - btcSig, err := schnorr.Sign(btcSK, hash) - if err != nil { - return nil, err - } - bip340Sig := bbn.NewBIP340SignatureFromBTCSig(btcSig) - pop.BtcSig = bip340Sig.MustMarshal() - - return &pop, nil -} - -// NewPoPWithECDSABTCSig generates a new proof of possession where Bitcoin signature is in ECDSA format -// a proof of possession contains two signatures: -// - pop.BtcSig = ecdsa_sign(sk_BTC, addr) -func NewPoPBTCWithECDSABTCSig(addr sdk.AccAddress, btcSK *btcec.PrivateKey) (*ProofOfPossessionBTC, error) { - pop := ProofOfPossessionBTC{ - BtcSigType: BTCSigType_ECDSA, - } - - // generate pop.BtcSig = ecdsa_sign(sk_BTC, pop.BabylonSig) - // NOTE: ecdsa.Sign has to take the message as string. - // So we have to hex addr before signing - addrHex := hex.EncodeToString(addr.Bytes()) - btcSig := ecdsa.Sign(btcSK, addrHex) - pop.BtcSig = btcSig - - return &pop, nil -} - -func newPoPBTCWithBIP322Sig[A btcutil.Address]( - addressToSign sdk.AccAddress, - btcSK *btcec.PrivateKey, - net *chaincfg.Params, - bip322SignFn bip322Sign[A], -) (*ProofOfPossessionBTC, error) { - pop := ProofOfPossessionBTC{ - BtcSigType: BTCSigType_BIP322, - } - - bip322SigEncoded, err := newBIP322Sig(tmhash.Sum(addressToSign.Bytes()), btcSK, net, bip322SignFn) - if err != nil { - return nil, err - } - pop.BtcSig = bip322SigEncoded - - return &pop, nil -} - -func newBIP322Sig[A btcutil.Address]( - msgToSign []byte, - btcSK *btcec.PrivateKey, - net *chaincfg.Params, - bip322SignFn bip322Sign[A], -) ([]byte, error) { - address, witnessSignture, err := bip322SignFn( - msgToSign, - btcSK, - net, - ) - if err != nil { - return nil, err - } - - bip322Sig := BIP322Sig{ - Address: address.EncodeAddress(), - Sig: witnessSignture, - } - - return bip322Sig.Marshal() -} - -// NewPoPBTCWithBIP322P2WPKHSig creates a proof of possession of type BIP322 -// that signs the address with the BTC secret key. -func NewPoPBTCWithBIP322P2WPKHSig( - addr sdk.AccAddress, - btcSK *btcec.PrivateKey, - net *chaincfg.Params, -) (*ProofOfPossessionBTC, error) { - return newPoPBTCWithBIP322Sig(addr, btcSK, net, bip322.SignWithP2WPKHAddress) -} - func NewPoPBTCFromHex(popHex string) (*ProofOfPossessionBTC, error) { popBytes, err := hex.DecodeString(popHex) if err != nil { @@ -185,14 +89,6 @@ func (pop *ProofOfPossessionBTC) VerifyBIP340(stakerAddr sdk.AccAddress, bip340P return VerifyBIP340(pop.BtcSigType, pop.BtcSig, bip340PK, stakerAddr.Bytes()) } -func NewPoPBTCWithBIP322P2TRBIP86Sig( - addrToSign sdk.AccAddress, - btcSK *btcec.PrivateKey, - net *chaincfg.Params, -) (*ProofOfPossessionBTC, error) { - return newPoPBTCWithBIP322Sig(addrToSign, btcSK, net, bip322.SignWithP2TrSpendAddress) -} - // isSupportedAddressAndWitness checks whether provided address and witness are // valid for proof of possession verification. // Currently the only supported options are: diff --git a/x/btcstaking/types/pop_test.go b/x/btcstaking/types/pop_test.go index 0c77a3cb7..84e7ed325 100644 --- a/x/btcstaking/types/pop_test.go +++ b/x/btcstaking/types/pop_test.go @@ -41,7 +41,7 @@ func FuzzPoP_BIP340(f *testing.F) { accAddr := datagen.GenRandomAccount().GetAddress() // generate and verify PoP, correct case - pop, err := types.NewPoPBTC(accAddr, btcSK) + pop, err := datagen.NewPoPBTC(accAddr, btcSK) require.NoError(t, err) err = pop.VerifyBIP340(accAddr, bip340PK) require.NoError(t, err) @@ -67,7 +67,7 @@ func FuzzPoP_ECDSA(f *testing.F) { accAddr := datagen.GenRandomAccount().GetAddress() // generate and verify PoP, correct case - pop, err := types.NewPoPBTCWithECDSABTCSig(accAddr, btcSK) + pop, err := datagen.NewPoPBTCWithECDSABTCSig(accAddr, btcSK) require.NoError(t, err) err = pop.VerifyECDSA(accAddr, bip340PK) require.NoError(t, err) @@ -88,7 +88,7 @@ func FuzzPoP_BIP322_P2WPKH(f *testing.F) { accAddr := datagen.GenRandomAccount().GetAddress() // generate and verify PoP, correct case - pop, err := types.NewPoPBTCWithBIP322P2WPKHSig(accAddr, btcSK, net) + pop, err := datagen.NewPoPBTCWithBIP322P2WPKHSig(accAddr, btcSK, net) require.NoError(t, err) err = pop.VerifyBIP322(accAddr, bip340PK, net) require.NoError(t, err) @@ -109,7 +109,7 @@ func FuzzPoP_BIP322_P2Tr_BIP86(f *testing.F) { accAddr := datagen.GenRandomAccount().GetAddress() // generate and verify PoP, correct case - pop, err := types.NewPoPBTCWithBIP322P2TRBIP86Sig(accAddr, btcSK, net) + pop, err := datagen.NewPoPBTCWithBIP322P2TRBIP86Sig(accAddr, btcSK, net) require.NoError(t, err) err = pop.VerifyBIP322(accAddr, bip340PK, net) require.NoError(t, err) @@ -134,7 +134,7 @@ func FuzzPop_ValidBip322SigNotMatchingBip340PubKey(f *testing.F) { accAddr := datagen.GenRandomAccount().GetAddress() // generate valid bip322 P2WPKH pop - pop, err := types.NewPoPBTCWithBIP322P2WPKHSig(accAddr, btcSK, net) + pop, err := datagen.NewPoPBTCWithBIP322P2WPKHSig(accAddr, btcSK, net) require.NoError(t, err) // verify bip322 pop with incorrect staker key @@ -142,7 +142,7 @@ func FuzzPop_ValidBip322SigNotMatchingBip340PubKey(f *testing.F) { require.Error(t, err) // generate valid bip322 P2Tr pop - pop, err = types.NewPoPBTCWithBIP322P2TRBIP86Sig(accAddr, btcSK, net) + pop, err = datagen.NewPoPBTCWithBIP322P2TRBIP86Sig(accAddr, btcSK, net) require.NoError(t, err) // verify bip322 pop with incorrect staker key @@ -159,13 +159,13 @@ func TestPoPBTCValidateBasic(t *testing.T) { addrToSign := sdk.MustAccAddressFromBech32(datagen.GenRandomAccount().Address) - popBip340, err := types.NewPoPBTC(addrToSign, btcSK) + popBip340, err := datagen.NewPoPBTC(addrToSign, btcSK) require.NoError(t, err) - popBip322, err := types.NewPoPBTCWithBIP322P2WPKHSig(addrToSign, btcSK, &chaincfg.MainNetParams) + popBip322, err := datagen.NewPoPBTCWithBIP322P2WPKHSig(addrToSign, btcSK, &chaincfg.MainNetParams) require.NoError(t, err) - popECDSA, err := types.NewPoPBTCWithECDSABTCSig(addrToSign, btcSK) + popECDSA, err := datagen.NewPoPBTCWithECDSABTCSig(addrToSign, btcSK) require.NoError(t, err) tcs := []struct { @@ -245,13 +245,13 @@ func TestPoPBTCVerify(t *testing.T) { netParams := &chaincfg.MainNetParams - popBip340, err := types.NewPoPBTC(addrToSign, btcSK) + popBip340, err := datagen.NewPoPBTC(addrToSign, btcSK) require.NoError(t, err) - popBip322, err := types.NewPoPBTCWithBIP322P2WPKHSig(addrToSign, btcSK, netParams) + popBip322, err := datagen.NewPoPBTCWithBIP322P2WPKHSig(addrToSign, btcSK, netParams) require.NoError(t, err) - popECDSA, err := types.NewPoPBTCWithECDSABTCSig(addrToSign, btcSK) + popECDSA, err := datagen.NewPoPBTCWithECDSABTCSig(addrToSign, btcSK) require.NoError(t, err) tcs := []struct { diff --git a/x/btcstaking/types/validate_parsed_message_test.go b/x/btcstaking/types/validate_parsed_message_test.go index c3d04f471..999a71d00 100644 --- a/x/btcstaking/types/validate_parsed_message_test.go +++ b/x/btcstaking/types/validate_parsed_message_test.go @@ -137,7 +137,7 @@ func createMsgDelegationForParams( require.NoError(t, err) stPk := bbn.NewBIP340PubKeyFromBTCPK(delPK) staker := sdk.MustAccAddressFromBech32(datagen.GenRandomAccount().Address) - pop, err := types.NewPoPBTC(staker, delSK) + pop, err := datagen.NewPoPBTC(staker, delSK) require.NoError(t, err) // finality provider related data _, fpPk, err := datagen.GenRandomBTCKeyPair(r)