fix: add error check in btc decode

changelog.md

@@ -33,6 +33,7 @@
 * [1733](https://github.com/zeta-chain/node/pull/1733) - remove the unnecessary 2x multiplier in the convertGasToZeta RPC
 * [1721](https://github.com/zeta-chain/node/issues/1721) - zetaclient should provide bitcoin_chain_id when querying TSS address
 * [1744](https://github.com/zeta-chain/node/pull/1744) - added cmd to encrypt tss keyshare file, allowing empty tss password for backward compatibility.
+* [1758](https://github.com/zeta-chain/node/pull/1758) - add error check for BTC decode function
 
 ### Tests
 

common/address.go

@@ -72,6 +72,9 @@ func DecodeBtcAddress(inputAddress string, chainID int64) (address btcutil.Addre
 		return nil, fmt.Errorf("chain params not found")
 	}
 	address, err = btcutil.DecodeAddress(inputAddress, chainParams)
+	if err != nil {
+		return nil, fmt.Errorf("decode address failed: %s , for input address %s", err.Error(), inputAddress)
+	}
 	ok := address.IsForNet(chainParams)
 	if !ok {
 		return nil, fmt.Errorf("address is not for network %s", chainParams.Name)

common/address_test.go

@@ -23,20 +23,41 @@ func TestAddress(t *testing.T) {
 
 func TestDecodeBtcAddress(t *testing.T) {
 	t.Run("invalid string", func(t *testing.T) {
-		_, err := DecodeBtcAddress("�U�ڷ���i߭����꿚�l", 18332)
+		_, err := DecodeBtcAddress("�U�ڷ���i߭����꿚�l", BtcTestNetChain().ChainId)
 		require.ErrorContains(t, err, "runtime error: index out of range")
 	})
 	t.Run("invalid chain", func(t *testing.T) {
 		_, err := DecodeBtcAddress("14CEjTd5ci3228J45GdnGeUKLSSeCWUQxK", 0)
-		require.ErrorContains(t, err, "is not a Bitcoin chain")
+		require.ErrorContains(t, err, "is not a bitcoin chain")
 	})
-	t.Run("nil pointer dereference", func(t *testing.T) {
-		_, err := DecodeBtcAddress("tb1qy9pqmk2pd9sv63g27jt8r657wy0d9uee4x2dt2", 18332)
-		require.ErrorContains(t, err, "runtime error: invalid memory address or nil pointer dereference")
+	t.Run("invalid checksum", func(t *testing.T) {
+		_, err := DecodeBtcAddress("tb1qy9pqmk2pd9sv63g27jt8r657wy0d9uee4x2dt2", BtcTestNetChain().ChainId)
+		require.ErrorContains(t, err, "invalid checksum")
 	})
-	t.Run("valid address", func(t *testing.T) {
-		_, err := DecodeBtcAddress("bcrt1qy9pqmk2pd9sv63g27jt8r657wy0d9uee4x2dt2", 18444)
+	t.Run("valid legacy main-net address address incorrect params TestNet", func(t *testing.T) {
+		_, err := DecodeBtcAddress("1EYVvXLusCxtVuEwoYvWRyN5EZTXwPVvo3", BtcTestNetChain().ChainId)
+		require.ErrorContains(t, err, "decode address failed")
+	})
+	t.Run("valid legacy main-net address address incorrect params RegTestNet", func(t *testing.T) {
+		_, err := DecodeBtcAddress("1EYVvXLusCxtVuEwoYvWRyN5EZTXwPVvo3", BtcRegtestChain().ChainId)
+		require.ErrorContains(t, err, "decode address failed")
+	})
+
+	t.Run("valid legacy main-net address address correct params", func(t *testing.T) {
+		_, err := DecodeBtcAddress("1EYVvXLusCxtVuEwoYvWRyN5EZTXwPVvo3", BtcMainnetChain().ChainId)
+		require.NoError(t, err)
+	})
+	t.Run("valid legacy testnet address with correct params", func(t *testing.T) {
+		_, err := DecodeBtcAddress("n2TCLD16i8SNjwPCcgGBkTEeG6CQAcYTN1", BtcTestNetChain().ChainId)
 		require.NoError(t, err)
 	})
 
+	t.Run("non legacy valid address with incorrect params", func(t *testing.T) {
+		_, err := DecodeBtcAddress("bcrt1qy9pqmk2pd9sv63g27jt8r657wy0d9uee4x2dt2", BtcMainnetChain().ChainId)
+		require.ErrorContains(t, err, "address is not for network mainnet")
+	})
+	t.Run("non legacy valid address with correct params", func(t *testing.T) {
+		_, err := DecodeBtcAddress("bcrt1qy9pqmk2pd9sv63g27jt8r657wy0d9uee4x2dt2", BtcRegtestChain().ChainId)
+		require.NoError(t, err)
+	})
 }

common/chain.go

@@ -191,7 +191,7 @@ func GetBTCChainParams(chainID int64) (*chaincfg.Params, error) {
 	case 8332:
 		return &chaincfg.MainNetParams, nil
 	default:
-		return nil, fmt.Errorf("error chainID %d is not a Bitcoin chain", chainID)
+		return nil, fmt.Errorf("error chainID %d is not a bitcoin chain", chainID)
 	}
 }
 
@@ -204,7 +204,7 @@ func GetBTCChainIDFromChainParams(params *chaincfg.Params) (int64, error) {
 	case chaincfg.MainNetParams.Name:
 		return 8332, nil
 	default:
-		return 0, fmt.Errorf("error chain %s is not a Bitcoin chain", params.Name)
+		return 0, fmt.Errorf("error chain %s is not a bitcoin chain", params.Name)
 	}
 }
 

x/crosschain/keeper/evm_hooks.go

@@ -55,6 +55,8 @@ func (k Keeper) PostTxProcessing(
 // ProcessLogs post-processes logs emitted by a zEVM contract; if the log contains Withdrawal event
 // from registered ZRC20 contract, new CCTX will be created to trigger and track outbound
 // transaction.
+// TODO: implement unit tests
+// https://github.com/zeta-chain/node/issues/1759
 func (k Keeper) ProcessLogs(ctx sdk.Context, logs []*ethtypes.Log, emittingContract ethcommon.Address, txOrigin string) error {
 	system, found := k.fungibleKeeper.GetSystemContract(ctx)
 	if !found {
@@ -71,7 +73,14 @@ func (k Keeper) ProcessLogs(ctx sdk.Context, logs []*ethtypes.Log, emittingContr
 			if err := k.ProcessZRC20WithdrawalEvent(ctx, eventWithdrawal, emittingContract, txOrigin); err != nil {
 				return err
 			}
+			// We were able to parse the ZRC20 withdrawal event. However, we were unable to process it as the information was incorrect
+			// This means that there are some funds locked in the contract which cannot have an outbound , this can be a candidate for a refund
+			// TODO : Consider returning error or auto refunding the funds to the user
+		} else if eventWithdrawal != nil {
+			ctx.Logger().Error(fmt.Sprintf("Error processing ZRC20 withdrawal event , from Address: %s m ,to : %s,value %s,gasfee %s, protocolfee %s, err %s",
+				eventWithdrawal.From.Hex(), string(eventWithdrawal.To), eventWithdrawal.Value.String(), eventWithdrawal.Gasfee.String(), eventWithdrawal.ProtocolFlatFee.String(), err.Error()))
 		}
+
 		eZeta, err := ParseZetaSentEvent(*log, connectorZEVMAddr)
 		if err == nil {
 			if err := k.ProcessZetaSentEvent(ctx, eZeta, emittingContract, txOrigin); err != nil {
@@ -88,7 +97,7 @@ func (k Keeper) ProcessZRC20WithdrawalEvent(ctx sdk.Context, event *zrc20.ZRC20W
 	if !k.zetaObserverKeeper.IsInboundEnabled(ctx) {
 		return types.ErrNotEnoughPermissions
 	}
-	ctx.Logger().Info("ZRC20 withdrawal to %s amount %d\n", hex.EncodeToString(event.To), event.Value)
+	ctx.Logger().Info(fmt.Sprintf("ZRC20 withdrawal to %s amount %d", hex.EncodeToString(event.To), event.Value))
 	tss, found := k.zetaObserverKeeper.GetTSS(ctx)
 	if !found {
 		return errorsmod.Wrap(types.ErrCannotFindTSSKeys, "ProcessZRC20WithdrawalEvent: cannot be processed without TSS keys")
@@ -137,7 +146,6 @@ func (k Keeper) ProcessZRC20WithdrawalEvent(ctx sdk.Context, event *zrc20.ZRC20W
 	// Get gas price and amount
 	gasprice, found := k.GetGasPrice(ctx, receiverChain.ChainId)
 	if !found {
-		fmt.Printf("gasprice not found for %s\n", receiverChain)
 		return fmt.Errorf("gasprice not found for %s", receiverChain)
 	}
 	cctx.GetCurrentOutTxParam().OutboundTxGasPrice = fmt.Sprintf("%d", gasprice.Prices[gasprice.MedianIndex])
@@ -257,24 +265,27 @@ func (k Keeper) ParseZRC20WithdrawalEvent(ctx sdk.Context, log ethtypes.Log) (*z
 	if err != nil {
 		return nil, err
 	}
-
+	// The event was parsed; that means the user has deposited tokens to the contract.
+	// We still need to check if the information entered by the user is correct.
+	// If the information is not correct, the outbound cctx will not be created.
 	coin, found := k.fungibleKeeper.GetForeignCoins(ctx, event.Raw.Address.Hex())
 	if !found {
-		return nil, fmt.Errorf("ParseZRC20WithdrawalEvent: cannot find foreign coin with contract address %s", event.Raw.Address.Hex())
+		return event, fmt.Errorf("ParseZRC20WithdrawalEvent: cannot find foreign coin with contract address %s", event.Raw.Address.Hex())
 	}
 	chainID := coin.ForeignChainId
 	if common.IsBitcoinChain(chainID) {
 		if event.Value.Cmp(big.NewInt(0)) <= 0 {
-			return nil, fmt.Errorf("ParseZRC20WithdrawalEvent: invalid amount %s", event.Value.String())
+			return event, fmt.Errorf("ParseZRC20WithdrawalEvent: invalid amount %s", event.Value.String())
 		}
 		addr, err := common.DecodeBtcAddress(string(event.To), chainID)
 		if err != nil {
-			return nil, fmt.Errorf("ParseZRC20WithdrawalEvent: invalid address %s: %s", event.To, err)
+			return event, fmt.Errorf("ParseZRC20WithdrawalEvent: invalid address %s: %s", event.To, err)
 		}
 		_, ok := addr.(*btcutil.AddressWitnessPubKeyHash)
 		if !ok {
-			return nil, fmt.Errorf("ParseZRC20WithdrawalEvent: invalid address %s (not P2WPKH address)", event.To)
+			return event, fmt.Errorf("ParseZRC20WithdrawalEvent: invalid address %s (not P2WPKH address)", event.To)
 		}
+
 	}
 	return event, nil
 }