From 508669436c5cbb70184175f9f1204414cf91a3ea Mon Sep 17 00:00:00 2001 From: Charlie Chen Date: Wed, 21 Feb 2024 22:31:37 -0600 Subject: [PATCH] fix some issues in e2e tests --- changelog.md | 1 + cmd/zetaclientd/init.go | 2 + zetaclient/bitcoin/bitcoin_client.go | 130 +++++++++++++++++---------- zetaclient/bitcoin/bitcoin_signer.go | 19 ++-- zetaclient/bitcoin/utils.go | 7 +- zetaclient/config/types.go | 2 +- zetaclient/evm/inbounds.go | 2 +- zetaclient/testutils/utils.go | 15 +++- 8 files changed, 116 insertions(+), 62 deletions(-) diff --git a/changelog.md b/changelog.md index f2fe3e0815..6f012cedf2 100644 --- a/changelog.md +++ b/changelog.md @@ -15,6 +15,7 @@ ### Features *[1728] (https://github.com/zeta-chain/node/pull/1728) - allow aborted transactions to be refunded by minting tokens to zEvm. +*[1789] (https://github.com/zeta-chain/node/pull/1789) - block cross-chain transactions that involved banned addresses ### Refactor * [1766](https://github.com/zeta-chain/node/pull/1766) - Refactors the `PostTxProcessing` EVM hook functionality to deal with invalid withdraw events diff --git a/cmd/zetaclientd/init.go b/cmd/zetaclientd/init.go index 3ae36cf66b..4af5a437b3 100644 --- a/cmd/zetaclientd/init.go +++ b/cmd/zetaclientd/init.go @@ -4,6 +4,7 @@ import ( "github.com/rs/zerolog" "github.com/spf13/cobra" "github.com/zeta-chain/zetacore/zetaclient/config" + "github.com/zeta-chain/zetacore/zetaclient/testutils" ) var InitCmd = &cobra.Command{ @@ -95,6 +96,7 @@ func Initialize(_ *cobra.Command, _ []string) error { configData.KeyringBackend = config.KeyringBackend(initArgs.KeyringBackend) configData.HsmMode = initArgs.HsmMode configData.HsmHotKey = initArgs.HsmHotKey + configData.ComplianceConfig = testutils.ComplianceConfigTest() //Save config file return config.Save(&configData, rootArgs.zetaCoreHome) diff --git a/zetaclient/bitcoin/bitcoin_client.go b/zetaclient/bitcoin/bitcoin_client.go index 0bb741a0e1..4938609014 100644 --- a/zetaclient/bitcoin/bitcoin_client.go +++ b/zetaclient/bitcoin/bitcoin_client.go @@ -513,13 +513,6 @@ func (ob *BTCChainClient) IsSendOutTxProcessed(cctx *types.CrossChainTx, logger res, included := ob.includedTxResults[outTxID] ob.Mu.Unlock() - // compliance check, special handling the cancelled cctx - banned := clientcommon.IsCctxBanned(cctx) - if banned { - params.Amount = cosmosmath.ZeroUint() // use 0 to bypass the amount check - logger.Warn().Msgf("IsSendOutTxProcessed: special handling banned outTx %s", outTxID) - } - if !included { if !broadcasted { return false, false, nil @@ -534,7 +527,7 @@ func (ob *BTCChainClient) IsSendOutTxProcessed(cctx *types.CrossChainTx, logger } // Try including this outTx broadcasted by myself - txResult, inMempool := ob.checkIncludedTx(txnHash, params) + txResult, inMempool := ob.checkIncludedTx(cctx, txnHash) if txResult == nil { // check failed, try again next time return false, false, nil } else if inMempool { // still in mempool (should avoid unnecessary Tss keysign) @@ -1107,16 +1100,9 @@ func (ob *BTCChainClient) observeOutTx() { ob.logger.ObserveOutTx.Info().Err(err).Msgf("observeOutTx: can't find cctx for nonce %d", tracker.Nonce) break } - params := *cctx.GetCurrentOutTxParam() - - // compliance check, special handling the cancelled cctx - banned := clientcommon.IsCctxBanned(cctx) - if banned { - params.Amount = cosmosmath.ZeroUint() // use 0 to bypass the amount check - ob.logger.ObserveOutTx.Warn().Msgf("observeOutTx: special handling banned outTx %s", outTxID) - } - if tracker.Nonce != params.OutboundTxTssNonce { // Tanmay: it doesn't hurt to check - ob.logger.ObserveOutTx.Error().Msgf("observeOutTx: tracker nonce %d not match cctx nonce %d", tracker.Nonce, params.OutboundTxTssNonce) + nonce := cctx.GetCurrentOutTxParam().OutboundTxTssNonce + if tracker.Nonce != nonce { // Tanmay: it doesn't hurt to check + ob.logger.ObserveOutTx.Error().Msgf("observeOutTx: tracker nonce %d not match cctx nonce %d", tracker.Nonce, nonce) break } if len(tracker.HashList) > 1 { @@ -1127,7 +1113,7 @@ func (ob *BTCChainClient) observeOutTx() { txCount := 0 var txResult *btcjson.GetTransactionResult for _, txHash := range tracker.HashList { - result, inMempool := ob.checkIncludedTx(txHash.TxHash, params) + result, inMempool := ob.checkIncludedTx(cctx, txHash.TxHash) if result != nil && !inMempool { // included txCount++ txResult = result @@ -1155,8 +1141,8 @@ func (ob *BTCChainClient) observeOutTx() { // checkIncludedTx checks if a txHash is included and returns (txResult, inMempool) // Note: if txResult is nil, then inMempool flag should be ignored. -func (ob *BTCChainClient) checkIncludedTx(txHash string, params types.OutboundTxParams) (*btcjson.GetTransactionResult, bool) { - outTxID := ob.GetTxID(params.OutboundTxTssNonce) +func (ob *BTCChainClient) checkIncludedTx(cctx *types.CrossChainTx, txHash string) (*btcjson.GetTransactionResult, bool) { + outTxID := ob.GetTxID(cctx.GetCurrentOutTxParam().OutboundTxTssNonce) hash, getTxResult, err := ob.GetTxResultByHash(txHash) if err != nil { ob.logger.ObserveOutTx.Error().Err(err).Msgf("checkIncludedTx: error GetTxResultByHash: %s", txHash) @@ -1167,7 +1153,7 @@ func (ob *BTCChainClient) checkIncludedTx(txHash string, params types.OutboundTx return nil, false } if getTxResult.Confirmations >= 0 { // check included tx only - err = ob.checkTssOutTxResult(hash, getTxResult, params, params.OutboundTxTssNonce) + err = ob.checkTssOutTxResult(cctx, hash, getTxResult) if err != nil { ob.logger.ObserveOutTx.Error().Err(err).Msgf("checkIncludedTx: error verify bitcoin outTx %s outTxID %s", txHash, outTxID) return nil, false @@ -1228,7 +1214,9 @@ func (ob *BTCChainClient) removeIncludedTx(nonce uint64) { // - check if all inputs are segwit && TSS inputs // // Returns: true if outTx passes basic checks. -func (ob *BTCChainClient) checkTssOutTxResult(hash *chainhash.Hash, res *btcjson.GetTransactionResult, params types.OutboundTxParams, nonce uint64) error { +func (ob *BTCChainClient) checkTssOutTxResult(cctx *types.CrossChainTx, hash *chainhash.Hash, res *btcjson.GetTransactionResult) error { + params := cctx.GetCurrentOutTxParam() + nonce := params.OutboundTxTssNonce rawResult, err := ob.getRawTxResult(hash, res) if err != nil { return errors.Wrapf(err, "checkTssOutTxResult: error GetRawTxResultByHash %s", hash.String()) @@ -1237,7 +1225,13 @@ func (ob *BTCChainClient) checkTssOutTxResult(hash *chainhash.Hash, res *btcjson if err != nil { return errors.Wrapf(err, "checkTssOutTxResult: invalid TSS Vin in outTx %s nonce %d", hash, nonce) } - err = ob.checkTSSVout(rawResult.Vout, params, nonce) + + // differentiate between normal and banned cctx + if clientcommon.IsCctxBanned(cctx) { + err = ob.checkTSSVoutCancelled(params, rawResult.Vout) + } else { + err = ob.checkTSSVout(params, rawResult.Vout) + } if err != nil { return errors.Wrapf(err, "checkTssOutTxResult: invalid TSS Vout in outTx %s nonce %d", hash, nonce) } @@ -1316,41 +1310,50 @@ func (ob *BTCChainClient) checkTSSVin(vins []btcjson.Vin, nonce uint64) error { return nil } +// DecodeP2WPKHVout decodes receiver and amount from P2WPKH output +func (ob *BTCChainClient) DecodeP2WPKHVout(vout btcjson.Vout) (string, int64, error) { + amount, err := GetSatoshis(vout.Value) + if err != nil { + return "", 0, errors.Wrap(err, "error getting satoshis") + } + // decode P2WPKH scriptPubKey + scriptPubKey := vout.ScriptPubKey.Hex + decodedScriptPubKey, err := hex.DecodeString(scriptPubKey) + if err != nil { + return "", 0, errors.Wrapf(err, "error decoding scriptPubKey %s", scriptPubKey) + } + if len(decodedScriptPubKey) != 22 { // P2WPKH script + return "", 0, fmt.Errorf("unsupported scriptPubKey: %s", scriptPubKey) + } + witnessVersion := decodedScriptPubKey[0] + witnessProgram := decodedScriptPubKey[2:] + if witnessVersion != 0 { + return "", 0, fmt.Errorf("unsupported witness in scriptPubKey %s", scriptPubKey) + } + recvAddress, err := ob.chain.BTCAddressFromWitnessProgram(witnessProgram) + if err != nil { + return "", 0, errors.Wrapf(err, "error getting receiver from witness program %s", witnessProgram) + } + return recvAddress, amount, nil +} + // checkTSSVout vout is valid if: // - The first output is the nonce-mark // - The second output is the correct payment to recipient // - The third output is the change to TSS (optional) -func (ob *BTCChainClient) checkTSSVout(vouts []btcjson.Vout, params types.OutboundTxParams, nonce uint64) error { +func (ob *BTCChainClient) checkTSSVout(params *types.OutboundTxParams, vouts []btcjson.Vout) error { // vouts: [nonce-mark, payment to recipient, change to TSS (optional)] if !(len(vouts) == 2 || len(vouts) == 3) { return fmt.Errorf("checkTSSVout: invalid number of vouts: %d", len(vouts)) } + nonce := params.OutboundTxTssNonce tssAddress := ob.Tss.BTCAddress() for _, vout := range vouts { - amount, err := GetSatoshis(vout.Value) - if err != nil { - return errors.Wrap(err, "checkTSSVout: error getting satoshis") - } - // decode P2WPKH scriptPubKey - scriptPubKey := vout.ScriptPubKey.Hex - decodedScriptPubKey, err := hex.DecodeString(scriptPubKey) - if err != nil { - return errors.Wrapf(err, "checkTSSVout: error decoding scriptPubKey %s", scriptPubKey) - } - if len(decodedScriptPubKey) != 22 { // P2WPKH script - return fmt.Errorf("checkTSSVout: unsupported scriptPubKey: %s", scriptPubKey) - } - witnessVersion := decodedScriptPubKey[0] - witnessProgram := decodedScriptPubKey[2:] - if witnessVersion != 0 { - return fmt.Errorf("checkTSSVout: unsupported witness in scriptPubKey %s", scriptPubKey) - } - recvAddress, err := ob.chain.BTCAddressFromWitnessProgram(witnessProgram) + recvAddress, amount, err := ob.DecodeP2WPKHVout(vout) if err != nil { - return errors.Wrapf(err, "checkTSSVout: error getting receiver from witness program %s", witnessProgram) + return errors.Wrap(err, "checkTSSVout: error decoding P2WPKH vout") } - // 1st vout: nonce-mark if vout.N == 0 { if recvAddress != tssAddress { @@ -1380,6 +1383,41 @@ func (ob *BTCChainClient) checkTSSVout(vouts []btcjson.Vout, params types.Outbou return nil } +// checkTSSVoutCancelled vout is valid if: +// - The first output is the nonce-mark +// - The second output is the change to TSS (optional) +func (ob *BTCChainClient) checkTSSVoutCancelled(params *types.OutboundTxParams, vouts []btcjson.Vout) error { + // vouts: [nonce-mark, change to TSS (optional)] + if !(len(vouts) == 1 || len(vouts) == 2) { + return fmt.Errorf("checkTSSVoutCancelled: invalid number of vouts: %d", len(vouts)) + } + + nonce := params.OutboundTxTssNonce + tssAddress := ob.Tss.BTCAddress() + for _, vout := range vouts { + recvAddress, amount, err := ob.DecodeP2WPKHVout(vout) + if err != nil { + return errors.Wrap(err, "checkTSSVoutCancelled: error decoding P2WPKH vout") + } + // 1st vout: nonce-mark + if vout.N == 0 { + if recvAddress != tssAddress { + return fmt.Errorf("checkTSSVoutCancelled: nonce-mark address %s not match TSS address %s", recvAddress, tssAddress) + } + if amount != common.NonceMarkAmount(nonce) { + return fmt.Errorf("checkTSSVoutCancelled: nonce-mark amount %d not match nonce-mark amount %d", amount, common.NonceMarkAmount(nonce)) + } + } + // 2nd vout: change to TSS (optional) + if vout.N == 2 { + if recvAddress != tssAddress { + return fmt.Errorf("checkTSSVoutCancelled: change address %s not match TSS address %s", recvAddress, tssAddress) + } + } + } + return nil +} + func (ob *BTCChainClient) BuildBroadcastedTxMap() error { var broadcastedTransactions []clienttypes.OutTxHashSQLType if err := ob.db.Find(&broadcastedTransactions).Error; err != nil { diff --git a/zetaclient/bitcoin/bitcoin_signer.go b/zetaclient/bitcoin/bitcoin_signer.go index 09124fea45..7fbb5b621d 100644 --- a/zetaclient/bitcoin/bitcoin_signer.go +++ b/zetaclient/bitcoin/bitcoin_signer.go @@ -81,6 +81,7 @@ func (signer *BTCSigner) SignWithdrawTx( height uint64, nonce uint64, chain *common.Chain, + cancelTx bool, ) (*wire.MsgTx, error) { estimateFee := float64(gasPrice.Uint64()*outTxBytesMax) / 1e8 nonceMark := common.NonceMarkAmount(nonce) @@ -160,12 +161,14 @@ func (signer *BTCSigner) SignWithdrawTx( tx.AddTxOut(txOut1) // 2nd output: the payment to the recipient - pkScript, err := PayToWitnessPubKeyHashScript(to.WitnessProgram()) - if err != nil { - return nil, err + if !cancelTx { + pkScript, err := PayToWitnessPubKeyHashScript(to.WitnessProgram()) + if err != nil { + return nil, err + } + txOut2 := wire.NewTxOut(amountSatoshis, pkScript) + tx.AddTxOut(txOut2) } - txOut2 := wire.NewTxOut(amountSatoshis, pkScript) - tx.AddTxOut(txOut2) // 3rd output: the remaining btc to TSS self if remainingSats > 0 { @@ -322,12 +325,13 @@ func (signer *BTCSigner) TryProcessOutTx( gasprice.Add(gasprice, satPerByte) // compliance check - if clientcommon.IsCctxBanned(cctx) { + cancelTx := clientcommon.IsCctxBanned(cctx) + if cancelTx { logMsg := fmt.Sprintf("Banned address detected in cctx: sender %s receiver %s chain %d nonce %d", cctx.InboundTxParams.Sender, to, params.ReceiverChainId, outboundTxTssNonce) logger.Warn().Msg(logMsg) signer.loggerCompliance.Warn().Msg(logMsg) - amount = 0 // zero out the amount to cancel the tx + amount = 0.0 // zero out the amount to cancel the tx } logger.Info().Msgf("SignWithdrawTx: to %s, value %d sats", addr.EncodeAddress(), params.Amount.Uint64()) @@ -342,6 +346,7 @@ func (signer *BTCSigner) TryProcessOutTx( height, outboundTxTssNonce, &btcClient.chain, + cancelTx, ) if err != nil { logger.Warn().Err(err).Msgf("SignOutboundTx error: nonce %d chain %d", outboundTxTssNonce, params.ReceiverChainId) diff --git a/zetaclient/bitcoin/utils.go b/zetaclient/bitcoin/utils.go index 2b90e21280..e6acae0763 100644 --- a/zetaclient/bitcoin/utils.go +++ b/zetaclient/bitcoin/utils.go @@ -20,7 +20,6 @@ import ( ) const ( - satoshiPerBitcoin = 1e8 bytesPerKB = 1000 bytesEmptyTx = 10 // an empty tx is about 10 bytes bytesPerInput = 41 // each input is about 41 bytes @@ -60,7 +59,7 @@ func PrettyPrintStruct(val interface{}) (string, error) { // FeeRateToSatPerByte converts a fee rate in BTC/KB to sat/byte. func FeeRateToSatPerByte(rate float64) *big.Int { // #nosec G701 always in range - satPerKB := new(big.Int).SetInt64(int64(rate * satoshiPerBitcoin)) + satPerKB := new(big.Int).SetInt64(int64(rate * btcutil.SatoshiPerBitcoin)) return new(big.Int).Div(satPerKB, big.NewInt(bytesPerKB)) } @@ -102,7 +101,7 @@ func SegWitTxSizeWithdrawer() uint64 { // DepositorFee calculates the depositor fee in BTC for a given sat/byte fee rate // Note: the depositor fee is charged in order to cover the cost of spending the deposited UTXO in the future func DepositorFee(satPerByte int64) float64 { - return float64(satPerByte) * float64(BtcOutTxBytesDepositor) / satoshiPerBitcoin + return float64(satPerByte) * float64(BtcOutTxBytesDepositor) / btcutil.SatoshiPerBitcoin } // CalcBlockAvgFeeRate calculates the average gas rate (in sat/vByte) for a given block @@ -209,7 +208,7 @@ func GetSatoshis(btc float64) (int64, error) { case btc < 0.0: return 0, errors.New("cannot be less than zero") } - return round(btc * satoshiPerBitcoin), nil + return round(btc * btcutil.SatoshiPerBitcoin), nil } func round(f float64) int64 { diff --git a/zetaclient/config/types.go b/zetaclient/config/types.go index 6756fbd8c4..e936f7dcb2 100644 --- a/zetaclient/config/types.go +++ b/zetaclient/config/types.go @@ -163,7 +163,7 @@ func (c *Config) GetBannedAddressBook() map[string]bool { bannedAddresses := make(map[string]bool) if c.ComplianceConfig != nil { for _, address := range c.ComplianceConfig.BannedAddresses { - if ethcommon.IsHexAddress(address) { + if address != "" { bannedAddresses[strings.ToLower(address)] = true } } diff --git a/zetaclient/evm/inbounds.go b/zetaclient/evm/inbounds.go index 5616d177d9..8264e0e3dd 100644 --- a/zetaclient/evm/inbounds.go +++ b/zetaclient/evm/inbounds.go @@ -303,7 +303,7 @@ func (ob *ChainClient) GetInboundVoteMsgForDepositedEvent(event *erc20custody.ER if err == nil && parsedAddress != (ethcommon.Address{}) { maybeReceiver = parsedAddress.Hex() } - if config.AnyBannedAddress(sender.Hex(), maybeReceiver) { + if config.AnyBannedAddress(sender.Hex(), clienttypes.BytesToEthHex(event.Recipient), maybeReceiver) { logMsg := fmt.Sprintf("Banned address detected in ERC20 Deposited event: sender %s receiver %s tx %s chain %d", sender, maybeReceiver, event.Raw.TxHash, ob.chain.ChainId) ob.logger.ExternalChainWatcher.Warn().Msg(logMsg) diff --git a/zetaclient/testutils/utils.go b/zetaclient/testutils/utils.go index e33bd4cef8..2c80b38b35 100644 --- a/zetaclient/testutils/utils.go +++ b/zetaclient/testutils/utils.go @@ -7,14 +7,17 @@ import ( "github.com/btcsuite/btcd/btcjson" "github.com/cosmos/cosmos-sdk/types" + "github.com/zeta-chain/zetacore/zetaclient/config" "github.com/zeta-chain/zetacore/zetaclient/keys" "github.com/zeta-chain/zetacore/zetaclient/zetabridge" ) const ( - TestDataPathEVM = "testdata/evm" - TestDataPathBTC = "testdata/btc" - TestDataPathCctx = "testdata/cctx" + TestDataPathEVM = "testdata/evm" + TestDataPathBTC = "testdata/btc" + TestDataPathCctx = "testdata/cctx" + BannedEVMAddressTest = "0x8a81Ba8eCF2c418CAe624be726F505332DF119C6" + BannedBtcAddressTest = "bcrt1qzp4gt6fc7zkds09kfzaf9ln9c5rvrzxmy6qmpp" ) // SaveObjectToJSONFile saves an object to a file in JSON format @@ -66,3 +69,9 @@ func DummyCoreBridge() *zetabridge.ZetaCoreBridge { nil) return bridge } + +func ComplianceConfigTest() *config.ComplianceConfig { + return &config.ComplianceConfig{ + BannedAddresses: []string{BannedEVMAddressTest, BannedBtcAddressTest}, + } +}