fix: fix SegWit tx size estimation (in vByte) and gas fee calculation (…

…#1669) * fix SegWit tx size and gas fee calculation * define outTxBytesMin and outTxBytesMax as constants
zeta-chain · Jan 30, 2024 · 6e660cc · 6e660cc
1 parent be1c040
commit 6e660cc
Show file tree

Hide file tree

Showing 4 changed files with 113 additions and 76 deletions.
diff --git a/changelog.md b/changelog.md
@@ -2,12 +2,13 @@
 
 ## Unreleased
 
-### Fixes
 ### Fixes
 
 * [1642](https://github.com/zeta-chain/node/pull/1642) - Change WhitelistERC20 authorization from group1 to group2
 * [1610](https://github.com/zeta-chain/node/issues/1610) - add pending outtx hash to tracker after monitoring for 10 minutes
 * [1656](https://github.com/zeta-chain/node/issues/1656) - schedule bitcoin keysign with intervals to avoid keysign failures
+* [1661](https://github.com/zeta-chain/node/issues/1661) - use estimated SegWit tx size for Bitcoin gas fee calculation
+* [1667](https://github.com/zeta-chain/node/issues/1667) - estimate SegWit tx size in uinit of vByte
 
 ## Version: v12.1.0
 

diff --git a/zetaclient/btc_signer.go b/zetaclient/btc_signer.go
@@ -23,19 +23,11 @@ import (
 
 const (
 	maxNoOfInputsPerTx = 20
-	consolidationRank  = 10 // the rank below (or equal to) which we consolidate UTXOs
+	consolidationRank  = 10           // the rank below (or equal to) which we consolidate UTXOs
+	outTxBytesMin      = uint64(239)  // 239vB == EstimateSegWitTxSize(2, 3)
+	outTxBytesMax      = uint64(1531) // 1531v == EstimateSegWitTxSize(21, 3)
 )
 
-var (
-	outTxBytesMin uint64
-	outTxBytesMax uint64
-)
-
-func init() {
-	outTxBytesMin = EstimateSegWitTxSize(2, 3)  // 403B, estimated size for a 2-input, 3-output SegWit tx
-	outTxBytesMax = EstimateSegWitTxSize(21, 3) // 3234B, estimated size for a 21-input, 3-output SegWit tx
-}
-
 type BTCSigner struct {
 	tssSigner TSSSigner
 	rpcClient BTCRPCClient
@@ -114,9 +106,9 @@ func (signer *BTCSigner) SignWithdrawTx(
 
 	// size checking
 	// #nosec G701 always positive
-	txSize := uint64(tx.SerializeSize())
-	if txSize > sizeLimit { // ZRC20 'withdraw' charged less fee from end user
-		signer.logger.Info().Msgf("sizeLimit %d is less than txSize %d for nonce %d", sizeLimit, txSize, nonce)
+	txSize := EstimateSegWitTxSize(uint64(len(prevOuts)), 3)
+	if sizeLimit < BtcOutTxBytesWithdrawer { // ZRC20 'withdraw' charged less fee from end user
+		signer.logger.Info().Msgf("sizeLimit %d is less than BtcOutTxBytesWithdrawer %d for nonce %d", sizeLimit, txSize, nonce)
 	}
 	if txSize < outTxBytesMin { // outbound shouldn't be blocked a low sizeLimit
 		signer.logger.Warn().Msgf("txSize %d is less than outTxBytesMin %d; use outTxBytesMin", txSize, outTxBytesMin)

diff --git a/zetaclient/btc_signer_test.go b/zetaclient/btc_signer_test.go
@@ -9,6 +9,7 @@ import (
 	"sync"
 	"testing"
 
+	"github.com/btcsuite/btcd/blockchain"
 	"github.com/btcsuite/btcd/btcec"
 	"github.com/btcsuite/btcd/btcjson"
 	"github.com/btcsuite/btcd/chaincfg"
@@ -31,6 +32,31 @@ type BTCSignerSuite struct {
 
 var _ = Suite(&BTCSignerSuite{})
 
+// 21 example UTXO txids to use in the test.
+var exampleTxids = []string{
+	"c1729638e1c9b6bfca57d11bf93047d98b65594b0bf75d7ee68bf7dc80dc164e",
+	"54f9ebbd9e3ad39a297da54bf34a609b6831acbea0361cb5b7b5c8374f5046aa",
+	"b18a55a34319cfbedebfcfe1a80fef2b92ad8894d06caf8293a0344824c2cfbc",
+	"969fb309a4df7c299972700da788b5d601c0c04bab4ab46fff79d0335a7d75de",
+	"6c71913061246ffc20e268c1b0e65895055c36bfbf1f8faf92dcad6f8242121e",
+	"ba6d6e88cb5a97556684a1232719a3ffe409c5c9501061e1f59741bc412b3585",
+	"69b56c3c8c5d1851f9eaec256cd49f290b477a5d43e2aef42ef25d3c1d9f4b33",
+	"b87effd4cb46fe1a575b5b1ba0289313dc9b4bc9e615a3c6cbc0a14186921fdf",
+	"3135433054523f5e220621c9e3d48efbbb34a6a2df65635c2a3e7d462d3e1cda",
+	"8495c22a9ce6359ab53aa048c13b41c64fdf5fe141f516ba2573cc3f9313f06e",
+	"f31583544b475370d7b9187c9a01b92e44fb31ac5fcfa7fc55565ac64043aa9a",
+	"c03d55f9f717c1df978623e2e6b397b720999242f9ead7db9b5988fee3fb3933",
+	"ee55688439b47a5410cdc05bac46be0094f3af54d307456fdfe6ba8caf336e0b",
+	"61895f86c70f0bc3eef55d9a00347b509fa90f7a344606a9774be98a3ee9e02a",
+	"ffabb401a19d04327bd4a076671d48467dbcde95459beeab23df21686fd01525",
+	"b7e1c03b9b73e4e90fc06da893072c5604203c49e66699acbb2f61485d822981",
+	"185614d21973990138e478ce10e0a4014352df58044276d4e4c0093aa140f482",
+	"4a2800f13d15dc0c82308761d6fe8f6d13b65e42d7ca96a42a3a7048830e8c55",
+	"fb98f52e91db500735b185797cebb5848afbfe1289922d87e03b98c3da5b85ef",
+	"7901c5e36d9e8456ac61b29b82048650672a889596cbd30a9f8910a589ffc5b3",
+	"6bcd0850fd2fa1404290ed04d78d4ae718414f16d4fbfd344951add8dcf60326",
+}
+
 func (s *BTCSignerSuite) SetUpTest(c *C) {
 	// test private key with EVM address
 	//// EVM: 0x236C7f53a90493Bb423411fe4117Cb4c2De71DfB
@@ -219,11 +245,10 @@ func generateKeyPair(t *testing.T, net *chaincfg.Params) (*btcec.PrivateKey, []b
 
 func addTxInputs(t *testing.T, tx *wire.MsgTx, txids []string) {
 	preTxSize := tx.SerializeSize()
-	require.Equal(t, bytesEmptyTx, preTxSize)
-	for i, txid := range txids {
+	for _, txid := range txids {
 		hash, err := chainhash.NewHashFromStr(txid)
 		require.Nil(t, err)
-		outpoint := wire.NewOutPoint(hash, uint32(i%3))
+		outpoint := wire.NewOutPoint(hash, uint32(rand.Intn(100)))
 		txIn := wire.NewTxIn(outpoint, nil, nil)
 		tx.AddTxIn(txIn)
 		require.Equal(t, bytesPerInput, tx.SerializeSize()-preTxSize)
@@ -307,63 +332,71 @@ func TestP2WPHSize2In3Out(t *testing.T) {
 	// Payer sign the redeeming transaction.
 	signTx(t, tx, payerScript, privateKey)
 
-	// Estimate the tx size
+	// Estimate the tx size in vByte
 	// #nosec G701 always positive
-	txSize := uint64(tx.SerializeSize())
-	sizeEstimated := EstimateSegWitTxSize(uint64(len(utxosTxids)), 3)
-	require.Equal(t, outTxBytesMin, sizeEstimated)
-	require.True(t, outTxBytesMin >= txSize)
-	require.True(t, outTxBytesMin-txSize <= 2) // 2 witness may vary
+	vBytes := uint64(blockchain.GetTransactionWeight(btcutil.NewTx(tx)) / blockchain.WitnessScaleFactor)
+	vBytesEstimated := EstimateSegWitTxSize(uint64(len(utxosTxids)), 3)
+	require.Equal(t, vBytes, vBytesEstimated)
+	require.Equal(t, vBytes, outTxBytesMin)
 }
 
 func TestP2WPHSize21In3Out(t *testing.T) {
 	// Generate payer/payee private keys and P2WPKH addresss
 	privateKey, payerScript := generateKeyPair(t, &chaincfg.TestNet3Params)
 	_, payeeScript := generateKeyPair(t, &chaincfg.TestNet3Params)
 
-	// 21 example UTXO txids to use in the test.
-	utxosTxids := []string{
-		"c1729638e1c9b6bfca57d11bf93047d98b65594b0bf75d7ee68bf7dc80dc164e",
-		"54f9ebbd9e3ad39a297da54bf34a609b6831acbea0361cb5b7b5c8374f5046aa",
-		"b18a55a34319cfbedebfcfe1a80fef2b92ad8894d06caf8293a0344824c2cfbc",
-		"969fb309a4df7c299972700da788b5d601c0c04bab4ab46fff79d0335a7d75de",
-		"6c71913061246ffc20e268c1b0e65895055c36bfbf1f8faf92dcad6f8242121e",
-		"ba6d6e88cb5a97556684a1232719a3ffe409c5c9501061e1f59741bc412b3585",
-		"69b56c3c8c5d1851f9eaec256cd49f290b477a5d43e2aef42ef25d3c1d9f4b33",
-		"b87effd4cb46fe1a575b5b1ba0289313dc9b4bc9e615a3c6cbc0a14186921fdf",
-		"3135433054523f5e220621c9e3d48efbbb34a6a2df65635c2a3e7d462d3e1cda",
-		"8495c22a9ce6359ab53aa048c13b41c64fdf5fe141f516ba2573cc3f9313f06e",
-		"f31583544b475370d7b9187c9a01b92e44fb31ac5fcfa7fc55565ac64043aa9a",
-		"c03d55f9f717c1df978623e2e6b397b720999242f9ead7db9b5988fee3fb3933",
-		"ee55688439b47a5410cdc05bac46be0094f3af54d307456fdfe6ba8caf336e0b",
-		"61895f86c70f0bc3eef55d9a00347b509fa90f7a344606a9774be98a3ee9e02a",
-		"ffabb401a19d04327bd4a076671d48467dbcde95459beeab23df21686fd01525",
-		"b7e1c03b9b73e4e90fc06da893072c5604203c49e66699acbb2f61485d822981",
-		"185614d21973990138e478ce10e0a4014352df58044276d4e4c0093aa140f482",
-		"4a2800f13d15dc0c82308761d6fe8f6d13b65e42d7ca96a42a3a7048830e8c55",
-		"fb98f52e91db500735b185797cebb5848afbfe1289922d87e03b98c3da5b85ef",
-		"7901c5e36d9e8456ac61b29b82048650672a889596cbd30a9f8910a589ffc5b3",
-		"6bcd0850fd2fa1404290ed04d78d4ae718414f16d4fbfd344951add8dcf60326",
-	}
-
 	// Create a new transaction and add inputs
 	tx := wire.NewMsgTx(wire.TxVersion)
-	require.Equal(t, bytesEmptyTx, tx.SerializeSize())
-	addTxInputs(t, tx, utxosTxids)
+	addTxInputs(t, tx, exampleTxids)
 
 	// Add P2WPKH outputs
 	addTxOutputs(t, tx, payerScript, payeeScript)
 
 	// Payer sign the redeeming transaction.
 	signTx(t, tx, payerScript, privateKey)
 
-	// Estimate the tx size
+	// Estimate the tx size in vByte
 	// #nosec G701 always positive
-	txSize := uint64(tx.SerializeSize())
-	sizeEstimated := EstimateSegWitTxSize(uint64(len(utxosTxids)), 3)
-	require.Equal(t, outTxBytesMax, sizeEstimated)
-	require.True(t, outTxBytesMax >= txSize)
-	require.True(t, outTxBytesMax-txSize <= 21) // 21 witness may vary
+	vError := uint64(21 / 4) // 5 vBytes error tolerance
+	vBytes := uint64(blockchain.GetTransactionWeight(btcutil.NewTx(tx)) / blockchain.WitnessScaleFactor)
+	vBytesEstimated := EstimateSegWitTxSize(uint64(len(exampleTxids)), 3)
+	require.Equal(t, vBytesEstimated, outTxBytesMax)
+	if vBytes > vBytesEstimated {
+		require.True(t, vBytes-vBytesEstimated <= vError)
+	} else {
+		require.True(t, vBytesEstimated-vBytes <= vError)
+	}
+}
+
+func TestP2WPHSizeXIn3Out(t *testing.T) {
+	// Generate payer/payee private keys and P2WPKH addresss
+	privateKey, payerScript := generateKeyPair(t, &chaincfg.TestNet3Params)
+	_, payeeScript := generateKeyPair(t, &chaincfg.TestNet3Params)
+
+	// Create new transactions with X (2 <= X <= 21) inputs and 3 outputs respectively
+	for x := 2; x <= 21; x++ {
+		tx := wire.NewMsgTx(wire.TxVersion)
+		addTxInputs(t, tx, exampleTxids[:x])
+
+		// Add P2WPKH outputs
+		addTxOutputs(t, tx, payerScript, payeeScript)
+
+		// Payer sign the redeeming transaction.
+		signTx(t, tx, payerScript, privateKey)
+
+		// Estimate the tx size
+		// #nosec G701 always positive
+		vError := uint64(0.25 + float64(x)/4) // 1st witness incur 0.25 vByte error, other witness incur 1/4 vByte error tolerance,
+		vBytes := uint64(blockchain.GetTransactionWeight(btcutil.NewTx(tx)) / blockchain.WitnessScaleFactor)
+		vBytesEstimated := EstimateSegWitTxSize(uint64(len(exampleTxids[:x])), 3)
+		if vBytes > vBytesEstimated {
+			require.True(t, vBytes-vBytesEstimated <= vError)
+			//fmt.Printf("%d error percentage: %.2f%%\n", float64(vBytes-vBytesEstimated)/float64(vBytes)*100)
+		} else {
+			require.True(t, vBytesEstimated-vBytes <= vError)
+			//fmt.Printf("error percentage: %.2f%%\n", float64(vBytesEstimated-vBytes)/float64(vBytes)*100)
+		}
+	}
 }
 
 func TestP2WPHSizeBreakdown(t *testing.T) {
@@ -374,14 +407,14 @@ func TestP2WPHSizeBreakdown(t *testing.T) {
 	fmt.Printf("1 input, 1 output: %d\n", sz)
 
 	txSizeDepositor := SegWitTxSizeDepositor()
-	require.Equal(t, uint64(149), txSizeDepositor)
+	require.Equal(t, uint64(68), txSizeDepositor)
 
 	txSizeWithdrawer := SegWitTxSizeWithdrawer()
-	require.Equal(t, uint64(254), txSizeWithdrawer)
-	require.Equal(t, txSize2In3Out, txSizeDepositor+txSizeWithdrawer) // 403 = 149 + 254
+	require.Equal(t, uint64(171), txSizeWithdrawer)
+	require.Equal(t, txSize2In3Out, txSizeDepositor+txSizeWithdrawer) // 239 = 68 + 171
 
-	depositFee := DepositorFee(5)
-	require.Equal(t, depositFee, 0.00000745)
+	depositFee := DepositorFee(20)
+	require.Equal(t, depositFee, 0.00001360)
 }
 
 // helper function to create a new BitcoinChainClient

diff --git a/zetaclient/utils.go b/zetaclient/utils.go
@@ -13,7 +13,9 @@ import (
 	"time"
 
 	sdkmath "cosmossdk.io/math"
+	"github.com/btcsuite/btcd/blockchain"
 	"github.com/btcsuite/btcd/txscript"
+	"github.com/btcsuite/btcd/wire"
 	ethcommon "github.com/ethereum/go-ethereum/common"
 	ethtypes "github.com/ethereum/go-ethereum/core/types"
 	"github.com/pkg/errors"
@@ -36,22 +38,18 @@ const (
 )
 
 var (
-	BtcOutTxBytesMin        uint64
-	BtcOutTxBytesMax        uint64
 	BtcOutTxBytesDepositor  uint64
 	BtcOutTxBytesWithdrawer uint64
 	BtcDepositorFeeMin      float64
 )
 
 func init() {
-	BtcOutTxBytesMin = EstimateSegWitTxSize(2, 3)      // 403B, estimated size for a 2-input, 3-output SegWit tx
-	BtcOutTxBytesMax = EstimateSegWitTxSize(21, 3)     // 3234B, estimated size for a 21-input, 3-output SegWit tx
-	BtcOutTxBytesDepositor = SegWitTxSizeDepositor()   // 149B, the outtx size incurred by the depositor
-	BtcOutTxBytesWithdrawer = SegWitTxSizeWithdrawer() // 254B, the outtx size incurred by the withdrawer
+	BtcOutTxBytesDepositor = SegWitTxSizeDepositor()   // 68vB, the outtx size incurred by the depositor
+	BtcOutTxBytesWithdrawer = SegWitTxSizeWithdrawer() // 171vB, the outtx size incurred by the withdrawer
 
-	// depositor fee calculation is based on a fixed fee rate of 5 sat/byte just for simplicity.
+	// depositor fee calculation is based on a fixed fee rate of 20 sat/byte just for simplicity.
 	// In reality, the fee rate on UTXO deposit is different from the fee rate when the UTXO is spent.
-	BtcDepositorFeeMin = DepositorFee(5) // 0.00000745 (5 * 149B / 100000000), the minimum deposit fee in BTC for 5 sat/byte
+	BtcDepositorFeeMin = DepositorFee(20) // 0.00001360 (20 * 68vB / 100000000), the minimum deposit fee in BTC for 20 sat/byte
 }
 
 func PrettyPrintStruct(val interface{}) (string, error) {
@@ -73,27 +71,40 @@ func FeeRateToSatPerByte(rate float64) *big.Int {
 	return new(big.Int).Div(satPerKB, big.NewInt(bytesPerKB))
 }
 
-// EstimateSegWitTxSize estimates SegWit tx size
+// WiredTxSize calculates the wired tx size in bytes
+func WiredTxSize(numInputs uint64, numOutputs uint64) uint64 {
+	// Version 4 bytes + LockTime 4 bytes + Serialized varint size for the
+	// number of transaction inputs and outputs.
+	// #nosec G701 always positive
+	return uint64(8 + wire.VarIntSerializeSize(numInputs) + wire.VarIntSerializeSize(numOutputs))
+}
+
+// EstimateSegWitTxSize estimates SegWit tx size in vByte
 func EstimateSegWitTxSize(numInputs uint64, numOutputs uint64) uint64 {
 	if numInputs == 0 {
 		return 0
 	}
+	bytesWiredTx := WiredTxSize(numInputs, numOutputs)
 	bytesInput := numInputs * bytesPerInput
 	bytesOutput := numOutputs * bytesPerOutput
 	bytesWitness := bytes1stWitness + (numInputs-1)*bytesPerWitness
-	return bytesEmptyTx + bytesInput + bytesOutput + bytesWitness
+
+	// https://github.com/bitcoin/bips/blob/master/bip-0141.mediawiki#transaction-size-calculations
+	// Calculation for signed SegWit tx: blockchain.GetTransactionWeight(tx) / 4
+	return bytesWiredTx + bytesInput + bytesOutput + bytesWitness/blockchain.WitnessScaleFactor
 }
 
-// SegWitTxSizeDepositor returns SegWit tx size (149B) incurred by the depositor
+// SegWitTxSizeDepositor returns SegWit tx size (68vB) incurred by the depositor
 func SegWitTxSizeDepositor() uint64 {
-	return bytesPerInput + bytesPerWitness
+	return bytesPerInput + bytesPerWitness/blockchain.WitnessScaleFactor
 }
 
-// SegWitTxSizeWithdrawer returns SegWit tx size (254B) incurred by the withdrawer
+// SegWitTxSizeWithdrawer returns SegWit tx size (171vB) incurred by the withdrawer (1 input, 3 outputs)
 func SegWitTxSizeWithdrawer() uint64 {
+	bytesWiredTx := WiredTxSize(1, 3)
 	bytesInput := uint64(1) * bytesPerInput   // nonce mark
 	bytesOutput := uint64(3) * bytesPerOutput // 3 outputs: new nonce mark, payment, change
-	return bytesEmptyTx + bytesInput + bytesOutput + bytes1stWitness
+	return bytesWiredTx + bytesInput + bytesOutput + bytes1stWitness/blockchain.WitnessScaleFactor
 }
 
 // DepositorFee calculates the depositor fee in BTC for a given sat/byte fee rate