refactor: cctx validate outbound

changelog.md

@@ -21,6 +21,7 @@
 * [2279](https://github.com/zeta-chain/node/pull/2279) - add a CCTXGateway field to chain static data
 * [2275](https://github.com/zeta-chain/node/pull/2275) - add ChainInfo singleton state variable in authority
 * [2291](https://github.com/zeta-chain/node/pull/2291) - initialize cctx gateway interface
+* [2317](https://github.com/zeta-chain/node/pull/2317) - add ValidateOutbound method for cctx orchestrator
 * [2289](https://github.com/zeta-chain/node/pull/2289) - add an authorization list to keep track of all authorizations on the chain
 * [2305](https://github.com/zeta-chain/node/pull/2305) - add new messages `MsgAddAuthorization` and `MsgRemoveAuthorization` that can be used to update the authorization list
 * [2313](https://github.com/zeta-chain/node/pull/2313) - add `CheckAuthorization` function to replace the `IsAuthorized` function. The new function uses the authorization list to verify the signer's authorization.

x/crosschain/keeper/cctx_gateway_zevm.go

@@ -1,8 +1,6 @@
 package keeper
 
 import (
-	"fmt"
-
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
 	"github.com/zeta-chain/zetacore/x/crosschain/types"
@@ -20,81 +18,22 @@ func NewCCTXGatewayZEVM(crosschainKeeper Keeper) CCTXGatewayZEVM {
 	}
 }
 
-/*
-InitiateOutbound handles evm deposit and call ValidateOutbound.
-TODO (https://github.com/zeta-chain/node/issues/2278): move remaining of this comment to ValidateOutbound once it's added.
-
-  - If the deposit is successful, the CCTX status is changed to OutboundMined.
-
-  - If the deposit returns an internal error i.e if HandleEVMDeposit() returns an error, but isContractReverted is false, the CCTX status is changed to Aborted.
-
-  - If the deposit is reverted, the function tries to create a revert cctx with status PendingRevert.
-
-  - If the creation of revert tx also fails it changes the status to Aborted.
-
-Note : Aborted CCTXs are not refunded in this function. The refund is done using a separate refunding mechanism.
-We do not return an error from this function , as all changes need to be persisted to the state.
-Instead we use a temporary context to make changes and then commit the context on for the happy path ,i.e cctx is set to OutboundMined.
-New CCTX status after preprocessing is returned.
-*/
+// InitiateOutbound handles evm deposit and immediately validates pending outbound
 func (c CCTXGatewayZEVM) InitiateOutbound(ctx sdk.Context, cctx *types.CrossChainTx) (newCCTXStatus types.CctxStatus) {
 	tmpCtx, commit := ctx.CacheContext()
 	isContractReverted, err := c.crosschainKeeper.HandleEVMDeposit(tmpCtx, cctx)
 
-	// TODO (https://github.com/zeta-chain/node/issues/2278): further processing will be in validateOutbound(...), for now keeping it here
 	if err != nil && !isContractReverted {
 		// exceptional case; internal error; should abort CCTX
 		cctx.SetAbort(err.Error())
 		return types.CctxStatus_Aborted
-	} else if err != nil && isContractReverted {
-		// contract call reverted; should refund via a revert tx
-		revertMessage := err.Error()
-		senderChain := c.crosschainKeeper.zetaObserverKeeper.GetSupportedChainFromChainID(ctx, cctx.InboundParams.SenderChainId)
-		if senderChain == nil {
-			cctx.SetAbort(fmt.Sprintf("invalid sender chain id %d", cctx.InboundParams.SenderChainId))
-			return types.CctxStatus_Aborted
-		}
-		gasLimit, err := c.crosschainKeeper.GetRevertGasLimit(ctx, *cctx)
-		if err != nil {
-			cctx.SetAbort(fmt.Sprintf("revert gas limit error: %s", err.Error()))
-			return types.CctxStatus_Aborted
-		}
-		if gasLimit == 0 {
-			// use same gas limit of outbound as a fallback -- should not be required
-			gasLimit = cctx.GetCurrentOutboundParam().GasLimit
-		}
+	}
 
-		err = cctx.AddRevertOutbound(gasLimit)
-		if err != nil {
-			cctx.SetAbort(fmt.Sprintf("revert outbound error: %s", err.Error()))
-			return types.CctxStatus_Aborted
-		}
-		// we create a new cached context, and we don't commit the previous one with EVM deposit
-		tmpCtxRevert, commitRevert := ctx.CacheContext()
-		err = func() error {
-			err := c.crosschainKeeper.PayGasAndUpdateCctx(
-				tmpCtxRevert,
-				senderChain.ChainId,
-				cctx,
-				cctx.InboundParams.Amount,
-				false,
-			)
-			if err != nil {
-				return err
-			}
-			// Update nonce using senderchain id as this is a revert tx and would go back to the original sender
-			return c.crosschainKeeper.UpdateNonce(tmpCtxRevert, senderChain.ChainId, cctx)
-		}()
-		if err != nil {
-			cctx.SetAbort(fmt.Sprintf("deposit revert message: %s err : %s", revertMessage, err.Error()))
-			return types.CctxStatus_Aborted
-		}
-		commitRevert()
-		cctx.SetPendingRevert(revertMessage)
-		return types.CctxStatus_PendingRevert
+	cctx.SetPendingOutbound("")
+	newCCTXStatus = c.crosschainKeeper.ValidateOutboundZEVM(ctx, cctx, err, isContractReverted)
+	if newCCTXStatus == types.CctxStatus_OutboundMined {
+		commit()
 	}
-	// successful HandleEVMDeposit;
-	commit()
-	cctx.SetOutBoundMined("Remote omnichain contract call completed")
-	return types.CctxStatus_OutboundMined
+
+	return newCCTXStatus
 }

x/crosschain/keeper/process_outbound.go → x/crosschain/keeper/cctx_orchestrator.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 
 	cosmoserrors "cosmossdk.io/errors"
+	"cosmossdk.io/math"
 	tmbytes "github.com/cometbft/cometbft/libs/bytes"
 	tmtypes "github.com/cometbft/cometbft/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -17,9 +18,54 @@ import (
 	observertypes "github.com/zeta-chain/zetacore/x/observer/types"
 )
 
-// ProcessOutbound processes the finalization of an outbound transaction based on the ballot status
-// The state is committed only if the individual steps are successful
-func (k Keeper) ProcessOutbound(
+/*
+ValidateOutboundZEVM processes the finalization of an outbound transaction if receiver is ZEVM.
+It takes deposit error and information if contract revert happened during deposit, to make a decision:
+
+- If the deposit was successful, the CCTX status is changed to OutboundMined.
+
+- If the deposit returned an internal error, but isContractReverted is false, the CCTX status is changed to Aborted.
+
+- If the deposit is reverted, the function tries to create a revert cctx with status PendingRevert.
+
+- If the creation of revert tx also fails it changes the status to Aborted.
+
+Note : Aborted CCTXs are not refunded in this function. The refund is done using a separate refunding mechanism.
+We do not return an error from this function, as all changes need to be persisted to the state.
+Instead we use a temporary context to make changes and then commit the context on for the happy path, i.e cctx is set to OutboundMined.
+New CCTX status after preprocessing is returned.
+*/
+func (k Keeper) ValidateOutboundZEVM(
+	ctx sdk.Context,
+	cctx *types.CrossChainTx,
+	depositErr error,
+	isContractReverted bool,
+) (newCCTXStatus types.CctxStatus) {
+	if depositErr != nil && isContractReverted {
+		tmpCtxRevert, commitRevert := ctx.CacheContext()
+		// contract call reverted; should refund via a revert tx
+		err := k.validateFailedOutbound(
+			tmpCtxRevert,
+			cctx,
+			types.CctxStatus_PendingOutbound,
+			depositErr.Error(),
+			cctx.InboundParams.Amount,
+		)
+		if err != nil {
+			cctx.SetAbort(err.Error())
+			return types.CctxStatus_Aborted
+		}
+
+		commitRevert()
+		return types.CctxStatus_PendingRevert
+	}
+	k.validateSuccessfulOutbound(ctx, cctx, "", false)
+	return types.CctxStatus_OutboundMined
+}
+
+// ValidateOutboundObservers processes the finalization of an outbound transaction based on the ballot status.
+// The state is committed only if the individual steps are successful.
+func (k Keeper) ValidateOutboundObservers(
 	ctx sdk.Context,
 	cctx *types.CrossChainTx,
 	ballotStatus observertypes.BallotStatus,
@@ -29,9 +75,9 @@ func (k Keeper) ProcessOutbound(
 	err := func() error {
 		switch ballotStatus {
 		case observertypes.BallotStatus_BallotFinalized_SuccessObservation:
-			k.ProcessSuccessfulOutbound(tmpCtx, cctx, valueReceived)
+			k.validateSuccessfulOutbound(tmpCtx, cctx, valueReceived, true)
 		case observertypes.BallotStatus_BallotFinalized_FailureObservation:
-			err := k.ProcessFailedOutbound(tmpCtx, cctx, valueReceived)
+			err := k.validateFailedOutboundObservers(tmpCtx, cctx, valueReceived)
 			if err != nil {
 				return err
 			}
@@ -49,35 +95,7 @@ func (k Keeper) ProcessOutbound(
 	return nil
 }
 
-// ProcessSuccessfulOutbound processes a successful outbound transaction. It does the following things in one function:
-//
-//  1. Change the status of the CCTX from
-//     - PendingRevert to Reverted
-//     - PendingOutbound to OutboundMined
-//
-//  2. Set the finalization status of the current outbound tx to executed
-//
-//  3. Emit an event for the successful outbound transaction
-//
-// This function sets CCTX status , in cases where the outbound tx is successful, but tx itself fails
-// This is done because SaveSuccessfulOutbound does not set the cctx status
-// For cases where the outbound tx is unsuccessful, the cctx status is automatically set to Aborted in the ProcessFailedOutbound function, so we can just return and error to trigger that
-func (k Keeper) ProcessSuccessfulOutbound(ctx sdk.Context, cctx *types.CrossChainTx, valueReceived string) {
-	oldStatus := cctx.CctxStatus.Status
-	switch oldStatus {
-	case types.CctxStatus_PendingRevert:
-		cctx.SetReverted("Outbound succeeded, revert executed")
-	case types.CctxStatus_PendingOutbound:
-		cctx.SetOutBoundMined("Outbound succeeded, mined")
-	default:
-		return
-	}
-	cctx.GetCurrentOutboundParam().TxFinalizationStatus = types.TxFinalizationStatus_Executed
-	newStatus := cctx.CctxStatus.Status.String()
-	EmitOutboundSuccess(ctx, valueReceived, oldStatus.String(), newStatus, cctx.Index)
-}
-
-// ProcessFailedOutbound processes a failed outbound transaction. It does the following things in one function:
+// validateFailedOutboundObservers processes a failed outbound transaction for observers. It does the following things in one function:
 //
 // 1. For Admin Tx or a withdrawal from Zeta chain, it aborts the CCTX
 //
@@ -91,12 +109,12 @@ func (k Keeper) ProcessSuccessfulOutbound(ctx sdk.Context, cctx *types.CrossChai
 //
 // This function sets CCTX status , in cases where the outbound tx is successful, but tx itself fails
 // This is done because SaveSuccessfulOutbound does not set the cctx status
-// For cases where the outbound tx is unsuccessful, the cctx status is automatically set to Aborted in the ProcessFailedOutbound function, so we can just return and error to trigger that
-func (k Keeper) ProcessFailedOutbound(ctx sdk.Context, cctx *types.CrossChainTx, valueReceived string) error {
+// For cases where the outbound tx is unsuccessful, the cctx status is automatically set to Aborted in the validateFailedOutboundObservers function, so we can just return and error to trigger that
+func (k Keeper) validateFailedOutboundObservers(ctx sdk.Context, cctx *types.CrossChainTx, valueReceived string) error {
 	oldStatus := cctx.CctxStatus.Status
 	// The following logic is used to handler the mentioned conditions separately. The reason being
-	// All admin tx is created using a policy message , there is no associated inbound tx , therefore we do not need any revert logic
-	// For transactions which originated from ZEVM , we can process the outbound in the same block as there is no TSS signing required for the revert
+	// All admin tx is created using a policy message, there is no associated inbound tx, therefore we do not need any revert logic
+	// For transactions which originated from ZEVM, we can process the outbound in the same block as there is no TSS signing required for the revert
 	// For all other transactions we need to create a revert tx and set the status to pending revert
 
 	if cctx.InboundParams.CoinType == coin.CoinType_Cmd {
@@ -108,9 +126,9 @@ func (k Keeper) ProcessFailedOutbound(ctx sdk.Context, cctx *types.CrossChainTx,
 		// Try revert if the coin-type is ZETA
 		case coin.CoinType_Zeta:
 			{
-				err := k.processFailedOutboundForZEVM(ctx, cctx)
+				err := k.validateFailedOutboundObserversForZEVM(ctx, cctx)
 				if err != nil {
-					return cosmoserrors.Wrap(err, "ProcessFailedOutboundForZEVMTx")
+					return cosmoserrors.Wrap(err, "validateFailedOutboundObserversForZEVMTx")
 				}
 			}
 		// For all other coin-types, we do not revert, the cctx is aborted
@@ -121,24 +139,30 @@ func (k Keeper) ProcessFailedOutbound(ctx sdk.Context, cctx *types.CrossChainTx,
 			}
 		}
 	} else {
-		err := k.processFailedOutboundForExternalChainTx(ctx, cctx, oldStatus)
+		err := k.validateFailedOutbound(ctx, cctx, oldStatus, "", cctx.GetCurrentOutboundParam().Amount)
 		if err != nil {
-			return cosmoserrors.Wrap(err, "ProcessFailedOutBoundForExternalChainTx")
+			return cosmoserrors.Wrap(err, "validateFailedOutbound")
 		}
 	}
 	newStatus := cctx.CctxStatus.Status.String()
 	EmitOutboundFailure(ctx, valueReceived, oldStatus.String(), newStatus, cctx.Index)
 	return nil
 }
 
-// processFailedOutboundForExternalChainTx processes the failed outbound transaction for external chain tx
-func (k Keeper) processFailedOutboundForExternalChainTx(
+// validateFailedOutbound processes the failed outbound transaction
+func (k Keeper) validateFailedOutbound(
 	ctx sdk.Context,
 	cctx *types.CrossChainTx,
 	oldStatus types.CctxStatus,
+	revertMsg string,
+	inputAmount math.Uint, // TODO: find different way for this
 ) error {
 	switch oldStatus {
 	case types.CctxStatus_PendingOutbound:
+		senderChain := k.zetaObserverKeeper.GetSupportedChainFromChainID(ctx, cctx.InboundParams.SenderChainId)
+		if senderChain == nil {
+			return observertypes.ErrSupportedChains
+		}
 
 		gasLimit, err := k.GetRevertGasLimit(ctx, *cctx)
 		if err != nil {
@@ -159,7 +183,7 @@ func (k Keeper) processFailedOutboundForExternalChainTx(
 			ctx,
 			cctx.InboundParams.SenderChainId,
 			cctx,
-			cctx.OutboundParams[0].Amount,
+			inputAmount,
 			false,
 		)
 		if err != nil {
@@ -169,20 +193,58 @@ func (k Keeper) processFailedOutboundForExternalChainTx(
 		if err != nil {
 			return err
 		}
+		if revertMsg == "" {
+			revertMsg = "Outbound failed, start revert"
+		}
 		// Not setting the finalization status here, the required changes have been made while creating the revert tx
-		cctx.SetPendingRevert("Outbound failed, start revert")
+		cctx.SetPendingRevert(revertMsg)
 	case types.CctxStatus_PendingRevert:
 		cctx.GetCurrentOutboundParam().TxFinalizationStatus = types.TxFinalizationStatus_Executed
 		cctx.SetAbort("Outbound failed: revert failed; abort TX")
 	}
 	return nil
 }
 
-// processFailedOutboundForZEVM processes the failed outbound transaction for ZEVM
-func (k Keeper) processFailedOutboundForZEVM(ctx sdk.Context, cctx *types.CrossChainTx) error {
+// validateSuccessfulOutbound processes a successful outbound transaction. It does the following things in one function:
+//
+//  1. Change the status of the CCTX from
+//     - PendingRevert to Reverted
+//     - PendingOutbound to OutboundMined
+//
+//  2. Set the finalization status of the current outbound tx to executed
+//
+//  3. Emit an event for the successful outbound transaction if flag is provided
+//
+// This function sets CCTX status, in cases where the outbound tx is successful, but tx itself fails
+// This is done because SaveSuccessfulOutbound does not set the cctx status
+// For cases where the outbound tx is unsuccessful, the cctx status is automatically set to Aborted in the validateFailedOutboundObservers function, so we can just return and error to trigger that
+func (k Keeper) validateSuccessfulOutbound(
+	ctx sdk.Context,
+	cctx *types.CrossChainTx,
+	valueReceived string,
+	emitEvent bool,
+) {
+	oldStatus := cctx.CctxStatus.Status
+	switch oldStatus {
+	case types.CctxStatus_PendingRevert:
+		cctx.SetReverted("Outbound succeeded, revert executed")
+	case types.CctxStatus_PendingOutbound:
+		cctx.SetOutBoundMined("Outbound succeeded, mined")
+	default:
+		return
+	}
+	cctx.GetCurrentOutboundParam().TxFinalizationStatus = types.TxFinalizationStatus_Executed
+	newStatus := cctx.CctxStatus.Status.String()
+	if emitEvent {
+		EmitOutboundSuccess(ctx, valueReceived, oldStatus.String(), newStatus, cctx.Index)
+	}
+}
+
+// validateFailedOutboundObserversForZEVM processes the failed outbound transaction for ZEVM
+func (k Keeper) validateFailedOutboundObserversForZEVM(ctx sdk.Context, cctx *types.CrossChainTx) error {
 	indexBytes, err := cctx.GetCCTXIndexBytes()
 	if err != nil {
-		// Return err to save the failed outbound ad set to aborted
+		// Return err to save the failed outbound and set to aborted
 		return fmt.Errorf("failed reverting GetCCTXIndexBytes: %s", err.Error())
 	}
 	// Finalize the older outbound tx