resolved some PR review feedback

e2e/e2etests/test_bitcoin_withdraw.go

@@ -12,6 +12,7 @@ import (
 	"github.com/zeta-chain/zetacore/common/bitcoin"
 	"github.com/zeta-chain/zetacore/e2e/runner"
 	"github.com/zeta-chain/zetacore/e2e/utils"
+	crosschaintypes "github.com/zeta-chain/zetacore/x/crosschain/types"
 	"github.com/zeta-chain/zetacore/zetaclient/testutils"
 )
 
@@ -30,7 +31,7 @@ func TestBitcoinWithdrawSegWit(r *runner.E2ERunner, args []string) {
 		panic("Invalid receiver address specified for TestBitcoinWithdrawSegWit.")
 	}
 
-	WithdrawBitcoin(r, receiver, amount)
+	withdrawBTCZRC20(r, receiver, amount)
 }
 
 func TestBitcoinWithdrawTaproot(r *runner.E2ERunner, args []string) {
@@ -48,7 +49,7 @@ func TestBitcoinWithdrawTaproot(r *runner.E2ERunner, args []string) {
 		panic("Invalid receiver address specified for TestBitcoinWithdrawTaproot.")
 	}
 
-	WithdrawBitcoin(r, receiver, amount)
+	withdrawBTCZRC20(r, receiver, amount)
 }
 
 func TestBitcoinWithdrawLegacy(r *runner.E2ERunner, args []string) {
@@ -66,7 +67,7 @@ func TestBitcoinWithdrawLegacy(r *runner.E2ERunner, args []string) {
 		panic("Invalid receiver address specified for TestBitcoinWithdrawLegacy.")
 	}
 
-	WithdrawBitcoin(r, receiver, amount)
+	withdrawBTCZRC20(r, receiver, amount)
 }
 
 func TestBitcoinWithdrawP2WSH(r *runner.E2ERunner, args []string) {
@@ -84,7 +85,7 @@ func TestBitcoinWithdrawP2WSH(r *runner.E2ERunner, args []string) {
 		panic("Invalid receiver address specified for TestBitcoinWithdrawP2WSH.")
 	}
 
-	WithdrawBitcoin(r, receiver, amount)
+	withdrawBTCZRC20(r, receiver, amount)
 }
 
 func TestBitcoinWithdrawP2SH(r *runner.E2ERunner, args []string) {
@@ -102,7 +103,7 @@ func TestBitcoinWithdrawP2SH(r *runner.E2ERunner, args []string) {
 		panic("Invalid receiver address specified for TestBitcoinWithdrawP2SH.")
 	}
 
-	WithdrawBitcoin(r, receiver, amount)
+	withdrawBTCZRC20(r, receiver, amount)
 }
 
 func TestBitcoinWithdrawRestricted(r *runner.E2ERunner, args []string) {
@@ -123,7 +124,7 @@ func TestBitcoinWithdrawRestricted(r *runner.E2ERunner, args []string) {
 
 	r.SetBtcAddress(r.Name, false)
 
-	WithdrawBitcoinRestricted(r, amount)
+	withdrawBitcoinRestricted(r, amount)
 }
 
 func parseBitcoinWithdrawArgs(args []string, defaultReceiver string) (btcutil.Address, *big.Int) {
@@ -185,7 +186,13 @@ func withdrawBTCZRC20(r *runner.E2ERunner, to btcutil.Address, amount *big.Int)
 		panic(err)
 	}
 
+	// get cctx and check status
 	cctx := utils.WaitCctxMinedByInTxHash(r.Ctx, receipt.TxHash.Hex(), r.CctxClient, r.Logger, r.CctxTimeout)
+	if cctx.CctxStatus.Status != crosschaintypes.CctxStatus_OutboundMined {
+		panic(fmt.Errorf("cctx status is not OutboundMined"))
+	}
+
+	// get bitcoin tx according to the outTxHash in cctx
 	outTxHash := cctx.GetCurrentOutTxParam().OutboundTxHash
 	hash, err := chainhash.NewHashFromStr(outTxHash)
 	if err != nil {
@@ -216,11 +223,7 @@ func withdrawBTCZRC20(r *runner.E2ERunner, to btcutil.Address, amount *big.Int)
 	return rawTx
 }
 
-func WithdrawBitcoin(r *runner.E2ERunner, receiver btcutil.Address, amount *big.Int) {
-	withdrawBTCZRC20(r, receiver, amount)
-}
-
-func WithdrawBitcoinRestricted(r *runner.E2ERunner, amount *big.Int) {
+func withdrawBitcoinRestricted(r *runner.E2ERunner, amount *big.Int) {
 	// use restricted BTC P2WPKH address
 	addressRestricted, err := common.DecodeBtcAddress(testutils.RestrictedBtcAddressTest, common.BtcRegtestChain().ChainId)
 	if err != nil {

x/crosschain/keeper/evm_hooks_test.go

@@ -715,6 +715,28 @@ func TestKeeper_ProcessLogs(t *testing.T) {
 		require.Len(t, cctxList, 0)
 	})
 
+	t.Run("error returned for invalid event data", func(t *testing.T) {
+		k, ctx, sdkk, zk := keepertest.CrosschainKeeper(t)
+		k.GetAuthKeeper().GetModuleAccount(ctx, fungibletypes.ModuleName)
+
+		// use the wrong (testnet) chain ID to make the btc address parsing fail
+		chain := common.BtcTestNetChain()
+		chainID := chain.ChainId
+		setSupportedChain(ctx, zk, chainID)
+		SetupStateForProcessLogs(t, ctx, k, zk, sdkk, chain)
+
+		block := sample.GetInvalidZRC20WithdrawToExternal(t)
+		gasZRC20 := setupGasCoin(t, ctx, zk.FungibleKeeper, sdkk.EvmKeeper, chainID, "bitcoin", "BTC")
+		for _, log := range block.Logs {
+			log.Address = gasZRC20
+		}
+
+		err := k.ProcessLogs(ctx, block.Logs, sample.EthAddress(), "")
+		require.ErrorContains(t, err, "ParseZRC20WithdrawalEvent: invalid address")
+		cctxList := k.GetAllCrossChainTx(ctx)
+		require.Len(t, cctxList, 0)
+	})
+
 	t.Run("error returned if unable to process an event", func(t *testing.T) {
 		k, ctx, sdkk, zk := keepertest.CrosschainKeeper(t)
 		k.GetAuthKeeper().GetModuleAccount(ctx, fungibletypes.ModuleName)

zetaclient/bitcoin/bitcoin_client_test.go

@@ -41,6 +41,22 @@ func MockBTCClientMainnet() *BTCChainClient {
 	}
 }
 
+// createRPCClientAndLoadTx is a helper function to load raw tx and feed it to mock rpc client
+func createRPCClientAndLoadTx(chainId int64, txHash string) *stub.MockBTCRPCClient {
+	// file name for the archived MsgTx
+	nameMsgTx := path.Join("../", testutils.TestDataPathBTC, testutils.FileNameBTCMsgTx(chainId, txHash))
+
+	// load archived MsgTx
+	var msgTx wire.MsgTx
+	testutils.LoadObjectFromJSONFile(&msgTx, nameMsgTx)
+	tx := btcutil.NewTx(&msgTx)
+
+	// feed tx to mock rpc client
+	rpcClient := stub.NewMockBTCRPCClient()
+	rpcClient.WithRawTransaction(tx)
+	return rpcClient
+}
+
 func TestNewBitcoinClient(t *testing.T) {
 	t.Run("should return error because zetacore doesn't update core context", func(t *testing.T) {
 		cfg := config.NewConfig()
@@ -357,22 +373,6 @@ func TestCheckTSSVoutCancelled(t *testing.T) {
 	})
 }
 
-// createRPCClientAndLoadTx is a helper function to load raw tx and feed it to mock rpc client
-func createRPCClientAndLoadTx(chainId int64, txHash string) *stub.MockBTCRPCClient {
-	// file name for the archived MsgTx
-	nameMsgTx := path.Join("../", testutils.TestDataPathBTC, testutils.FileNameBTCMsgTx(chainId, txHash))
-
-	// load archived MsgTx
-	var msgTx wire.MsgTx
-	testutils.LoadObjectFromJSONFile(&msgTx, nameMsgTx)
-	tx := btcutil.NewTx(&msgTx)
-
-	// feed tx to mock rpc client
-	rpcClient := stub.NewMockBTCRPCClient()
-	rpcClient.WithRawTransaction(tx)
-	return rpcClient
-}
-
 func TestGetSenderAddressByVin(t *testing.T) {
 	chain := common.BtcMainnetChain()
 	net := &chaincfg.MainNetParams

zetaclient/bitcoin/bitcoin_signer.go

@@ -98,7 +98,10 @@ func (signer *BTCSigner) GetERC20CustodyAddress() ethcommon.Address {
 	return ethcommon.Address{}
 }
 
-// AddWithdrawTxOutputs adds the outputs to the withdraw tx
+// AddWithdrawTxOutputs adds the 3 outputs to the withdraw tx
+// 1st output: the nonce-mark btc to TSS itself
+// 2nd output: the payment to the recipient
+// 3rd output: the remaining btc to TSS itself
 func (signer *BTCSigner) AddWithdrawTxOutputs(
 	tx *wire.MsgTx,
 	to btcutil.Address,
@@ -200,7 +203,10 @@ func (signer *BTCSigner) SignWithdrawTx(
 
 	// size checking
 	// #nosec G701 always positive
-	txSize := EstimateOuttxSize(uint64(len(prevOuts)), []btcutil.Address{to})
+	txSize, err := EstimateOuttxSize(uint64(len(prevOuts)), []btcutil.Address{to})
+	if err != nil {
+		return nil, err
+	}
 	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)
 	}

zetaclient/bitcoin/bitcoin_signer_test.go

@@ -292,6 +292,20 @@ func TestAddWithdrawTxOutputs(t *testing.T) {
 				{Value: 80000000, PkScript: tssScript},
 			},
 		},
+		{
+			name:   "should add outputs without change successfully",
+			tx:     wire.NewMsgTx(wire.TxVersion),
+			to:     to,
+			total:  0.20012000,
+			amount: 0.2,
+			nonce:  10000,
+			fees:   big.NewInt(2000),
+			fail:   false,
+			txout: []*wire.TxOut{
+				{Value: 10000, PkScript: tssScript},
+				{Value: 20000000, PkScript: toScript},
+			},
+		},
 		{
 			name:     "should cancel tx successfully",
 			tx:       wire.NewMsgTx(wire.TxVersion),

zetaclient/bitcoin/fee.go

@@ -9,19 +9,17 @@ import (
 	"github.com/btcsuite/btcd/blockchain"
 	"github.com/btcsuite/btcd/btcjson"
 	"github.com/btcsuite/btcd/chaincfg"
+	"github.com/btcsuite/btcd/wire"
 	"github.com/btcsuite/btcutil"
+	"github.com/pkg/errors"
 	"github.com/rs/zerolog"
 	"github.com/zeta-chain/zetacore/common"
 	"github.com/zeta-chain/zetacore/common/bitcoin"
 	clientcommon "github.com/zeta-chain/zetacore/zetaclient/common"
-
-	"github.com/btcsuite/btcd/wire"
-	"github.com/pkg/errors"
 )
 
 const (
 	bytesPerKB              = 1000
-	bytesEmptyTx            = 10  // an empty tx is 10 bytes
 	bytesPerInput           = 41  // each input is 41 bytes
 	bytesPerOutputP2TR      = 43  // each P2TR output is 43 bytes
 	bytesPerOutputP2WSH     = 43  // each P2WSH output is 43 bytes
@@ -39,19 +37,16 @@ const (
 )
 
 var (
-	BtcOutTxBytesDepositor  uint64
-	BtcOutTxBytesWithdrawer uint64
-	DefaultDepositorFee     float64
-)
+	// The outtx size incurred by the depositor: 68vB
+	BtcOutTxBytesDepositor = OuttxSizeDepositor()
 
-func init() {
-	BtcOutTxBytesDepositor = OuttxSizeDepositor()   // 68vB, the outtx size incurred by the depositor
-	BtcOutTxBytesWithdrawer = OuttxSizeWithdrawer() // 177vB, the outtx size incurred by the withdrawer
+	// The outtx size incurred by the withdrawer: 177vB
+	BtcOutTxBytesWithdrawer = OuttxSizeWithdrawer()
 
+	// The default depositor fee is 0.00001360 BTC (20 * 68vB / 100000000)
 	// default 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.
-	DefaultDepositorFee = DepositorFee(defaultDepositorFeeRate) // 0.00001360 (20 * 68vB / 100000000)
-}
+	DefaultDepositorFee = DepositorFee(defaultDepositorFeeRate)
+)
 
 // FeeRateToSatPerByte converts a fee rate in BTC/KB to sat/byte.
 func FeeRateToSatPerByte(rate float64) *big.Int {
@@ -69,9 +64,9 @@ func WiredTxSize(numInputs uint64, numOutputs uint64) uint64 {
 }
 
 // EstimateOuttxSize estimates the size of a outtx in vBytes
-func EstimateOuttxSize(numInputs uint64, payees []btcutil.Address) uint64 {
+func EstimateOuttxSize(numInputs uint64, payees []btcutil.Address) (uint64, error) {
 	if numInputs == 0 {
-		return 0
+		return 0, nil
 	}
 	// #nosec G701 always positive
 	numOutputs := 2 + uint64(len(payees))
@@ -82,45 +77,49 @@ func EstimateOuttxSize(numInputs uint64, payees []btcutil.Address) uint64 {
 	// calculate the size of the outputs to payees
 	bytesToPayees := uint64(0)
 	for _, to := range payees {
-		bytesToPayees += GetOutputSizeByAddress(to)
+		sizeOutput, err := GetOutputSizeByAddress(to)
+		if err != nil {
+			return 0, err
+		}
+		bytesToPayees += sizeOutput
 	}
 	// calculate the size of the witness
 	bytesWitness := bytes1stWitness + (numInputs-1)*bytesPerWitness
 	// 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 + bytesToPayees + bytesWitness/blockchain.WitnessScaleFactor
+	return bytesWiredTx + bytesInput + bytesOutput + bytesToPayees + bytesWitness/blockchain.WitnessScaleFactor, nil
 }
 
 // GetOutputSizeByAddress returns the size of a tx output in bytes by the given address
-func GetOutputSizeByAddress(to btcutil.Address) uint64 {
+func GetOutputSizeByAddress(to btcutil.Address) (uint64, error) {
 	switch addr := to.(type) {
 	case *bitcoin.AddressTaproot:
 		if addr == nil {
-			return 0
+			return 0, nil
 		}
-		return bytesPerOutputP2TR
+		return bytesPerOutputP2TR, nil
 	case *btcutil.AddressWitnessScriptHash:
 		if addr == nil {
-			return 0
+			return 0, nil
 		}
-		return bytesPerOutputP2WSH
+		return bytesPerOutputP2WSH, nil
 	case *btcutil.AddressWitnessPubKeyHash:
 		if addr == nil {
-			return 0
+			return 0, nil
 		}
-		return bytesPerOutputP2WPKH
+		return bytesPerOutputP2WPKH, nil
 	case *btcutil.AddressScriptHash:
 		if addr == nil {
-			return 0
+			return 0, nil
 		}
-		return bytesPerOutputP2SH
+		return bytesPerOutputP2SH, nil
 	case *btcutil.AddressPubKeyHash:
 		if addr == nil {
-			return 0
+			return 0, nil
 		}
-		return bytesPerOutputP2PKH
+		return bytesPerOutputP2PKH, nil
 	default:
-		return bytesPerOutputP2WPKH
+		return 0, fmt.Errorf("cannot get output size for address type %T", to)
 	}
 }
 
@@ -207,18 +206,12 @@ func CalcBlockAvgFeeRate(blockVb *btcjson.GetBlockVerboseTxResult, netParams *ch
 
 // CalcDepositorFee calculates the depositor fee for a given block
 func CalcDepositorFee(blockVb *btcjson.GetBlockVerboseTxResult, chainID int64, netParams *chaincfg.Params, logger zerolog.Logger) float64 {
-	// use dynamic fee or default
-	dynamicFee := true
-
 	// use default fee for regnet
 	if common.IsBitcoinRegnet(chainID) {
-		dynamicFee = false
+		return DefaultDepositorFee
 	}
 	// mainnet dynamic fee takes effect only after a planned upgrade height
 	if common.IsBitcoinMainnet(chainID) && blockVb.Height < DynamicDepositorFeeHeight {
-		dynamicFee = false
-	}
-	if !dynamicFee {
 		return DefaultDepositorFee
 	}