From a877b19c12a7525671952c3aee0b672356c37ece Mon Sep 17 00:00:00 2001
From: beer-1 <147697694+beer-1@users.noreply.github.com>
Date: Fri, 14 Jun 2024 13:52:48 +0900
Subject: [PATCH] fix: do not return error in ack and timeout (#204)

* do not return error in ack and timeout

* change error log
---
 x/ibc-hooks/move-hooks/ack.go     | 54 ++++++++++++++++++++-----------
 x/ibc-hooks/move-hooks/hooks.go   |  5 ++-
 x/ibc-hooks/move-hooks/timeout.go | 50 +++++++++++++++++++---------
 3 files changed, 72 insertions(+), 37 deletions(-)

diff --git a/x/ibc-hooks/move-hooks/ack.go b/x/ibc-hooks/move-hooks/ack.go
index 5b487a56..5eb1ed6f 100644
--- a/x/ibc-hooks/move-hooks/ack.go
+++ b/x/ibc-hooks/move-hooks/ack.go
@@ -27,26 +27,31 @@ func (h MoveHooks) onAckIcs20Packet(
 	if !isMoveRouted || hookData.AsyncCallback == nil {
 		return nil
 	} else if err != nil {
-		return err
+		h.moveKeeper.Logger(ctx).Error("failed to parse memo", "error", err)
+		return nil
 	}
 
+	// create a new cache context to ignore errors during
+	// the execution of the callback
+	cacheCtx, write := ctx.CacheContext()
+
 	callback := hookData.AsyncCallback
-	if allowed, err := h.checkACL(im, ctx, callback.ModuleAddress); err != nil {
-		return err
+	if allowed, err := h.checkACL(im, cacheCtx, callback.ModuleAddress); err != nil {
+		h.moveKeeper.Logger(cacheCtx).Error("failed to check ACL", "error", err)
+		return nil
 	} else if !allowed {
+		h.moveKeeper.Logger(cacheCtx).Error("failed to check ACL", "not allowed")
 		return nil
 	}
-
 	callbackIdBz, err := vmtypes.SerializeUint64(callback.Id)
 	if err != nil {
-		return err
+		return nil
 	}
 	successBz, err := vmtypes.SerializeBool(!isAckError(h.codec, acknowledgement))
 	if err != nil {
-		return err
+		return nil
 	}
-
-	_, err = h.execMsg(ctx, &movetypes.MsgExecute{
+	_, err = h.execMsg(cacheCtx, &movetypes.MsgExecute{
 		Sender:        data.Sender,
 		ModuleAddress: callback.ModuleAddress,
 		ModuleName:    callback.ModuleName,
@@ -55,9 +60,13 @@ func (h MoveHooks) onAckIcs20Packet(
 		Args:          [][]byte{callbackIdBz, successBz},
 	})
 	if err != nil {
-		return err
+		h.moveKeeper.Logger(cacheCtx).Error("failed to execute callback", "error", err)
+		return nil
 	}
 
+	// write the cache context only if the callback execution was successful
+	write()
+
 	return nil
 }
 
@@ -77,26 +86,31 @@ func (h MoveHooks) onAckIcs721Packet(
 	if !isMoveRouted || hookData.AsyncCallback == nil {
 		return nil
 	} else if err != nil {
-		return err
+		h.moveKeeper.Logger(ctx).Error("failed to parse memo", "error", err)
+		return nil
 	}
 
+	// create a new cache context to ignore errors during
+	// the execution of the callback
+	cacheCtx, write := ctx.CacheContext()
+
 	callback := hookData.AsyncCallback
-	if allowed, err := h.checkACL(im, ctx, callback.ModuleAddress); err != nil {
-		return err
+	if allowed, err := h.checkACL(im, cacheCtx, callback.ModuleAddress); err != nil {
+		h.moveKeeper.Logger(cacheCtx).Error("failed to check ACL", "error", err)
+		return nil
 	} else if !allowed {
+		h.moveKeeper.Logger(cacheCtx).Error("failed to check ACL", "not allowed")
 		return nil
 	}
-
 	callbackIdBz, err := vmtypes.SerializeUint64(callback.Id)
 	if err != nil {
-		return err
+		return nil
 	}
 	successBz, err := vmtypes.SerializeBool(!isAckError(h.codec, acknowledgement))
 	if err != nil {
-		return err
+		return nil
 	}
-
-	_, err = h.execMsg(ctx, &movetypes.MsgExecute{
+	_, err = h.execMsg(cacheCtx, &movetypes.MsgExecute{
 		Sender:        data.Sender,
 		ModuleAddress: callback.ModuleAddress,
 		ModuleName:    callback.ModuleName,
@@ -105,8 +119,12 @@ func (h MoveHooks) onAckIcs721Packet(
 		Args:          [][]byte{callbackIdBz, successBz},
 	})
 	if err != nil {
-		return err
+		h.moveKeeper.Logger(cacheCtx).Error("failed to execute callback", "error", err)
+		return nil
 	}
 
+	// write the cache context only if the callback execution was successful
+	write()
+
 	return nil
 }
diff --git a/x/ibc-hooks/move-hooks/hooks.go b/x/ibc-hooks/move-hooks/hooks.go
index 5eb74385..79465930 100644
--- a/x/ibc-hooks/move-hooks/hooks.go
+++ b/x/ibc-hooks/move-hooks/hooks.go
@@ -10,8 +10,6 @@ import (
 	ibchooks "github.com/initia-labs/initia/x/ibc-hooks"
 	movekeeper "github.com/initia-labs/initia/x/move/keeper"
 	movetypes "github.com/initia-labs/initia/x/move/types"
-
-	vmtypes "github.com/initia-labs/movevm/types"
 )
 
 var (
@@ -71,10 +69,11 @@ func (h MoveHooks) OnTimeoutPacketOverride(im ibchooks.IBCMiddleware, ctx sdk.Co
 }
 
 func (h MoveHooks) checkACL(im ibchooks.IBCMiddleware, ctx sdk.Context, addrStr string) (bool, error) {
-	vmAddr, err := vmtypes.NewAccountAddress(addrStr)
+	vmAddr, err := movetypes.AccAddressFromString(h.ac, addrStr)
 	if err != nil {
 		return false, err
 	}
+
 	sdkAddr := movetypes.ConvertVMAddressToSDKAddress(vmAddr)
 	return im.HooksKeeper.GetAllowed(ctx, sdkAddr)
 }
diff --git a/x/ibc-hooks/move-hooks/timeout.go b/x/ibc-hooks/move-hooks/timeout.go
index 4bd8670c..147b8f84 100644
--- a/x/ibc-hooks/move-hooks/timeout.go
+++ b/x/ibc-hooks/move-hooks/timeout.go
@@ -26,22 +26,27 @@ func (h MoveHooks) onTimeoutIcs20Packet(
 	if !isMoveRouted || hookData.AsyncCallback == nil {
 		return nil
 	} else if err != nil {
-		return err
+		h.moveKeeper.Logger(ctx).Error("failed to parse memo", "error", err)
+		return nil
 	}
 
+	// create a new cache context to ignore errors during
+	// the execution of the callback
+	cacheCtx, write := ctx.CacheContext()
+
 	callback := hookData.AsyncCallback
-	if allowed, err := h.checkACL(im, ctx, callback.ModuleAddress); err != nil {
-		return err
+	if allowed, err := h.checkACL(im, cacheCtx, callback.ModuleAddress); err != nil {
+		h.moveKeeper.Logger(cacheCtx).Error("failed to check ACL", "error", err)
+		return nil
 	} else if !allowed {
+		h.moveKeeper.Logger(cacheCtx).Error("failed to check ACL", "not allowed")
 		return nil
 	}
-
 	callbackIdBz, err := vmtypes.SerializeUint64(callback.Id)
 	if err != nil {
-		return err
+		return nil
 	}
-
-	_, err = h.execMsg(ctx, &movetypes.MsgExecute{
+	_, err = h.execMsg(cacheCtx, &movetypes.MsgExecute{
 		Sender:        data.Sender,
 		ModuleAddress: callback.ModuleAddress,
 		ModuleName:    callback.ModuleName,
@@ -50,9 +55,13 @@ func (h MoveHooks) onTimeoutIcs20Packet(
 		Args:          [][]byte{callbackIdBz},
 	})
 	if err != nil {
-		return err
+		h.moveKeeper.Logger(cacheCtx).Error("failed to execute callback", "error", err)
+		return nil
 	}
 
+	// write the cache context only if the callback execution was successful
+	write()
+
 	return nil
 }
 
@@ -71,22 +80,27 @@ func (h MoveHooks) onTimeoutIcs721Packet(
 	if !isMoveRouted || hookData.AsyncCallback == nil {
 		return nil
 	} else if err != nil {
-		return err
+		h.moveKeeper.Logger(ctx).Error("failed to parse memo", "error", err)
+		return nil
 	}
 
+	// create a new cache context to ignore errors during
+	// the execution of the callback
+	cacheCtx, write := ctx.CacheContext()
+
 	callback := hookData.AsyncCallback
-	if allowed, err := h.checkACL(im, ctx, callback.ModuleAddress); err != nil {
-		return err
+	if allowed, err := h.checkACL(im, cacheCtx, callback.ModuleAddress); err != nil {
+		h.moveKeeper.Logger(cacheCtx).Error("failed to check ACL", "error", err)
+		return nil
 	} else if !allowed {
+		h.moveKeeper.Logger(cacheCtx).Error("failed to check ACL", "not allowed")
 		return nil
 	}
-
 	callbackIdBz, err := vmtypes.SerializeUint64(callback.Id)
 	if err != nil {
-		return err
+		return nil
 	}
-
-	_, err = h.execMsg(ctx, &movetypes.MsgExecute{
+	_, err = h.execMsg(cacheCtx, &movetypes.MsgExecute{
 		Sender:        data.Sender,
 		ModuleAddress: callback.ModuleAddress,
 		ModuleName:    callback.ModuleName,
@@ -95,8 +109,12 @@ func (h MoveHooks) onTimeoutIcs721Packet(
 		Args:          [][]byte{callbackIdBz},
 	})
 	if err != nil {
-		return err
+		h.moveKeeper.Logger(cacheCtx).Error("failed to execute callback", "error", err)
+		return nil
 	}
 
+	// write the cache context only if the callback execution was successful
+	write()
+
 	return nil
 }