From fdc3c5d9678dfb86c04d5b1a53c176b02bf3d427 Mon Sep 17 00:00:00 2001 From: kevinssgh <79858682+kevinssgh@users.noreply.github.com> Date: Wed, 12 Jul 2023 15:06:00 -0400 Subject: [PATCH] fix(zetaclient): fix bitcoin outbound pending utxo state management and fee calculation (#784) * Added log to print Broadcasted bitcoin transaction * Continue regardless of serialization error * added warning if value of a btc output is negative * add number of inputs to log when remaindervalue < 0 * fix linter complaint * add one more log to pending UTXOs * increased fee to 10000 stat and used gasPrice for fee calculation (#796) Co-authored-by: charliec * fix: enforced minFee (#797) * increased fee to 10000 stat and used gasPrice for fee calculation * enforced minFee --------- Co-authored-by: charliec * fix: cleaned up pending utxo (#801) * removed pending utxo logic * fix lint for unused func * fix lint pre-allocating --------- Co-authored-by: charliec --------- Co-authored-by: brewmaster012 <> Co-authored-by: brewmaster012 <88689859+brewmaster012@users.noreply.github.com> Co-authored-by: Charlie Chen <34498985+ws4charlie@users.noreply.github.com> Co-authored-by: charliec --- .gitignore | 1 + zetaclient/bitcoin_client.go | 94 +++-------------------------- zetaclient/btc_signer.go | 76 +++++++++-------------- zetaclient/btc_test.go | 75 ++--------------------- zetaclient/btc_util.go | 6 -- zetaclient/telemetry.go | 6 +- zetaclient/types/sql_btc.go | 7 +-- zetaclient/types/telemetry_types.go | 5 +- 8 files changed, 49 insertions(+), 221 deletions(-) diff --git a/.gitignore b/.gitignore index 02e5c1133f..9dba8ee04e 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,7 @@ .DS_Store .env .idea/ +.vscode/ .secrets .actrc .zetacored/ diff --git a/zetaclient/bitcoin_client.go b/zetaclient/bitcoin_client.go index e926a4a56c..5659a7113d 100644 --- a/zetaclient/bitcoin_client.go +++ b/zetaclient/bitcoin_client.go @@ -2,9 +2,10 @@ package zetaclient import ( "bytes" - "cosmossdk.io/math" "encoding/hex" "fmt" + + "cosmossdk.io/math" "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/pkg/errors" "gorm.io/driver/sqlite" @@ -580,68 +581,16 @@ func (ob *BitcoinChainClient) fetchUTXOS() error { // fmt.Printf(" confirmations: %d\n", utxo.Confirmations) //} } - // filter pending - var filtered []btcjson.ListUnspentResult - for _, utxo := range utxos { - pending, err := ob.isPending(utxoKey(utxo)) - if err != nil { - return fmt.Errorf("btc: error accessing pending utxos pendingUtxos: %v", err.Error()) - } - if !pending { - filtered = append(filtered, utxo) - } - } - ob.ts.SetNumberOfUTXOs(len(utxos), len(filtered)) + + ob.ts.SetNumberOfUTXOs(len(utxos)) // sort by value - sort.Slice(filtered, func(i, j int) bool { - return filtered[i].Amount < filtered[j].Amount + sort.SliceStable(utxos, func(i, j int) bool { + return utxos[i].Amount < utxos[j].Amount }) - ob.utxos = filtered - // remove completed from pending pendingUtxos - ob.housekeepPending() + ob.utxos = utxos return nil } -func (ob *BitcoinChainClient) housekeepPending() { - // create map with utxos - utxosMap := make(map[string]bool, len(ob.utxos)) - for _, utxo := range ob.utxos { - utxosMap[utxoKey(utxo)] = true - } - - // traverse pending pendingUtxos - removed := 0 - var utxos []clienttypes.PendingUTXOSQLType - if err := ob.db.Find(&utxos).Error; err != nil { - ob.logger.WatchUTXOS.Error().Err(err).Msg("error querying pending UTXOs from db") - return - } - for i := range utxos { - key := utxos[i].Key - // if key not in utxos map, remove from pendingUtxos - if !utxosMap[key] { - if err := ob.db.Where("Key = ?", key).Delete(&utxos[i]).Error; err != nil { - ob.logger.WatchUTXOS.Warn().Err(err).Msgf("btc: error removing key [%s] from pending utxos pendingUtxos", key) - continue - } - removed++ - } - } - if removed > 0 { - ob.logger.WatchUTXOS.Info().Msgf("btc : %d txs purged from pending pendingUtxos", removed) - } -} - -func (ob *BitcoinChainClient) isPending(utxoKey string) (bool, error) { - if _, err := getPendingUTXO(ob.db, utxoKey); err != nil { - if err == gorm.ErrRecordNotFound { - return false, nil - } - return false, err - } - return true, nil -} - func (ob *BitcoinChainClient) observeOutTx() { ticker := time.NewTicker(time.Duration(ob.GetChainConfig().CoreParams.OutTxTicker) * time.Second) for { @@ -689,26 +638,6 @@ func (ob *BitcoinChainClient) observeOutTx() { } } -func getPendingUTXO(db *gorm.DB, key string) (*btcjson.ListUnspentResult, error) { - var utxo clienttypes.PendingUTXOSQLType - if err := db.Where("Key = ?", key).First(&utxo).Error; err != nil { - return nil, err - } - return &utxo.UTXO, nil -} - -func (ob *BitcoinChainClient) BuildPendingUTXOList() error { - var pendingUtxos []clienttypes.PendingUTXOSQLType - if err := ob.db.Find(&pendingUtxos).Error; err != nil { - ob.logger.ChainLogger.Error().Err(err).Msg("error iterating over db") - return err - } - for _, entry := range pendingUtxos { - ob.utxos = append(ob.utxos, entry.UTXO) - } - return nil -} - func (ob *BitcoinChainClient) BuildSubmittedTxMap() error { var submittedTransactions []clienttypes.TransactionResultSQLType if err := ob.db.Find(&submittedTransactions).Error; err != nil { @@ -808,20 +737,13 @@ func (ob *BitcoinChainClient) loadDB(dbpath string) error { } ob.db = db - err = db.AutoMigrate(&clienttypes.PendingUTXOSQLType{}, - &clienttypes.TransactionResultSQLType{}, + err = db.AutoMigrate(&clienttypes.TransactionResultSQLType{}, &clienttypes.TransactionHashSQLType{}, &clienttypes.LastBlockSQLType{}) if err != nil { return err } - //Load pending utxos - err = ob.BuildPendingUTXOList() - if err != nil { - return err - } - //Load submitted transactions err = ob.BuildSubmittedTxMap() if err != nil { diff --git a/zetaclient/btc_signer.go b/zetaclient/btc_signer.go index b91330d530..c4c08f14b8 100644 --- a/zetaclient/btc_signer.go +++ b/zetaclient/btc_signer.go @@ -1,6 +1,7 @@ package zetaclient import ( + "bytes" "encoding/hex" "fmt" "math/big" @@ -45,25 +46,19 @@ func NewBTCSigner(tssSigner TSSSigner, rpcClient *rpcclient.Client, logger zerol } // SignWithdrawTx receives utxos sorted by value, amount in BTC, feeRate in BTC per Kb -func (signer *BTCSigner) SignWithdrawTx(to *btcutil.AddressWitnessPubKeyHash, amount float64, feeRate float64, utxos []btcjson.ListUnspentResult, db *gorm.DB, height uint64) (*wire.MsgTx, error) { +func (signer *BTCSigner) SignWithdrawTx(to *btcutil.AddressWitnessPubKeyHash, amount float64, gasPrice *big.Int, utxos []btcjson.ListUnspentResult, db *gorm.DB, height uint64) (*wire.MsgTx, error) { var total float64 - var prevOuts []btcjson.ListUnspentResult + prevOuts := make([]btcjson.ListUnspentResult, 0, len(utxos)) // select N utxo sufficient to cover the amount //estimateFee := size (100 inputs + 2 output) * feeRate - estimateFee := 0.00001 // FIXME: proper fee estimation + estimateFee := 0.0001 // 10,000 sats, should be good for testnet + minFee := 0.00005 for _, utxo := range utxos { - // check for pending utxos - if _, err := getPendingUTXO(db, utxoKey(utxo)); err != nil { - if err == gorm.ErrRecordNotFound { - total = total + utxo.Amount - prevOuts = append(prevOuts, utxo) + total = total + utxo.Amount + prevOuts = append(prevOuts, utxo) - if total >= amount+estimateFee { - break - } - } else { - return nil, err - } + if total >= amount+estimateFee { + break } } if total < amount { @@ -87,10 +82,11 @@ func (signer *BTCSigner) SignWithdrawTx(to *btcutil.AddressWitnessPubKeyHash, am return nil, err } // add txout with remaining btc - btcFees := float64(tx.SerializeSize()) * feeRate / 1024 //FIXME: feeRate KB is 1000B or 1024B? - fees, err := getSatoshis(btcFees) - if err != nil { - return nil, err + fees := new(big.Int).Mul(big.NewInt(int64(tx.SerializeSize())), gasPrice) + fees.Div(fees, big.NewInt(1000)) //FIXME: feeRate KB is 1000B or 1024B? + if fees.Int64() < int64(minFee*1e8) { + fmt.Printf("fees %d is less than minFee %f; use minFee", fees, minFee*1e8) + fees = big.NewInt(int64(minFee * 1e8)) } tssAddrWPKH := signer.tssSigner.BTCAddressWitnessPubkeyHash() @@ -103,7 +99,15 @@ func (signer *BTCSigner) SignWithdrawTx(to *btcutil.AddressWitnessPubKeyHash, am return nil, err } txOut := wire.NewTxOut(remainingSatoshis, pkScript2) - txOut.Value = remainingSatoshis - fees + + remainderValue := remainingSatoshis - fees.Int64() + if remainderValue < 0 { + fmt.Printf("BTCSigner: SignWithdrawTx: Remainder Value is negative! : %d\n", remainderValue) + fmt.Printf("BTCSigner: SignWithdrawTx: Number of inputs : %d\n", len(tx.TxIn)) + return nil, fmt.Errorf("remainder value is negative") + } + + txOut.Value = remainderValue tx.AddTxOut(txOut) // add txout @@ -158,18 +162,17 @@ func (signer *BTCSigner) SignWithdrawTx(to *btcutil.AddressWitnessPubKeyHash, am txWitness := wire.TxWitness{append(sig.Serialize(), byte(hashType)), pkCompressed} tx.TxIn[ix].Witness = txWitness } - - // update pending utxos pendingUtxos - err = signer.updatePendingUTXOs(db, prevOuts) - if err != nil { - return nil, err - } return tx, nil } func (signer *BTCSigner) Broadcast(signedTx *wire.MsgTx) error { fmt.Printf("BTCSigner: Broadcasting: %s\n", signedTx.TxHash().String()) + var outBuff bytes.Buffer + _ = signedTx.Serialize(&outBuff) + str := hex.EncodeToString(outBuff.Bytes()) + fmt.Printf("BTCSigner: Transaction Data: %s\n", str) + hash, err := signer.rpcClient.SendRawTransaction(signedTx, true) if err != nil { return err @@ -235,10 +238,9 @@ func (signer *BTCSigner) TryProcessOutTx(send *types.CrossChainTx, outTxMan *Out return } - logger.Info().Msgf("SignWithdrawTx: to %s, value %d", addr.EncodeAddress(), send.GetCurrentOutTxParam().Amount.Uint64()/1e8) + logger.Info().Msgf("SignWithdrawTx: to %s, value %d sats", addr.EncodeAddress(), send.GetCurrentOutTxParam().Amount.Uint64()) logger.Info().Msgf("using utxos: %v", btcClient.utxos) - // FIXME: gas price? - tx, err := signer.SignWithdrawTx(to, float64(send.GetCurrentOutTxParam().Amount.Uint64())/1e8, float64(gasprice.Int64())/1e8*1024, btcClient.utxos, btcClient.db, height) + tx, err := signer.SignWithdrawTx(to, float64(send.GetCurrentOutTxParam().Amount.Uint64())/1e8, gasprice, btcClient.utxos, btcClient.db, height) if err != nil { logger.Warn().Err(err).Msgf("SignOutboundTx error: nonce %d chain %d", send.GetCurrentOutTxParam().OutboundTxTssNonce, send.GetCurrentOutTxParam().ReceiverChainId) return @@ -284,22 +286,4 @@ func (signer *BTCSigner) TryProcessOutTx(send *types.CrossChainTx, outTxMan *Out } } - //} - -} - -func (signer *BTCSigner) updatePendingUTXOs(db *gorm.DB, utxos []btcjson.ListUnspentResult) error { - for _, utxo := range utxos { - // Try to find existing record in DB to populate primary key - var pendingUTXO clienttypes.PendingUTXOSQLType - db.Where("Key = ?", utxoKey(utxo)).First(&pendingUTXO) - - // If record doesn't exist, it will be created by the Save function - pendingUTXO.UTXO = utxo - pendingUTXO.Key = utxoKey(utxo) - if err := db.Save(&pendingUTXO).Error; err != nil { - return err - } - } - return nil } diff --git a/zetaclient/btc_test.go b/zetaclient/btc_test.go index 797caca7e8..705eecb696 100644 --- a/zetaclient/btc_test.go +++ b/zetaclient/btc_test.go @@ -4,14 +4,14 @@ import ( "bytes" "encoding/hex" "fmt" - "github.com/btcsuite/btcd/btcjson" - clienttypes "github.com/zeta-chain/zetacore/zetaclient/types" - "gorm.io/driver/sqlite" - "gorm.io/gorm" "math/big" "strconv" "testing" + "github.com/btcsuite/btcd/btcjson" + "gorm.io/driver/sqlite" + "gorm.io/gorm" + "github.com/stretchr/testify/suite" "github.com/btcsuite/btcd/btcec" @@ -55,8 +55,6 @@ func (suite *BTCSignTestSuite) SetupTest() { suite.NoError(err) suite.db = db - err = db.AutoMigrate(&clienttypes.PendingUTXOSQLType{}) - suite.NoError(err) //Create UTXOs for i := 0; i < utxoCount; i++ { @@ -95,32 +93,6 @@ func (suite *BTCSignTestSuite) TestSign() { suite.T().Logf("tss signed tx : %v\n", tssSignedTX) } -func (suite *BTCSignTestSuite) TestPendingUTXO() { - //Update Pending Utxos - suite.updatePendingUtxos() - - //Remove one and perform housekeeping - suite.utxos = suite.utxos[:len(suite.utxos)-1] - suite.housekeepPending() - - //Modify utxos and update - suite.utxos[0].Amount = 0.0123 - suite.updatePendingUtxos() - - //Get Pending Utxos from db - var haveDB []clienttypes.PendingUTXOSQLType - var have []btcjson.ListUnspentResult - err := suite.db.Find(&haveDB).Error - suite.NoError(err) - for _, utxo := range haveDB { - have = append(have, utxo.UTXO) - } - - //Assert utxos in db are Equal to utxos in memory - want := suite.utxos - suite.Equal(want, have) -} - func (suite *BTCSignTestSuite) TestSubmittedTx() { } @@ -214,42 +186,3 @@ func getTSSTX(tss *TestSigner, tx *wire.MsgTx, sigHashes *txscript.TxSigHashes, tssTX := hex.EncodeToString(buf.Bytes()) return tssTX, nil } - -// Copied housekeepPending from btc_client and updatePendingUtxos from btc_signer since they are private -func (suite *BTCSignTestSuite) housekeepPending() { - // create map with utxos - utxosMap := make(map[string]bool, len(suite.utxos)) - for _, utxo := range suite.utxos { - utxosMap[utxoKey(utxo)] = true - } - - // traverse pending pendingUtxos - removed := 0 - var utxos []clienttypes.PendingUTXOSQLType - err := suite.db.Find(&utxos).Error - suite.NoError(err) - - for _, utxo := range utxos { - key := utxo.Key - // if key not in utxos map, remove from pendingUtxos - if !utxosMap[key] { - err := suite.db.Where("Key = ?", key).Delete(&utxo).Error - suite.NoError(err) - removed++ - } - } -} - -func (suite *BTCSignTestSuite) updatePendingUtxos() { - for _, utxo := range suite.utxos { - // Try to find existing record in DB to populate primary key - var pendingUTXO clienttypes.PendingUTXOSQLType - suite.db.Where("Key = ?", utxoKey(utxo)).First(&pendingUTXO) - - // If record doesn't exist, it will be created by the Save function - pendingUTXO.UTXO = utxo - pendingUTXO.Key = utxoKey(utxo) - err := suite.db.Save(&pendingUTXO).Error - suite.NoError(err) - } -} diff --git a/zetaclient/btc_util.go b/zetaclient/btc_util.go index 6eb94a9f6c..13ebedf561 100644 --- a/zetaclient/btc_util.go +++ b/zetaclient/btc_util.go @@ -2,10 +2,8 @@ package zetaclient import ( "errors" - "fmt" "math" - "github.com/btcsuite/btcd/btcjson" "github.com/btcsuite/btcd/txscript" ) @@ -42,7 +40,3 @@ func round(f float64) int64 { func payToWitnessPubKeyHashScript(pubKeyHash []byte) ([]byte, error) { return txscript.NewScriptBuilder().AddOp(txscript.OP_0).AddData(pubKeyHash).Script() } - -func utxoKey(utxo btcjson.ListUnspentResult) string { - return fmt.Sprintf("%s_%d", utxo.TxID, utxo.Vout) -} diff --git a/zetaclient/telemetry.go b/zetaclient/telemetry.go index aa3d7ab7cf..13de1ebf32 100644 --- a/zetaclient/telemetry.go +++ b/zetaclient/telemetry.go @@ -5,11 +5,12 @@ import ( "encoding/json" "errors" "fmt" - "github.com/zeta-chain/zetacore/common" "net/http" "sync" "time" + "github.com/zeta-chain/zetacore/common" + "github.com/gorilla/mux" "github.com/rs/zerolog" "github.com/rs/zerolog/log" @@ -101,10 +102,9 @@ func (t *TelemetryServer) GetNextNonce() int { return t.status.BTCNextNonce } -func (t *TelemetryServer) SetNumberOfUTXOs(numberOfUTXOs, numberOfFilteredUTXOs int) { +func (t *TelemetryServer) SetNumberOfUTXOs(numberOfUTXOs int) { t.mu.Lock() t.status.BTCNumberOfUTXOs = numberOfUTXOs - t.status.BTCNumberOfFilteredUTXOs = numberOfFilteredUTXOs t.mu.Unlock() } diff --git a/zetaclient/types/sql_btc.go b/zetaclient/types/sql_btc.go index 6c6f9e969d..eccd5c78cd 100644 --- a/zetaclient/types/sql_btc.go +++ b/zetaclient/types/sql_btc.go @@ -2,6 +2,7 @@ package types import ( "encoding/json" + "github.com/btcsuite/btcd/btcjson" "github.com/btcsuite/btcd/chaincfg/chainhash" "gorm.io/gorm" @@ -22,12 +23,6 @@ type TransactionResultDB struct { Hex string } -type PendingUTXOSQLType struct { - gorm.Model - Key string - UTXO btcjson.ListUnspentResult `gorm:"embedded"` -} - type TransactionResultSQLType struct { gorm.Model Key string diff --git a/zetaclient/types/telemetry_types.go b/zetaclient/types/telemetry_types.go index 32dc522b4c..e675516dc6 100644 --- a/zetaclient/types/telemetry_types.go +++ b/zetaclient/types/telemetry_types.go @@ -2,7 +2,6 @@ package types // Status type for telemetry. More fields can be added as needed type Status struct { - BTCNextNonce int `json:"btc_next_nonce"` - BTCNumberOfUTXOs int `json:"btc_number_of_utxos"` - BTCNumberOfFilteredUTXOs int `json:"btc_number_of_filtered_utxos"` + BTCNextNonce int `json:"btc_next_nonce"` + BTCNumberOfUTXOs int `json:"btc_number_of_utxos"` }