feat: make Bitcoin deposit with invalid memo reverting (#3615)

* move new inbound function

* revert invalid memo cct

* add e2e test

* add test with invalid memo

* changelog

* fix unit test

* e2e tests

* generate

* reduce tests run

* make generate

changelog.md

@@ -27,6 +27,7 @@
 * [3600](https://github.com/zeta-chain/node/pull/3600) - add dedicated zetaclient restricted addresses config. This file will be automatically reloaded when it changes without needing to restart zetaclient.
 * [3578](https://github.com/zeta-chain/node/pull/3578) - Add disable_tss_block_scan parameter. This parameter will be used to disable expensive block scanning actions on non-ethereum EVM Chains.
 * [3551](https://github.com/zeta-chain/node/pull/3551) - support for EVM chain and Bitcoin chain inbound fast confirmation
+* [3615](https://github.com/zeta-chain/node/pull/3615) - make Bitcoin deposit with invalid memo reverting
 
 ### Refactor
 

cmd/zetae2e/local/bitcoin.go

@@ -30,6 +30,7 @@ func startBitcoinTests(
 		e2etests.TestBitcoinDepositFastConfirmationName,
 		e2etests.TestBitcoinDepositAndCallName,
 		e2etests.TestBitcoinDepositAndCallRevertName,
+		e2etests.TestBitcoinDepositInvalidMemoRevertName,
 		e2etests.TestBitcoinStdMemoDepositName,
 		e2etests.TestBitcoinStdMemoDepositAndCallName,
 		e2etests.TestBitcoinStdMemoDepositAndCallRevertName,

docs/openapi/openapi.swagger.yaml

@@ -57685,11 +57685,13 @@ definitions:
       - SUCCESS
       - INSUFFICIENT_DEPOSITOR_FEE
       - INVALID_RECEIVER_ADDRESS
+      - INVALID_MEMO
     default: SUCCESS
     description: |-
       - INSUFFICIENT_DEPOSITOR_FEE: this field is specifically for Bitcoin when the deposit amount is less than
       depositor fee
        - INVALID_RECEIVER_ADDRESS: the receiver address parsed from the inbound is invalid
+       - INVALID_MEMO: parse memo is invalid
     title: InboundStatus represents the status of an observed inbound
   zetachain.zetacore.crosschain.InboundTracker:
     type: object

e2e/e2etests/e2etests.go

@@ -119,6 +119,7 @@ const (
 	TestBitcoinWithdrawP2SHName                            = "bitcoin_withdraw_p2sh"
 	TestBitcoinWithdrawInvalidAddressName                  = "bitcoin_withdraw_invalid"
 	TestBitcoinWithdrawRestrictedName                      = "bitcoin_withdraw_restricted"
+	TestBitcoinDepositInvalidMemoRevertName                = "bitcoin_deposit_invalid_memo_revert"
 
 	/*
 	 Application tests
@@ -908,6 +909,13 @@ var AllE2ETests = []runner.E2ETest{
 		},
 		TestBitcoinWithdrawRestricted,
 	),
+	runner.NewE2ETest(
+		TestBitcoinDepositInvalidMemoRevertName,
+		"deposit Bitcoin with invalid memo; expect revert",
+		[]runner.ArgDefinition{},
+		TestBitcoinDepositInvalidMemoRevert,
+		runner.WithMinimumVersion("v29.0.0"),
+	),
 	/*
 	 Application tests
 	*/

e2e/e2etests/test_bitcoin_deposit_abort_with_low_deposit_fee.go

@@ -6,7 +6,7 @@ import (
 
 	"github.com/zeta-chain/node/e2e/runner"
 	"github.com/zeta-chain/node/e2e/utils"
-	"github.com/zeta-chain/node/x/crosschain/types"
+	crosschaintypes "github.com/zeta-chain/node/x/crosschain/types"
 	zetabitcoin "github.com/zeta-chain/node/zetaclient/chains/bitcoin/common"
 )
 
@@ -30,5 +30,7 @@ func TestBitcoinDepositAndAbortWithLowDepositFee(r *runner.E2ERunner, args []str
 	require.Equal(r, cctx.GetCurrentOutboundParam().Amount.Uint64(), uint64(0))
 
 	// check cctx error message
-	require.Contains(r, cctx.CctxStatus.StatusMessage, types.InboundStatus_INSUFFICIENT_DEPOSITOR_FEE.String())
+	require.Contains(r, cctx.CctxStatus.StatusMessage, "inbound observation failed")
+	require.Contains(r, cctx.CctxStatus.ErrorMessage, "insufficient depositor fee")
+	require.EqualValues(r, crosschaintypes.InboundStatus_INSUFFICIENT_DEPOSITOR_FEE, cctx.InboundParams.Status)
 }

e2e/e2etests/test_bticoin_deposit_invalid_memo_revert.go

@@ -0,0 +1,35 @@
+package e2etests
+
+import (
+	"github.com/stretchr/testify/require"
+
+	"github.com/zeta-chain/node/e2e/runner"
+	"github.com/zeta-chain/node/e2e/utils"
+	crosschaintypes "github.com/zeta-chain/node/x/crosschain/types"
+)
+
+func TestBitcoinDepositInvalidMemoRevert(r *runner.E2ERunner, args []string) {
+	require.Len(r, args, 0)
+
+	// make a deposit with a empty memo
+	utxos := r.ListDeployerUTXOs()
+	txHash, err := r.SendToTSSFromDeployerWithMemo(0.1, utxos[:1], []byte{})
+	require.NoError(r, err)
+
+	// wait for the cctx to be mined
+	cctx := utils.WaitCctxMinedByInboundHash(r.Ctx, txHash.String(), r.CctxClient, r.Logger, r.CctxTimeout)
+	r.Logger.CCTX(*cctx, "deposit empty memo")
+	utils.RequireCCTXStatus(r, cctx, crosschaintypes.CctxStatus_Reverted)
+	require.EqualValues(r, crosschaintypes.InboundStatus_INVALID_MEMO, cctx.InboundParams.Status)
+
+	// make a deposit with an invalid memo
+	utxos = r.ListDeployerUTXOs()
+	txHash, err = r.SendToTSSFromDeployerWithMemo(0.1, utxos[:1], []byte("invalid memo"))
+	require.NoError(r, err)
+
+	// wait for the cctx to be mined
+	cctx = utils.WaitCctxMinedByInboundHash(r.Ctx, txHash.String(), r.CctxClient, r.Logger, r.CctxTimeout)
+	r.Logger.CCTX(*cctx, "deposit invalid memo")
+	utils.RequireCCTXStatus(r, cctx, crosschaintypes.CctxStatus_Reverted)
+	require.EqualValues(r, crosschaintypes.InboundStatus_INVALID_MEMO, cctx.InboundParams.Status)
+}

proto/zetachain/zetacore/crosschain/cross_chain_tx.proto

@@ -42,6 +42,8 @@ enum InboundStatus {
   INSUFFICIENT_DEPOSITOR_FEE = 1;
   // the receiver address parsed from the inbound is invalid
   INVALID_RECEIVER_ADDRESS = 2;
+  // parse memo is invalid
+  INVALID_MEMO = 3;
 }
 
 message InboundParams {

testutil/sample/crosschain.go

@@ -172,14 +172,14 @@ func InboundParams(r *rand.Rand) *types.InboundParams {
 		Sender:                 EthAddress().String(),
 		SenderChainId:          r.Int63(),
 		TxOrigin:               EthAddress().String(),
-		CoinType:               coin.CoinType(r.Intn(100)),
+		CoinType:               coin.CoinType_Gas,
 		Asset:                  StringRandom(r, 32),
 		Amount:                 sdkmath.NewUint(uint64(r.Int63())),
 		ObservedHash:           StringRandom(r, 32),
 		ObservedExternalHeight: r.Uint64(),
 		BallotIndex:            StringRandom(r, 32),
 		FinalizedZetaHeight:    r.Uint64(),
-		Status:                 InboundStatusFromRand(r),
+		Status:                 types.InboundStatus_SUCCESS,
 		ConfirmationMode:       ConfirmationModeFromRand(r),
 	}
 }

typescript/zetachain/zetacore/crosschain/cross_chain_tx_pb.d.ts

@@ -124,6 +124,13 @@ export declare enum InboundStatus {
    * @generated from enum value: INVALID_RECEIVER_ADDRESS = 2;
    */
   INVALID_RECEIVER_ADDRESS = 2,
+
+  /**
+   * parse memo is invalid
+   *
+   * @generated from enum value: INVALID_MEMO = 3;
+   */
+  INVALID_MEMO = 3,
 }
 
 /**

x/crosschain/keeper/cctx_gateway_zevm.go

@@ -1,6 +1,7 @@
 package keeper
 
 import (
+	"errors"
 	"fmt"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -26,36 +27,46 @@ func (c CCTXGatewayZEVM) InitiateOutbound(
 	ctx sdk.Context,
 	config InitiateOutboundConfig,
 ) (newCCTXStatus types.CctxStatus, err error) {
-	// abort if CCTX inbound observation status indicates failure
-	// this is specifically for Bitcoin inbound error 'INSUFFICIENT_DEPOSITOR_FEE'
-	if config.CCTX.InboundParams.Status == types.InboundStatus_INSUFFICIENT_DEPOSITOR_FEE {
+	switch config.CCTX.InboundParams.Status {
+	case types.InboundStatus_INSUFFICIENT_DEPOSITOR_FEE:
+		// abort if CCTX has insufficient depositor fee for Bitcoin, the CCTX can't be reverted in this case
+		// because there is no fund to pay for the revert tx
 		c.crosschainKeeper.ProcessAbort(ctx, config.CCTX, types.StatusMessages{
-			StatusMessage: fmt.Sprintf(
-				"observation failed, reason %s",
-				types.InboundStatus_INSUFFICIENT_DEPOSITOR_FEE.String(),
-			),
+			ErrorMessageOutbound: "insufficient depositor fee",
+			StatusMessage:        "inbound observation failed",
 		})
 		return types.CctxStatus_Aborted, nil
-	}
+	case types.InboundStatus_INVALID_MEMO:
+		// when invalid memo is reported, the CCTX is reverted to the sender
+		newCCTXStatus = c.crosschainKeeper.ValidateOutboundZEVM(ctx, config.CCTX, errors.New("invalid memo"), true)
+		return newCCTXStatus, nil
+	case types.InboundStatus_SUCCESS:
+		// process the deposit normally
+		tmpCtx, commit := ctx.CacheContext()
+		isContractReverted, err := c.crosschainKeeper.HandleEVMDeposit(tmpCtx, config.CCTX)
+
+		if err != nil && !isContractReverted {
+			// exceptional case; internal error; should abort CCTX
+			// use ctx as tmpCtx is dismissed to not save any side effects performed during the evm deposit
+			c.crosschainKeeper.ProcessAbort(ctx, config.CCTX, types.StatusMessages{
+				StatusMessage:        "outbound failed but the universal contract did not revert",
+				ErrorMessageOutbound: cctxerror.NewCCTXErrorJSONMessage("failed to deposit tokens in ZEVM", err),
+			})
+			return types.CctxStatus_Aborted, err
+		}
 
-	// process the deposit
-	tmpCtx, commit := ctx.CacheContext()
-	isContractReverted, err := c.crosschainKeeper.HandleEVMDeposit(tmpCtx, config.CCTX)
+		newCCTXStatus = c.crosschainKeeper.ValidateOutboundZEVM(ctx, config.CCTX, err, isContractReverted)
+		if newCCTXStatus == types.CctxStatus_OutboundMined || newCCTXStatus == types.CctxStatus_PendingRevert {
+			commit()
+		}
 
-	if err != nil && !isContractReverted {
-		// exceptional case; internal error; should abort CCTX
-		// use ctx as tmpCtx is dismissed to not save any side effects performed during the evm deposit
+		return newCCTXStatus, nil
+	default:
+		// unknown observation status, abort the CCTX
 		c.crosschainKeeper.ProcessAbort(ctx, config.CCTX, types.StatusMessages{
-			StatusMessage:        "outbound failed but the universal contract did not revert",
-			ErrorMessageOutbound: cctxerror.NewCCTXErrorJSONMessage("failed to deposit tokens in ZEVM", err),
+			ErrorMessageOutbound: fmt.Sprintf("invalid observation status %d", config.CCTX.InboundParams.Status),
+			StatusMessage:        "inbound observation failed",
 		})
-		return types.CctxStatus_Aborted, err
-	}
-
-	newCCTXStatus = c.crosschainKeeper.ValidateOutboundZEVM(ctx, config.CCTX, err, isContractReverted)
-	if newCCTXStatus == types.CctxStatus_OutboundMined || newCCTXStatus == types.CctxStatus_PendingRevert {
-		commit()
+		return types.CctxStatus_Aborted, nil
 	}
-
-	return newCCTXStatus, nil
 }

x/crosschain/keeper/cctx_gateway_zevm_test.go

@@ -1,6 +1,9 @@
 package keeper_test
 
 import (
+	ethcommon "github.com/ethereum/go-ethereum/common"
+	fungibletypes "github.com/zeta-chain/node/x/fungible/types"
+	observertypes "github.com/zeta-chain/node/x/observer/types"
 	"math/big"
 	"strings"
 	"testing"
@@ -82,6 +85,85 @@ func TestKeeper_InitiateOutboundZEVM(t *testing.T) {
 		require.Equal(t, types.CctxStatus_Aborted, newStatus)
 	})
 
+	t.Run("should return aborted status on unknown inbound status", func(t *testing.T) {
+		// ARRANGE
+		k, ctx, _, _ := keepertest.CrosschainKeeperWithMocks(t, keepertest.CrosschainMockOptions{
+			UseFungibleMock: true,
+		})
+		gatewayZEVM := keeper.NewCCTXGatewayZEVM(*k)
+
+		// mock up CCTX data
+		cctx := sample.CrossChainTx(t, "test")
+		cctx.CctxStatus = &types.Status{Status: types.CctxStatus_PendingOutbound}
+		cctx.InboundParams.Status = types.InboundStatus(1000)
+
+		// ACT
+		// call InitiateOutbound
+		newStatus, err := gatewayZEVM.InitiateOutbound(
+			ctx,
+			keeper.InitiateOutboundConfig{CCTX: cctx, ShouldPayGas: true},
+		)
+
+		// ASSERT
+		require.NoError(t, err)
+		require.Equal(t, types.CctxStatus_Aborted, cctx.CctxStatus.Status)
+		require.Equal(t, types.CctxStatus_Aborted, newStatus)
+	})
+
+	t.Run("should return reverted status on invalid memo inbound status", func(t *testing.T) {
+		// ARRANGE
+		k, ctx, _, _ := keepertest.CrosschainKeeperWithMocks(t, keepertest.CrosschainMockOptions{
+			UseFungibleMock: true,
+			UseObserverMock: true,
+		})
+		gatewayZEVM := keeper.NewCCTXGatewayZEVM(*k)
+
+		k.SetGasPrice(ctx, types.GasPrice{
+			ChainId:     1,
+			MedianIndex: 0,
+			Prices:      []uint64{1},
+		})
+
+		//mock necessary calls made during creation of the revert outbound
+		observerMock := keepertest.GetCrosschainObserverMock(t, k)
+		fungibleMock := keepertest.GetCrosschainFungibleMock(t, k)
+		observerMock.On("GetSupportedChainFromChainID", mock.Anything, mock.Anything).Return(sample.Chain(1), true)
+		fungibleMock.On("GetGasCoinForForeignCoin", mock.Anything, mock.Anything).
+			Return(fungibletypes.ForeignCoins{Zrc20ContractAddress: "0x"}, true)
+		fungibleMock.On("QueryGasLimit", mock.Anything, mock.Anything).Return(big.NewInt(1), nil)
+		fungibleMock.On("QuerySystemContractGasCoinZRC20", mock.Anything, mock.Anything).
+			Return(ethcommon.Address{}, nil)
+		fungibleMock.On("QuerySystemContractGasCoinZRC20", mock.Anything, mock.Anything).
+			Return(ethcommon.Address{}, nil)
+		fungibleMock.On("QueryProtocolFlatFee", mock.Anything, mock.Anything).Return(big.NewInt(1), nil)
+		observerMock.On("GetChainNonces", mock.Anything, mock.Anything).
+			Return(observertypes.ChainNonces{Nonce: 1}, true)
+		observerMock.On("GetTSS", mock.Anything).Return(observertypes.TSS{}, true)
+		observerMock.On("GetPendingNonces", mock.Anything, mock.Anything, mock.Anything).
+			Return(observertypes.PendingNonces{NonceHigh: int64(1)}, true)
+		observerMock.On("SetChainNonces", mock.Anything, mock.Anything)
+		observerMock.On("SetPendingNonces", mock.Anything, mock.Anything)
+
+		// mock up CCTX data
+		cctx := sample.CrossChainTx(t, "test")
+		cctx.CctxStatus = &types.Status{Status: types.CctxStatus_PendingOutbound}
+		cctx.InboundParams.Status = types.InboundStatus_INVALID_MEMO
+		cctx.InboundParams.SenderChainId = 1
+		cctx.OutboundParams = []*types.OutboundParams{cctx.OutboundParams[0]}
+
+		// ACT
+		// call InitiateOutbound
+		newStatus, err := gatewayZEVM.InitiateOutbound(
+			ctx,
+			keeper.InitiateOutboundConfig{CCTX: cctx, ShouldPayGas: true},
+		)
+
+		// ASSERT
+		require.NoError(t, err)
+		require.Equal(t, types.CctxStatus_PendingRevert, cctx.CctxStatus.Status)
+		require.Equal(t, types.CctxStatus_PendingRevert, newStatus)
+	})
+
 	t.Run(
 		"should return aborted status on 'error during deposit that is not smart contract revert'",
 		func(t *testing.T) {

x/crosschain/keeper/cctx_orchestrator_validate_outbound.go

@@ -36,9 +36,9 @@ func (k Keeper) ValidateOutboundZEVM(
 	ctx sdk.Context,
 	cctx *types.CrossChainTx,
 	depositErr error,
-	isContractReverted bool,
+	shouldRevert bool,
 ) (newCCTXStatus types.CctxStatus) {
-	if depositErr != nil && isContractReverted {
+	if depositErr != nil && shouldRevert {
 		tmpCtxRevert, commitRevert := ctx.CacheContext()
 		// contract call reverted; should refund via a revert tx
 		revertErr := k.processFailedOutboundOnExternalChain(