refactor: (BitcoinRBF-Step2): some minimum code refactoring for Bitcoin RBF adoption

changelog.md

@@ -7,6 +7,7 @@
 * [3332](https://github.com/zeta-chain/node/pull/3332) - implement orchestrator V2. Move BTC observer-signer to V2
 * [3360](https://github.com/zeta-chain/node/pull/3360) - update protocol contract imports using consolidated path
 * [3349](https://github.com/zeta-chain/node/pull/3349) - implement new bitcoin rpc in zetaclient with improved performance and observability
+* [3381](https://github.com/zeta-chain/node/pull/3381) - some code refactoring in zetaclient for Bitcoin RBF adoption
 
 ### Fixes
 

testutil/sample/crypto.go

@@ -12,6 +12,7 @@ import (
 	"github.com/btcsuite/btcd/btcutil"
 	"github.com/btcsuite/btcd/chaincfg"
 	"github.com/btcsuite/btcd/chaincfg/chainhash"
+	"github.com/btcsuite/btcd/txscript"
 	"github.com/cometbft/cometbft/crypto/secp256k1"
 	"github.com/cosmos/cosmos-sdk/crypto/keys/ed25519"
 	cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
@@ -91,15 +92,23 @@ func EthAddressFromRand(r *rand.Rand) ethcommon.Address {
 }
 
 // BtcAddressP2WPKH returns a sample btc P2WPKH address
-func BtcAddressP2WPKH(t *testing.T, net *chaincfg.Params) string {
+func BtcAddressP2WPKH(t *testing.T, net *chaincfg.Params) *btcutil.AddressWitnessPubKeyHash {
 	privateKey, err := btcec.NewPrivateKey()
 	require.NoError(t, err)
 
 	pubKeyHash := btcutil.Hash160(privateKey.PubKey().SerializeCompressed())
 	addr, err := btcutil.NewAddressWitnessPubKeyHash(pubKeyHash, net)
 	require.NoError(t, err)
 
-	return addr.String()
+	return addr
+}
+
+// BtcAddressP2WPKH returns a pkscript for a sample btc P2WPKH address
+func BtcAddressP2WPKHScript(t *testing.T, net *chaincfg.Params) []byte {
+	addr := BtcAddressP2WPKH(t, net)
+	script, err := txscript.PayToAddrScript(addr)
+	require.NoError(t, err)
+	return script
 }
 
 // SolanaPrivateKey returns a sample solana private key

x/crosschain/types/cctx_test.go

@@ -140,7 +140,7 @@ func Test_SetRevertOutboundValues(t *testing.T) {
 		cctx := sample.CrossChainTx(t, "test")
 		cctx.InboundParams.SenderChainId = chains.BitcoinTestnet.ChainId
 		cctx.OutboundParams = cctx.OutboundParams[:1]
-		cctx.RevertOptions.RevertAddress = sample.BtcAddressP2WPKH(t, &chaincfg.TestNet3Params)
+		cctx.RevertOptions.RevertAddress = sample.BtcAddressP2WPKH(t, &chaincfg.TestNet3Params).String()
 
 		err := cctx.AddRevertOutbound(100)
 		require.NoError(t, err)

x/crosschain/types/revert_options_test.go

@@ -49,7 +49,7 @@ func TestRevertOptions_GetEVMRevertAddress(t *testing.T) {
 
 func TestRevertOptions_GetBTCRevertAddress(t *testing.T) {
 	t.Run("valid Bitcoin revert address", func(t *testing.T) {
-		addr := sample.BtcAddressP2WPKH(t, &chaincfg.TestNet3Params)
+		addr := sample.BtcAddressP2WPKH(t, &chaincfg.TestNet3Params).String()
 		actualAddr, valid := types.RevertOptions{
 			RevertAddress: addr,
 		}.GetBTCRevertAddress(chains.BitcoinTestnet.ChainId)

zetaclient/chains/bitcoin/client/client_test.go

@@ -137,7 +137,7 @@ func TestClientLive(t *testing.T) {
 
 		require.NoError(t, err)
 		require.Len(t, inbounds, 1)
-		require.Equal(t, inbounds[0].Value, 0.0001)
+		require.Equal(t, inbounds[0].Value+inbounds[0].DepositorFee, 0.0001)
 		require.Equal(t, inbounds[0].ToAddress, "tb1qsa222mn2rhdq9cruxkz8p2teutvxuextx3ees2")
 
 		// the text memo is base64 std encoded string:DSRR1RmDCwWmxqY201/TMtsJdmA=
@@ -332,7 +332,7 @@ func TestClientLive(t *testing.T) {
 		require.NoError(t, err)
 
 		// go back whatever blocks as needed
-		endBlock := startBlock - 100
+		endBlock := startBlock - 1
 
 		// loop through mempool.space blocks backwards
 		for bn := startBlock; bn >= endBlock; {

zetaclient/chains/bitcoin/client/helpers.go

@@ -3,6 +3,7 @@
 import (
 	"context"
 	"fmt"
+	"math/big"
 	"time"
 
 	types "github.com/btcsuite/btcd/btcjson"
@@ -11,6 +12,17 @@
 	"github.com/pkg/errors"
 )
 
+const (
+	// FeeRateRegnet is the hardcoded fee rate for regnet
+	FeeRateRegnet = 1
+
+	// maxBTCSupply is the maximum supply of Bitcoin
+	maxBTCSupply = 21000000.0
+
+	// bytesPerKB is the number of vB in a KB
+	bytesPerKB = 1000
+)
+
 // GetBlockVerboseByStr alias for GetBlockVerbose
 func (c *Client) GetBlockVerboseByStr(ctx context.Context, blockHash string) (*types.GetBlockVerboseTxResult, error) {
 	h, err := strToHash(blockHash)
@@ -104,6 +116,37 @@
 	}
 }
 
+// FeeRateToSatPerByte converts a fee rate from BTC/KB to sat/vB.
+func FeeRateToSatPerByte(rate float64) *big.Int {
+	// #nosec G115 always in range
+	satPerKB := new(big.Int).SetInt64(int64(rate * btcutil.SatoshiPerBitcoin))
+	return new(big.Int).Div(satPerKB, big.NewInt(bytesPerKB))
+}
+
+// GetEstimatedFeeRate gets estimated smart fee rate (BTC/Kb) targeting given block confirmation
+func (c *Client) GetEstimatedFeeRate(ctx context.Context, confTarget int64, regnet bool) (int64, error) {
+	// RPC 'EstimateSmartFee' is not available in regnet
+	if regnet {
+		return FeeRateRegnet, nil
+	}
+
+	feeResult, err := c.EstimateSmartFee(ctx, confTarget, &types.EstimateModeEconomical)
+	if err != nil {
+		return 0, errors.Wrap(err, "unable to estimate smart fee")
+	}
+	if feeResult.Errors != nil {
+		return 0, fmt.Errorf("fee result contains errors: %s", feeResult.Errors)
+	}
+	if feeResult.FeeRate == nil {
+		return 0, fmt.Errorf("nil fee rate")
+	}
+	if *feeResult.FeeRate <= 0 || *feeResult.FeeRate >= maxBTCSupply {
+		return 0, fmt.Errorf("invalid fee rate: %f", *feeResult.FeeRate)
+	}
+
+	return FeeRateToSatPerByte(*feeResult.FeeRate).Int64(), nil
+}
+
 // GetTransactionFeeAndRate gets the transaction fee and rate for a given tx result
 func (c *Client) GetTransactionFeeAndRate(ctx context.Context, rawResult *types.TxRawResult) (int64, int64, error) {
 	var (

zetaclient/chains/bitcoin/client/mockgen.go

@@ -45,6 +45,7 @@ type client interface {
 
 	SendRawTransaction(ctx context.Context, tx *wire.MsgTx, allowHighFees bool) (*hash.Hash, error)
 
+	GetEstimatedFeeRate(ctx context.Context, confTarget int64, regnet bool) (int64, error)
 	GetTransactionFeeAndRate(ctx context.Context, tx *types.TxRawResult) (int64, int64, error)
 	EstimateSmartFee(
 		ctx context.Context,

zetaclient/chains/bitcoin/common/fee.go

@@ -5,7 +5,6 @@
 	"encoding/hex"
 	"fmt"
 	"math"
-	"math/big"
 
 	"github.com/btcsuite/btcd/blockchain"
 	"github.com/btcsuite/btcd/btcjson"
@@ -20,19 +19,17 @@
 
 const (
 	// constants related to transaction size calculations
-	bytesPerKB           = 1000
-	bytesPerInput        = 41           // each input is 41 bytes
-	bytesPerOutputP2TR   = 43           // each P2TR output is 43 bytes
-	bytesPerOutputP2WSH  = 43           // each P2WSH output is 43 bytes
-	bytesPerOutputP2WPKH = 31           // each P2WPKH output is 31 bytes
-	bytesPerOutputP2SH   = 32           // each P2SH output is 32 bytes
-	bytesPerOutputP2PKH  = 34           // each P2PKH output is 34 bytes
-	bytesPerOutputAvg    = 37           // average size of all above types of outputs (36.6 bytes)
-	bytes1stWitness      = 110          // the 1st witness incurs about 110 bytes and it may vary
-	bytesPerWitness      = 108          // each additional witness incurs about 108 bytes and it may vary
-	OutboundBytesMin     = uint64(239)  // 239vB == EstimateSegWitTxSize(2, 2, toP2WPKH)
-	OutboundBytesMax     = uint64(1543) // 1543v == EstimateSegWitTxSize(21, 2, toP2TR)
-	OutboundBytesAvg     = uint64(245)  // 245vB is a suggested gas limit for zetacore
+	bytesPerInput        = 41          // each input is 41 bytes
+	bytesPerOutputP2TR   = 43          // each P2TR output is 43 bytes
+	bytesPerOutputP2WSH  = 43          // each P2WSH output is 43 bytes
+	bytesPerOutputP2WPKH = 31          // each P2WPKH output is 31 bytes
+	bytesPerOutputP2SH   = 32          // each P2SH output is 32 bytes
+	bytesPerOutputP2PKH  = 34          // each P2PKH output is 34 bytes
+	bytesPerOutputAvg    = 37          // average size of all above types of outputs (36.6 bytes)
+	bytes1stWitness      = 110         // the 1st witness incurs about 110 bytes and it may vary
+	bytesPerWitness      = 108         // each additional witness incurs about 108 bytes and it may vary
+	OutboundBytesMin     = int64(239)  // 239vB == EstimateOutboundSize(2, 2, toP2WPKH)
+	OutboundBytesMax     = int64(1543) // 1543v == EstimateOutboundSize(21, 2, toP2TR)
 
 	// defaultDepositorFeeRate is the default fee rate for depositor fee, 20 sat/vB
 	defaultDepositorFeeRate = 20
@@ -49,6 +46,7 @@
 	BtcOutboundBytesDepositor = OutboundSizeDepositor()
 
 	// BtcOutboundBytesWithdrawer is the outbound size incurred by the withdrawer: 177vB
+	// This will be the suggested gas limit used for zetacore
 	BtcOutboundBytesWithdrawer = OutboundSizeWithdrawer()
 
 	// DefaultDepositorFee is the default depositor fee is 0.00001360 BTC (20 * 68vB / 100000000)
@@ -67,34 +65,28 @@
 // DepositorFeeCalculator is a function type to calculate the Bitcoin depositor fee
 type DepositorFeeCalculator func(context.Context, RPC, *btcjson.TxRawResult, *chaincfg.Params) (float64, error)
 
-// FeeRateToSatPerByte converts a fee rate in BTC/KB to sat/byte.
-func FeeRateToSatPerByte(rate float64) *big.Int {
-	// #nosec G115 always in range
-	satPerKB := new(big.Int).SetInt64(int64(rate * btcutil.SatoshiPerBitcoin))
-	return new(big.Int).Div(satPerKB, big.NewInt(bytesPerKB))
-}
-
 // WiredTxSize calculates the wired tx size in bytes
-func WiredTxSize(numInputs uint64, numOutputs uint64) uint64 {
+func WiredTxSize(numInputs uint64, numOutputs uint64) int64 {
 	// Version 4 bytes + LockTime 4 bytes + Serialized varint size for the
 	// number of transaction inputs and outputs.
 	// #nosec G115 always positive
-	return uint64(8 + wire.VarIntSerializeSize(numInputs) + wire.VarIntSerializeSize(numOutputs))
+	return int64(8 + wire.VarIntSerializeSize(numInputs) + wire.VarIntSerializeSize(numOutputs))
 }
 
 // EstimateOutboundSize estimates the size of an outbound in vBytes
-func EstimateOutboundSize(numInputs uint64, payees []btcutil.Address) (uint64, error) {
-	if numInputs == 0 {
+func EstimateOutboundSize(numInputs int64, payees []btcutil.Address) (int64, error) {
+	if numInputs <= 0 {
 		return 0, nil
 	}
 	// #nosec G115 always positive
 	numOutputs := 2 + uint64(len(payees))
-	bytesWiredTx := WiredTxSize(numInputs, numOutputs)
+	// #nosec G115 checked positive
+	bytesWiredTx := WiredTxSize(uint64(numInputs), numOutputs)
 	bytesInput := numInputs * bytesPerInput
-	bytesOutput := uint64(2) * bytesPerOutputP2WPKH // new nonce mark, change
+	bytesOutput := int64(2) * bytesPerOutputP2WPKH // new nonce mark, change
 
 	// calculate the size of the outputs to payees
-	bytesToPayees := uint64(0)
+	bytesToPayees := int64(0)
 	for _, to := range payees {
 		sizeOutput, err := GetOutputSizeByAddress(to)
 		if err != nil {
@@ -112,7 +104,7 @@
 }
 
 // GetOutputSizeByAddress returns the size of a tx output in bytes by the given address
-func GetOutputSizeByAddress(to btcutil.Address) (uint64, error) {
+func GetOutputSizeByAddress(to btcutil.Address) (int64, error) {
 	switch addr := to.(type) {
 	case *btcutil.AddressTaproot:
 		if addr == nil {
@@ -145,16 +137,16 @@
 }
 
 // OutboundSizeDepositor returns outbound size (68vB) incurred by the depositor
-func OutboundSizeDepositor() uint64 {
+func OutboundSizeDepositor() int64 {
 	return bytesPerInput + bytesPerWitness/blockchain.WitnessScaleFactor
 }
 
 // OutboundSizeWithdrawer returns outbound size (177vB) incurred by the withdrawer (1 input, 3 outputs)
-func OutboundSizeWithdrawer() uint64 {
+func OutboundSizeWithdrawer() int64 {
 	bytesWiredTx := WiredTxSize(1, 3)
-	bytesInput := uint64(1) * bytesPerInput         // nonce mark
-	bytesOutput := uint64(2) * bytesPerOutputP2WPKH // 2 P2WPKH outputs: new nonce mark, change
-	bytesOutput += bytesPerOutputAvg                // 1 output to withdrawer's address
+	bytesInput := int64(1) * bytesPerInput         // nonce mark
+	bytesOutput := int64(2) * bytesPerOutputP2WPKH // 2 P2WPKH outputs: new nonce mark, change
+	bytesOutput += bytesPerOutputAvg               // 1 output to withdrawer's address
 
 	return bytesWiredTx + bytesInput + bytesOutput + bytes1stWitness/blockchain.WitnessScaleFactor
 }
@@ -255,7 +247,7 @@
 
 // GetRecentFeeRate gets the highest fee rate from recent blocks
 // Note: this method should be used for testnet ONLY
-func GetRecentFeeRate(ctx context.Context, rpc RPC, netParams *chaincfg.Params) (uint64, error) {
+func GetRecentFeeRate(ctx context.Context, rpc RPC, netParams *chaincfg.Params) (int64, error) {
 	// should avoid using this method for mainnet
 	if netParams.Name == chaincfg.MainNetParams.Name {
 		return 0, errors.New("GetRecentFeeRate should not be used for mainnet")
@@ -295,6 +287,5 @@
 		highestRate = defaultTestnetFeeRate
 	}
 
-	// #nosec G115 always in range
-	return uint64(highestRate), nil
+	return highestRate, nil
 }